Skip to content
This repository was archived by the owner on Nov 16, 2023. It is now read-only.

Update the permission check of add/delete item's categories and tags #312

Merged
yiyione merged 9 commits into from
Jun 21, 2021
Merged

Update the permission check of add/delete item's categories and tags #312

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
24b861c
update the permission check of add/delete item's tag & category
yiyione Jun 10, 2021
cdbaa7b
update the permission check of add/delete item's tag & category
yiyione Jun 10, 2021
5dfa0a3
update
yiyione Jun 10, 2021
5167afe
update
yiyione Jun 10, 2021
ed46180
Merge branch 'master' into yiyi/category_and_tag_permission
yiyione Jun 10, 2021
a8ef127
use Array.prototype.some() instead of ...map().includes()
yiyione Jun 10, 2021
7ce8630
move all OFFICIAL_EXAMPLE to a same place
yiyione Jun 11, 2021
61453a3
fix
yiyione Jun 11, 2021
5c34f2c
merge checkReadPermission/checkWritePermission
yiyione Jun 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 42 additions & 21 deletions rest_server/src/controllers/item_controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,25 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
const { isNil } = require('lodash');
const { MarketplaceItem } = require('../models');
const { MarketplaceItem, ItemCategory } = require('../models');
const asyncHandler = require('./async_handler');
const yaml = require('js-yaml');
const protocolValidator = require('../utils/protocol');
const error = require('../models/error');

