From c6adeef81d2ebb8f430a454ae924939e5008f9d2 Mon Sep 17 00:00:00 2001 From: Kevin Bader Date: Mon, 1 Apr 2019 16:36:49 +0200 Subject: [PATCH] api: refine NotAuthorized error + return correct http status code --- api/src/http_errors.ts | 5 ++-- api/src/project_view_details.ts | 4 +-- .../service/domain/errors/not_authorized.ts | 22 +++++++++++----- .../domain/organization/group_create.ts | 5 ++-- .../domain/organization/group_member_add.ts | 5 ++-- .../organization/group_member_remove.ts | 5 ++-- .../domain/organization/user_create.ts | 6 ++--- .../workflow/global_permission_grant.ts | 5 ++-- .../workflow/global_permission_revoke.ts | 5 ++-- .../service/domain/workflow/project_assign.ts | 5 ++-- .../service/domain/workflow/project_close.ts | 5 ++-- .../service/domain/workflow/project_create.ts | 8 ++++-- .../service/domain/workflow/project_get.ts | 9 +++---- .../workflow/project_permission_grant.ts | 5 ++-- .../workflow/project_permission_revoke.ts | 5 ++-- .../project_projected_budget_delete.ts | 5 ++-- .../project_projected_budget_update.ts | 5 ++-- .../service/domain/workflow/project_update.ts | 5 ++-- .../domain/workflow/subproject_assign.ts | 5 ++-- .../domain/workflow/subproject_close.ts | 7 ++++-- .../domain/workflow/subproject_create.ts | 10 ++++---- .../service/domain/workflow/subproject_get.ts | 10 ++++---- .../workflow/subproject_permission_grant.ts | 5 ++-- .../workflow/subproject_permission_revoke.ts | 5 ++-- .../workflow/subproject_permissions_list.ts | 5 ++-- .../subproject_projected_budget_delete.ts | 10 ++++---- .../subproject_projected_budget_update.ts | 10 ++++---- .../domain/workflow/subproject_update.ts | 5 ++-- .../domain/workflow/workflowitem_assign.ts | 10 ++++---- .../domain/workflow/workflowitem_close.ts | 10 ++++---- .../domain/workflow/workflowitem_create.ts | 25 +++++++++++-------- .../domain/workflow/workflowitem_get.ts | 7 ++++-- .../workflow/workflowitem_permission_grant.ts | 10 ++++---- .../workflowitem_permission_revoke.ts | 5 ++-- .../workflow/workflowitem_permissions_list.ts | 5 ++-- .../domain/workflow/workflowitem_update.ts | 14 +++++------ .../domain/workflow/workflowitems_reorder.ts | 16 ++++++------ api/src/service/project_get.ts | 6 ++--- 38 files changed, 167 insertions(+), 127 deletions(-) diff --git a/api/src/http_errors.ts b/api/src/http_errors.ts index 4251749041..c8df8038f0 100644 --- a/api/src/http_errors.ts +++ b/api/src/http_errors.ts @@ -1,5 +1,6 @@ +import { VError } from "verror"; + import logger from "./lib/logger"; -import { NotAuthorized } from "./service/domain/errors/not_authorized"; import { NotFound } from "./service/domain/errors/not_found"; import { PreconditionError } from "./service/domain/errors/precondition_error"; @@ -26,7 +27,7 @@ function convertError(error: any): { code: number; message: string } { if (error instanceof PreconditionError) { logger.debug({ error }, error.message); return { code: 400, message: error.message }; - } else if (error instanceof NotAuthorized) { + } else if (VError.hasCauseWithName(error, "NotAuthorized")) { logger.debug({ error }, error.message); return { code: 403, message: error.message }; } else if (error instanceof NotFound) { diff --git a/api/src/project_view_details.ts b/api/src/project_view_details.ts index 75b72fc5d5..d9bc4c9390 100644 --- a/api/src/project_view_details.ts +++ b/api/src/project_view_details.ts @@ -1,4 +1,5 @@ import { FastifyInstance } from "fastify"; +import { VError } from "verror"; import { getAllowedIntents } from "./authz"; import Intent from "./authz/intents"; @@ -202,8 +203,7 @@ export function addHttpHandler(server: FastifyInstance, urlPrefix: string, servi try { const projectResult = await service.getProject(ctx, user, projectId); if (Result.isErr(projectResult)) { - projectResult.message = `project.viewDetails failed: ${projectResult.message}`; - throw projectResult; + throw new VError(projectResult, "project.viewDetails failed"); } const project: Project.Project = projectResult; diff --git a/api/src/service/domain/errors/not_authorized.ts b/api/src/service/domain/errors/not_authorized.ts index 67231887a3..056b35ed11 100644 --- a/api/src/service/domain/errors/not_authorized.ts +++ b/api/src/service/domain/errors/not_authorized.ts @@ -1,23 +1,33 @@ +import { isArray } from "util"; + import Intent from "../../../authz/intents"; import { Ctx } from "../../../lib/ctx"; -import { BusinessEvent } from "../business_event"; export class NotAuthorized extends Error { + private readonly target?: object; + constructor( private readonly ctx: Ctx, private readonly userId: string, - private readonly businessEvent?: BusinessEvent, - private readonly intent?: Intent, + private readonly intent: Intent | Intent[], + target?: object, ) { super( - `User ${userId} is not authorized${ - businessEvent ? ` to apply ${businessEvent.type}` : ` to invoke ${intent}` - }.`, + `user ${userId} is not authorized for ${ + isArray(intent) ? `any of ${intent.join(", ")}` : intent + }`, ); + this.name = "NotAuthorized"; // Maintains proper stack trace for where our error was thrown (only available on V8): if (Error.captureStackTrace) { Error.captureStackTrace(this, NotAuthorized); } + + // Removing trace events as they're not needed and spam the log output when printed: + if (target !== undefined && (target as any).log !== undefined) { + delete (target as any).log; + } + this.target = target; } } diff --git a/api/src/service/domain/organization/group_create.ts b/api/src/service/domain/organization/group_create.ts index 6addd788dc..f8ab7803fa 100644 --- a/api/src/service/domain/organization/group_create.ts +++ b/api/src/service/domain/organization/group_create.ts @@ -71,12 +71,13 @@ export async function createGroup( // Check authorization (if not root): if (creatingUser.id !== "root") { + const intent = "global.createGroup"; const permissions = await repository.getGlobalPermissions(); - const isAuthorized = identitiesAuthorizedFor(permissions, "global.createGroup").some(identity => + const isAuthorized = identitiesAuthorizedFor(permissions, intent).some(identity => canAssumeIdentity(creatingUser, identity), ); if (!isAuthorized) { - return { newEvents: [], errors: [new NotAuthorized(ctx, creatingUser.id, createEvent)] }; + return { newEvents: [], errors: [new NotAuthorized(ctx, creatingUser.id, intent)] }; } } diff --git a/api/src/service/domain/organization/group_member_add.ts b/api/src/service/domain/organization/group_member_add.ts index 0e87b2ed32..f688afee9a 100644 --- a/api/src/service/domain/organization/group_member_add.ts +++ b/api/src/service/domain/organization/group_member_add.ts @@ -32,10 +32,11 @@ export async function addMember( // Check authorization (if not root): if (issuer.id !== "root") { - if (!Group.permits(group, issuer, ["group.addUser"])) { + const intent = "group.addUser"; + if (!Group.permits(group, issuer, [intent])) { return { newEvents: [], - errors: [new NotAuthorized(ctx, issuer.id, memberAdded)], + errors: [new NotAuthorized(ctx, issuer.id, intent, group)], }; } } diff --git a/api/src/service/domain/organization/group_member_remove.ts b/api/src/service/domain/organization/group_member_remove.ts index 42f9d22a41..1b121c8a31 100644 --- a/api/src/service/domain/organization/group_member_remove.ts +++ b/api/src/service/domain/organization/group_member_remove.ts @@ -32,10 +32,11 @@ export async function removeMember( // Check authorization (if not root): if (issuer.id !== "root") { - if (!Group.permits(group, issuer, ["group.removeUser"])) { + const intent = "group.removeUser"; + if (!Group.permits(group, issuer, [intent])) { return { newEvents: [], - errors: [new NotAuthorized(ctx, issuer.id, memberRemoved)], + errors: [new NotAuthorized(ctx, issuer.id, intent, group)], }; } } diff --git a/api/src/service/domain/organization/user_create.ts b/api/src/service/domain/organization/user_create.ts index 87b474ce10..3995ab97fe 100644 --- a/api/src/service/domain/organization/user_create.ts +++ b/api/src/service/domain/organization/user_create.ts @@ -77,15 +77,15 @@ export async function createUser( // Check authorization (if not root): if (creatingUser.id !== "root") { + const intent = "global.createUser"; const permissions = await repository.getGlobalPermissions(); - const isAuthorized = identitiesAuthorizedFor(permissions, "global.createUser").some(identity => + const isAuthorized = identitiesAuthorizedFor(permissions, intent).some(identity => canAssumeIdentity(creatingUser, identity), ); if (!isAuthorized) { - const unfinishedBusinessEvent = UserCreated.createEvent(source, publisher, eventTemplate); return { newEvents: [], - errors: [new NotAuthorized(ctx, creatingUser.id, unfinishedBusinessEvent)], + errors: [new NotAuthorized(ctx, creatingUser.id, intent)], }; } } diff --git a/api/src/service/domain/workflow/global_permission_grant.ts b/api/src/service/domain/workflow/global_permission_grant.ts index 51174198ef..b5f55d3901 100644 --- a/api/src/service/domain/workflow/global_permission_grant.ts +++ b/api/src/service/domain/workflow/global_permission_grant.ts @@ -30,11 +30,12 @@ export async function grantGlobalPermission( // Check authorization (if not root): if (issuer.id !== "root") { + const grantIntent = "global.grantPermission"; const currentGlobalPermissions = await repository.getGlobalPermissions(); - if (!GlobalPermissions.permits(currentGlobalPermissions, issuer, ["global.grantPermission"])) { + if (!GlobalPermissions.permits(currentGlobalPermissions, issuer, [grantIntent])) { return { newEvents: [], - errors: [new NotAuthorized(ctx, issuer.id, globalPermissionGranted)], + errors: [new NotAuthorized(ctx, issuer.id, grantIntent, currentGlobalPermissions)], }; } } diff --git a/api/src/service/domain/workflow/global_permission_revoke.ts b/api/src/service/domain/workflow/global_permission_revoke.ts index 34702fb943..2070322e9c 100644 --- a/api/src/service/domain/workflow/global_permission_revoke.ts +++ b/api/src/service/domain/workflow/global_permission_revoke.ts @@ -30,11 +30,12 @@ export async function revokeGlobalPermission( // Check authorization (if not root): if (issuer.id !== "root") { + const revokeIntent = "global.revokePermission"; const currentGlobalPermissions = await repository.getGlobalPermissions(); - if (!GlobalPermissions.permits(currentGlobalPermissions, issuer, ["global.revokePermission"])) { + if (!GlobalPermissions.permits(currentGlobalPermissions, issuer, [revokeIntent])) { return { newEvents: [], - errors: [new NotAuthorized(ctx, issuer.id, globalPermissionRevoked)], + errors: [new NotAuthorized(ctx, issuer.id, revokeIntent, currentGlobalPermissions)], }; } } diff --git a/api/src/service/domain/workflow/project_assign.ts b/api/src/service/domain/workflow/project_assign.ts index 28588747d0..d61f64a0cf 100644 --- a/api/src/service/domain/workflow/project_assign.ts +++ b/api/src/service/domain/workflow/project_assign.ts @@ -42,8 +42,9 @@ export async function assignProject( } // Check authorization (if not root): - if (issuer.id !== "root" && !Project.permits(project, issuer, ["project.assign"])) { - return new NotAuthorized(ctx, issuer.id, projectAssigned); + const intent = "project.assign"; + if (issuer.id !== "root" && !Project.permits(project, issuer, [intent])) { + return new NotAuthorized(ctx, issuer.id, intent, project); } // Check that the new event is indeed valid: diff --git a/api/src/service/domain/workflow/project_close.ts b/api/src/service/domain/workflow/project_close.ts index 823d244af3..bbd77fa1eb 100644 --- a/api/src/service/domain/workflow/project_close.ts +++ b/api/src/service/domain/workflow/project_close.ts @@ -58,8 +58,9 @@ export async function closeProject( } // Check authorization (if not root): - if (issuer.id !== "root" && !Project.permits(project, issuer, ["project.close"])) { - return new NotAuthorized(ctx, issuer.id, projectClosed); + const intent = "project.close"; + if (issuer.id !== "root" && !Project.permits(project, issuer, [intent])) { + return new NotAuthorized(ctx, issuer.id, intent, project); } // Check that the new event is indeed valid: diff --git a/api/src/service/domain/workflow/project_create.ts b/api/src/service/domain/workflow/project_create.ts index 9e7358baba..54bc744c9f 100644 --- a/api/src/service/domain/workflow/project_create.ts +++ b/api/src/service/domain/workflow/project_create.ts @@ -96,9 +96,13 @@ export async function createProject( // Check authorization (if not root): if (creatingUser.id !== "root") { + const intent = "global.createProject"; const permissions = await repository.getGlobalPermissions(); - if (!GlobalPermissions.permits(permissions, creatingUser, ["global.createProject"])) { - return { newEvents: [], errors: [new NotAuthorized(ctx, creatingUser.id, createEvent)] }; + if (!GlobalPermissions.permits(permissions, creatingUser, [intent])) { + return { + newEvents: [], + errors: [new NotAuthorized(ctx, creatingUser.id, intent, permissions)], + }; } } diff --git a/api/src/service/domain/workflow/project_get.ts b/api/src/service/domain/workflow/project_get.ts index 2c389f3e0c..d4c85d5353 100644 --- a/api/src/service/domain/workflow/project_get.ts +++ b/api/src/service/domain/workflow/project_get.ts @@ -1,4 +1,3 @@ -import { VError } from "verror"; import Intent from "../../../authz/intents"; import { Ctx } from "../../../lib/ctx"; import * as Result from "../../../result"; @@ -25,11 +24,9 @@ export async function getProject( return new NotFound(ctx, "project", projectId); } - if ( - user.id !== "root" && - !Project.permits(project, user, ["project.viewSummary", "project.viewDetails"]) - ) { - return new NotAuthorized(ctx, user.id, undefined, "project.viewSummary"); + const intents: Intent[] = ["project.viewSummary", "project.viewDetails"]; + if (user.id !== "root" && !Project.permits(project, user, intents)) { + return new NotAuthorized(ctx, user.id, intents, project); } return dropHiddenHistoryEvents(project, user); diff --git a/api/src/service/domain/workflow/project_permission_grant.ts b/api/src/service/domain/workflow/project_permission_grant.ts index bfd60618cf..3a0a658859 100644 --- a/api/src/service/domain/workflow/project_permission_grant.ts +++ b/api/src/service/domain/workflow/project_permission_grant.ts @@ -42,8 +42,9 @@ export async function grantProjectPermission( // Check authorization (if not root): if (issuer.id !== "root") { - if (!Project.permits(project, issuer, ["project.intent.grantPermission"])) { - return new NotAuthorized(ctx, issuer.id, permissionGranted); + const grantIntent = "project.intent.grantPermission"; + if (!Project.permits(project, issuer, [grantIntent])) { + return new NotAuthorized(ctx, issuer.id, grantIntent, project); } } diff --git a/api/src/service/domain/workflow/project_permission_revoke.ts b/api/src/service/domain/workflow/project_permission_revoke.ts index f692270a13..10a910114c 100644 --- a/api/src/service/domain/workflow/project_permission_revoke.ts +++ b/api/src/service/domain/workflow/project_permission_revoke.ts @@ -42,8 +42,9 @@ export async function revokeProjectPermission( // Check authorization (if not root): if (issuer.id !== "root") { - if (!Project.permits(project, issuer, ["project.intent.revokePermission"])) { - return new NotAuthorized(ctx, issuer.id, permissionRevoked); + const revokeIntent = "project.intent.revokePermission"; + if (!Project.permits(project, issuer, [revokeIntent])) { + return new NotAuthorized(ctx, issuer.id, revokeIntent, project); } } diff --git a/api/src/service/domain/workflow/project_projected_budget_delete.ts b/api/src/service/domain/workflow/project_projected_budget_delete.ts index c00636afe1..41b9e38d99 100644 --- a/api/src/service/domain/workflow/project_projected_budget_delete.ts +++ b/api/src/service/domain/workflow/project_projected_budget_delete.ts @@ -40,8 +40,9 @@ export async function deleteProjectedBudget( // Check authorization (if not root): if (issuer.id !== "root") { - if (!Project.permits(project, issuer, ["project.budget.deleteProjected"])) { - return new NotAuthorized(ctx, issuer.id, budgetDeleted); + const intent = "project.budget.deleteProjected"; + if (!Project.permits(project, issuer, [intent])) { + return new NotAuthorized(ctx, issuer.id, intent, project); } } diff --git a/api/src/service/domain/workflow/project_projected_budget_update.ts b/api/src/service/domain/workflow/project_projected_budget_update.ts index d3f438a3a0..ce9dbb561c 100644 --- a/api/src/service/domain/workflow/project_projected_budget_update.ts +++ b/api/src/service/domain/workflow/project_projected_budget_update.ts @@ -43,8 +43,9 @@ export async function updateProjectedBudget( // Check authorization (if not root): if (issuer.id !== "root") { - if (!Project.permits(project, issuer, ["project.budget.updateProjected"])) { - return new NotAuthorized(ctx, issuer.id, budgetUpdated); + const intent = "project.budget.updateProjected"; + if (!Project.permits(project, issuer, [intent])) { + return new NotAuthorized(ctx, issuer.id, intent, project); } } diff --git a/api/src/service/domain/workflow/project_update.ts b/api/src/service/domain/workflow/project_update.ts index e38afa7751..a662db1e1b 100644 --- a/api/src/service/domain/workflow/project_update.ts +++ b/api/src/service/domain/workflow/project_update.ts @@ -45,8 +45,9 @@ export async function updateProject( // Check authorization (if not root): if (issuer.id !== "root") { - if (!Project.permits(project, issuer, ["project.update"])) { - return new NotAuthorized(ctx, issuer.id, projectUpdated); + const intent = "project.update"; + if (!Project.permits(project, issuer, [intent])) { + return new NotAuthorized(ctx, issuer.id, intent, project); } } diff --git a/api/src/service/domain/workflow/subproject_assign.ts b/api/src/service/domain/workflow/subproject_assign.ts index 6ad8f04b0d..34730306be 100644 --- a/api/src/service/domain/workflow/subproject_assign.ts +++ b/api/src/service/domain/workflow/subproject_assign.ts @@ -51,8 +51,9 @@ export async function assignSubproject( // Check authorization (if not root): if (issuer.id !== "root") { - if (!Subproject.permits(subproject, issuer, ["subproject.assign"])) { - return new NotAuthorized(ctx, issuer.id, subprojectAssigned); + const intent = "subproject.assign"; + if (!Subproject.permits(subproject, issuer, [intent])) { + return new NotAuthorized(ctx, issuer.id, intent, subproject); } } diff --git a/api/src/service/domain/workflow/subproject_close.ts b/api/src/service/domain/workflow/subproject_close.ts index f8a2b75407..de44656047 100644 --- a/api/src/service/domain/workflow/subproject_close.ts +++ b/api/src/service/domain/workflow/subproject_close.ts @@ -74,8 +74,11 @@ export async function closeSubproject( } // Check authorization (if not root): - if (issuer.id !== "root" && !Subproject.permits(subproject, issuer, ["subproject.close"])) { - return new NotAuthorized(ctx, issuer.id, subprojectClosed); + if (issuer.id !== "root") { + const intent = "subproject.close"; + if (!Subproject.permits(subproject, issuer, [intent])) { + return new NotAuthorized(ctx, issuer.id, intent, subproject); + } } // Check that the new event is indeed valid: diff --git a/api/src/service/domain/workflow/subproject_create.ts b/api/src/service/domain/workflow/subproject_create.ts index fc3443af22..10182fcd78 100644 --- a/api/src/service/domain/workflow/subproject_create.ts +++ b/api/src/service/domain/workflow/subproject_create.ts @@ -111,11 +111,11 @@ export async function createSubproject( return new NotFound(ctx, "project", projectId); } - if ( - creatingUser.id !== "root" && - !AuthToken.permits(projectPermissions, creatingUser, ["project.createSubproject"]) - ) { - return new NotAuthorized(ctx, creatingUser.id, subprojectCreated); + if (creatingUser.id !== "root") { + const intent = "project.createSubproject"; + if (!AuthToken.permits(projectPermissions, creatingUser, [intent])) { + return new NotAuthorized(ctx, creatingUser.id, intent, { projectId, projectPermissions }); + } } // Check that the event is valid by trying to "apply" it: diff --git a/api/src/service/domain/workflow/subproject_get.ts b/api/src/service/domain/workflow/subproject_get.ts index f8c3a85c49..ca83de7f7a 100644 --- a/api/src/service/domain/workflow/subproject_get.ts +++ b/api/src/service/domain/workflow/subproject_get.ts @@ -25,11 +25,11 @@ export async function getSubproject( } const subproject = subprojectResult; - if ( - user.id !== "root" && - !Subproject.permits(subproject, user, ["subproject.viewSummary", "subproject.viewDetails"]) - ) { - return new NotAuthorized(ctx, user.id, undefined, "subproject.viewDetails"); + if (user.id !== "root") { + const intents: Intent[] = ["subproject.viewSummary", "subproject.viewDetails"]; + if (!Subproject.permits(subproject, user, intents)) { + return new NotAuthorized(ctx, user.id, intents, subproject); + } } return dropHiddenHistoryEvents(subproject, user); diff --git a/api/src/service/domain/workflow/subproject_permission_grant.ts b/api/src/service/domain/workflow/subproject_permission_grant.ts index 016fae6ec7..90f6fff572 100644 --- a/api/src/service/domain/workflow/subproject_permission_grant.ts +++ b/api/src/service/domain/workflow/subproject_permission_grant.ts @@ -48,8 +48,9 @@ export async function grantSubprojectPermission( // Check authorization (if not root): if (issuer.id !== "root") { - if (!Subproject.permits(subproject, issuer, ["subproject.intent.grantPermission"])) { - return new NotAuthorized(ctx, issuer.id, permissionGranted); + const grantIntent = "subproject.intent.grantPermission"; + if (!Subproject.permits(subproject, issuer, [grantIntent])) { + return new NotAuthorized(ctx, issuer.id, grantIntent, subproject); } } diff --git a/api/src/service/domain/workflow/subproject_permission_revoke.ts b/api/src/service/domain/workflow/subproject_permission_revoke.ts index 7186278ef3..2699f4f6ae 100644 --- a/api/src/service/domain/workflow/subproject_permission_revoke.ts +++ b/api/src/service/domain/workflow/subproject_permission_revoke.ts @@ -48,8 +48,9 @@ export async function revokeSubprojectPermission( // Check authorization (if not root): if (issuer.id !== "root") { - if (!Subproject.permits(subproject, issuer, ["subproject.intent.revokePermission"])) { - return new NotAuthorized(ctx, issuer.id, permissionRevoked); + const revokeIntent = "subproject.intent.revokePermission"; + if (!Subproject.permits(subproject, issuer, [revokeIntent])) { + return new NotAuthorized(ctx, issuer.id, revokeIntent, subproject); } } diff --git a/api/src/service/domain/workflow/subproject_permissions_list.ts b/api/src/service/domain/workflow/subproject_permissions_list.ts index 5633984a76..c8e2106990 100644 --- a/api/src/service/domain/workflow/subproject_permissions_list.ts +++ b/api/src/service/domain/workflow/subproject_permissions_list.ts @@ -30,8 +30,9 @@ export async function getSubprojectPermissions( const subproject: Subproject.Subproject = subprojectResult; if (user.id !== "root") { - if (!Subproject.permits(subproject, user, ["subproject.intent.listPermissions"])) { - return new NotAuthorized(ctx, user.id, undefined, "subproject.intent.listPermissions"); + const intent = "subproject.intent.listPermissions"; + if (!Subproject.permits(subproject, user, [intent])) { + return new NotAuthorized(ctx, user.id, intent, subproject); } } return subproject.permissions; diff --git a/api/src/service/domain/workflow/subproject_projected_budget_delete.ts b/api/src/service/domain/workflow/subproject_projected_budget_delete.ts index dc39824e21..340c166d1a 100644 --- a/api/src/service/domain/workflow/subproject_projected_budget_delete.ts +++ b/api/src/service/domain/workflow/subproject_projected_budget_delete.ts @@ -43,11 +43,11 @@ export async function deleteProjectedBudget( ); // Check authorization (if not root): - if ( - issuer.id !== "root" && - !Subproject.permits(subproject, issuer, ["subproject.budget.deleteProjected"]) - ) { - return new NotAuthorized(ctx, issuer.id, budgetDeleted); + if (issuer.id !== "root") { + const intent = "subproject.budget.deleteProjected"; + if (!Subproject.permits(subproject, issuer, [intent])) { + return new NotAuthorized(ctx, issuer.id, intent, subproject); + } } // Check that the new event is indeed valid: diff --git a/api/src/service/domain/workflow/subproject_projected_budget_update.ts b/api/src/service/domain/workflow/subproject_projected_budget_update.ts index dbcd4f45a4..113bfcea89 100644 --- a/api/src/service/domain/workflow/subproject_projected_budget_update.ts +++ b/api/src/service/domain/workflow/subproject_projected_budget_update.ts @@ -43,11 +43,11 @@ export async function updateProjectedBudget( ); // Check authorization (if not root): - if ( - issuer.id !== "root" && - !Subproject.permits(subproject, issuer, ["subproject.budget.updateProjected"]) - ) { - return new NotAuthorized(ctx, issuer.id, budgetUpdated); + if (issuer.id !== "root") { + const intent = "subproject.budget.updateProjected"; + if (!Subproject.permits(subproject, issuer, [intent])) { + return new NotAuthorized(ctx, issuer.id, intent, subproject); + } } // Check that the new event is indeed valid: diff --git a/api/src/service/domain/workflow/subproject_update.ts b/api/src/service/domain/workflow/subproject_update.ts index d05fcaf3b7..43be109aaf 100644 --- a/api/src/service/domain/workflow/subproject_update.ts +++ b/api/src/service/domain/workflow/subproject_update.ts @@ -60,8 +60,9 @@ export async function updateSubproject( // Check authorization (if not root): if (issuer.id !== "root") { - if (!Subproject.permits(subproject, issuer, ["subproject.update"])) { - return new NotAuthorized(ctx, issuer.id, subprojectUpdated); + const intent = "subproject.update"; + if (!Subproject.permits(subproject, issuer, [intent])) { + return new NotAuthorized(ctx, issuer.id, intent, subproject); } } diff --git a/api/src/service/domain/workflow/workflowitem_assign.ts b/api/src/service/domain/workflow/workflowitem_assign.ts index 8207595ff8..b9be8d237d 100644 --- a/api/src/service/domain/workflow/workflowitem_assign.ts +++ b/api/src/service/domain/workflow/workflowitem_assign.ts @@ -55,11 +55,11 @@ export async function assignWorkflowitem( } // Check authorization: - if ( - publisher.id !== "root" && - !Workflowitem.permits(workflowitem, publisher, ["workflowitem.assign"]) - ) { - return new NotAuthorized(ctx, publisher.id, assignEvent); + if (publisher.id !== "root") { + const intent = "workflowitem.assign"; + if (!Workflowitem.permits(workflowitem, publisher, [intent])) { + return new NotAuthorized(ctx, publisher.id, intent, workflowitem); + } } // Check that the new event is indeed valid: diff --git a/api/src/service/domain/workflow/workflowitem_close.ts b/api/src/service/domain/workflow/workflowitem_close.ts index 80dec63e19..70afabb30a 100644 --- a/api/src/service/domain/workflow/workflowitem_close.ts +++ b/api/src/service/domain/workflow/workflowitem_close.ts @@ -74,11 +74,11 @@ export async function closeWorkflowitem( return new VError(closeEvent, "failed to create event"); } - if ( - closingUser.id !== "root" && - !Workflowitem.permits(workflowitemToClose, closingUser, ["workflowitem.close"]) - ) { - return new NotAuthorized(ctx, closingUser.id, closeEvent); + if (closingUser.id !== "root") { + const intent = "workflowitem.close"; + if (!Workflowitem.permits(workflowitemToClose, closingUser, [intent])) { + return new NotAuthorized(ctx, closingUser.id, intent, workflowitemToClose); + } } // Make sure all previous items (wrt. the ordering) are already closed: diff --git a/api/src/service/domain/workflow/workflowitem_create.ts b/api/src/service/domain/workflow/workflowitem_create.ts index aec4cce2a2..5f078d00df 100644 --- a/api/src/service/domain/workflow/workflowitem_create.ts +++ b/api/src/service/domain/workflow/workflowitem_create.ts @@ -1,15 +1,14 @@ import Joi = require("joi"); +import { VError } from "verror"; import Intent from "../../../authz/intents"; import { Ctx } from "../../../lib/ctx"; -import logger from "../../../lib/logger"; import * as Result from "../../../result"; import { randomString } from "../../hash"; import * as AdditionalData from "../additional_data"; import { BusinessEvent } from "../business_event"; import { InvalidCommand } from "../errors/invalid_command"; import { NotAuthorized } from "../errors/not_authorized"; -import { NotFound } from "../errors/not_found"; import { PreconditionError } from "../errors/precondition_error"; import { ServiceUser } from "../organization/service_user"; import { Permissions } from "../permissions"; @@ -118,16 +117,20 @@ export async function createWorkflowitem( // Check authorization (if not root): if (creatingUser.id !== "root") { - const subprojectResult = await repository.getSubproject( - reqData.projectId, - reqData.subprojectId, + const authorizationResult = Result.map( + await repository.getSubproject(reqData.projectId, reqData.subprojectId), + subproject => { + const intent = "subproject.createWorkflowitem"; + if (!Subproject.permits(subproject, creatingUser, [intent])) { + return new NotAuthorized(ctx, creatingUser.id, intent, subproject); + } + }, ); - if (Result.isErr(subprojectResult)) { - return new NotFound(ctx, "subproject", reqData.subprojectId); - } - const subproject = subprojectResult; - if (!Subproject.permits(subproject, creatingUser, ["subproject.createWorkflowitem"])) { - return new NotAuthorized(ctx, creatingUser.id, workflowitemCreated); + if (Result.isErr(authorizationResult)) { + return new VError( + authorizationResult, + "failed to create workflowitem, permission check failed", + ); } } diff --git a/api/src/service/domain/workflow/workflowitem_get.ts b/api/src/service/domain/workflow/workflowitem_get.ts index f5f8c63b3b..a12d8a2268 100644 --- a/api/src/service/domain/workflow/workflowitem_get.ts +++ b/api/src/service/domain/workflow/workflowitem_get.ts @@ -23,8 +23,11 @@ export async function getWorkflowitem( return new NotFound(ctx, "workflowitem", workflowitemId); } - if (user.id !== "root" && !Workflowitem.permits(workflowitem, user, ["workflowitem.view"])) { - return new NotAuthorized(ctx, user.id, undefined, "workflowitem.view"); + if (user.id !== "root") { + const intent = "workflowitem.view"; + if (!Workflowitem.permits(workflowitem, user, [intent])) { + return new NotAuthorized(ctx, user.id, intent, workflowitem); + } } return dropHiddenHistoryEvents(workflowitem, user); diff --git a/api/src/service/domain/workflow/workflowitem_permission_grant.ts b/api/src/service/domain/workflow/workflowitem_permission_grant.ts index 70c523f059..9647605c73 100644 --- a/api/src/service/domain/workflow/workflowitem_permission_grant.ts +++ b/api/src/service/domain/workflow/workflowitem_permission_grant.ts @@ -49,11 +49,11 @@ export async function grantWorkflowitemPermission( grantee, ); - if ( - issuer.id !== "root" && - !Workflowitem.permits(workflowitem, issuer, ["workflowitem.intent.grantPermission"]) - ) { - return new NotAuthorized(ctx, issuer.id, permissionGranted); + if (issuer.id !== "root") { + const grantIntent = "workflowitem.intent.grantPermission"; + if (!Workflowitem.permits(workflowitem, issuer, [grantIntent])) { + return new NotAuthorized(ctx, issuer.id, grantIntent, workflowitem); + } } // Check that the new event is indeed valid: diff --git a/api/src/service/domain/workflow/workflowitem_permission_revoke.ts b/api/src/service/domain/workflow/workflowitem_permission_revoke.ts index a787ed95d2..4eeeae3687 100644 --- a/api/src/service/domain/workflow/workflowitem_permission_revoke.ts +++ b/api/src/service/domain/workflow/workflowitem_permission_revoke.ts @@ -52,8 +52,9 @@ export async function revokeWorkflowitemPermission( // Check authorization (if not root): if (issuer.id !== "root") { - if (!Workflowitem.permits(workflowitem, issuer, ["workflowitem.intent.revokePermission"])) { - return new NotAuthorized(ctx, issuer.id, permissionRevoked); + const revokeIntent = "workflowitem.intent.revokePermission"; + if (!Workflowitem.permits(workflowitem, issuer, [revokeIntent])) { + return new NotAuthorized(ctx, issuer.id, revokeIntent, workflowitem); } } diff --git a/api/src/service/domain/workflow/workflowitem_permissions_list.ts b/api/src/service/domain/workflow/workflowitem_permissions_list.ts index 7729ac6581..3e5125ed1e 100644 --- a/api/src/service/domain/workflow/workflowitem_permissions_list.ts +++ b/api/src/service/domain/workflow/workflowitem_permissions_list.ts @@ -32,8 +32,9 @@ export async function getAll( const workflowitem: Workflowitem.Workflowitem = result; if (user.id !== "root") { - if (!Workflowitem.permits(workflowitem, user, ["workflowitem.intent.listPermissions"])) { - return new NotAuthorized(ctx, user.id, undefined, "workflowitem.intent.listPermissions"); + const intent = "workflowitem.intent.listPermissions"; + if (!Workflowitem.permits(workflowitem, user, [intent])) { + return new NotAuthorized(ctx, user.id, intent, workflowitem); } } diff --git a/api/src/service/domain/workflow/workflowitem_update.ts b/api/src/service/domain/workflow/workflowitem_update.ts index c0fc617ad7..93f9fbf553 100644 --- a/api/src/service/domain/workflow/workflowitem_update.ts +++ b/api/src/service/domain/workflow/workflowitem_update.ts @@ -1,5 +1,7 @@ import { produce } from "immer"; +import isEqual = require("lodash.isequal"); import { VError } from "verror"; + import { Ctx } from "../../../lib/ctx"; import * as Result from "../../../result"; import { BusinessEvent } from "../business_event"; @@ -14,8 +16,6 @@ import * as Project from "./project"; import * as Subproject from "./subproject"; import * as Workflowitem from "./workflowitem"; import * as WorkflowitemUpdated from "./workflowitem_updated"; -import Joi = require("joi"); -import isEqual = require("lodash.isequal"); export type RequestData = WorkflowitemUpdated.Modification; export const requestDataSchema = WorkflowitemUpdated.modificationSchema; @@ -53,11 +53,11 @@ export async function updateWorkflowitem( } // Check authorization (if not root): - if ( - issuer.id !== "root" && - !Workflowitem.permits(workflowitem, issuer, ["workflowitem.update"]) - ) { - return new NotAuthorized(ctx, issuer.id, newEvent); + if (issuer.id !== "root") { + const intent = "workflowitem.update"; + if (!Workflowitem.permits(workflowitem, issuer, [intent])) { + return new NotAuthorized(ctx, issuer.id, intent, workflowitem); + } } // Check that the new event is indeed valid: diff --git a/api/src/service/domain/workflow/workflowitems_reorder.ts b/api/src/service/domain/workflow/workflowitems_reorder.ts index a03e65caee..3ff91d8529 100644 --- a/api/src/service/domain/workflow/workflowitems_reorder.ts +++ b/api/src/service/domain/workflow/workflowitems_reorder.ts @@ -1,4 +1,6 @@ import { produce } from "immer"; +import isEqual = require("lodash.isequal"); + import { Ctx } from "../../../lib/ctx"; import * as Result from "../../../result"; import { BusinessEvent } from "../business_event"; @@ -8,10 +10,8 @@ import { NotFound } from "../errors/not_found"; import { ServiceUser } from "../organization/service_user"; import * as Project from "./project"; import * as Subproject from "./subproject"; -import * as WorkflowitemsReordered from "./workflowitems_reordered"; import * as WorkflowitemOrdering from "./workflowitem_ordering"; - -import isEqual = require("lodash.isequal"); +import * as WorkflowitemsReordered from "./workflowitems_reordered"; interface Repository { getSubproject( @@ -50,11 +50,11 @@ export async function setWorkflowitemOrdering( ); // Check authorization (if not root): - if ( - issuer.id !== "root" && - !Subproject.permits(subproject, issuer, ["subproject.reorderWorkflowitems"]) - ) { - return new NotAuthorized(ctx, issuer.id, reorderEvent); + if (issuer.id !== "root") { + const intent = "subproject.reorderWorkflowitems"; + if (!Subproject.permits(subproject, issuer, [intent])) { + return new NotAuthorized(ctx, issuer.id, intent, subproject); + } } // Check that the new event is indeed valid: diff --git a/api/src/service/project_get.ts b/api/src/service/project_get.ts index e9cac73beb..f391fd482d 100644 --- a/api/src/service/project_get.ts +++ b/api/src/service/project_get.ts @@ -16,13 +16,13 @@ export async function getProject( ): Promise> { const projectResult = await Cache.withCache(conn, ctx, async cache => ProjectGet.getProject(ctx, serviceUser, projectId, { - getProject: async projectId => { - return cache.getProject(projectId); + getProject: async pId => { + return cache.getProject(pId); }, }), ); return Result.mapErr( projectResult, - err => new VError(err, `could not read project ${projectId} from chain`), + err => new VError(err, `could not fetch project ${projectId}`), ); }