Public OIDC client support

Readme.md

@@ -65,7 +65,9 @@ services:
 Register an Open ID Connect client at the central half of this component.
 
 Secret type: `OIDC`  
-Arguments: A comma separated list of urls permitted for redirection  
+Each argument is separated by a semicolon. The arguments are: 
+- The type of OIDC client which gets created. Either `public` or `private`
+- A comma separated list of urls permitted for redirection  
 
 Example:
-`OIDC:MY_OIDC_CLIENT_SECRET:https://foo.com,https://bar.com`
+`OIDC:MY_OIDC_CLIENT_SECRET:public;https://foo.com,https://bar.com`

central/Cargo.toml

@@ -14,4 +14,3 @@ serde = { workspace = true }
 futures = { workspace = true }
 serde_json = "1"
 rand = "0.8"
-assert-json-diff = "2.0"

central/src/config.rs

@@ -2,7 +2,7 @@ use std::{net::SocketAddr, convert::Infallible};
 
 use beam_lib::{AppId, reqwest::Url};
 use clap::Parser;
-use shared::SecretResult;
+use shared::{SecretResult, OIDCConfig};
 
 use crate::keycloak::{KeyCloakConfig, self};
 
@@ -36,19 +36,19 @@ impl OIDCProvider {
         KeyCloakConfig::try_parse().map_err(|e| println!("{e}")).ok().map(Self::Keycloak)
     }
 
