diff --git a/proto/src/oauth2.rs b/proto/src/oauth2.rs index db5ed9ac5..a276e0386 100644 --- a/proto/src/oauth2.rs +++ b/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, pub client_id: String, pub state: String, #[serde(flatten)] @@ -57,6 +66,39 @@ pub struct AuthorisationRequest { pub unknown_keys: BTreeMap, } +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 { + 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, diff --git a/server/core/src/actors/v1_read.rs b/server/core/src/actors/v1_read.rs index c145c26ae..8bfa7853b 100644 --- a/server/core/src/actors/v1_read.rs +++ b/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 { + ) -> Result { let ct = duration_from_epoch_now(); let mut idms_prox_read = self.idms.proxy_read().await?; let ident = idms_prox_read diff --git a/server/core/src/https/oauth2.rs b/server/core/src/https/oauth2.rs index aedeb3ba7..ae986119a 100644 --- a/server/core/src/https/oauth2.rs +++ b/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, - AuthoriseResponse, ErrorResponse, Oauth2Error, TokenRevokeRequest, + AccessTokenIntrospectRequest, AccessTokenRequest, AuthorisationRequest, AuthoriseResponse, + 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 { - mut redirect_uri, - state, - code, - })) => { + Ok(AuthoriseResponse::Permitted(success)) => { // 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 { - mut redirect_uri, - state, - code, - }) => { + Ok(success) => { // https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics#section-4.11 // We could consider changing this to 303? - redirect_uri - .query_pairs_mut() - .clear() - .append_pair("state", &state) - .append_pair("code", &code); + let redirect_uri = success.build_redirect_uri(); + #[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) => { - redirect_uri - .query_pairs_mut() - .clear() - .append_pair("error", "access_denied") - .append_pair("error_description", "authorisation rejected"); + Ok(reject) => { + let redirect_uri = reject.build_redirect_uri(); + #[allow(clippy::unwrap_used)] Response::builder() .header(LOCATION, redirect_uri.as_str()) diff --git a/server/core/src/https/views/oauth2.rs b/server/core/src/https/views/oauth2.rs index 373d47208..b3d7ab37f 100644 --- a/server/core/src/https/views/oauth2.rs +++ b/server/core/src/https/views/oauth2.rs @@ -3,9 +3,7 @@ use crate::https::{ middleware::KOpId, ServerState, }; -use kanidmd_lib::idm::oauth2::{ - AuthorisationRequest, AuthorisePermitSuccess, AuthoriseResponse, Oauth2Error, -}; +use kanidmd_lib::idm::oauth2::{AuthorisationRequest, 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 { - mut redirect_uri, - state, - code, - })) => { - redirect_uri - .query_pairs_mut() - .clear() - .append_pair("state", &state) - .append_pair("code", &code); + Ok(AuthoriseResponse::Permitted(success)) => { + let redirect_uri = success.build_redirect_uri(); ( jar, @@ -259,32 +249,24 @@ pub async fn view_consent_post( .await; match res { - Ok(AuthorisePermitSuccess { - mut redirect_uri, - state, - code, - }) => { + Ok(success) => { 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 - .query_pairs_mut() - .clear() - .append_pair("state", &state) - .append_pair("code", &code); + let redirect_uri = success.build_redirect_uri(); Ok(( jar, [ diff --git a/server/lib/src/idm/oauth2.rs b/server/lib/src/idm/oauth2.rs index e3613729c..b791d59c2 100644 --- a/server/lib/src/idm/oauth2.rs +++ b/server/lib/src/idm/oauth2.rs @@ -138,12 +138,14 @@ struct ConsentToken { as = "Option>" )] pub code_challenge: Option>, - // 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, // We stash some details here for oidc. pub nonce: Option, + /// 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" { - admin_warn!("Invalid OAuth2 response_type (should be 'code')"); + if auth_req.response_type != ResponseType::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 { + ) -> Result { // 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 // `urn:ietf:params:oauth:grant-type:device_code` @@ -2936,7 +3012,8 @@ mod tests { let scope: BTreeSet = $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 { diff --git a/server/testkit/tests/oauth2_test.rs b/server/testkit/tests/oauth2_test.rs index aa085f4e3..21f3096f3 100644 --- a/server/testkit/tests/oauth2_test.rs +++ b/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] -async fn test_oauth2_openid_basic_flow(rsclient: KanidmClient) { +/// Tests an OAuth 2.0 / OpenID confidential 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_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,19 +240,25 @@ async fn test_oauth2_openid_basic_flow(rsclient: KanidmClient) { let (pkce_code_challenge, pkce_code_verifier) = PkceCodeChallenge::new_random_sha256(); + let mut query = vec![ + ("response_type", "code"), + ("client_id", TEST_INTEGRATION_RS_ID), + ("state", "YWJjZGVm"), + ("code_challenge", pkce_code_challenge.as_str()), + ("code_challenge_method", "S256"), + ("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(&[ - ("response_type", "code"), - ("client_id", TEST_INTEGRATION_RS_ID), - ("state", "YWJjZGVm"), - ("code_challenge", pkce_code_challenge.as_str()), - ("code_challenge_method", "S256"), - ("redirect_uri", TEST_INTEGRATION_RS_REDIRECT_URL), - ("scope", "email read openid"), - ("max_age", "1"), - ]) + .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,18 +692,24 @@ 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 mut query = vec![ + ("response_type", "code"), + ("client_id", TEST_INTEGRATION_RS_ID), + ("state", "YWJjZGVm"), + ("code_challenge", pkce_code_challenge.as_str()), + ("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(&[ - ("response_type", "code"), - ("client_id", TEST_INTEGRATION_RS_ID), - ("state", "YWJjZGVm"), - ("code_challenge", pkce_code_challenge.as_str()), - ("code_challenge_method", "S256"), - ("redirect_uri", TEST_INTEGRATION_RS_REDIRECT_URL), - ("scope", "email read openid"), - ]) + .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