Codecov Report

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

📢 Thoughts on this report? Let us know!

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.

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,

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.

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.

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()?.

The PR #3326 took over the contents, therefore, I would close this PR.