fix: add admin role to WXYCRoles

eslint.config.mjs

@@ -93,6 +93,8 @@ export default tseslint.config(
       '@typescript-eslint/no-unsafe-call': 'off',
       '@typescript-eslint/no-unsafe-member-access': 'off',
       '@typescript-eslint/no-unsafe-return': 'off',
+      '@typescript-eslint/no-unused-vars': ['warn', { argsIgnorePattern: '^_' }],
+      '@typescript-eslint/unbound-method': 'off',
     },
   }
 );

jest.unit.config.ts

@@ -19,6 +19,9 @@ const config: Config = {
     '^@wxyc/database$': '<rootDir>/tests/mocks/database.mock.ts',
     // Mock database client for any path resolving to shared/database/src/client
     '^.*/shared/database/src/client(\\.js)?$': '<rootDir>/tests/mocks/database.mock.ts',
+    // Mock better-auth access control modules (ESM-only, can't be transformed by ts-jest)
+    '^better-auth/plugins/access$': '<rootDir>/tests/mocks/better-auth-access.mock.ts',
+    '^better-auth/plugins/organization/access$': '<rootDir>/tests/mocks/better-auth-org-access.mock.ts',
     // Remove .js extensions from relative imports (ESM compatibility)
     '^(\\.{1,2}/.*)\\.(js)$': '$1',
   },

shared/authentication/src/auth.roles.ts

@@ -37,11 +37,19 @@ export const stationManager = accessControl.newRole({
   flowsheet: ['read', 'write'],
 });
 
+export const admin = accessControl.newRole({
+  ...adminAc.statements,
+  bin: ['read', 'write'],
+  catalog: ['read', 'write'],
+  flowsheet: ['read', 'write'],
+});
+
 export const WXYCRoles = {
   member,
   dj,
   musicDirector,
   stationManager,
+  admin,
 };
 
 export type WXYCRole = keyof typeof WXYCRoles;

tests/mocks/better-auth-access.mock.ts

@@ -0,0 +1,31 @@
+/**
+ * Minimal mock for better-auth/plugins/access
+ *
+ * Provides createAccessControl and newRole that mimic the real behavior
+ * closely enough to validate role permission definitions.
+ */
+
+type Statement = Record<string, readonly string[]>;
+
+export function createAccessControl<S extends Statement>(_statements: S) {
+  return {
+    newRole(permissions: Partial<{ [K in keyof S]: readonly string[] }>) {
+      return {
+        authorize(request: Partial<{ [K in keyof S]: string[] }>) {
+          for (const [resource, actions] of Object.entries(request)) {
+            if (!actions || (actions as string[]).length === 0) continue;
+            const allowed = permissions[resource as keyof S];
+            if (!allowed) return { success: false };
+            for (const action of actions as string[]) {
+              if (!(allowed as readonly string[]).includes(action)) {
+                return { success: false };
+              }
+            }
+          }
+          return { success: true };
+        },
+        statements: permissions,
+      };
+    },
+  };
+}

tests/mocks/better-auth-org-access.mock.ts

@@ -0,0 +1,23 @@
+/**
+ * Minimal mock for better-auth/plugins/organization/access
+ *
+ * Provides the same defaultStatements and adminAc values as the real module.
+ */
+
+export const defaultStatements = {
+  organization: ['update', 'delete'],
+  member: ['create', 'update', 'delete'],
+  invitation: ['create', 'cancel'],
+  team: ['create', 'update', 'delete'],
+  ac: ['create', 'read', 'update', 'delete'],
+} as const;
+
+export const adminAc = {
+  statements: {
+    organization: ['update'],
+    invitation: ['create', 'cancel'],
+    member: ['create', 'update', 'delete'],
+    team: ['create', 'update', 'delete'],
+    ac: ['create', 'read', 'update', 'delete'],
+  } as const,
+};

tests/unit/authentication/auth.middleware.test.ts

