Skip to content

Conversation

@kendallstrautman
Copy link
Contributor

@kendallstrautman kendallstrautman commented Nov 26, 2025

For multiple user roles in the session jwt.

@linear
Copy link

linear bot commented Nov 26, 2025

ENT-4685 Remix

@kendallstrautman kendallstrautman marked this pull request as ready for review November 26, 2025 23:15
@greptile-apps
Copy link

greptile-apps bot commented Nov 26, 2025

Greptile Overview

Greptile Summary

This PR adds support for multiple user roles to the session JWT, extending the existing single role field. The implementation properly threads the new roles array through all authentication interfaces and functions while maintaining backward compatibility.

Key changes:

  • Added optional roles?: string[] field to all authentication-related interfaces (AccessToken, UserInfo, AuthorizedData, etc.)
  • Updated getClaimsFromAccessToken to extract and return the roles array from JWT
  • Modified authkitLoader, refreshSession, and withAuth to propagate roles through the authentication flow
  • Set default value to null for roles in AuthorizedData to match the pattern of other optional fields like role and organizationId
  • Added comprehensive test coverage for multiple roles scenarios and backward compatibility when roles field is missing

The implementation is clean, consistent with the existing codebase patterns, and properly handles both the presence and absence of the roles field in JWTs for backward compatibility.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The implementation is clean, follows existing patterns consistently, and includes comprehensive test coverage. The roles field is properly typed as optional across all interfaces, uses appropriate null defaults for backward compatibility, and is threaded correctly through all authentication flows. No security concerns were identified, and the changes align with the existing single role field pattern.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
src/interfaces.ts 5/5 Added optional roles field to AccessToken, UserInfo, NoUserInfo, AuthorizedData, and UnauthorizedData interfaces for multi-role support
src/session.ts 5/5 Threaded roles through refreshSession, authkitLoader, and getClaimsFromAccessToken functions with proper null defaults for backward compatibility
src/auth.ts 5/5 Added roles extraction and propagation in withAuth function to support multiple user roles from JWT claims

Sequence Diagram

sequenceDiagram
    participant Client
    participant authkitLoader
    participant getClaimsFromAccessToken
    participant JWT
    participant Session

    Client->>authkitLoader: Request with session cookie
    authkitLoader->>Session: getSessionFromCookie()
    Session-->>authkitLoader: Session with accessToken
    authkitLoader->>getClaimsFromAccessToken: Extract claims from JWT
    getClaimsFromAccessToken->>JWT: decodeJwt(accessToken)
    JWT-->>getClaimsFromAccessToken: {sid, org_id, role, roles, permissions, entitlements}
    getClaimsFromAccessToken-->>authkitLoader: Claims with roles array
    authkitLoader->>authkitLoader: Build AuthorizedData with roles
    authkitLoader-->>Client: {user, role, roles, permissions, ...}
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Member

@nicknisi nicknisi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good! Since Remix is being succeeded by React Router, we'll want to make this change there as well. https://github.com/workos/authkit-react-router

@kendallstrautman
Copy link
Contributor Author

Okay great, will make a follow up ticket!

@kendallstrautman kendallstrautman force-pushed the feature/ent-4685-remix branch 3 times, most recently from 17f64c5 to 9592ec9 Compare December 4, 2025 19:25
@kendallstrautman kendallstrautman merged commit cdfc47c into main Dec 4, 2025
6 checks passed
@kendallstrautman kendallstrautman deleted the feature/ent-4685-remix branch December 4, 2025 19:37
@greptile-apps greptile-apps bot mentioned this pull request Dec 4, 2025
@kendallstrautman
Copy link
Contributor Author

kendallstrautman commented Dec 4, 2025

@nicknisi took a look and it seems this is already supported in the authkit-react-router package. We must have missed remix during initial multiple roles launch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants