Refactor Session.roles: scopeless set (#3441)

Author: CarsonF

src/components/authorization/dto/role.dto.ts

@@ -1,13 +1,6 @@
 import { type Role } from '~/common';
 
-export type ProjectScope = 'project';
-export type GlobalScope = 'global';
-
-// Scope for roles. Does this role apply anywhere or only with project membership?
-export type AuthScope = GlobalScope | ProjectScope;
-
-export type ProjectScopedRole = `${ProjectScope}:${Role}`;
-export type GlobalScopedRole = `${GlobalScope}:${Role}`;
+type AuthScope = 'global' | 'project';
 
 export type ScopedRole = `${AuthScope}:${Role}` | 'member:true';
 

src/components/authorization/handler/can-impersonate.handler.ts

@@ -1,6 +1,5 @@
 import { CanImpersonateEvent } from '~/core/authentication/events/can-impersonate.event';
 import { EventsHandler } from '~/core/events';
-import { withoutScope } from '../dto';
 import { AssignableRoles } from '../dto/assignable-roles.dto';
 import { Privileges } from '../policy';
 
@@ -10,9 +9,9 @@ export class CanImpersonateHandler {
 
   handle(event: CanImpersonateEvent) {
     const p = this.privileges.for(AssignableRoles);
-    const valid = event.session.roles.every((role) =>
-      p.can('edit', withoutScope(role)),
-    );
+    const valid = event.session.roles
+      .values()
+      .every((role) => p.can('edit', role));
     event.allow.vote(valid);
   }
 }

src/components/authorization/policies/conditions/role.condition.ts

@@ -1,6 +1,5 @@
 import { inspect } from 'util';
 import { type Role } from '~/common';
-import { withoutScope } from '../../dto';
 import {
   type AsEdgeQLParams,
   type Condition,
@@ -13,7 +12,7 @@ export class RoleCondition implements Condition {
   constructor(readonly allowed: ReadonlySet<Role>) {}
 
   isAllowed({ session }: IsAllowedParams<any>) {
-    const given = session.roles.map(withoutScope);
+    const given = session.roles.values();
     return given.some((role) => this.allowed.has(role));
   }
 

src/components/authorization/policy/executor/policy-executor.ts

@@ -1,10 +1,9 @@
 import { forwardRef, Inject, Injectable } from '@nestjs/common';
 import { CachedByArg } from '@seedcompany/common';
-import { identity, intersection } from 'lodash';
+import { identity } from 'lodash';
 import { type EnhancedResource } from '~/common';
 import { Identity, type Session } from '~/core/authentication';
 import { type QueryFragment } from '~/core/database/query';
-import { withoutScope } from '../../dto';
 import { RoleCondition } from '../../policies/conditions/role.condition';
 import { type Permission } from '../builder/perm-granter';
 import {
@@ -148,8 +147,8 @@ export class PolicyExecutor {
       }
 
       const roleCondition =
-        policy.roles && policy.roles.length > 0
-          ? new RoleCondition(new Set(policy.roles))
+        policy.roles && policy.roles.size > 0
+          ? new RoleCondition(policy.roles)
           : undefined;
 
       if (!roleCondition && condition === true) {
@@ -280,11 +279,10 @@ export class PolicyExecutor {
       if (!policy.roles) {
         return true; // policy doesn't limit roles
       }
-      const rolesSpecifiedByPolicyThatUserHas = intersection(
-        policy.roles,
-        session.roles.map(withoutScope),
+      const rolesSpecifiedByPolicyThatUserHas = policy.roles.intersection(
+        session.roles,
       );
-      return rolesSpecifiedByPolicyThatUserHas.length > 0;
+      return rolesSpecifiedByPolicyThatUserHas.size > 0;
     });
     return policies;
   }

src/components/authorization/policy/policy.factory.ts

@@ -3,7 +3,7 @@ import {
   DiscoveryService,
 } from '@golevelup/nestjs-discovery';
 import { Injectable, type OnModuleInit } from '@nestjs/common';
-import { entries, mapEntries, mapValues } from '@seedcompany/common';
+import { entries, mapEntries, mapValues, setOf } from '@seedcompany/common';
 import { pick, startCase } from 'lodash';
 import { type DeepWritable, type Writable } from 'ts-essentials';
 import { type EnhancedResource, many, type Role } from '~/common';
@@ -35,7 +35,7 @@ export interface Policy {
   /* Policy Name */
   name: string;
   /* Only applies to these roles */
-  roles?: readonly Role[];
+  roles?: ReadonlySet<Role>;
   /* What the policy grants */
   grants: Grants;
   /* An optimization to determine Powers this policy contains */
@@ -103,7 +103,7 @@ export class PolicyFactory implements OnModuleInit {
   ): PlainPolicy {
     const name = startCase(discoveredClass.name.replace(/Policy$/, ''));
 
-    const roles = meta.role === 'all' ? undefined : many(meta.role);
+    const roles = meta.role === 'all' ? undefined : setOf(many(meta.role));
 
     const grants: WritableGrants = new Map();
     const resultList = many(meta.def(resGranter)).flat();

src/components/engagement/engagement-status.resolver.ts

@@ -20,7 +20,6 @@ export class EngagementStatusResolver {
     }
     return await this.engagementRules.getAvailableTransitions(
       status.parentId,
-      undefined,
       status.changeset,
     );
   }
@@ -31,7 +30,7 @@ export class EngagementStatusResolver {
       and change the status to any other status?
    `,
   })
-  async canBypassTransitions(): Promise<boolean> {
-    return await this.engagementRules.canBypassWorkflow();
+  canBypassTransitions(): boolean {
+    return this.engagementRules.canBypassWorkflow();
   }
 }

src/components/engagement/engagement.rules.ts

@@ -1,5 +1,6 @@
 /* eslint-disable no-case-declarations */
 import { Injectable } from '@nestjs/common';
+import { setOf } from '@seedcompany/common';
 import { node, relation } from 'cypher-query-builder';
 import { first, intersection } from 'lodash';
 import {
@@ -12,7 +13,6 @@ import { ILogger, Logger } from '~/core';
 import { Identity } from '~/core/authentication';
 import { DatabaseService } from '~/core/database';
 import { ACTIVE, INACTIVE } from '~/core/database/query';
-import { withoutScope } from '../authorization/dto';
 import { ProjectStep } from '../project/dto';
 import {
   EngagementStatus,
@@ -29,7 +29,7 @@ interface StatusRule {
   transitions: Transition[];
 }
 
-const rolesThatCanBypassWorkflow: Role[] = [Role.Administrator];
+const rolesThatCanBypassWorkflow = setOf<Role>([Role.Administrator]);
 
 @Injectable()
 export class EngagementRules {
@@ -314,7 +314,6 @@ export class EngagementRules {
 
   async getAvailableTransitions(
     engagementId: ID,
-    currentUserRoles?: Role[],
     changeset?: ID,
   ): Promise<EngagementStatusTransition[]> {
     const session = this.identity.current;
@@ -330,8 +329,7 @@ export class EngagementRules {
     );
 
     // If current user is not an approver (based on roles) then don't allow any transitions
-    currentUserRoles ??= session.roles.map(withoutScope);
-    if (intersection(approvers, currentUserRoles).length === 0) {
+    if (session.roles.intersection(setOf(approvers)).size === 0) {
       return [];
     }
 
@@ -356,28 +354,22 @@ export class EngagementRules {
     return availableTransitionsAccordingToProject;
   }
 
-  async canBypassWorkflow() {
-    const session = this.identity.current;
-    const roles = session.roles.map(withoutScope);
-    return intersection(rolesThatCanBypassWorkflow, roles).length > 0;
+  canBypassWorkflow() {
+    const { roles } = this.identity.current;
+    return roles.intersection(rolesThatCanBypassWorkflow).size > 0;
   }
 
   async verifyStatusChange(
     engagementId: ID,
     nextStatus: EngagementStatus,
     changeset?: ID,
   ) {
-    // If current user's roles include a role that can bypass workflow
-    // stop the check here.
-    const session = this.identity.current;
-    const currentUserRoles = session.roles.map(withoutScope);
-    if (intersection(rolesThatCanBypassWorkflow, currentUserRoles).length > 0) {
+    if (this.canBypassWorkflow()) {
       return;
     }
 
     const transitions = await this.getAvailableTransitions(
       engagementId,
-      currentUserRoles,
       changeset,
     );
 

src/components/project/project.service.ts

@@ -25,7 +25,6 @@ import { Identity } from '~/core/authentication';
 import { Transactional } from '~/core/database';
 import { type AnyChangesOf } from '~/core/database/changes';
 import { Privileges } from '../authorization';
-import { withoutScope } from '../authorization/dto';
 import { BudgetService } from '../budget';
 import { BudgetStatus, type SecuredBudget } from '../budget/dto';
 import { EngagementService } from '../engagement';
@@ -161,8 +160,9 @@ export class ProjectService {
         {
           userId: session.userId,
           roles: session.roles
-            .map(withoutScope)
-            .filter((role) => Role.applicableToProjectMembership.has(role)),
+            .values()
+            .filter((role) => Role.applicableToProjectMembership.has(role))
+            .toArray(),
           projectId: project,
         },
         false,

src/core/authentication/authentication.gel.repository.ts

@@ -2,7 +2,7 @@ import { Injectable } from '@nestjs/common';
 import { IntegrityError } from 'gel';
 import { type ID, type PublicOf, ServerException } from '~/common';
 import { RootUserAlias } from '../config/root-user.config';
-import { DbTraceLayer, disableAccessPolicies, e, Gel, withScope } from '../gel';
+import { DbTraceLayer, disableAccessPolicies, e, Gel } from '../gel';
 import type { AuthenticationRepository } from './authentication.repository';
 import { type LoginInput } from './dto';
 import { type Session } from './session/session.dto';
@@ -138,8 +138,8 @@ export class AuthenticationGelRepository
       return e.select(e.Auth.Session, (session) => ({
         filter_single: { token },
         userId: session.user.id,
-        roles: withScope('global', session.user.roles),
-        impersonateeRoles: withScope('global', impersonatee.roles),
+        roles: session.user.roles,
+        impersonateeRoles: impersonatee.roles,
       }));
     },
   );
@@ -151,7 +151,7 @@ export class AuthenticationGelRepository
     { userId: e.uuid },
     ({ userId }) => {
       const user = e.cast(e.User, userId);
-      return withScope('global', user.roles);
+      return user.roles;
     },
   );
 

src/core/authentication/authentication.repository.ts

@@ -1,8 +1,7 @@
 import { Injectable } from '@nestjs/common';
 import { node, relation } from 'cypher-query-builder';
 import { DateTime } from 'luxon';
-import { type ID, ServerException } from '~/common';
-import { type ScopedRole } from '../../components/authorization/dto';
+import { type ID, type Role, ServerException } from '~/common';
 import { DatabaseService, DbTraceLayer, OnIndex } from '../database';
 import {
   ACTIVE,
@@ -201,8 +200,8 @@ export class AuthenticationRepository {
       )
       .return<{
         userId: ID | null;
-        roles: readonly ScopedRole[];
-        impersonateeRoles: readonly ScopedRole[] | null;
+        roles: readonly Role[];
+        impersonateeRoles: readonly Role[] | null;
       }>([
         'user.id as userId',
         'roles',