Auth Req omitted response_mode does not validate the default response_mode against the ResponseModeClient

[james-d-elliott]

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{