fix(oidc): store requested response_mode (#8145)

# Which Problems Are Solved

Zitadel never stored or returned the requested `response_mode` in oidc
Auth Requests. This caused the oidc library to fallback to the default
based on the response_type.

# How the Problems Are Solved

- Store the `response_mode` in the Auth request repo
- Store the `response_mode` in the Auth request v2 events
- Return the `resonse_mode` from the Auth Request v1 and v2
`ResponseMode()` methods. (Was hard-coded to an empty string)

# Additional Changes

- Populate the `response_modes_supported` to the oidc Discovery
Configuration. When it was empty, the standard specifies the default of
`query` and `fragment`. However, our oidc library also supports
`form_post` and by this fix, zitadel now also supports this.

# Additional Context

- Closes #6586
- Reported
https://discord.com/channels/927474939156643850/1151508313717084220

---------

Co-authored-by: Livio Spring <livio.a@gmail.com>
This commit is contained in:
Tim Möhlmann
2024-06-17 12:50:12 +03:00
committed by GitHub
parent 85d7536d44
commit 1aa8c49e41
15 changed files with 287 additions and 20 deletions

View File

@@ -75,6 +75,7 @@ func (o *OPStorage) createAuthRequestLoginClient(ctx context.Context, req *oidc.
Audience: audience,
NeedRefreshToken: slices.Contains(scope, oidc.ScopeOfflineAccess),
ResponseType: ResponseTypeToBusiness(req.ResponseType),
ResponseMode: ResponseModeToBusiness(req.ResponseMode),
CodeChallenge: CodeChallengeToBusiness(req.CodeChallenge, req.CodeChallengeMethod),
Prompt: PromptToBusiness(req.Prompt),
UILocales: UILocalesToBusiness(req.UILocales),

View File

@@ -6,6 +6,7 @@ import (
"strings"
"time"
"github.com/zitadel/logging"
"github.com/zitadel/oidc/v3/pkg/oidc"
"github.com/zitadel/oidc/v3/pkg/op"
"golang.org/x/text/language"
@@ -75,7 +76,7 @@ func (a *AuthRequest) GetResponseType() oidc.ResponseType {
}
func (a *AuthRequest) GetResponseMode() oidc.ResponseMode {
return ""
return ResponseModeToOIDC(a.oidc().ResponseMode)
}
func (a *AuthRequest) GetScopes() []string {
@@ -121,6 +122,7 @@ func CreateAuthRequestToBusiness(ctx context.Context, authReq *oidc.AuthRequest,
Request: &domain.AuthRequestOIDC{
Scopes: authReq.Scopes,
ResponseType: ResponseTypeToBusiness(authReq.ResponseType),
ResponseMode: ResponseModeToBusiness(authReq.ResponseMode),
Nonce: authReq.Nonce,
CodeChallenge: CodeChallengeToBusiness(authReq.CodeChallenge, authReq.CodeChallengeMethod),
},
@@ -232,6 +234,27 @@ func ResponseTypeToOIDC(responseType domain.OIDCResponseType) oidc.ResponseType
}
}
// ResponseModeToBusiness returns the OIDCResponseMode enum value from the domain package.
// An empty or invalid value defaults to unspecified.
func ResponseModeToBusiness(responseMode oidc.ResponseMode) domain.OIDCResponseMode {
if responseMode == "" {
return domain.OIDCResponseModeUnspecified
}
out, err := domain.OIDCResponseModeString(string(responseMode))
logging.OnError(err).Debugln("invalid oidc response_mode, using default")
return out
}
// ResponseModeToOIDC return the oidc string representation of the enum value from the domain package.
// When responseMode is `0 - unspecified`, an empty string is returned.
// This allows the oidc package to pick the appropriate response mode based on the response type.
func ResponseModeToOIDC(responseMode domain.OIDCResponseMode) oidc.ResponseMode {
if responseMode == domain.OIDCResponseModeUnspecified || !responseMode.IsAOIDCResponseMode() {
return ""
}
return oidc.ResponseMode(responseMode.String())
}
func CodeChallengeToBusiness(challenge string, method oidc.CodeChallengeMethod) *domain.OIDCCodeChallenge {
if challenge == "" {
return nil

View File

@@ -0,0 +1,96 @@
package oidc
import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/zitadel/oidc/v3/pkg/oidc"
"github.com/zitadel/zitadel/internal/domain"
)
func TestResponseModeToBusiness(t *testing.T) {
type args struct {
responseMode oidc.ResponseMode
}
tests := []struct {
name string
args args
want domain.OIDCResponseMode
}{
{
name: "empty",
args: args{""},
want: domain.OIDCResponseModeUnspecified,
},
{
name: "invalid",
args: args{"foo"},
want: domain.OIDCResponseModeUnspecified,
},
{
name: "query",
args: args{oidc.ResponseModeQuery},
want: domain.OIDCResponseModeQuery,
},
{
name: "fragment",
args: args{oidc.ResponseModeFragment},
want: domain.OIDCResponseModeFragment,
},
{
name: "post_form",
args: args{oidc.ResponseModeFormPost},
want: domain.OIDCResponseModeFormPost,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := ResponseModeToBusiness(tt.args.responseMode)
assert.Equal(t, tt.want, got)
})
}
}
func TestResponseModeToOIDC(t *testing.T) {
type args struct {
responseMode domain.OIDCResponseMode
}
tests := []struct {
name string
args args
want oidc.ResponseMode
}{
{
name: "unspecified",
args: args{domain.OIDCResponseModeUnspecified},
want: "",
},
{
name: "invalid",
args: args{99},
want: "",
},
{
name: "query",
args: args{domain.OIDCResponseModeQuery},
want: oidc.ResponseModeQuery,
},
{
name: "fragment",
args: args{domain.OIDCResponseModeFragment},
want: oidc.ResponseModeFragment,
},
{
name: "form_post",
args: args{domain.OIDCResponseModeFormPost},
want: oidc.ResponseModeFormPost,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := ResponseModeToOIDC(tt.args.responseMode)
assert.Equal(t, tt.want, got)
})
}
}

View File

@@ -53,7 +53,7 @@ func (a *AuthRequestV2) GetResponseType() oidc.ResponseType {
}
func (a *AuthRequestV2) GetResponseMode() oidc.ResponseMode {
return ""
return ResponseModeToOIDC(a.ResponseMode)
}
func (a *AuthRequestV2) GetScopes() []string {

View File

@@ -173,23 +173,28 @@ func (s *Server) EndSession(ctx context.Context, r *op.Request[oidc.EndSessionRe
func (s *Server) createDiscoveryConfig(ctx context.Context, supportedUILocales oidc.Locales) *oidc.DiscoveryConfiguration {
issuer := op.IssuerFromContext(ctx)
return &oidc.DiscoveryConfiguration{
Issuer: issuer,
AuthorizationEndpoint: s.Endpoints().Authorization.Absolute(issuer),
TokenEndpoint: s.Endpoints().Token.Absolute(issuer),
IntrospectionEndpoint: s.Endpoints().Introspection.Absolute(issuer),
UserinfoEndpoint: s.Endpoints().Userinfo.Absolute(issuer),
RevocationEndpoint: s.Endpoints().Revocation.Absolute(issuer),
EndSessionEndpoint: s.Endpoints().EndSession.Absolute(issuer),
JwksURI: s.Endpoints().JwksURI.Absolute(issuer),
DeviceAuthorizationEndpoint: s.Endpoints().DeviceAuthorization.Absolute(issuer),
ScopesSupported: op.Scopes(s.Provider()),
ResponseTypesSupported: op.ResponseTypes(s.Provider()),
GrantTypesSupported: op.GrantTypes(s.Provider()),
SubjectTypesSupported: op.SubjectTypes(s.Provider()),
IDTokenSigningAlgValuesSupported: []string{s.signingKeyAlgorithm},
RequestObjectSigningAlgValuesSupported: op.RequestObjectSigAlgorithms(s.Provider()),
TokenEndpointAuthMethodsSupported: op.AuthMethodsTokenEndpoint(s.Provider()),
TokenEndpointAuthSigningAlgValuesSupported: op.TokenSigAlgorithms(s.Provider()),
Issuer: issuer,
AuthorizationEndpoint: s.Endpoints().Authorization.Absolute(issuer),
TokenEndpoint: s.Endpoints().Token.Absolute(issuer),
IntrospectionEndpoint: s.Endpoints().Introspection.Absolute(issuer),
UserinfoEndpoint: s.Endpoints().Userinfo.Absolute(issuer),
RevocationEndpoint: s.Endpoints().Revocation.Absolute(issuer),
EndSessionEndpoint: s.Endpoints().EndSession.Absolute(issuer),
JwksURI: s.Endpoints().JwksURI.Absolute(issuer),
DeviceAuthorizationEndpoint: s.Endpoints().DeviceAuthorization.Absolute(issuer),
ScopesSupported: op.Scopes(s.Provider()),
ResponseTypesSupported: op.ResponseTypes(s.Provider()),
ResponseModesSupported: []string{
string(oidc.ResponseModeQuery),
string(oidc.ResponseModeFragment),
string(oidc.ResponseModeFormPost),
},
GrantTypesSupported: op.GrantTypes(s.Provider()),
SubjectTypesSupported: op.SubjectTypes(s.Provider()),
IDTokenSigningAlgValuesSupported: []string{s.signingKeyAlgorithm},
RequestObjectSigningAlgValuesSupported: op.RequestObjectSigAlgorithms(s.Provider()),
TokenEndpointAuthMethodsSupported: op.AuthMethodsTokenEndpoint(s.Provider()),
TokenEndpointAuthSigningAlgValuesSupported: op.TokenSigAlgorithms(s.Provider()),
IntrospectionEndpointAuthSigningAlgValuesSupported: op.IntrospectionSigAlgorithms(s.Provider()),
IntrospectionEndpointAuthMethodsSupported: op.AuthMethodsIntrospectionEndpoint(s.Provider()),
RevocationEndpointAuthSigningAlgValuesSupported: op.RevocationSigAlgorithms(s.Provider()),

View File

@@ -73,7 +73,7 @@ func TestServer_createDiscoveryConfig(t *testing.T) {
RegistrationEndpoint: "",
ScopesSupported: []string{oidc.ScopeOpenID, oidc.ScopeProfile, oidc.ScopeEmail, oidc.ScopePhone, oidc.ScopeAddress, oidc.ScopeOfflineAccess},
ResponseTypesSupported: []string{string(oidc.ResponseTypeCode), string(oidc.ResponseTypeIDTokenOnly), string(oidc.ResponseTypeIDToken)},
ResponseModesSupported: nil,
ResponseModesSupported: []string{string(oidc.ResponseModeQuery), string(oidc.ResponseModeFragment), string(oidc.ResponseModeFormPost)},
GrantTypesSupported: []oidc.GrantType{oidc.GrantTypeCode, oidc.GrantTypeImplicit, oidc.GrantTypeRefreshToken, oidc.GrantTypeBearer},
ACRValuesSupported: nil,
SubjectTypesSupported: []string{"public"},