Skip to content

Commit

Permalink
Merge pull request #273 from swisstopo/feature/assets-252_limit-admin…
Browse files Browse the repository at this point in the history
…-permissions

Prevent admins from editing in workgroups they are no members of
  • Loading branch information
vej-ananas authored Sep 20, 2024
2 parents 7b6534f + f49d685 commit a6bfd1c
Show file tree
Hide file tree
Showing 11 changed files with 30 additions and 32 deletions.
8 changes: 4 additions & 4 deletions apps/server-asset-sg/src/core/authorize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,19 @@ class Authorize<T> {
constructor(private readonly policy: Policy<T>) {}

canShow(value: T): void {
check(this.policy.canDoEverything() || this.policy.canShow(value));
check(this.policy.canShow(value));
}

canCreate(): void {
check(this.policy.canDoEverything() || this.policy.canCreate());
check(this.policy.canCreate());
}

canUpdate(value: T): void {
check(this.policy.canDoEverything() || this.policy.canUpdate(value));
check(this.policy.canUpdate(value));
}

canDelete(value: T): void {
check(this.policy.canDoEverything() || this.policy.canDelete(value));
check(this.policy.canDelete(value));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ const validatePatch = (user: User, patch: PatchAsset, record?: AssetEditDetail)

// Specialization of the policy where we disallow assets to be moved to another workgroup
// if the current user is not an editor for that workgroup.
if (!policy.canDoEverything() && !policy.hasRole(Role.Editor, patch.workgroupId)) {
if (!policy.hasRole(Role.Editor, patch.workgroupId)) {
throw new HttpException(
"Can't move asset to a workgroup for which the user is not an editor",
HttpStatus.FORBIDDEN
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ const validateData = (user: User, data: AssetData, record?: Asset) => {

// Specialization of the policy where we disallow assets to be moved to another workgroup
// if the current user is not an editor for that workgroup.
if (!policy.canDoEverything() && !policy.hasRole(Role.Editor, data.workgroupId)) {
if (!policy.hasRole(Role.Editor, data.workgroupId)) {
throw new HttpException(
"Can't move asset to a workgroup for which the user is not an editor",
HttpStatus.FORBIDDEN
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,22 @@ export const simpleWorkgroupSelection = (userId: UserId) =>
type SelectedWorkgroup = Prisma.WorkgroupGetPayload<{ select: ReturnType<typeof simpleWorkgroupSelection> }>;

const parse = (data: SelectedWorkgroup, isAdmin: boolean, isAnonymousMode = false): SimpleWorkgroup => {
let role: Role;
if (isAdmin) {
role = Role.MasterEditor;
} else if (isAnonymousMode) {
role = Role.Viewer;
} else {
role = data.users[0].role;
}
return {
const simpleWorkgroup: SimpleWorkgroup = {
id: data.id,
name: data.name,
role,
role: Role.Viewer,
};
if (isAnonymousMode) {
return simpleWorkgroup;
}

if (data.users.length !== 0) {
simpleWorkgroup.role = data.users[0].role;
}

if (isAdmin) {
return simpleWorkgroup;
}

throw new Error('User is not authorized to access this workgroup');
};
7 changes: 2 additions & 5 deletions libs/asset-editor/src/lib/asset-editor.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export const canLeaveEdit: CanDeactivateFn<AssetEditorPageComponent> = (c) => c.
filter(isNotNull),
map((user) => {
const policy = new AssetEditPolicy(user);
return policy.canDoEverything() || policy.canCreate();
return policy.canCreate();
})
);
}) as CanActivateFn,
Expand Down Expand Up @@ -126,10 +126,7 @@ export const canLeaveEdit: CanDeactivateFn<AssetEditorPageComponent> = (c) => c.
]).pipe(
map(([assetEditDetail, user]) => {
const policy = new AssetEditPolicy(user);
return (
policy.canDoEverything() ||
(assetEditDetail == null ? policy.canCreate() : policy.canUpdate(assetEditDetail))
);
return assetEditDetail == null ? policy.canCreate() : policy.canUpdate(assetEditDetail);
})
);
}) as CanActivateFn,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export abstract class BasePolicyDirective<T> implements OnInit, OnChanges, OnDes

private render(): void {
const policyInstance = this.user == null ? null : new this.policy(this.user);
if (policyInstance != null && (policyInstance.canDoEverything() || this.check(policyInstance))) {
if (policyInstance != null && this.check(policyInstance)) {
this.viewContainer.createEmbeddedView(this.templateRef);
} else {
this.viewContainer.clear();
Expand Down
2 changes: 1 addition & 1 deletion libs/shared/v2/src/lib/policies/asset-edit.policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ type Asset = { workgroupId: WorkgroupId };
export class AssetEditPolicy extends Policy<Asset> {
override canShow(value: Asset): boolean {
// A user can see all assets in all workgroups that they are assigned to.
return this.hasWorkgroup(value.workgroupId);
return this.user.isAdmin || this.hasWorkgroup(value.workgroupId);
}

override canCreate(): boolean {
Expand Down
2 changes: 1 addition & 1 deletion libs/shared/v2/src/lib/policies/asset.policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { Policy } from './base/policy';
export class AssetPolicy extends Policy<Asset> {
canShow(value: Asset): boolean {
// A user can see all assets in all workgroups that they are assigned to.
return this.hasWorkgroup(value.workgroupId);
return this.user.isAdmin || this.hasWorkgroup(value.workgroupId);
}

override canCreate(): boolean {
Expand Down
4 changes: 0 additions & 4 deletions libs/shared/v2/src/lib/policies/base/policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,6 @@ export abstract class Policy<T> {
return this.withWorkgroupRole(ids, (role) => getRoleIndex(role) >= roleIndex);
}

canDoEverything(): boolean {
return this.user.isAdmin;
}

abstract canShow(value: T): boolean;

abstract canCreate(): boolean;
Expand Down
4 changes: 2 additions & 2 deletions libs/shared/v2/src/lib/policies/contact.policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ import { Policy } from './base/policy';

export class ContactPolicy extends Policy<Contact> {
canShow(_value: Contact): boolean {
return this.hasRole(Role.Editor);
return this.user.isAdmin || this.hasRole(Role.Editor);
}

canCreate(): boolean {
// A user can create assets for workgroups for which they are an Editor.
// A user can create contacts for workgroups for which they are an Editor.
return this.hasRole(Role.Editor);
}
}
4 changes: 2 additions & 2 deletions libs/shared/v2/src/lib/policies/workgroup.policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ import { Policy } from './base/policy';
export class WorkgroupPolicy extends Policy<Workgroup> {
canShow(value: Workgroup): boolean {
// A user can see every workgroup assigned to them.
return this.hasWorkgroup(value.id);
return this.user.isAdmin || this.hasWorkgroup(value.id);
}

canCreate(): boolean {
// Only admins can create workgroups.
return false;
return this.user.isAdmin;
}
}

0 comments on commit a6bfd1c

Please sign in to comment.