feat: read device.id from credentials.toml when basic auth mode

[rina23q]

Proposed changes

Draft for further discussion.

1. Technically, it would be very difficult to switch "read-only" mode or "read-write" mode depending on c8y.auth_mode. This PR proposes to keep device_id in the credentials.toml file alternatively.
2. Reading all configs under c8y requires a profile name. However, it cannot be determined when tedge config get device.id is called. I parsed None as the device profile. Correct me if I am wrong.

    let c8y_profile: Option<&ProfileName> = None;
    let c8y_config = reader.c8y.try_get(c8y_profile)?;

Types of changes

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
• Documentation Update (if none of the other choices apply)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue

#3240

Checklist

• I have read the CONTRIBUTING doc
• I have signed the CLA (in all commits with git commit -s)
• I ran cargo fmt as mentioned in CODING_GUIDELINES
• I used cargo clippy as mentioned in CODING_GUIDELINES
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)

Further comments

[codecov]

Codecov Report

Attention: Patch coverage is 25.00000% with 15 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
..._config/src/tedge_config_cli/models/auth_method.rs	0.00%	10 Missing ⚠️
.../tedge_config/src/tedge_config_cli/tedge_config.rs	50.00%	4 Missing and 1 partial ⚠️

Additional details and impacted files

📢 Thoughts on this report? Let us know!

[albinsuresh]

Technically, it would be very difficult to switch "read-only" mode or "read-write" mode depending on c8y.auth_mode. This PR proposes to keep device_id in the credentials.toml file alternatively.

Why don't we permanently switch this setting to read-write mode and apply the following rule:

1. If auth_mode == basic then read the written value
2. If auth_mode == certificate then read from the certificate

If someone consciously sets device.id in the tedge.toml and then uses the certificate mode as well, then we can just validate if both values are the same, else fail during tedge connect.

[rina23q]

Since both c8y.auth_mode and c8y.credentials_path can differ based on a cloud profile, we should not use any c8y config for device.id. In essence, device.id must be identical regardless of cloud profiles, so now I'm convinced to use tedge.toml to store the value of device ID for basic auth.

Proposal 1:

device.id returns a value from the certificate when the certificate file exists. Else, returns a value from tedge.toml.

If tedge config macro has try_from() attribute, the implementation would be nice. We already have from() attribute. This usecase needs something that returns Result<String, ReadError>.

Probably the device.id definition can be like below:

#[tedge_config(try_from(
    error = "\
        The device id is read from the device certificate if it exists.\n\
        To create a device certificate, you can use `tedge cert create --device-id <id>`.",
    function = "device_id",
))]
#[tedge_config(example = "Raspberrypi-4d18303a-6d3a-11eb-b1a6-175f6bb72665")]
#[tedge_config(note = "This setting is derived from the device certificate if it is set.")]
#[doku(as = "String")]
id: Result<String, ReadError>,

Proposal 2:

device.id returns a value from the certificate if it is not set (=default). If it is set, the value is read from tedge.toml.

Since it removes the block for the inconsistency of the certificate's CN and the value of device.id, it can be checked while tedge connect as proposed here.

If someone consciously sets device.id in the tedge.toml and then uses the certificate mode as well, then we can just validate if both values are the same, else fail during tedge connect.

The code can look like below.

#[tedge_config(note ="\
        The device id is read from the device certificate if it exists.\n\
        To create a device certificate, you can use `tedge cert create --device-id <id>`."
)]
#[tedge_config(default(function = "device_id")]
#[tedge_config(example = "Raspberrypi-4d18303a-6d3a-11eb-b1a6-175f6bb72665")]
#[doku(as = "String")]
id: String,

[albinsuresh]

Since both c8y.auth_mode and c8y.credentials_path can differ based on a cloud profile, we should not use any c8y config for device.id. In essence, device.id must be identical regardless of cloud profiles, so now I'm convinced to use tedge.toml to store the value of device ID for basic auth.

Proposal 1:

device.id returns a value from the certificate when the certificate file exists. Else, returns a value from tedge.toml.

Proposal 2:

device.id returns a value from the certificate if it is not set (=default). If it is set, the value is read from tedge.toml.

I'd go with proposal 2, as that is sounds straightforward. If someone has explicitly set a value using tedge config set, he'd want to use it. And do that extra validation if they are equal in tedge connect.

[didier-wenzek]

Since both c8y.auth_mode and c8y.credentials_path can differ based on a cloud profile, we should not use any c8y config for device.id. In essence, device.id must be identical regardless of cloud profiles, so now I'm convinced to use tedge.toml to store the value of device ID for basic auth.
Proposal 1:
device.id returns a value from the certificate when the certificate file exists. Else, returns a value from tedge.toml.
Proposal 2:
device.id returns a value from the certificate if it is not set (=default). If it is set, the value is read from tedge.toml.

Things are already too complicated and both proposals are only making things more complicated.

I do think we must stop reading the device id from the certificate:

• today, we have this issue with password authentication where there is no more certificate to read from
• tomorrow, we will have issues with cloud endpoints using each a different certificate.

If someone consciously sets device.id in the tedge.toml and then uses the certificate mode as well, then we can just validate if both values are the same, else fail during tedge connect.

This is the key. I propose to:

• store device.id in tedge.toml as a regular setting.
• update the tedge cert create command, to make the --device-id optional and to update the config if provided.
• add a check on tedge connect to reject a certificate which CN is not the device id or a password that is not for the device id.

[jarhodes314]

Adding a bit of further detail to Didier's proposed solution as we discussed, we'll need to gracefully handle cases where the device ID isn't explicitly set. This can be done by setting #[tedge_config(reader(private))] on all the device.id fields in the config macro, preventing any code trying to read the device ID from accessing it directly. Then you can add a method fn id(&self) -> Result<_, _> to TEdgeConfgReaderC8yDevice etc. that can read the device certificate or use the ID field set in the toml. This should work as a drop in replacement for the existing read-only device ID field, which is already accessed by calling tedge_config.c8y.try_get(profile)?.device.id()?.