ory / fosite

Extensible security first OAuth 2.0 and OpenID Connect SDK for Go.
https://www.ory.sh/?utm_source=github&utm_medium=banner&utm_campaign=fosite
Apache License 2.0
2.28k stars 356 forks source link

id_token_hint should not persist to storage #798

Open mitar opened 4 months ago

mitar commented 4 months ago

Preflight checklist

Ory Network Project

No response

Describe the bug

In this discussion I realized that id_token_hint is persisted to the storage in Fosite and it probably should not be. I think subject from the id_token_hint should be extracted early and only subject should be stored in the session, not the whole id_token_hint.

Reproducing the bug

Looking at the code here: https://github.com/ory/fosite/blob/master/handler/openid/flow_explicit_auth.go#L29-L35

Relevant log output

No response

Relevant configuration

No response

Version

latest master

On which operating system are you observing this issue?

None

In which environment are you deploying?

None

Additional Context

No response

james-d-elliott commented 4 months ago

Would probably need to retain the aud and sid claims as well for OpenID Connect RP-Initiated Logout 1.0, there are very likely other specifications that use other values too. I was also under the impression that the OpenID Sessions were deleted upon completion of the flow (maybe it's only for successful ones).

Maybe the best option here would be to store solely the claims as some form of child claims like id_token_hint_claims? Since it has no signature it has no intrinsic security value, only a privacy value.

mitar commented 4 months ago

I was also under the impression that the OpenID Sessions were deleted upon completion of the flow (maybe it's only for successful ones).

Oh, this has been changed just recently: https://github.com/ory/fosite/commit/c0b30f65bca686b3583384c611278fa8079f022a

But I do not see where DeleteAccessTokenSession would be called?