Skip to content
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 92 additions & 4 deletions src/auth/src/credentials/service_account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
//! "private_key_id": "test-private-key-id",
//! "private_key": "<YOUR_PKCS8_PEM_KEY_HERE>",
//! "project_id": "test-project-id",
//! "universe_domain": "test-universe-domain",
//! "universe_domain": "googleapis.com",
//! });
//! let credentials: Credentials = Builder::new(service_account_key)
//! .with_quota_project_id("my-quota-project")
Expand Down Expand Up @@ -196,7 +196,7 @@ impl AccessSpecifier {
/// "private_key_id": "test-private-key-id",
/// "private_key": "<YOUR_PKCS8_PEM_KEY_HERE>",
/// "project_id": "test-project-id",
/// "universe_domain": "test-universe-domain",
/// "universe_domain": "googleapis.com",
/// });
/// let credentials = Builder::new(key)
/// .with_access_specifier(AccessSpecifier::from_audience("https://pubsub.googleapis.com"))
Expand All @@ -207,6 +207,7 @@ pub struct Builder {
service_account_key: Value,
access_specifier: AccessSpecifier,
quota_project_id: Option<String>,
universe_domain: Option<String>,
iam_endpoint_override: Option<String>,
}

Expand All @@ -222,6 +223,7 @@ impl Builder {
service_account_key,
access_specifier: AccessSpecifier::Scopes([DEFAULT_SCOPE].map(str::to_string).to_vec()),
quota_project_id: None,
universe_domain: None,
iam_endpoint_override: None,
}
}
Expand Down Expand Up @@ -265,6 +267,31 @@ impl Builder {
self
}

/// Sets the universe domain for this credentials.
///
/// The universe domain is the default service domain for a given cloud universe.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I don't think "default" is correct.

I asked Gemini to rewrite this and it spat out:

/// Sets the Google Cloud universe domain for these credentials.
///
/// A "universe" is an isolated Google Cloud environment, such as the public 
/// cloud or a sovereign/air-gapped deployment (e.g., Google Distributed Cloud). 
/// The universe domain acts as the base URL for constructing API endpoints 
/// within that environment.
///
/// By default, this is set to `googleapis.com`. You should only override this 
/// if your application is operating within a custom Cloud universe and needs 
/// to direct authentication and service requests to a different base endpoint.

Consider taking any pieces of that that look good.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, #5259.

Maybe this should be the documentation for the Credentials::universe_domain accessor, and here we just say "override the value" without needing to explain what is a universe.

/// Any value provided here overrides a `universe_domain` value from the input service account JSON.
///
/// # Example
/// ```
/// # use google_cloud_auth::credentials::service_account::Builder;
/// # use serde_json::json;
/// # async fn sample() -> anyhow::Result<()> {
/// # let config = json!({
/// # "type": "service_account",
/// # "client_email": "foo@bar.com",
/// # "private_key": "---BEGIN---"
/// # });
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this could just be json!({}) if we are hiding it.

/// let credentials = Builder::new(config)
/// .with_universe_domain("googleapis.com")
/// .build()?;
/// # Ok(()) }
/// ```
pub fn with_universe_domain<S: Into<String>>(mut self, universe_domain: S) -> Self {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment: hmm, if we can override things with a builder function, why can't we override things with the GOOGLE_CLOUD_UNIVERSE_DOMAIN env var?

w/e. Not my spec.

self.universe_domain = Some(universe_domain.into());
self
}

