From a291ac56ca4059ec9ad51cc9c5735d748dd847df Mon Sep 17 00:00:00 2001 From: Daniel Bingham Date: Sun, 6 Oct 2024 14:28:34 -0400 Subject: [PATCH 1/4] Issue #257 -- Entity:Action permissions for PaperController. --- database/initialization-scripts/schema.sql | 52 ++++++++ packages/backend/index.js | 1 + .../backend/services/PermissionService.js | 43 +++++++ .../server/controllers/PaperController.js | 117 ++++++++++-------- 4 files changed, 158 insertions(+), 55 deletions(-) create mode 100644 packages/backend/services/PermissionService.js diff --git a/database/initialization-scripts/schema.sql b/database/initialization-scripts/schema.sql index 6d8b94d..9ab0333 100644 --- a/database/initialization-scripts/schema.sql +++ b/database/initialization-scripts/schema.sql @@ -593,3 +593,55 @@ CREATE TABLE response_versions ( PRIMARY KEY(response_id, version) ); +/****************************************************************************** + * Permissions + ******************************************************************************/ + + +CREATE TABLE permissions ( + user_id bigint REFERENCES users(id), + entity varchar(512), + action varchar(512), + + paper_id bigint REFERENCES papers(id) DEFAULT NULL, + paper_version_id uuid REFERENCES paper_versions(id) DEFAULT NULL, + event_id bigint REFERENCES paper_events(id) DEFAULT NULL, + review_id bigint REFERENCES reviews(id) DEFAULT NULL, + paper_comment_id bigint REFERENCES paper_comments(id) DEFAULT NULL, + submission_id bigint REFERENCES journal_submissions(id) DEFAULT NULL, + journal_id bigint REFERENCES journals(id) DEFAULT NULL +); + +CREATE TYPE role_type AS ENUM('public', 'author', 'editor', 'reviewer'); +CREATE TABLE roles ( + id bigserial PRIMARY KEY, + name varchar(1024), + short_description varchar(1024), + type role_type, + is_owner boolean, + + description text, + journal_id bigint REFERENCES journals(id) DEFAULT NULL, + paper_id bigint REFERENCES papers(id) DEFAULT NULL +); +INSERT INTO roles (name, type, description) VALUES ('public', 'public', 'The general public.'); + +CREATE TABLE role_permissions ( + role_id bigint REFERENCES roles(id) DEFAULT NULL, + permission permission_type, + + paper_id bigint REFERENCES papers(id) DEFAULT null, + version int DEFAULT null, + event_id bigint REFERENCES paper_events(id) DEFAULT NULL, + review_id bigint REFERENCES reviews(id) DEFAULT null, + paper_comment_id bigint REFERENCES paper_comments(id) DEFAULT NULL, + submission_id bigint REFERENCES journal_submissions(id) DEFAULT NULL, + journal_id bigint REFERENCES journals(id) DEFAULT NULL +); +INSERT INTO role_permissions (role_id, permission) + SELECT roles.id, 'Papers:create' FROM roles WHERE roles.name = 'public'; + +CREATE TABLE user_roles ( + role_id bigint REFERENCS roles(id) DEFAULT NULL, + user_id bigint REFERENCES users(id) DEFAULT NULL +); diff --git a/packages/backend/index.js b/packages/backend/index.js index b96d848..62f346a 100644 --- a/packages/backend/index.js +++ b/packages/backend/index.js @@ -30,6 +30,7 @@ exports.OpenAlexService = require('./services/OpenAlexService') exports.PageMetadataService = require('./services/PageMetadataService') exports.PaperEventService = require('./services/PaperEventService') exports.PaperService = require('./services/PaperService') +exports.PermissionService = require('./services/PermissionsService') exports.ReputationGenerationService = require('./services/ReputationGenerationService') exports.ReputationPermissionService = require('./services/ReputationPermissionService') exports.S3FileService = require('./services/S3FileService') diff --git a/packages/backend/services/PermissionService.js b/packages/backend/services/PermissionService.js new file mode 100644 index 0000000..d6627f6 --- /dev/null +++ b/packages/backend/services/PermissionService.js @@ -0,0 +1,43 @@ +/****************************************************************************** + * + * JournalHub -- Universal Scholarly Publishing + * Copyright (C) 2022 - 2024 Daniel Bingham + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + ******************************************************************************/ +module.exports = class PermissionService { + constructor(core) { + this.core = core + } + + /** + * Can `user` perform `action` on `entity` identified by `context. + * + * @returns {boolean} True if the `user` can perform `action` on `entity` + * identified by `context`, false otherwise. + */ + async can(user, action, entity, context) { + + } + + async let(user, action, entity, context) { + + } + + async has(user, role, context) { + + } + +} diff --git a/web-application/server/controllers/PaperController.js b/web-application/server/controllers/PaperController.js index 33d4f7d..42da085 100644 --- a/web-application/server/controllers/PaperController.js +++ b/web-application/server/controllers/PaperController.js @@ -45,9 +45,10 @@ module.exports = class PaperController { this.journalSubmissionDAO = new backend.JournalSubmissionDAO(core) this.submissionService = new backend.SubmissionService(core) - this.PaperService = new backend.PaperService(core) + this.paperService = new backend.PaperService(core) this.paperEventService = new backend.PaperEventService(core) this.notificationService = new backend.NotificationService(core) + this.permissionService = new backend.PermissionService(core) } @@ -178,17 +179,17 @@ module.exports = class PaperController { // Preprints the session user can review. if ( query.type == 'preprint') { - visibleIds = await this.PaperService.getPreprints() + visibleIds = await this.paperService.getPreprints() // Retrieves all of a } else if (session.user && query.type == 'drafts' ) { - visibleIds = await this.PaperService.getDrafts(session.user.id) + visibleIds = await this.paperService.getDrafts(session.user.id) } else if ( session.user && query.type == 'private-drafts' ) { - visibleIds = await this.PaperService.getPrivateDrafts(session.user.id) + visibleIds = await this.paperService.getPrivateDrafts(session.user.id) } else if ( session.user && query.type == 'user-submissions' ) { - visibleIds = await this.PaperService.getUserSubmissions(session.user.id) + visibleIds = await this.paperService.getUserSubmissions(session.user.id) } else if (session.user && query.type == 'review-submissions' ) { - visibleIds = await this.PaperService.getVisibleDraftSubmissions(session.user.id) + visibleIds = await this.paperService.getVisibleDraftSubmissions(session.user.id) } else if ( session.user && query.type == 'assigned-review' ) { const assignedResults = await this.database.query(` SELECT journal_submissions.paper_id @@ -491,7 +492,8 @@ module.exports = class PaperController { * Permissions Checking and Input Validation * * 1. User is logged in. - * 2. User is an author and owner of the paper being submitted. + * 2. User must have 'create' permissions on 'paper'. + * 3. User is an author and owner of the paper being submitted. * * Data validation: * @@ -510,7 +512,14 @@ module.exports = class PaperController { const user = request.session.user - // 2. User is an author and owner of the paper being submitted. + // 2. User must have 'create' permissions on 'paper'. + const canCreate = await this.permissionService.can(user, 'create', 'Paper') + if ( ! canCreate ) { + throw new ControllerError(403, 'not-authorized', + `User(${user.id}) attempted to create a paper without permissions.`) + } + + // 3. User is an author and owner of the paper being submitted. if ( ! paper.authors.find((a) => a.userId == user.id && a.owner) ) { throw new ControllerError(403, 'not-authorized:not-owner', `User(${user.id}) submitted a paper with out being an owner of that paper!`) @@ -658,29 +667,30 @@ module.exports = class PaperController { * @returns {Promise} Resolves to void. */ async getPaper(request, response) { - const results = await this.paperDAO.selectPapers('WHERE papers.id=$1', [request.params.id]) - /************************************************************* * Permissions Checking and Input Validation * - * 1. If the paper is a draft, user must be logged in and have review - * privileges on that draft. + * 1. User must have 'view' permissions on 'paper'. + * * * **********************************************************/ + const currentUser = request.session.user - if ( ! results.dictionary[request.params.id] ) { - throw new ControllerError(404, 'not-found', `Paper(${request.params.id}) not found.`) + // 1. User must have 'view' permissions on 'paper'. + const canView = await this.permissionService.can(currentUser, 'view', 'Paper', { paperId: request.params.id }) + if ( ! canView ) { + throw new ControllerError(403, 'not-authorized', + `User attempted to access a Paper they were not authorized to view.`) } - const paper = results.dictionary[request.params.id] - if ( paper.isDraft ) { - if ( ! request.session.user && ! paper.showPreprint ) { - throw new ControllerError(403, 'not-authenticated', `Unauthenticated user attempting to view draft.`) - } - // TODO update visibility permissions + const results = await this.paperDAO.selectPapers('WHERE papers.id=$1', [request.params.id]) + + if ( ! results.dictionary[request.params.id] ) { + throw new ControllerError(404, 'not-found', `Paper(${request.params.id}) not found.`) } + const paper = results.dictionary[request.params.id] /************************************************************ * Permissions Checking Complete @@ -694,18 +704,6 @@ module.exports = class PaperController { }) } - /** - * PUT /paper/:id - * - * Replace an existing paper wholesale with the provided JSON. - * - * NOTE: Intentionally left unimplemented until we have a need for it, or - * have time to decide how to secure it. - */ - async putPaper(request, response) { - throw new ControllerError(501, 'not-implemented', `Attempt to put a paper, when PUT /paper/:id is unimplemented.`) - } - /** * PATCH /paper/:id * @@ -722,21 +720,21 @@ module.exports = class PaperController { * @returns {Promise} Resolves to void. */ async patchPaper(request, response) { - const paper = request.body - // We want to use the params.id over any id in the body. - paper.id = request.params.id - /************************************************************* * Permissions Checking and Input Validation * * 1. User must be logged in. - * 2. Paper(:paper_id) must exist. - * 3. User must be an owning author on Paper(:paper_id). + * 2. User must have 'edit' on Paper(:paperId) + * 3. Paper(:paper_id) must exist. * 4. Paper(:paper_id) must be a draft. * 5. Only title and isDraft may be patched. * * **********************************************************/ + const paper = request.body + // We want to use the params.id over any id in the body. + paper.id = request.params.id + // 1. User must be logged in. if ( ! request.session.user ) { throw new ControllerError(401, 'not-authenticated', `Unauthenticated user attempting to patch paper(${paper.id}).`) @@ -744,24 +742,25 @@ module.exports = class PaperController { const user = request.session.user + // 2. User must have 'edit' on Paper(:paperId) + const canEdit = await this.permissionService.can(user, 'edit', 'Paper', { paperId: paper.id }) + if ( ! canEdit ) { + throw new ControllerError(403, 'not-authorized', + `User(${user.id}) attempted to edit Paper(${paper.id}) without permissions.`) + } + const existingResults = await this.paperDAO.selectPapers('WHERE papers.id=$1', [ paper.id ]) const existing = existingResults.dictionary[paper.id] - // 2. Paper(:paper_id) must exist. + // 3. Paper(:paper_id) must exist. if ( ! existing ) { throw new ControllerError(404, 'not-found', `Attempt to patch a paper(${paper.id}) that doesn't exist!`) } - // 3. User must be an owning author on the Paper(:paper_id) - if ( ! existing.authors.find((a) => a.userId == user.id && a.owner) ) { - throw new ControllerError(403, 'not-authorized:not-owner', - `Non-owner user(${user.id}) attempting to PATCH paper(${paper.id}).`) - } - // 4. Paper(:paper_id) must be a draft. if ( ! existing.isDraft ) { throw new ControllerError(403, `not-authorized:published`, - `User(${user.id}) attempting to PATCH a published paper.`) + `User(${user.id}) attempting to PATCH published Paper(${paper.id}).`) } @@ -831,34 +830,38 @@ module.exports = class PaperController { * @returns {Promise} Resolves to void. */ async deletePaper(request, response) { - const paperId = request.params.id - /************************************************************* * Permissions Checking and Input Validation * * 1. User must be logged in. - * 2. Paper(:paper_id) must exist. - * 3. User must be an owning author on Paper(:paper_id). + * 2. User must have 'delete' on Paper(:paperId) + * 3. Paper(:paper_id) must exist. * 4. Paper(:paper_id) must be a draft. * * **********************************************************/ + const paperId = request.params.id // 1. User must be logged in. if ( ! request.session.user ) { - throw new ControllerError(403, 'not-authorized', `Unauthenticated user attempting to delete paper(${request.params.id}).`) + throw new ControllerError(403, 'not-authorized', `Unauthenticated user attempting to delete paper(${paperId}).`) } const user = request.session.user + + // 2. User must have 'delete' on Paper(:paperId) + const canDelete = await this.permissionService.can(user, 'delete', 'Paper', { paperId: paperId }) + if ( ! canDelete ) { + throw new ControllerError(403, 'not-authorized', + `User(${user.id}) attempted to DELETE Paper(${paperId}) without permissions.`) + } const existingResults = await this.database.query(` - SELECT paper_authors.user_id, paper_authors.owner, papers.is_draft as "isDraft" + SELECT papers.is_draft as "isDraft" FROM papers - JOIN paper_authors on papers.id = paper_authors.paper_id - WHERE papers.id = $1 AND paper_authors.user_id = $2 AND owner = true + WHERE papers.id = $1 `, [ paperId, user.id]) // 2. Paper(:paper_id) must exist. - // 3. User must be an owning author on Paper(:paper_id) if ( existingResults.rows.length <= 0 ) { throw new ControllerError(403, 'not-owner', `Non-owner user(${user.id}) attempting to delete paper(${request.params.id}).`) @@ -908,6 +911,10 @@ module.exports = class PaperController { const user = request.session.user + const visibleSubmissionsResults = await this.database.query(` + SELECT submission_id FROM permissions WHERE entity='Submission' AND action='view' AND paper_id = $1 + `, [ paperId ]) + const paperAuthorResults = await this.database.query(` SELECT paper_authors.user_id FROM papers From 51e8d41c68abf4cc2e5a39c0c0cae6ed72797b6f Mon Sep 17 00:00:00 2001 From: Daniel Bingham Date: Thu, 17 Oct 2024 22:07:33 -0400 Subject: [PATCH 2/4] Issue #257 -- Progress on permissions. --- database/initialization-scripts/schema.sql | 45 ++---- documentation/permission-system.md | 54 +++++++ packages/backend/errors/ServiceError.js | 20 +++ packages/backend/index.js | 1 + .../backend/services/PermissionService.js | 59 ++++++++ packages/backend/services/RoleService.js | 133 ++++++++++++++++++ .../backend/services/SubmissionService.js | 59 ++++---- .../server/controllers/JournalController.js | 15 +- .../server/controllers/PaperController.js | 122 ++++++++-------- .../controllers/PaperVersionController.js | 26 +++- 10 files changed, 404 insertions(+), 130 deletions(-) create mode 100644 documentation/permission-system.md create mode 100644 packages/backend/services/RoleService.js diff --git a/database/initialization-scripts/schema.sql b/database/initialization-scripts/schema.sql index 9ab0333..9ed47fd 100644 --- a/database/initialization-scripts/schema.sql +++ b/database/initialization-scripts/schema.sql @@ -599,49 +599,32 @@ CREATE TABLE response_versions ( CREATE TABLE permissions ( - user_id bigint REFERENCES users(id), - entity varchar(512), - action varchar(512), + entity varchar(512) NOT NULL, + action varchar(512) NOT NULL, + + user_id bigint REFERENCES users(id) DEFAULT NULL, + role_id uuid REFERENCES roles(id) DEFAULT NULL, paper_id bigint REFERENCES papers(id) DEFAULT NULL, paper_version_id uuid REFERENCES paper_versions(id) DEFAULT NULL, event_id bigint REFERENCES paper_events(id) DEFAULT NULL, review_id bigint REFERENCES reviews(id) DEFAULT NULL, - paper_comment_id bigint REFERENCES paper_comments(id) DEFAULT NULL, - submission_id bigint REFERENCES journal_submissions(id) DEFAULT NULL, - journal_id bigint REFERENCES journals(id) DEFAULT NULL + paper_comment_id bigint REFERENCES paper_comments(id) DEFAULT NULL, + submission_id bigint REFERENCES journal_submissions(id) DEFAULT NULL, + journal_id bigint REFERENCES journals(id) DEFAULT NULL ); -CREATE TYPE role_type AS ENUM('public', 'author', 'editor', 'reviewer'); CREATE TABLE roles ( - id bigserial PRIMARY KEY, - name varchar(1024), - short_description varchar(1024), - type role_type, - is_owner boolean, + id uuid PRIMARY KEY NOT NULL DEFAULT gen_random_uuid(), + name varchar(1024) NOT NULL, + description varchar(1024) NOT NULL, - description text, journal_id bigint REFERENCES journals(id) DEFAULT NULL, paper_id bigint REFERENCES papers(id) DEFAULT NULL ); -INSERT INTO roles (name, type, description) VALUES ('public', 'public', 'The general public.'); - -CREATE TABLE role_permissions ( - role_id bigint REFERENCES roles(id) DEFAULT NULL, - permission permission_type, - - paper_id bigint REFERENCES papers(id) DEFAULT null, - version int DEFAULT null, - event_id bigint REFERENCES paper_events(id) DEFAULT NULL, - review_id bigint REFERENCES reviews(id) DEFAULT null, - paper_comment_id bigint REFERENCES paper_comments(id) DEFAULT NULL, - submission_id bigint REFERENCES journal_submissions(id) DEFAULT NULL, - journal_id bigint REFERENCES journals(id) DEFAULT NULL -); -INSERT INTO role_permissions (role_id, permission) - SELECT roles.id, 'Papers:create' FROM roles WHERE roles.name = 'public'; +INSERT INTO roles (name, description) VALUES ('public', 'The general public.'); CREATE TABLE user_roles ( - role_id bigint REFERENCS roles(id) DEFAULT NULL, - user_id bigint REFERENCES users(id) DEFAULT NULL + role_id uuid REFERENCS roles(id) NOT NULL, + user_id bigint REFERENCES users(id) NOT NULL ); diff --git a/documentation/permission-system.md b/documentation/permission-system.md new file mode 100644 index 0000000..84fffab --- /dev/null +++ b/documentation/permission-system.md @@ -0,0 +1,54 @@ +# Entity:Action Permission System + +JournalHub uses an Entity:Action permission system where `action` is granted to +`user` on `entity`. This has a number of benefits and some costs. + +The benefits are: + +* Querying for permissions is fast, cheap, and easy. +* High level of flexibility in terms of what `action` we can define. Eg. `identify` +* Enables user defined permission models. + +The costs are: + +* Granting permissions is difficult and expensive. We need to make sure we're thorough. + +## The Entities + +The top level entities are defined by the controller/DAO combinations. Any +entity that has both a controller and DAO is considered a top-level entities. +Permissions can also be granted to sub-entities using `:` as a separator. For +example, to grant a permission on a Paper Author entity, you would use +`Paper:author`. Top level entities are always capitalized, while sub-entities +are always lower case. + +Currently, the top level entities are: + +* Field +* File +* Journal +* JournalSubmission +* Notification +* PaperComment +* Paper +* PaperEvent +* PaperVersion +* Review +* Token +* User + +## Actions + +The actions that may be granted correspond to the basic CRUD actions +and enable each of the REST endpoints. + +They are: + +* `create` enabling `POST` +* `read` enabling `GET` +* `update` enabling `PATCH` +* `delete` enabling `DELETE` + +Additional actions that don't correspond to the basic CRUD are: + +* `identify` allowing a user to identify an anonymous individual diff --git a/packages/backend/errors/ServiceError.js b/packages/backend/errors/ServiceError.js index 89937d3..4f6e257 100644 --- a/packages/backend/errors/ServiceError.js +++ b/packages/backend/errors/ServiceError.js @@ -1,3 +1,23 @@ +/****************************************************************************** + * + * JournalHub -- Universal Scholarly Publishing + * Copyright (C) 2022 - 2024 Daniel Bingham + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + ******************************************************************************/ + module.exports = class ServiceError extends Error { constructor(type, message) { super(message) diff --git a/packages/backend/index.js b/packages/backend/index.js index 62f346a..0ed1f0b 100644 --- a/packages/backend/index.js +++ b/packages/backend/index.js @@ -33,6 +33,7 @@ exports.PaperService = require('./services/PaperService') exports.PermissionService = require('./services/PermissionsService') exports.ReputationGenerationService = require('./services/ReputationGenerationService') exports.ReputationPermissionService = require('./services/ReputationPermissionService') +exports.RoleService = require('./services/RoleService') exports.S3FileService = require('./services/S3FileService') exports.ServerSideRenderingService = require('./services/ServerSideRenderingService') exports.SessionService = require('./services/SessionService') diff --git a/packages/backend/services/PermissionService.js b/packages/backend/services/PermissionService.js index d6627f6..e09fbe3 100644 --- a/packages/backend/services/PermissionService.js +++ b/packages/backend/services/PermissionService.js @@ -17,11 +17,31 @@ * along with this program. If not, see . * ******************************************************************************/ +const ServiceError = require('../errors/ServiceError') + module.exports = class PermissionService { constructor(core) { this.core = core } + async getRoleIds(user) { + let where = '' + let params = [] + + if ( user ) { + where += 'OR user_roles.user_id = $1' + params.push(user.id) + } + + const roleResults = await this.core.database.query(` + SELECT id FROM roles + ${ user ? 'LEFT OUTER JOIN user_roles ON user_roles.role_id = roles.id' : ''} + WHERE roles.name = 'public' ${where} + `, params) + + return roleResults.rows.map((r) => r.id) + } + /** * Can `user` perform `action` on `entity` identified by `context. * @@ -29,7 +49,46 @@ module.exports = class PermissionService { * identified by `context`, false otherwise. */ async can(user, action, entity, context) { + let where = '' + let params = [ entity, action ] + + const roleIds = await this.getRoleIds(user) + if ( user ) { + where = ` AND ( user_id = ${params.length+1} OR role_id = ANY(${params.length+2}::uuid[]))` + params.push(user.id) + params.push(roleIds) + } else { + where = ` AND role_id = ANY(${params.length+1}::uuid[])` + params.push(roleIds) + } + + const contextMap = { + paperId: 'paper_id', + paperVersionId: 'paper_version_id', + eventId: 'event_id', + reviewId: 'review_id', + paperCommentId: 'paper_comment_id', + submissionId: 'submission_id', + journalId: 'journal_id' + } + for(const [key, value] of Object.entries(context)) { + if ( ! ( key in contextMap ) ) { + throw new ServiceError('invalid-context', + `Invalid context '${key}'.`) + } + + where += ` AND ${contextMap[key]} = ${params.length+1}` + params.push(value) + } + + const results = await this.core.database.query(` + SELECT user_id, role_id + FROM permissions + WHERE entity = $1 AND action = $2${where} + `, params) + + return results.rows.length > 0 } async let(user, action, entity, context) { diff --git a/packages/backend/services/RoleService.js b/packages/backend/services/RoleService.js new file mode 100644 index 0000000..7a6fd7e --- /dev/null +++ b/packages/backend/services/RoleService.js @@ -0,0 +1,133 @@ +/****************************************************************************** + * + * JournalHub -- Universal Scholarly Publishing + * Copyright (C) 2022 - 2024 Daniel Bingham + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + ******************************************************************************/ + +import ServiceError from '../errors/ServiceError' + +module.exports = class RoleService { + + constructor(core) { + this.core = core + } + + /** + * Grant a role to a user. + * + * @param {string} role The role name. + * @param {number} userId The id of the user. + * @param {Object} context The required context for the role being granted. + * + * @return {Promise} + */ + async grant(role, userId, context) { + const sql = `SELECT id FROM roles WHERE name = $1` + const params = [ role ] + + if ( context.paperId ) { + sql += ` AND paper_id = $2` + params.push(context.paperId) + } else if ( context.journalId ) { + sql += ` AND journal_id = $2` + params.push(context.journalId) + } else { + throw new ServiceError('missing-context', + `Roles may only be granted on papers or journals.`) + } + + const roleResults = await this.database.query(sql, params) + + if ( roleResults.rows.length <= 0 ) { + throw new ServiceError('missing-role', + `No Role named ${role} exists for context.`) + } else if ( roleResults.rows.length > 1 ) { + throw new ServiceError('invalid-state', + `Multiple Roles named ${role} exist for context!`) + } + + const id = roleResults.rows[0].id + + await this.database.query(` + INSERT INTO user_roles (role_id, user_id) + VALUES ($1, $2) + `, [ id, userId ]) + + return true + } + + /** + * Create the initial roles for a paper and grant the initial permissions for those roles. + * + * @param {number} paperId + * + * @return {Promise} + */ + async createPaperRoles(paperId) { + const correspondingAuthorResults = await this.core.database.query(` + INSERT INTO roles (name, description, paper_id) + VALUES + ( 'corresponding-author', 'One of this paper\'s corresponding authors.', $1) + RETURNING id + `, [ paperId ]) + + if ( correspondingAuthorResults.rows.length <= 0 ) { + throw new ServiceError('failed-insert', + `Failed to create Role 'corresponding-author' for Paper(${paperId}).`) + } + + const correspondingAuthorId = correspondingAuthorResults.rows[0].id + + await this.core.database.query(` + INSERT INTO permissions (entity, action, role_id, paper_id) + VALUES + ('Paper', 'update', $1, $2), + ('Paper', 'read', $1, $2), + ('Paper', 'delete', $1, $2), + ('Paper', 'grant', $1, $2), + ('PaperVersion', 'create', $1, $2), + ('PaperVersion', 'read', $1, $2), + ('PaperVersion', 'update', $1, $2), + ('PaperVersion', 'delete', $1, $2), + ('PaperVersion', 'grant', $1, $2) + `, [ correspondingAuthorId, paperId ]) + + const authorResults = await this.core.database.query(` + INSERT INTO roles (name, description, paper_id) + VALUES + ( 'author', 'One of this paper\'s authors.', $1) + RETURNING id + `, [ paperId ]) + + if ( authorResults.rows.length <= 0 ) { + throw new ServiceError('failed-insert', + `Failed to create Role 'author' for Paper(${paperId}).`) + } + + const authorId = authorResults.rows[0].id + + await this.core.database.query(` + INSERT INTO permissions (entity, action, role_id, paper_id) + VALUES + ('Paper', 'read', $1, $2), + ('PaperVersion', 'create', $1, $2), + ('PaperVersion', 'read', $1, $2), + `, [ authorId, paperId ]) + } + +} + diff --git a/packages/backend/services/SubmissionService.js b/packages/backend/services/SubmissionService.js index f27cf28..514e511 100644 --- a/packages/backend/services/SubmissionService.js +++ b/packages/backend/services/SubmissionService.js @@ -61,6 +61,12 @@ module.exports = class SubmissionService { this.logger = core.logger } + /** + * @param {Object} user + * @param {number} paperId + * + * @return {Promise} + */ async getActiveSubmission(user, paperId) { // Get the currently active submission for the paper. const results = await this.database.query(` @@ -121,45 +127,28 @@ module.exports = class SubmissionService { * * @param {User} user The user who's visibility we want to check. * - * @return {int[]} An array of the visible submissionIds. + * @return {Promise} An array of the visible submissionIds. */ async getVisibleSubmissionIds(user) { - const sql = ` - SELECT DISTINCT journal_submissions.id - FROM journal_submissions - LEFT OUTER JOIN journals ON journal_submissions.journal_id = journals.id - LEFT OUTER JOIN journal_members ON journal_submissions.journal_id = journal_members.journal_id - LEFT OUTER JOIN journal_submission_editors ON journal_submissions.id = journal_submission_editors.submission_id - LEFT OUTER JOIN journal_submission_reviewers ON journal_submissions.id = journal_submission_reviewers.submission_id - WHERE - journals.model = 'public' - OR (journals.model = 'open-public' AND (journal_submissions.status = 'published' - ${ user ? 'OR journal_members.user_id = $1' : ''})) - OR (journals.model = 'open-closed' AND (journal_submissions.status = 'published' - ${ user ? 'OR journal_members.user_id = $1' : '' })) - OR (journals.model = 'closed' - AND (journal_submissions.status = 'published' - ${ user ? `OR ( journal_members.permissions = 'owner' - OR journal_submission_editors.user_id = $1 - OR journal_submission_reviewers.user_id = $1 - )` : '' } - ) - ) - ` - const params = [] - if ( user ) { - params.push(user.id) - } - - const results = await this.database.query(sql, params) - - if ( results.rows.length <= 0 ) { - return [] - } - - return results.rows.map((r) => r.id) + const results = await this.database.query(` + SELECT permissions.submission_id + FROM permissions + LEFT OUTER JOIN roles ON permissions.role_id = roles.id + LEFT OUTER JOIN user_roles ON user_roles.role_id = roles.id + WHERE permissions.entity = 'JournalSubmission' + AND permissions.action = 'read' + AND (permissions.user_id = $1 OR user_roles.user_id = $1) + `, [ user.id ]) + + return results.rows.map((r) => r.submission_id) } + /** + * @param {Object} user The populated user. + * @param {number} paperId + * + * @return {Promise} + */ async canViewSubmission(user, paperId) { const visibleSubmissionIds = await this.getVisibleSubmissionIds(user) diff --git a/web-application/server/controllers/JournalController.js b/web-application/server/controllers/JournalController.js index 24a360c..bfc0d24 100644 --- a/web-application/server/controllers/JournalController.js +++ b/web-application/server/controllers/JournalController.js @@ -12,7 +12,8 @@ const { UserDAO, DAOError, SessionService, - NotificationService + NotificationService, + PermissionService } = require('@danielbingham/peerreview-backend') const ControllerError = require('../errors/ControllerError') @@ -29,6 +30,7 @@ module.exports = class JournalController { this.sessionService = new SessionService(this.core) this.notificationService = new NotificationService(this.core) + this.permissionService = new PermissionService(this.core) } async getRelations(results, requestedRelations) { @@ -231,11 +233,12 @@ module.exports = class JournalController { * Permissions Checking and Input Validation * * 1. User is authenticated. - * 2. Authenticated user must be JournalUser with 'owner' permissions. + * 2. User must have 'create' for Journal + * 3. Authenticated user must be JournalUser with 'owner' permissions. * * Data validation: * - * 3. Journal has at least 1 valid user. + * 4. Journal has at least 1 valid user. * * **********************************************************/ @@ -247,6 +250,12 @@ module.exports = class JournalController { const user = request.session.user + const canCreate = await this.permissionService.can(user, 'create', 'Journal') + if ( ! canCreate ) { + throw new ControllerError(403, 'not-authorized', + `User(${user.id}) attempted to create a Journal without permissions!`) + } + // 2. Authenticated user must be JournalUser with 'owner' permissions. // 3. Journal has at least 1 valid user. if ( ! journal.members.find((m) => m.userId == user.id && m.permissions == 'owner' )) { diff --git a/web-application/server/controllers/PaperController.js b/web-application/server/controllers/PaperController.js index 42da085..b767895 100644 --- a/web-application/server/controllers/PaperController.js +++ b/web-application/server/controllers/PaperController.js @@ -49,6 +49,7 @@ module.exports = class PaperController { this.paperEventService = new backend.PaperEventService(core) this.notificationService = new backend.NotificationService(core) this.permissionService = new backend.PermissionService(core) + this.roleService = new backend.RoleService(core) } @@ -166,6 +167,44 @@ module.exports = class PaperController { let count = 0 let and = '' + // Make sure we're only retrieving papers the user has `read` permissions on. + if ( session.user ) { + count += 1 + and = ( count > 1 ? ' AND ' : '') + + const permissionResults = await this.database.query(` + SELECT permissions.paper_id + FROM permissions + LEFT OUTER JOIN roles ON permissions.role_id = roles.id + LEFT OUTER JOIN user_roles ON user_roles.role_id = roles.id + WHERE permissions.entity = 'Paper' + AND permissions.action = 'read' + AND (permissions.user_id = $1 OR user_roles.user_id = $1 OR roles.name = 'public') + `, [ session.user.id ]) + + const visibleIds = permissionResults.rows.map((r) => r.paper_id) + + result.where += `${and} papers.id = ANY($${count}::bigint[])` + result.params.push(visibleIds) + } else { + count += 1 + and = ( count > 1 ? ' AND ' : '') + + const permissionResults = await this.database.query(` + SELECT permissions.paper_id + FROM permissions + LEFT OUTER JOIN roles ON permissions.role_id = roles.id + WHERE permissions.entity = 'Paper' + AND permissions.action = 'read' + AND roles.name = 'public' + `, []) + + const visibleIds = permissionResults.rows.map((r) => r.paper_id) + + result.where += `${and} papers.id = ANY($${count}::bigint[])` + result.params.push(visibleIds) + } + // If we're not intentionally retrieving drafts then we're getting // published papers. // @@ -619,12 +658,21 @@ module.exports = class PaperController { for ( const version of paper.versions) { version.id = await this.paperVersionDAO.insertPaperVersion(paper, version) } - + const results = await this.paperDAO.selectPapers("WHERE papers.id=$1", [paper.id]) const entity = results.dictionary[paper.id] if ( ! entity ) { throw new ControllerError(500, `server-error`, `Paper ${paper.id} does not exist after insert!`) } + + await this.roleService.createPaperRoles(entity.id) + for(const author of entity.authors) { + await this.roleService.grant( + ( author.owner ? 'corresponding-author' : 'author'), + author.userId, + { paperId: entity.id } + ) + } for(const version of paper.versions) { const event = { @@ -670,15 +718,15 @@ module.exports = class PaperController { /************************************************************* * Permissions Checking and Input Validation * - * 1. User must have 'view' permissions on 'paper'. + * 1. User must have 'read' permissions on 'paper'. * * * **********************************************************/ const currentUser = request.session.user - // 1. User must have 'view' permissions on 'paper'. - const canView = await this.permissionService.can(currentUser, 'view', 'Paper', { paperId: request.params.id }) - if ( ! canView ) { + // 1. User must have 'read' permissions on 'paper'. + const canRead = await this.permissionService.can(currentUser, 'read', 'Paper', { paperId: request.params.id }) + if ( ! canRead ) { throw new ControllerError(403, 'not-authorized', `User attempted to access a Paper they were not authorized to view.`) } @@ -724,7 +772,7 @@ module.exports = class PaperController { * Permissions Checking and Input Validation * * 1. User must be logged in. - * 2. User must have 'edit' on Paper(:paperId) + * 2. User must have 'update' on Paper(:paperId) * 3. Paper(:paper_id) must exist. * 4. Paper(:paper_id) must be a draft. * 5. Only title and isDraft may be patched. @@ -742,9 +790,9 @@ module.exports = class PaperController { const user = request.session.user - // 2. User must have 'edit' on Paper(:paperId) - const canEdit = await this.permissionService.can(user, 'edit', 'Paper', { paperId: paper.id }) - if ( ! canEdit ) { + // 2. User must have 'update' on Paper(:paperId) + const canUpdate = await this.permissionService.can(user, 'update', 'Paper', { paperId: paper.id }) + if ( ! canUpdate ) { throw new ControllerError(403, 'not-authorized', `User(${user.id}) attempted to edit Paper(${paper.id}) without permissions.`) } @@ -892,10 +940,7 @@ module.exports = class PaperController { * Permissions Checking and Input Validation * * 1. User is logged in. - * 2. If authenticated user is paper author, can see all submissions. - * 3. If authenticated user is not paper author, but is Journal Member can see: - * 3a. IF authenticated user is 'reviewer', may only view submissions in review. - * 3b. IF authenticated user is 'editor' or 'owner', may view all submissions. + * 2. User had `read` permissions on `JournalSubmission` for `paperId` * * Data validation: * @@ -909,56 +954,13 @@ module.exports = class PaperController { `User must be authenticated to create a journal!`) } - const user = request.session.user + const currentUser = request.session.user const visibleSubmissionsResults = await this.database.query(` - SELECT submission_id FROM permissions WHERE entity='Submission' AND action='view' AND paper_id = $1 - `, [ paperId ]) - - const paperAuthorResults = await this.database.query(` - SELECT paper_authors.user_id - FROM papers - LEFT OUTER JOIN paper_authors ON papers.id = paper_authors.paper_id - WHERE papers.id = $1 - `, [ paperId ]) - - if ( paperAuthorResults.rows.length <= 0 ) { - throw new ControllerError(404, 'not-found', `Paper(${paperId}) not found when requesting submissions.`) - } - - // 2. If authenticated user is paper author, can see all submissions. - const isAuthor = paperAuthorResults.rows.find((r) => r.user_id == user.id) ? true : false - if ( isAuthor ) { - const results = await this.journalSubmissionDAO.selectJournalSubmissions('WHERE journal_submissions.paper_id = $1', [ paperId ]) + SELECT submission_id FROM permissions WHERE entity='JournalSubmission' AND action='read' AND paper_id = $1 AND user_id = $2 + `, [ paperId, currentUser.id ]) - // Just return an empty result. - if ( results.list.length <= 0 ) { - return response.status(200).json([]) - } else { - return response.status(200).json(results.list) - } - } - - // 3. If authenticated user is not paper author, but is Journal Member can see: - const submissionResults = await this.database.query(` - SELECT journal_submissions.id, journal_submissions.status, journal_members.permissions - FROM journal_members - LEFT OUTER JOIN journal_submissions ON journal_submissions.journal_id = journal_members.journal_id - WHERE journal_submissions.paper_id = $1 AND journal_members.user_id = $2 - `, [ paperId, user.id ]) - - const submissionIds = [] - for(const submission of submissionResults.rows) { - // 3a. IF authenticated user is 'reviewer', may only view submissions in review. - if ( submission.permissions == 'reviewer' && submission.status == 'in-review' ) { - submissionIds.push(submission.id) - } - - // 3b. IF authenticated user is 'editor' or 'owner', may view all submissions. - else if ( submission.permissions == 'editor' || submission.permissions == 'owner' ) { - submissionIds.push(submission.id) - } - } + const submissionIds = visibleSubmissionsResults.rows.map((r) => r.id) const submissions = await this.journalSubmissionDAO.selectJournalSubmissions( 'WHERE journal_submissions.id = ANY($1::bigint[])', diff --git a/web-application/server/controllers/PaperVersionController.js b/web-application/server/controllers/PaperVersionController.js index 5deb92a..2dd0272 100644 --- a/web-application/server/controllers/PaperVersionController.js +++ b/web-application/server/controllers/PaperVersionController.js @@ -33,6 +33,7 @@ module.exports = class PaperVersionController { this.paperService = new backend.PaperService(core) this.paperEventService = new backend.PaperEventService(core) this.notificationService = new backend.NotificationService(core) + this.permissionService = new backend.PermissionService(core) } async getRelations(currentUser, results, requestedRelations) { @@ -48,6 +49,20 @@ module.exports = class PaperVersionController { requestedRelations: query.requestedRelations } + const roleIds = await this.permissionService.getRoleIds(currentUser) + const visibleVersionResults = await this.core.database.query(` + SELECT paper_version_id + FROM permissions + WHERE ( + ${ currentUser ? 'user_id = $1 OR' : ''} role_id = ANY($2::uuid[]) + ) + AND entity = 'PaperVersion' AND action = 'view' + `, ( currentUser ? [ currentUser.id, roleIds ] : [ roleIds ])) + const visiblePaperVersionIds = visibleVersionResults.rows.map((r) => r.paper_version_id) + + parsedQuery.where += ` AND paper_versions.id = ANY(${parsedQuery.params.length}::uuid[])` + parsedQuery.params.push(visiblePaperVersionIds) + if ( ! currentUser ) { parsedQuery.where += ' AND ( paper_versions.is_preprint = true OR paper_versions.is_published = true )' } else if ( currentUser ) { @@ -142,6 +157,7 @@ module.exports = class PaperVersionController { * Permissions Checking and Input Validation * * 1. User must be logged in. + * 2. User must have 'create' on PaperVersion for Paper(:paper_id) * 2. Paper(:paper_id) must exist. * 3. User must be an owning author on Paper(:paper_id). * 4. Paper(:paper_id) must be a draft. @@ -157,6 +173,8 @@ module.exports = class PaperVersionController { const currentUser = request.session.user + const canCreate = await this.permissionService.can(currentUser, 'create', 'PaperVersion', { paperId: paperId }) + const existingResults = await this.paperDAO.selectPapers('WHERE papers.id = $1', [ paperId ]) const existing = existingResults.dictionary[paperId] @@ -264,12 +282,18 @@ module.exports = class PaperVersionController { const currentUser = request.session.user // 1. Must be able to view Paper(:paperId) - const canViewPaper = await this.paperService.canViewPaper(currentUser, paperId) + const canViewPaper = await this.permissionService.can(currentUser, 'view', 'Paper', { paperId: paperId }) if ( ! canViewPaper ) { throw new ControllerError(404, 'not-found' `Attempt to view private PaperVersion(${id}) of Paper(${paperId}).`) } + const canViewPaperVersion = await this.permissionService.can(currentUser, 'view', 'Paper', { paperVersionId: id }) + if ( ! canViewPaperVersion ) { + throw new ControllerError(404, 'not-found', + `Attempt to view PaperVersion(${id}) by User(${currentUser.id}) without permissions.`) + } + const versionResult = await this.paperVersionDAO.selectPaperVersions( 'WHERE paper_versions.id = $1', [ id ] From 6178f4e7cb3da29d84cb35cc26e623126aee58a0 Mon Sep 17 00:00:00 2001 From: Daniel Bingham Date: Sun, 20 Oct 2024 13:22:18 -0400 Subject: [PATCH 3/4] Issue #257 -- Progress on Roles and Permissions. Adding proper DAOs for both roles and permissions and refactoring services to use DAOs rather than SQL directly. --- database/initialization-scripts/schema.sql | 15 +- packages/backend/daos/DAO.js | 82 +++++++++ packages/backend/daos/PermissionDAO.js | 167 ++++++++++++++++++ packages/backend/daos/RoleDAO.js | 129 ++++++++++++++ .../backend/services/PermissionService.js | 130 +++++++++----- packages/backend/services/RoleService.js | 97 +++++----- .../server/controllers/ReviewController.js | 51 +++--- 7 files changed, 548 insertions(+), 123 deletions(-) create mode 100644 packages/backend/daos/DAO.js create mode 100644 packages/backend/daos/PermissionDAO.js create mode 100644 packages/backend/daos/RoleDAO.js diff --git a/database/initialization-scripts/schema.sql b/database/initialization-scripts/schema.sql index 9ed47fd..0afe93e 100644 --- a/database/initialization-scripts/schema.sql +++ b/database/initialization-scripts/schema.sql @@ -599,6 +599,7 @@ CREATE TABLE response_versions ( CREATE TABLE permissions ( + id uuid PRIMARY KEY NOT NULL DEFAULT gen_random_uuid(), entity varchar(512) NOT NULL, action varchar(512) NOT NULL, @@ -611,7 +612,10 @@ CREATE TABLE permissions ( review_id bigint REFERENCES reviews(id) DEFAULT NULL, paper_comment_id bigint REFERENCES paper_comments(id) DEFAULT NULL, submission_id bigint REFERENCES journal_submissions(id) DEFAULT NULL, - journal_id bigint REFERENCES journals(id) DEFAULT NULL + journal_id bigint REFERENCES journals(id) DEFAULT NULL, + + created_date timestamptz, + updated_date timestamptz ); CREATE TABLE roles ( @@ -620,11 +624,16 @@ CREATE TABLE roles ( description varchar(1024) NOT NULL, journal_id bigint REFERENCES journals(id) DEFAULT NULL, - paper_id bigint REFERENCES papers(id) DEFAULT NULL + paper_id bigint REFERENCES papers(id) DEFAULT NULL, + + created_date timestamptz, + updated_date timestamptz ); INSERT INTO roles (name, description) VALUES ('public', 'The general public.'); CREATE TABLE user_roles ( role_id uuid REFERENCS roles(id) NOT NULL, - user_id bigint REFERENCES users(id) NOT NULL + user_id bigint REFERENCES users(id) NOT NULL, + + created_date timestamptz ); diff --git a/packages/backend/daos/DAO.js b/packages/backend/daos/DAO.js new file mode 100644 index 0000000..3caa060 --- /dev/null +++ b/packages/backend/daos/DAO.js @@ -0,0 +1,82 @@ +/****************************************************************************** + * + * JournalHub -- Universal Scholarly Publishing + * Copyright (C) 2022 - 2024 Daniel Bingham + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + ******************************************************************************/ + +export class DAO { + + constructor(core) { + this.core = core + } + + + /** + * @return Promise + */ + async insert(entityName, table, fieldMap, entities) { + if ( ! Array.isArray(entities) ) { + entities = [ entities ] + } + + let columns = '(' + for (const [field, meta] of Object.entries(fieldMap)) { + columns += ( columns == '(' ? '' : ', ') + field + } + + if ( columns == '(' ) { + throw new DAOError('missing-fields', + `Empty field map sent to DAO::insert().`) + } + + columns += ', created_date, updated_date)' + + let rows = '' + let params = [] + for(const entity of entities) { + let row = '(' + for(const [field, meta] of Object.entries(fieldMap)) { + if ( meta.required && ! ( meta.key in entity ) ) { + throw new DAOError('missing-field', + `Required '${meta.key}' not found in ${entityName}.`) + } + + params.push(( entity[meta.key] ? entity[meta.key] : null )) + row += ( row == '(' ? '' : ', ') + `$${params.length}` + } + row += ', now(), now())' + + if ( rows !== '' ) { + rows += ', ' + row + } else { + rows += row + } + } + + + let sql = ` + INSERT INTO ${table} ${columns} + VALUES ${rows}` + + await this.core.database.query(sql, params) + } + + async update(entityName, table, fieldMap, entities) { + + + } +} diff --git a/packages/backend/daos/PermissionDAO.js b/packages/backend/daos/PermissionDAO.js new file mode 100644 index 0000000..482a2c4 --- /dev/null +++ b/packages/backend/daos/PermissionDAO.js @@ -0,0 +1,167 @@ +/****************************************************************************** + * + * JournalHub -- Universal Scholarly Publishing + * Copyright (C) 2022 - 2024 Daniel Bingham + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + ******************************************************************************/ +import { DAO } from './DAO' + +export class PermissionDAO extends DAO { + + constructor(core) { + super(core) + + this.fieldMap = { + 'id': { + required: false, + key: 'id' + }, + 'entity': { + required: true, + key: 'entity' + }, + 'action': { + required: true, + key: 'action' + }, + 'user_id': { + required: false, + key: 'userId' + }, + 'role_id': { + required: false, + key: 'roleId' + }, + 'paper_id': { + rquired: false, + key: 'paperId' + }, + 'paper_version_id': { + required: false, + key: 'paperVersionId' + }, + 'event_id': { + required: false, + key: 'event_id' + }, + 'review_id': { + required: false, + key: 'review_id' + }, + 'paper_comment_id': { + required: false, + key: 'paperCommentId' + }, + 'submission_id': { + required: false, + key: 'submissionId' + }, + 'journal_id': { + required: false, + key: 'journalId' + } + } + + } + + /* + * @return {string} + */ + getPermissionsSelectionString() { + return ` + permissions.id as "Permission_id", + permissions.entity as "Permission_entity", + permissions.action as "Permission_action", + permissions.user_id as "Permission_userId", + permissions.role_id as "Permission_roleId", + permissions.paper_id as "Permission_paperId", + permissions.paper_version_id as "Permission_paperVersionId", + permissions.event_id as "Permission_eventId", + permissions.review_id as "Permission_reviewId", + permissions.paper_comment_id as "Permission_paperCommentId", + permissions.submission_id as "Permission_submissionId", + permissions.journal_id as "Permission_journalId", + permissions.created_date as "Permission_createdDate", + permissions.updated_date as "Permission_updatedDate" + ` + } + + /** + * @return {any} + */ + hydratePermission(row) { + return { + id: row.Permissions_id, + entity: row.Permission_entity, + action: row.Permission_action, + userId: row.Permission_userId, + roleId: row.Permission_roleId, + paperId: row.Permission_paperId, + paperVersionId: row.Permission_paperVersionId, + eventId: row.Permission_eventId, + reviewId: row.Permission_reviewId, + paperCommentId: row.Permission_paperCommentId, + submissionId: row.Permission_submissionId, + journalId: row.Permission_journalId, + createdDate: row.Permission_createdDate, + updatedDate: row.Permission_updatedDate + } + } + + /** + * @return dictionary: { [id: string]: any, list: any[]} + */ + hydratePermissions(rows) { + const dictionary = {} + const list = [] + + for(const row of rows) { + dictionary[row.Permission_id] = this.hydratePermission(row) + list.push(row.Permission_id) + } + + return { dictionary: dictionary, list: list } + } + + /** + * @param {string} where + * @param {any[]} params + * + * @return {Promise} + */ + async selectPermissions(where, params) { + where = where ? `WHERE ${where}` : '' + params = params ? params : [] + + const sql = ` + SELECT + ${this.getPermissionsSelectionString()} + FROM permissions + LEFT OUTER JOIN user_roles ON user_roles.role_id = permissions.role_id + ${where} + ` + + const results = await this.core.database.query(sql, params) + return this.hydratePermissions(results.rows) + } + + /** + * @return {Promise} + */ + async insertPermissions(permissions) { + await this.insert('Permission', 'permissions', this.fieldMap, permissions) + } +} diff --git a/packages/backend/daos/RoleDAO.js b/packages/backend/daos/RoleDAO.js new file mode 100644 index 0000000..1f3f84e --- /dev/null +++ b/packages/backend/daos/RoleDAO.js @@ -0,0 +1,129 @@ +/****************************************************************************** + * + * JournalHub -- Universal Scholarly Publishing + * Copyright (C) 2022 - 2024 Daniel Bingham + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + ******************************************************************************/ + +import { DAO } from './DAO' + +export class RoleDAO extends DAO { + constructor(core) { + super(core) + + this.fieldMap = { + 'id': { + required: false, + key: 'id' + }, + 'name': { + required: true, + key: 'name' + }, + 'description': { + required: true, + key: 'description' + }, + 'journal_id': { + required: false, + key: 'journalId' + }, + 'paper_id': { + required: false, + key: 'paperId' + } + } + } + + /** + * @return string + */ + getRoleSelectionString() { + return ` + roles.id as "Role_id", + roles.name as "Role_name", + roles.description as "Role_description", + roles.journal_id as "Role_journalId", + roles.paper_id as "Role_paperId", + roles.created_date as "Role_createdDate", + roles.updated_date as "Role_updatedDate" + ` + } + + /** + * @return any + */ + hydrateRole(row) { + return { + id: row.Role_id, + name: row.Role_name, + description: row.Role_description, + journalId: row.Role_journalId, + paperId: row.Role_paperId, + createdDate: row.Role_createdDate, + updatedDate: row.Role_updatedDate + } + } + + /** + * @return { dictionary: { [id: string]: any }, list: number[] } + */ + hydrateRoles(rows) { + const dictionary = {} + const list = [] + + for(const row of rows) { + dictionary[row.Role_id] = this.hydrateRole(row) + list.push(row.Role_id) + } + + return { dictionary: dictionary, list: list } + } + + + /** + * @return Promise<{ dictionary: [id: string]: any, list: number[] }> + */ + async selectRoles(where, params) { + where = where ? `WHERE ${where}` : '' + params = params ? params : [] + + const results = await this.core.database.query(` + SELECT + ${this.getRoleSelectionString()} + FROM roles + ${where} + `, params) + + return this.hydrateRoles(results.rows) + } + + /** + * @return Promise + */ + async getRole(id) { + const results = await this.selectRoles(`roles.id = $1`, [ id ]) + return results.dictionary[id] + } + + + /** + * @return Promise + */ + async insertRoles(roles) { + await this.insert('Role', 'roles', this.fieldMap, roles) + } +} diff --git a/packages/backend/services/PermissionService.js b/packages/backend/services/PermissionService.js index e09fbe3..ee151a7 100644 --- a/packages/backend/services/PermissionService.js +++ b/packages/backend/services/PermissionService.js @@ -17,51 +17,42 @@ * along with this program. If not, see . * ******************************************************************************/ + +const PermissionDAO = requre('../daos/PermissionDAO') + const ServiceError = require('../errors/ServiceError') module.exports = class PermissionService { constructor(core) { this.core = core - } - async getRoleIds(user) { - let where = '' - let params = [] - - if ( user ) { - where += 'OR user_roles.user_id = $1' - params.push(user.id) - } + this.permissionDAO = new PermissionDAO(this.core) - const roleResults = await this.core.database.query(` - SELECT id FROM roles - ${ user ? 'LEFT OUTER JOIN user_roles ON user_roles.role_id = roles.id' : ''} - WHERE roles.name = 'public' ${where} - `, params) - - return roleResults.rows.map((r) => r.id) + this.publicRoleId = null } /** - * Can `user` perform `action` on `entity` identified by `context. - * - * @returns {boolean} True if the `user` can perform `action` on `entity` - * identified by `context`, false otherwise. + * @return {Promise} */ - async can(user, action, entity, context) { - let where = '' - let params = [ entity, action ] + async getPublicRoleId() { + if ( this.publicRoleId !== null ) { + return this.publicRoleId + } - const roleIds = await this.getRoleIds(user) - if ( user ) { - where = ` AND ( user_id = ${params.length+1} OR role_id = ANY(${params.length+2}::uuid[]))` - params.push(user.id) - params.push(roleIds) - } else { - where = ` AND role_id = ANY(${params.length+1}::uuid[])` - params.push(roleIds) + const results = await this.core.database.query( + `SELECT id FROM roles WHERE name='public'`, + [] + ) + + if ( results.rows.length <= 0 ) { + throw new ServiceError('missing-public', 'Failed to find the public role id!') } + this.publicRoleId = results.rows[0].id + return this.publicRoleId + } + + addContextSQL(query, context) { const contextMap = { paperId: 'paper_id', paperVersionId: 'paper_version_id', @@ -77,26 +68,81 @@ module.exports = class PermissionService { `Invalid context '${key}'.`) } - where += ` AND ${contextMap[key]} = ${params.length+1}` - params.push(value) + query.where += ` AND ${contextMap[key]} = ${query.params.length+1}` + query.params.push(value) + } + return query + } + + /** + * Can `user` perform `action` on `entity` identified by `context. + * + * @returns {Promise} True if the `user` can perform `action` on `entity` + * identified by `context`, false otherwise. + */ + async can(user, action, entity, context) { + const query = { + where: 'permissions.entity = $1 and permissions.action = $2', + params: [ entity, action ] + } + + const publicRoleId = await this.getPublicRoleId() + + if ( user ) { + query.where += ` AND + ( permissions.user_id = $${query.params.length+1} + OR user_roles.user_id = $${query.params.length+1} + OR permissions.role_id = $${query.params.length+2} + )` + query.params.push(user.id) + query.params.push(publicRoleId) + } else { + query.where += ` AND permissions.role_id = $${query.params.length+1}` + query.params.push(publicRoleId) } - const results = await this.core.database.query(` - SELECT user_id, role_id - FROM permissions - WHERE entity = $1 AND action = $2${where} - `, params) + this.addContextSQL(query, context) + const results = await this.permissionDAO.selectPermissions(query.where, query.params) - return results.rows.length > 0 + return results.list.length > 0 } - async let(user, action, entity, context) { + /** + * @returns {Promise} + */ + async get(user, entity, action, context) { + const query = { + where: '', + params: [] + } - } + const publicRoleId = await this.getPublicRoleId() + + if ( user ) { + query.where += '(permissions.user_id = $1 OR user_roles.user_id = $1 OR permissions.role_id = $2)' + query.params.push(user.id, publicRoleId) + } else { + query.where += 'permissions.role_id = $1' + query.params.push(publicRoleId) + } - async has(user, role, context) { + if ( entity) { + query.params.push(entity) + query.where += ` AND permissions.entity = $${query.params.length}` + } + if ( action ) { + query.params.push(action) + query.where += ` AND permissions.action = $${query.params.length}` + } + this.addContextSQL(query,context) + + const results = await this.permissionDAO.selectPermissions(query.where, query.params) + return results.list.map((id) => results.dictionary[id]) } + async grant(permissions) { + await this.insertPermissions(permissions) + } } diff --git a/packages/backend/services/RoleService.js b/packages/backend/services/RoleService.js index 7a6fd7e..1a14178 100644 --- a/packages/backend/services/RoleService.js +++ b/packages/backend/services/RoleService.js @@ -18,12 +18,22 @@ * ******************************************************************************/ +import { Uuid } from 'uuid' + import ServiceError from '../errors/ServiceError' -module.exports = class RoleService { +import { RoleDAO } from '../daos/RoleDAO' + +import { PermissionService } from './PermissionService' + +export class RoleService { constructor(core) { this.core = core + + this.roleDAO = new RoleDAO(core) + + this.permissionService = new PermissionService(core) } /** @@ -78,56 +88,41 @@ module.exports = class RoleService { * @return {Promise} */ async createPaperRoles(paperId) { - const correspondingAuthorResults = await this.core.database.query(` - INSERT INTO roles (name, description, paper_id) - VALUES - ( 'corresponding-author', 'One of this paper\'s corresponding authors.', $1) - RETURNING id - `, [ paperId ]) - - if ( correspondingAuthorResults.rows.length <= 0 ) { - throw new ServiceError('failed-insert', - `Failed to create Role 'corresponding-author' for Paper(${paperId}).`) - } - - const correspondingAuthorId = correspondingAuthorResults.rows[0].id - - await this.core.database.query(` - INSERT INTO permissions (entity, action, role_id, paper_id) - VALUES - ('Paper', 'update', $1, $2), - ('Paper', 'read', $1, $2), - ('Paper', 'delete', $1, $2), - ('Paper', 'grant', $1, $2), - ('PaperVersion', 'create', $1, $2), - ('PaperVersion', 'read', $1, $2), - ('PaperVersion', 'update', $1, $2), - ('PaperVersion', 'delete', $1, $2), - ('PaperVersion', 'grant', $1, $2) - `, [ correspondingAuthorId, paperId ]) - - const authorResults = await this.core.database.query(` - INSERT INTO roles (name, description, paper_id) - VALUES - ( 'author', 'One of this paper\'s authors.', $1) - RETURNING id - `, [ paperId ]) - - if ( authorResults.rows.length <= 0 ) { - throw new ServiceError('failed-insert', - `Failed to create Role 'author' for Paper(${paperId}).`) - } - - const authorId = authorResults.rows[0].id - - await this.core.database.query(` - INSERT INTO permissions (entity, action, role_id, paper_id) - VALUES - ('Paper', 'read', $1, $2), - ('PaperVersion', 'create', $1, $2), - ('PaperVersion', 'read', $1, $2), - `, [ authorId, paperId ]) + const correspondingAuthorId = Uuid.v4() + const authorId = Uuid.v4() + + await this.roleDAO.insertRoles([ + { + id: correspondingAuthorId, + name: 'Corresponding Author', + description: `One of this paper's corresponding authors.`, + paperId: paperId + }, + { + id: authorId, + name: 'Author', + description: `One of this paper's authors.`, + paperId: paperId + } + ]) + + await this.permissionService.grant([ + { entity:'Paper', action:'update', roleId:correspondingAuthorId, paperId:paperId }, + { entity:'Paper', action:'read', roleId:correspondingAuthorId, paperId:paperId }, + { entity:'Paper', action:'delete', roleId:correspondingAuthorId, paperId:paperId }, + { entity:'Paper', action:'grant', roleId:correspondingAuthorId, paperId:paperId }, + { entity:'PaperVersion', action:'create', roleId:correspondingAuthorId, paperId:paperId }, + { entity:'PaperVersion', action:'read', roleId:correspondingAuthorId, paperId:paperId }, + { entity:'PaperVersion', action:'update', roleId:correspondingAuthorId, paperId:paperId }, + { entity:'PaperVersion', action:'delete', roleId:correspondingAuthorId, paperId:paperId }, + { entity:'PaperVersion', action:'grant', roleId:correspondingAuthorId, paperId:paperId } + ]) + + await this.permissionService.grant([ + { entity:'Paper', action:'read', roleId:authorId, paperId:paperId }, + { entity:'PaperVersion', action:'create', roleId:authorId, paperId:paperId }, + { entity:'PaperVersion', action:'read', roleId:authorId, paperId:paperId } + ]) } - } diff --git a/web-application/server/controllers/ReviewController.js b/web-application/server/controllers/ReviewController.js index b2cd007..563c2f9 100644 --- a/web-application/server/controllers/ReviewController.js +++ b/web-application/server/controllers/ReviewController.js @@ -41,6 +41,9 @@ module.exports = class ReviewController { this.paperService = new backend.PaperService(core) this.paperEventService = new backend.PaperEventService(core) this.notificationService = new backend.NotificationService(core) + + this.permissionService = new backend.PermissionService(core) + this.roleService = new backend.RoleService(core) } async getRelations(currentUser, results, requestedRelations) { @@ -90,6 +93,7 @@ module.exports = class ReviewController { async getReviews(request, response) { const paperId = request.params.paper_id + const currentUser = request.session.user const userId = request.session.user?.id /************************************************************* @@ -105,23 +109,16 @@ module.exports = class ReviewController { * **********************************************************/ - const canViewPaper = await this.paperService.canViewPaper(request.session.user, paperId) + const canReadPaper = await this.permissionService.can(currentUser, 'read', 'Paper', { paperId: paperId }) // 1. Paper(:paper_id) exists. // 2. Paper(:paper_id) is visible to CurrentUser or Public. - if ( ! canViewPaper ) { + if ( ! canReadPaper ) { throw new ControllerError(404, 'no-resource', `No Paper(${paperId}) to return reviews for.`) } - // 2. Visibility is controlled on the event. - // TECHDEBT This is not going to be efficient. - const visibleIds = await this.paperEventService.getVisibleEventIds(userId) - const eventResults = await this.database.query(` - SELECT review_id FROM paper_events WHERE id = ANY($1::bigint[]) - `, [ visibleIds ]) - - const reviewIds = eventResults.rows.map((r) => r.review_id) - + const permissions = await this.permissionService.get('currentUser', 'read', 'Review', { paperId: paperId }) + const reviewIds = permissions.map((p) => p.reviewId) /******************************************************** * Permission Checks Complete @@ -131,13 +128,8 @@ module.exports = class ReviewController { let where = '' let params = [] - if ( userId ) { - where = `WHERE reviews.paper_id = $1 AND (reviews.id = ANY($2::bigint[]) OR reviews.user_id = $3)` - params = [ paperId, reviewIds, userId ] - } else { - where = `WHERE reviews.paper_id = $1 AND reviews.id = ANY($2::bigint[])` - params = [ paperId, reviewIds ] - } + where = `WHERE reviews.paper_id = $1 AND reviews.id = ANY($2::bigint[])` + params = [ paperId, reviewIds ] const results = await this.reviewDAO.selectReviews(where, params) results.meta = await this.reviewDAO.countReviews(where, params) @@ -165,10 +157,6 @@ module.exports = class ReviewController { * @returns {Promise} Resolves to void. */ async postReviews(request, response) { - const paperId = request.params.paper_id - - const review = request.body - /************************************************************* * Permissions Checking and Input Validation * @@ -187,22 +175,31 @@ module.exports = class ReviewController { * * ***********************************************************/ + const paperId = request.params.paper_id + const review = request.body + + const currentUser = request.session.user + // 1. Be logged in. - if ( ! request.session.user ) { + if ( ! currentUser ) { throw new ControllerError(401, 'not-authenticated', `Unauthenticated user attempted to POST review on Paper(${review.paperId}).`) } - const userId = request.session.user.id - - const canViewPaper = await this.paperService.canViewPaper(request.session.user, paperId) + const canReadPaper = await this.permissionService.can(currentUser, 'read', 'Paper', { paperId: paperId }) // 2. Paper(:paper_id) exists. // 3. Paper(:paper_id) is visible to CurrentUser. - if ( ! canViewPaper ) { + if ( ! canReadPaper ) { throw new ControllerError(404, 'no-resource', `No Paper(${paperId}) to return reviews for.`) } + const canCreateReview = await this.permissionService.can(currentUser, 'create', 'Review', { paperId: paperId }) + if ( ! canCreateReview ) { + throw new ControlleError(403, 'not-authorized', + `User(${currentUser.id}) attempted to POST Review to Paper(${paperId}) when not authorized.`) + } + /******************************************************** * Permissions Checks Complete * Begin Input Validation From 74bcc99facbe0c7f6a6df29289b471876b55d4ca Mon Sep 17 00:00:00 2001 From: Daniel Bingham Date: Mon, 21 Oct 2024 09:11:32 -0400 Subject: [PATCH 4/4] Issue #257 -- Progress on permissions. Futher improvements to RoleDAO, PermissionService, and RoleService. Progress on ReviewController permissions. --- packages/backend/daos/RoleDAO.js | 57 +++-- .../backend/services/PermissionService.js | 6 +- packages/backend/services/RoleService.js | 5 +- .../server/controllers/PaperController.js | 2 +- .../server/controllers/ReviewController.js | 238 ++++++------------ 5 files changed, 117 insertions(+), 191 deletions(-) diff --git a/packages/backend/daos/RoleDAO.js b/packages/backend/daos/RoleDAO.js index 1f3f84e..e11c3d0 100644 --- a/packages/backend/daos/RoleDAO.js +++ b/packages/backend/daos/RoleDAO.js @@ -25,25 +25,37 @@ export class RoleDAO extends DAO { super(core) this.fieldMap = { - 'id': { - required: false, - key: 'id' + 'roles': { + 'id': { + required: false, + key: 'id' + }, + 'name': { + required: true, + key: 'name' + }, + 'description': { + required: true, + key: 'description' + }, + 'journal_id': { + required: false, + key: 'journalId' + }, + 'paper_id': { + required: false, + key: 'paperId' + } }, - 'name': { - required: true, - key: 'name' - }, - 'description': { - required: true, - key: 'description' - }, - 'journal_id': { - required: false, - key: 'journalId' - }, - 'paper_id': { - required: false, - key: 'paperId' + 'user_roles': { + 'user_id': { + required: true, + key: 'userId' + }, + 'role_id': { + required: true, + key: 'roleId' + } } } } @@ -124,6 +136,13 @@ export class RoleDAO extends DAO { * @return Promise */ async insertRoles(roles) { - await this.insert('Role', 'roles', this.fieldMap, roles) + await this.insert('Role', 'roles', this.fieldMap['roles'], roles) + } + + /** + * @return Promise + **/ + async insertUserRoles(userRoles) { + await this.insert('UserRole', 'user_roles', this.fieldMap['userRoles'], userRoles) } } diff --git a/packages/backend/services/PermissionService.js b/packages/backend/services/PermissionService.js index ee151a7..296f28d 100644 --- a/packages/backend/services/PermissionService.js +++ b/packages/backend/services/PermissionService.js @@ -127,16 +127,16 @@ module.exports = class PermissionService { query.params.push(publicRoleId) } - if ( entity) { + if ( entity && entity !== '*' ) { query.params.push(entity) query.where += ` AND permissions.entity = $${query.params.length}` } - if ( action ) { + if ( action && action !== '*' ) { query.params.push(action) query.where += ` AND permissions.action = $${query.params.length}` } - this.addContextSQL(query,context) + this.addContextSQL(query, context) const results = await this.permissionDAO.selectPermissions(query.where, query.params) return results.list.map((id) => results.dictionary[id]) diff --git a/packages/backend/services/RoleService.js b/packages/backend/services/RoleService.js index 1a14178..e419c60 100644 --- a/packages/backend/services/RoleService.js +++ b/packages/backend/services/RoleService.js @@ -72,10 +72,7 @@ export class RoleService { const id = roleResults.rows[0].id - await this.database.query(` - INSERT INTO user_roles (role_id, user_id) - VALUES ($1, $2) - `, [ id, userId ]) + await this.roleDAO.insertUserRoles({ roleId: id, userId: userId }) return true } diff --git a/web-application/server/controllers/PaperController.js b/web-application/server/controllers/PaperController.js index b767895..041fb9c 100644 --- a/web-application/server/controllers/PaperController.js +++ b/web-application/server/controllers/PaperController.js @@ -668,7 +668,7 @@ module.exports = class PaperController { await this.roleService.createPaperRoles(entity.id) for(const author of entity.authors) { await this.roleService.grant( - ( author.owner ? 'corresponding-author' : 'author'), + ( author.owner ? 'Corresponding Author' : 'Author'), author.userId, { paperId: entity.id } ) diff --git a/web-application/server/controllers/ReviewController.js b/web-application/server/controllers/ReviewController.js index 563c2f9..223bb3f 100644 --- a/web-application/server/controllers/ReviewController.js +++ b/web-application/server/controllers/ReviewController.js @@ -196,7 +196,7 @@ module.exports = class ReviewController { const canCreateReview = await this.permissionService.can(currentUser, 'create', 'Review', { paperId: paperId }) if ( ! canCreateReview ) { - throw new ControlleError(403, 'not-authorized', + throw new ControllerError(403, 'not-authorized', `User(${currentUser.id}) attempted to POST Review to Paper(${paperId}) when not authorized.`) } @@ -254,8 +254,19 @@ module.exports = class ReviewController { if ( ! entity ) { throw new ControllerError(500, 'server-error', `Failed to find newly inserted review ${review.id}.`) } - - + + // Grant permissions on the newly created entity. + // + // For now we're just going to grant the author appropriate permissions and we'll handle granting + await this.permissionService.grant([ + { entity: 'Review', action: 'read', userId: userId, paperId: entity.paperId, reviewId: entity.id }, + { entity: 'Review', action: 'update', userId: userId, paperId: entity.paperId, reviewId: entity.id }, + { entity: 'Review', action: 'delete',userId: userId, paperId: entity.paperId, reviewId: entity.id }, + { entity: 'Review', action: 'grant', userId: userId, paperId: entity.paperId, reviewId: entity.id } + ]) + + // Create the event for the review. + const event = { paperId: entity.paperId, actorId: userId, @@ -266,6 +277,7 @@ module.exports = class ReviewController { } await this.paperEventService.createEvent(request.session.user, event) + // Send notifications for the created review. if ( entity.status == 'submitted' ) { // Update the review count on the version @@ -309,29 +321,19 @@ module.exports = class ReviewController { * @returns {Promise} Resolves to void. */ async getReview(request, response) { - const paperId = request.params.paper_id - const reviewId = request.params.id - /************************************************************* * Permissions Checking and Input Validation * * We need to do basic input validation. * - * 1. Review(:review_id) exists. - * 2. Review(:review_id) is on Paper(:paper_id) - * - * Then we need to check the view permissions. They are different - * depending on whether Paper(:paper_id) is a draft or not. - * - * 3. Review(:review_id) is in-progress AND User is logged in AND User - * is author of Review(:review_id) - * 4. Review(:review_id) is not in-progress AND Paper(:paper_id) is a - * draft AND User is logged in and has Review permissions. - * 5. Review(:review_id) is not in-progress AND Paper(:paper_id) is NOT - * a draft. (Anyone may view.) - * + * 1. Review(:reviewId) exists. + * 2. Review(:reviewId) is on Paper(:paperId) + * 3. CurrentUser has 'read' on Paper(:paperId) + * 4. CurrentUser has 'read' on Review(:reviewId) * * ***********************************************************/ + const paperId = request.params.paper_id + const reviewId = request.params.id const existingResults = await this.database.query(` SELECT @@ -357,43 +359,17 @@ module.exports = class ReviewController { `Review(${reviewId}) is on Paper(${existing.paper_id}) not Paper(${paperId}).`) } - // 3. Review(:review_id) is in-progress AND User is logged in AND User - // is author of Review(:review_id) - if ( existing.review_status == 'in-progress') { - // ... AND User is logged in - if ( ! request.session.user ) { - // Return a 404, so we don't let them know that a review they - // aren't allowed to see exists. - throw new ControllerError(404, 'no-resource', - `Unauthenticated user attempted to view in-progress Review(${reviewId}).`) - } - - // ...AND User is author of Review(:review_id) - if ( existing.review_userId != request.session.user.id ) { - // Return a 404, so we don't let them know that a review they - // aren't allowed to see exists. - throw new ControllerError(404, 'no-resource', - `User(${request.session.user.id}) attempted to view in-progress Review(${reviewId}) they didn't write.`) - } - } - - // 4. Review(:review_id) is not in-progress AND Paper(:paper_id) is a - // draft AND User is logged in and has Review permissions. - - if ( existing.review_status != 'in-progress' && existing.paper_isDraft ) { - if ( ! request.session.user ) { - // Return a 404, so we don't let them know that a review they - // aren't allowed to see exists. - throw new ControllerError(404, 'no-resource', - `Unauthenticated user attempted to view Review(${reviewId}) on draft Paper(${paperId}).`) - } - + // 3. CurrentUser has 'read' on Paper(:paperId) + const canReadPaper = await this.permissionService.can(currentUser, 'read', 'Paper', { paperId: paperId }) + if ( ! canReadPaper ) { + throw new ControllerError(404, 'no-resource', `No Paper(${paperId}) to return reviews for.`) } - // 5. Review(:review_id) is not in-progress then CurrentUser must have permission to view the paper. - const canViewPaper = await this.paperService.canViewPaper(request.session.user, paperId) - if ( ! canViewPaper ) { - throw new ControllerError(404, 'no-resource', `No Paper(${paperId}) or User doesn't have permission.`) + // 4. CurrentUser has 'read' on Review(:reviewId) + const canCreateReview = await this.permissionService.can(currentUser, 'read', 'Review', { paperId: paperId, reviewId: reviewId }) + if ( ! canCreateReview ) { + throw new ControllerError(403, 'not-authorized', + `User(${currentUser?.id}) attempted to GET Review(${reviewId}) of Paper(${paperId}) when not authorized.`) } /******************************************************** @@ -418,28 +394,6 @@ module.exports = class ReviewController { return response.status(200).json({ entity: results.dictionary[reviewId], relations: results.relations }) } - /** - * PUT /paper/:paper_id/review/:id - * - * Replace an existing review wholesale with the provided JSON. - * - * NOT IMPLEMENTED - */ - async putReview(request, response) { - throw new ControllerError(501, 'not-implemented', - `Attempt to call unimplemented PUT /paper/:paper_id/review/:id.`) - - // =================================================================== - // ############## Intentionally Left Unimplemented ################## - // - // This is not wired into the controller. It is intentionally left - // unimplemented because we don't actually want to allow users to - // replace their reviews wholesale in any circumstances. They are - // allowed to edit select fields through PATCH. - // - // =================================================================== - } - /** * PATCH /paper/:paper_id/review/:review_id * @@ -460,33 +414,28 @@ module.exports = class ReviewController { * @returns {Promise} Resolves to void. */ async patchReview(request, response) { - const paperId = request.params.paper_id - const reviewId = request.params.review_id - - const review = request.body - /************************************************************* * Permissions Checking and Input Validation * * To call this endpoint you must: * * 1. Be logged in. - * 2. Be the author of Review(:review_id) OR an owning author of - * Paper(:paper_id) + * 2. CurrentUser must have 'update' on Review(${reviewId}) * * Then we need to do basic input validation: * - * 3. Review(:review_id) exists. - * 4. Review(:review_id) is on Paper(:paper_id) + * 3. Review(:reviewI) exists. + * 4. Review(:reviewId) is on Paper(:paper_id) * * Finally, we need to validate the review content (PATCH body): * - * 7. Paper authors may only edit the `status` field. - * 8. Review authors may only edit the `status`, `summary`, and `recommendation` - * fields. Status may only be 'submitted' when it is currently 'in-progress'. - * 9. No one may PATCH `paperId`, `userId`, `version`, or `number` + * 5. No one may PATCH `paperId`, `userId`, `version`, or `number` * * ***********************************************************/ + const paperId = request.params.paper_id + const reviewId = request.params.review_id + + const review = request.body // 1. Be logged in. if ( ! request.session.user ) { @@ -494,58 +443,19 @@ module.exports = class ReviewController { 'not-authenticated', `Unauthenticated user attempted to PATCH review on Paper(${review.paperId}).`) } + const currentUser = request.session.user const userId = request.session.user.id - const existingResults = await this.database.query(` - SELECT - papers.id as paper_id, papers.is_draft as "paper_isDraft", - reviews.id as review_id, reviews.user_id as "review_userId", reviews.status as "review_status" - FROM reviews - JOIN papers on reviews.paper_id = papers.id - WHERE reviews.id = $1 - `, [ reviewId ]) - + // 2. CurrentUser must have 'update' on Review(${reviewId}) // 3. Review(:review_id) exists. - if ( existingResults.rows.length <= 0) { - throw new ControllerError(404, 'no-resource', - `Attempt to POST thread to Review(${reviewId}), but it doesn't exist!`) - } - - const existing = existingResults.rows[0] - // 4. Review(:review_id) is on Paper(:paper_id) - if ( existing.paper_id != paperId) { - throw new ControllerError(400, 'id-mismatch:paper', - `Review(${reviewId}) is on Paper(${existing.paper_id}) not Paper(${paperId}).`) - } - - const isReviewAuthor = (existing.review_userId == userId) - // 2. Be the author of Review(:review_id)... - if ( ! isReviewAuthor ) { - throw new ControllError(403, 'not-authorized', - `User(${userid}) attempted to PATCH Review(${reviewId}) that they didn't author.`) - } - - // 6. Review(:review_id) is in progress or User is paper author and patch is changing status to 'accepted' or 'rejected'. - if (existing.review_status != 'in-progress' ) { - throw new ControllerError(403, 'not-authorized:not-in-progress', - `User(${userId}) attempted to PATCH Review(${reviewId}), but it was not in progress.`) - } - - // 7. Paper authors may only edit the `status` field. - if ( ! isReviewAuthor && (review.summary || review.recommendation)) { - throw new ControllerError(403, 'not-authorized:forbidden-fields', - `User(${userId}) attempted to PATCH forbidden fields on Review(${reviewId}).`) - } - - // 8. Review authors may only edit the `summary` and `recommendation` - // fields, or the `status` field if they are setting it to 'submitted'. - if ( isReviewAuthor && review.status && review.status != 'submitted') { - throw new ControllerError(403, 'not-authorized:forbidden-fields', - `User(${userId}) attempted to PATCH forbidden fields on Review(${reviewId}).`) + const canUpdateReview = await this.permissionService.can(currentUser, 'update', 'Review', { paperId: paperId, reviewId: reviewId }) + if ( ! canUpdateReview ) { + throw new ControllerError(403, 'not-authorized', + `User(${currentUser?.id}) attempted to PATCH Review(${reviewId}) of Paper(${paperId}) without permission.`) } - // 9. No one may PATCH `paperId`, `userId`, `version`, or `number` + // 5. No one may PATCH `paperId`, `userId`, `version`, or `number` if ( review.paperId || review.userId || review.paperVersionId|| review.number ) { throw new ControllerError(403, 'not-authorized:forbidden-fields', `User(${userId}) attempted to PATCH forbidden fields on Review(${reviewId}).`) @@ -646,16 +556,13 @@ module.exports = class ReviewController { * @returns {Promise} Resolves to void. */ async deleteReview(request, response) { - const paperId = request.params.paper_id - const reviewId = request.params.review_id - /************************************************************* * Permissions Checking and Input Validation * * To call this endpoint you must: * - * 1. Be logged in. - * 2. Be the author of Review(:review_id) + * 1. CurrentUser must be logged in. + * 2. CurrentUser have 'delete' on Review(:review_id) * * Then we need to do basic input validation: * @@ -667,14 +574,23 @@ module.exports = class ReviewController { * 5. Review(:review_id) is in progress. * * ***********************************************************/ + const paperId = request.params.paper_id + const reviewId = request.params.review_id + + const currentUser = request.session.user // 1. Be logged in. - if ( ! request.session.user ) { + if ( ! currentUser ) { throw new ControllerError(401, 'not-authenticated', `Unauthenticated user attempted to DELETE review on Paper(${paperId}).`) } - const userId = request.session.user.id + // 2. CurrentUser have 'delete' on Review(:review_id) + const canDeleteReview = await this.permissionService.can(currentUser, 'delete', 'Review', { paperId: paperId, reviewId: reviewId}) + if ( ! canDeleteReview ) { + throw new ControllerError(403, 'not-authorized', + `User(${currentUser?.id}) attempting to DELETE Review(${reviewId}) without permission.`) + } const existingResults = await this.database.query(` SELECT @@ -693,12 +609,6 @@ module.exports = class ReviewController { const existing = existingResults.rows[0] - // 2. Be the author of Review(:review_id) - if ( existing.review_userId != userId ) { - throw new ControllerError(403, 'not-authorized', - `User(${userId}) attempted to DELETE Review(${reviewId}) that they did not write.`) - } - // 4. Review(:review_id) is on Paper(:paper_id) if ( existing.paper_id != paperId) { throw new ControllerError(400, 'id-mismatch', @@ -708,7 +618,7 @@ module.exports = class ReviewController { // 5. Review(:review_id) is in progress. if (existing.review_status != 'in-progress' ) { throw new ControllerError(400, 'not-in-progress', - `User(${userId}) attempted to PUT Review(${reviewId}), but it was not in progress.`) + `User(${currentUser?.id}) attempted to DELETE Review(${reviewId}), but it was not in progress.`) } /******************************************************** @@ -743,16 +653,13 @@ module.exports = class ReviewController { * @returns {Promise} Resolves to void. */ async postThreads(request, response) { - const paperId = request.params.paper_id - const reviewId = request.params.review_id - /************************************************************* * Permissions Checking and Input Validation * * To call this endpoint you must: * * 1. Be logged in. - * 2. Be the author of Review(:review_id) + * 2. Have 'create' on Review:thread for Review(:review_id). * * Then we need to do basic data validation: * @@ -769,15 +676,23 @@ module.exports = class ReviewController { * 5. Review(:review_id) is in progress. * * ***********************************************************/ + const paperId = request.params.paper_id + const reviewId = request.params.review_id + + const currentUser = request.session.user // 1. Be logged in. // Have to be authenticated to add a comment thread to a review. - if ( ! request.session.user ) { + if ( ! currentUser ) { throw new ControllerError(401, 'not-authenticated', `Unauthenticated user attempted to POST review thread on Paper(${review.paperId}).`) } - const userId = request.session.user.id + const canCreateThread = await this.permissionService.can(currentUser, 'create', 'Review:thread', { paperId: paperId, reviewId: reviewId }) + if ( ! canCreateThread ) { + throw new ControllerError(403, 'not-authorized', + `User(${currentUser.id}) attempted to create a thread on Review(${reviewId}) without permission.`) + } const existingResults = await this.database.query(` SELECT @@ -796,12 +711,6 @@ module.exports = class ReviewController { const existing = existingResults.rows[0] - // 2. Be the author of Review(:review_id) - if ( existing.review_userId != userId ) { - throw new ControllerError(403, 'not-authorized', - `User(${userId}) attempting to add comment thread to Review(${reviewId}) when not authorized.`) - } - // 4. Review(:review_id) is on Paper(:paper_id) if ( ! existing.paper_id == paperId) { throw new ControllerError(400, 'id-mismatch', @@ -811,7 +720,7 @@ module.exports = class ReviewController { // 5. Review(:review_id) is in progress if ( existing.review_status != 'in-progress') { throw new ControllerError(400, 'not-in-progress', - `User(${userId}) attempting to add a new thread to Review(${reviewId}) after it has been submitted.`) + `User(${currentUser.id}) attempting to add a new thread to Review(${reviewId}) after it has been submitted.`) } /******************************************************** @@ -841,8 +750,9 @@ module.exports = class ReviewController { `Failed to find review ${reviewId} after inserting new threads.`) } - this.reviewDAO.selectVisibleComments(userId, results.dictionary) - results.relations = await this.getRelations(request.session.user, results) + // TODO + this.reviewDAO.selectVisibleComments(currentUser.id, results.dictionary) + results.relations = await this.getRelations(currentUser, results) return response.status(200).json({ entity: results.dictionary[reviewId], threadIds: threadIds, relations: results.relations }) }