ITADN

OAuth: finer scopes & no tms on jwt

#12944Openjordanh 创建于 2026-03-29
enhancement
J
jordanhcommented
Parabol's current OAuth 2.0 implementation has two fundamental limitations: 1. **Scopes are too coarse.** Only two scopes exist: `graphql:query` and `graphql:mutation`. An OAuth token with `graphql:mutation` can execute *any* mutation — from updating a task to deleting an organization. There is no resource-level granularity, violating the principle of least privilege. 2. **The `tms` (team memberships) pattern creates a divergence between session and OAuth tokens.** Session tokens embed `tms: string[]` for fast team membership checks. OAuth tokens are issued with `tms: []` (empty), which causes team-scoped mutations to fail. Additionally, `tms` is mutated in-memory during requests (e.g., `acceptTeamInvitation` pushes to `authToken.tms`), which is fragile, and tokens grow with team count. This plan addresses both issues with an architecture that aligns with industry best practices (GitHub, Slack, Linear, Shopify patterns). --- ## Part 1: Resource-Based OAuth Scopes ### Design Decision: `resource:action` Pattern **Why this pattern:** GitHub, Slack, and Shopify all use variations of `resource:action`. It maps naturally to GraphQL operations, is self-documenting, and hits the sweet spot between security and usability (Linear's 5 scopes are too coarse; Slack's 100+ are too granular). ### Scope Inventory | Scope | Covers | Maps to existing rules | |-------|--------|----------------------| | `meetings:read` | Query meeting data, phases, stages, reflections | `isTeamMemberOfMeeting`, `isMeetingMember` | | `meetings:write` | Start/end meetings, create reflections, vote, group | `isTeamMemberOfMeeting`, `isMeetingMember` | | `teams:read` | Query team info, members, settings | `isTeamMember` | | `teams:write` | Update team settings, manage members, invitations | `isTeamMember`, `isViewerTeamLead` | | `tasks:read` | Query tasks, task integrations | `isTeamMember` (via task's teamId) | | `tasks:write` | Create/update/delete tasks | `isTeamMember` | | `users:read` | Read own user profile, preferences | `isUserViewer` | | `users:write` | Update own profile, settings | `isUserViewer` | | `org:read` | Read org info, members, billing status | `isViewerOnOrg` | | `org:write` | Update org settings, domains | `isViewerBillingLeader` | | `org:admin` | Manage billing, SAML, SCIM, OAuth providers | `hasOrgRole('ORG_ADMIN')` | | `templates:read` | Read meeting templates | `isTeamMember` | | `templates:write` | Create/modify templates | `isTeamMember` | | `pages:read` | Read pages (collaborative docs) | `hasPageAccess('viewer')` | | `pages:write` | Create/edit pages | `hasPageAccess('editor')` | | `pages:admin` | Owner-only operations (archive, manage access, reparent) | `hasPageAccess('owner')` | | `comments:read` | Read discussion threads, comments | `isMeetingMember` | | `comments:write` | Post/edit comments | `isMeetingMember` | **Convenience scopes** (aggregate): | Scope | Implies | |-------|---------| | `read` | All `*:read` scopes | | `write` | All `*:read` + all `*:write` scopes | **Future scopes** (not implemented now, reserved for design consistency): | Scope | Purpose | |-------|---------| | `meetings:subscribe` | Real-time meeting updates via subscription | | `teams:subscribe` | Real-time team updates via subscription | | `tasks:subscribe` | Real-time task updates via subscription | Total: **18 resource scopes + 2 convenience scopes = 20 scopes** (with `*:subscribe` reserved for future) ### Schema-Level Scope Enforcement via Custom Directive Rather than hard-coding scope checks in a Yoga plugin, annotate the schema with a `@requireScope` directive. This makes scope requirements visible, introspectable, and co-located with the operations they protect. ```graphql directive @requireScope(scope: String!) on FIELD_DEFINITION type Mutation { createReflection(input: CreateReflectionInput!): CreateReflectionSuccess! @requireScope(scope: "meetings:write") updateTask(input: UpdateTaskInput!): UpdateTaskSuccess! @requireScope(scope: "tasks:write") archivePage(pageId: ID!): ArchivePageSuccess! @requireScope(scope: "pages:admin") } type Query { viewer: User! @requireScope(scope: "users:read") } ``` **Implementation approach:** 1. Define the directive in SDL 2. Replace `useOAuthScopeValidation` Yoga plugin with a directive-based transformer that: - Reads the `@requireScope` directive from the field definition - For OAuth tokens (`aud === 'action-oauth2'`), checks `authToken.scope` includes the required scope (or a convenience scope that implies it) - For session tokens, skips scope checks entirely (session tokens are fully privileged) 3. The existing `graphql-shield` permission rules (`isTeamMember`, `isMeetingMember`, etc.) continue to enforce resource-level access — scopes are **additive**, not a replacement **Why directives over middleware:** - Self-documenting: scope requirements are in the schema, not hidden in code - Auditable: can programmatically extract all scope requirements for docs - Aligned with Apollo Federation / WunderGraph Cosmo `@requiresScopes` pattern - Co-located: scope lives next to the operation, not in a separate permissions map ### Scope Hierarchy Resolution ```typescript // New file: packages/server/oauth2/scopeHierarchy.ts const SCOPE_IMPLIES: Record<string, string[]> = { 'write': ['read', ...ALL_WRITE_SCOPES, ...ALL_READ_SCOPES], 'read': [...ALL_READ_SCOPES], 'org:admin': ['org:write', 'org:read'], 'org:write': ['org:read'], 'pages:admin': ['pages:write', 'pages:read'], 'pages:write': ['pages:read'], 'teams:write': ['teams:read'], 'meetings:write': ['meetings:read'], 'tasks:write': ['tasks:read'], 'users:write': ['users:read'], 'templates:write': ['templates:read'], 'comments:write': ['comments:read'], } export function hasScope(tokenScopes: string[], requiredScope: string): boolean { return tokenScopes.some(s => s === requiredScope || (SCOPE_IMPLIES[s]?.includes(requiredScope) ?? false) ) } ``` --- ## Part 2: Removing `tms` from Tokens ### Design Decision: DataLoader-Based Team Membership Resolution **Why not keep `tms` on session tokens?** Even for session tokens, `tms` is problematic: - Token size grows with team count - Must refresh token on every team join/leave - In-memory mutation of `authToken.tms` during request handling is fragile - Creates a structural divergence between session and OAuth tokens ### Implementation #### Step 1: Create async team membership helper ```typescript // packages/server/utils/authorization.ts (new export) export const isTeamMemberAsync = async ( userId: string, teamId: string, dataLoader: DataLoaderWorker ): Promise<boolean> => { const teamMembers = await dataLoader.get('teamMembersByUserId').load(userId) return teamMembers.some((tm) => tm.teamId === teamId && tm.isNotRemoved) } ``` #### Step 2: Migrate graphql-shield rules to use DataLoader The `isTeamMember` rule (used in `permissions.ts`) already receives `context: GQLContext` which has `dataLoader`. Change from: ```typescript // Current: sync check against token if (!authToken.tms.includes(teamId)) return new GraphQLError(...) ``` To: ```typescript // New: async check against database via DataLoader const viewerId = getUserId(authToken) const isMember = await isTeamMemberAsync(viewerId, teamId, context.dataLoader) if (!isMember) return new GraphQLError(...) ``` Similarly migrate `isTeamMemberOfMeeting` which currently checks `authToken.tms`. #### Step 3: Migrate inline `authToken.tms` references 16 files reference `authToken.tms` directly. Each needs migration: | File | Current pattern | New pattern | |------|----------------|-------------| | `rules/isTeamMember.ts` | `authToken.tms.includes(teamId)` | `await isTeamMemberAsync(viewerId, teamId, dataLoader)` | | `rules/isTeamMemberOfMeeting.ts` | `authToken.tms.includes(meeting.teamId)` | `await isTeamMemberAsync(viewerId, meeting.teamId, dataLoader)` | | `mutations/acceptTeamInvitation.ts` | `tms.push(teamId)` + cookie refresh | Remove tms mutation; DB trigger handles membership | | `mutations/joinTeam.ts` | `authToken.tms = tms` | Remove tms mutation | | `mutations/updateDragLocation.ts` | `authToken.tms.includes(teamId)` | `await isTeamMemberAsync(...)` | | `mutations/setJiraDisplayFieldIds.ts` | `authToken.tms.includes(teamId)` | `await isTeamMemberAsync(...)` | | `types/User.ts` | `authToken.tms` for team filtering | DataLoader lookup | | `types/Organization.ts` | `authToken.tms` for team filtering | DataLoader lookup | | `types/TeamMember.ts` | `authToken.tms` | DataLoader lookup | | `types/*Payload.ts` (4 files) | `authToken.tms` for filtering published data | DataLoader lookup | | `fields/search.ts` | `authToken.tms` for search scope | DataLoader lookup | | `resolvers.ts` | `authToken.tms` for filtering | DataLoader lookup | #### Step 4: Replace `AuthTokenPayload` with `teamMembershipChanged` subscription Currently, `acceptTeamInvitation`, `addTeam`, and `joinTeam` push to `authToken.tms` in-memory and publish an `AuthTokenPayload` notification to refresh the client's token/tms. Since `tms` will no longer be on the token, this subscription no longer serves its original purpose. **Replace with a dedicated `teamMembershipChanged` subscription** that notifies the client when their team membership changes. The subscription should carry the information the client needs to update its sidebar (e.g., the added/removed team ID and the action taken). The client reacts by refetching its team list rather than parsing token claims. **Why a dedicated subscription (not reusing `AuthTokenPayload`):** - Subscriptions should be named for what they do — `AuthTokenPayload` describes a token update that is no longer happening - A `teamMembershipChanged` subscription can carry richer, purpose-specific data (team name, action type) rather than raw token claims - Cleaner separation of concerns: auth events vs. membership events #### Step 5: Phased removal of `tms` from AuthToken While the OAuth scope changes are internal-only, `tms` has been on session tokens for ~10 years and there are tens of thousands of users with valid 30-day session tokens that contain `tms`. We must handle this gracefully. **Phase A — Stop writing `tms` to new tokens, tolerate it on old ones:** 1. Make `tms` optional on `AuthToken`: ```typescript interface Input { sub: string tms?: string[] // Optional — old tokens have it, new tokens don't // ... } ``` 2. Stop populating `tms` when creating new tokens (in `attemptLogin.ts`, `refreshSession.ts`, `acceptTeamInvitation.ts`, etc.) 3. All authorization code already migrated to DataLoader in Steps 1-3, so `tms` is unused for auth decisions 4. Old tokens with `tms` still verify correctly — the field is simply ignored **Phase B — Wait for old tokens to expire, then remove `tms`:** Session tokens have a 30-day lifespan (`Threshold.JWT_LIFESPAN = 2592000000`). After 30 days from deployment of Phase A, all tokens in circulation will be new-format (no `tms`). At that point: 1. Remove the `tms` field from `AuthToken` entirely 2. Remove the `User.tms` column's PostgreSQL triggers (`updateUserTmsAfterTeam`, `updateUserTmsAfterTeamMember`) since nothing reads `tms` from tokens anymore 3. The `User.tms` column itself can remain in the database as a denormalized query optimization if other queries use it, or be dropped if unused
1 条评论