fix: device.id is derived from device certificate if certificate exists

* device.id always uses a value from device certificate. If no
certificate exists, returns a value from tedge.toml file. Both don't
exist, finally it returns an error.

* tedge connect doesn't validate if device certficate's CN matches the
value from tedge.toml file. It just consume the value from device.id.

* The CN of `tedge cert create` is determined by this order:
1) the value of --device-id option
2) the value of tedge config get <device_id_key>

Signed-off-by: Rina Fujino <rina.fujino.23@gmail.com>

crates/common/tedge_config/src/tedge_config_cli/tedge_config.rs

@@ -1030,65 +1030,52 @@ fn device_id(
     device: &TEdgeConfigReaderDevice,
     dto_value: &OptionalConfig<String>,
 ) -> Result<String, ReadError> {
-    match dto_value.or_none() {
-        Some(id) => Ok(id.clone()),
-        None => device_id_from_cert(&device.cert_path),
+    match (device_id_from_cert(&device.cert_path), dto_value.or_none()) {
+        (Ok(common_name), _) => Ok(common_name),
+        (Err(_), Some(dto_value)) => Ok(dto_value.to_string()),
+        (Err(err), None) => Err(err),
     }
 }
 
 fn c8y_device_id(
     c8y_device: &TEdgeConfigReaderC8yDevice,
     dto_value: &OptionalConfig<String>,
 ) -> Result<String, ReadError> {
-    match dto_value.or_none() {
-        Some(id) => Ok(id.clone()),
-        None => device_id_from_cert(&c8y_device.cert_path),
+    match (
+        device_id_from_cert(&c8y_device.cert_path),
+        dto_value.or_none(),
+    ) {
+        (Ok(common_name), _) => Ok(common_name),
+        (Err(_), Some(dto_value)) => Ok(dto_value.to_string()),
+        (Err(err), None) => Err(err),
     }
 }
 
 fn az_device_id(
     az_device: &TEdgeConfigReaderAzDevice,
     dto_value: &OptionalConfig<String>,
 ) -> Result<String, ReadError> {
-    match dto_value.or_none() {
-        Some(id) => Ok(id.clone()),
-        None => device_id_from_cert(&az_device.cert_path),
+    match (
+        device_id_from_cert(&az_device.cert_path),
+        dto_value.or_none(),
+    ) {
+        (Ok(common_name), _) => Ok(common_name),
+        (Err(_), Some(dto_value)) => Ok(dto_value.to_string()),
+        (Err(err), None) => Err(err),
     }
 }
 
 fn aws_device_id(
     aws_device: &TEdgeConfigReaderAwsDevice,
     dto_value: &OptionalConfig<String>,
 ) -> Result<String, ReadError> {
-    match dto_value.or_none() {
-        Some(id) => Ok(id.clone()),
-        None => device_id_from_cert(&aws_device.cert_path),
-    }
-}
-
-pub fn explicit_device_id(
-    config_location: &TEdgeConfigLocation,
-    cloud: &Option<Cloud>,
-) -> Option<String> {
-    let dto = config_location.load_dto_from_toml_and_env().ok()?;
-
-    match cloud {
-        None => dto.device.id.clone(),
-        Some(Cloud::C8y(profile)) => {
-            let key = profile.map(|name| name.to_string());
-            let c8y_dto = dto.c8y.try_get(key.as_deref(), "c8y").ok()?;
-            c8y_dto.device.id.clone()
-        }
-        Some(Cloud::Az(profile)) => {
-            let key = profile.map(|name| name.to_string());
-            let az_dto = dto.az.try_get(key.as_deref(), "az").ok()?;
-            az_dto.device.id.clone()
-        }
-        Some(Cloud::Aws(profile)) => {
-            let key = profile.map(|name| name.to_string());
-            let aws_dto = dto.aws.try_get(key.as_deref(), "aws").ok()?;
-            aws_dto.device.id.clone()
-        }
+    match (
+        device_id_from_cert(&aws_device.cert_path),
+        dto_value.or_none(),
+    ) {
+        (Ok(common_name), _) => Ok(common_name),
+        (Err(_), Some(dto_value)) => Ok(dto_value.to_string()),
+        (Err(err), None) => Err(err),
     }
 }
 

crates/core/tedge/src/cli/certificate/cli.rs

@@ -8,11 +8,9 @@ use super::upload::*;
 use anyhow::anyhow;
 use camino::Utf8PathBuf;
 use clap::ValueHint;
-use tedge_config::explicit_device_id;
 use tedge_config::OptionalConfigError;
 use tedge_config::ProfileName;
 use tedge_config::TEdgeConfig;
-use tedge_config::TEdgeConfigLocation;
 
 use crate::cli::common::Cloud;
 use crate::cli::common::CloudArg;
@@ -84,12 +82,11 @@ impl BuildCommand for TEdgeCertCli {
                 let cloud: Option<Cloud> = cloud.map(<_>::try_into).transpose()?;
 
                 let cmd = CreateCertCmd {
-                    id: get_device_id(id, &config, &context.config_location, &cloud)?,
+                    id: get_device_id(id, &config, &cloud)?,
                     cert_path: config.device_cert_path(cloud.as_ref())?.to_owned(),
                     key_path: config.device_key_path(cloud.as_ref())?.to_owned(),
                     user: user.to_owned(),
                     group: group.to_owned(),
-                    config_location: context.config_location,
                 };
                 cmd.into_boxed()
             }
@@ -102,7 +99,7 @@ impl BuildCommand for TEdgeCertCli {
                 let cloud: Option<Cloud> = cloud.map(<_>::try_into).transpose()?;
 
                 let cmd = CreateCsrCmd {
-                    id: get_device_id(id, &config, &context.config_location, &cloud)?,
+                    id: get_device_id(id, &config, &cloud)?,
                     key_path: config.device_key_path(cloud.as_ref())?.to_owned(),
                     // Use output file instead of csr_path from tedge config if provided
                     csr_path: output_path.unwrap_or_else(|| config.device.csr_path.clone()),
@@ -194,32 +191,14 @@ pub enum UploadCertCli {
 fn get_device_id(
     id: Option<String>,
     config: &TEdgeConfig,
-    config_location: &TEdgeConfigLocation,
     cloud: &Option<Cloud>,
 ) -> Result<String, anyhow::Error> {
     match (id, config.device_id(cloud.as_ref()).ok()) {
         (None, None) => Err(anyhow!(
             "No device ID is provided. Use `--device-id <name>` option to specify the device ID."
         )),
         (None, Some(config_id)) => Ok(config_id.into()),
-        (Some(input_id), None) => Ok(input_id),
-        (Some(input_id), Some(config_id)) if input_id == config_id => Ok(input_id),
-        (Some(input_id), Some(_config_id)) => {
-            match explicit_device_id(config_location, &cloud.as_ref().map(Into::into)) {
-                None => {
-                    // the cloud profile doesn't have its own device.id explicitly, so using the input id is fine
-                    Ok(input_id)
-                }
-                Some(explicit_id) => {
-                    Err(anyhow!(
-                        "`--device-id` option conflicts with tedge config settings.\n\
-                        Configured value: '{explicit_id}', but input: '{input_id}'\n\n\
-                        Please either update the configuration using `tedge config set <key> <new_id>`\n\
-                        or provide the correct value with the `--device-id` option."
-            ))
-                }
-            }
-        }
+        (Some(input_id), _) => Ok(input_id),
     }
 }
 
@@ -328,51 +307,54 @@ mod tests {
         },
         "input"
     )]
-    fn validate_get_device_id_returns_ok(
-        input_id: Option<&str>,
-        cloud_arg: Option<CloudArg>,
-        toml: toml::Table,
-        expected: &str,
-    ) {
-        let cloud: Option<Cloud> = cloud_arg.map(<_>::try_into).transpose().unwrap();
-        let ttd = TempTedgeDir::new();
-        ttd.file("tedge.toml").with_toml_content(toml);
-        let location = TEdgeConfigLocation::from_custom_root(ttd.path());
-        let reader = location.load().unwrap();
-        let id = input_id.map(|s| s.to_string());
-        let result = get_device_id(id, &reader, &location, &cloud);
-        assert_eq!(result.unwrap().as_str(), expected);
-    }
-
-    #[test_case(
-        None,
-        None,
-        toml::toml!{
-            [device]
-        }
-    )]
     #[test_case(
         Some("input"),
         None,
         toml::toml!{
             [device]
             id = "test"
-        }
+        },
+        "input"
     )]
     #[test_case(
         Some("input"),
         Some(CloudArg::C8y{ profile: None }),
         toml::toml!{
             [c8y.device]
             id = "c8y-test"
-        }
+        },
+        "input"
     )]
     #[test_case(
         Some("input"),
         Some(CloudArg::C8y{ profile: Some("foo".parse().unwrap()) }),
         toml::toml!{
             [c8y.profiles.foo.device]
             id = "c8y-foo-test"
+        },
+        "input"
+    )]
+    fn validate_get_device_id_returns_ok(
+        input_id: Option<&str>,
+        cloud_arg: Option<CloudArg>,
+        toml: toml::Table,
+        expected: &str,
+    ) {
+        let cloud: Option<Cloud> = cloud_arg.map(<_>::try_into).transpose().unwrap();
+        let ttd = TempTedgeDir::new();
+        ttd.file("tedge.toml").with_toml_content(toml);
+        let location = TEdgeConfigLocation::from_custom_root(ttd.path());
+        let reader = location.load().unwrap();
+        let id = input_id.map(|s| s.to_string());
+        let result = get_device_id(id, &reader, &cloud);
+        assert_eq!(result.unwrap().as_str(), expected);
+    }
+
+    #[test_case(
+        None,
+        None,
+        toml::toml!{
+            [device]
         }
     )]
     fn validate_get_device_id_returns_err(
@@ -386,7 +368,7 @@ mod tests {
         let location = TEdgeConfigLocation::from_custom_root(ttd.path());
         let reader = location.load().unwrap();
         let id = input_id.map(|s| s.to_string());
-        let result = get_device_id(id, &reader, &location, &cloud);
+        let result = get_device_id(id, &reader, &cloud);
         assert!(result.is_err());
     }
 }

crates/core/tedge/src/cli/certificate/create.rs

@@ -11,7 +11,6 @@ use std::fs::File;
 use std::fs::OpenOptions;
 use std::io::prelude::*;
 use std::path::Path;
-use tedge_config::TEdgeConfigLocation;
 use tedge_utils::paths::set_permission;
 use tedge_utils::paths::validate_parent_dir_exists;
 
@@ -29,9 +28,6 @@ pub struct CreateCertCmd {
     /// The owner of the private key
     pub user: String,
     pub group: String,
-
-    /// The tedge.toml file location, required to access to TEdgeConfigDto
-    pub config_location: TEdgeConfigLocation,
 }
 
 impl Command for CreateCertCmd {
@@ -194,7 +190,6 @@ mod tests {
             key_path: key_path.clone(),
             user: "mosquitto".to_string(),
             group: "mosquitto".to_string(),
-            config_location: TEdgeConfigLocation::from_custom_root(dir.path()),
         };
 
         assert_matches!(
@@ -224,7 +219,6 @@ mod tests {
             key_path: key_path.clone(),
             user: "mosquitto".to_string(),
             group: "mosquitto".to_string(),
-            config_location: TEdgeConfigLocation::from_custom_root(dir.path()),
         };
 
         assert!(cmd
@@ -248,7 +242,6 @@ mod tests {
             key_path,
             user: "mosquitto".to_string(),
             group: "mosquitto".to_string(),
-            config_location: TEdgeConfigLocation::from_custom_root(dir.path()),
         };
 
         let cert_error = cmd
@@ -269,7 +262,6 @@ mod tests {
             key_path,
             user: "mosquitto".to_string(),
             group: "mosquitto".to_string(),
-            config_location: TEdgeConfigLocation::from_custom_root(dir.path()),
         };
 
         let cert_error = cmd

crates/core/tedge/src/cli/certificate/create_csr.rs

@@ -71,7 +71,6 @@ mod tests {
     use crate::CreateCertCmd;
     use assert_matches::assert_matches;
     use std::path::Path;
-    use tedge_config::TEdgeConfigLocation;
     use tempfile::*;
     use x509_parser::der_parser::asn1_rs::FromDer;
     use x509_parser::nom::AsBytes;
@@ -114,7 +113,6 @@ mod tests {
             key_path: key_path.clone(),
             user: "mosquitto".to_string(),
             group: "mosquitto".to_string(),
-            config_location: TEdgeConfigLocation::from_custom_root(dir.path()),
         };
 
         // create private key and public cert with standard command

crates/core/tedge/src/cli/certificate/renew.rs

@@ -59,7 +59,6 @@ mod tests {
     use std::path::Path;
     use std::thread::sleep;
     use std::time::Duration;
-    use tedge_config::TEdgeConfigLocation;
     use tempfile::*;
 
     #[test]
@@ -74,7 +73,6 @@ mod tests {
             key_path: key_path.clone(),
             user: "mosquitto".to_string(),
             group: "mosquitto".to_string(),
-            config_location: TEdgeConfigLocation::from_custom_root(dir.path()),
         };
 
         // First create both cert and key