diff --git a/book/src/integrations/oauth2.md b/book/src/integrations/oauth2.md index 2326752e4..e3c7478d2 100644 --- a/book/src/integrations/oauth2.md +++ b/book/src/integrations/oauth2.md @@ -183,7 +183,7 @@ class: object displayname: Nextcloud Production oauth2_rs_basic_secret: hidden oauth2_rs_name: nextcloud -oauth2_rs_origin: https://nextcloud.example.com +oauth2_rs_origin_landing: https://nextcloud.example.com oauth2_rs_token_key: hidden ``` @@ -195,15 +195,15 @@ kanidm system oauth2 show-basic-secret nextcloud ``` -### Configure the Resource Server +### Configure the Client/Resource Server On your client, you should configure the client ID as the `oauth2_rs_name` from Kanidm, and the password to be the value shown in `oauth2_rs_basic_secret`. Ensure that the code challenge/verification method is set to S256. -You should now be able to test authorisation. +You should now be able to test authorisation to the client. -## Resetting Resource Server Security Material +## Resetting Client Security Material In the case of disclosure of the basic secret or some other security event where you may wish to invalidate a services active sessions/tokens. You can reset the secret material of the server with: @@ -286,6 +286,25 @@ kanidm system oauth2 disable-localhost-redirects kanidm system oauth2 enable-localhost-redirects mywebapp ``` +## Alternate Redirect Origins + +Some services may have a website URL as well as native applications. These native applications +require alternate redirection URLs to be configured so that after an OAuth2 exchange, the system can +redirect to the native application. + +To support this Kanidm allows supplemental origins to be configured on clients. + +{{#template ../templates/kani-warning.md imagepath=../images title=WARNING text=The ability to configure multiple origins is NOT intended to allow you to share a single Kanidm client definition between multiple OAuth2 clients. This fundamentally breaks the OAuth2 security model and is NOT SUPPORTED as a configuration. Multiple origins is only to allow supplemental redirects within the _same_ client application. }} + +```bash +kanidm system oauth2 add-origin +kanidm system oauth2 remove-origin + +kanidm system oauth2 add-origin nextcloud app://ios-nextcloud +``` + +Supplemental URLs are shown in the OAuth2 client configuration in the `oauth2_rs_origin` attribute. + ## Extended Options for Legacy Clients Not all clients support modern standards like PKCE or ECDSA. In these situations it may be necessary diff --git a/libs/client/src/oauth.rs b/libs/client/src/oauth.rs index 2d0198542..3f29d8adc 100644 --- a/libs/client/src/oauth.rs +++ b/libs/client/src/oauth.rs @@ -10,6 +10,7 @@ use kanidm_proto::internal::{ImageValue, Oauth2ClaimMapJoin}; use kanidm_proto::v1::Entry; use reqwest::multipart; use std::collections::BTreeMap; +use url::Url; impl KanidmClient { // ==== Oauth2 resource server configuration @@ -31,9 +32,10 @@ impl KanidmClient { new_oauth2_rs .attrs .insert(ATTR_DISPLAYNAME.to_string(), vec![displayname.to_string()]); - new_oauth2_rs - .attrs - .insert(ATTR_OAUTH2_RS_ORIGIN.to_string(), vec![origin.to_string()]); + new_oauth2_rs.attrs.insert( + ATTR_OAUTH2_RS_ORIGIN_LANDING.to_string(), + vec![origin.to_string()], + ); self.perform_post_request("/v1/oauth2/_basic", new_oauth2_rs) .await } @@ -51,9 +53,10 @@ impl KanidmClient { new_oauth2_rs .attrs .insert(ATTR_DISPLAYNAME.to_string(), vec![displayname.to_string()]); - new_oauth2_rs - .attrs - .insert(ATTR_OAUTH2_RS_ORIGIN.to_string(), vec![origin.to_string()]); + new_oauth2_rs.attrs.insert( + ATTR_OAUTH2_RS_ORIGIN_LANDING.to_string(), + vec![origin.to_string()], + ); self.perform_post_request("/v1/oauth2/_public", new_oauth2_rs) .await } @@ -78,7 +81,6 @@ impl KanidmClient { id: &str, name: Option<&str>, displayname: Option<&str>, - origin: Option<&str>, landing: Option<&str>, reset_secret: bool, reset_token_key: bool, @@ -99,12 +101,6 @@ impl KanidmClient { vec![newdisplayname.to_string()], ); } - if let Some(neworigin) = origin { - update_oauth2_rs.attrs.insert( - ATTR_OAUTH2_RS_ORIGIN.to_string(), - vec![neworigin.to_string()], - ); - } if let Some(newlanding) = landing { update_oauth2_rs.attrs.insert( ATTR_OAUTH2_RS_ORIGIN_LANDING.to_string(), @@ -393,4 +389,30 @@ impl KanidmClient { ) .await } + + pub async fn idm_oauth2_client_add_origin( + &self, + id: &str, + origin: &Url, + ) -> Result<(), ClientError> { + let url_to_add = &[origin.as_str()]; + self.perform_post_request( + format!("/v1/oauth2/{}/_attr/{}", id, ATTR_OAUTH2_RS_ORIGIN).as_str(), + url_to_add, + ) + .await + } + + pub async fn idm_oauth2_client_remove_origin( + &self, + id: &str, + origin: &Url, + ) -> Result<(), ClientError> { + let url_to_remove = &[origin.as_str()]; + self.perform_delete_request_with_body( + format!("/v1/oauth2/{}/_attr/{}", id, ATTR_OAUTH2_RS_ORIGIN).as_str(), + url_to_remove, + ) + .await + } } diff --git a/server/core/src/https/v1.rs b/server/core/src/https/v1.rs index b5cdca75e..47cfe31a7 100644 --- a/server/core/src/https/v1.rs +++ b/server/core/src/https/v1.rs @@ -3074,6 +3074,11 @@ pub(crate) fn route_setup(state: ServerState) -> Router { .patch(super::v1_oauth2::oauth2_id_patch) .delete(super::v1_oauth2::oauth2_id_delete), ) + .route( + "/v1/oauth2/:rs_name/_attr/:attr", + post(super::v1_oauth2::oauth2_id_attr_post) + .delete(super::v1_oauth2::oauth2_id_attr_delete), + ) .route( "/v1/oauth2/:rs_name/_image", post(super::v1_oauth2::oauth2_id_image_post) diff --git a/server/core/src/https/v1_oauth2.rs b/server/core/src/https/v1_oauth2.rs index 489d84137..5e481afab 100644 --- a/server/core/src/https/v1_oauth2.rs +++ b/server/core/src/https/v1_oauth2.rs @@ -2,7 +2,10 @@ use super::apidocs::response_schema::{ApiResponseWithout200, DefaultApiResponse} use super::errors::WebError; use super::middleware::KOpId; use super::oauth2::oauth2_id; -use super::v1::{json_rest_event_get, json_rest_event_post}; +use super::v1::{ + json_rest_event_delete_id_attr, json_rest_event_get, json_rest_event_post, + json_rest_event_post_id_attr, +}; use super::ServerState; use crate::https::extractors::VerifiedClientInformation; @@ -205,6 +208,57 @@ pub(crate) async fn oauth2_id_scopemap_post( .map_err(WebError::from) } +#[utoipa::path( + post, + path = "/v1/oauth2/{rs_name}/_attr/{attr}", + request_body=Vec, + responses( + DefaultApiResponse, + ), + security(("token_jwt" = [])), + tag = "v1/oauth2/attr", + operation_id = "oauth2_id_attr_post", +)] +pub async fn oauth2_id_attr_post( + Path((id, attr)): Path<(String, String)>, + State(state): State, + Extension(kopid): Extension, + VerifiedClientInformation(client_auth_info): VerifiedClientInformation, + Json(values): Json>, +) -> Result, WebError> { + let filter = filter_all!(f_eq( + Attribute::Class, + EntryClass::OAuth2ResourceServer.into() + )); + json_rest_event_post_id_attr(state, id, attr, filter, values, kopid, client_auth_info).await +} + +#[utoipa::path( + delete, + path = "/v1/oauth2/{rs_name}/_attr/{attr}", + request_body=Option>, + responses( + DefaultApiResponse, + ), + security(("token_jwt" = [])), + tag = "v1/oauth2/attr", + operation_id = "oauth2_id_attr_delete", +)] +pub async fn oauth2_id_attr_delete( + Path((id, attr)): Path<(String, String)>, + State(state): State, + Extension(kopid): Extension, + VerifiedClientInformation(client_auth_info): VerifiedClientInformation, + values: Option>>, +) -> Result, WebError> { + let filter = filter_all!(f_eq( + Attribute::Class, + EntryClass::OAuth2ResourceServer.into() + )); + let values = values.map(|v| v.0); + json_rest_event_delete_id_attr(state, id, attr, filter, values, kopid, client_auth_info).await +} + #[utoipa::path( delete, path = "/v1/oauth2/{rs_name}/_scopemap/{group}", diff --git a/server/lib/PROFILING.md b/server/lib/PROFILING.md index 74703e80a..cea56b40c 100644 --- a/server/lib/PROFILING.md +++ b/server/lib/PROFILING.md @@ -1,5 +1,3 @@ - - ``` cargo test --features=dhat-heap test_idm_authsession_simple_password_mech diff --git a/server/lib/src/constants/schema.rs b/server/lib/src/constants/schema.rs index 69e0719f1..d32ddfd83 100644 --- a/server/lib/src/constants/schema.rs +++ b/server/lib/src/constants/schema.rs @@ -322,6 +322,16 @@ pub static ref SCHEMA_ATTR_OAUTH2_RS_ORIGIN: SchemaAttribute = SchemaAttribute { ..Default::default() }; +pub static ref SCHEMA_ATTR_OAUTH2_RS_ORIGIN_DL7: SchemaAttribute = SchemaAttribute { + uuid: UUID_SCHEMA_ATTR_OAUTH2_RS_ORIGIN, + name: Attribute::OAuth2RsOrigin.into(), + description: "The origin domain of an OAuth2 client".to_string(), + + syntax: SyntaxType::Url, + multivalue: true, + ..Default::default() +}; + pub static ref SCHEMA_ATTR_OAUTH2_RS_ORIGIN_LANDING: SchemaAttribute = SchemaAttribute { uuid: UUID_SCHEMA_ATTR_OAUTH2_RS_ORIGIN_LANDING, name: Attribute::OAuth2RsOriginLanding.into(), @@ -1235,6 +1245,31 @@ pub static ref SCHEMA_CLASS_OAUTH2_RS_DL5: SchemaClass = SchemaClass { ..Default::default() }; +pub static ref SCHEMA_CLASS_OAUTH2_RS_DL7: SchemaClass = SchemaClass { + uuid: UUID_SCHEMA_CLASS_OAUTH2_RS, + name: EntryClass::OAuth2ResourceServer.into(), + description: "The class representing a configured OAuth2 Client".to_string(), + + systemmay: vec![ + Attribute::Description.into(), + Attribute::OAuth2RsScopeMap.into(), + Attribute::OAuth2RsSupScopeMap.into(), + Attribute::Rs256PrivateKeyDer.into(), + Attribute::OAuth2JwtLegacyCryptoEnable.into(), + Attribute::OAuth2PreferShortUsername.into(), + Attribute::Image.into(), + Attribute::OAuth2RsClaimMap.into(), + Attribute::OAuth2Session.into(), + Attribute::OAuth2RsOrigin.into(), + ], + systemmust: vec![ + Attribute::OAuth2RsOriginLanding.into(), + Attribute::OAuth2RsTokenKey.into(), + Attribute::Es256PrivateKeyDer.into(), + ], + ..Default::default() +}; + pub static ref SCHEMA_CLASS_OAUTH2_RS_BASIC: SchemaClass = SchemaClass { uuid: UUID_SCHEMA_CLASS_OAUTH2_RS_BASIC, name: EntryClass::OAuth2ResourceServerBasic.into(), @@ -1249,7 +1284,7 @@ pub static ref SCHEMA_CLASS_OAUTH2_RS_BASIC: SchemaClass = SchemaClass { pub static ref SCHEMA_CLASS_OAUTH2_RS_BASIC_DL5: SchemaClass = SchemaClass { uuid: UUID_SCHEMA_CLASS_OAUTH2_RS_BASIC, name: EntryClass::OAuth2ResourceServerBasic.into(), - description: "The class representing a configured Oauth2 Resource Server authenticated with http basic authentication".to_string(), + description: "The class representing a configured OAuth2 client authenticated with HTTP basic authentication".to_string(), systemmay: vec![ Attribute::OAuth2AllowInsecureClientDisablePkce.into(), @@ -1272,7 +1307,7 @@ pub static ref SCHEMA_CLASS_OAUTH2_RS_PUBLIC: SchemaClass = SchemaClass { pub static ref SCHEMA_CLASS_OAUTH2_RS_PUBLIC_DL4: SchemaClass = SchemaClass { uuid: UUID_SCHEMA_CLASS_OAUTH2_RS_PUBLIC, name: EntryClass::OAuth2ResourceServerPublic.into(), - description: "The class representing a configured Oauth2 Resource Server with public clients and pkce verification".to_string(), + description: "The class representing a configured Public OAuth2 Client with PKCE verification".to_string(), systemmay: vec![Attribute::OAuth2AllowLocalhostRedirect.into()], systemexcludes: vec![EntryClass::OAuth2ResourceServerBasic.into()], diff --git a/server/lib/src/idm/applinks.rs b/server/lib/src/idm/applinks.rs index 8297a9b8c..0eb9d4bdc 100644 --- a/server/lib/src/idm/applinks.rs +++ b/server/lib/src/idm/applinks.rs @@ -45,7 +45,6 @@ impl<'a> IdmServerProxyReadTransaction<'a> { let redirect_url = entry .get_ava_single_url(Attribute::OAuth2RsOriginLanding) - .or_else(|| entry.get_ava_single_url(Attribute::OAuth2RsOrigin)) .cloned()?; let name = entry diff --git a/server/lib/src/idm/oauth2.rs b/server/lib/src/idm/oauth2.rs index 1aea52210..22e545bef 100644 --- a/server/lib/src/idm/oauth2.rs +++ b/server/lib/src/idm/oauth2.rs @@ -12,6 +12,8 @@ use std::str::FromStr; use std::sync::Arc; use std::time::Duration; +use hashbrown::HashSet; + use base64::{engine::general_purpose, Engine as _}; use base64urlsafedata::Base64UrlSafeData; @@ -268,8 +270,11 @@ pub struct Oauth2RS { name: String, displayname: String, uuid: Uuid, - origin: Origin, - origin_https: bool, + + origins: HashSet, + opaque_origins: HashSet, + origin_https_required: bool, + claim_map: BTreeMap>, scope_maps: BTreeMap>, sup_scope_maps: BTreeMap>, @@ -301,7 +306,8 @@ impl std::fmt::Debug for Oauth2RS { .field("displayname", &self.displayname) .field("uuid", &self.uuid) .field("type", &self.type_) - .field("origin", &self.origin) + .field("origins", &self.origins) + .field("opaque_origins", &self.opaque_origins) .field("scope_maps", &self.scope_maps) .field("sup_scope_maps", &self.sup_scope_maps) .field("claim_map", &self.claim_map) @@ -418,18 +424,46 @@ impl<'a> Oauth2ResourceServersWriteTransaction<'a> { .map(str::to_string) .ok_or(OperationError::InvalidValueState)?; - let (origin, origin_https) = ent - .get_ava_single_url(Attribute::OAuth2RsOrigin) - .map(|url| (url.origin(), url.scheme() == "https")) + + // Setup the landing uri and its implied origin, as well as + // the supplemental origins. + let landing_url = ent + .get_ava_single_url(Attribute::OAuth2RsOriginLanding) + .cloned() .ok_or(OperationError::InvalidValueState)?; - let landing_valid = ent - .get_ava_single_url(Attribute::OAuth2RsOriginLanding) - .map(|url| url.origin() == origin). - unwrap_or(true); + let maybe_extra_origins = ent.get_ava_set(Attribute::OAuth2RsOrigin).and_then(|s| s.as_url_set()); - if !landing_valid { - warn!("{} has a landing page that is not part of origin. May be invalid.", name); + let len_uris = maybe_extra_origins.map(|s| s.len() + 1).unwrap_or(1); + + // The reason we have to allocate this is that we need to do some processing on these + // urls to determine if they are opaque or not. + let mut redirect_uris = Vec::with_capacity(len_uris); + + redirect_uris.push(landing_url); + if let Some(extra_origins) = maybe_extra_origins { + for x_origin in extra_origins { + redirect_uris.push(x_origin.clone()); + } + } + + // Now redirect_uris has the full set of the landing uri and the other uris + // that may or may not be an opaque origin. We need to split these up now. + + let mut origins = HashSet::with_capacity(len_uris); + let mut opaque_origins = HashSet::with_capacity(len_uris); + let mut origin_https_required = false; + + for uri in redirect_uris.into_iter() { + // Given the presence of a single https url, then all other urls must be https. + if uri.scheme() == "https" { + origin_https_required = true; + origins.insert(uri.origin()); + } else if uri.scheme() == "http" { + origins.insert(uri.origin()); + } else { + opaque_origins.insert(uri); + } } let token_fernet = ent @@ -604,8 +638,9 @@ impl<'a> Oauth2ResourceServersWriteTransaction<'a> { name, displayname, uuid, - origin, - origin_https, + origins, + opaque_origins, + origin_https_required, scope_maps, sup_scope_maps, client_scopes, @@ -1623,23 +1658,27 @@ impl<'a> IdmServerProxyReadTransaction<'a> { .unwrap_or_default(); // redirect_uri must be part of the client_id origin, unless the client is public and then it MAY - // be localhost. - if !(auth_req.redirect_uri.origin() == o2rs.origin + // be localhost exempting it from this check and enforcement. + if !(o2rs.origins.contains(&auth_req.redirect_uri.origin()) + || o2rs.opaque_origins.contains(&auth_req.redirect_uri) || (allow_localhost_redirect && localhost_redirect)) { - admin_warn!( - origin = ?o2rs.origin, - "Invalid OAuth2 redirect_uri (must be related to origin {:?}) - got {:?}", - o2rs.origin, + warn!( + "Invalid OAuth2 redirect_uri (must be related to origin) - got {:?}", auth_req.redirect_uri.origin() ); return Err(Oauth2Error::InvalidOrigin); } - if !localhost_redirect && o2rs.origin_https && auth_req.redirect_uri.scheme() != "https" { + // We have to specifically match on http here because non-http origins may be exempt from this + // enforcement. + if !localhost_redirect + && o2rs.origin_https_required + && auth_req.redirect_uri.scheme() == "http" + { admin_warn!( - origin = ?o2rs.origin, - "Invalid OAuth2 redirect_uri (must be https for secure origin) - got {:?}", auth_req.redirect_uri.scheme() + "Invalid OAuth2 redirect_uri (must be https for secure origin) - got {:?}", + auth_req.redirect_uri.scheme() ); return Err(Oauth2Error::InvalidOrigin); } @@ -1707,6 +1746,7 @@ impl<'a> IdmServerProxyReadTransaction<'a> { .split_ascii_whitespace() .map(str::to_string) .collect(); + if req_scopes.is_empty() { admin_error!("Invalid OAuth2 request - must contain at least one requested scope"); return Err(Oauth2Error::InvalidRequest); @@ -1782,6 +1822,8 @@ impl<'a> IdmServerProxyReadTransaction<'a> { let consent_previously_granted = if let Some(consent_scopes) = ident.get_oauth2_consent_scopes(o2rs.uuid) { + trace!(?granted_scopes); + trace!(?consent_scopes); granted_scopes.eq(consent_scopes) } else { false @@ -1790,11 +1832,14 @@ impl<'a> IdmServerProxyReadTransaction<'a> { let session_id = ident.get_session_id(); if consent_previously_granted { - let pretty_scopes: Vec = granted_scopes.iter().map(|s| s.to_owned()).collect(); - admin_info!( - "User has previously consented, permitting with scopes: {}", - pretty_scopes.join(",") - ); + if event_enabled!(tracing::Level::DEBUG) { + let pretty_scopes: Vec = + granted_scopes.iter().map(|s| s.to_owned()).collect(); + debug!( + "User has previously consented, permitting with scopes: {}", + pretty_scopes.join(",") + ); + } // Setup for the permit success let xchg_code = TokenExchangeCode { @@ -2711,9 +2756,18 @@ mod tests { Value::new_utf8s("test_resource_server") ), ( - Attribute::OAuth2RsOrigin, + Attribute::OAuth2RsOriginLanding, Value::new_url_s("https://demo.example.com").unwrap() ), + // Supplemental origins + ( + Attribute::OAuth2RsOrigin, + Value::new_url_s("https://portal.example.com").unwrap() + ), + ( + Attribute::OAuth2RsOrigin, + Value::new_url_s("app://cheese").unwrap() + ), // System admins ( Attribute::OAuth2RsScopeMap, @@ -2863,7 +2917,7 @@ mod tests { Value::new_utf8s("test_resource_server") ), ( - Attribute::OAuth2RsOrigin, + Attribute::OAuth2RsOriginLanding, Value::new_url_s("https://demo.example.com").unwrap() ), // System admins @@ -3535,6 +3589,145 @@ mod tests { assert!(idms_prox_write.commit().is_ok()); } + #[idm_test] + async fn test_idm_oauth2_supplemental_origin_redirect( + idms: &IdmServer, + _idms_delayed: &mut IdmServerDelayed, + ) { + let ct = Duration::from_secs(TEST_CURRENT_TIME); + let (secret, uat, ident, _) = + setup_oauth2_resource_server_basic(idms, ct, true, false, false).await; + + let idms_prox_read = idms.proxy_read().await; + + // == Setup the authorisation request + let (code_verifier, code_challenge) = create_code_verifier!("Whar Garble"); + + let auth_req = AuthorisationRequest { + response_type: "code".to_string(), + client_id: "test_resource_server".to_string(), + state: "123".to_string(), + pkce_request: Some(PkceRequest { + code_challenge: code_challenge.clone().into(), + code_challenge_method: CodeChallengeMethod::S256, + }), + redirect_uri: Url::parse("https://portal.example.com/oauth2/result").unwrap(), + scope: OAUTH2_SCOPE_OPENID.to_string(), + nonce: Some("abcdef".to_string()), + oidc_ext: Default::default(), + unknown_keys: Default::default(), + }; + + let consent_request = idms_prox_read + .check_oauth2_authorisation(&ident, &auth_req, ct) + .expect("OAuth2 authorisation failed"); + + trace!(?consent_request); + + // Should be in the consent phase; + let consent_token = + if let AuthoriseResponse::ConsentRequested { consent_token, .. } = consent_request { + consent_token + } else { + unreachable!(); + }; + + // == Manually submit the consent token to the permit for the permit_success + drop(idms_prox_read); + let mut idms_prox_write = idms.proxy_write(ct).await; + + let permit_success = idms_prox_write + .check_oauth2_authorise_permit(&ident, &consent_token, ct) + .expect("Failed to perform OAuth2 permit"); + + // Check we are reflecting the CSRF properly. + assert!(permit_success.state == "123"); + + // == Submit the token exchange code. + // ⚠️ This is where we submit a different origin! + let token_req = AccessTokenRequest { + grant_type: GrantTypeReq::AuthorizationCode { + code: permit_success.code, + redirect_uri: Url::parse("https://portal.example.com/oauth2/result").unwrap(), + // From the first step. + code_verifier: code_verifier.clone(), + }, + client_id: Some("test_resource_server".to_string()), + client_secret: Some(secret.clone()), + }; + + let token_response = idms_prox_write + .check_oauth2_token_exchange(&ClientAuthInfo::none(), &token_req, ct) + .expect("Failed to perform OAuth2 token exchange"); + + // 🎉 We got a token! In the future we can then check introspection from this point. + assert!(token_response.token_type == "Bearer"); + + assert!(idms_prox_write.commit().is_ok()); + + // ============================================================================ + // Now repeat the test with the app url. + + let mut idms_prox_read = idms.proxy_read().await; + + // Reload the ident since it pins an entry in memory. + let ident = idms_prox_read + .process_uat_to_identity(&uat, ct, Source::Internal) + .expect("Unable to process uat"); + + let auth_req = AuthorisationRequest { + response_type: "code".to_string(), + client_id: "test_resource_server".to_string(), + state: "123".to_string(), + pkce_request: Some(PkceRequest { + code_challenge: code_challenge.into(), + code_challenge_method: CodeChallengeMethod::S256, + }), + redirect_uri: Url::parse("app://cheese").unwrap(), + scope: OAUTH2_SCOPE_OPENID.to_string(), + nonce: Some("abcdef".to_string()), + oidc_ext: Default::default(), + unknown_keys: Default::default(), + }; + + let consent_request = idms_prox_read + .check_oauth2_authorisation(&ident, &auth_req, ct) + .expect("OAuth2 authorisation failed"); + + trace!(?consent_request); + + let AuthoriseResponse::Permitted(permit_success) = consent_request else { + unreachable!(); + }; + + // == Manually submit the consent token to the permit for the permit_success + // Check we are reflecting the CSRF properly. + assert!(permit_success.state == "123"); + + // == Submit the token exchange code. + // ⚠️ This is where we submit a different origin! + let token_req = AccessTokenRequest { + grant_type: GrantTypeReq::AuthorizationCode { + code: permit_success.code, + redirect_uri: Url::parse("app://cheese").unwrap(), + // From the first step. + code_verifier, + }, + client_id: Some("test_resource_server".to_string()), + client_secret: Some(secret), + }; + + drop(idms_prox_read); + let mut idms_prox_write = idms.proxy_write(ct).await; + + let token_response = idms_prox_write + .check_oauth2_token_exchange(&ClientAuthInfo::none(), &token_req, ct) + .expect("Failed to perform OAuth2 token exchange"); + + // 🎉 We got a token! In the future we can then check introspection from this point. + assert!(token_response.token_type == "Bearer"); + } + #[idm_test] async fn test_idm_oauth2_token_introspect( idms: &IdmServer, diff --git a/server/lib/src/plugins/jwskeygen.rs b/server/lib/src/plugins/jwskeygen.rs index f678a2e18..848a8a42f 100644 --- a/server/lib/src/plugins/jwskeygen.rs +++ b/server/lib/src/plugins/jwskeygen.rs @@ -134,7 +134,7 @@ mod tests { ), (Attribute::Name, Value::new_iname("test_resource_server")), ( - Attribute::OAuth2RsOrigin, + Attribute::OAuth2RsOriginLanding, Value::new_url_s("https://demo.example.com").unwrap() ), ( @@ -186,7 +186,7 @@ mod tests { Value::new_utf8s("test_resource_server") ), ( - Attribute::OAuth2RsOrigin, + Attribute::OAuth2RsOriginLanding, Value::new_url_s("https://demo.example.com").unwrap() ), ( diff --git a/server/lib/src/plugins/refint.rs b/server/lib/src/plugins/refint.rs index 4b458276b..db35c2384 100644 --- a/server/lib/src/plugins/refint.rs +++ b/server/lib/src/plugins/refint.rs @@ -1024,7 +1024,7 @@ mod tests { Value::new_utf8s("test_resource_server") ), ( - Attribute::OAuth2RsOrigin, + Attribute::OAuth2RsOriginLanding, Value::new_url_s("https://demo.example.com").unwrap() ), ( @@ -1112,7 +1112,7 @@ mod tests { Value::new_utf8s("test_resource_server") ), ( - Attribute::OAuth2RsOrigin, + Attribute::OAuth2RsOriginLanding, Value::new_url_s("https://demo.example.com").unwrap() ), // System admins @@ -1342,7 +1342,7 @@ mod tests { Value::new_utf8s("test_resource_server") ), ( - Attribute::OAuth2RsOrigin, + Attribute::OAuth2RsOriginLanding, Value::new_url_s("https://demo.example.com").unwrap() ), ( diff --git a/server/lib/src/plugins/session.rs b/server/lib/src/plugins/session.rs index f501b7b9b..040834368 100644 --- a/server/lib/src/plugins/session.rs +++ b/server/lib/src/plugins/session.rs @@ -360,7 +360,7 @@ mod tests { Value::new_utf8s("test_resource_server") ), ( - Attribute::OAuth2RsOrigin, + Attribute::OAuth2RsOriginLanding, Value::new_url_s("https://demo.example.com").unwrap() ), // System admins @@ -533,7 +533,7 @@ mod tests { Value::new_utf8s("test_resource_server") ), ( - Attribute::OAuth2RsOrigin, + Attribute::OAuth2RsOriginLanding, Value::new_url_s("https://demo.example.com").unwrap() ), // System admins @@ -699,7 +699,7 @@ mod tests { Value::new_utf8s("test_resource_server") ), ( - Attribute::OAuth2RsOrigin, + Attribute::OAuth2RsOriginLanding, Value::new_url_s("https://demo.example.com").unwrap() ), // System admins diff --git a/server/lib/src/server/access/mod.rs b/server/lib/src/server/access/mod.rs index ff8656879..6dc7aea2a 100644 --- a/server/lib/src/server/access/mod.rs +++ b/server/lib/src/server/access/mod.rs @@ -2695,9 +2695,13 @@ mod tests { Value::new_utf8s("test_resource_server") ), ( - Attribute::OAuth2RsOrigin, + Attribute::OAuth2RsOriginLanding, Value::new_url_s("https://demo.example.com").unwrap() ), + ( + Attribute::OAuth2RsOrigin, + Value::new_url_s("app://hidden").unwrap() + ), ( Attribute::OAuth2RsScopeMap, Value::new_oauthscopemap(UUID_TEST_GROUP_1, btreeset!["groups".to_string()]) @@ -2737,7 +2741,7 @@ mod tests { Value::new_utf8s("test_resource_server") ), ( - Attribute::OAuth2RsOrigin, + Attribute::OAuth2RsOriginLanding, Value::new_url_s("https://demo.example.com").unwrap() ) ) @@ -2760,9 +2764,13 @@ mod tests { Value::new_utf8s("second_resource_server") ), ( - Attribute::OAuth2RsOrigin, + Attribute::OAuth2RsOriginLanding, Value::new_url_s("https://noaccess.example.com").unwrap() ), + ( + Attribute::OAuth2RsOrigin, + Value::new_url_s("app://hidden").unwrap() + ), ( Attribute::OAuth2RsScopeMap, Value::new_oauthscopemap(UUID_SYSTEM_ADMINS, btreeset!["groups".to_string()]) diff --git a/server/lib/src/server/access/search.rs b/server/lib/src/server/access/search.rs index 50fc43c1f..47dc2373a 100644 --- a/server/lib/src/server/access/search.rs +++ b/server/lib/src/server/access/search.rs @@ -193,7 +193,6 @@ fn search_oauth2_filter_entry<'a>( Attribute::DisplayName.as_ref(), Attribute::Uuid.as_ref(), Attribute::Name.as_ref(), - Attribute::OAuth2RsOrigin.as_ref(), Attribute::OAuth2RsOriginLanding.as_ref(), Attribute::Image.as_ref() )); diff --git a/server/lib/src/server/migrations.rs b/server/lib/src/server/migrations.rs index 6796f2d30..2110fc042 100644 --- a/server/lib/src/server/migrations.rs +++ b/server/lib/src/server/migrations.rs @@ -653,6 +653,41 @@ impl<'a> QueryServerWriteTransaction<'a> { // =========== Apply changes ============== + // For each oauth2 client, if it is missing a landing page then we clone the origin + // into landing. This is because previously we implied the landing to be origin if + // unset, but now landing is the primary url and implies an origin. + let filter = filter!(f_and!([ + f_eq(Attribute::Class, EntryClass::OAuth2ResourceServer.into()), + f_pres(Attribute::OAuth2RsOrigin), + f_andnot(f_pres(Attribute::OAuth2RsOriginLanding)), + ])); + + let pre_candidates = self.internal_search(filter).map_err(|err| { + error!(?err, "migrate_domain_6_to_7 internal search failure"); + err + })?; + + let modset: Vec<_> = pre_candidates + .into_iter() + .filter_map(|ent| { + ent.get_ava_single_url(Attribute::OAuth2RsOrigin) + .map(|origin_url| { + // Copy the origin url to the landing. + let modlist = vec![Modify::Present( + Attribute::OAuth2RsOriginLanding.into(), + Value::Url(origin_url.clone()), + )]; + + (ent.get_uuid(), ModifyList::new_list(modlist)) + }) + }) + .collect(); + + // If there is nothing, we don't need to do anything. + if !modset.is_empty() { + self.internal_batch_modify(modset.into_iter())?; + } + // Do this before schema change since domain info has cookie key // as may at this point. // @@ -681,10 +716,12 @@ impl<'a> QueryServerWriteTransaction<'a> { SCHEMA_ATTR_DOMAIN_DEVELOPMENT_TAINT_DL7.clone().into(), SCHEMA_ATTR_REFERS_DL7.clone().into(), SCHEMA_ATTR_CERTIFICATE_DL7.clone().into(), + SCHEMA_ATTR_OAUTH2_RS_ORIGIN_DL7.clone().into(), SCHEMA_CLASS_DOMAIN_INFO_DL7.clone().into(), SCHEMA_CLASS_SERVICE_ACCOUNT_DL7.clone().into(), SCHEMA_CLASS_SYNC_ACCOUNT_DL7.clone().into(), SCHEMA_CLASS_CLIENT_CERTIFICATE_DL7.clone().into(), + SCHEMA_CLASS_OAUTH2_RS_DL7.clone().into(), ]; idm_schema_classes @@ -1239,6 +1276,36 @@ mod tests { assert_eq!(db_domain_version, DOMAIN_LEVEL_6); + // Create an oauth2 client that doesn't have a landing url set. + let oauth2_client_uuid = Uuid::new_v4(); + + let ea: Entry = entry_init!( + (Attribute::Class, EntryClass::Object.to_value()), + (Attribute::Class, EntryClass::Account.to_value()), + (Attribute::Uuid, Value::Uuid(oauth2_client_uuid)), + ( + Attribute::Class, + EntryClass::OAuth2ResourceServer.to_value() + ), + ( + Attribute::Class, + EntryClass::OAuth2ResourceServerPublic.to_value() + ), + (Attribute::Name, Value::new_iname("test_resource_server")), + ( + Attribute::DisplayName, + Value::new_utf8s("test_resource_server") + ), + ( + Attribute::OAuth2RsOrigin, + Value::new_url_s("https://demo.example.com").unwrap() + ) + ); + + write_txn + .internal_create(vec![ea]) + .expect("Unable to create oauth2 client"); + // per migration verification. let domain_entry = write_txn .internal_search_uuid(UUID_DOMAIN_INFO) @@ -1267,6 +1334,21 @@ mod tests { assert!(!domain_entry.attribute_pres(Attribute::PrivateCookieKey)); + let oauth2_entry = write_txn + .internal_search_uuid(oauth2_client_uuid) + .expect("Unable to access oauth2 client entry"); + + let origin = oauth2_entry + .get_ava_single_url(Attribute::OAuth2RsOrigin) + .expect("Unable to access oauth2 client origin"); + + // The origin should have been cloned to the landing. + let landing = oauth2_entry + .get_ava_single_url(Attribute::OAuth2RsOriginLanding) + .expect("Unable to access oauth2 client landing"); + + assert_eq!(origin, landing); + write_txn.commit().expect("Unable to commit"); } } diff --git a/server/testkit/tests/oauth2_test.rs b/server/testkit/tests/oauth2_test.rs index a8f5a5547..ad09744b0 100644 --- a/server/testkit/tests/oauth2_test.rs +++ b/server/testkit/tests/oauth2_test.rs @@ -87,7 +87,7 @@ async fn test_oauth2_openid_basic_flow(rsclient: KanidmClient) { .expect("Failed to configure account password"); rsclient - .idm_oauth2_rs_update("test_integration", None, None, None, None, true, true, true) + .idm_oauth2_rs_update("test_integration", None, None, None, true, true, true) .await .expect("Failed to update oauth2 config"); @@ -517,7 +517,7 @@ async fn test_oauth2_openid_public_flow(rsclient: KanidmClient) { .expect("Failed to configure account password"); rsclient - .idm_oauth2_rs_update("test_integration", None, None, None, None, true, true, true) + .idm_oauth2_rs_update("test_integration", None, None, None, true, true, true) .await .expect("Failed to update oauth2 config"); diff --git a/server/testkit/tests/proto_v1_test.rs b/server/testkit/tests/proto_v1_test.rs index 98cef04af..5a5019e5d 100644 --- a/server/testkit/tests/proto_v1_test.rs +++ b/server/testkit/tests/proto_v1_test.rs @@ -878,7 +878,6 @@ async fn test_server_rest_oauth2_basic_lifecycle(rsclient: KanidmClient) { None, Some("Test Integration"), Some("https://new_demo.example.com"), - None, true, true, true, diff --git a/tools/cli/src/cli/oauth2.rs b/tools/cli/src/cli/oauth2.rs index 5cd01d3c6..37cb33b63 100644 --- a/tools/cli/src/cli/oauth2.rs +++ b/tools/cli/src/cli/oauth2.rs @@ -34,8 +34,9 @@ impl Oauth2Opt { | Oauth2Opt::UpdateClaimMapJoin { copt, .. } | Oauth2Opt::DeleteClaimMap { copt, .. } | Oauth2Opt::EnablePublicLocalhost { copt, .. } - | Oauth2Opt::DisablePublicLocalhost { copt, .. } => copt.debug, - Oauth2Opt::SetOrigin { nopt, .. } => nopt.copt.debug, + | Oauth2Opt::DisablePublicLocalhost { copt, .. } + | Oauth2Opt::AddOrigin { copt, .. } + | Oauth2Opt::RemoveOrigin { copt, .. } => copt.debug, } } @@ -160,16 +161,7 @@ impl Oauth2Opt { Oauth2Opt::ResetSecrets(cbopt) => { let client = cbopt.copt.to_client(OpType::Write).await; match client - .idm_oauth2_rs_update( - cbopt.name.as_str(), - None, - None, - None, - None, - true, - true, - true, - ) + .idm_oauth2_rs_update(cbopt.name.as_str(), None, None, None, true, true, true) .await { Ok(_) => println!("Success"), @@ -210,7 +202,6 @@ impl Oauth2Opt { None, Some(cbopt.displayname.as_str()), None, - None, false, false, false, @@ -229,7 +220,6 @@ impl Oauth2Opt { Some(name.as_str()), None, None, - None, false, false, false, @@ -247,8 +237,7 @@ impl Oauth2Opt { nopt.name.as_str(), None, None, - None, - Some(url), + Some(url.as_str()), false, false, false, @@ -313,23 +302,19 @@ impl Oauth2Opt { Err(e) => handle_client_error(e, nopt.copt.output_mode), } } - Oauth2Opt::SetOrigin { nopt, origin } => { - let client = nopt.copt.to_client(OpType::Write).await; - match client - .idm_oauth2_rs_update( - &nopt.name, - None, - None, - Some(origin), - None, - false, - false, - false, - ) - .await - { + + Oauth2Opt::AddOrigin { name, origin, copt } => { + let client = copt.to_client(OpType::Write).await; + match client.idm_oauth2_client_add_origin(name, origin).await { Ok(_) => println!("Success"), - Err(e) => handle_client_error(e, nopt.copt.output_mode), + Err(e) => handle_client_error(e, copt.output_mode), + } + } + Oauth2Opt::RemoveOrigin { name, origin, copt } => { + let client = copt.to_client(OpType::Write).await; + match client.idm_oauth2_client_remove_origin(name, origin).await { + Ok(_) => println!("Success"), + Err(e) => handle_client_error(e, copt.output_mode), } } Oauth2Opt::UpdateClaimMap { diff --git a/tools/cli/src/opt/kanidm.rs b/tools/cli/src/opt/kanidm.rs index 8de8c4c86..dddbeeecd 100644 --- a/tools/cli/src/opt/kanidm.rs +++ b/tools/cli/src/opt/kanidm.rs @@ -1038,15 +1038,39 @@ pub enum Oauth2Opt { #[clap(name = "newname")] name: String, }, - /// When redirecting from the Kanidm Apps Listing page, some linked applications may need to - /// land on a specific page to trigger oauth2/oidc interactions. + + /// The landing URL is the default origin of the OAuth2 client. Additionally, this landing + /// URL is the target when Kanidm redirects the user from the apps listing page. #[clap(name = "set-landing-url")] SetLandingUrl { #[clap(flatten)] nopt: Named, #[clap(name = "landing-url")] - url: String, + url: Url, }, + + /// Add a supplemental origin as a redirection target. For example a phone app + /// may use a redirect URL such as `app://my-cool-app` to trigger a native + /// redirection event out of a browser. + #[clap(name = "add-origin")] + AddOrigin { + name: String, + #[clap(name = "origin-url")] + origin: Url, + #[clap(flatten)] + copt: CommonOpt, + }, + + /// Remove a supplemental origin from the OAuth2 client configuration. + #[clap(name = "remove-origin")] + RemoveOrigin { + name: String, + #[clap(name = "origin-url")] + origin: Url, + #[clap(flatten)] + copt: CommonOpt, + }, + #[clap(name = "enable-pkce")] /// Enable PKCE on this oauth2 client. This defaults to being enabled. EnablePkce(Named), @@ -1084,14 +1108,6 @@ pub enum Oauth2Opt { #[clap(name = "prefer-spn-username")] /// Use the 'spn' attribute instead of 'name' for the preferred_username PreferSPNUsername(Named), - /// Set the origin of an oauth2 client - #[clap(name = "set-origin")] - SetOrigin { - #[clap(flatten)] - nopt: Named, - #[clap(name = "origin")] - origin: String, - }, } #[derive(Args, Debug)]