From 918e9da182b5b3ccc37fc34d5c8cbfeee4948a39 Mon Sep 17 00:00:00 2001 From: Cameron Taggart Date: Fri, 5 Jan 2024 22:20:41 -0600 Subject: [PATCH] authority_host URL parsing in create --- .../client_secret_credentials.rs | 13 +++++++---- sdk/identity/src/token_credentials/options.rs | 23 ++++++++----------- .../specific_azure_credential.rs | 6 +---- .../workload_identity_credentials.rs | 22 +++++++++++++----- 4 files changed, 35 insertions(+), 29 deletions(-) diff --git a/sdk/identity/src/token_credentials/client_secret_credentials.rs b/sdk/identity/src/token_credentials/client_secret_credentials.rs index 4d09c9a262..5a5f2f2e3b 100644 --- a/sdk/identity/src/token_credentials/client_secret_credentials.rs +++ b/sdk/identity/src/token_credentials/client_secret_credentials.rs @@ -41,15 +41,15 @@ pub struct ClientSecretCredential { impl ClientSecretCredential { /// Create a new `ClientSecretCredential` pub fn new( - options: impl Into, + http_client: Arc, + authority_host: Url, tenant_id: String, client_id: String, client_secret: String, ) -> ClientSecretCredential { - let options = options.into(); ClientSecretCredential { - http_client: options.http_client().clone(), - authority_host: options.authority_host().clone(), + http_client, + authority_host, tenant_id, client_id: oauth2::ClientId::new(client_id), client_secret: Some(oauth2::ClientSecret::new(client_secret)), @@ -109,6 +109,8 @@ impl ClientSecretCredential { options: impl Into, ) -> azure_core::Result { let options = options.into(); + let http_client = options.http_client(); + let authority_host = options.authority_host()?; let env = options.env(); let tenant_id = env.var(AZURE_TENANT_ID_ENV_KEY) @@ -136,7 +138,8 @@ impl ClientSecretCredential { })?; Ok(ClientSecretCredential::new( - options, + http_client, + authority_host, tenant_id, client_id, client_secret, diff --git a/sdk/identity/src/token_credentials/options.rs b/sdk/identity/src/token_credentials/options.rs index eb5af35720..72055fed7a 100644 --- a/sdk/identity/src/token_credentials/options.rs +++ b/sdk/identity/src/token_credentials/options.rs @@ -1,9 +1,10 @@ use crate::env::Env; -use azure_core::authority_hosts::AZURE_PUBLIC_CLOUD; +use azure_core::error::{ErrorKind, ResultExt}; use std::sync::Arc; use url::Url; const AZURE_AUTHORITY_HOST_ENV_KEY: &str = "AZURE_AUTHORITY_HOST"; +const AZURE_PUBLIC_CLOUD: &str = "https://login.microsoftonline.com"; /// Provides options to configure how the Identity library makes authentication /// requests to Azure Active Directory. @@ -11,7 +12,7 @@ const AZURE_AUTHORITY_HOST_ENV_KEY: &str = "AZURE_AUTHORITY_HOST"; pub struct TokenCredentialOptions { env: Env, http_client: Arc, - authority_host: Url, + authority_host: String, } /// The default token credential options. @@ -22,8 +23,6 @@ impl Default for TokenCredentialOptions { let env = Env::default(); let authority_host = env .var(AZURE_AUTHORITY_HOST_ENV_KEY) - .map(|s| Url::parse(&s)) - .unwrap_or_else(|_| Ok(AZURE_PUBLIC_CLOUD.to_owned())) .unwrap_or_else(|_| AZURE_PUBLIC_CLOUD.to_owned()); Self { env: Env::default(), @@ -35,26 +34,24 @@ impl Default for TokenCredentialOptions { impl TokenCredentialOptions { #[cfg(test)] - pub(crate) fn new( - env: Env, - http_client: Arc, - authority_host: Url, - ) -> Self { + pub(crate) fn new(env: Env, http_client: Arc) -> Self { Self { env, http_client, - authority_host, + authority_host: AZURE_PUBLIC_CLOUD.to_owned(), } } /// Set the authority host for authentication requests. - pub fn set_authority_host(&mut self, authority_host: Url) { + pub fn set_authority_host(&mut self, authority_host: String) { self.authority_host = authority_host; } /// The authority host to use for authentication requests. The default is /// `https://login.microsoftonline.com`. - pub fn authority_host(&self) -> &Url { - &self.authority_host + pub fn authority_host(&self) -> azure_core::Result { + Url::parse(&self.authority_host).with_context(ErrorKind::DataConversion, || { + format!("invalid authority host URL {}", &self.authority_host) + }) } pub fn http_client(&self) -> Arc { diff --git a/sdk/identity/src/token_credentials/specific_azure_credential.rs b/sdk/identity/src/token_credentials/specific_azure_credential.rs index 8964f2c79e..2580f59d0b 100644 --- a/sdk/identity/src/token_credentials/specific_azure_credential.rs +++ b/sdk/identity/src/token_credentials/specific_azure_credential.rs @@ -207,11 +207,7 @@ impl TokenCredential for SpecificAzureCredential { pub fn test_options(env_vars: &[(&str, &str)]) -> TokenCredentialOptions { let env = crate::env::Env::from(env_vars); let http_client = azure_core::new_noop_client(); - TokenCredentialOptions::new( - env, - http_client, - azure_core::authority_hosts::AZURE_PUBLIC_CLOUD.to_owned(), - ) + TokenCredentialOptions::new(env, http_client) } #[cfg(test)] diff --git a/sdk/identity/src/token_credentials/workload_identity_credentials.rs b/sdk/identity/src/token_credentials/workload_identity_credentials.rs index bfba6d9eb1..67f5193cac 100644 --- a/sdk/identity/src/token_credentials/workload_identity_credentials.rs +++ b/sdk/identity/src/token_credentials/workload_identity_credentials.rs @@ -33,7 +33,8 @@ pub struct WorkloadIdentityCredential { impl WorkloadIdentityCredential { /// Create a new `WorkloadIdentityCredential` pub fn new( - options: impl Into, + http_client: Arc, + authority_host: Url, tenant_id: String, client_id: String, token: T, @@ -41,10 +42,9 @@ impl WorkloadIdentityCredential { where T: Into, { - let options = options.into(); Self { - http_client: options.http_client().clone(), - authority_host: options.authority_host().clone(), + http_client, + authority_host, tenant_id, client_id, token: token.into(), @@ -56,6 +56,8 @@ impl WorkloadIdentityCredential { options: impl Into, ) -> azure_core::Result { let options = options.into(); + let http_client = options.http_client(); + let authority_host = options.authority_host()?; let env = options.env(); let tenant_id = env.var(AZURE_TENANT_ID_ENV_KEY) @@ -79,7 +81,11 @@ impl WorkloadIdentityCredential { .map_kind(ErrorKind::Credential) { return Ok(WorkloadIdentityCredential::new( - options, tenant_id, client_id, token, + http_client, + authority_host, + tenant_id, + client_id, + token, )); } @@ -97,7 +103,11 @@ impl WorkloadIdentityCredential { }, )?; return Ok(WorkloadIdentityCredential::new( - options, tenant_id, client_id, token, + http_client, + authority_host, + tenant_id, + client_id, + token, )); }