-
Notifications
You must be signed in to change notification settings - Fork 33
expose authentication object even if expired #398
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Summary
This PR modifies the WorkOS Ruby SDK's JWT session authentication to expose session data even when tokens are expired. The core change refactors the `authenticate` method in `lib/workos/session.rb` to use a retry mechanism that catches `JWT::ExpiredSignature` exceptions and re-decodes expired tokens with `verify_expiration: false`. This allows applications to access critical session information like `organization_id` from expired tokens, which is essential for admin users who need to maintain organizational context when refreshing their sessions.The implementation removes the is_valid_jwt helper method and replaces it with inline JWT decoding logic. When a token is expired, the method now returns an authentication object with authenticated: false and reason: 'INVALID_JWT' while still providing all decoded claims (session_id, organization_id, role, permissions, etc.). This addresses a specific use case in SaaS platforms where admin users can impersonate different tenant accounts - without access to the organization_id from expired sessions, these users would be incorrectly treated as regular users and logged out frequently.
The changes are accompanied by comprehensive test coverage that verifies the new behavior works correctly for expired tokens while maintaining existing functionality for valid tokens.
Important Files Changed
Changed Files
| Filename | Score | Overview |
|---|---|---|
| lib/workos/session.rb | 4/5 | Refactored JWT authentication to return session data even for expired tokens using retry mechanism with verify_expiration=false |
| spec/lib/workos/session_spec.rb | 5/5 | Added comprehensive test coverage for expired JWT token behavior ensuring authentication object is returned with authenticated=false |
Confidence score: 4/5
- This PR is generally safe to merge with some attention needed for the JWT handling changes
- Score reflects solid implementation with proper error handling and test coverage, but the retry mechanism adds complexity to critical authentication logic
- Pay close attention to lib/workos/session.rb for the new JWT decoding retry logic and ensure edge cases are properly handled
Sequence Diagram
sequenceDiagram
participant User
participant Session
participant JWT
participant UserManagement
User->>Session: "authenticate()"
Session->>Session: "check session_data exists"
alt session_data is nil
Session-->>User: "NO_SESSION_COOKIE_PROVIDED"
else
Session->>Session: "unseal_data(session_data, cookie_password)"
alt unseal fails
Session-->>User: "INVALID_SESSION_COOKIE"
else
Session->>JWT: "decode(access_token, verify_expiration: true)"
alt JWT valid
JWT-->>Session: "decoded payload"
Session-->>User: "authenticated: true + user data"
else JWT expired
Session->>JWT: "decode(access_token, verify_expiration: false)"
JWT-->>Session: "decoded payload (expired)"
Session-->>User: "authenticated: false + user data + INVALID_JWT reason"
else JWT decode error
Session-->>User: "INVALID_JWT"
end
end
end
Note over User,UserManagement: If expired token contains organization_id
User->>Session: "refresh(organization_id: extracted_org_id)"
Session->>Session: "unseal_data(session_data, cookie_password)"
Session->>UserManagement: "authenticate_with_refresh_token(organization_id)"
UserManagement-->>Session: "new auth_response with sealed_session"
Session-->>User: "authenticated: true + new sealed_session"
2 files reviewed, no comments
|
@nicknisi rebased against master, including the "roles" changes, would be good to include this in the next release if possible |
Description
We are a SaaS platform with both admins (our staff) and users (our clients) authenticate with WorkOS. Admins can go in and out of any tenant's account they have access to. However once in the tenant's account when the JWT expires, we have no idea from the expired session which
organization_idthey were part of so we end up trying to authenticate them as a regular user so they end up being logged out every 5 minutes (or whatever our expiration is set to).This PR firstly cleans up the jwt validation to use JWT::DecodeError instead of StandardError and uses
verify_expirationto allow the JWT to be decoded even if its expired, and pass that back, so even for expired sessions we can inspect the user'sorganization_idand make sure we then refresh the session with the correctorganization_id.EDIT: I would have thought also returning
EXPIRED_JWTinstead ofINVALID_JWTin this instance would also be helpful, but I'm unclear if that would introduce a breaking change so I avoided that for now.Documentation
Does this require changes to the WorkOS Docs? E.g. the API Reference or code snippets need updates.
If yes, link a related docs PR and add a docs maintainer as a reviewer. Their approval is required.