#[cfg(all(test, google_cloud_unstable_trusted_boundaries))]
fn maybe_iam_endpoint_override(mut self, iam_endpoint_override: Option<String>) -> Self {
self.iam_endpoint_override = iam_endpoint_override;
Expand Down Expand Up @@ -313,7 +340,7 @@ impl Builder {
/// "private_key_id": "test-private-key-id",
/// "private_key": "-----BEGIN PRIVATE KEY-----\nBLAHBLAHBLAH\n-----END PRIVATE KEY-----\n",
/// "project_id": "test-project-id",
/// "universe_domain": "test-universe-domain",
/// "universe_domain": "googleapis.com",
/// });
/// let credentials: AccessTokenCredentials = Builder::new(service_account_key)
/// .with_quota_project_id("my-quota-project")
Expand Down Expand Up @@ -343,15 +370,19 @@ impl Builder {
) -> BuildResult<CredentialsWithAccessBoundary<ServiceAccountCredentials<TokenCache>>> {
let iam_endpoint = self.iam_endpoint_override.clone();
let quota_project_id = self.quota_project_id.clone();
let universe_domain_override = self.universe_domain.clone();
let token_provider = self.build_token_provider()?;
let client_email = token_provider.service_account_key.client_email.clone();
let universe_domain =
universe_domain_override.or(token_provider.service_account_key.universe_domain.clone());
let access_boundary_url = crate::access_boundary::service_account_lookup_url(
&client_email,
iam_endpoint.as_deref(),
);
let creds = ServiceAccountCredentials {
quota_project_id,
token_provider: TokenCache::new(token_provider),
universe_domain: universe_domain.clone(),
};

Ok(CredentialsWithAccessBoundary::new(
Expand Down Expand Up @@ -485,6 +516,7 @@ where
{
token_provider: T,
quota_project_id: Option<String>,
universe_domain: Option<String>,
}

#[derive(Debug)]
Expand Down Expand Up @@ -601,6 +633,10 @@ where
.maybe_quota_project_id(self.quota_project_id.as_deref())
.build()
}

async fn universe_domain(&self) -> Option<String> {
self.universe_domain.clone()
}
}

#[async_trait::async_trait]
Expand Down Expand Up @@ -686,6 +722,7 @@ mod tests {
let sac = ServiceAccountCredentials {
token_provider: TokenCache::new(mock),
quota_project_id: None,
universe_domain: None,
};

let mut extensions = Extensions::new();
Expand Down Expand Up @@ -729,6 +766,7 @@ mod tests {
let sac = ServiceAccountCredentials {
token_provider: TokenCache::new(mock),
quota_project_id: Some(quota_project.to_string()),
universe_domain: None,
};

let headers = get_headers_from_cache(sac.headers(Extensions::new()).await.unwrap())?;
Expand Down Expand Up @@ -757,6 +795,7 @@ mod tests {
let sac = ServiceAccountCredentials {
token_provider: TokenCache::new(mock),
quota_project_id: None,
universe_domain: None,
};
let result = sac.headers(Extensions::new()).await;
assert!(result.is_err(), "{result:?}");
Expand Down Expand Up @@ -832,7 +871,7 @@ mod tests {
"private_key_id": "test-private-key-id",
"private_key": private_key,
"project_id": "test-project-id",
"universe_domain": "test-universe-domain"
"universe_domain": "googleapis.com"
});

let credentials = Builder::new(json_value).build()?;
Expand Down Expand Up @@ -866,6 +905,55 @@ mod tests {
Ok(())
}

#[tokio::test]
#[parallel]
async fn universe_domain() -> TestResult {
let private_key = PKCS8_PK.clone();
// SA key without universe_domain
let json_value = json!({
"client_email": "test-client-email",
"private_key_id": "test-private-key-id",
"private_key": private_key,
"project_id": "test-project-id",
});

let credentials = Builder::new(json_value).build()?;

let universe_domain = credentials.universe_domain().await;
assert_eq!(universe_domain, None, "{universe_domain:?}");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: no need for the extra printing when using assert_eq!. It only adds value for like assert!(universe_domain.is_none(), "{universe_domain:?}");

here and below.


// SA key with universe_domain
let json_value = json!({
"client_email": "test-client-email",
"private_key_id": "test-private-key-id",
"private_key": private_key,
"project_id": "test-project-id",
"universe_domain": "some-universe-domain.com"
});

let credentials = Builder::new(json_value.clone()).build()?;

let universe_domain = credentials.universe_domain().await;
assert_eq!(
universe_domain.as_deref(),
Some("some-universe-domain.com"),
"{universe_domain:?}"
);

let credentials = Builder::new(json_value)
.with_universe_domain("other-universe-domain.com")
.build()?;

let universe_domain = credentials.universe_domain().await;
assert_eq!(
universe_domain.as_deref(),
Some("other-universe-domain.com"),
"{universe_domain:?}"
);

Ok(())
}

#[tokio::test]
#[parallel]
async fn get_service_account_headers_invalid_key_failure() -> TestResult {
Expand Down
Loading