Skip to content

Commit

Permalink
Merge pull request #311 from swisstopo/bug/assets-309-restrict-editin…
Browse files Browse the repository at this point in the history
…g-permissions-for-admins

Fix: don't allow to update usage status or work status if not masterEditor
  • Loading branch information
TIL-EBP authored Oct 30, 2024
2 parents 9a28c3d + 339befe commit 8a554c8
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 15 deletions.
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import { PatchAsset } from '@asset-sg/shared';
import { User } from '@asset-sg/shared/v2';
import { Role } from '@asset-sg/shared/v2';
import { AssetEditPolicy } from '@asset-sg/shared/v2';
import { AssetEditPolicy, Role, User } from '@asset-sg/shared/v2';
import { Controller, Get, HttpException, HttpStatus, Param, ParseIntPipe, Post, Put } from '@nestjs/common';
import * as E from 'fp-ts/Either';
import * as O from 'fp-ts/Option';
import { authorize } from '@/core/authorize';
import { CurrentUser } from '@/core/decorators/current-user.decorator';
import { ParseBody } from '@/core/decorators/parse.decorator';
Expand Down Expand Up @@ -69,16 +68,35 @@ const validatePatch = (user: User, patch: PatchAsset, record?: AssetEditDetail)
);
}

const hasStatusChanged = (key: 'internalUse' | 'publicUse') => {
const hasChanged = record == null || record[key].statusAssetUseItemCode !== patch[key].statusAssetUseItemCode;
return (
hasChanged &&
patch[key].statusAssetUseItemCode !== 'tobechecked' &&
((record != null && !policy.hasRole(Role.MasterEditor, record.workgroupId)) ||
!policy.hasRole(Role.MasterEditor, patch.workgroupId))
);
};

// Specialization of the policy where we disallow the internal status to be changed to anything else than `tobechecked`
// if the current user is not a master-editor for the asset's current or future workgroup.
const hasInternalUseChanged =
record == null || record.internalUse.statusAssetUseItemCode !== patch.internalUse.statusAssetUseItemCode;
if (hasStatusChanged('internalUse')) {
throw new HttpException("Changing the asset's internalUse status is not allowed", HttpStatus.FORBIDDEN);
}

// Specialization of the policy where we disallow the public status to be changed to anything else than `tobechecked`
// if the current user is not a master-editor for the asset's current or future workgroup.
if (hasStatusChanged('publicUse')) {
throw new HttpException("Changing the asset's public use status is not allowed", HttpStatus.FORBIDDEN);
}

// Specialization of the policy where we disallow the status work item code to be changed to `published`
// if the current user is not a master-editor for the asset's current or future workgroup.
if (
hasInternalUseChanged &&
patch.internalUse.statusAssetUseItemCode !== 'tobechecked' &&
O.toNullable(patch.newStatusWorkItemCode) === 'published' &&
((record != null && !policy.hasRole(Role.MasterEditor, record.workgroupId)) ||
!policy.hasRole(Role.MasterEditor, patch.workgroupId))
) {
throw new HttpException("Changing the asset's status is not allowed", HttpStatus.FORBIDDEN);
throw new HttpException("Changing the asset's working status is not allowed", HttpStatus.FORBIDDEN);
}
};
3 changes: 2 additions & 1 deletion apps/server-asset-sg/src/features/asset-edit/asset-edit.http
Original file line number Diff line number Diff line change
Expand Up @@ -71,5 +71,6 @@ Content-Type: application/json
"titleOriginal": "My Cool Asset",
"titlePublic": "Our Cool Asset",
"typeNatRels": [],
"workgroupId": 1
"workgroupId": 1,
"assetFiles": []
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { Dialog, DialogRef } from '@angular/cdk/dialog';
import { ChangeDetectionStrategy, Component, Input, OnInit, TemplateRef, ViewChild, inject } from '@angular/core';
import { ChangeDetectionStrategy, Component, inject, Input, OnInit, TemplateRef, ViewChild } from '@angular/core';
import { FormGroupDirective } from '@angular/forms';
import { MatCheckboxChange } from '@angular/material/checkbox';
import { LifecycleHooks, LifecycleHooksDirective, fromAppShared } from '@asset-sg/client-shared';
import { fromAppShared, LifecycleHooks, LifecycleHooksDirective } from '@asset-sg/client-shared';
import { isNotNull } from '@asset-sg/core';
import { isMasterEditor } from '@asset-sg/shared/v2';
import * as RD from '@devexperts/remote-data-ts';
Expand All @@ -12,7 +12,7 @@ import { TranslateService } from '@ngx-translate/core';
import { RxState } from '@rx-angular/state';

import * as O from 'fp-ts/Option';
import { Observable, map, of, startWith, switchMap, withLatestFrom, filter } from 'rxjs';
import { filter, map, Observable, of, startWith, switchMap, withLatestFrom } from 'rxjs';
import { AssetEditDetailVM } from '../../models';
import { AssetEditorFormGroup, AssetEditorUsageFormGroup } from '../asset-editor-form-group';

Expand Down
3 changes: 0 additions & 3 deletions libs/shared/v2/src/lib/models/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@ const hasRole = (role: Role) => (user: User | null | undefined, workgroupId?: Wo
if (user == null) {
return false;
}
if (user.isAdmin) {
return true;
}
const roleIndex = getRoleIndex(role);
if (workgroupId != null) {
const role = user.roles.get(workgroupId);
Expand Down

0 comments on commit 8a554c8

Please sign in to comment.