-
Notifications
You must be signed in to change notification settings - Fork 93
feat(auth): allow customers to provide a SubjectTokenProvider impl #2383
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
56be8e6
094c769
11573ee
d15957d
4ee6c3a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,7 @@ | |
|
|
||
| use super::dynamic::CredentialsProvider; | ||
| use super::external_account_sources::executable_sourced::ExecutableSourcedCredentials; | ||
| use super::external_account_sources::programmatic_sourced::ProgrammaticSourcedCredentials; | ||
| use super::external_account_sources::url_sourced::UrlSourcedCredentials; | ||
| use super::internal::sts_exchange::{ClientAuthentication, ExchangeTokenRequest, STSHandler}; | ||
| use super::{CacheableResource, Credentials}; | ||
|
|
@@ -27,13 +28,31 @@ use http::{Extensions, HeaderMap}; | |
| use serde::{Deserialize, Serialize}; | ||
| use serde_json::Value; | ||
| use std::collections::HashMap; | ||
| use std::future::Future; | ||
| use std::sync::Arc; | ||
| use tokio::time::{Duration, Instant}; | ||
|
|
||
| #[async_trait::async_trait] | ||
| pub(crate) trait SubjectTokenProvider: std::fmt::Debug + Send + Sync { | ||
| /// Generate subject token that will be used on STS exchange. | ||
| async fn subject_token(&self) -> Result<String>; | ||
| pub trait SubjectTokenProvider: std::fmt::Debug + Send + Sync { | ||
| fn subject_token(&self) -> impl Future<Output = Result<String>> + Send; | ||
| } | ||
|
Comment on lines
+35
to
+37
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So to override this I need to implement my own client, error detection, parsing, etc.? Bummer.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
|
|
||
| pub(crate) mod dynamic { | ||
| use super::Result; | ||
| #[async_trait::async_trait] | ||
| pub trait SubjectTokenProvider: std::fmt::Debug + Send + Sync { | ||
| /// Generate subject token that will be used on STS exchange. | ||
| async fn subject_token(&self) -> Result<String>; | ||
| } | ||
|
|
||
| #[async_trait::async_trait] | ||
| impl<T> SubjectTokenProvider for T | ||
| where | ||
| T: super::SubjectTokenProvider, | ||
| { | ||
| async fn subject_token(&self) -> Result<String> { | ||
| T::subject_token(self).await | ||
| } | ||
| } | ||
| } | ||
|
|
||
| #[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] | ||
|
|
@@ -61,6 +80,7 @@ enum CredentialSourceFile { | |
| Executable { | ||
| executable: ExecutableConfig, | ||
| }, | ||
| Programmatic {}, | ||
| File {}, | ||
| Aws {}, | ||
| } | ||
|
|
@@ -111,6 +131,7 @@ impl From<CredentialSourceFile> for CredentialSource { | |
| CredentialSourceFile::Executable { executable } => { | ||
| Self::Executable(ExecutableSourcedCredentials::new(executable)) | ||
| } | ||
| CredentialSourceFile::Programmatic {} => Self::Programmatic {}, | ||
| CredentialSourceFile::File { .. } => { | ||
| unimplemented!("file sourced credential not supported yet") | ||
| } | ||
|
|
@@ -139,6 +160,7 @@ enum CredentialSource { | |
| Executable(ExecutableSourcedCredentials), | ||
| File {}, | ||
| Aws {}, | ||
| Programmatic {}, | ||
| } | ||
|
|
||
| impl ExternalAccountConfig { | ||
|
|
@@ -151,6 +173,11 @@ impl ExternalAccountConfig { | |
| CredentialSource::Executable(source) => { | ||
| Self::make_credentials_from_source(source, config, quota_project_id) | ||
| } | ||
| CredentialSource::Programmatic {} => { | ||
| panic!( | ||
| "programmatic sourced credential should set a subject token provider implementation via external_account::Builder::with_subject_token_provider method" | ||
| ) | ||
| } | ||
| CredentialSource::File { .. } => { | ||
| unimplemented!("file sourced credential not supported yet") | ||
| } | ||
|
|
@@ -166,7 +193,7 @@ impl ExternalAccountConfig { | |
| quota_project_id: Option<String>, | ||
| ) -> Credentials | ||
| where | ||
| T: SubjectTokenProvider + 'static, | ||
| T: dynamic::SubjectTokenProvider + 'static, | ||
| { | ||
| let token_provider = ExternalAccountTokenProvider { | ||
| subject_token_provider, | ||
|
|
@@ -185,7 +212,7 @@ impl ExternalAccountConfig { | |
| #[derive(Debug)] | ||
| struct ExternalAccountTokenProvider<T> | ||
| where | ||
| T: SubjectTokenProvider, | ||
| T: dynamic::SubjectTokenProvider, | ||
| { | ||
| subject_token_provider: T, | ||
| config: ExternalAccountConfig, | ||
|
|
@@ -194,7 +221,7 @@ where | |
| #[async_trait::async_trait] | ||
| impl<T> TokenProvider for ExternalAccountTokenProvider<T> | ||
| where | ||
| T: SubjectTokenProvider, | ||
| T: dynamic::SubjectTokenProvider, | ||
| { | ||
| async fn token(&self) -> Result<Token> { | ||
| let subject_token = self.subject_token_provider.subject_token().await?; | ||
|
|
@@ -284,6 +311,7 @@ pub struct Builder { | |
| external_account_config: Value, | ||
| quota_project_id: Option<String>, | ||
| scopes: Option<Vec<String>>, | ||
| subject_token_provider: Option<Box<dyn dynamic::SubjectTokenProvider>>, | ||
| } | ||
|
|
||
| impl Builder { | ||
|
|
@@ -295,6 +323,7 @@ impl Builder { | |
| external_account_config, | ||
| quota_project_id: None, | ||
| scopes: None, | ||
| subject_token_provider: None, | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -323,6 +352,16 @@ impl Builder { | |
| self | ||
| } | ||
|
|
||
| /// bring your own custom implementation of | ||
| /// SubjectTokenProvider for OIDC/SAML credentials. | ||
| 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 | ||
| } | ||
|
Comment on lines
+357
to
+363
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when setting the subject token provider via the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm that maybe ok. When implementing this change lets make sure to document that clearly.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another way could be to introduce |
||
|
|
||
| /// Returns a [Credentials] instance with the configured settings. | ||
| /// | ||
| /// # Errors | ||
|
|
@@ -344,6 +383,16 @@ impl Builder { | |
| } | ||
|
|
||
| let config: ExternalAccountConfig = file.into(); | ||
| if let Some(subject_token_provider) = self.subject_token_provider { | ||
| let source = ProgrammaticSourcedCredentials { | ||
| subject_token_provider, | ||
| }; | ||
| return Ok(ExternalAccountConfig::make_credentials_from_source( | ||
| source, | ||
| config, | ||
| self.quota_project_id, | ||
| )); | ||
| } | ||
|
|
||
| Ok(config.make_credentials(self.quota_project_id)) | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| // Copyright 2025 Google LLC | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // https://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| use crate::Result; | ||
| use crate::credentials::external_account::dynamic::SubjectTokenProvider; | ||
|
|
||
| #[derive(Debug)] | ||
| pub(crate) struct ProgrammaticSourcedCredentials { | ||
| pub subject_token_provider: Box<dyn SubjectTokenProvider>, | ||
| } | ||
|
|
||
| #[async_trait::async_trait] | ||
| impl SubjectTokenProvider for ProgrammaticSourcedCredentials { | ||
| async fn subject_token(&self) -> Result<String> { | ||
| return self.subject_token_provider.subject_token().await; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the advantage of using a trait with a single function vs. a thing that implements the
AsyncFntrait?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 likeCredentialsProvider, but this one in particular has two methods, so makes sense to be a trait.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
trying to use
AsyncFnand as it's optional on theexternal_account::Builder, I'm not finding a way to use withoutdynandAsyncFnis notdyncompatible. Any tips ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is not quite right.
AsyncFnis stable:rust-lang/rust#132706
Manually implementing the
AsyncFntrait is not stable. That is, you cannot sayimpl AsyncFn for MyType ...because the types and methods of the trait are unstable. But you can assume thatAsyncFnwill be around, and that you can call such functions.As to the
dyn-compatibleproblems: you could solve that with a private trait:But overall it seems that
AsyncFnis more trouble than it is worth.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Thank you for the context!.