From 525230bb85e74a14748dd713d28f40cc9b43c317 Mon Sep 17 00:00:00 2001 From: Frinze Erin Lapuz Date: Mon, 8 Feb 2021 23:05:39 +0800 Subject: [PATCH 01/16] ADDED comment and better interface display on names --- client/components/Coordinator/Evaluation/EvaluationList.js | 2 -- client/components/Coordinator/Evaluation/EvaluationListing.js | 4 ++-- client/components/reviewer/ReviewList.js | 3 +-- client/components/reviewer/ReviewListing.js | 2 +- 4 files changed, 4 insertions(+), 7 deletions(-) diff --git a/client/components/Coordinator/Evaluation/EvaluationList.js b/client/components/Coordinator/Evaluation/EvaluationList.js index 854f9aff..626ec0a5 100644 --- a/client/components/Coordinator/Evaluation/EvaluationList.js +++ b/client/components/Coordinator/Evaluation/EvaluationList.js @@ -80,8 +80,6 @@ const EvaluationList = () => { ({ _id, courseId, reviewDescription }) => { // select out the coordinators with the permission for this evaluation - console.log('users', users); - console.log('_id', _id); const coordinators = users.filter(({ perms }) => perms.some( ({ course_id, role }) => course_id === _id && role === 'Coordinator' diff --git a/client/components/Coordinator/Evaluation/EvaluationListing.js b/client/components/Coordinator/Evaluation/EvaluationListing.js index 1563d907..4cb05b55 100644 --- a/client/components/Coordinator/Evaluation/EvaluationListing.js +++ b/client/components/Coordinator/Evaluation/EvaluationListing.js @@ -22,7 +22,7 @@ const EvaluationListing = ({ }) => { const classes = useStyles(); - const coordinatorNames = coordinators.map(({ name }) => name)?.join(', '); + const coordinatorNames = coordinators.map(({ name,email }) => name || email)?.join(', '); return (

{courseCode}

-

{coordinatorNames}.

+

{coordinatorNames}

{evaluationDescription}
 diff --git a/client/components/reviewer/ReviewList.js b/client/components/reviewer/ReviewList.js index a5beab2b..f0e1dcb3 100644 --- a/client/components/reviewer/ReviewList.js +++ b/client/components/reviewer/ReviewList.js @@ -45,8 +45,7 @@ const EvaluationList = () => { const authUser = useSelector((state) => state.auth.user); useEffect(() => { - // 1. Find all CourseEvaluations where the createdBy key matches the logged in user - services['course-evaluation'].find(); + services['course-evaluation'].find(); // This is already a filtered lists based on perms }, []); // Executes on Component Remount (after auth user is fetched) diff --git a/client/components/reviewer/ReviewListing.js b/client/components/reviewer/ReviewListing.js index 9a2296da..21052f3b 100644 --- a/client/components/reviewer/ReviewListing.js +++ b/client/components/reviewer/ReviewListing.js @@ -30,7 +30,7 @@ const ReviewListing = ({

{courseCode}

-

{coordinatorNames}.............

+

{coordinatorNames}

{evaluationDescription}
 From c00096ae10c591ba14d950ba659114a65f769ec0 Mon Sep 17 00:00:00 2001 From: Frinze Erin Lapuz Date: Mon, 8 Feb 2021 23:56:01 +0800 Subject: [PATCH 02/16] ADDED user service method permission protection --- server/src/hooks/role-based-restrictions.js | 1 - server/src/services/users/users.hooks.js | 7 +++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/server/src/hooks/role-based-restrictions.js b/server/src/hooks/role-based-restrictions.js index c8202e21..41151e83 100644 --- a/server/src/hooks/role-based-restrictions.js +++ b/server/src/hooks/role-based-restrictions.js @@ -25,7 +25,6 @@ module.exports = (roleBasedPermissionsRequired) => { for (let roleRequired of roleBasedPermissionsRequired ){ // If a user does not have the required permission, then disallow if(!availablePerms.has(roleRequired)){ - console.log(roleRequired); return true; } } diff --git a/server/src/services/users/users.hooks.js b/server/src/services/users/users.hooks.js index 9f99ed1d..bd0e79d7 100644 --- a/server/src/services/users/users.hooks.js +++ b/server/src/services/users/users.hooks.js @@ -9,14 +9,13 @@ module.exports = { before: { all: [], find: [authenticate('jwt'), - // attachUser(), roleBasedRestrictions(['Coordinator']) ], get: [authenticate('jwt')], create: [firstUser(), compileGoogleAndSystemPerms()], - update: [authenticate('jwt')], - patch: [authenticate('jwt')], - remove: [authenticate('jwt')], + update: [authenticate('jwt'), roleBasedRestrictions(['Administrator'])], // Only Admin + patch: [authenticate('jwt'), roleBasedRestrictions(['Coordinator'])], // Only Coordinator + Admin + remove: [authenticate('jwt'), roleBasedRestrictions(['Administrator'])], // Only Admin }, after: { From 13add9da202ddec43a250b061004ccf3992730a4 Mon Sep 17 00:00:00 2001 From: Frinze Erin Lapuz Date: Mon, 8 Feb 2021 23:56:57 +0800 Subject: [PATCH 03/16] IMPROVED filter-based-permission to also block GET methods This does not fully work yet for review service --- server/src/hooks/filter-based-permission.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/server/src/hooks/filter-based-permission.js b/server/src/hooks/filter-based-permission.js index 30b6070b..520c321b 100644 --- a/server/src/hooks/filter-based-permission.js +++ b/server/src/hooks/filter-based-permission.js @@ -1,6 +1,7 @@ // Use this hook to manipulate incoming or outgoing data. // For more information on hooks see: http://docs.feathersjs.com/api/hooks.html +const { Forbidden } = require('@feathersjs/errors'); const { getAvailablePermissionsOfUser} = require('../utils'); @@ -15,7 +16,7 @@ const { getAvailablePermissionsOfUser} = require('../utils'); */ module.exports = (options = {}) => { return async context => { - const {serviceName,params} = context; + const {serviceName,params,method, id} = context; // No filter in internal server use if (typeof params.provider!=='undefined'){ @@ -36,9 +37,13 @@ module.exports = (options = {}) => { { // Convert to set to eliminate redundancies const courseIDsAllowed = Array.from(new Set(coordinatorCoursePermissions.concat(reviewerCoursePermissions))); + if(method==='get' && !courseIDsAllowed.includes(id)){ + throw new Forbidden('You do not have the correct permission to access this'); + } params.query = {...params.query, _id: {$in: courseIDsAllowed} }; } else if(serviceName==='review'){ + // TODO: Create a permission filter for a review params.query={ ...params.query, $or: [ From 8a03ce40adb186d950d291fa50ad53ab5d96af40 Mon Sep 17 00:00:00 2001 From: Frinze Erin Lapuz Date: Mon, 8 Feb 2021 23:57:33 +0800 Subject: [PATCH 04/16] ADDED coordinators and reviewers as meta information to course-evaluation --- ...oordinators-and-reviewers-to-evaluation.js | 44 +++++++++++++++++++ .../course-evaluation.hooks.js | 3 +- 2 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 server/src/hooks/add-coordinators-and-reviewers-to-evaluation.js diff --git a/server/src/hooks/add-coordinators-and-reviewers-to-evaluation.js b/server/src/hooks/add-coordinators-and-reviewers-to-evaluation.js new file mode 100644 index 00000000..68121bf4 --- /dev/null +++ b/server/src/hooks/add-coordinators-and-reviewers-to-evaluation.js @@ -0,0 +1,44 @@ +// Use this hook to manipulate incoming or outgoing data. +// For more information on hooks see: http://docs.feathersjs.com/api/hooks.html + +// This hook is used for adding more information about the course-evaluation endpoint +// This is an After Hook +const errors = require('@feathersjs/errors'); + +// eslint-disable-next-line no-unused-vars +module.exports = function (options = {}) { + return async context => { + const {app, id,method} = context; + + // Function for selection only a couple of information from each user + // that has a role in a course + const getUsersForRole = async (course_id,role) =>{ + const query = await app.service('users').find({query: + { + $select:['_id','name','email'], + perms: { + $elemMatch: { course_id: course_id, role}, + } + } + }); + return query.data; + + }; + + if(method!=='find'){ // All Services except find + const [coordinators,reviewers] = await Promise.all([getUsersForRole(id,'Coordinator'),getUsersForRole(id,'Reviewer')]); + context.result.coordinators = coordinators; + context.result.reviewers = reviewers; + } + else{ + context.result.data = await Promise.all(context.result.data.map(async(datum)=>{ + const [coordinators,reviewers] = await Promise.all([getUsersForRole(datum._id,'Coordinator'),getUsersForRole(datum._id,'Reviewer')]); + return({ + ...datum, + coordinators, + reviewers + }); + })); + } + }; +}; diff --git a/server/src/services/course-evaluation/course-evaluation.hooks.js b/server/src/services/course-evaluation/course-evaluation.hooks.js index ae88322a..61018c08 100644 --- a/server/src/services/course-evaluation/course-evaluation.hooks.js +++ b/server/src/services/course-evaluation/course-evaluation.hooks.js @@ -1,6 +1,7 @@ const { authenticate } = require('@feathersjs/authentication').hooks; const filterBasedPermission = require('../../hooks/filter-based-permission'); +const addCoordinatorsAndReviewersToEvaluation = require('../../hooks/add-coordinators-and-reviewers-to-evaluation'); const addPermissionToUser = require('../../hooks/add-permission-to-user'); module.exports = { @@ -15,7 +16,7 @@ module.exports = { }, after: { - all: [], + all: [addCoordinatorsAndReviewersToEvaluation()], find: [], get: [], create: [addPermissionToUser()], From 64a915979d80855d0c57535826c76e11d7e83612 Mon Sep 17 00:00:00 2001 From: Frinze Erin Lapuz Date: Mon, 8 Feb 2021 23:58:02 +0800 Subject: [PATCH 05/16] ADDED role-based restrictions to course-evaluation and reviews --- .../src/hooks/role-and-course-based-restrictions.js | 1 + .../course-evaluation/course-evaluation.hooks.js | 13 ++++++++----- server/src/services/review/review.hooks.js | 13 ++++++++----- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/server/src/hooks/role-and-course-based-restrictions.js b/server/src/hooks/role-and-course-based-restrictions.js index 36f964ce..1d8463e0 100644 --- a/server/src/hooks/role-and-course-based-restrictions.js +++ b/server/src/hooks/role-and-course-based-restrictions.js @@ -23,6 +23,7 @@ module.exports = () => { } if(serviceName==='review'){ // This is for review service + // TODO: this needs to be improve to only accomodate authors // Get course id const course_id = context.data.course_id || context.existing.course_id; diff --git a/server/src/services/course-evaluation/course-evaluation.hooks.js b/server/src/services/course-evaluation/course-evaluation.hooks.js index 61018c08..75de519f 100644 --- a/server/src/services/course-evaluation/course-evaluation.hooks.js +++ b/server/src/services/course-evaluation/course-evaluation.hooks.js @@ -1,6 +1,9 @@ const { authenticate } = require('@feathersjs/authentication').hooks; +const {iff, disallow } = require('feathers-hooks-common'); const filterBasedPermission = require('../../hooks/filter-based-permission'); +const roleBasedRestrictions = require('../../hooks/role-based-restrictions'); +const roleAndCourseBasedRestrictions = require('../../hooks/role-and-course-based-restrictions'); const addCoordinatorsAndReviewersToEvaluation = require('../../hooks/add-coordinators-and-reviewers-to-evaluation'); const addPermissionToUser = require('../../hooks/add-permission-to-user'); @@ -8,11 +11,11 @@ module.exports = { before: { all: [ authenticate('jwt') ], find: [filterBasedPermission()], - get: [], - create: [], - update: [], - patch: [], - remove: [] + get: [filterBasedPermission()], + create: [ roleBasedRestrictions(['Coordinator'])], // Only create for a coordinator + update: [ roleAndCourseBasedRestrictions()], // Only modifications of a specific unit + patch: [ roleAndCourseBasedRestrictions()], // is allowed for a the coordinator of that specific unit + remove: [disallow('external')] // Prevent any removal of Course-Evaluation }, after: { diff --git a/server/src/services/review/review.hooks.js b/server/src/services/review/review.hooks.js index 48dd858c..b242501c 100644 --- a/server/src/services/review/review.hooks.js +++ b/server/src/services/review/review.hooks.js @@ -1,15 +1,18 @@ const { authenticate } = require('@feathersjs/authentication').hooks; +const {iff, disallow } = require('feathers-hooks-common'); const filterBasedPermission = require('../../hooks/filter-based-permission'); +const roleAndCourseBasedRestrictions = require('../../hooks/role-and-course-based-restrictions'); + module.exports = { before: { all: [ authenticate('jwt') ], find: [filterBasedPermission()], - get: [], - create: [], - update: [], - patch: [], - remove: [] + get: [filterBasedPermission()], + create: [roleAndCourseBasedRestrictions()], // Creation and modificaiton of review + update: [roleAndCourseBasedRestrictions()], // is only allowed for the author/reviewer + patch: [roleAndCourseBasedRestrictions()], + remove: [disallow('external')] }, after: { From d515c93dbeac499ee82ae58567a5149d46eed195 Mon Sep 17 00:00:00 2001 From: Frinze Erin Lapuz Date: Tue, 9 Feb 2021 00:06:36 +0800 Subject: [PATCH 06/16] FIXED where string courseId cannot find in the Array of Object courseId --- server/src/hooks/filter-based-permission.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/hooks/filter-based-permission.js b/server/src/hooks/filter-based-permission.js index 520c321b..4160f339 100644 --- a/server/src/hooks/filter-based-permission.js +++ b/server/src/hooks/filter-based-permission.js @@ -37,7 +37,8 @@ module.exports = (options = {}) => { { // Convert to set to eliminate redundancies const courseIDsAllowed = Array.from(new Set(coordinatorCoursePermissions.concat(reviewerCoursePermissions))); - if(method==='get' && !courseIDsAllowed.includes(id)){ + const courseIDsAllowedStringCasted = courseIDsAllowed.map(course_id => String(course_id)); + if(method==='get' && !courseIDsAllowedStringCasted.includes(id)){ throw new Forbidden('You do not have the correct permission to access this'); } params.query = {...params.query, _id: {$in: courseIDsAllowed} }; From c3dab092e520fab68ecf6031c21469644797163c Mon Sep 17 00:00:00 2001 From: Frinze Erin Lapuz Date: Wed, 17 Feb 2021 23:04:47 +0800 Subject: [PATCH 07/16] evaluationList remove user service as a dependence --- .../Coordinator/Evaluation/EvaluationList.js | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/client/components/Coordinator/Evaluation/EvaluationList.js b/client/components/Coordinator/Evaluation/EvaluationList.js index 626ec0a5..e846396f 100644 --- a/client/components/Coordinator/Evaluation/EvaluationList.js +++ b/client/components/Coordinator/Evaluation/EvaluationList.js @@ -52,15 +52,13 @@ const EvaluationList = () => { useEffect(() => { // 1. Find all CourseEvaluations where the createdBy key matches the logged in user services['course-evaluation'].find(); - services['users'].find(); setLoading(false); }, []); const courseEvaluations = useSelector((state) => state['course-evaluation']) ?.queryResult?.data; - const users = useSelector((state) => state['users'])?.queryResult?.data; - if (loading || !users || !courseEvaluations) { + if (loading || !courseEvaluations) { return ( Loading... @@ -77,15 +75,9 @@ const EvaluationList = () => { console.log('evals:', evaluationListings); // 3. Render course list elemnts evaluationListings = evaluationListings.map( - ({ _id, courseId, reviewDescription }) => { + ({ _id, courseId, reviewDescription, coordinators }) => { // select out the coordinators with the permission for this evaluation - const coordinators = users.filter(({ perms }) => - perms.some( - ({ course_id, role }) => course_id === _id && role === 'Coordinator' - ) - ); - return ( Date: Wed, 17 Feb 2021 23:08:50 +0800 Subject: [PATCH 08/16] remove dependence of the other information component to user service --- .../General/OtherInformation.js | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/client/components/Coordinator/EvaluationOverview/General/OtherInformation.js b/client/components/Coordinator/EvaluationOverview/General/OtherInformation.js index 673e8be1..4b6f137c 100644 --- a/client/components/Coordinator/EvaluationOverview/General/OtherInformation.js +++ b/client/components/Coordinator/EvaluationOverview/General/OtherInformation.js @@ -2,27 +2,20 @@ import Card from 'components/MaterialKit/Card/Card.js'; import CardBody from 'components/MaterialKit/Card/CardBody.js'; import CardHeader from 'components/MaterialKit/Card/CardHeader.js'; +import Grid from 'components/MaterialKit/Grid/GridContainer.js'; +import GridItem from 'components/MaterialKit/Grid/GridItem.js'; // Store Actions and Redux import { useSelector } from 'react-redux'; -import { services } from 'store/feathersClient'; -import { useEffect } from 'react'; const OtherInformation = () => { - useEffect(() => { - services['users'].find(); - }, []); - const courseEval = useSelector((state) => state['course-evaluation']); const evalData = courseEval?.data; - const users = useSelector((state) => state['users']); - const userData = users?.queryResult?.data; + const coordinators = evalData?.coordinators || []; const createdOn = new Date(evalData?.createdAt); - const createdBy = evalData?.createdBy; - const author = userData?.find((elem) => elem._id == createdBy); const dateString = createdOn?.toLocaleDateString('en-gb', { year: 'numeric', month: 'short', @@ -33,11 +26,18 @@ const OtherInformation = () => { Other Information -

Created by:

-

- {author?.name ?? 'Unknown creator'} (on {dateString ?? 'unknown date'} - ) -

+ + +

Coordinators

+ {coordinators.map(({name},index) =>

{name}

)} + + +

Date Started

+

+ {dateString ?? 'unknown date'} +

+ + ); From 806ab62e4edb107b4958fd8c3b7d55fd64252a52 Mon Sep 17 00:00:00 2001 From: Frinze Erin Lapuz Date: Wed, 17 Feb 2021 23:26:02 +0800 Subject: [PATCH 09/16] remove dependency of reviewProgress component in users service --- .../Reviews/ReviewProgress/index.js | 76 +++++++------------ 1 file changed, 27 insertions(+), 49 deletions(-) diff --git a/client/components/Coordinator/EvaluationOverview/Reviews/ReviewProgress/index.js b/client/components/Coordinator/EvaluationOverview/Reviews/ReviewProgress/index.js index f1a98a5f..d1d3decd 100644 --- a/client/components/Coordinator/EvaluationOverview/Reviews/ReviewProgress/index.js +++ b/client/components/Coordinator/EvaluationOverview/Reviews/ReviewProgress/index.js @@ -12,62 +12,40 @@ import { useSelector } from 'react-redux'; import { services } from 'store/feathersClient'; import { useRouter } from 'next/router'; -import { useState, useEffect } from 'react'; +import { useEffect } from 'react'; const ReviewProgress = () => { const router = useRouter(); - const [reviewers, setReviewers] = useState([]); - const [loading, setLoading] = useState(true); - - const hasPath = router.query.hasOwnProperty('courseID'); - + const { courseID } = router.query; useEffect(() => { - if (router.query.hasOwnProperty('courseID')) { - const { courseID } = router.query; - - services['review'].find({ - course_id: courseID, - }); - services['users'].find( - {query: - { - perms: { - $elemMatch: { course_id: courseID, role: 'Reviewer' }, - } - } - } - ); - } - }, [hasPath]); - - if (hasPath) { - const reviews = useSelector((state) => state['review']); - const reviewData = reviews?.queryResult?.data; - - const users = useSelector((state) => state['users']); - - const usersData = users?.queryResult?.data; - - // TODO Note: Something similar is done in utils/compileResult - const progressCards = usersData?.map(reviewer=>{ - const reviewOfUser = reviewData?.find(review=> review.user_id === reviewer._id) || {}; - return( - - ); + services['review'].find({ + course_id: courseID, }); - - return ( - - Review Progress - - {progressCards} - - + }, []); + + const courseEval = useSelector((state) => state['course-evaluation']); + const evalData = courseEval?.data; + const usersData = evalData?.reviewers; + const reviews = useSelector((state) => state['review']); + const reviewData = reviews?.queryResult?.data; + + // TODO Note: Something similar is done in utils/compileResult + const progressCards = usersData?.map(reviewer=>{ + const reviewOfUser = reviewData?.find(review=> review.user_id === reviewer._id) || {}; + return( + ); - } else { - return

invalid...

; - } + }); + + return ( + + Review Progress + + {progressCards} + + + ); }; export default ReviewProgress; From b744c1706ebf97bddb98b1617a9973d5a4d7829a Mon Sep 17 00:00:00 2001 From: Frinze Erin Lapuz Date: Wed, 17 Feb 2021 23:26:19 +0800 Subject: [PATCH 10/16] disable view if reviewer hasnt started on review --- .../Reviews/ReviewProgress/ProgressDisplay.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/client/components/Coordinator/EvaluationOverview/Reviews/ReviewProgress/ProgressDisplay.js b/client/components/Coordinator/EvaluationOverview/Reviews/ReviewProgress/ProgressDisplay.js index 381708fe..0cf7c18f 100644 --- a/client/components/Coordinator/EvaluationOverview/Reviews/ReviewProgress/ProgressDisplay.js +++ b/client/components/Coordinator/EvaluationOverview/Reviews/ReviewProgress/ProgressDisplay.js @@ -55,9 +55,10 @@ const ProgressDisplay = ({ reviewer, review }) => { display="inline-block" color="white" onClick={handleView} + disabled={!review?._id} > - View + View {review?.submittedDate && ( From 545b26bfa639f6245cec2dace13d872e8941365c Mon Sep 17 00:00:00 2001 From: Frinze Erin Lapuz Date: Wed, 17 Feb 2021 23:28:05 +0800 Subject: [PATCH 11/16] remove dependency of compiled report component from user service --- .../coordinator/[courseID]/review/compiled.js | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/client/pages/coordinator/[courseID]/review/compiled.js b/client/pages/coordinator/[courseID]/review/compiled.js index 1113ca8b..3358bc4b 100644 --- a/client/pages/coordinator/[courseID]/review/compiled.js +++ b/client/pages/coordinator/[courseID]/review/compiled.js @@ -24,19 +24,11 @@ const CompiledPage = () => { course_id: courseID }, }); //Accessed by queryResult.data - services['users'].find( - {query: - { - perms: { - $elemMatch: { course_id: courseID, role: 'Reviewer' }, - } - } - } - ); // Accessed by queryResult.data }, [courseID]); - const userState = useSelector(state=> state.users); - const reviewers = userState?.queryResult.data; + const courseEval = useSelector((state) => state['course-evaluation']); + const evalData = courseEval?.data; + const reviewers = evalData?.reviewers || []; const reviewState = useSelector(state=> state.review); const reviews = reviewState?.queryResult.data; From 9a6d0c3763e1797705be922e9b95f893e9cd0712 Mon Sep 17 00:00:00 2001 From: Frinze Erin Lapuz Date: Wed, 17 Feb 2021 23:44:23 +0800 Subject: [PATCH 12/16] remove dependence of ManageReviewers with course-evaluation --- .../General/Controls/ManageReviewers.js | 33 +++---------------- .../General/Controls/ReviewerListing.js | 1 - 2 files changed, 4 insertions(+), 30 deletions(-) diff --git a/client/components/Coordinator/EvaluationOverview/General/Controls/ManageReviewers.js b/client/components/Coordinator/EvaluationOverview/General/Controls/ManageReviewers.js index b4d739d3..ad33bf72 100644 --- a/client/components/Coordinator/EvaluationOverview/General/Controls/ManageReviewers.js +++ b/client/components/Coordinator/EvaluationOverview/General/Controls/ManageReviewers.js @@ -29,49 +29,24 @@ const useStyles = makeStyles({ import { useSelector } from 'react-redux'; import { services, rawServices } from 'store/feathersClient'; -import { useState, useEffect } from 'react'; +import { useState } from 'react'; const ManageReviewers = ({ evaluationID }) => { const classes = useStyles(); const [modal, setModal] = useState(false); const [email, setEmail] = useState(''); - useEffect(() => { - services['users'].find({ - perms: { - $in: [{ course_id: evaluationID, role: 'Reviewer' }], - }, - }); - }, []); - const courseEval = useSelector((state) => state['course-evaluation']); const evalData = courseEval?.data; - const users = useSelector((state) => state['users']); - const userData = users?.queryResult?.data; // selects out all reviewers with correct permission - const reviewers = userData.filter((user) => - user.perms.reduce( - (acc, permission) => - acc || - (permission.course_id == evaluationID && permission.role == 'Reviewer'), - false - ) - ); + const reviewers = evalData?.reviewers || []; // removes a user from the evaluation const removePermission = async (userId, evaluationId) => { try { - const oldPermissions = reviewers.find((user) => user._id == userId).perms; - const newPerms = oldPermissions.filter( - (permission) => - !( - permission.course_id == evaluationId && - permission.role == 'Reviewer' - ) - ); - const response = await services['users'].patch(userId, { - perms: newPerms, + services['users'].patch(userId, { + $pull: { perms: {course_id:evaluationId,role:'Reviewer'}} }); } catch (error) { console.error(error); diff --git a/client/components/Coordinator/EvaluationOverview/General/Controls/ReviewerListing.js b/client/components/Coordinator/EvaluationOverview/General/Controls/ReviewerListing.js index 457e61d6..9701ddba 100644 --- a/client/components/Coordinator/EvaluationOverview/General/Controls/ReviewerListing.js +++ b/client/components/Coordinator/EvaluationOverview/General/Controls/ReviewerListing.js @@ -24,7 +24,6 @@ const useStyles = makeStyles({ const ReviewerListing = ({ email, name, - inviter, googleId, removeReviewer, }) => { From a03e976fd97a39e3fe91484306d428f9c3a1a377 Mon Sep 17 00:00:00 2001 From: Frinze Erin Lapuz Date: Wed, 17 Feb 2021 23:50:16 +0800 Subject: [PATCH 13/16] fixed bug where the interface is just showing [object.object] in coordinator name --- client/components/reviewer/ReviewListing.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/components/reviewer/ReviewListing.js b/client/components/reviewer/ReviewListing.js index 21052f3b..b46194be 100644 --- a/client/components/reviewer/ReviewListing.js +++ b/client/components/reviewer/ReviewListing.js @@ -23,7 +23,7 @@ const ReviewListing = ({ }) => { const classes = useStyles(); - const coordinatorNames = coordinators?.join(', '); + const coordinatorNames = coordinators?.map(({name,email})=>name || email).join(', '); return ( From c49e7b001de4777927a33e6bdd8c591056e91dec Mon Sep 17 00:00:00 2001 From: Frinze Erin Lapuz Date: Thu, 18 Feb 2021 00:11:55 +0800 Subject: [PATCH 14/16] fix bug where reviewer cannot update review --- server/src/hooks/role-and-course-based-restrictions.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/server/src/hooks/role-and-course-based-restrictions.js b/server/src/hooks/role-and-course-based-restrictions.js index 1d8463e0..210c8389 100644 --- a/server/src/hooks/role-and-course-based-restrictions.js +++ b/server/src/hooks/role-and-course-based-restrictions.js @@ -22,13 +22,17 @@ module.exports = () => { return false; // Allow service } + // Difficult to do comparison when Mongoose Object ID is being compared + const userPermissionsWithCourseIdCastedAsString = params.user.perms.map(({course_id,role})=> { + String(course_id), + role; + }); if(serviceName==='review'){ // This is for review service // TODO: this needs to be improve to only accomodate authors // Get course id const course_id = context.data.course_id || context.existing.course_id; - // If the service is a review, then the user needs to have a Reviewer role - if (!params.user.perms.includes({course_id,role:'Reviewer'})){ + if (userPermissionsWithCourseIdCastedAsString.includes({course_id,role:'Reviewer'})){ return true; } } @@ -38,7 +42,7 @@ module.exports = () => { const course_id = context.data._id || context.existing._id; // If the service is a course-evaluation role - if (!params.user.perms.includes({course_id,role:'Coordinator'})){ + if (userPermissionsWithCourseIdCastedAsString.includes({course_id,role:'Coordinator'})){ return true; } } From 4d5ba7783bf564640ccd93e8bf95e9f4bdc4b141 Mon Sep 17 00:00:00 2001 From: Frinze Erin Lapuz Date: Thu, 18 Feb 2021 00:15:40 +0800 Subject: [PATCH 15/16] linting of console log --- server/.eslintrc.json | 3 ++- server/src/hooks/attach-user.js | 2 -- server/src/hooks/filter-based-permission.js | 1 - 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/server/.eslintrc.json b/server/.eslintrc.json index fa5d75ff..8b813e86 100644 --- a/server/.eslintrc.json +++ b/server/.eslintrc.json @@ -33,6 +33,7 @@ "unused-imports/no-unused-vars": [ "warn", { "vars": "all", "varsIgnorePattern": "^_", "args": "after-used", "argsIgnorePattern": "^_" } - ] + ], + "no-console": ["error", { "allow": ["warn","info","error","groupCollapsed","groupEnd"] }] } } \ No newline at end of file diff --git a/server/src/hooks/attach-user.js b/server/src/hooks/attach-user.js index db8b4193..bc83fabe 100644 --- a/server/src/hooks/attach-user.js +++ b/server/src/hooks/attach-user.js @@ -11,9 +11,7 @@ module.exports = (options = {}) => { if(context.params.authStrategies && context.params.authStrategies.includes('google')) { const googleId = context.params.query.googleId; - console.log(googleId); context.params.user = (await userModel.findOne({googleId}))._doc; - console.log(context.user); } return context; diff --git a/server/src/hooks/filter-based-permission.js b/server/src/hooks/filter-based-permission.js index 4160f339..4990603e 100644 --- a/server/src/hooks/filter-based-permission.js +++ b/server/src/hooks/filter-based-permission.js @@ -55,7 +55,6 @@ module.exports = (options = {}) => { } } } - console.log(params.query); return context; }; }; From 570f70378bb845c79959dc3ea8172910148b8bab Mon Sep 17 00:00:00 2001 From: Frinze Erin Lapuz Date: Thu, 18 Feb 2021 00:50:15 +0800 Subject: [PATCH 16/16] documentation for permission and service metadata --- mkdocs/docs/developer/backend/permission.md | 56 +++++++++++++++++++ mkdocs/docs/developer/backend/services.md | 30 ++++++++++ .../{ => frontend}/feathers_redux.md | 0 .../developer/{ => frontend}/notifications.md | 0 mkdocs/mkdocs.yml | 7 ++- 5 files changed, 91 insertions(+), 2 deletions(-) create mode 100644 mkdocs/docs/developer/backend/permission.md create mode 100644 mkdocs/docs/developer/backend/services.md rename mkdocs/docs/developer/{ => frontend}/feathers_redux.md (100%) rename mkdocs/docs/developer/{ => frontend}/notifications.md (100%) diff --git a/mkdocs/docs/developer/backend/permission.md b/mkdocs/docs/developer/backend/permission.md new file mode 100644 index 00000000..3f908c4d --- /dev/null +++ b/mkdocs/docs/developer/backend/permission.md @@ -0,0 +1,56 @@ +# Permission +Permission refers to the authority of the user to access or modify information. In business rules, the only reason why a user has to have permission if they need to have access to that information or authority to have action. This means that if a user does not need to access that information, they should not have it. + +In IndEAA, this is essential in restricting each of the users to perform only actions that they have permission over. The permission has 2 main elements: + +- `role` (what sets of permission does the user have?) +- `course_id` (which `course_evaluation` where this role persists?) + + +## Relationship to Frontend +Although this part of the documentation applies for both the backend and frontend, this documentation is mainly for backend as it is where most business rules will apply. Permissions for frontend mainly apply only for the convenience of the user, but does not protect certain information or action from being exposed. Hence, all data-driven action (CRUD operations) should be protected by the backend. + +## Role + +### Administrator +Administrators have the capability to adjust all the permissions of the user in the system, and even has the ability to delete users from the system. + +### Coordinator +Coordinators have the capability adjust everything related to the `course_evaluation` and should be able to view details about other coordinators and reviewers of their `course_evaluation` + +### Reviewer + +Reviewers have the capability to adjust everything related to their own `review` for a `course_evaluation`, and should be able to view details about coordinators for the `course_evaluation` they are assigned to. + +## Hooks + +### Role-Based Restriction +This hook restricts actions/service methods only to users with a specific role regardless of `course_id`. + +???+ example "Examples of Times where you want this" + + - Admiinistrator Role + + The administrator role does not need `course_id`. Hence, should be used for it + + - Coordinator Role + + Creating a `course_evaluation` requires a Coordinator role, but does not need to check for `course_id` + +### Filter-Based Restriction +This hook restricts query (`GET` and `FIND` service methods) to limit (`FIND`) based on query or restricts access (`GET`). + +### Role-And-Course Based Restriction +This hook is a stricter version of `Role-Based Restrictions` as this also applies with `course_id` that the user has access to. + +???+ example "Example of Times where you want this" + + - Multi-Role Coordinator + + A coordinator should not have access to other `course_evaluation` they do not have access on. + + - Multi-Role Reviewer + + A reviewer should not have access to `course_evaluation` they do not have access on, and should also not be able to view `review` that they are not the review person for. + + \ No newline at end of file diff --git a/mkdocs/docs/developer/backend/services.md b/mkdocs/docs/developer/backend/services.md new file mode 100644 index 00000000..b1f5bbed --- /dev/null +++ b/mkdocs/docs/developer/backend/services.md @@ -0,0 +1,30 @@ +# Services +Services refers to endpoints of the backend that interfaces directly with the database and may have relations to other service (as opposed to microservices). + +See below for the services that IndEAA has. + +## Users `/users` +This is responsible for data-driven actions related to users. + +## Course Evaluation`/course_evaluation` +This is responsible for data-driven actions related to course that is being evaluated. + +### Metadata +This service has two metadata field that are not mapped in the model. + +### `coordinators` + +This is an array of users object that has a permission `course_id` for this `course_evaluation` and `role` as "Coordinator. The object has these following fields: + +- `_id` +- `name` +- `email` + +### `reviewers` +This is an array of users object that has a permission `course_id` for this `course_evaluation` and `role` as "Reviewer". The object has the same following fields as `coordinators` object structure. + +???+ note "Model != Service" + There is some difference to a service to a model field, hence should not be relied upon if deciding on which endpoint to use. + +## Review `/review` +This is responsible for data-driven actions related to reviews for a specific course. \ No newline at end of file diff --git a/mkdocs/docs/developer/feathers_redux.md b/mkdocs/docs/developer/frontend/feathers_redux.md similarity index 100% rename from mkdocs/docs/developer/feathers_redux.md rename to mkdocs/docs/developer/frontend/feathers_redux.md diff --git a/mkdocs/docs/developer/notifications.md b/mkdocs/docs/developer/frontend/notifications.md similarity index 100% rename from mkdocs/docs/developer/notifications.md rename to mkdocs/docs/developer/frontend/notifications.md diff --git a/mkdocs/mkdocs.yml b/mkdocs/mkdocs.yml index 5019be35..5ef2ab16 100644 --- a/mkdocs/mkdocs.yml +++ b/mkdocs/mkdocs.yml @@ -76,9 +76,12 @@ nav: - Introduction: developer/index.md - Coding Standards: developer/coding_standards.md - Data Engineering: developer/data_engineering.md - - Transport/Integration Layer (Feathers-redux): developer/feathers_redux.md - Database Support Scripts: developer/database_support_scripts.md - - Notifications with Redux Saga: developer/notifications.md - Frontend: - Component vs Page: developer/frontend/component_vs_page.md + - Transport/Integration Layer (Feathers-redux): developer/frontend/feathers_redux.md + - Notifications with Redux Saga: developer/frontend/notifications.md + - Backend: + - Permission: developer/backend/permission.md + - Services: developer/backend/services.md - Changelog: changelog.md