zitadel / oidc

Easy to use OpenID Connect client and server library written for Go and certified by the OpenID Foundation
https://zitadel.com
Apache License 2.0
1.41k stars 148 forks source link

The authentication response should also be able to return session_state. #670

Open nannany opened 1 month ago

nannany commented 1 month ago

Preflight Checklist

Describe your problem

The authentication response in the authorization code grant is controlled in the following section. If we incorporate the OIDC session management specifications, it will be necessary to include session_state in the authentication response. I want to enable handling of session_state.


https://github.com/zitadel/oidc/blob/main/pkg/op/auth_request.go#L482-L488

    codeResponse := struct {
        Code  string `schema:"code"`
        State string `schema:"state,omitempty"`
    }{
        Code:  code,
        State: authReq.GetState(),
    }

Describe your ideal solution

To allow session_state to be passed in the authentication request, I will modify the codeResponse interface to accept session information as an argument.

Version

No response

Environment

Self-hosted

Additional Context

No response

muhlemmer commented 3 weeks ago

We have nothing against adding it to the response object. Just curious, how do you plan to get / store the session state in the fitst place?

nannany commented 3 weeks ago

I understand the question as asking how session_state is initially stored and subsequently retrieved by the OpenID Provider.

Regarding storage, since session_state is a value associated with the session established between the OpenID Provider and the browser, I assume it would be created in the database or memory when the session is created. This likely depends on the specific implementation, so it may not necessarily be related to zitadel/oidc.

For retrieving session_state in AuthResponseCode, I believe it would be effective to create an interface within Storage or a similar structure specifically for obtaining session_state.

muhlemmer commented 3 weeks ago

Yeah, that sounds reasonable. Just as long as you are aware you need to touch the Storage or AuthRequest interface or probably need an optional interface to prevent breaking changes.