Skip to content

Commit

Permalink
api: refine NotAuthorized error + return correct http status code
Browse files Browse the repository at this point in the history
  • Loading branch information
kevinbader committed Apr 1, 2019
1 parent 9005f99 commit 9399e25
Show file tree
Hide file tree
Showing 38 changed files with 167 additions and 127 deletions.
5 changes: 3 additions & 2 deletions api/src/http_errors.ts
Original file line number Diff line number Diff line change
@@ -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";

Expand All @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions api/src/project_view_details.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { FastifyInstance } from "fastify";
import { VError } from "verror";

import { getAllowedIntents } from "./authz";
import Intent from "./authz/intents";
Expand Down Expand Up @@ -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;

Expand Down
22 changes: 16 additions & 6 deletions api/src/service/domain/errors/not_authorized.ts
Original file line number Diff line number Diff line change
@@ -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;
}
}
5 changes: 3 additions & 2 deletions api/src/service/domain/organization/group_create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)] };
}
}

Expand Down
5 changes: 3 additions & 2 deletions api/src/service/domain/organization/group_member_add.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)],
};
}
}
Expand Down
5 changes: 3 additions & 2 deletions api/src/service/domain/organization/group_member_remove.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)],
};
}
}
Expand Down
6 changes: 3 additions & 3 deletions api/src/service/domain/organization/user_create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)],
};
}
}
Expand Down
5 changes: 3 additions & 2 deletions api/src/service/domain/workflow/global_permission_grant.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)],
};
}
}
Expand Down
5 changes: 3 additions & 2 deletions api/src/service/domain/workflow/global_permission_revoke.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)],
};
}
}
Expand Down
5 changes: 3 additions & 2 deletions api/src/service/domain/workflow/project_assign.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
5 changes: 3 additions & 2 deletions api/src/service/domain/workflow/project_close.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
8 changes: 6 additions & 2 deletions api/src/service/domain/workflow/project_create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)],
};
}
}

Expand Down
9 changes: 3 additions & 6 deletions api/src/service/domain/workflow/project_get.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { VError } from "verror";
import Intent from "../../../authz/intents";
import { Ctx } from "../../../lib/ctx";
import * as Result from "../../../result";
Expand All @@ -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);
Expand Down
5 changes: 3 additions & 2 deletions api/src/service/domain/workflow/project_permission_grant.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
5 changes: 3 additions & 2 deletions api/src/service/domain/workflow/project_permission_revoke.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
5 changes: 3 additions & 2 deletions api/src/service/domain/workflow/project_update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
5 changes: 3 additions & 2 deletions api/src/service/domain/workflow/subproject_assign.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
7 changes: 5 additions & 2 deletions api/src/service/domain/workflow/subproject_close.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
10 changes: 5 additions & 5 deletions api/src/service/domain/workflow/subproject_create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
10 changes: 5 additions & 5 deletions api/src/service/domain/workflow/subproject_get.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading

0 comments on commit 9399e25

Please sign in to comment.