fix(api): query scope for RolesService and OrganizationService

Author: akdasa

modules/services/api/src/auth/utils/user-permissions.util.ts

@@ -15,58 +15,38 @@ export class UserPermissions {
    */
   public check(
     permissions: domain.PermissionKey[],
-    scope: { organizationId: string; schoolId?: string },
+    scope?: { organizationId: string; schoolId?: string },
   ) {
-    // Check if user has permission on the organization level
-    const permittedOnOrgLevel = this.getOrganizations(permissions).includes(
-      scope.organizationId,
-    );
-
-    // Check if user has permission on the school level
-    const permittedOnSchoolLevel = scope.schoolId
-      ? this.getSchools(permissions).includes(scope.schoolId)
-      : true;
-
-    // If user does not have permission on either the organization
-    // or school level, throw an error indicating that the user
-    // does not have permission
-    if (!permittedOnOrgLevel || !permittedOnSchoolLevel) {
+    const scopes = this.getScopes(permissions);
+    if (!scopes.length) {
       throw new ForbiddenException('User does not have permission');
     }
-  }
 
-  /**
-   * Returns list of organization ids that user has access to
-   * based on the provided permissions.
-   * @param permissions List of permissions to check
-   * @returns List of organization ids that user has access to
-   */
-  public getOrganizations(permissions: domain.PermissionKey[]): string[] {
-    return (
-      this.userPermissions
-        // organization level permissions
-        .filter((p) => p.oid && !p.sid)
-        // check if user has all required permissions
-        .filter((p) =>
-          permissions.every((permission) => p.p.includes(permission)),
-        )
-        .map((p) => p.oid)
+    if (!scope) {
+      return;
+    }
+
+    const permitted = scopes.some(
+      (s) =>
+        s.organizationId == scope?.organizationId &&
+        s.schoolId == scope?.schoolId,
     );
+
+    if (!permitted) {
+      throw new ForbiddenException('User does not have permission');
+    }
   }
 
-  /**
-   * Returns list of school ids that user has access to
-   * based on the provided permissions
-   * @param permissions List of permissions to check
-   * @returns List of school ids that user has access to
-   */
-  public getSchools(permissions: domain.PermissionKey[]): string[] {
+  public getScopes(
+    permissions: domain.PermissionKey[],
+  ): { organizationId: string; schoolId?: string }[] {
     return this.userPermissions
-      .filter(
-        (userPermission) =>
-          userPermission.sid &&
-          permissions.every((p) => userPermission.p.includes(p)),
+      .filter((p) =>
+        permissions.every((permission) => p.p.includes(permission)),
       )
-      .map((p) => p.sid);
+      .map((p) => ({
+        organizationId: p.oid,
+        schoolId: p.sid,
+      }));
   }
 }

modules/services/api/src/edu/controllers/organizations/organizations.controller.ts

@@ -46,6 +46,7 @@ export class OrganizationsController {
     @Param('id', new ParseUUIDPipe()) id: string,
     @UserWithPermissions() userPermissions: UserPermissions,
   ): Promise<dto.GetOrganizationResponse> {
+    userPermissions.check(['orgs:read']);
     const orgs = await this.organizationsService
       .scopedBy({ permissions: userPermissions })
       .findAll({ where: { id } });
@@ -67,6 +68,7 @@ export class OrganizationsController {
   async getMany(
     @UserWithPermissions() userPermissions: UserPermissions,
   ): Promise<GetOrganizationsResponse> {
+    userPermissions.check(['orgs:read']);
     const orgs = await this.organizationsService
       .scopedBy({ permissions: userPermissions })
       .findAll({});

modules/services/api/src/edu/controllers/organizations/specs/organizations.controller.e2e.get.spec.ts

@@ -67,7 +67,6 @@ describe('/edu/orgs', () => {
     return request(app.getHttpServer())
       .get(Routes().edu.org.find())
       .set('Authorization', `Bearer ${ctx.empty.tokens.noPermissions}`)
-      .expect(200)
-      .expect({ items: [] });
+      .expect(403);
   });
 });

modules/services/api/src/edu/controllers/organizations/specs/organizations.controller.test.get.spec.ts

@@ -20,8 +20,9 @@ describe('OrganizationsController', () => {
 
   describe('getOrganizations', () => {
     it('should return an empty array if no permissions granted', async () => {
-      const res = await ctr.getMany(ctx.empty.permissions.no);
-      expect(res.items).toEqual([]);
+      await expect(
+        async () => await ctr.getMany(ctx.empty.permissions.no),
+      ).rejects.toThrow();
     });
 
     it('should return all permitted organizations', async () => {

modules/services/api/src/edu/controllers/roles/roles.controller.ts

@@ -47,6 +47,7 @@ export class RolesController {
     @Param('id', new ParseUUIDPipe()) id: string,
     @UserWithPermissions() permissions: UserPermissions,
   ): Promise<dto.GetRoleResponse> {
+    permissions.check(['roles:read']);
     const roles = await this.rolesService
       .scopedBy({ permissions })
       .findAll({ where: { id } });
@@ -65,7 +66,13 @@ export class RolesController {
     @Query() query: dto.GetRoleSummariesListQuery,
     @UserWithPermissions() permissions: UserPermissions,
   ): Promise<dto.GetRolesResponse> {
-    const roles = await this.rolesService.scopedBy({ permissions }).findAll({});
+    permissions.check(['roles:read']);
+    const roles = await this.rolesService.scopedBy({ permissions }).findAll({
+      where: {
+        organizationId: query.organizationId,
+        schoolId: query.schoolId,
+      },
+    });
     return new dto.GetRolesResponse({
       items: this.mapper.mapArray(roles, entities.Role, dto.RoleSummary),
     });

modules/services/api/src/edu/controllers/roles/specs/roles.controller.e2e.get.spec.ts

@@ -101,7 +101,6 @@ describe('/edu/roles', () => {
     return request(app.getHttpServer())
       .get(Routes().edu.roles.find())
       .set('Authorization', `Bearer ${ctx.empty.tokens.noPermissions}`)
-      .expect(200)
-      .expect({ items: [] });
+      .expect(403);
   });
 });

modules/services/api/src/edu/services/organizations.service.ts

@@ -18,20 +18,23 @@ export class OrganizationsService extends ScopedEntitiesService<
       //       work here, so we have to cast it to any. It doesn't contain
       //       any fields like id, name, etc. But it should.
       const where = query?.where as any;
-      let orgIds = scope.permissions.getOrganizations(['orgs:read']);
 
-      // If the query has an id, so check if it is in the list of
-      // organization ids that the user has access to.
-      if (where?.id) {
-        orgIds = [where.id].filter((id) => orgIds.includes(id));
-      }
+      // Get all scopes that have the required permission and match the
+      // organization id in the query if it is provided
+      const scopes = scope.permissions
+        .getScopes(['orgs:read'])
+        .filter((s) => !where?.id || s.organizationId === where?.id);
 
-      return {
-        where: {
-          ...query?.where,
-          id: In(orgIds),
-        },
-      };
+      // Return the query with the scopes applied or an empty query
+      // if no scopes were found for the user permissions
+      return scopes.length > 0
+        ? {
+            where: scopes.map((s) => ({
+              ...query?.where,
+              id: s.organizationId,
+            })),
+          }
+        : { where: { id: In([]) } };
     });
   }
 }

modules/services/api/src/edu/services/roles.service.ts

@@ -12,15 +12,33 @@ export class RolesService extends ScopedEntitiesService<Role, Scope> {
     @InjectRepository(UserRole) private userRolesRepo: Repository<UserRole>,
   ) {
     super(repository, (query, scope) => {
-      const orgIds = scope.permissions.getOrganizations(['roles:read']);
-      const schoolIds = scope.permissions.getSchools(['roles:read']);
-      return {
-        where: {
-          ...query?.where,
-          organizationId: schoolIds.length == 0 ? In(orgIds) : undefined,
-          schoolId: schoolIds.length > 0 ? In(schoolIds) : undefined,
-        },
-      };
+      // HACK: For some reason FindOptionsWhere<Organization> doesn't
+      //       work here, so we have to cast it to any. It doesn't contain
+      //       any fields like id, name, etc. But it should.
+      const { organizationId, schoolId } = query?.where as any;
+
+      // Get all scopes that have the required permission
+      // and match the organization and school ids in the query
+      // if they are provided
+      const scopes = scope.permissions
+        .getScopes(['roles:read'])
+        .filter(
+          (s) =>
+            (!organizationId || s.organizationId === organizationId) &&
+            (!schoolId || s.schoolId === schoolId),
+        );
+
+      // Return the query with the scopes applied or an empty query
+      // if no scopes were found for the user permissions
+      return scopes.length > 0
+        ? {
+            where: scopes.map((s) => ({
+              ...query?.where,
+              organizationId: s.organizationId,
+              schoolId: s.schoolId,
+            })),
+          }
+        : { where: { organizationId: In([]) } };
     });
   }