From cd016bd3f2076c2d5b17852b9769f2690c66457c Mon Sep 17 00:00:00 2001 From: sebadob Date: Tue, 19 Nov 2024 11:18:32 +0100 Subject: [PATCH] fix: clients not being updated on custom scope update / delete (#622) * fix clients not being updated on custom scope update / delete * include client update check in tests --- src/bin/tests/handler_scopes.rs | 84 +++++++++++++++++++++++++++++--- src/models/src/entity/clients.rs | 5 +- src/models/src/entity/scopes.rs | 3 ++ 3 files changed, 84 insertions(+), 8 deletions(-) diff --git a/src/bin/tests/handler_scopes.rs b/src/bin/tests/handler_scopes.rs index 028cae0d..4b510042 100644 --- a/src/bin/tests/handler_scopes.rs +++ b/src/bin/tests/handler_scopes.rs @@ -1,5 +1,6 @@ use crate::common::{get_auth_headers, get_backend_url}; use pretty_assertions::assert_eq; +use rauthy_api_types::clients::{ClientResponse, UpdateClientRequest}; use rauthy_api_types::scopes::ScopeRequest; use rauthy_models::entity::scopes::Scope; use std::error::Error; @@ -10,9 +11,10 @@ mod common; async fn test_scopes() -> Result<(), Box> { let auth_headers = get_auth_headers().await?; let backend_url = get_backend_url(); + let client = reqwest::Client::new(); let url = format!("{}/scopes", backend_url); - let res = reqwest::Client::new() + let res = client .get(&url) .headers(auth_headers.clone()) .send() @@ -28,7 +30,7 @@ async fn test_scopes() -> Result<(), Box> { attr_include_access: None, attr_include_id: None, }; - let res = reqwest::Client::new() + let res = client .post(&url) .headers(auth_headers.clone()) .json(&new_scope) @@ -37,7 +39,55 @@ async fn test_scopes() -> Result<(), Box> { assert_eq!(res.status(), 200); let scope = res.json::().await?; - assert_eq!(scope.name, "scope123"); + assert_eq!(scope.name, new_scope.scope); + + // link the scope to a client and check + let url_client = format!("{}/clients/init_client", backend_url); + let res = client + .get(&url_client) + .headers(auth_headers.clone()) + .send() + .await?; + assert_eq!(res.status(), 200); + let init_client = res.json::().await?; + assert!(!init_client.scopes.contains(&new_scope.scope)); + assert!(!init_client.default_scopes.contains(&new_scope.scope)); + + let mut scopes = init_client.scopes; + scopes.push(new_scope.scope.clone()); + let mut default_scopes = init_client.default_scopes; + default_scopes.push(new_scope.scope.clone()); + + let update_client = UpdateClientRequest { + id: init_client.id, + name: init_client.name, + confidential: init_client.confidential, + redirect_uris: init_client.redirect_uris, + post_logout_redirect_uris: init_client.post_logout_redirect_uris, + allowed_origins: Some(vec!["http://localhost:8080".to_string()]), + enabled: init_client.enabled, + flows_enabled: init_client.flows_enabled, + access_token_alg: init_client.access_token_alg, + id_token_alg: init_client.id_token_alg, + auth_code_lifetime: init_client.auth_code_lifetime, + access_token_lifetime: init_client.access_token_lifetime, + scopes, + default_scopes, + challenges: init_client.challenges, + force_mfa: init_client.force_mfa, + client_uri: init_client.client_uri, + contacts: init_client.contacts, + }; + let res = client + .put(&url_client) + .headers(auth_headers.clone()) + .json(&update_client) + .send() + .await?; + assert_eq!(res.status(), 200); + let init_client = res.json::().await?; + assert!(init_client.scopes.contains(&new_scope.scope)); + assert!(init_client.default_scopes.contains(&new_scope.scope)); // modify the scope let upd_scope = ScopeRequest { @@ -46,7 +96,7 @@ async fn test_scopes() -> Result<(), Box> { attr_include_id: None, }; let url_name = format!("{}/{}", url, scope.id); - let res = reqwest::Client::new() + let res = client .put(&url_name) .headers(auth_headers.clone()) .json(&upd_scope) @@ -57,9 +107,20 @@ async fn test_scopes() -> Result<(), Box> { let upd_scp = res.json::().await?; assert_eq!(upd_scope.scope, upd_scp.name); + // check the linked client update + let res = client + .get(&url_client) + .headers(auth_headers.clone()) + .send() + .await?; + assert_eq!(res.status(), 200); + let init_client = res.json::().await?; + assert!(init_client.scopes.contains(&upd_scp.name)); + assert!(init_client.default_scopes.contains(&upd_scp.name)); + // delete the scope let url_del = format!("{}/{}", url, upd_scp.id); - let res = reqwest::Client::new() + let res = client .delete(&url_del) .headers(auth_headers.clone()) .send() @@ -67,7 +128,7 @@ async fn test_scopes() -> Result<(), Box> { assert_eq!(res.status(), 200); // verify it is gone - let res = reqwest::Client::new() + let res = client .get(&url) .headers(auth_headers.clone()) .send() @@ -77,5 +138,16 @@ async fn test_scopes() -> Result<(), Box> { let scopes = res.json::>().await?; assert_eq!(scopes.len(), 6); + // check the linked client deletion + let res = client + .get(&url_client) + .headers(auth_headers.clone()) + .send() + .await?; + assert_eq!(res.status(), 200); + let init_client = res.json::().await?; + assert!(!init_client.scopes.contains(&upd_scp.name)); + assert!(!init_client.default_scopes.contains(&upd_scp.name)); + Ok(()) } diff --git a/src/models/src/entity/clients.rs b/src/models/src/entity/clients.rs index 6938969f..1141cebd 100644 --- a/src/models/src/entity/clients.rs +++ b/src/models/src/entity/clients.rs @@ -404,18 +404,19 @@ VALUES ($1, $2, $3, $4)"#, Ok(slf) } + /// This is an expensive query using `LIKE`, only use when necessary. pub async fn find_with_scope(scope_name: &str) -> Result, ErrorResponse> { let like = format!("%{scope_name}%"); let clients = if is_hiqlite() { DB::client() .query_as( - "SELECT * FROM clients WHERE scopes = $1 OR default_scopes = $1", + "SELECT * FROM clients WHERE scopes LIKE $1 OR default_scopes LIKE $1", params!(like), ) .await? } else { - sqlx::query_as("SELECT * FROM clients WHERE scopes = $1 OR default_scopes = $1") + sqlx::query_as("SELECT * FROM clients WHERE scopes LIKE $1 OR default_scopes LIKE $1") .bind(like) .fetch_all(DB::conn()) .await? diff --git a/src/models/src/entity/scopes.rs b/src/models/src/entity/scopes.rs index f3b3c052..e2ccdd89 100644 --- a/src/models/src/entity/scopes.rs +++ b/src/models/src/entity/scopes.rs @@ -242,6 +242,7 @@ VALUES ($1, $2, $3, $4)"#, c }) .collect::>(); + debug!("\n\n{:?}\n", clients); Some(clients) } else { None @@ -383,6 +384,7 @@ WHERE id = $4"#, } impl Scope { + #[inline] pub fn clean_up_attrs( req_attrs: Option>, existing_attrs: &HashSet, @@ -414,6 +416,7 @@ impl Scope { /// Returns `true` if the given scope is not a default OIDC scope. /// Note: `groups` is not a default scope, but it will be handled like one for performance /// and efficiency reasons. + #[inline] pub fn is_custom(scope: &str) -> bool { scope != "openid" && scope != "profile" && scope != "email" && scope != "groups" }