feat(auth): allow customers to provide a SubjectTokenProvider impl #2383
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2383 +/- ##
==========================================
- Coverage 95.24% 95.23% -0.02%
==========================================
Files 76 77 +1
Lines 3050 3061 +11
==========================================
+ Hits 2905 2915 +10
- Misses 145 146 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| fn subject_token(&self) -> impl Future<Output = Result<String>> + Send; | ||
| } |
There was a problem hiding this comment.
What is the advantage of using a trait with a single function vs. a thing that implements the AsyncFn trait?
There was a problem hiding this comment.
TIL AsyncFn. Gonna check it out how to use that. But to your point, I was just trying to follow the same pattern from the repo with things like CredentialsProvider, but this one in particular has two methods, so makes sense to be a trait.
There was a problem hiding this comment.
AsyncFn is a Rust 2024 feature.... I feat it might not work if the caller is using Rust Edition 2021, so make sure it does, otherwise the trait is a better idea.
There was a problem hiding this comment.
trying to use AsyncFn and as it's optional on the external_account::Builder , I'm not finding a way to use without dyn and AsyncFn is not dyn compatible. Any tips ?
There was a problem hiding this comment.
Says here that asyncfn is nightly-only experimental API https://doc.rust-lang.org/std/ops/trait.AsyncFn.html
There was a problem hiding this comment.
Says here that asyncfn is nightly-only experimental API https://doc.rust-lang.org/std/ops/trait.AsyncFn.html
That is not quite right. AsyncFn is stable:
Manually implementing the AsyncFn trait is not stable. That is, you cannot say impl AsyncFn for MyType ... because the types and methods of the trait are unstable. But you can assume that AsyncFn will be around, and that you can call such functions.
As to the dyn-compatible problems: you could solve that with a private trait:
struct Foo<T> {
function: T
}
#[async_trait]
trait MyAsyncFn {
async fn call() -> Blah;
}
#[async_trait]
impl MyAsyncFn for Foo<T> where T: AsyncFn<....> {
async fn call() -> Blah { function().await }
}But overall it seems that AsyncFn is more trouble than it is worth.
There was a problem hiding this comment.
I see. Thank you for the context!.
| pub fn with_subject_token_provider<T: SubjectTokenProvider + 'static>( | ||
| mut self, | ||
| subject_token_provider: T, | ||
| ) -> Self { | ||
| self.subject_token_provider = Some(Box::new(subject_token_provider)); | ||
| self | ||
| } |
There was a problem hiding this comment.
If we are taking subject token provider as an input in the builder, we need to make some more changes as well. It becomes tricky now. There is also a credential source in the config they initialized the builder with.
There was a problem hiding this comment.
when setting the subject token provider via the Bulder, it overrides the credential source from the config
There was a problem hiding this comment.
Hmm that maybe ok. When implementing this change lets make sure to document that clearly.
There was a problem hiding this comment.
Another way could be to introduce new(subject_token_provider). But that means we add builder methods to provide all other fields like impersonation url, provider ppol, token url etc.
| pub trait SubjectTokenProvider: std::fmt::Debug + Send + Sync { | ||
| fn subject_token(&self) -> impl Future<Output = Result<String>> + Send; | ||
| } |
There was a problem hiding this comment.
So to override this I need to implement my own client, error detection, parsing, etc.? Bummer.
There was a problem hiding this comment.
I think the main point of this custom subject token provider is indeed for the customer own all the stack around fetching the subject token, specially in complex scenarios like the ones we discussed where the customers needs to use a custom openssl version and other details. I think we are also evaluating how to allow injecting custom headers and/or provide a custom http client cc @sai-sunder-s
There was a problem hiding this comment.
Yeah this is for people with advanced use case and know what they are doing. It is just complete freedom to do whatever they want. Yes that means they have to handle error detection and parsing as well but that should be ok. For example, they know best how to handle a kerberos error.
|
Can we close this? |
|
superseded by #2541 |
Draft to showcase one idea on how to allow customers to provide their own implementation of SubjectTokenProvider.