fix: implement id token hint on RP-Initiated logout

[jnfrati]

Overview

Hello again! Thanks to all the Dex team for taking the time to review my contributions ❤️

The purpose of this PR is to introduce the id_token_hint when redirecting to upstream on RP-Initiated logout, while testing sessions we encountered the issue that some OIDC (e.g Pocket-id, Authentik) providers force-require the id_token_hint to be added when provided a post_logout_redirect_uri, this means that the chain RP -> OP(Dex) -> OP(Pocket-id) gets broken due to Dex not sending that extra parameter.

Even though the current implementation of Dex is not "broken" as it's compliant with the specification, having this extra id_token_hint in the URL makes integration easier when working with a wide variety of third party services. Still, I understand that the intention might not be to send the id_token_hint always, so happy to apply any recommended approach on how to "flag" when to send this or not 🙌

What this PR does / why we need it

This PR introduces:

• A new field AuthSession.ConnectorData []byte
• Adds logic to store the id_token received from upstream during login under AuthSession.ConnectorData
• Implements logic to add the query parameter id_token_hint in case connectorData is provided to LogoutCallbackConnector.LogoutURL, worth mentioning that:
  • The id_token_hint is added if it's present in the connectorData, meaning it does not depend on whether the post_redirect_logout_uri is present or not.
  • This is only implemented on oidc connector

This enables a much smoother integration with OIDC providers that require the id_token_hint to validate the identity of the requesting party.

Tests introduced

• TestLogoutURL: covers IDToken present/absent, malformed connector data, and hint emission without post_logout_redirect_uri.
• TestTryUpstreamLogoutPrefersSessionConnectorData: verifies precedence of auth-session data over offline-session data, plus fallback behavior.
• storage/conformance testAuthSessionCRUD now asserts ConnectorData round-trips through both create and update.

Special notes for your reviewer

My assumption on this PR is that the logout process is only done if sessions exist, meaning that there's a higher chance of finding the id_token on the AuthSession.ConnectorData than in the OfflineSession object. This is why in logout, the auth session takes precedence over the offline session, but please let me know if this is a correct assumption!

[nabokihms]

Thanks for the PR.

I have concerns about the approach of storing full ConnectorData on AuthSession. The connectorData struct in the OIDC connector carries both RefreshToken and IDToken. Storing it as-is in the auth session means upstream refresh tokens now persist in a second table — that's an unnecessary expansion of the attack surface.

More fundamentally, AuthSession and OfflineSession have different lifecycles and different needs. They shouldn't share the same opaque blob. See my inline comments for a proposed alternative.

[nabokihms]

NIT: Let's log the error here for debugging

[nabokihms]

Two issues here:

1. rawIDToken is already extracted and verified at the top of createIdentity (line 563). You can reuse it instead of extracting twice.
2. This is the core of my concern — IDToken gets packed into the same connectorData struct as RefreshToken, and this whole blob ends up in AuthSession.ConnectorData. That leaks the upstream refresh token into the auth session table.
   I think we should separate session-scoped data from refresh-scoped data at the connector.Identity level. Something like:

type Identity struct {
    // existing fields...
    ConnectorData []byte // refresh lifecycle → OfflineSession
    SessionData   []byte // session lifecycle → AuthSession
}

This is a struct, not an interface — adding a field is backward compatible. Connectors that don't set SessionData will have nil, no behavior change.

Then here you'd do:

// ConnectorData — for refresh, as before
cd := connectorData{RefreshToken: []byte(token.RefreshToken)}
identity.ConnectorData = marshal(cd)
 
// SessionData — only what's needed for session lifecycle (logout)
if rawIDToken, ok := token.Extra("id_token").(string); ok {
    sd := sessionData{IDToken: rawIDToken}
    identity.SessionData = marshal(sd)
}

This way auth session never sees the refresh token. SAML connectors can store SessionIndex in SessionData later without touching the model.

[nabokihms]

This overwrites ConnectorData on every session update. If we go with the SessionData approach above, this becomes old.SessionData = authReq.SessionData and carries only what the connector explicitly marked as session-scoped.

Same on line 293 (create path).

[nabokihms]

The precedence logic (auth session over offline session) makes sense, but the comment says "should keep an id_token" — that's OIDC-specific language in generic server code. With SessionData this becomes cleaner:

if len(session.SessionData) > 0 {
    connectorData = session.SessionData
}

No need to explain what's inside — the connector decides.

[nabokihms]

The comment references OIDC and id_token specifically — this shouldn't be in the generic storage model. With SessionData the comment becomes something like "connector-provided data for the session lifecycle, set during login".

[nabokihms]

Test data includes RefreshToken in auth session — this confirms the leak concern. With SessionData this should only contain session-scoped data (e.g. {"IDToken":"..."}).