Skip to content

fix(auth): Scope manager otherEventTokens to caller's events and mint MANAGER tokens - #52

Merged
tylermenezes merged 1 commit into
mainfrom
detail/bug-fix/fix-auth-scope-manager-othereventtokens-to-caller-ac5eb7
Sep 18, 2026
Merged

tylermenezes merged 1 commit into
mainfrom
detail/bug-fix/fix-auth-scope-manager-othereventtokens-to-caller-ac5eb7

Conversation

@detail-app

@detail-app detail-app Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Detail bug report: View on Detail

Bug

The isManager branch of otherEventTokens (src/resolvers/Login.ts) was a verbatim copy of the isAdmin branch: it ran an unfiltered prisma.event.findMany() (returning every event in the DB) and signed each result with signTokenAdmin. A caller presenting a MANAGER JWT 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 dedicated signTokenManager helper (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 EventsWhereInput and the other role branches:

  • src/utils/signToken.tssignTokenManager(event, managerUsername) now embeds caller identity (tgt: AuthByTarget.USERNAME, sid: managerUsername). Without this, auth.username resolves to undefined in the resolver and a managerUsername: auth.username || '' predicate would match empty-string managers — the same class of defect fixed elsewhere in commit 23d6242.
  • src/resolvers/Login.ts — the isManager branch now scopes with where: { mentors: { some: { managerUsername: auth.username || '' } } } and signs with signTokenManager(event, auth.username!), returning only the events the caller manages and minting MANAGER (not ADMIN) tokens.

The isAdmin, isStudent, isMentor, and isPartner branches are unchanged.

Testing

  • Unit test added (tests/testOtherEventTokens.ts, following the repo's node:test convention): asserts signTokenManager mints MANAGER tokens with identity, the manager branch scopes to the caller's managerUsername and 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 (unscoped findMany, admin tokens); restoring the fix makes all 5 pass.
  • Typecheck (npx tsc --skipLibCheck --noEmit) and build pass; the pre-existing tests/testSlackReporting.ts still passes (no regression).
  • End-to-end (not versioned; required standing up infra): installed and started a local PostgreSQL 16, applied the schema via 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 with MANAGER/mm tokens carrying her username, and the admin regression returns all events with ADMIN/a tokens. The same HTTP and in-process checks fail against the buggy branch (manager gets both events + admin tokens), confirming the verification catches the defect.
  • Lint: the repo's @typescript-eslint/parser@3.10.1 is 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 repo node_modules changes), signToken.ts is clean and the single Login.ts finding ('AuthContext' is defined but never used) is identical in git show HEAD:src/resolvers/Login.ts, i.e. not introduced by this fix.

Automatic Fixes PRs can be configured here.

@detail-app
detail-app Bot requested a review from tylermenezes September 18, 2026 02:53
@tylermenezes
tylermenezes merged commit cf09749 into main Sep 18, 2026
1 check passed
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