@@ -0,0 +1,189 @@
+// Set required env vars before module load (ts-jest transforms imports to requires,
+// so these execute before the middleware module's top-level code runs)
+process.env.BETTER_AUTH_JWKS_URL = 'https://test.example.com/.well-known/jwks.json';
+process.env.BETTER_AUTH_ISSUER = 'https://test.example.com';
+process.env.BETTER_AUTH_AUDIENCE = 'https://test.example.com';
+
+// Mock jose — the middleware calls createRemoteJWKSet at module scope
+// and jwtVerify on each request
+jest.mock('jose', () => ({
+  createRemoteJWKSet: jest.fn(() => jest.fn()),
+  jwtVerify: jest.fn(),
+}));
+
+import { jwtVerify } from 'jose';
+import { requirePermissions } from '../../../shared/authentication/src/auth.middleware';
+import type { Request, Response, NextFunction } from 'express';
+
+const mockedJwtVerify = jwtVerify as jest.MockedFunction<typeof jwtVerify>;
+
+/** Build minimal Express req/res/next mocks */
+function createMocks(authHeader?: string) {
+  const req = {
+    headers: authHeader ? { authorization: authHeader } : {},
+  } as unknown as Request;
+
+  const res = {
+    status: jest.fn().mockReturnThis(),
+    json: jest.fn().mockReturnThis(),
+  } as unknown as Response;
+
+  const next = jest.fn() as NextFunction;
+
+  return { req, res, next };
+}
+
+function mockJwtPayload(overrides: Record<string, unknown> = {}) {
+  mockedJwtVerify.mockResolvedValue({
+    payload: {
+      sub: 'test-user-id',
+      email: 'test@wxyc.org',
+      role: 'dj',
+      ...overrides,
+    },
+    protectedHeader: { alg: 'RS256' },
+    key: {} as any,
+  } as any);
+}
+
+describe('requirePermissions middleware', () => {
+  const originalAuthBypass = process.env.AUTH_BYPASS;
+
+  beforeEach(() => {
+    delete process.env.AUTH_BYPASS;
+  });
+
+  afterAll(() => {
+    if (originalAuthBypass !== undefined) {
+      process.env.AUTH_BYPASS = originalAuthBypass;
+    } else {
+      delete process.env.AUTH_BYPASS;
+    }
+  });
+
+  describe('AUTH_BYPASS', () => {
+    it('should skip all validation when AUTH_BYPASS is "true"', async () => {
+      process.env.AUTH_BYPASS = 'true';
+      const { req, res, next } = createMocks(); // no auth header
+      const middleware = requirePermissions({ catalog: ['read'] });
+
+      await middleware(req, res, next);
+
+      expect(next).toHaveBeenCalled();
+      expect(res.status).not.toHaveBeenCalled();
+    });
+  });
+
+  describe('token validation', () => {
+    it('should return 401 when Authorization header is missing', async () => {
+      const { req, res, next } = createMocks();
+      const middleware = requirePermissions({ catalog: ['read'] });
+
+      await middleware(req, res, next);
+
+      expect(res.status).toHaveBeenCalledWith(401);
+      expect(next).not.toHaveBeenCalled();
+    });
+
+    it('should return 401 when JWT verification fails', async () => {
+      jest.spyOn(console, 'error').mockImplementation(() => {}); // suppress expected log
+      mockedJwtVerify.mockRejectedValue(new Error('invalid signature'));
+      const { req, res, next } = createMocks('Bearer bad-token');
+      const middleware = requirePermissions({ catalog: ['read'] });
+
+      await middleware(req, res, next);
+
+      expect(res.status).toHaveBeenCalledWith(401);
+      expect(next).not.toHaveBeenCalled();
+    });
+  });
+
+  describe('role validation', () => {
+    it('should return 403 "Invalid role" for an unrecognized role', async () => {
+      mockJwtPayload({ role: 'nonexistent_role' });
+      const { req, res, next } = createMocks('Bearer valid-token');
+      const middleware = requirePermissions({ catalog: ['read'] });
+
+      await middleware(req, res, next);
+
+      expect(res.status).toHaveBeenCalledWith(403);
+      expect(res.json).toHaveBeenCalledWith(
+        expect.objectContaining({ error: expect.stringContaining('Invalid role') })
+      );
+      expect(next).not.toHaveBeenCalled();
+    });
+
+    /**
+     * This is the exact regression test for Jackson's 403 bug.
+     *
+     * Before the fix, WXYCRoles did not include "admin". When better-auth
+     * assigned role="admin" to a user's JWT (via org hooks / syncAdminRoles),
+     * the middleware hit line 106-107:
+     *
+     *   const roleImpl = WXYCRoles[payload.role]; // undefined
+     *   if (!roleImpl) return res.status(403)      // 403 "Invalid role"
+     *
+     * With the fix, WXYCRoles.admin is defined, so the middleware proceeds.
+     */
+    it('should NOT return 403 for the "admin" role', async () => {
+      mockJwtPayload({ role: 'admin' });
+      const { req, res, next } = createMocks('Bearer valid-token');
+      const middleware = requirePermissions({ catalog: ['read'] });
+
+      await middleware(req, res, next);
+
+      expect(next).toHaveBeenCalled();
+      expect(res.status).not.toHaveBeenCalled();
+    });
+  });
+
+  describe('permission checks', () => {
+    it.each(['member', 'dj', 'musicDirector', 'stationManager', 'admin'] as const)(
+      '"%s" should be authorized for catalog:read',
+      async (role) => {
+        mockJwtPayload({ role });
+        const { req, res, next } = createMocks('Bearer valid-token');
+        const middleware = requirePermissions({ catalog: ['read'] });
+
+        await middleware(req, res, next);
+
+        expect(next).toHaveBeenCalled();
+        expect(res.status).not.toHaveBeenCalled();
+      }
+    );
+
+    it('should return 403 "insufficient permissions" when role lacks required permission', async () => {
+      mockJwtPayload({ role: 'member' });
+      const { req, res, next } = createMocks('Bearer valid-token');
+      const middleware = requirePermissions({ catalog: ['write'] });
+
+      await middleware(req, res, next);
+
+      expect(res.status).toHaveBeenCalledWith(403);
+      expect(res.json).toHaveBeenCalledWith(
+        expect.objectContaining({ error: expect.stringContaining('insufficient permissions') })
+      );
+      expect(next).not.toHaveBeenCalled();
+    });
+
+    it('admin should be authorized for flowsheet:write', async () => {
+      mockJwtPayload({ role: 'admin' });
+      const { req, res, next } = createMocks('Bearer valid-token');
+      const middleware = requirePermissions({ flowsheet: ['write'] });
+
+      await middleware(req, res, next);
+
+      expect(next).toHaveBeenCalled();
+    });
+
+    it('admin should be authorized for catalog:write', async () => {
+      mockJwtPayload({ role: 'admin' });
+      const { req, res, next } = createMocks('Bearer valid-token');
+      const middleware = requirePermissions({ catalog: ['write'] });
+
+      await middleware(req, res, next);
+
+      expect(next).toHaveBeenCalled();
+    });
+  });
+});

