Make the final AccessControl test pass, fixing a revocation inconsi…

…stency
beverloo · Jul 21, 2024 · 1e60664 · 1e60664
1 parent e4b51c8
commit 1e60664
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 4 deletions.
diff --git a/app/lib/auth/AccessControl.test.ts b/app/lib/auth/AccessControl.test.ts
@@ -214,9 +214,7 @@ describe('AccessControl', () => {
                 .not.toThrow();
     });
 
-    it.failing('should be able to revoke permissions for specific events or teams', () => {
-        // FIXME
-
+    it('should be able to revoke permissions for specific events or teams', () => {
         const accessControl = new AccessControl({
             grants: 'test',
             revokes: [

diff --git a/app/lib/auth/AccessControl.ts b/app/lib/auth/AccessControl.ts
@@ -92,7 +92,11 @@ export class AccessControl {
     #revokes: AccessList;
 
     constructor(grants: AccessControlParams) {
-        this.#revokes = new AccessList({ grants: grants.revokes })
+        this.#revokes = new AccessList({
+            grants: grants.revokes,
+            requireSpecificScope: true,
+        });
+
         this.#grants = new AccessList({
             expansions: kPermissionGroups,
             grants: grants.grants,

diff --git a/app/lib/auth/AccessList.test.ts b/app/lib/auth/AccessList.test.ts
@@ -543,4 +543,35 @@ describe('AccessList', () => {
         expect(accessList.query('test3', { team: 'staff' })).not.toBeUndefined();
         expect(accessList.query('test3', { event: '2024', team: 'staff' })).not.toBeUndefined();
     });
+
+    it('should have the ability to require a specific scope', () => {
+        // This tests a feature that ignores `kAnyEvent` and `kAnyTeam` values in scoped queries,
+        // which is behaviour that revocations specific to a particular event depend on without
+        // invalidating results for other events.
+
+        const regularAccessList = new AccessList({
+            grants: {
+                permission: 'test',
+                event: '2024',
+            },
+        });
+
+        expect(regularAccessList.query('test')).not.toBeUndefined();
+        expect(regularAccessList.query('test', { event: kAnyEvent })).not.toBeUndefined();  // <--
+        expect(regularAccessList.query('test', { event: '2024' })).not.toBeUndefined();
+        expect(regularAccessList.query('test', { event: '2025' })).toBeUndefined();
+
+        const requiredScopeAccessList = new AccessList({
+            grants: {
+                permission: 'test',
+                event: '2024',
+            },
+            requireSpecificScope: true,
+        });
+
+        expect(requiredScopeAccessList.query('test')).not.toBeUndefined();
+        expect(requiredScopeAccessList.query('test', { event: kAnyEvent })).toBeUndefined();  // <--
+        expect(requiredScopeAccessList.query('test', { event: '2024' })).not.toBeUndefined();
+        expect(requiredScopeAccessList.query('test', { event: '2025' })).toBeUndefined();
+    });
 });
diff --git a/app/lib/auth/AccessList.ts b/app/lib/auth/AccessList.ts
@@ -42,6 +42,12 @@ type AccessListParams = {
      * Teams that scoped permission queries should be granted for, on top of scoped grants.
      */
     teams?: string;
+
+    /**
+     * Whether the `kAnyEvent` and `kAnyTeam` qualifiers on an access check should be ignored for
+     * explicitly scoped entries. Important to be able to exclude specific scopes as a revoke.
+     */
+    requireSpecificScope?: boolean;
 };
 
 /**
@@ -101,11 +107,15 @@ export type Result = Pick<Access, 'expanded' | 'global'> & {
  * be used directly. Accessors may be exposed on there to provide access when necessary.
  */
 export class AccessList {
+    #requireSpecificScope: boolean;
+
     #access: Map<string, Access> = new Map;
     #events: Set<string> = new Set;
     #teams: Set<string> = new Set;
 
     constructor(params?: AccessListParams) {
+        this.#requireSpecificScope = !!params?.requireSpecificScope;
+
         if (!!params?.grants) {
             const grants = Array.isArray(params.grants) ? params.grants : [ params.grants ];
 
@@ -203,6 +213,13 @@ export class AccessList {
             for (const accessScope of access.scopes) {
                 let global: 'global' | undefined = undefined;
 
+                if (this.#requireSpecificScope) {
+                    if (scope.event === kAnyEvent && accessScope.event !== kAnyEvent)
+                        continue;
+                    if (scope.team === kAnyTeam && accessScope.team !== kAnyTeam)
+                        continue;
+                }
+
                 if (!!scope.event && scope.event !== kAnyEvent) {
                     if (accessScope.event !== scope.event && accessScope.event !== kAnyEvent) {
                         if (!globalEventAccess)