N21 2202 fix shd school data cannot be updated (#5261)

* update school rule * migration: Migration20240925165112 --------- Co-authored-by: Alexander Weber <[email protected]>
hpi-schul-cloud · Oct 7, 2024 · a6b710f · a6b710f
1 parent 5be1238
commit a6b710f
Show file tree

Hide file tree

Showing 6 changed files with 91 additions and 9 deletions.
diff --git a/apps/server/src/migrations/mikro-orm/Migration20240925165112.ts b/apps/server/src/migrations/mikro-orm/Migration20240925165112.ts
@@ -0,0 +1,37 @@
+import { Migration } from '@mikro-orm/migrations-mongodb';
+
+export class Migration20240925165112 extends Migration {
+	async up(): Promise<void> {
+		const adminRoleUpdate = await this.getCollection('roles').updateOne(
+			{ name: 'superhero' },
+			{
+				$addToSet: {
+					permissions: {
+						$each: ['SCHOOL_EDIT_ALL'],
+					},
+				},
+			}
+		);
+
+		if (adminRoleUpdate.modifiedCount > 0) {
+			console.info('Permission SCHOOL_EDIT_ALL added to role superhero.');
+		}
+	}
+
+	async down(): Promise<void> {
+		const adminRoleUpdate = await this.getCollection('roles').updateOne(
+			{ name: 'superhero' },
+			{
+				$pull: {
+					permissions: {
+						$in: ['SCHOOL_EDIT_ALL'],
+					},
+				},
+			}
+		);
+
+		if (adminRoleUpdate.modifiedCount > 0) {
+			console.info('Rollback: Permission SCHOOL_EDIT_ALL added to role superhero.');
+		}
+	}
+}
diff --git a/apps/server/src/modules/authorization/domain/rules/school.rule.spec.ts b/apps/server/src/modules/authorization/domain/rules/school.rule.spec.ts
@@ -1,3 +1,4 @@
+import { Permission } from '@shared/domain/interface/permission.enum';
 import { createMock, DeepMocked } from '@golevelup/ts-jest';
 import { schoolFactory } from '@modules/school/testing/school.factory';
 import { Test, TestingModule } from '@nestjs/testing';
@@ -9,11 +10,12 @@ import { SchoolRule } from './school.rule';
 describe('SchoolRule', () => {
 	let rule: SchoolRule;
 	let authorizationHelper: DeepMocked<AuthorizationHelper>;
+	let module: TestingModule;
 
 	beforeAll(async () => {
 		await setupEntities();
 
-		const module: TestingModule = await Test.createTestingModule({
+		module = await Test.createTestingModule({
 			providers: [SchoolRule, { provide: AuthorizationHelper, useValue: createMock<AuthorizationHelper>() }],
 		}).compile();
 
@@ -24,10 +26,19 @@ describe('SchoolRule', () => {
 	const setupSchoolAndUser = () => {
 		const school = schoolFactory.build();
 		const user = userFactory.build({ school: schoolEntityFactory.buildWithId(undefined, school.id) });
+		const superUser = userFactory.asSuperhero([Permission.SCHOOL_EDIT_ALL]).build();
 
-		return { school, user };
+		return { school, user, superUser };
 	};
 
+	afterEach(() => {
+		jest.clearAllMocks();
+	});
+
+	afterAll(async () => {
+		await module.close();
+	});
+
 	describe('isApplicable', () => {
 		describe('when object is instance of School', () => {
 			const setup = () => {
@@ -88,7 +99,7 @@ describe('SchoolRule', () => {
 				const { user, school } = setupSchoolAndUser();
 				const context = AuthorizationContextBuilder.read([]);
 
-				authorizationHelper.hasAllPermissions.mockReturnValueOnce(false);
+				authorizationHelper.hasAllPermissions.mockReturnValue(false);
 
 				return { user, school, context };
 			};
@@ -108,7 +119,7 @@ describe('SchoolRule', () => {
 				const someOtherSchool = schoolFactory.build();
 				const context = AuthorizationContextBuilder.read([]);
 
-				authorizationHelper.hasAllPermissions.mockReturnValueOnce(true);
+				authorizationHelper.hasAllPermissions.mockReturnValueOnce(false);
 
 				return { user, someOtherSchool, context };
 			};
@@ -121,5 +132,25 @@ describe('SchoolRule', () => {
 				expect(result).toBe(false);
 			});
 		});
+
+		describe('when the user has super powers', () => {
+			const setup = () => {
+				const { superUser } = setupSchoolAndUser();
+				const someOtherSchool = schoolFactory.build();
+				const context = AuthorizationContextBuilder.read([]);
+
+				authorizationHelper.hasAllPermissions.mockReturnValueOnce(true);
+
+				return { superUser, someOtherSchool, context };
+			};
+
+			it('should return true', () => {
+				const { superUser, someOtherSchool, context } = setup();
+
+				const result = rule.hasPermission(superUser, someOtherSchool, context);
+
+				expect(result).toBe(true);
+			});
+		});
 	});
 });
diff --git a/apps/server/src/modules/authorization/domain/rules/school.rule.ts b/apps/server/src/modules/authorization/domain/rules/school.rule.ts
@@ -1,5 +1,6 @@
 import { Injectable } from '@nestjs/common';
 import { User } from '@shared/domain/entity';
+import { Permission } from '@shared/domain/interface/permission.enum';
 import { School } from '@src/modules/school/domain/do';
 import { AuthorizationHelper } from '../service/authorization.helper';
 import { AuthorizationContext, Rule } from '../type';
@@ -15,11 +16,13 @@ export class SchoolRule implements Rule<School> {
 	}
 
 	public hasPermission(user: User, school: School, context: AuthorizationContext): boolean {
-		const hasRequiredPermissions = this.authorizationHelper.hasAllPermissions(user, context.requiredPermissions);
-
+		let hasPermission = false;
 		const isUsersSchool = user.school.id === school.id;
-
-		const hasPermission = hasRequiredPermissions && isUsersSchool;
+		if (isUsersSchool) {
+			hasPermission = this.authorizationHelper.hasAllPermissions(user, context.requiredPermissions);
+		} else {
+			hasPermission = this.authorizationHelper.hasAllPermissions(user, [Permission.SCHOOL_EDIT_ALL]);
+		}
 
 		return hasPermission;
 	}

diff --git a/apps/server/src/shared/domain/interface/permission.enum.ts b/apps/server/src/shared/domain/interface/permission.enum.ts
@@ -103,6 +103,7 @@ export enum Permission {
 	SCHOOL_CHAT_MANAGE = 'SCHOOL_CHAT_MANAGE',
 	SCHOOL_CREATE = 'SCHOOL_CREATE',
 	SCHOOL_EDIT = 'SCHOOL_EDIT',
+	SCHOOL_EDIT_ALL = 'SCHOOL_EDIT_ALL',
 	SCHOOL_LOGO_MANAGE = 'SCHOOL_LOGO_MANAGE',
 	SCHOOL_NEWS_EDIT = 'SCHOOL_NEWS_EDIT',
 	SCHOOL_PERMISSION_CHANGE = 'SCHOOL_PERMISSION_CHANGE',

diff --git a/backup/setup/migrations.json b/backup/setup/migrations.json
@@ -215,6 +215,15 @@
 			"$date": "2024-08-23T15:25:05.360Z"
 		}
 	},
+	{
+		"_id": {
+			"$oid": "66f440bf0dbeeb6747a4242c"
+		},
+		"name": "Migration20240925165112",
+		"created_at": {
+			"$date": "2024-09-25T16:56:31.889Z"
+		}
+	},
 	{
 		"_id": {
 			"$oid": "66fda9462a63b5749b3a64c9"

diff --git a/backup/setup/roles.json b/backup/setup/roles.json
@@ -203,7 +203,8 @@
 			"ACCOUNT_DELETE",
 			"USER_LOGIN_MIGRATION_FORCE",
 			"USER_LOGIN_MIGRATION_ROLLBACK",
-			"INSTANCE_VIEW"
+			"INSTANCE_VIEW",
+			"SCHOOL_EDIT_ALL"
 		],
 		"__v": 2
 	},