From 7da4477573b4a80e93c7bfa74b99e7d929e5d32d Mon Sep 17 00:00:00 2001 From: Matthias Twardawski Date: Mon, 15 Jul 2024 10:55:45 +0200 Subject: [PATCH] issue-149: delete OIDC client once a peer gets deleted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Anna Völker anna.voelker@mercedes-benz.com Co-authored-by: Matthias Twardawski matthias.twardawski@mercedes-benz.com --- Cargo.lock | 1 + opendut-carl/src/actions/peers.rs | 11 ++ opendut-carl/src/grpc/peer_manager.rs | 1 + opendut-util/opendut-auth/Cargo.toml | 1 + .../opendut-auth-tests/src/lib.rs | 106 ++---------------- .../opendut-auth/src/registration/client.rs | 105 ++++++++++++++++- 6 files changed, 128 insertions(+), 97 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d905b4cea..28396a946 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2727,6 +2727,7 @@ dependencies = [ "reqwest", "rstest", "serde", + "serde_json", "shadow-rs", "thiserror", "tokio", diff --git a/opendut-carl/src/actions/peers.rs b/opendut-carl/src/actions/peers.rs index 5ef2d7f69..c571484cd 100644 --- a/opendut-carl/src/actions/peers.rs +++ b/opendut-carl/src/actions/peers.rs @@ -151,6 +151,7 @@ pub struct DeletePeerDescriptorParams { pub resources_manager: ResourcesManagerRef, pub vpn: Vpn, pub peer: PeerId, + pub oidc_registration_client: Option, } #[tracing::instrument(skip(params), level="trace")] @@ -182,6 +183,16 @@ pub async fn delete_peer_descriptor(params: DeletePeerDescriptorParams) -> Resul let peer_name = &peer_descriptor.name; + if let Some(registration_client) = params.oidc_registration_client { + let resource_id = peer_id.into(); + debug!("Deleting OIDC client for peer '{peer_name}' <{peer_id}>."); + let deleted_clients = registration_client.delete_client(resource_id) + .await + .map_err(|cause| DeletePeerDescriptorError::Internal { peer_id, peer_name: Clone::clone(peer_name), cause: cause.to_string() })?; + let deleted_client_ids = deleted_clients.0.into_iter().map(|client| client.client_id).collect::>(); + debug!("Successfully deleted oidc clients for peer '{peer_name}' <{peer_id}>. OIDC client_ids='{}'.", deleted_client_ids.join(",")); + }; + if let Vpn::Enabled { vpn_client } = params.vpn { debug!("Deleting vpn peer <{peer_id}>."); vpn_client.delete_peer(peer_id) diff --git a/opendut-carl/src/grpc/peer_manager.rs b/opendut-carl/src/grpc/peer_manager.rs index c3256e141..c6ea1f1ac 100644 --- a/opendut-carl/src/grpc/peer_manager.rs +++ b/opendut-carl/src/grpc/peer_manager.rs @@ -107,6 +107,7 @@ impl PeerManagerService for PeerManagerFacade { resources_manager: Arc::clone(&self.resources_manager), vpn: Clone::clone(&self.vpn), peer: peer_id, + oidc_registration_client: self.oidc_registration_client.clone(), }).await; match result { diff --git a/opendut-util/opendut-auth/Cargo.toml b/opendut-util/opendut-auth/Cargo.toml index 7413a5eb8..919ba3a15 100644 --- a/opendut-util/opendut-auth/Cargo.toml +++ b/opendut-util/opendut-auth/Cargo.toml @@ -56,6 +56,7 @@ tonic = { workspace = true } tower = { workspace = true, optional = true } tracing = { workspace = true, optional = true } url = { workspace = true } +serde_json = { workspace = true } [dev-dependencies] diff --git a/opendut-util/opendut-auth/opendut-auth-tests/src/lib.rs b/opendut-util/opendut-auth/opendut-auth-tests/src/lib.rs index d3b90f25f..4bca0664b 100644 --- a/opendut-util/opendut-auth/opendut-auth-tests/src/lib.rs +++ b/opendut-util/opendut-auth/opendut-auth-tests/src/lib.rs @@ -3,7 +3,6 @@ use oauth2::{ClientId, ClientSecret, RedirectUrl}; use openidconnect::RegistrationUrl; use pem::Pem; use rstest::fixture; -use serde::Deserialize; use url::Url; use opendut_auth::confidential::client::{ConfidentialClient, ConfidentialClientRef}; @@ -15,12 +14,6 @@ use opendut_auth::registration::config::RegistrationClientConfig; use opendut_auth::registration::resources::ResourceHomeUrl; use opendut_util_core::project; -#[fixture] -pub async fn issuer_certificate_authority() -> Pem { - Pem::from_file_path("resources/development/tls/insecure-development-ca.pem").await - .expect("Failed to resolve development ca in resources directory.") -} - #[fixture] pub async fn confidential_carl_client() -> ConfidentialClientRef { let issuer_url = "https://keycloak/realms/opendut/".to_string(); // This is the URL for the keycloak server in the test environment @@ -61,114 +54,37 @@ pub async fn registration_client(#[future] confidential_carl_client: Confidentia RegistrationClient::new(carl_idp_config, client) } -#[derive(Deserialize, Debug)] -#[serde(rename_all = "camelCase")] -struct Client { - id: String, - client_id: String, - name: Option, - base_url: Option, -} - #[cfg(test)] mod auth_tests { use googletest::assert_that; use googletest::matchers::eq; - use http::{HeaderMap, HeaderValue}; - use oauth2::HttpRequest; - use pem::Pem; use rstest::rstest; - - use opendut_auth::confidential::reqwest_client::OidcReqwestClient; - use opendut_auth::registration::client::{RegistrationClientError, RegistrationClientRef}; + + use opendut_auth::registration::client::{RegistrationClientRef}; use opendut_types::resources::Id; - use crate::{Client, issuer_certificate_authority, registration_client}; - - async fn delete_client(client: RegistrationClientRef, delete_client_id: String, issuer_ca: Pem) -> Result<(), RegistrationClientError> { - let client_id = client.inner.config.client_id.clone().to_string(); - let access_token = client.inner.get_token().await - .map_err(|error| RegistrationClientError::RequestError { error: format!("Could not fetch token to delete client {}!", client_id), cause: error.into() })?; - let delete_client_url = client.inner.config.issuer_url.join("/admin/realms/opendut/clients/").unwrap().join(&delete_client_id.to_string()) - .map_err(|error| RegistrationClientError::InvalidConfiguration { error: format!("Invalid client URL: {}", error) })?; - - let mut headers = HeaderMap::new(); - let bearer_header = format!("Bearer {}", access_token.to_string()); - let access_token_value = HeaderValue::from_str(&bearer_header) - .map_err(|error| RegistrationClientError::InvalidConfiguration { error: error.to_string() })?; - headers.insert(http::header::AUTHORIZATION, access_token_value); - - let request = HttpRequest { - method: http::Method::DELETE, - url: delete_client_url, - headers, - body: vec![], - }; - - let reqwest_client = OidcReqwestClient::from_pem(issuer_ca) - .map_err(|error| RegistrationClientError::InvalidConfiguration { error: format!("Failed to load certificate authority. {}", error) })?; - - let response = reqwest_client.async_http_client(request) - .await - .map_err(|error| RegistrationClientError::RequestError { error: "OIDC client delete request failed!".to_string(), cause: Box::new(error) })?; - assert_eq!(response.status_code, 204, "Failed to delete client with id '{:?}': {:?}", client_id, response.body); - - Ok(()) - } - - async fn list_clients_for_user(client: RegistrationClientRef, user_id: String, issuer_ca: Pem) -> Result<(), RegistrationClientError> { - let client_id = client.inner.config.client_id.clone().to_string(); - let access_token = client.inner.get_token().await - .map_err(|error| RegistrationClientError::RequestError { error: format!("Could not fetch token to delete client {}!", client_id), cause: error.into() })?; - let list_client_url = client.inner.config.issuer_url.join("/admin/realms/opendut/clients").unwrap(); - - let mut headers = HeaderMap::new(); - let bearer_header = format!("Bearer {}", access_token); - let access_token_value = HeaderValue::from_str(&bearer_header) - .map_err(|error| RegistrationClientError::InvalidConfiguration { error: error.to_string() })?; - headers.insert(http::header::AUTHORIZATION, access_token_value); - - let request = HttpRequest { - method: http::Method::GET, - url: list_client_url, - headers, - body: vec![], - }; - - let reqwest_client = OidcReqwestClient::from_pem(issuer_ca) - .map_err(|error| RegistrationClientError::InvalidConfiguration { error: format!("Failed to load certificate authority. {}", error) })?; - - let response = reqwest_client.async_http_client(request) - .await - .map_err(|error| RegistrationClientError::RequestError { error: "OIDC client list request failed!".to_string(), cause: Box::new(error) })?; - - let clients: Vec = serde_json::from_slice(&response.body).unwrap(); - let filtered_clients: Vec = clients.into_iter().filter(|client| client.base_url.clone().is_some_and(|url| url.contains(&user_id))).collect(); - assert_eq!(response.status_code, 200, "Failed to list clients for user'{:?}'", user_id); - assert!(!filtered_clients.is_empty()); - - Ok(()) - } + use crate::{registration_client}; #[rstest] #[tokio::test] #[ignore] - async fn test_register_new_oidc_client(#[future] registration_client: RegistrationClientRef, #[future] issuer_certificate_authority: Pem) { + async fn test_register_new_oidc_client(#[future] registration_client: RegistrationClientRef) { /* * This test is ignored because it requires a running keycloak server from the test environment. * To run this test, execute the following command: cargo test -- --include-ignored */ let client = registration_client.await; - let pem = issuer_certificate_authority.await; println!("{:?}", client); let resource_id = Id::random(); - let user_id = String::from("testUser"); + let user_id = String::from("deleteTest"); let credentials = client.register_new_client_for_user(resource_id, user_id.clone()).await.unwrap(); let (client_id, client_secret) = (credentials.client_id.value(), credentials.client_secret.value()); - println!("New client id: {}, secret: {}", client_id, client_secret); - list_clients_for_user(client.clone(), user_id, pem.clone()).await.unwrap(); - delete_client(client, client_id.clone(), pem).await.unwrap(); assert_that!(client_id.len().gt(&10), eq(true)); + println!("New client id: {}, secret: {}", client_id, client_secret); + let client_list = client.list_clients(resource_id).await.unwrap(); + assert!(!client_list.0.is_empty()); + client.delete_client(resource_id).await.unwrap(); + let client_list = client.list_clients(resource_id).await.unwrap(); + assert!(client_list.0.is_empty()); } - } diff --git a/opendut-util/opendut-auth/src/registration/client.rs b/opendut-util/opendut-auth/src/registration/client.rs index 2708b6bf0..2dc114e93 100644 --- a/opendut-util/opendut-auth/src/registration/client.rs +++ b/opendut-util/opendut-auth/src/registration/client.rs @@ -1,14 +1,18 @@ use std::sync::Arc; use config::Config; +use http::{HeaderMap, HeaderValue}; +use oauth2::HttpRequest; use openidconnect::{ClientName, ClientUrl}; use openidconnect::core::{CoreClientRegistrationRequest, CoreGrantType}; use openidconnect::registration::EmptyAdditionalClientMetadata; - +use serde::Deserialize; +use tracing::error; +use url::Url; use opendut_types::resources::Id; use opendut_types::util::net::{ClientCredentials, ClientId, ClientSecret}; -use crate::confidential::client::{ConfidentialClient, ConfidentialClientRef}; +use crate::confidential::client::{ConfidentialClient, ConfidentialClientRef, Token}; use crate::registration::config::RegistrationClientConfig; use crate::registration::error::WrappedClientRegistrationError; @@ -42,6 +46,12 @@ pub enum RegistrationClientError { Registration { cause: WrappedClientRegistrationError, }, + #[error("Client could not be found")] + ClientNotFound, + #[error("Following clients could not be deleted: {client_ids}")] + ClientDeletionError { + client_ids: String + }, } @@ -126,4 +136,95 @@ impl RegistrationClient { } } } + + pub async fn list_clients(&self, resource_id: Id) -> Result { + let access_token = self.inner.get_token().await + .map_err(|error| RegistrationClientError::RequestError { error: error.to_string(), cause: Box::new(error) })?; + + let issuer_remote_url = request_uri(&self.config.issuer_remote_url, None)?; + let request = create_http_request_client(&access_token, &issuer_remote_url, http::Method::GET)?; + + let response = self.inner.reqwest_client.async_http_client(request) + .await; + match response { + Ok(response) => { + let clients: Clients = serde_json::from_slice(&response.body).unwrap(); + let filtered_clients = clients.0.into_iter().filter(|client| client.base_url.clone().is_some_and(|url| url.contains(&resource_id.value().to_string()))).collect::>(); + + Ok(Clients(filtered_clients)) + } + Err(error) => { + Err(RegistrationClientError::RequestError { error: "OIDC client list request failed!".to_string(), cause: Box::new(error) }) + } + } + } + + pub async fn delete_client(&self, resource_id: Id) -> Result { + let access_token = self.inner.get_token().await + .map_err(|error| RegistrationClientError::RequestError { error: error.to_string(), cause: Box::new(error) })?; + + let clients_url = request_uri(&self.config.issuer_remote_url, None)?; + + let clients = self.list_clients(resource_id).await?; + + let mut failed_deletion_clients = Vec::new(); + + for client in clients.clone().0 { + let delete_client_url = clients_url.join(&client.client_id.to_string()) + .map_err(|error| RegistrationClientError::InvalidConfiguration { error: format!("Invalid client URL: {}", error) })?; + + let request = create_http_request_client(&access_token, &delete_client_url, http::Method::DELETE)?; + + let response = self.inner.reqwest_client.async_http_client(request) + .await; + + if response.is_err() { + failed_deletion_clients.push(client.client_id); + } + } + + if failed_deletion_clients.is_empty() { + Ok(clients) + } else { + Err( RegistrationClientError::ClientDeletionError { client_ids: failed_deletion_clients.join(",") } ) + } + } +} + +fn request_uri(issuer_remote_url: &Url, client_id: Option) -> Result { + match client_id { + Some(client_id) => { + issuer_remote_url.join(format!("/admin/realms/opendut/clients/{}", client_id).as_str()) + .map_err(|error| RegistrationClientError::RequestError { error: error.to_string(), cause: Box::new(error) }) + } + None => { + issuer_remote_url.join("/admin/realms/opendut/clients/") + .map_err(|error| RegistrationClientError::RequestError { error: error.to_string(), cause: Box::new(error) }) + } + } +} + +fn create_http_request_client(access_token: &Token, issuer_remote_url: &Url, http_method: http::Method) -> Result { + let mut headers = HeaderMap::new(); + let bearer_header = format!("Bearer {}", access_token); + let access_token_value = HeaderValue::from_str(&bearer_header) + .map_err(|error| RegistrationClientError::InvalidConfiguration { error: error.to_string() })?; + headers.insert(http::header::AUTHORIZATION, access_token_value); + + Ok(HttpRequest { + method: http_method, + url: issuer_remote_url.clone(), + headers, + body: vec![], + }) +} + +#[derive(Deserialize, Clone)] +pub struct Clients(pub Vec); + +#[derive(Deserialize, Clone)] +#[serde(rename_all = "camelCase")] +pub struct Client { + pub client_id: String, + base_url: Option, }