fix(auth): Scope manager otherEventTokens to caller's events and mint MANAGER tokens - #52
Merged
tylermenezes merged 1 commit intoSep 18, 2026
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Detail bug report: View on Detail
Bug
The
isManagerbranch ofotherEventTokens(src/resolvers/Login.ts) was a verbatim copy of theisAdminbranch: it ran an unfilteredprisma.event.findMany()(returning every event in the DB) and signed each result withsignTokenAdmin. A caller presenting aMANAGERJWT would receive admin JWTs (typ=a, 24-week validity) for every event in the system — a vertical escalation (manager→admin) and horizontal escalation (their event→all events) in one query. The branch additionally ignored the dedicatedsignTokenManagerhelper (which had zero call sites), and unlike every other role branch, neither scoped by caller identity nor used the role-matching signer.Fix
Two changes, matching the canonical "caller's own events" pattern already used in
EventsWhereInputand the other role branches:src/utils/signToken.ts—signTokenManager(event, managerUsername)now embeds caller identity (tgt: AuthByTarget.USERNAME,sid: managerUsername). Without this,auth.usernameresolves toundefinedin the resolver and amanagerUsername: auth.username || ''predicate would match empty-string managers — the same class of defect fixed elsewhere in commit23d6242.src/resolvers/Login.ts— theisManagerbranch now scopes withwhere: { mentors: { some: { managerUsername: auth.username || '' } } }and signs withsignTokenManager(event, auth.username!), returning only the events the caller manages and mintingMANAGER(notADMIN) tokens.The
isAdmin,isStudent,isMentor, andisPartnerbranches are unchanged.Testing
tests/testOtherEventTokens.ts, following the repo'snode:testconvention): assertssignTokenManagermints MANAGER tokens with identity, the manager branch scopes to the caller'smanagerUsernameand never mints admin tokens, and the admin branch is unchanged (still unfiltered + admin tokens). Verified as a negative control by reverting only the manager branch to the buggy form — tests 3 & 4 fail (unscopedfindMany, admin tokens); restoring the fix makes all 5 pass.npx tsc --skipLibCheck --noEmit) and build pass; the pre-existingtests/testSlackReporting.tsstill passes (no regression).prisma db push(legacy migrations drift on a 2022 index drop), and seeded two events where the manager owns only one. Verified the fix at three layers — the Prisma scoping predicate directly, in-process GraphQL execution against the compiled schema + real DB, and over the full HTTP stack (a fake Elasticsearch stub on :9200 was needed because the server otherwise crashes on boot from the ES client's unhandled startup ping — no real ES available). In all layers the manager receives only her own event withMANAGER/mmtokens carrying her username, and the admin regression returns all events withADMIN/atokens. The same HTTP and in-process checks fail against the buggy branch (manager gets both events + admin tokens), confirming the verification catches the defect.@typescript-eslint/parser@3.10.1is incompatible with TypeScript 5 (a pre-existing breakage that fires on every file, including unmodified ones, and is not wired into any script or CI). Confirmed pre-existing by stashing the fix and linting the original code. Using an isolated compatible eslint (v8.57.1 + parser v8, no reponode_moduleschanges),signToken.tsis clean and the singleLogin.tsfinding ('AuthContext' is defined but never used) is identical ingit show HEAD:src/resolvers/Login.ts, i.e. not introduced by this fix.Automatic Fixes PRs can be configured here.