Skip to content

Comments

fix: add admin role to WXYCRoles#156

Closed
jakebromberg wants to merge 7 commits intomainfrom
fix/add-admin-role-to-wxyc-roles
Closed

fix: add admin role to WXYCRoles#156
jakebromberg wants to merge 7 commits intomainfrom
fix/add-admin-role-to-wxyc-roles

Conversation

@jakebromberg
Copy link
Member

@jakebromberg jakebromberg commented Feb 14, 2026

Summary

  • Adds admin role definition to WXYCRoles so the requirePermissions middleware recognizes it
  • The admin role has the same permissions as stationManager plus better-auth's adminAc org-management statements
  • Adds 25 unit tests covering all role definitions and their permissions
  • Adds Jest mocks for better-auth access control modules (ESM-only packages that ts-jest can't transform)

Problem

The requirePermissions middleware validates JWT roles against WXYCRoles. On main, only member, dj, musicDirector, and stationManager are recognized. However, the auth system can assign "admin" as an organization member role (via the admin panel, org hooks, or better-auth's built-in roles). When that happens, the JWT contains role: "admin", but WXYCRoles["admin"] is undefined, causing 403 "Forbidden: Invalid role" on every authenticated request.

Test plan

  • Unit tests pass locally (145/145, including 25 new role tests)
  • Verify a user with member.role = 'admin' can access protected endpoints
  • Verify existing roles still work as before

Jake Bromberg and others added 7 commits February 13, 2026 16:45
The requirePermissions middleware validates JWT roles against WXYCRoles,
but WXYCRoles only included member, dj, musicDirector, and
stationManager. Users whose organization member role is "admin" get
403 "Invalid role" on every authenticated request because
WXYCRoles["admin"] is undefined.

Add an admin role definition with the same permissions as
stationManager (plus adminAc org-management statements) and register
it in WXYCRoles so the middleware recognizes it.

Also add unit tests for all WXYCRoles definitions and the mocks
needed to test better-auth access control modules under Jest.
Tests the exact code path that caused Jackson's 403 bug: a JWT with
role="admin" going through requirePermissions. Verifies the middleware
calls next() instead of returning 403 "Invalid role" now that
WXYCRoles includes admin.
The _prefix convention for unused params was configured for apps/ and
shared/ but not for tests/. This caused a lint error on the
_statements param in the better-auth mock.
Jest mock assertions like expect(res.status).toHaveBeenCalled()
trigger false positives from @typescript-eslint/unbound-method.
This is already downgraded to warn for app/shared code.
@jakebromberg
Copy link
Member Author

Closing: admin is a better-auth system role (auth_user.role), not a station role (auth_member.role). Instead of adding admin to WXYCRoles, we're removing it from wxyc-shared's hierarchy to match the 4-role model that Backend-Service and dj-site already use (see WXYC/wxyc-shared#8).

The test infrastructure in this PR (mocks, parameterized middleware tests) can be cherry-picked into a future PR.

The 403 bug for users with member.role = "admin" or "owner" will be addressed by:

  1. Ensuring no org members have those as their member.role (data fix)
  2. Adding role normalization in requirePermissions middleware (defense in depth)

jakebromberg pushed a commit that referenced this pull request Feb 14, 2026
Proposal 1 converges the auth model across wxyc-shared, Backend-Service,
and dj-site by removing admin from the station role hierarchy.

Proposal 2 adds cross-cutting capabilities (editor, webmaster) end-to-end
from Backend-Service storage through JWT to dj-site UI.

Proposal 1 status:
- wxyc-shared admin removal: done (PR WXYC/wxyc-shared#8)
- Backend-Service PR #156: closed
- Org hooks audit: completed, hooks are safe
- Remaining: re-add organizationClient, isAdmin in JWT, dj-site adoption
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant