Change AuthConfig to enum

opendut-carl/src/actions/peers.rs

@@ -280,7 +280,7 @@ pub async fn generate_peer_setup(params: GeneratePeerSetupParams) -> Result<Peer
 
         let auth_config = match params.oidc_client_manager {
             None => {
-                AuthConfig::disabled()
+                AuthConfig::Disabled
             }
             Some(oidc_client_manager) => {
                 debug!("Generating OIDC client for peer '{peer_name}' <{peer_id}>.");

opendut-edgar/edgar.toml

@@ -15,7 +15,7 @@ ca = "/etc/opendut/tls/ca.pem"
 domain.name.override = ""
 
 [network.oidc]
-enabled = true
+enabled = false
 
 [network.oidc.client]
 id = "opendut-edgar-client"

opendut-edgar/src/setup/tasks/write_configuration.rs

@@ -46,9 +46,6 @@ impl Task for WriteConfiguration {
             let peer_id = self.config_override.peer_id.to_string();
             let carl_host = self.config_override.carl_url.host_str().expect("Host name should be defined in CARL URL.");
             let carl_port = self.config_override.carl_url.port().unwrap_or(443);
-            let network_oidc_client_id = self.config_override.auth_config.client_id.clone().value();
-            let network_oidc_client_secret = self.config_override.auth_config.client_secret.clone().value();
-            let auth_config = self.config_override.auth_config.clone();
 
             if new_settings.get("peer").is_none() {
                 new_settings["peer"] = toml_edit::table();
@@ -63,21 +60,34 @@ impl Task for WriteConfiguration {
             new_settings["network"]["carl"]["host"] = toml_edit::value(carl_host);
             new_settings["network"]["carl"]["port"] = toml_edit::value(i64::from(carl_port));
 
-            if new_settings.get("network").and_then(|network| network.get("oidc")).is_none() {
-                new_settings["network"]["oidc"] = toml_edit::table();
-            }
-            new_settings["network"]["oidc"]["enabled"] = toml_edit::value(auth_config.is_oidc_enabled());
-
-
-            if new_settings.get("network")
-                .and_then(|network| network.get("oidc"))
-                .and_then(|network| network.get("client"))
-                .is_none() {
-
-                new_settings["network"]["oidc"]["client"] = toml_edit::table();
-            }
-            new_settings["network"]["oidc"]["client"]["id"] = toml_edit::value(network_oidc_client_id);
-            new_settings["network"]["oidc"]["client"]["secret"] = toml_edit::value(network_oidc_client_secret);
+            match &self.config_override.auth_config {
+                AuthConfig::Disabled => {
+                    if new_settings.get("network").and_then(|network| network.get("oidc")).is_none() {
+                        new_settings["network"]["oidc"] = toml_edit::table();
+                    }
+                    new_settings["network"]["oidc"]["enabled"] = toml_edit::value(false);
+                }
+                AuthConfig::Enabled { client_id, client_secret, ..} => {
+                    let network_oidc_client_id = client_id.clone().value();
+                    let network_oidc_client_secret = client_secret.clone().value();
+
+                    if new_settings.get("network").and_then(|network| network.get("oidc")).is_none() {
+                        new_settings["network"]["oidc"] = toml_edit::table();
+                    }
+                    new_settings["network"]["oidc"]["enabled"] = toml_edit::value(true);
+
+                    if new_settings.get("network")
+                        .and_then(|network| network.get("oidc"))
+                        .and_then(|network| network.get("client"))
+                        .is_none() {
+
+                        new_settings["network"]["oidc"]["client"] = toml_edit::table();
+                    }
+                    new_settings["network"]["oidc"]["client"]["id"] = toml_edit::value(network_oidc_client_id);
+                    new_settings["network"]["oidc"]["client"]["secret"] = toml_edit::value(network_oidc_client_secret);
+                }
+            };
+
 
             new_settings.to_string()
 
@@ -168,10 +178,10 @@ mod tests {
     const PORT: u16 = 1234;
     const CLIENT_ID: &str = "ClientId";
     const CLIENT_SECRET: &str = "ClientSecret";
-    const OIDC_NOT_ENABLED: bool = true;
+    const OIDC_ENABLED: bool = true;
 
     #[test]
-    fn should_write_a_fresh_configuration() -> anyhow::Result<()> {
+    fn should_write_a_fresh_configuration_with_auth_config_enabled() -> anyhow::Result<()> {
         let temp = TempDir::new()?;
         let config_file = temp.child("edgar.toml");
         let config_merge_suggestion_file = temp.child("edgar-merge-suggestion.toml");
@@ -184,7 +194,7 @@ mod tests {
             config_override: ConfigOverride {
                 peer_id: new_peer_id,
                 carl_url: Url::parse("https://example.com:1234").unwrap(),
-                auth_config: AuthConfig {
+                auth_config: AuthConfig::Enabled {
                     issuer_url: Url::parse("https://test.com:1234").unwrap(),
                     client_secret: ClientSecret(CLIENT_SECRET.to_string()),
                     client_id: ClientId(CLIENT_ID.to_string()),
@@ -219,6 +229,46 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn should_write_a_fresh_configuration_with_auth_config_disabled() -> anyhow::Result<()> {
+        let temp = TempDir::new()?;
+        let config_file = temp.child("edgar.toml");
+        let config_merge_suggestion_file = temp.child("edgar-merge-suggestion.toml");
+
+        let new_peer_id = PeerId::from(uuid!("dc72f6d9-d700-455f-8c31-9f15438e7503"));
+
+        let task = WriteConfiguration {
+            config_file_to_write_to: config_file.to_path_buf(),
+            config_merge_suggestion_file: config_merge_suggestion_file.to_path_buf(),
+            config_override: ConfigOverride {
+                peer_id: new_peer_id,
+                carl_url: Url::parse("https://example.com:1234").unwrap(),
+                auth_config: AuthConfig::Disabled,
+            },
+        };
+
+        assert!(predicate::path::missing().eval(&config_file));
+
+        runner::test::unchecked(task)?;
+
+        assert!(predicate::path::exists().eval(&config_file));
+        let file_content = fs::read_to_string(&config_file)?;
+
+        assert_that!(file_content, eq(indoc!(r#"
+            [peer]
+            id = "dc72f6d9-d700-455f-8c31-9f15438e7503"
+
+            [network]
+            carl.host = "example.com"
+            carl.port = 1234
+
+            [network.oidc]
+            enabled = false
+        "#)));
+
+        Ok(())
+    }
+
     #[test]
     fn should_provide_an_merge_suggestion_for_an_already_existing_configuration_but_should_not_delete_existing_unknown_keys() -> anyhow::Result<()> {
         let temp = TempDir::new()?;
@@ -231,7 +281,7 @@ mod tests {
             [peer.unknown]
             key = "value"
             [Hallo.Welt]
-            key = "value"
+            key = "value"        
         "#))?;
 
         let new_peer_id = PeerId::random();
@@ -242,7 +292,7 @@ mod tests {
             config_override: ConfigOverride {
                 peer_id: new_peer_id,
                 carl_url: Url::parse("https://example.com:1234").unwrap(),
-                auth_config: AuthConfig {
+                auth_config: AuthConfig::Enabled {
                     issuer_url: Url::parse("https://test.com:1234").unwrap(),
                     client_secret: ClientSecret(CLIENT_SECRET.to_string()),
                     client_id: ClientId(CLIENT_ID.to_string()),
@@ -267,6 +317,52 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn should_provide_an_merge_suggestion_for_an_already_existing_configuration_with_auth_config_disabled() -> anyhow::Result<()> {
+        let temp = TempDir::new()?;
+        let config_file = temp.child("edgar.toml");
+        let config_merge_suggestion_file = temp.child("edgar-merge-suggestion.toml");
+
+        config_file.write_str(&format!(indoc!(r#"
+            [peer]
+            id = "eef8997e-56a0-4d8d-978e-40d1f2e68db0"
+            
+            [network.oidc]
+            enabled = {}
+
+            [network.oidc.client]
+            id = "{}"
+            secret = "{}"
+        "#), OIDC_ENABLED, CLIENT_ID, CLIENT_SECRET))?;
+
+        let new_peer_id = PeerId::random();
+
+        let task = WriteConfiguration {
+            config_file_to_write_to: config_file.to_path_buf(),
+            config_merge_suggestion_file: config_merge_suggestion_file.to_path_buf(),
+            config_override: ConfigOverride {
+                peer_id: new_peer_id,
+                carl_url: Url::parse("https://example.com:1234").unwrap(),
+                auth_config: AuthConfig::Disabled,
+            },
+        };
+
+        let file_content = fs::read_to_string(&config_file)?;
+        assert!(predicate::str::is_empty().not().eval(&file_content));
+        assert!(predicate::path::missing().eval(&config_merge_suggestion_file));
+
+        let result = runner::test::unchecked(task);
+        assert!(result.is_err());
+
+        assert!(predicate::path::exists().eval(&config_merge_suggestion_file));
+        let merge_suggestion = fs::read_to_string(config_merge_suggestion_file)?;
+        assert!(predicate::str::contains(new_peer_id.to_string()).eval(&merge_suggestion));
+        assert!(predicate::str::contains("enabled = false".to_string()).eval(&merge_suggestion));
+        assert!(predicate::str::contains("secret = \"ClientSecret\"".to_string()).not().eval(&merge_suggestion));
+
+        Ok(())
+    }
+
     #[test]
     fn should_not_provide_a_merge_suggestion_if_the_existing_config_matches() -> anyhow::Result<()> {
         let temp = TempDir::new()?;
@@ -289,15 +385,15 @@ mod tests {
             [network.oidc.client]
             id = "{}"
             secret = "{}"
-        "#), new_peer_id, HOST, PORT, OIDC_NOT_ENABLED, CLIENT_ID, CLIENT_SECRET))?;
+        "#), new_peer_id, HOST, PORT, OIDC_ENABLED, CLIENT_ID, CLIENT_SECRET))?;
 
         let task = WriteConfiguration {
             config_file_to_write_to: config_file.to_path_buf(),
             config_merge_suggestion_file: config_merge_suggestion_file.to_path_buf(),
             config_override: ConfigOverride {
                 peer_id: new_peer_id,
                 carl_url: Url::parse(&format!("https://{HOST}:{PORT}")).unwrap(),
-                auth_config: AuthConfig {
+                auth_config: AuthConfig::Enabled {
                     issuer_url: Url::parse("https://test.com:1234").unwrap(),
                     client_secret: ClientSecret(CLIENT_SECRET.to_string()),
                     client_id: ClientId(CLIENT_ID.to_string()),

opendut-types/proto/opendut/types/util/net.proto

@@ -57,6 +57,15 @@ message NetworkInterfaceDescriptor {
 }
 
 message AuthConfig {
+  oneof config {
+    AuthConfigEnabled enabled = 1;
+    AuthConfigDisabled disabled = 2;
+  }
+}
+
+message AuthConfigDisabled {}
+
+message AuthConfigEnabled {
   Url issuer_url = 1;
   ClientId client_id = 2;
   ClientSecret client_secret = 3;

opendut-types/src/peer/mod.rs

@@ -325,10 +325,10 @@ mod tests {
             id: PeerId::try_from("01bf3f8c-cc7c-4114-9520-91bce71dcead").unwrap(),
             carl: Url::parse("https://carl.opendut.local")?,
             ca: Certificate(Pem::new("Test Tag".to_string(), vec![])),
-            auth_config: AuthConfig {
-                client_id: ClientId::try_from("client_id").unwrap(),
-                client_secret: ClientSecret::try_from("my-secure!-random-string-with-at-least-x-chars%").unwrap(),
-                scopes: vec![OAuthScope::try_from("manage-realm").unwrap()],
+            auth_config: AuthConfig::Enabled {
+                client_id: ClientId::from("client_id"),
+                client_secret: ClientSecret::from("my-secure!-random-string-with-at-least-x-chars%"),
+                scopes: vec![OAuthScope::from("manage-realm")],
                 issuer_url: Url::parse("https://keycloak/realms/opendut/").unwrap(),
             },
             vpn: VpnPeerConfiguration::Netbird {
@@ -338,7 +338,7 @@ mod tests {
         };
 
         let encoded = setup.encode()?;
-        assert_that!(encoded, eq("F78BIBwHdiz4lWbaSDYvtcjeFlWr5lS6N1PXfcBE-dIHAFqHpavv4S2wCQwHQt-LW_10GkbPi4GFFnAAlm4TPII-CSnSNn266h-pM0LIpNBFlCJNfQojpUFslUAskzT3MkvzkFHSsHTEs025eLYhZPvGvAa-jjUPlVzwuxe8UIK8l_cAXhbXkwtK2LfqxwWQHPa5_HbX0za_024MLae671ceBfcvefC_cx1NlMR9RPQ3AE8PkBeRIh7oAsKGfO5x4TN78PSSEsNKLrkDZEJ7jmkItxnyra64dUD5BtGrZVfA1WGquyjVd7T5TWQ-TpVQBRKl4wcTxx6RMTmcjwrlnXC5TOk_"));
+        assert_that!(encoded, eq("F8sBABwHzrk8VikIvG691CJ7W1Stsq0oZqoIe8UvyoH_PRFpnb4ullM3wV7g8UFoTRxt-svCaEwMLLSAA7DMXeWhPNSyiLPbPh7iH2lyQsi01GXIkGU-g5HSILFKIJFpVniZZ0XMabK49sS7TWmybMacHztb1vGBJFsITgY9fIGLFdBn_YygcXHdu6CJx17_kdGTXHZEVbbx5_p3wJh2xZP1U-w47d2wcUQUC-fE_u3xqINxDkbE8MD44QUiPdzFwtIZcYdvd7jxsT3zrsw9ho2cozsAmNJrznkEXI_FHmRUPZj8BvjFuCsrVnFuCovbd3TFQ-Q-yZRQJVKBGx9MkngEYwo4H0rlnXCFzKgC"));
 
         let decoded = PeerSetup::decode(&encoded)?;
         assert_that!(decoded, eq(setup));

opendut-types/src/proto/util.rs

@@ -1,5 +1,6 @@
 use pem::Pem;
 use crate::proto::{ConversionError, ConversionErrorBuilder};
+use crate::proto::util::auth_config::Config;
 use crate::proto::util::ip_address::Address;
 use crate::util;
 use crate::util::net::NetworkInterfaceConfiguration;
@@ -331,11 +332,24 @@ impl TryFrom<OAuthScope> for crate::util::net::OAuthScope {
 
 impl From<crate::util::net::AuthConfig> for AuthConfig {
     fn from(value: crate::util::net::AuthConfig) -> Self {
+        let config = match value {
+            crate::util::net::AuthConfig::Disabled => { auth_config::Config::Disabled( AuthConfigDisabled {}) }
+            crate::util::net::AuthConfig::Enabled {
+                issuer_url,
+                client_id,
+                client_secret,
+                scopes
+            }
+             => { auth_config::Config::Enabled ( AuthConfigEnabled {
+                issuer_url: Some(issuer_url.into()),
+                client_id: Some(client_id.into()),
+                client_secret: Some(client_secret.into()),
+                scopes: scopes.into_iter().map(|scope| scope.into()).collect()
+            }) }
+
+        };
         Self {
-            issuer_url: Some(value.issuer_url.into()),
-            client_id: Some(value.client_id.into()),
-            client_secret: Some(value.client_secret.into()),
-            scopes: value.scopes.into_iter().map(|scope| scope.into()).collect(),
+            config: Some(config),
         }
     }
 }
@@ -345,32 +359,39 @@ impl TryFrom<AuthConfig> for crate::util::net::AuthConfig {
 
     fn try_from(value: AuthConfig) -> Result<Self, Self::Error> {
         type ErrorBuilder = ConversionErrorBuilder<AuthConfig, crate::util::net::AuthConfig>;
+
+        let config = match value.config
+            .ok_or(ErrorBuilder::new("Configuration not set"))? {
+            Config::Disabled(_) => crate::util::net::AuthConfig::Disabled,
+            Config::Enabled(auth_config) => {
+                let issuer_url = auth_config.issuer_url
+                    .ok_or(ErrorBuilder::new("Authorization Provider Issuer URL not set"))
+                    .and_then(|url| url::Url::parse(&url.value)
+                        .map_err(|cause| ErrorBuilder::new(format!("Authorization Provider Issuer URL could not be parsed: {}", cause)))
+                    )?;
+
+                let client_id: crate::util::net::ClientId = auth_config.client_id
+                    .ok_or(ErrorBuilder::new("ClientId not set"))?
+                    .try_into()?;
+
+                let client_secret: crate::util::net::ClientSecret = auth_config.client_secret
+                    .ok_or(ErrorBuilder::new("ClientSecret not set"))?
+                    .try_into()?;
+
+                let scopes: Vec<crate::util::net::OAuthScope> = auth_config
+                    .scopes
+                    .into_iter()
+                    .map(TryFrom::try_from)
+                    .collect::<Result<_, _>>()?;
+                crate::util::net::AuthConfig::Enabled {
+                    issuer_url,
+                    client_id,
+                    client_secret,
+                    scopes,
+                }
+            }
+        };
 
-        let issuer_url = value.issuer_url
-            .ok_or(ErrorBuilder::new("Authorization Provider Issuer URL not set"))
-            .and_then(|url| url::Url::parse(&url.value)
-                .map_err(|cause| ErrorBuilder::new(format!("Authorization Provider Issuer URL could not be parsed: {}", cause)))
-            )?;
-
-        let client_id: crate::util::net::ClientId = value.client_id
-            .ok_or(ErrorBuilder::new("ClientId not set"))?
-            .try_into()?;
-
-        let client_secret: crate::util::net::ClientSecret = value.client_secret
-            .ok_or(ErrorBuilder::new("ClientSecret not set"))?
-            .try_into()?;
-
-        let scopes: Vec<crate::util::net::OAuthScope> = value
-            .scopes
-            .into_iter()
-            .map(TryFrom::try_from)
-            .collect::<Result<_, _>>()?;
-
-        Ok(Self {
-            issuer_url,
-            client_id,
-            client_secret,
-            scopes,
-        })
+        Ok(config)
     }
 }