Address comments

Author: MatMaul

crates/handlers/src/oauth2/authorization/mod.rs

@@ -106,25 +106,18 @@ fn resolve_response_mode(
 ) -> Result<ResponseMode, RouteError> {
     use ResponseMode as M;
 
-    if response_type.has_token() || response_type.has_id_token() {
         // If the response type includes either "token" or "id_token", the default
         // response mode is "fragment" and the response mode "query" must not be
         // used
-        if let Some(suggested_response_mode) = suggested_response_mode {
+    if response_type.has_token() || response_type.has_id_token() {
             match suggested_response_mode {
-                M::Query => Err(RouteError::InvalidResponseMode),
-                mode => Ok(mode),
-            }
-        } else {
-            Ok(M::Fragment)
+            None => Ok(M::Fragment),
+            Some(M::Query) => Err(RouteError::InvalidResponseMode),
+            Some(mode) => Ok(mode),
         }
     } else {
         // In other cases, all response modes are allowed, defaulting to "query"
-        if let Some(suggested_response_mode) = suggested_response_mode {
-            Ok(suggested_response_mode)
-        } else {
-            Ok(M::Query)
-        }
+        Ok(suggested_response_mode.unwrap_or(M::Query))
     }
 }
 

crates/handlers/src/upstream_oauth2/callback.rs

@@ -113,9 +113,6 @@ pub(crate) enum RouteError {
         expected: UpstreamOAuthProviderResponseMode,
     },
 
-    #[error("Invalid request method '{method}'")]
-    InvalidReqMethod { method: Method },
-
     #[error(transparent)]
     Internal(Box<dyn std::error::Error + Send + Sync + 'static>),
 }
@@ -187,46 +184,26 @@ pub(crate) async fn handler(
     // The `Form` extractor will use the body of the request for POST requests and
     // the query parameters for GET requests. We need to then look at the method do
     // make sure it matches the expected `response_mode`
-    match method {
-        Method::GET => {
-            match provider.response_mode {
-                Some(UpstreamOAuthProviderResponseMode::Query) | None => {}
-                Some(UpstreamOAuthProviderResponseMode::FormPost) => {
-                    return Err(RouteError::InvalidResponseMode {
-                        expected: UpstreamOAuthProviderResponseMode::Query,
-                    })
-                }
-            };
-        }
-        Method::POST => {
-            match provider.response_mode {
-                Some(UpstreamOAuthProviderResponseMode::FormPost) => {
-                    // We set the cookies with a `Same-Site` policy set to `Lax`, so because this is
-                    // usually a cross-site form POST, we need to render a form with the
-                    // same values, which posts back to the same URL. However, there are
-                    // other valid reasons for the cookie to be missing, so to track whether we did
-                    // this POST ourselves, we set a flag.
-                    if sessions_cookie.is_empty() && !params.did_mas_repost_to_itself {
-                        let params = Params {
-                            did_mas_repost_to_itself: true,
-                            ..params
-                        };
-                        let context =
-                            FormPostContext::new_for_current_url(params).with_language(&locale);
-                        let html = templates.render_form_post(&context)?;
-                        return Ok(Html(html).into_response());
-                    }
-                }
-                Some(UpstreamOAuthProviderResponseMode::Query) | None => {
-                    return Err(RouteError::InvalidResponseMode {
-                        expected: UpstreamOAuthProviderResponseMode::FormPost,
-                    })
-                }
-            };
-        }
-        method => {
-            return Err(RouteError::InvalidReqMethod { method });
+    match (provider.response_mode, method) {
+        (Some(UpstreamOAuthProviderResponseMode::Query) | None, Method::GET) => {}
+        (Some(UpstreamOAuthProviderResponseMode::FormPost) | None, Method::POST) => {
+            // We set the cookies with a `Same-Site` policy set to `Lax`, so because this is
+            // usually a cross-site form POST, we need to render a form with the
+            // same values, which posts back to the same URL. However, there are
+            // other valid reasons for the cookie to be missing, so to track whether we did
+            // this POST ourselves, we set a flag.
+            if sessions_cookie.is_empty() && !params.did_mas_repost_to_itself {
+                let params = Params {
+                    did_mas_repost_to_itself: true,
+                    ..params
+                };
+                let context = FormPostContext::new_for_current_url(params).with_language(&locale);
+                let html = templates.render_form_post(&context)?;
+                return Ok(Html(html).into_response());
+            }
         }
+        (None, _) => {}
+        (Some(expected), _) => return Err(RouteError::InvalidResponseMode { expected }),
     }
 
     let (session_id, _post_auth_action) = sessions_cookie