diff --git a/internal/handlers/handler_oidc_authorization.go b/internal/handlers/handler_oidc_authorization.go index 29147a0de..5cb193920 100644 --- a/internal/handlers/handler_oidc_authorization.go +++ b/internal/handlers/handler_oidc_authorization.go @@ -63,6 +63,28 @@ func OpenIDConnectAuthorization(ctx *middlewares.AutheliaCtx, rw http.ResponseWr return } + if !oidc.IsPushedAuthorizedRequest(requester, ctx.Providers.OpenIDConnect.GetPushedAuthorizeRequestURIPrefix(ctx)) { + if err = client.ValidatePKCEPolicy(requester); err != nil { + rfc := fosite.ErrorToRFC6749Error(err) + + ctx.Logger.Errorf("Authorization Request with id '%s' on client with id '%s' failed to validate the PKCE policy: %s", requester.GetID(), client.GetID(), rfc.WithExposeDebug(true).GetDescription()) + + ctx.Providers.OpenIDConnect.WriteAuthorizeError(ctx, rw, requester, err) + + return + } + + if err = client.ValidateResponseModePolicy(requester); err != nil { + rfc := fosite.ErrorToRFC6749Error(err) + + ctx.Logger.Errorf("Authorization Request with id '%s' on client with id '%s' failed to validate the Response Mode: %s", requester.GetID(), client.GetID(), rfc.WithExposeDebug(true).GetDescription()) + + ctx.Providers.OpenIDConnect.WriteAuthorizeError(ctx, rw, requester, err) + + return + } + } + if err = client.ValidatePKCEPolicy(requester); err != nil { rfc := fosite.ErrorToRFC6749Error(err) @@ -175,9 +197,19 @@ func OpenIDConnectPushedAuthorizationRequest(ctx *middlewares.AutheliaCtx, rw ht if err = client.ValidatePKCEPolicy(requester); err != nil { rfc := fosite.ErrorToRFC6749Error(err) - ctx.Logger.Errorf("Pushed Authorization Request with id '%s' on client with id '%s' failed to validate the PKCE policy: %s", requester.GetID(), clientID, rfc.WithExposeDebug(true).GetDescription()) + ctx.Logger.Errorf("Pushed Authorization Request with id '%s' on client with id '%s' failed to validate the PKCE policy: %s", requester.GetID(), client.GetID(), rfc.WithExposeDebug(true).GetDescription()) - ctx.Providers.OpenIDConnect.WritePushedAuthorizeError(ctx, rw, requester, err) + ctx.Providers.OpenIDConnect.WriteAuthorizeError(ctx, rw, requester, err) + + return + } + + if err = client.ValidateResponseModePolicy(requester); err != nil { + rfc := fosite.ErrorToRFC6749Error(err) + + ctx.Logger.Errorf("Pushed Authorization Request with id '%s' on client with id '%s' failed to validate the Response Mode: %s", requester.GetID(), client.GetID(), rfc.WithExposeDebug(true).GetDescription()) + + ctx.Providers.OpenIDConnect.WriteAuthorizeError(ctx, rw, requester, err) return } diff --git a/internal/oidc/client.go b/internal/oidc/client.go index 8388db9e7..41d302d03 100644 --- a/internal/oidc/client.go +++ b/internal/oidc/client.go @@ -1,8 +1,6 @@ package oidc import ( - "strings" - "github.com/ory/fosite" "github.com/ory/x/errorsx" @@ -30,7 +28,7 @@ func NewClient(config schema.OpenIDConnectClientConfiguration) (client *Client) RedirectURIs: config.RedirectURIs, GrantTypes: config.GrantTypes, ResponseTypes: config.ResponseTypes, - ResponseModes: []fosite.ResponseModeType{fosite.ResponseModeDefault}, + ResponseModes: []fosite.ResponseModeType{}, EnforcePAR: config.EnforcePAR, @@ -73,21 +71,44 @@ func (c *Client) ValidatePKCEPolicy(r fosite.Requester) (err error) { // ValidatePARPolicy is a helper function to validate additional policy constraints on a per-client basis. func (c *Client) ValidatePARPolicy(r fosite.Requester, prefix string) (err error) { - form := r.GetRequestForm() - if c.EnforcePAR { - if requestURI := form.Get(FormParameterRequestURI); !strings.HasPrefix(requestURI, prefix) { - if requestURI == "" { + if !IsPushedAuthorizedRequest(r, prefix) { + switch requestURI := r.GetRequestForm().Get(FormParameterRequestURI); requestURI { + case "": return errorsx.WithStack(ErrPAREnforcedClientMissingPAR.WithDebug("The request_uri parameter was empty.")) + default: + return errorsx.WithStack(ErrPAREnforcedClientMissingPAR.WithDebugf("The request_uri parameter '%s' is malformed.", requestURI)) } - - return errorsx.WithStack(ErrPAREnforcedClientMissingPAR.WithDebugf("The request_uri parameter '%s' is malformed.", requestURI)) } } return nil } +// ValidateResponseModePolicy is an additional check to the response mode parameter to ensure if it's omitted that the +// default response mode for the fosite.AuthorizeRequester is permitted. +func (c *Client) ValidateResponseModePolicy(r fosite.AuthorizeRequester) (err error) { + if r.GetResponseMode() != fosite.ResponseModeDefault { + return nil + } + + m := r.GetDefaultResponseMode() + + modes := c.GetResponseModes() + + if len(modes) == 0 { + return nil + } + + for _, mode := range modes { + if m == mode { + return nil + } + } + + return errorsx.WithStack(fosite.ErrUnsupportedResponseMode.WithHintf(`The request omitted the response_mode making the default response_mode "%s" based on the other authorization request parameters but registered OAuth 2.0 client doesn't support this response_mode`, m)) +} + // IsAuthenticationLevelSufficient returns if the provided authentication.Level is sufficient for the client of the AutheliaClient. func (c *Client) IsAuthenticationLevelSufficient(level authentication.Level) bool { if level == authentication.NotAuthenticated { diff --git a/internal/oidc/client_test.go b/internal/oidc/client_test.go index 747f1e8c2..1546644c2 100644 --- a/internal/oidc/client_test.go +++ b/internal/oidc/client_test.go @@ -1,6 +1,7 @@ package oidc import ( + "fmt" "testing" "github.com/ory/fosite" @@ -19,8 +20,7 @@ func TestNewClient(t *testing.T) { assert.Equal(t, "", blankClient.ID) assert.Equal(t, "", blankClient.Description) assert.Equal(t, "", blankClient.Description) - require.Len(t, blankClient.ResponseModes, 1) - assert.Equal(t, fosite.ResponseModeDefault, blankClient.ResponseModes[0]) + assert.Len(t, blankClient.ResponseModes, 0) exampleConfig := schema.OpenIDConnectClientConfiguration{ ID: "myapp", @@ -36,11 +36,10 @@ func TestNewClient(t *testing.T) { exampleClient := NewClient(exampleConfig) assert.Equal(t, "myapp", exampleClient.ID) - require.Len(t, exampleClient.ResponseModes, 4) - assert.Equal(t, fosite.ResponseModeDefault, exampleClient.ResponseModes[0]) - assert.Equal(t, fosite.ResponseModeFormPost, exampleClient.ResponseModes[1]) - assert.Equal(t, fosite.ResponseModeQuery, exampleClient.ResponseModes[2]) - assert.Equal(t, fosite.ResponseModeFragment, exampleClient.ResponseModes[3]) + require.Len(t, exampleClient.ResponseModes, 3) + assert.Equal(t, fosite.ResponseModeFormPost, exampleClient.ResponseModes[0]) + assert.Equal(t, fosite.ResponseModeQuery, exampleClient.ResponseModes[1]) + assert.Equal(t, fosite.ResponseModeFragment, exampleClient.ResponseModes[2]) assert.Equal(t, authorization.TwoFactor, exampleClient.Policy) } @@ -226,6 +225,7 @@ func TestNewClientPKCE(t *testing.T) { expected string r *fosite.Request err string + desc string }{ { "ShouldNotEnforcePKCEAndNotErrorOnNonPKCERequest", @@ -235,6 +235,7 @@ func TestNewClientPKCE(t *testing.T) { "", &fosite.Request{}, "", + "", }, { "ShouldEnforcePKCEAndErrorOnNonPKCERequest", @@ -244,6 +245,7 @@ func TestNewClientPKCE(t *testing.T) { "", &fosite.Request{}, "invalid_request", + "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed. Clients must include a code_challenge when performing the authorize code flow, but it is missing. The server is configured in a way that enforces PKCE for this client.", }, { "ShouldEnforcePKCEAndNotErrorOnPKCERequest", @@ -253,6 +255,7 @@ func TestNewClientPKCE(t *testing.T) { "", &fosite.Request{Form: map[string][]string{"code_challenge": {"abc"}}}, "", + "", }, {"ShouldEnforcePKCEFromChallengeMethodAndErrorOnNonPKCERequest", schema.OpenIDConnectClientConfiguration{PKCEChallengeMethod: "S256"}, @@ -261,6 +264,7 @@ func TestNewClientPKCE(t *testing.T) { "S256", &fosite.Request{}, "invalid_request", + "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed. Clients must include a code_challenge when performing the authorize code flow, but it is missing. The server is configured in a way that enforces PKCE for this client.", }, {"ShouldEnforcePKCEFromChallengeMethodAndErrorOnInvalidChallengeMethod", schema.OpenIDConnectClientConfiguration{PKCEChallengeMethod: "S256"}, @@ -269,6 +273,7 @@ func TestNewClientPKCE(t *testing.T) { "S256", &fosite.Request{Form: map[string][]string{"code_challenge": {"abc"}}}, "invalid_request", + "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed. Client must use code_challenge_method=S256, is not allowed. The server is configured in a way that enforces PKCE S256 as challenge method for this client.", }, {"ShouldEnforcePKCEFromChallengeMethodAndNotErrorOnValidRequest", schema.OpenIDConnectClientConfiguration{PKCEChallengeMethod: "S256"}, @@ -277,6 +282,7 @@ func TestNewClientPKCE(t *testing.T) { "S256", &fosite.Request{Form: map[string][]string{"code_challenge": {"abc"}, "code_challenge_method": {"S256"}}}, "", + "", }, } @@ -292,7 +298,136 @@ func TestNewClientPKCE(t *testing.T) { err := client.ValidatePKCEPolicy(tc.r) if tc.err != "" { + require.NotNil(t, err) assert.EqualError(t, err, tc.err) + assert.Equal(t, tc.desc, fosite.ErrorToRFC6749Error(err).WithExposeDebug(true).GetDescription()) + } else { + assert.NoError(t, err) + } + } + }) + } +} + +func TestNewClientPAR(t *testing.T) { + testCases := []struct { + name string + have schema.OpenIDConnectClientConfiguration + expected bool + r *fosite.Request + err string + desc string + }{ + { + "ShouldNotEnforcEPARAndNotErrorOnNonPARRequest", + schema.OpenIDConnectClientConfiguration{}, + false, + &fosite.Request{}, + "", + "", + }, + { + "ShouldEnforcePARAndErrorOnNonPARRequest", + schema.OpenIDConnectClientConfiguration{EnforcePAR: true}, + true, + &fosite.Request{}, + "invalid_request", + "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed. Pushed Authorization Requests are enforced for this client but no such request was sent. The request_uri parameter was empty.", + }, + { + "ShouldEnforcePARAndErrorOnNonPARRequest", + schema.OpenIDConnectClientConfiguration{EnforcePAR: true}, + true, + &fosite.Request{Form: map[string][]string{FormParameterRequestURI: {"https://example.com"}}}, + "invalid_request", + "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed. Pushed Authorization Requests are enforced for this client but no such request was sent. The request_uri parameter 'https://example.com' is malformed."}, + { + "ShouldEnforcePARAndNotErrorOnPARRequest", + schema.OpenIDConnectClientConfiguration{EnforcePAR: true}, + true, + &fosite.Request{Form: map[string][]string{FormParameterRequestURI: {fmt.Sprintf("%sabc", urnPARPrefix)}}}, + "", + "", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + client := NewClient(tc.have) + + assert.Equal(t, tc.expected, client.EnforcePAR) + + if tc.r != nil { + err := client.ValidatePARPolicy(tc.r, urnPARPrefix) + + if tc.err != "" { + require.NotNil(t, err) + assert.EqualError(t, err, tc.err) + assert.Equal(t, tc.desc, fosite.ErrorToRFC6749Error(err).WithExposeDebug(true).GetDescription()) + } else { + assert.NoError(t, err) + } + } + }) + } +} + +func TestNewClientResponseModes(t *testing.T) { + testCases := []struct { + name string + have schema.OpenIDConnectClientConfiguration + expected []fosite.ResponseModeType + r *fosite.AuthorizeRequest + err string + desc string + }{ + { + "ShouldEnforceResponseModePolicyAndAllowDefaultModeQuery", + schema.OpenIDConnectClientConfiguration{ResponseModes: []string{ResponseModeQuery}}, + []fosite.ResponseModeType{fosite.ResponseModeQuery}, + &fosite.AuthorizeRequest{DefaultResponseMode: fosite.ResponseModeQuery, ResponseMode: fosite.ResponseModeDefault, Request: fosite.Request{Form: map[string][]string{FormParameterResponseMode: nil}}}, + "", + "", + }, + { + "ShouldEnforceResponseModePolicyAndFailOnDefaultMode", + schema.OpenIDConnectClientConfiguration{ResponseModes: []string{ResponseModeFormPost}}, + []fosite.ResponseModeType{fosite.ResponseModeFormPost}, + &fosite.AuthorizeRequest{DefaultResponseMode: fosite.ResponseModeQuery, ResponseMode: fosite.ResponseModeDefault, Request: fosite.Request{Form: map[string][]string{FormParameterResponseMode: nil}}}, + "unsupported_response_mode", + "The authorization server does not support obtaining a response using this response mode. The request omitted the response_mode making the default response_mode 'query' based on the other authorization request parameters but registered OAuth 2.0 client doesn't support this response_mode", + }, + { + "ShouldNotEnforceConfiguredResponseMode", + schema.OpenIDConnectClientConfiguration{ResponseModes: []string{ResponseModeFormPost}}, + []fosite.ResponseModeType{fosite.ResponseModeFormPost}, + &fosite.AuthorizeRequest{DefaultResponseMode: fosite.ResponseModeQuery, ResponseMode: fosite.ResponseModeQuery, Request: fosite.Request{Form: map[string][]string{FormParameterResponseMode: {ResponseModeQuery}}}}, + "", + "", + }, + { + "ShouldNotEnforceUnconfiguredResponseMode", + schema.OpenIDConnectClientConfiguration{ResponseModes: []string{}}, + []fosite.ResponseModeType{}, + &fosite.AuthorizeRequest{DefaultResponseMode: fosite.ResponseModeQuery, ResponseMode: fosite.ResponseModeDefault, Request: fosite.Request{Form: map[string][]string{FormParameterResponseMode: {ResponseModeQuery}}}}, + "", + "", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + client := NewClient(tc.have) + + assert.Equal(t, tc.expected, client.GetResponseModes()) + + if tc.r != nil { + err := client.ValidateResponseModePolicy(tc.r) + + if tc.err != "" { + require.NotNil(t, err) + assert.EqualError(t, err, tc.err) + assert.Equal(t, tc.desc, fosite.ErrorToRFC6749Error(err).WithExposeDebug(true).GetDescription()) } else { assert.NoError(t, err) } diff --git a/internal/oidc/const.go b/internal/oidc/const.go index 1629ef7c7..db8c3a23d 100644 --- a/internal/oidc/const.go +++ b/internal/oidc/const.go @@ -112,6 +112,7 @@ const ( const ( FormParameterRequestURI = "request_uri" + FormParameterResponseMode = "response_mode" FormParameterCodeChallenge = "code_challenge" FormParameterCodeChallengeMethod = "code_challenge_method" ) diff --git a/internal/oidc/util.go b/internal/oidc/util.go new file mode 100644 index 000000000..2e7c14d37 --- /dev/null +++ b/internal/oidc/util.go @@ -0,0 +1,12 @@ +package oidc + +import ( + "strings" + + "github.com/ory/fosite" +) + +// IsPushedAuthorizedRequest returns true if the requester has a PushedAuthorizationRequest redirect_uri value. +func IsPushedAuthorizedRequest(r fosite.Requester, prefix string) bool { + return strings.HasPrefix(r.GetRequestForm().Get(FormParameterRequestURI), prefix) +} diff --git a/internal/templates/src/oidc/AuthorizeResponseFormPost.html b/internal/templates/src/oidc/AuthorizeResponseFormPost.html index 93569b59b..32c6911a6 100644 --- a/internal/templates/src/oidc/AuthorizeResponseFormPost.html +++ b/internal/templates/src/oidc/AuthorizeResponseFormPost.html @@ -10,10 +10,10 @@
- {{ range $key,$value := .Parameters }} - {{ range $parameter:= $value}} - - {{end}} + {{ range $key, $value := .Parameters }} + {{ range $parameter := $value }} + + {{ end }} {{ end }}