Skip to content

fix: prevent partial state mutation in Session#refresh #471

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.

Sign up for GitHub

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

Merged
gjtorikian merged 1 commit into from
Apr 29, 2026
+33 −2
Merged

fix: prevent partial state mutation in Session#refresh #471

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions lib/workos/session.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,13 @@ def refresh(organization_id: nil, cookie_password: nil)
user: auth_response["user"],
impersonator: auth_response["impersonator"]
)
@seal_data = sealed
@cookie_password = effective_password

# Decode before mutating session state so a malformed access_token
# doesn't leave the Session half-updated.
decoded = @manager.decode_jwt(auth_response["access_token"])
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot Apr 29, 2026

Choose a reason for hiding this comment

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

P2 iss/aud claims not validated on decode

decode_jwt is called here without validating the iss or aud claims. The PR description explicitly notes this is intentional (sealed cookie already authenticates the data, and all WorkOS SDKs share this behaviour), but the team's engineering rule requires both claims to be verified. If the cross-SDK consistency constraint ever relaxes, the missing validation here would allow a JWT signed for a different audience or issuer to be accepted.

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


@seal_data = sealed
@cookie_password = effective_password
SessionManager::RefreshSuccess.new(
authenticated: true,
sealed_session: sealed,
Expand All @@ -123,6 +126,8 @@ def refresh(organization_id: nil, cookie_password: nil)
)
rescue WorkOS::AuthenticationError, WorkOS::InvalidRequestError => e
SessionManager::RefreshError.new(authenticated: false, reason: e.message)
rescue JWT::DecodeError => e
SessionManager::RefreshError.new(authenticated: false, reason: e.message)
end

# Build the WorkOS session-logout URL for the currently authenticated session.
Expand Down
26 changes: 26 additions & 0 deletions test/workos/test_session.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,32 @@ def test_refresh_does_not_send_session_param_to_api
assert_requested(stub)
end

def test_refresh_returns_error_on_malformed_access_token_without_mutating_state
rsa, pub = signing_key_pair
old_access = make_jwt({"sid" => "session_old", "exp" => Time.now.to_i - 60}, rsa)
sealed = @sm.seal_data({"access_token" => old_access, "refresh_token" => "rt_old", "user" => {"id" => "u_1"}}, PASSWORD)

api_response = {
"access_token" => "not-a-valid-jwt",
"refresh_token" => "rt_new",
"user" => {"id" => "u_1"}
}

stub_request(:post, "https://api.workos.com/user_management/authenticate")
.to_return(status: 200, body: api_response.to_json)
stub_request(:get, "https://api.workos.com/sso/jwks/client_001")
.to_return(status: 200, body: jwks_payload(pub).to_json)

session = @sm.load(seal_data: sealed, cookie_password: PASSWORD)
result = session.refresh

assert_kind_of WorkOS::SessionManager::RefreshError, result
refute result.authenticated

# Session state should not have been mutated
assert_equal sealed, session.seal_data
end

# --- Session constructor validation ---------------------------------------

def test_session_load_requires_cookie_password
Expand Down