tests/unit/authentication/auth.roles.test.ts

@@ -0,0 +1,59 @@
+import { WXYCRoles, type WXYCRole } from '../../../shared/authentication/src/auth.roles';
+
+describe('WXYCRoles', () => {
+  const allRoles = Object.keys(WXYCRoles) as WXYCRole[];
+
+  it('should include the admin role', () => {
+    expect(WXYCRoles).toHaveProperty('admin');
+  });
+
+  /**
+   * These are the roles that better-auth's organization plugin and
+   * the auth hooks in auth.definition.ts may assign to members.
+   * If any of them are missing from WXYCRoles, the requirePermissions
+   * middleware will return 403 "Invalid role" for those users.
+   */
+  it.each(['member', 'dj', 'musicDirector', 'stationManager', 'admin'])('should recognize the "%s" role', (role) => {
+    expect(WXYCRoles).toHaveProperty(role);
+    expect(WXYCRoles[role as WXYCRole]).toBeDefined();
+  });
+
+  describe('role permissions', () => {
+    it.each(allRoles)('"%s" should have an authorize function', (role) => {
+      const roleDef = WXYCRoles[role];
+      expect(typeof (roleDef as any).authorize).toBe('function');
+    });
+
+    it.each(allRoles)('"%s" should authorize catalog:read', (role) => {
+      const roleDef = WXYCRoles[role];
+      const result = (roleDef as any).authorize({ catalog: ['read'] });
+      expect(result.success).toBe(true);
+    });
+
+    it.each(allRoles)('"%s" should authorize bin:read', (role) => {
+      const roleDef = WXYCRoles[role];
+      const result = (roleDef as any).authorize({ bin: ['read'] });
+      expect(result.success).toBe(true);
+    });
+
+    it('admin should authorize catalog:write', () => {
+      const result = (WXYCRoles.admin as any).authorize({ catalog: ['write'] });
+      expect(result.success).toBe(true);
+    });
+
+    it('admin should authorize flowsheet:write', () => {
+      const result = (WXYCRoles.admin as any).authorize({ flowsheet: ['write'] });
+      expect(result.success).toBe(true);
+    });
+
+    it('member should NOT authorize flowsheet:write', () => {
+      const result = (WXYCRoles.member as any).authorize({ flowsheet: ['write'] });
+      expect(result.success).toBe(false);
+    });
+
+    it('member should NOT authorize catalog:write', () => {
+      const result = (WXYCRoles.member as any).authorize({ catalog: ['write'] });
+      expect(result.success).toBe(false);
+    });
+  });
+});