-    pub async fn create_client(&self, name: &str, redirect_urls: Vec<String>) -> Result<SecretResult, String> {
+    pub async fn create_client(&self, name: &str, oidc_client_config: OIDCConfig) -> Result<SecretResult, String> {
         match self {
-            OIDCProvider::Keycloak(conf) => keycloak::create_client(name, redirect_urls, conf).await,
+            OIDCProvider::Keycloak(conf) => keycloak::create_client(name, oidc_client_config, conf).await,
         }.map_err(|e| {
             println!("Failed to create client: {e}");
             "Error creating OIDC client".into()
         })
     }
 
-    pub async fn validate_client(&self, name: &str, secret: &str, redirect_urls: &[String]) -> Result<bool, String> {
+    pub async fn validate_client(&self, name: &str, secret: &str, oidc_client_config: &OIDCConfig) -> Result<bool, String> {
         match self {
             OIDCProvider::Keycloak(conf) => {
-                keycloak::validate_client(name, redirect_urls, secret, conf)
+                keycloak::validate_client(name, oidc_client_config, secret, conf)
                     .await
                     .map_err(|e| {
                         eprintln!("Failed to validate client {name}: {e}");

central/src/keycloak.rs

@@ -2,7 +2,7 @@ use crate::CLIENT;
 use beam_lib::reqwest::{self, StatusCode, Url};
 use clap::Parser;
 use serde_json::{json, Value};
-use shared::SecretResult;
+use shared::{SecretResult, OIDCConfig};
 
 #[derive(Debug, Parser, Clone)]
 pub struct KeyCloakConfig {
@@ -43,16 +43,13 @@ async fn get_access_token(conf: &KeyCloakConfig) -> reqwest::Result<String> {
 }
 
 #[cfg(test)]
-async fn get_access_token_via_admin_login(conf: &KeyCloakConfig) -> reqwest::Result<String> {
+async fn get_access_token_via_admin_login() -> reqwest::Result<String> {
     #[derive(serde::Deserialize)]
     struct Token {
         access_token: String,
     }
     CLIENT
-        .post(&format!(
-            "{}/realms/{}/protocol/openid-connect/token",
-            conf.keycloak_url, conf.keycloak_realm
-        ))
+        .post("http://localhost:1337/realms/master/protocol/openid-connect/token")
         .form(&json!({
             "client_id": "admin-cli",
             "username": "admin",
@@ -67,10 +64,12 @@ async fn get_access_token_via_admin_login(conf: &KeyCloakConfig) -> reqwest::Res
 }
 
 async fn get_client(
-    id: &str,
+    name: &str,
     token: &str,
+    oidc_client_config: &OIDCConfig,
     conf: &KeyCloakConfig,
 ) -> reqwest::Result<serde_json::Value> {
+    let id = format!("{name}-{}", if oidc_client_config.is_public { "public" } else { "private" });
     CLIENT
         .get(&format!(
             "{}/admin/realms/{}/clients/{id}",
@@ -84,35 +83,47 @@ async fn get_client(
 }
 
 pub async fn validate_client(
-    id: &str,
-    redirect_urls: &[String],
+    name: &str,
+    oidc_client_config: &OIDCConfig,
     secret: &str,
     conf: &KeyCloakConfig,
 ) -> reqwest::Result<bool> {
     let token = get_access_token(conf).await?;
-    let client = get_client(id, &token, conf).await?;
-    let wanted_client = generate_client(id, redirect_urls, secret);
-    Ok(client_configs_match(&client, &wanted_client))
+    compare_clients(&token, name, oidc_client_config, conf, secret).await
+}
+
+async fn compare_clients(token: &str, name: &str, oidc_client_config: &OIDCConfig, conf: &KeyCloakConfig, secret: &str) -> Result<bool, reqwest::Error> {
+    let client = get_client(&name, &token, oidc_client_config, conf).await?;
+    let wanted_client = generate_client(name, oidc_client_config, secret);
+    Ok(client.get("secret") == wanted_client.get("secret")
+        && client_configs_match(&client, &wanted_client))
 }
 
 fn client_configs_match(a: &Value, b: &Value) -> bool {
-    assert_json_diff::assert_json_matches_no_panic(
-        &a,
-        &b,
-        assert_json_diff::Config::new(assert_json_diff::CompareMode::Inclusive)
-    )
-    .map_err(|e| eprintln!("Clients did not match: {e}"))
-    .is_ok()
+    let includes_other_json_array = |key, comparator: &dyn Fn(_, _) -> bool| a
+        .get(key)
+        .and_then(Value::as_array)
+        .is_some_and(|a_values| b
+            .get(key)
+            .and_then(Value::as_array)
+            .is_some_and(|vec| vec.iter().all(|v| comparator(a_values, v)))
+        );
+
+    a.get("name") == b.get("name")
+        && includes_other_json_array("defaultClientScopes", &|a_v, v| a_v.contains(v))
+        && includes_other_json_array("redirectUris", &|a_v, v| a_v.contains(v))
+        && includes_other_json_array("protocolMappers", &|a_v, v| a_v.iter().any(|a_v| a_v.get("name") == v.get("name")))
 }
 
-fn generate_client(name: &str, redirect_urls: &[String], secret: &str) -> Value {
-    json!({
+fn generate_client(name: &str, oidc_client_config: &OIDCConfig, secret: &str) -> Value {
+    let secret = (!oidc_client_config.is_public).then_some(secret);
+    let name = format!("{name}-{}", if oidc_client_config.is_public { "public" } else { "private" });
+    let mut json = json!({
         "name": name,
         "id": name,
         "clientId": name,
-        "redirectUris": redirect_urls,
-        "secret": secret,
-        "publicClient": false,
+        "redirectUris": oidc_client_config.redirect_urls,
+        "publicClient": oidc_client_config.is_public,
         "defaultClientScopes": [
             "web-origins",
             "acr",
@@ -133,31 +144,66 @@ fn generate_client(name: &str, redirect_urls: &[String], secret: &str) -> Value
                 "access.token.claim": "true"
             }
         }]
-    })
+    });
+    if let Some(secret) = secret {
+        json.as_object_mut().unwrap().insert("secret".to_owned(), secret.into());
+    }
+    json
+}
+
+#[cfg(test)]
+async fn setup_keycloak() -> reqwest::Result<(String, KeyCloakConfig)> {
+    let token = get_access_token_via_admin_login().await?;
+    let res = CLIENT
+        .post("http://localhost:1337/admin/realms/master/client-scopes")
+        .bearer_auth(&token)
+        .json(&json!({
+            "name": "groups",
+            "protocol": "openid-connect"
+        }))
+        .send()
+        .await?;
+    dbg!(&res.status());
+    Ok((token, KeyCloakConfig { keycloak_url: "http://localhost:1337".parse().unwrap(), keycloak_id: "unused in tests".into(), keycloak_secret: "unused in tests".into(), keycloak_realm: "master".into() }))
 }
 
 #[tokio::test]
 async fn test_create_client() -> reqwest::Result<()> {
-    let conf = KeyCloakConfig {
-        keycloak_url: "http://localhost:1337".parse().unwrap(),
-        keycloak_id: "".to_owned(),
-        keycloak_secret: "".to_owned(),
-        keycloak_realm: "master".to_owned(),
+    let (token, conf) = setup_keycloak().await?;
+    let name = "test";
+    // public client
+    let client_config = OIDCConfig { is_public: true, redirect_urls: vec!["http://foo/bar".into()] };
+    let (SecretResult::Created(pw) | SecretResult::AlreadyExisted(pw)) = dbg!(post_client(&token, name, &client_config, &conf).await?) else {
+        panic!("Not created or existed")
+    };
+    let c = dbg!(get_client(name, &token, &client_config, &conf).await.unwrap());
+    assert!(client_configs_match(&c, &generate_client(name, &client_config, &pw)));
+    assert!(dbg!(compare_clients(&token, name, &client_config, &conf, &pw).await?));
+
+    // private client
+    let client_config = OIDCConfig { is_public: true, redirect_urls: vec!["http://foo/bar".into()] };
+    let (SecretResult::Created(pw) | SecretResult::AlreadyExisted(pw)) = dbg!(post_client(&token, name, &client_config, &conf).await?) else {
+        panic!("Not created or existed")
     };
-    let token = get_access_token_via_admin_login(&conf).await?;
-    dbg!(post_client(&token, "test", vec!["http://test.bk".into()], &conf).await?);
-    dbg!(get_client("test", &token, &conf).await.unwrap());
+    let c = dbg!(get_client(name, &token, &client_config, &conf).await.unwrap());
+    assert!(client_configs_match(&c, &generate_client(name, &client_config, &pw)));
+    assert!(dbg!(compare_clients(&token, name, &client_config, &conf, &pw).await?));
+
     Ok(())
 }
 
 async fn post_client(
     token: &str,
     name: &str,
-    redirect_urls: Vec<String>,
+    oidc_client_config: &OIDCConfig,
     conf: &KeyCloakConfig,
 ) -> reqwest::Result<SecretResult> {
-    let secret = generate_secret();
-    let generated_client = generate_client(name, &redirect_urls, &secret);
+    let secret = if !oidc_client_config.is_public {
+        generate_secret()
+    } else {
+        String::with_capacity(0)
+    };
+    let generated_client = generate_client(name, oidc_client_config, &secret);
     let res = CLIENT
         .post(&format!(
             "{}/admin/realms/{}/clients",
@@ -168,20 +214,23 @@ async fn post_client(
         .send()
         .await?;
     match res.status() {
-        StatusCode::CREATED => Ok(SecretResult::Created(secret)),
+        StatusCode::CREATED => {
+            println!("Client for {name} created.");
+            Ok(SecretResult::Created(secret))
+        },
         StatusCode::CONFLICT => {
-            let conflicting_client = get_client(name, token, conf).await?;
+            let conflicting_client = get_client(name, token, oidc_client_config, conf).await?;
             if client_configs_match(&conflicting_client, &generated_client) {
-                Ok(conflicting_client
+                Ok(SecretResult::AlreadyExisted(conflicting_client
                     .as_object()
                     .and_then(|o| o.get("secret"))
                     .and_then(|v| v.as_str())
-                    .map(|v| SecretResult::AlreadyExisted(v.into()))
-                    .expect("These values should have a secret"))
+                    .unwrap_or("")
+                    .to_owned()))
             } else {
                 Ok(CLIENT
                     .put(&format!(
-                        "{}/admin/realms/{}/clients",
+                        "{}/admin/realms/{}/clients/{name}",
                         conf.keycloak_url, conf.keycloak_realm
                     ))
                     .bearer_auth(token)
@@ -190,9 +239,8 @@ async fn post_client(
                     .await?
                     .status()
                     .is_success()
-                    .then_some(secret)
-                    .map(SecretResult::Created)
-                    .expect("Put should be successfull"))
+                    .then_some(SecretResult::Created(secret))
+                    .expect("We know the client already exists so updating should be successful"))
             }
         }
         s => unreachable!("Unexpected statuscode {s} while creating keycloak client"),
@@ -217,9 +265,9 @@ fn generate_secret() -> String {
 
 pub async fn create_client(
     name: &str,
-    redirect_urls: Vec<String>,
+    oidc_client_config: OIDCConfig,
     conf: &KeyCloakConfig,
 ) -> reqwest::Result<SecretResult> {
     let token = get_access_token(conf).await?;
-    post_client(&token, name, redirect_urls, conf).await
+    post_client(&token, name, &oidc_client_config, conf).await
 }

central/src/main.rs

@@ -84,22 +84,22 @@ pub async fn handle_secret_task(task: SecretRequestType, from: &AppId) -> Result
 
 pub async fn create_secret(request: SecretRequest, name: &str) -> Result<SecretResult, String> {
     match request {
-        SecretRequest::OpenIdConnect { redirect_urls } => {
+        SecretRequest::OpenIdConnect(oidc_client_config) => {
             let Some(oidc_provider) = OIDC_PROVIDER.as_ref() else {
                 return Err("No OIDC provider configured!".into());
             };
-            oidc_provider.create_client(name, redirect_urls).await
+            oidc_provider.create_client(name, oidc_client_config).await
         }
     }
 }
 
 pub async fn is_valid(secret: &str, request: &SecretRequest, name: &str) -> Result<bool, String> {
     match request {
-        SecretRequest::OpenIdConnect { redirect_urls } => {
+        SecretRequest::OpenIdConnect(oidc_client_config) => {
             let Some(oidc_provider) = OIDC_PROVIDER.as_ref() else {
                 return Err("No OIDC provider configured!".into());
             };
-            oidc_provider.validate_client(name, secret, redirect_urls).await
+            oidc_provider.validate_client(name, secret, oidc_client_config).await
         },
     }
 }

dev/docker-compose.yaml

@@ -26,7 +26,7 @@ services:
       dockerfile: Dockerfile.central
     image: samply/secret-sync-central:latest
     environment:
-      - BEAM_URL=http://proxy:8082
+      # - BEAM_URL=http://proxy:8082
       - BEAM_ID=app2.proxy2.broker
       - BEAM_SECRET=App1Secret
       - KEYCLOAK_URL=http://keycloak:8080

dev/start.sh

@@ -40,7 +40,7 @@ if [ "$1" = "-b" ]; then
     shift
 fi
 
-args="OIDC:test:http://foo.com,http://bar.com"
+args="OIDC:test:public;http://foo.com,http://bar.com"
 # first=true
 # delimiter=$'\x1E'
 # # https://unix.stackexchange.com/a/460466

local/src/cache.rs

@@ -25,13 +25,13 @@ impl Cache {
         Self(
             file.split('\n')
                 .flat_map(|l| l.split_once('='))
-                .map(|(k, v)| (k.to_string(), v.to_string()))
+                .map(|(k, v)| (k.to_string(), v.trim_start_matches('"').trim_end_matches('"').to_string()))
                 .collect(),
         )
     }
 
     pub fn write(&self, path: impl AsRef<Path>) -> io::Result<()> {
-        let data: Vec<_> = self.0.iter().map(|(k, v)| format!("{k}={v}")).collect();
+        let data: Vec<_> = self.0.iter().map(|(k, v)| format!(r#"{k}="{v}""#)).collect();
         fs::write(path, data.join("\n"))
     }
 }

local/src/config.rs

@@ -2,7 +2,7 @@ use std::{path::PathBuf, convert::Infallible, str::FromStr};
 
 use beam_lib::AppId;
 use clap::Parser;
-use shared::SecretRequest;
+use shared::{SecretRequest, OIDCConfig};
 
 /// Local secret sync
 #[derive(Debug, Parser)]
@@ -53,8 +53,13 @@ impl FromStr for SecretArg {
         // Add new `SecretRequest` variants here
         let request = match secret_type {
             "OIDC" => {
+                let (is_public, args) = match args.split_once(';') {
+                    Some((is_public, args)) if is_public == "public" => (true, args),
+                    Some((is_public, args)) if is_public == "private" => (false, args),
+                    _ => return Err(format!("Invalid OIDC parameters. Syntax is <public|private>;<redirect_url1,redirect_url2,...>")),
+                };
                 let redirect_urls = args.split(',').map(ToString::to_string).collect();
-                Ok(SecretRequest::OpenIdConnect { redirect_urls })
+                Ok(SecretRequest::OpenIdConnect(OIDCConfig{ redirect_urls, is_public }))
             },
             _ => Err(format!("Unknown secret type {secret_type}"))
         }?;

shared/src/lib.rs

@@ -5,11 +5,14 @@ use serde::{Serialize, Deserialize};
 
 #[derive(Debug, Serialize, Deserialize, Clone)]
 pub enum SecretRequest {
-    OpenIdConnect {
-        redirect_urls: Vec<String>,
-    }
+    OpenIdConnect(OIDCConfig)
 }
 
+#[derive(Debug, Serialize, Deserialize, Clone)]
+pub struct OIDCConfig {
+    pub is_public: bool,
+    pub redirect_urls: Vec<String>,
+}
 
 #[derive(Debug, Serialize, Deserialize, Clone)]
 pub enum SecretResult {