function checkReadPermission(userInfo, item) {
const OFFICIAL_EXAMPLE = 'official example';

async function checkReadPermission(userInfo, item, categories) {
if (userInfo.admin === true) {
return true;
}
if (userInfo.username === item.author) {
if (categories === undefined) {
categories = await MarketplaceItem.getCategories(item);
}
if (
!categories.some(category => category.name === OFFICIAL_EXAMPLE) &&
userInfo.username === item.author
) {
return true;
}
if (item.isPublic) {
Expand All @@ -27,11 +35,17 @@ function checkReadPermission(userInfo, item) {
return false;
}

function checkWritePermission(tokenInfo, item) {
async function checkWritePermission(tokenInfo, item, categories) {
if (tokenInfo.admin === true) {
return true;
}
if (tokenInfo.username === item.author) {
if (categories === undefined) {
categories = await MarketplaceItem.getCategories(item);
}
if (
!categories.some(category => category.name === OFFICIAL_EXAMPLE) &&
tokenInfo.username === item.author
) {
return true;
}
return false;
Expand Down Expand Up @@ -96,7 +110,7 @@ const get = asyncHandler(async (req, res, next) => {
if (isNil(result)) {
return next(error.createNotFound());
} else {
if (checkReadPermission(req.userInfo, result)) {
if (await checkReadPermission(req.userInfo, result)) {
res.status(200).json(result);
} else {
return next(
Expand All @@ -114,7 +128,7 @@ const get = asyncHandler(async (req, res, next) => {
const update = asyncHandler(async (req, res, next) => {
try {
let result = await MarketplaceItem.get(req.params.itemId);
if (checkWritePermission(req.tokenInfo, result)) {
if (await checkWritePermission(req.tokenInfo, result)) {
result = await MarketplaceItem.update(req.params.itemId, req.body);
if (isNil(result)) {
return next(error.createNotFound());
Expand All @@ -136,7 +150,7 @@ const update = asyncHandler(async (req, res, next) => {
const del = asyncHandler(async (req, res, next) => {
try {
let result = await MarketplaceItem.get(req.params.itemId);
if (checkWritePermission(req.tokenInfo, result)) {
if (await checkWritePermission(req.tokenInfo, result)) {
result = await MarketplaceItem.del(req.params.itemId);
if (isNil(result)) {
return next(error.createNotFound());
Expand All @@ -158,7 +172,7 @@ const del = asyncHandler(async (req, res, next) => {
const listTags = asyncHandler(async (req, res, next) => {
try {
let result = await MarketplaceItem.get(req.params.itemId);
if (checkReadPermission(req.userInfo, result)) {
if (await checkReadPermission(req.userInfo, result)) {
result = await MarketplaceItem.getTags(result);
if (isNil(result)) {
return next(error.createNotFound());
Expand All @@ -180,7 +194,7 @@ const listTags = asyncHandler(async (req, res, next) => {
const addTag = asyncHandler(async (req, res, next) => {
try {
let result = await MarketplaceItem.get(req.params.itemId);
if (checkReadPermission(req.userInfo, result)) {
if (await checkWritePermission(req.tokenInfo, result)) {
result = await MarketplaceItem.addTag(result, req.params.tagId);
if (isNil(result)) {
return next(error.createNotFound());
Expand All @@ -202,7 +216,7 @@ const addTag = asyncHandler(async (req, res, next) => {
const deleteTag = asyncHandler(async (req, res, next) => {
try {
let result = await MarketplaceItem.get(req.params.itemId);
if (checkReadPermission(req.userInfo, result)) {
if (await checkWritePermission(req.tokenInfo, result)) {
result = await MarketplaceItem.deleteTag(result, req.params.tagId);
if (isNil(result)) {
return next(error.createNotFound());
Expand All @@ -223,14 +237,13 @@ const deleteTag = asyncHandler(async (req, res, next) => {

const listCategories = asyncHandler(async (req, res, next) => {
try {
let result = await MarketplaceItem.get(req.params.itemId);
if (checkReadPermission(req.userInfo, result)) {
result = await MarketplaceItem.getCategories(result);
if (isNil(result)) {
return next(error.createNotFound());
} else {
res.status(200).json(result);
}
const result = await MarketplaceItem.get(req.params.itemId);
const categories = await MarketplaceItem.getCategories(result);
if (isNil(result)) {
return next(error.createNotFound());
}
if (await checkReadPermission(req.userInfo, result, categories)) {
res.status(200).json(categories);
} else {
return next(
error.createForbidden(
Expand All @@ -246,7 +259,15 @@ const listCategories = asyncHandler(async (req, res, next) => {
const addCategory = asyncHandler(async (req, res, next) => {
try {
let result = await MarketplaceItem.get(req.params.itemId);
if (checkReadPermission(req.userInfo, result)) {
if (await checkWritePermission(req.tokenInfo, result)) {
const category = await ItemCategory.get(req.params.categoryId);
if (category.name === OFFICIAL_EXAMPLE && req.tokenInfo.admin !== true) {
return next(
error.createForbidden(
'Only admin can set "official example" category.',
),
);
}
result = await MarketplaceItem.addCategory(result, req.params.categoryId);
if (isNil(result)) {
return next(error.createNotFound());
Expand All @@ -268,7 +289,7 @@ const addCategory = asyncHandler(async (req, res, next) => {
const deleteCategory = asyncHandler(async (req, res, next) => {
try {
let result = await MarketplaceItem.get(req.params.itemId);
if (checkReadPermission(req.userInfo, result)) {
if (await checkWritePermission(req.tokenInfo, result)) {
result = await MarketplaceItem.deleteCategory(
result,
req.params.categoryId,
Expand Down
107 changes: 65 additions & 42 deletions rest_server/src/models/marketplace_item.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ const { isNil, toLower } = require('lodash');
const { Op, fn, col, where, cast } = require('sequelize');
const modelSyncHandler = require('./model_init_handler');

const OFFICIAL_EXAMPLE = 'official example';

class MarketplaceItem {
constructor(sequelize, DataTypes) {
this.orm = sequelize.define('MarketplaceItem', {
Expand Down Expand Up @@ -123,44 +125,6 @@ class MarketplaceItem {
},
];
}
if (!userInfo.admin) {
filterStatement[Op.or] = [
{
isPublic: {
[Op.eq]: true,
},
},
{
[Op.and]: [
{
isPrivate: {
[Op.eq]: true,
},
},
{
author: {
[Op.eq]: userInfo.username,
},
},
],
},
{
[Op.and]: [
{
isPublic: {
[Op.eq]: false,
},
isPrivate: {
[Op.eq]: false,
},
groupList: {
[Op.overlap]: userInfo.groupList,
},
},
],
},
];
}

const havings = [];
const havingArrayAgg = (queryParameter, colName) => {
Expand All @@ -177,6 +141,65 @@ class MarketplaceItem {
havingArrayAgg(tags, 'ItemTags.name');
havingArrayAgg(categories, 'ItemCategories.name');

if (!userInfo.admin) {
if (filterStatement[Op.and] === undefined) {
filterStatement[Op.and] = [];
}
filterStatement[Op.and].push({
[Op.or]: [
{
isPublic: {
[Op.eq]: true,
},
},
{
[Op.and]: [
{
isPrivate: {
[Op.eq]: true,
},
},
{
author: {
[Op.eq]: userInfo.username,
},
},
],
},
{
[Op.and]: [
{
isPublic: {
[Op.eq]: false,
},
isPrivate: {
[Op.eq]: false,
},
groupList: {
[Op.overlap]: userInfo.groupList,
},
},
],
},
],
});

havings.push({
[Op.not]: {
[Op.and]: [
{
isPrivate: true,
},
where(
fn('array_agg', col('ItemCategories.name')),
Op.contains,
cast([OFFICIAL_EXAMPLE], 'varchar[]'),
),
],
},
});
}

const items = await this.orm.findAll({
where: filterStatement,
having: havings,
Expand Down Expand Up @@ -373,17 +396,17 @@ class MarketplaceItem {
return await handler(item, this.models);
}

async addCategory(item, tagId) {
async addCategory(item, categoryId) {
const handler = modelSyncHandler(async item => {
return await item.addItemCategory(tagId);
return await item.addItemCategory(categoryId);
});

return await handler(item, this.models);
}

async deleteCategory(item, tagId) {
async deleteCategory(item, categoryId) {
const handler = modelSyncHandler(async item => {
return await item.removeItemCategory(tagId);
return await item.removeItemCategory(categoryId);
});

return await handler(item, this.models);
Expand Down
8 changes: 4 additions & 4 deletions rest_server/src/router.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,17 @@ router

router
.route('/items/:itemId/tag/:tagId')
.post(token.checkAuthAndGetUserInfo, itemController.addTag)
.delete(token.checkAuthAndGetUserInfo, itemController.deleteTag);
.post(token.checkAuthAndGetTokenInfo, itemController.addTag)
.delete(token.checkAuthAndGetTokenInfo, itemController.deleteTag);

router
.route('/items/:itemId/category')
.get(token.checkAuthAndGetUserInfo, itemController.listCategories);

router
.route('/items/:itemId/category/:categoryId')
.post(token.checkAuthAndGetUserInfo, itemController.addCategory)
.delete(token.checkAuthAndGetUserInfo, itemController.deleteCategory);
.post(token.checkAuthAndGetTokenInfo, itemController.addCategory)
.delete(token.checkAuthAndGetTokenInfo, itemController.deleteCategory);

router
.route('/tags')
Expand Down