Auth Req omitted response_mode does not validate the default response_mode against the ResponseModeClient
james-d-elliott opened this issue · 0 comments
james-d-elliott commented
Preflight checklist
- I could not find a solution in the existing issues, docs, nor discussions.
- I agree to follow this project's Code of Conduct.
- I have read and am following this repository's Contribution Guidelines.
- This issue affects my Ory Network project.
- I have joined the Ory Community Slack.
- I am signed up to the Ory Security Patch Newsletter.
Describe the bug
It appears there is no check of the permitted response modes for a client if the response mode itself is omitted. I don't think this is intended. Theoretically if a client is configured to only allow the form_post response mode, and the response_mode parameter is omitted, this authorization request should always fail. However maybe I'm missing something?
Reproducing the bug
Add the following case to the TestNewAuthorizeRequest test:
/* determine correct response mode if default */
{
desc: "failure with default response mode",
conf: &Fosite{Store: store, Config: &Config{ScopeStrategy: ExactScopeStrategy, AudienceMatchingStrategy: DefaultAudienceMatchingStrategy}},
query: url.Values{
"redirect_uri": {"https://foo.bar/cb"},
"client_id": {"1234"},
"response_type": {"code"},
"state": {"strong-state"},
"scope": {"foo bar"},
"audience": {"https://cloud.ory.sh/api https://www.ory.sh/api"},
},
mock: func() {
store.EXPECT().GetClient(gomock.Any(), "1234").Return(&DefaultResponseModeClient{
DefaultClient: &DefaultClient{
RedirectURIs: []string{"https://foo.bar/cb"},
Scopes: []string{"foo", "bar"},
ResponseTypes: []string{"code"},
Audience: []string{"https://cloud.ory.sh/api", "https://www.ory.sh/api"},
},
ResponseModes: []ResponseModeType{ResponseModeFormPost},
}, nil)
},
expectedError: ErrUnsupportedResponseMode,
},Relevant log output
No response
Relevant configuration
It's important to note that the behavior can realistically handled by implementers as well as fixed directly in the library which may be a consideration in deciding on a fix.
Version
0.44.0
On which operating system are you observing this issue?
Linux
In which environment are you deploying?
Other
Additional Context
The following patch should resolve the issue:
Subject: [PATCH] fix: default response mode not checked
---
Index: authorize_request_handler.go
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/authorize_request_handler.go b/authorize_request_handler.go
--- a/authorize_request_handler.go (revision daf3b15f0433c207e40601065cb9c92c52a025f2)
+++ b/authorize_request_handler.go (date 1679955288254)
@@ -235,29 +235,47 @@
return nil
}
+// validateResponseMode MUST be called after validateResponseTypes due to the check on the request.GetResponseTypes.
func (f *Fosite) validateResponseMode(r *http.Request, request *AuthorizeRequest) error {
- if request.ResponseMode == ResponseModeDefault {
- return nil
+ useDefault := request.GetResponseMode() == ResponseModeDefault
+
+ // A fallback handler to set the default response mode in cases where we can not reach the Authorize Handlers
+ // but still need the e.g. correct error response mode.
+ if useDefault {
+ if request.ResponseTypes.ExactOne("code") {
+ request.SetDefaultResponseMode(ResponseModeQuery)
+ } else {
+ // If the response type is not `code` it is an implicit/hybrid (fragment) response mode.
+ request.SetDefaultResponseMode(ResponseModeFragment)
+ }
}
responseModeClient, ok := request.GetClient().(ResponseModeClient)
if !ok {
+ if useDefault {
+ return nil
+ }
+
return errorsx.WithStack(ErrUnsupportedResponseMode.WithHintf("The request has response_mode \"%s\". set but registered OAuth 2.0 client doesn't support response_mode", r.Form.Get("response_mode")))
}
- var found bool
+ responseMode := request.GetResponseMode()
+
+ if useDefault {
+ responseMode = request.GetDefaultResponseMode()
+ }
+
for _, t := range responseModeClient.GetResponseModes() {
if request.ResponseMode == t {
- found = true
- break
- }
- }
-
- if !found {
- return errorsx.WithStack(ErrUnsupportedResponseMode.WithHintf("The client is not allowed to request response_mode '%s'.", r.Form.Get("response_mode")))
- }
-
- return nil
+ return nil
+ }
+ }
+
+ if useDefault {
+ return errorsx.WithStack(ErrUnsupportedResponseMode.WithHintf("The request did not specify a response_mode and the default response_mode '%s' is not allowed on this client.", responseMode))
+ }
+
+ return errorsx.WithStack(ErrUnsupportedResponseMode.WithHintf("The client is not allowed to request response_mode '%s'.", request.ResponseMode))
}
func (f *Fosite) authorizeRequestFromPAR(ctx context.Context, r *http.Request, request *AuthorizeRequest) (bool, error) {
@@ -382,17 +400,6 @@
return request, err
}
- // A fallback handler to set the default response mode in cases where we can not reach the Authorize Handlers
- // but still need the e.g. correct error response mode.
- if request.GetResponseMode() == ResponseModeDefault {
- if request.ResponseTypes.ExactOne("code") {
- request.SetDefaultResponseMode(ResponseModeQuery)
- } else {
- // If the response type is not `code` it is an implicit/hybrid (fragment) response mode.
- request.SetDefaultResponseMode(ResponseModeFragment)
- }
- }
-
// rfc6819 4.4.1.8. Threat: CSRF Attack against redirect-uri
// The "state" parameter should be used to link the authorization
// request with the redirect URI used to deliver the access token (Section 5.3.5).
Index: authorize_request_handler_test.go
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/authorize_request_handler_test.go b/authorize_request_handler_test.go
--- a/authorize_request_handler_test.go (revision daf3b15f0433c207e40601065cb9c92c52a025f2)
+++ b/authorize_request_handler_test.go (date 1679954955533)
@@ -492,6 +492,31 @@
},
/* determine correct response mode if default */
{
+ desc: "failure with default response mode",
+ conf: &Fosite{Store: store, Config: &Config{ScopeStrategy: ExactScopeStrategy, AudienceMatchingStrategy: DefaultAudienceMatchingStrategy}},
+ query: url.Values{
+ "redirect_uri": {"https://foo.bar/cb"},
+ "client_id": {"1234"},
+ "response_type": {"code"},
+ "state": {"strong-state"},
+ "scope": {"foo bar"},
+ "audience": {"https://cloud.ory.sh/api https://www.ory.sh/api"},
+ },
+ mock: func() {
+ store.EXPECT().GetClient(gomock.Any(), "1234").Return(&DefaultResponseModeClient{
+ DefaultClient: &DefaultClient{
+ RedirectURIs: []string{"https://foo.bar/cb"},
+ Scopes: []string{"foo", "bar"},
+ ResponseTypes: []string{"code"},
+ Audience: []string{"https://cloud.ory.sh/api", "https://www.ory.sh/api"},
+ },
+ ResponseModes: []ResponseModeType{ResponseModeFormPost},
+ }, nil)
+ },
+ expectedError: ErrUnsupportedResponseMode,
+ },
+ /* determine correct response mode if default */
+ {
desc: "success with response mode",
conf: &Fosite{Store: store, Config: &Config{ScopeStrategy: ExactScopeStrategy, AudienceMatchingStrategy: DefaultAudienceMatchingStrategy}},
query: url.Values{