Add OAuth2 response_mode=fragment (#3335)

* Add response_mode=fragment to discovery documents
* Add test for `response_mode=query`
* refactor OAuth 2.0 tests back into regular functions, because macros are messy
* Disallow some `response_type` x `response_mode` combinations per spec

proto/src/oauth2.rs

@@ -38,7 +38,16 @@ pub struct PkceRequest {
 #[derive(Serialize, Deserialize, Debug, Clone)]
 pub struct AuthorisationRequest {
     // Must be "code". (or token, see 4.2.1)
-    pub response_type: String,
+    pub response_type: ResponseType,
+    /// Response mode.
+    ///
+    /// Optional; defaults to `query` for `response_type=code` (Auth Code), and
+    /// `fragment` for `response_type=token` (Implicit Grant, which we probably
+    /// won't support).
+    ///
+    /// Reference:
+    /// [OAuth 2.0 Multiple Response Type Encoding Practices: Response Modes](https://openid.net/specs/oauth-v2-multiple-response-types-1_0.html#ResponseModes)
+    pub response_mode: Option<ResponseMode>,
     pub client_id: String,
     pub state: String,
     #[serde(flatten)]
@@ -57,6 +66,39 @@ pub struct AuthorisationRequest {
     pub unknown_keys: BTreeMap<String, serde_json::value::Value>,
 }
 
+impl AuthorisationRequest {
+    /// Get the `response_mode` appropriate for this request, taking into
+    /// account defaults from the `response_type` parameter.
+    ///
+    /// Returns `None` if the selection is invalid.
+    ///
+    /// Reference:
+    /// [OAuth 2.0 Multiple Response Type Encoding Practices: Response Modes](https://openid.net/specs/oauth-v2-multiple-response-types-1_0.html#ResponseModes)
+    pub const fn get_response_mode(&self) -> Option<ResponseMode> {
+        match (self.response_mode, self.response_type) {
+            // https://openid.net/specs/oauth-v2-multiple-response-types-1_0.html#id_token
+            // The default Response Mode for this Response Type is the fragment
+            // encoding and the query encoding MUST NOT be used.
+            (None, ResponseType::IdToken) => Some(ResponseMode::Fragment),
+            (Some(ResponseMode::Query), ResponseType::IdToken) => None,
+
+            // https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2
+            (None, ResponseType::Code) => Some(ResponseMode::Query),
+            // https://datatracker.ietf.org/doc/html/rfc6749#section-4.2.2
+            (None, ResponseType::Token) => Some(ResponseMode::Fragment),
+
+            // https://openid.net/specs/oauth-v2-multiple-response-types-1_0.html#Security
+            // In no case should a set of Authorization Response parameters
+            // whose default Response Mode is the fragment encoding be encoded
+            // using the query encoding.
+            (Some(ResponseMode::Query), ResponseType::Token) => None,
+
+            // Allow others.
+            (Some(m), _) => Some(m),
+        }
+    }
+}
+
 /// An OIDC client redirects to the authorisation server with Authorisation Request
 /// parameters.
 #[skip_serializing_none]
@@ -290,15 +332,20 @@ impl AccessTokenIntrospectResponse {
     }
 }
 
-#[derive(Serialize, Deserialize, Debug, PartialEq, Eq)]
+#[derive(Clone, Copy, Serialize, Deserialize, Debug, PartialEq, Eq)]
 #[serde(rename_all = "snake_case")]
 pub enum ResponseType {
+    // Auth Code flow
+    // https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.1
     Code,
+    // Implicit Grant flow
+    // https://datatracker.ietf.org/doc/html/rfc6749#section-4.2.1
     Token,
+    // https://openid.net/specs/oauth-v2-multiple-response-types-1_0.html#id_token
     IdToken,
 }
 
-#[derive(Serialize, Deserialize, Debug, PartialEq, Eq)]
+#[derive(Clone, Copy, Serialize, Deserialize, Debug, PartialEq, Eq)]
 #[serde(rename_all = "snake_case")]
 pub enum ResponseMode {
     Query,

server/core/src/actors/v1_read.rs

@@ -37,7 +37,7 @@ use kanidmd_lib::{
     idm::ldap::{LdapBoundToken, LdapResponseState},
     idm::oauth2::{
         AccessTokenIntrospectRequest, AccessTokenIntrospectResponse, AuthorisationRequest,
-        AuthoriseResponse, JwkKeySet, Oauth2Error, Oauth2Rfc8414MetadataResponse,
+        AuthoriseReject, AuthoriseResponse, JwkKeySet, Oauth2Error, Oauth2Rfc8414MetadataResponse,
         OidcDiscoveryResponse, OidcToken,
     },
     idm::server::{DomainInfoRead, IdmServerTransaction},
@@ -1441,7 +1441,7 @@ impl QueryServerReadV1 {
         client_auth_info: ClientAuthInfo,
         consent_req: String,
         eventid: Uuid,
-    ) -> Result<Url, OperationError> {
+    ) -> Result<AuthoriseReject, OperationError> {
         let ct = duration_from_epoch_now();
         let mut idms_prox_read = self.idms.proxy_read().await?;
         let ident = idms_prox_read

server/core/src/https/oauth2.rs

@@ -29,8 +29,8 @@ use kanidm_proto::oauth2::AuthorisationResponse;
 #[cfg(feature = "dev-oauth2-device-flow")]
 use kanidm_proto::oauth2::DeviceAuthorizationResponse;
 use kanidmd_lib::idm::oauth2::{
-    AccessTokenIntrospectRequest, AccessTokenRequest, AuthorisationRequest, AuthorisePermitSuccess,
+    AccessTokenIntrospectRequest, AccessTokenRequest, AuthorisationRequest, AuthoriseResponse,
-    AuthoriseResponse, ErrorResponse, Oauth2Error, TokenRevokeRequest,
+    ErrorResponse, Oauth2Error, TokenRevokeRequest,
 };
 use kanidmd_lib::prelude::f_eq;
 use kanidmd_lib::prelude::*;
@@ -257,22 +257,14 @@ async fn oauth2_authorise(
                 .body(body.into())
                 .unwrap()
         }
-        Ok(AuthoriseResponse::Permitted(AuthorisePermitSuccess {
+        Ok(AuthoriseResponse::Permitted(success)) => {
-            mut redirect_uri,
-            state,
-            code,
-        })) => {
             // https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics#section-4.11
             // We could consider changing this to 303?
             #[allow(clippy::unwrap_used)]
             let body =
                 Body::from(serde_json::to_string(&AuthorisationResponse::Permitted).unwrap());
+            let redirect_uri = success.build_redirect_uri();
 
-            redirect_uri
-                .query_pairs_mut()
-                .clear()
-                .append_pair("state", &state)
-                .append_pair("code", &code);
             #[allow(clippy::unwrap_used)]
             Response::builder()
                 .status(StatusCode::FOUND)
@@ -377,18 +369,11 @@ async fn oauth2_authorise_permit(
         .await;
 
     match res {
-        Ok(AuthorisePermitSuccess {
+        Ok(success) => {
-            mut redirect_uri,
-            state,
-            code,
-        }) => {
             // https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics#section-4.11
             // We could consider changing this to 303?
-            redirect_uri
+            let redirect_uri = success.build_redirect_uri();
-                .query_pairs_mut()
+
-                .clear()
-                .append_pair("state", &state)
-                .append_pair("code", &code);
             #[allow(clippy::expect_used)]
             Response::builder()
                 .status(StatusCode::FOUND)
@@ -463,12 +448,9 @@ async fn oauth2_authorise_reject(
         .await;
 
     match res {
-        Ok(mut redirect_uri) => {
+        Ok(reject) => {
-            redirect_uri
+            let redirect_uri = reject.build_redirect_uri();
-                .query_pairs_mut()
+
-                .clear()
-                .append_pair("error", "access_denied")
-                .append_pair("error_description", "authorisation rejected");
             #[allow(clippy::unwrap_used)]
             Response::builder()
                 .header(LOCATION, redirect_uri.as_str())

server/core/src/https/views/oauth2.rs

@@ -3,9 +3,7 @@ use crate::https::{
     middleware::KOpId,
     ServerState,
 };
-use kanidmd_lib::idm::oauth2::{
+use kanidmd_lib::idm::oauth2::{AuthorisationRequest, AuthoriseResponse, Oauth2Error};
-    AuthorisationRequest, AuthorisePermitSuccess, AuthoriseResponse, Oauth2Error,
-};
 use kanidmd_lib::prelude::*;
 
 use kanidm_proto::internal::COOKIE_OAUTH2_REQ;
@@ -117,16 +115,8 @@ async fn oauth2_auth_req(
         .await;
 
     match res {
-        Ok(AuthoriseResponse::Permitted(AuthorisePermitSuccess {
+        Ok(AuthoriseResponse::Permitted(success)) => {
-            mut redirect_uri,
+            let redirect_uri = success.build_redirect_uri();
-            state,
-            code,
-        })) => {
-            redirect_uri
-                .query_pairs_mut()
-                .clear()
-                .append_pair("state", &state)
-                .append_pair("code", &code);
 
             (
                 jar,
@@ -259,32 +249,24 @@ pub async fn view_consent_post(
         .await;
 
     match res {
-        Ok(AuthorisePermitSuccess {
+        Ok(success) => {
-            mut redirect_uri,
-            state,
-            code,
-        }) => {
             let jar = cookies::destroy(jar, COOKIE_OAUTH2_REQ, &server_state);
 
             if let Some(redirect) = consent_form.redirect {
                 Ok((
                     jar,
                     [
-                        (HX_REDIRECT, redirect_uri.as_str().to_string()),
+                        (HX_REDIRECT, success.redirect_uri.as_str().to_string()),
                         (
                             ACCESS_CONTROL_ALLOW_ORIGIN.as_str(),
-                            redirect_uri.origin().ascii_serialization(),
+                            success.redirect_uri.origin().ascii_serialization(),
                         ),
                     ],
                     Redirect::to(&redirect),
                 )
                     .into_response())
             } else {
-                redirect_uri
+                let redirect_uri = success.build_redirect_uri();
-                    .query_pairs_mut()
-                    .clear()
-                    .append_pair("state", &state)
-                    .append_pair("code", &code);
                 Ok((
                     jar,
                     [

server/lib/src/idm/oauth2.rs

@@ -138,12 +138,14 @@ struct ConsentToken {
         as = "Option<serde_with::base64::Base64<serde_with::base64::UrlSafe, formats::Unpadded>>"
     )]
     pub code_challenge: Option<Vec<u8>>,
-    // Where the RS wants us to go back to.
+    // Where the client wants us to go back to.
     pub redirect_uri: Url,
     // The scopes being granted
     pub scopes: BTreeSet<String>,
     // We stash some details here for oidc.
     pub nonce: Option<String>,
+    /// The format the response should be returned to the application in.
+    pub response_mode: ResponseMode,
 }
 
 #[serde_as]
@@ -230,12 +232,72 @@ pub enum AuthoriseResponse {
 
 #[derive(Debug)]
 pub struct AuthorisePermitSuccess {
-    // Where the RS wants us to go back to.
+    // Where the client wants us to go back to.
     pub redirect_uri: Url,
     // The CSRF as a string
     pub state: String,
     // The exchange code as a String
     pub code: String,
+    /// The format the response should be returned to the application in.
+    pub response_mode: ResponseMode,
+}
+
+impl AuthorisePermitSuccess {
+    /// Builds a redirect URI to go back to the application when permission was
+    /// granted.
+    pub fn build_redirect_uri(&self) -> Url {
+        let mut redirect_uri = self.redirect_uri.clone();
+
+        // Always clear query and fragment, regardless of the response mode
+        redirect_uri.set_query(None);
+        redirect_uri.set_fragment(None);
+
+        // We can't set query pairs on fragments, only query.
+        let encoded = url::form_urlencoded::Serializer::new(String::new())
+            .append_pair("state", &self.state)
+            .append_pair("code", &self.code)
+            .finish();
+
+        match self.response_mode {
+            ResponseMode::Query => redirect_uri.set_query(Some(&encoded)),
+            ResponseMode::Fragment => redirect_uri.set_fragment(Some(&encoded)),
+        }
+
+        redirect_uri
+    }
+}
+
+#[derive(Debug)]
+pub struct AuthoriseReject {
+    // Where the client wants us to go back to.
+    pub redirect_uri: Url,
+    /// The format the response should be returned to the application in.
+    pub response_mode: ResponseMode,
+}
+
+impl AuthoriseReject {
+    /// Builds a redirect URI to go back to the application when permission was
+    /// rejected.
+    pub fn build_redirect_uri(&self) -> Url {
+        let mut redirect_uri = self.redirect_uri.clone();
+
+        // Always clear query and fragment, regardless of the response mode
+        redirect_uri.set_query(None);
+        redirect_uri.set_fragment(None);
+
+        // We can't set query pairs on fragments, only query.
+        let encoded = url::form_urlencoded::Serializer::new(String::new())
+            .append_pair("error", "access_denied")
+            .append_pair("error_description", "authorisation rejected")
+            .finish();
+
+        match self.response_mode {
+            ResponseMode::Query => redirect_uri.set_query(Some(&encoded)),
+            ResponseMode::Fragment => redirect_uri.set_fragment(Some(&encoded)),
+        }
+
+        redirect_uri
+    }
 }
 
 #[derive(Clone)]
@@ -1188,6 +1250,7 @@ impl IdmServerProxyWriteTransaction<'_> {
             redirect_uri: consent_req.redirect_uri,
             state: consent_req.state,
             code,
+            response_mode: consent_req.response_mode,
         })
     }
 
@@ -1793,10 +1856,18 @@ impl IdmServerProxyReadTransaction<'_> {
         // * is within it's valid time window.
         trace!(?auth_req);
 
-        if auth_req.response_type != "code" {
+        if auth_req.response_type != ResponseType::Code {
-            admin_warn!("Invalid OAuth2 response_type (should be 'code')");
+            admin_warn!("Unsupported OAuth2 response_type (should be 'code')");
             return Err(Oauth2Error::UnsupportedResponseType);
         }
+        let Some(response_mode) = auth_req.get_response_mode() else {
+            admin_warn!(
+                "Invalid response_mode {:?} for response_type {:?}",
+                auth_req.response_mode,
+                auth_req.response_type
+            );
+            return Err(Oauth2Error::InvalidRequest);
+        };
 
         /*
          * 4.1.2.1.  Error Response
@@ -2046,6 +2117,7 @@ impl IdmServerProxyReadTransaction<'_> {
                 redirect_uri: auth_req.redirect_uri.clone(),
                 state: auth_req.state.clone(),
                 code,
+                response_mode,
             }))
         } else {
             //  Check that the scopes are the same as a previous consent (if any)
@@ -2084,6 +2156,7 @@ impl IdmServerProxyReadTransaction<'_> {
                 redirect_uri: auth_req.redirect_uri.clone(),
                 scopes: granted_scopes.iter().cloned().collect(),
                 nonce: auth_req.nonce.clone(),
+                response_mode,
             };
 
             let consent_data = serde_json::to_vec(&consent_req).map_err(|e| {
@@ -2112,7 +2185,7 @@ impl IdmServerProxyReadTransaction<'_> {
         ident: &Identity,
         consent_token: &str,
         ct: Duration,
-    ) -> Result<Url, OperationError> {
+    ) -> Result<AuthoriseReject, OperationError> {
         // Decode the consent req with our system fernet key. Use a ttl of 5 minutes.
         let consent_req: ConsentToken = self
             .oauth2rs
@@ -2154,7 +2227,10 @@ impl IdmServerProxyReadTransaction<'_> {
             })?;
 
         // All good, now confirm the rejection to the client application.
-        Ok(consent_req.redirect_uri)
+        Ok(AuthoriseReject {
+            redirect_uri: consent_req.redirect_uri,
+            response_mode: consent_req.response_mode,
+        })
     }
 
     #[instrument(level = "debug", skip_all)]
@@ -2492,7 +2568,7 @@ impl IdmServerProxyReadTransaction<'_> {
         let jwks_uri = Some(o2rs.jwks_uri.clone());
         let scopes_supported = Some(o2rs.scopes_supported.iter().cloned().collect());
         let response_types_supported = vec![ResponseType::Code];
-        let response_modes_supported = vec![ResponseMode::Query];
+        let response_modes_supported = vec![ResponseMode::Query, ResponseMode::Fragment];
         let grant_types_supported = vec![GrantType::AuthorisationCode];
 
         let token_endpoint_auth_methods_supported = vec![
@@ -2563,7 +2639,7 @@ impl IdmServerProxyReadTransaction<'_> {
         let jwks_uri = o2rs.jwks_uri.clone();
         let scopes_supported = Some(o2rs.scopes_supported.iter().cloned().collect());
         let response_types_supported = vec![ResponseType::Code];
-        let response_modes_supported = vec![ResponseMode::Query];
+        let response_modes_supported = vec![ResponseMode::Query, ResponseMode::Fragment];
 
         // TODO: add device code if the rs supports it per <https://www.rfc-editor.org/rfc/rfc8628#section-4>
         // `urn:ietf:params:oauth:grant-type:device_code`
@@ -2936,7 +3012,8 @@ mod tests {
             let scope: BTreeSet<String> = $scope.split(" ").map(|s| s.to_string()).collect();
 
             let auth_req = AuthorisationRequest {
-                response_type: "code".to_string(),
+                response_type: ResponseType::Code,
+                response_mode: None,
                 client_id: "test_resource_server".to_string(),
                 state: "123".to_string(),
                 pkce_request: Some(PkceRequest {
@@ -3431,7 +3508,9 @@ mod tests {
 
         //  * response type != code.
         let auth_req = AuthorisationRequest {
-            response_type: "NOTCODE".to_string(),
+            // We're unlikely to support Implicit Grant
+            response_type: ResponseType::Token,
+            response_mode: None,
             client_id: "test_resource_server".to_string(),
             state: "123".to_string(),
             pkce_request: pkce_request.clone(),
@@ -3452,7 +3531,8 @@ mod tests {
 
         // * No pkce in pkce enforced mode.
         let auth_req = AuthorisationRequest {
-            response_type: "code".to_string(),
+            response_type: ResponseType::Code,
+            response_mode: None,
             client_id: "test_resource_server".to_string(),
             state: "123".to_string(),
             pkce_request: None,
@@ -3473,7 +3553,8 @@ mod tests {
 
         //  * invalid rs name
         let auth_req = AuthorisationRequest {
-            response_type: "code".to_string(),
+            response_type: ResponseType::Code,
+            response_mode: None,
             client_id: "NOT A REAL RESOURCE SERVER".to_string(),
             state: "123".to_string(),
             pkce_request: pkce_request.clone(),
@@ -3494,7 +3575,8 @@ mod tests {
 
         //  * mismatched origin in the redirect.
         let auth_req = AuthorisationRequest {
-            response_type: "code".to_string(),
+            response_type: ResponseType::Code,
+            response_mode: None,
             client_id: "test_resource_server".to_string(),
             state: "123".to_string(),
             pkce_request: pkce_request.clone(),
@@ -3515,7 +3597,8 @@ mod tests {
 
         // * invalid uri in the redirect
         let auth_req = AuthorisationRequest {
-            response_type: "code".to_string(),
+            response_type: ResponseType::Code,
+            response_mode: None,
             client_id: "test_resource_server".to_string(),
             state: "123".to_string(),
             pkce_request: pkce_request.clone(),
@@ -3536,7 +3619,8 @@ mod tests {
 
         // Not Authenticated
         let auth_req = AuthorisationRequest {
-            response_type: "code".to_string(),
+            response_type: ResponseType::Code,
+            response_mode: None,
             client_id: "test_resource_server".to_string(),
             state: "123".to_string(),
             pkce_request: pkce_request.clone(),
@@ -3559,7 +3643,8 @@ mod tests {
 
         // Requested scope is not available
         let auth_req = AuthorisationRequest {
-            response_type: "code".to_string(),
+            response_type: ResponseType::Code,
+            response_mode: None,
             client_id: "test_resource_server".to_string(),
             state: "123".to_string(),
             pkce_request: pkce_request.clone(),
@@ -3580,7 +3665,8 @@ mod tests {
 
         // Not a member of the group.
         let auth_req = AuthorisationRequest {
-            response_type: "code".to_string(),
+            response_type: ResponseType::Code,
+            response_mode: None,
             client_id: "test_resource_server".to_string(),
             state: "123".to_string(),
             pkce_request: pkce_request.clone(),
@@ -3601,7 +3687,8 @@ mod tests {
 
         // Deny Anonymous auth methods
         let auth_req = AuthorisationRequest {
-            response_type: "code".to_string(),
+            response_type: ResponseType::Code,
+            response_mode: None,
             client_id: "test_resource_server".to_string(),
             state: "123".to_string(),
             pkce_request,
@@ -3892,7 +3979,8 @@ mod tests {
         let (code_verifier, code_challenge) = create_code_verifier!("Whar Garble");
 
         let auth_req = AuthorisationRequest {
-            response_type: "code".to_string(),
+            response_type: ResponseType::Code,
+            response_mode: None,
             client_id: "test_resource_server".to_string(),
             state: "123".to_string(),
             pkce_request: Some(PkceRequest {
@@ -3962,7 +4050,8 @@ mod tests {
             .expect("Unable to process uat");
 
         let auth_req = AuthorisationRequest {
-            response_type: "code".to_string(),
+            response_type: ResponseType::Code,
+            response_mode: None,
             client_id: "test_resource_server".to_string(),
             state: "123".to_string(),
             pkce_request: Some(PkceRequest {
@@ -4428,7 +4517,7 @@ mod tests {
             .check_oauth2_authorise_reject(&ident, &consent_token, ct)
             .expect("Failed to perform OAuth2 reject");
 
-        assert_eq!(reject_success, redirect_uri);
+        assert_eq!(reject_success.redirect_uri, redirect_uri);
 
         // Too much time past to reject
         let past_ct = Duration::from_secs(TEST_CURRENT_TIME + 301);
@@ -4523,7 +4612,7 @@ mod tests {
         assert_eq!(discovery.response_types_supported, vec![ResponseType::Code]);
         assert_eq!(
             discovery.response_modes_supported,
-            vec![ResponseMode::Query]
+            vec![ResponseMode::Query, ResponseMode::Fragment]
         );
         assert_eq!(
             discovery.grant_types_supported,
@@ -4683,7 +4772,7 @@ mod tests {
         assert_eq!(discovery.response_types_supported, vec![ResponseType::Code]);
         assert_eq!(
             discovery.response_modes_supported,
-            vec![ResponseMode::Query]
+            vec![ResponseMode::Query, ResponseMode::Fragment]
         );
         assert_eq!(
             discovery.grant_types_supported,
@@ -5171,7 +5260,8 @@ mod tests {
 
         // Check we allow none.
         let auth_req = AuthorisationRequest {
-            response_type: "code".to_string(),
+            response_type: ResponseType::Code,
+            response_mode: None,
             client_id: "test_resource_server".to_string(),
             state: "123".to_string(),
             pkce_request: None,
@@ -5382,7 +5472,8 @@ mod tests {
         let (_code_verifier, code_challenge) = create_code_verifier!("Whar Garble");
 
         let auth_req = AuthorisationRequest {
-            response_type: "code".to_string(),
+            response_type: ResponseType::Code,
+            response_mode: None,
             client_id: "test_resource_server".to_string(),
             state: "123".to_string(),
             pkce_request: Some(PkceRequest {
@@ -5440,7 +5531,8 @@ mod tests {
         let (_code_verifier, code_challenge) = create_code_verifier!("Whar Garble");
 
         let auth_req = AuthorisationRequest {
-            response_type: "code".to_string(),
+            response_type: ResponseType::Code,
+            response_mode: None,
             client_id: "test_resource_server".to_string(),
             state: "123".to_string(),
             pkce_request: Some(PkceRequest {
@@ -5582,7 +5674,8 @@ mod tests {
 
         // First, the user does not request pkce in their exchange.
         let auth_req = AuthorisationRequest {
-            response_type: "code".to_string(),
+            response_type: ResponseType::Code,
+            response_mode: None,
             client_id: "test_resource_server".to_string(),
             state: "123".to_string(),
             pkce_request: None,
@@ -5657,7 +5750,8 @@ mod tests {
 
         // First, NOTE the lack of https on the redir uri.
         let auth_req = AuthorisationRequest {
-            response_type: "code".to_string(),
+            response_type: ResponseType::Code,
+            response_mode: None,
             client_id: "test_resource_server".to_string(),
             state: "123".to_string(),
             pkce_request: Some(PkceRequest {
@@ -6621,7 +6715,8 @@ mod tests {
         let (code_verifier, code_challenge) = create_code_verifier!("Whar Garble");
 
         let auth_req = AuthorisationRequest {
-            response_type: "code".to_string(),
+            response_type: ResponseType::Code,
+            response_mode: None,
             client_id: "test_resource_server".to_string(),
             state: "123".to_string(),
             pkce_request: Some(PkceRequest {

server/testkit/tests/oauth2_test.rs

@@ -17,7 +17,7 @@ use oauth2_ext::PkceCodeChallenge;
 use reqwest::header::{HeaderValue, CONTENT_TYPE};
 use reqwest::StatusCode;
 use uri::{OAUTH2_TOKEN_ENDPOINT, OAUTH2_TOKEN_INTROSPECT_ENDPOINT, OAUTH2_TOKEN_REVOKE_ENDPOINT};
-use url::Url;
+use url::{form_urlencoded::parse as query_parse, Url};
 
 use kanidm_client::KanidmClient;
 use kanidmd_testkit::{
@@ -27,8 +27,23 @@ use kanidmd_testkit::{
     TEST_INTEGRATION_RS_URL,
 };
 
-#[kanidmd_testkit::test]
+/// Tests an OAuth 2.0 / OpenID confidential client Authorisation Client flow.
-async fn test_oauth2_openid_basic_flow(rsclient: KanidmClient) {
+///
+/// ## Arguments
+///
+/// * `response_mode`: If `Some`, the `response_mode` parameter to pass in the
+///   `/oauth2/authorise` request.
+///
+/// * `response_in_fragment`: If `false`, use the `code` passed in the
+///   callback URI's query parameter, and require the fragment to be empty.
+///
+///   If `true`, use the `code` passed in the callback URI's fragment, and
+///   require the query parameter to be empty.
+async fn test_oauth2_openid_basic_flow_impl(
+    rsclient: KanidmClient,
+    response_mode: Option<&str>,
+    response_in_fragment: bool,
+) {
     let res = rsclient
         .auth_simple_password(ADMIN_TEST_USER, ADMIN_TEST_PASSWORD)
         .await;
@@ -225,10 +240,7 @@ async fn test_oauth2_openid_basic_flow(rsclient: KanidmClient) {
 
     let (pkce_code_challenge, pkce_code_verifier) = PkceCodeChallenge::new_random_sha256();
 
-    let response = client
+    let mut query = vec![
-        .get(rsclient.make_url(OAUTH2_AUTHORISE))
-        .bearer_auth(oauth_test_uat.clone())
-        .query(&[
         ("response_type", "code"),
         ("client_id", TEST_INTEGRATION_RS_ID),
         ("state", "YWJjZGVm"),
@@ -237,7 +249,16 @@ async fn test_oauth2_openid_basic_flow(rsclient: KanidmClient) {
         ("redirect_uri", TEST_INTEGRATION_RS_REDIRECT_URL),
         ("scope", "email read openid"),
         ("max_age", "1"),
-        ])
+    ];
+
+    if let Some(response_mode) = response_mode {
+        query.push(("response_mode", response_mode));
+    }
+
+    let response = client
+        .get(rsclient.make_url(OAUTH2_AUTHORISE))
+        .bearer_auth(oauth_test_uat.clone())
+        .query(&query)
         .send()
         .await
         .expect("Failed to send request.");
@@ -288,14 +309,19 @@ async fn test_oauth2_openid_basic_flow(rsclient: KanidmClient) {
 
     // Now check it's content
     let redir_url = Url::parse(&redir_str).expect("Url parse failure");
+    let pairs: BTreeMap<_, _> = if response_in_fragment {
+        assert!(redir_url.query().is_none());
+        let fragment = redir_url.fragment().expect("missing URL fragment");
+        query_parse(fragment.as_bytes()).collect()
+    } else {
+        // response_mode = query is default for response_type = code
+        assert!(redir_url.fragment().is_none());
+        redir_url.query_pairs().collect()
+    };
 
     // We should have state and code.
-    let pairs: BTreeMap<_, _> = redir_url.query_pairs().collect();
-
     let code = pairs.get("code").expect("code not found!");
-
     let state = pairs.get("state").expect("state not found!");
-
     assert_eq!(state, "YWJjZGVm");
 
     // Step 3 - the "resource server" then uses this state and code to directly contact
@@ -485,8 +511,50 @@ async fn test_oauth2_openid_basic_flow(rsclient: KanidmClient) {
         .expect("Failed to update oauth2 scopes");
 }
 
+/// Test an OAuth 2.0/OpenID confidential client Authorisation Code flow, with
+/// `response_mode` unset.
+///
+/// The response should be returned as a query parameter.
 #[kanidmd_testkit::test]
-async fn test_oauth2_openid_public_flow(rsclient: KanidmClient) {
+async fn test_oauth2_openid_basic_flow_mode_unset(rsclient: KanidmClient) {
+    test_oauth2_openid_basic_flow_impl(rsclient, None, false).await;
+}
+
+/// Test an OAuth 2.0/OpenID confidential client Authorisation Code flow, with
+/// `response_mode=query`.
+///
+/// The response should be returned as a query parameter.
+#[kanidmd_testkit::test]
+async fn test_oauth2_openid_basic_flow_mode_query(rsclient: KanidmClient) {
+    test_oauth2_openid_basic_flow_impl(rsclient, Some("query"), false).await;
+}
+
+/// Test an OAuth 2.0/OpenID confidential client Authorisation Code flow, with
+/// `response_mode=fragment`.
+///
+/// The response should be returned in the URI's fragment.
+#[kanidmd_testkit::test]
+async fn test_oauth2_openid_basic_flow_mode_fragment(rsclient: KanidmClient) {
+    test_oauth2_openid_basic_flow_impl(rsclient, Some("fragment"), true).await;
+}
+
+/// Tests an OAuth 2.0 / OpenID public client Authorisation Client flow.
+///
+/// ## Arguments
+///
+/// * `response_mode`: If `Some`, the `response_mode` parameter to pass in the
+///   `/oauth2/authorise` request.
+///
+/// * `response_in_fragment`: If `false`, use the `code` passed in the
+///   callback URI's query parameter, and require the fragment to be empty.
+///
+///   If `true`, use the `code` passed in the callback URI's fragment, and
+///   require the query parameter to be empty.
+async fn test_oauth2_openid_public_flow_impl(
+    rsclient: KanidmClient,
+    response_mode: Option<&str>,
+    response_in_fragment: bool,
+) {
     let res = rsclient
         .auth_simple_password(ADMIN_TEST_USER, ADMIN_TEST_PASSWORD)
         .await;
@@ -624,10 +692,7 @@ async fn test_oauth2_openid_public_flow(rsclient: KanidmClient) {
     // get call directly. This should be a 200. (?)
     let (pkce_code_challenge, pkce_code_verifier) = PkceCodeChallenge::new_random_sha256();
 
-    let response = client
+    let mut query = vec![
-        .get(rsclient.make_url(OAUTH2_AUTHORISE))
-        .bearer_auth(oauth_test_uat.clone())
-        .query(&[
         ("response_type", "code"),
         ("client_id", TEST_INTEGRATION_RS_ID),
         ("state", "YWJjZGVm"),
@@ -635,7 +700,16 @@ async fn test_oauth2_openid_public_flow(rsclient: KanidmClient) {
         ("code_challenge_method", "S256"),
         ("redirect_uri", TEST_INTEGRATION_RS_REDIRECT_URL),
         ("scope", "email read openid"),
-        ])
+    ];
+
+    if let Some(response_mode) = response_mode {
+        query.push(("response_mode", response_mode));
+    }
+
+    let response = client
+        .get(rsclient.make_url(OAUTH2_AUTHORISE))
+        .bearer_auth(oauth_test_uat.clone())
+        .query(&query)
         .send()
         .await
         .expect("Failed to send request.");
@@ -685,13 +759,19 @@ async fn test_oauth2_openid_public_flow(rsclient: KanidmClient) {
     // Now check it's content
     let redir_url = Url::parse(&redir_str).expect("Url parse failure");
 
+    let pairs: BTreeMap<_, _> = if response_in_fragment {
+        assert!(redir_url.query().is_none());
+        let fragment = redir_url.fragment().expect("missing URL fragment");
+        query_parse(fragment.as_bytes()).collect()
+    } else {
+        // response_mode = query is default for response_type = code
+        assert!(redir_url.fragment().is_none());
+        redir_url.query_pairs().collect()
+    };
+
     // We should have state and code.
-    let pairs: BTreeMap<_, _> = redir_url.query_pairs().collect();
-
     let code = pairs.get("code").expect("code not found!");
-
     let state = pairs.get("state").expect("state not found!");
-
     assert_eq!(state, "YWJjZGVm");
 
     // Step 3 - the "resource server" then uses this state and code to directly contact
@@ -796,6 +876,33 @@ async fn test_oauth2_openid_public_flow(rsclient: KanidmClient) {
         .expect("Failed to update oauth2 scopes");
 }
 
+/// Test an OAuth 2.0/OpenID public client Authorisation Code flow, with
+/// `response_mode` unset.
+///
+/// The response should be returned as a query parameter.
+#[kanidmd_testkit::test]
+async fn test_oauth2_openid_public_flow_mode_unset(rsclient: KanidmClient) {
+    test_oauth2_openid_public_flow_impl(rsclient, None, false).await;
+}
+
+/// Test an OAuth 2.0/OpenID public client Authorisation Code flow, with
+/// `response_mode=query`.
+///
+/// The response should be returned as a query parameter.
+#[kanidmd_testkit::test]
+async fn test_oauth2_openid_public_flow_mode_query(rsclient: KanidmClient) {
+    test_oauth2_openid_public_flow_impl(rsclient, Some("query"), false).await;
+}
+
+/// Test an OAuth 2.0/OpenID public client Authorisation Code flow, with
+/// `response_mode=fragment`.
+///
+/// The response should be returned in the URI's fragment.
+#[kanidmd_testkit::test]
+async fn test_oauth2_openid_public_flow_mode_fragment(rsclient: KanidmClient) {
+    test_oauth2_openid_public_flow_impl(rsclient, Some("fragment"), true).await;
+}
+
 #[kanidmd_testkit::test]
 async fn test_oauth2_token_post_bad_bodies(rsclient: KanidmClient) {
     let res = rsclient