ory / hydra

The most scalable and customizable OpenID Certified™ OpenID Connect and OAuth Provider on the market. Become an OpenID Connect and OAuth2 Provider over night. Broad support for related RFCs. Written in Go, cloud native, headless, API-first. Available as a service on Ory Network and for self-hosters.
https://www.ory.sh/?utm_source=github&utm_medium=banner&utm_campaign=hydra
Apache License 2.0
15.56k stars 1.49k forks source link

External SessionID for integrators #3203

Closed sgal closed 2 years ago

sgal commented 2 years ago

Preflight checklist

Describe your problem

In our system, we have a concept of a security zone or access level that can change over time for the same login session. Login sessions can be as long as 1 year, but access would be floating to reduce the risk of exposing sensitive or regulated data (GDPR, PSD2, etc.)

Example:

In order to achieve this floating access level, we need some sort of session identifier, that would be consistent between token refreshes until the session is ended or expired.

Describe your ideal solution

An ideal solution would be:

Workarounds or alternatives

Without a consistent session identifier, we are not able to create access-level logic. The current set of parameters that are sent in the Refresh Webhook is not enough to uniquely identify a login session. Our customers can have multiple sessions in different browsers, so we need something more specific for the session tracking.

Version

1.x

Additional Context

No response

aeneasr commented 2 years ago

That should generally be possible as we already (kinda) keep track of an internal ID in order to track refresh token chains. It’s not a session ID, but a consent ID, but it can be used for the purpose you describe.

sgal commented 2 years ago

@aeneasr That is great, do you think it would be a big change? Also, it would be great for us to have this in the 1.x version.

aeneasr commented 2 years ago

@aeneasr That is great, do you think it would be a big change?

Since this parameter is internal and not consistent, it is possible without a large data schema change, but also not trivial in terms of consistency across the APIs.

Also, it would be great for us to have this in the 1.x version.

Sorry, those changes are only possible in 2.x data schema! The 1.x schema does not have this tracking id.

sgal commented 2 years ago

Is there an ETA for 2.x release? We have a major product rollout blocked by this, so it would be great to know the timeline.

aeneasr commented 2 years ago

We're done with v2.x but blocked by https://github.com/ory/hydra/issues/3157 as those changes are not yet ready and then need to be backported to v2.x (as discussed). Once that is done, the first version of v2.x will be released (most likely a pre-release to iron out any data migration issues, but stable in terms of operations).

aeneasr commented 2 years ago

What you need might already be contained in the ID Token, there we have (iirc) a session identifier. It would also be the correct token type to use for authentication, as opposed to the access token, which is not a session substitute and highly discouraged to use it as a substitute. Would that work?

aeneasr commented 2 years ago

Alternatively you could add your own session ID to the access token or id token extra field during the consent step :)

sgal commented 2 years ago

Would it be possible to receive that extra field in the refresh webhook? If so, that would definitely be easier for us to implement.

sgal commented 2 years ago

Which token to use is not really an issue for us here - it is more about how to update a correct session on refresh.

sgal commented 2 years ago

If I understand it correctly, we can easily add ID token claims to the refresh webhook, by modifying this part https://github.com/ory/hydra/blob/master/oauth2/hook.go#L62 Something like

reqBody := RefreshTokenHookRequest{
    Subject:         session.GetSubject(),
    SessionID:       session.IDTokenClaims().JTI,
        ...
}

Or even

reqBody := RefreshTokenHookRequest{
    Subject:         session.GetSubject(),
    Extra:           session.IDTokenClaims().Extra,
        ...
}
aeneasr commented 2 years ago

Yes exactly, adding the extra key would probably be the solution

sgal commented 2 years ago

The most important question - can you accept this change in 1.x branch?

aeneasr commented 2 years ago

v1.x is officially feature frozen (with the exception of customizable TTL) for several weeks and we do not have capacity to introduce new changes as it further delays the timelines on bringing Ory Hydra to the cloud and v2.x out.

If the change as scoped out above is enough to make this work on your end, the feature in its scope is „go live“ critical, and there is no alternate solution available, it would be best to clear up the priorities with Stephan to see how we can address this :)

Timing wise this would land after TTL is finished regardless of v1/v2

sgal commented 2 years ago

This change seems easy enough for me to implement, would that help with the priority?