Pass current session (access token extensions) to refresh token hook

[alekbarszczewski]

Preflight checklist

• I could not find a solution in the existing issues, docs, nor discussions.
• I agree to follow this project's Code of Conduct.
• I have read and am following this repository's Contribution Guidelines.
• This issue affects my Ory Cloud project.
• I have joined the Ory Community Slack.
• I am signed up to the Ory Security Patch Newsletter.

Describe your problem

I am adding custom session extensions to access token when accepting consent request:

var request = new HydraAcceptConsentRequest(
    grantScope: scope.ToList(),
    session: new HydraConsentRequestSession(accessToken: new Dictionary<string, object>()
    {
        { "sid", sessionId }
    })
);

var result = await _adminApi.AcceptConsentRequestAsync(consentChallenge, request);

And JWT token looks like this:

{
  "sub": "xxx",
  ...
  "ext": { // <--- extensions
    "sid": "14628d10-8c63-4c7f-8a9c-3e37318ce7d6"
  },
  ...
}

Normally when token is refreshed "ext" field remain the same after each refresh which is good, "sid" remains the same which is what I want. However when using refresh token hook (a webhook which is called before token refresh to refresh claims described here https://www.ory.sh/docs/hydra/guides/claims-at-refresh#webhook-configuration) there is no option to keep "ext" field the same.

1. When refresh token webhook returns 200 OK with empty json then "ext" field is removed from new JWT
2. When refresh token webhook returns 200 OK with no body then hydra throws error (body parsing error)
3. When refresh token webhook return 204 No Content then hydra throws error (it's expecting 200 and json content)

I could return extensions from refresh token webhook, but the problem is that previous extensions are not passed to the webhook payload which looks like this:

{
  "subject": "foo",
  "client_id": "bar",
  "granted_scopes": ["openid", "offline"],
  "granted_audience": []
}

So it's not possible to set same { "sid": "14628d10-8c63-4c7f-8a9c-3e37318ce7d6" } extension.

Describe your ideal solution

Refresh token hook payload should contain previous session data / extensions:

{
  "subject": "foo",
  "client_id": "bar",
  "granted_scopes": ["openid", "offline"],
  "granted_audience": [],

  "session": { // <--- new field in refresh token hook payload
    "access_token": {
      "foo": "bar"
    },
    "id_token": {
      "bar": "baz"
    }
  }
}

This would allow to return exactly same session data as in "previous" access token (return from refresh token webhook)

Workarounds or alternatives

If refresh token hook returned 204 No Content Hydra would not override session data (which is default behaviour when refresh token hook is not being used at all).

Version

v1.10.7

Additional Context

No response

[aeneasr]

That makes total sense - PRs snd contributions welcomed!

[svrakitin]

Hey @aeneasr, I can work on this and extend the payload of refresh token hook request.

[aeneasr]

Awesome! If you do, please make your changes against the v2.x branch :)

[pjediny]

@aeneasr I wonder if such session data could be passed to login phase too? I want to build an OIDC broker and it would be handy to get the connector id to upstream IDP without id_token_hint(_claims) or encoding connector id into a sub.

[sneko]

Hi all,

I'm facing this after enabling the refresh hook. Hydra always throws to my users Logout failed because query parameter id_token_hint is missing sid claim. when they try to log out.

I saw https://www.ory.sh/docs/hydra/cli/hydra-clients-create where it's specified:

      --backchannel-logout-session-required      Boolean flag specifying whether the client requires that a sid (session ID) Claim be included in the Logout Token to identify the client session with the OP when the backchannel-logout-callback is used. If omitted, the default value is false.
      --frontchannel-logout-session-required     Boolean flag specifying whether the client requires that a sid (session ID) Claim be included in the Logout Token to identify the client session with the OP when the frontchannel-logout-callback is used. If omitted, the default value is false.

but I'm not sure this is the right way of doing it since https://github.com/ory/hydra/blob/master/consent/strategy_default.go#L823-L826
shows the sid is always looked after.

I'm on v1.11.7, is there any way to fix this easily? I can confirm at login I have the sid but I loose it after refreshing 😢

Thank you,

[sneko]

I investigated more about the refresh hook and I wanted to manually patch the sessionID into the refreshed tokens claims... but there is no way since Hydra does not give me the initial loginChallenge ^^...

Solutions from now:

• Wait for the Hydra team to patch the v1.x as done for v2.x (but https://github.com/ory/hydra/pull/3146/files does not allow to overrides claims while keeping sid, it just prevents overriding claims from what I understand)
• Use a custom image for Hydra that includes a patch to merge "new claims" over "old claims"
• Find a way to disable the "sid" check when refreshing (temporarly) but according to the code I saw it's not possible
• Set a fixed "sid" (sessionID) both on login and refresh_token... sounds like a possible way even if really hacky (and not ideal from a security perspective... but should work until a proper fix is found?). And it would not solve the issue for people already logged in unfortunately.

cc @aeneasr

[kruczjak]

I investigated more about the refresh hook and I wanted to manually patch the sessionID into the refreshed tokens claims... but there is no way since Hydra does not give me the initial loginChallenge ^^...

Solutions from now:

• Wait for the Hydra team to patch the v1.x as done for v2.x (but https://github.com/ory/hydra/pull/3146/files does not allow to overrides claims while keeping sid, it just prevents overriding claims from what I understand)
• Use a custom image for Hydra that includes a patch to merge "new claims" over "old claims"
• Find a way to disable the "sid" check when refreshing (temporarly) but according to the code I saw it's not possible
• Set a fixed "sid" (sessionID) both on login and refresh_token... sounds like a possible way even if really hacky (and not ideal from a security perspective... but should work until a proper fix is found?). And it would not solve the issue for people already logged in unfortunately.

cc @aeneasr

Hi, I'm just experiencing same problem and I've decided to go with option Use a custom image for Hydra that includes a patch to merge "new claims" over "old claims" for now.

I've made a small change here: https://github.com/ory/hydra/blob/master/oauth2/hook.go#L122

Would be nice to have proper solution as it seems like a bug - but I don't know which solution works best.
Maybe add sid to refresh token hook request params to be able to render it?

[sneko]

@kruczjak I saw yesterday it was fixed into v1.11.9 which is great. So you have to upgrade your Hydra instance, but it requires more work from your hook handler because:

1. Hydra release is malformed (ref: Invalid snapshot filename breaks importing the Hydra packages #3213) you cannot import the package for now
2. In the hook you can try to reuse their structs to decode the JSON payload but on DefaultSession JSON tags are missing... and also their tags are wrong (double maps instead of a map).

To avoid this the workaround I made is to duplicate properly structs:

package hook

import (
	"time"

	"github.com/go-openapi/strfmt"
)

type IDTokenClaims struct {
	Acr      *string    `json:"acr,omitempty"`
	Amr      []string   `json:"amr,omitempty"`
	AtHash   *string    `json:"at_hash,omitempty"`
	Aud      []string   `json:"aud,omitempty"`
	AuthTime *time.Time `json:"auth_time,omitempty"`
	CHash    *string    `json:"c_hash,omitempty"`
	Exp      *time.Time `json:"exp,omitempty"`
	// Ext      map[string]map[string]interface{} `json:"ext,omitempty"`
	Ext   map[string]interface{} `json:"ext,omitempty"`
	Iat   *time.Time             `json:"iat,omitempty"`
	Iss   *string                `json:"iss,omitempty"`
	Jti   *string                `json:"jti,omitempty"`
	Nonce *string                `json:"nonce,omitempty"`
	Rat   *time.Time             `json:"rat,omitempty"`
	Sub   *string                `json:"sub,omitempty"`
}

type Headers struct {
	// Extra map[string]map[string]interface{} `json:"extra,omitempty"`
	Extra map[string]interface{} `json:"extra,omitempty"`
}

type DefaultSession struct {
	ExpiresAt     map[string]strfmt.DateTime `json:"expires_at,omitempty"`
	Headers       *Headers                   `json:"headers,omitempty"`
	IDTokenClaims *IDTokenClaims             `json:"id_token_claims,omitempty"`
	Subject       string                     `json:"subject,omitempty"`
	Username      string                     `json:"username,omitempty"`
}

type Session struct {
	*DefaultSession       `json:"id_token"`
	Extra                 map[string]interface{} `json:"extra"`
	KID                   string                 `json:"kid"`
	ClientID              string                 `json:"client_id"`
	ConsentChallenge      string                 `json:"consent_challenge"`
	ExcludeNotBeforeClaim bool                   `json:"exclude_not_before_claim"`
	AllowedTopLevelClaims []string               `json:"allowed_top_level_claims"`
}

type Requester struct {
	ClientID        string   `json:"client_id"`
	GrantedScopes   []string `json:"granted_scopes"`
	GrantedAudience []string `json:"granted_audience"`
	GrantTypes      []string `json:"grant_types"`
}

type RefreshTokenHookRequest struct {
	Subject         string    `json:"subject"`
	Session         *Session  `json:"session"`
	Requester       Requester `json:"requester"`
	ClientID        string    `json:"client_id"`
	GrantedScopes   []string  `json:"granted_scopes"`
	GrantedAudience []string  `json:"granted_audience"`
}

In my hook I just added:

	// WORKAROUND: include manually `sid` because it's not passed otherwise
	// Ref: https://github.com/ory/hydra/issues/3082#issuecomment-1190556162
	if hookReq.Session != nil {
		previousClaims := hookReq.Session.IDTokenClaims

		if sid, ok := previousClaims.Ext["sid"]; ok {
			claims["sid"] = sid
		}
	}

	hookResp := hydra_oauth2.RefreshTokenHookResponse{
		Session: hydra_consent.ConsentRequestSessionData{
			IDToken: claims,
		},
	}

Tested on my side and everything works then!

[aeneasr]

Thank you for confirming the fix! Closing this :)