Implement provider and auth bootstrapping #58
Conversation
hug-dev
left a comment
hug-dev
left a comment
There was a problem hiding this comment.
Thanks for the changes! Will make it much more easier to use the client. Just a few comments but agree globally.
src/core/basic_client.rs
Outdated
| AuthType::UnixPeerCredentials => { | ||
| self.auth_data = AuthenticationData::UnixPeerCredentials | ||
| } | ||
| _ => continue, |
There was a problem hiding this comment.
Are you sure we want a wildcard watch here? Might be good to trigger a compilation error if we add one more AuthType to the interface.
edit: I see you might have done that to cover the case where the clients do not support an authentication method. But even in those case, it might be good to have a compilation error to at least be aware of it and have a specific log message maybe?
There was a problem hiding this comment.
We could have a specific log message without having to have one match branch per type - just capturing and printing the AuthType that was not supported. Apart from that I don't think this failing is of much use, since there's no guarantee that when we add the new variant in the interface is also when we'll implement the functionality for it.
There was a problem hiding this comment.
I've added a log, but not a separate arm for each type
| }; | ||
| client.set_default_provider()?; | ||
| client.set_default_auth(app_name)?; | ||
| Ok(client) |
There was a problem hiding this comment.
Might be good to log which authenticator and provider were selected at the end!
src/auth.rs
Outdated
| /// Please check the | ||
| /// [Parsec Threat Model](https://parallaxsecond.github.io/parsec-book/parsec_security/parsec_threat_model/threat_model.html) | ||
| /// for more information. | ||
| AppIdentity(String), |
There was a problem hiding this comment.
Do we keep AppIdentity here then and not use Direct 😬 ?
There was a problem hiding this comment.
Oh, indeed, forgot about that
This commit implements bootstrapping of the `BasicClient`. Previously, the client would be initialised without any implicit provider and with a mandatory authentication value. Now the client can be initialised with default values for both, with a conditional parameter required for the case when direct authentication is in use. Signed-off-by: Ionut Mihalcea <ionut.mihalcea@arm.com>
2 participants
This commit implements bootstrapping of the
BasicClient. Previously,the client would be initialised without any implicit provider and
with a mandatory authentication value. Now the client can be initialised
with default values for both, with a conditional parameter required for
the case when direct authentication is in use.
Signed-off-by: Ionut Mihalcea ionut.mihalcea@arm.com