diff --git a/eslint.config.mjs b/eslint.config.mjs index 8ebbdb6..970de5c 100644 --- a/eslint.config.mjs +++ b/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', }, } ); diff --git a/jest.unit.config.ts b/jest.unit.config.ts index 405242e..5cd7809 100644 --- a/jest.unit.config.ts +++ b/jest.unit.config.ts @@ -19,6 +19,9 @@ const config: Config = { '^@wxyc/database$': '/tests/mocks/database.mock.ts', // Mock database client for any path resolving to shared/database/src/client '^.*/shared/database/src/client(\\.js)?$': '/tests/mocks/database.mock.ts', + // Mock better-auth access control modules (ESM-only, can't be transformed by ts-jest) + '^better-auth/plugins/access$': '/tests/mocks/better-auth-access.mock.ts', + '^better-auth/plugins/organization/access$': '/tests/mocks/better-auth-org-access.mock.ts', // Remove .js extensions from relative imports (ESM compatibility) '^(\\.{1,2}/.*)\\.(js)$': '$1', }, diff --git a/shared/authentication/src/auth.roles.ts b/shared/authentication/src/auth.roles.ts index fbfea1d..1a5bbe5 100644 --- a/shared/authentication/src/auth.roles.ts +++ b/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; diff --git a/tests/mocks/better-auth-access.mock.ts b/tests/mocks/better-auth-access.mock.ts new file mode 100644 index 0000000..3605c05 --- /dev/null +++ b/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; + +export function createAccessControl(_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, + }; + }, + }; +} diff --git a/tests/mocks/better-auth-org-access.mock.ts b/tests/mocks/better-auth-org-access.mock.ts new file mode 100644 index 0000000..ac3e6d6 --- /dev/null +++ b/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, +}; diff --git a/tests/unit/authentication/auth.middleware.test.ts b/tests/unit/authentication/auth.middleware.test.ts new file mode 100644 index 0000000..a2d7fbb --- /dev/null +++ b/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; + +/** 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 = {}) { + 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(); + }); + }); +}); diff --git a/tests/unit/authentication/auth.roles.test.ts b/tests/unit/authentication/auth.roles.test.ts new file mode 100644 index 0000000..b627bb4 --- /dev/null +++ b/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); + }); + }); +});