fix: seal session client-side in Session#refresh

[gjtorikian]

Summary

Fixes #469.

• Session#refresh was sending a session param to the API expecting it to return a pre-sealed cookie, but the API does not populate sealed_session for refresh-token grants — so it always came back as "".
• Removed the session body parameter and now seals the refreshed tokens client-side via SessionManager#seal_session_from_auth_response, consistent with how every other v7 auth flow handles sealing.
• Added 5 tests covering refresh success, round-trip unsealing, subsequent authenticate after refresh, error cases, and verifying the session param is no longer sent to the API.

Test plan

• test_refresh_seals_session_client_side_and_returns_refresh_success — verifies RefreshSuccess with valid sealed_session that round-trips through unseal
• test_refresh_updates_internal_seal_data_for_subsequent_authenticate — verifies a subsequent session.authenticate uses the refreshed token
• test_refresh_returns_error_on_invalid_cookie — garbage input returns RefreshError
• test_refresh_returns_error_when_no_refresh_token — missing refresh token returns RefreshError
• test_refresh_does_not_send_session_param_to_api — asserts seal_session is not in the request body
• Full suite passes (857 tests, 0 failures)

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

This PR fixes a broken Session#refresh flow where the WorkOS API never populated sealed_session for refresh-token grants, causing every refresh to persist an empty cookie. The fix moves sealing client-side via the existing seal_session_from_auth_response helper, consistent with all other v7 auth flows. The previous code also inadvertently sent cookie_password in the request body to the WorkOS API, which this change correctly removes.

Confidence Score: 4/5

Safe to merge — the core bug fix is correct and well-tested; remaining findings are P2 pre-existing concerns.

No P0 or P1 issues in the changed code. The fix is minimal, targeted, and consistent with how other auth flows work in v7. The two P2 observations (state mutation order before decode_jwt, and iss/aud JWT claim validation) are pre-existing in the codebase and not introduced by this PR. Test coverage for the new behaviour is thorough.

lib/workos/session_manager.rb — decode_jwt skips iss/aud validation (pre-existing, not changed in this PR)

Security Review

• JWT iss/aud claims not validated (lib/workos/session_manager.rb decode_jwt): verify_aud: false is set and no iss check is configured, so tokens from unintended issuers or audiences are accepted. This affects both Session#authenticate and the new Session#refresh path.
• Positive change: The previous code sent cookie_password to the WorkOS API inside the request body. This PR correctly removes that exposure.

Important Files Changed

Filename	Overview
lib/workos/session.rb	Removes server-side sealing (which was broken — the API never populated sealed_session) and replaces it with client-side sealing via SessionManager#seal_session_from_auth_response, consistent with all other v7 auth flows. Minor: state is mutated before decode_jwt, so a nil/malformed API access_token yields an unhandled exception with partial state; and decode_jwt still skips iss/aud validation.
test/workos/test_session.rb	Adds 5 well-structured tests covering the happy path, round-trip unsealing, subsequent authenticate after refresh, error cases (invalid cookie, missing refresh token), and the explicit assertion that seal_session is no longer sent in the request body.

Sequence Diagram

sequenceDiagram
    participant App
    participant Session
    participant SessionManager
    participant WorkOS API

    App->>Session: refresh(organization_id, cookie_password)
    Session->>SessionManager: unseal_data(seal_data, password)
    SessionManager-->>Session: access_token, refresh_token, user

    Session->>WorkOS API: POST /user_management/authenticate
    Note over Session,WorkOS API: grant_type=refresh_token (no cookie_password)
    WorkOS API-->>Session: new access_token, refresh_token, user

    Session->>SessionManager: seal_session_from_auth_response(tokens, user)
    SessionManager-->>Session: sealed_cookie

    Session->>Session: update seal_data and cookie_password
    Session->>SessionManager: decode_jwt(access_token)
    SessionManager-->>Session: decoded claims

    Session-->>App: RefreshSuccess with sealed_session

Loading

Comments Outside Diff (2)

1. lib/workos/session.rb, line 107-110 (link)

   Unhandled JWT errors after state mutation

   @seal_data and @cookie_password are mutated on lines 107–108 before decode_jwt is called. If the API returns HTTP 200 with a nil or malformed access_token, JWT.decode will raise JWT::DecodeError (or ArgumentError for nil) — neither is covered by the rescue WorkOS::AuthenticationError, WorkOS::InvalidRequestError clause. The session object is then left with partially-updated state while the caller receives an unhandled exception. A nil-guard on auth_response["access_token"] before the state mutation, or broadening the rescue to include JWT::DecodeError, would prevent this.

2. lib/workos/session.rb, line 110 (link)

   iss and aud claims not validated on the refresh JWT

   The decode_jwt call here (and in Session#authenticate) uses verify_aud: false and does not configure verify_iss. Both the issuer and audience claims are therefore never checked, meaning a JWT issued by a different provider or for a different audience would be accepted as valid. Per the project's JWT policy, both iss and aud must be validated before use. This is pre-existing but is now also exercised by the new refresh code path.

   Rule Used: JWTs should always be validated before use and the... (source)

_{Reviews (1): Last reviewed commit: "fix: seal session client-side in Session..." | Re-trigger Greptile}