Move to using api:// for AAD Application "identifier uris" (#1243)

This commit is contained in:
Stas
2021-09-17 11:04:19 -07:00
committed by GitHub
parent 3f891e5572
commit 2e267a894f
4 changed files with 108 additions and 60 deletions

View File

@ -111,19 +111,20 @@ impl ClientCredentials {
} }
pub async fn access_token(&self) -> Result<AccessToken> { pub async fn access_token(&self) -> Result<AccessToken> {
let (authority, resource) = if let Some(domain) = &self.multi_tenant_domain { let (authority, scope) = {
let url = Url::parse(&self.resource.clone())?; let url = Url::parse(&self.resource.clone())?;
let host = url.host_str().ok_or_else(|| { let host = url.host_str().ok_or_else(|| {
anyhow::format_err!("resource URL does not have a host string: {}", url) anyhow::format_err!("resource URL does not have a host string: {}", url)
})?; })?;
if let Some(domain) = &self.multi_tenant_domain {
let instance: Vec<&str> = host.split('.').collect(); let instance: Vec<&str> = host.split('.').collect();
( (
String::from("common"), String::from("common"),
format!("https://{}/{}/", &domain, instance[0]), format!("api://{}/{}/", &domain, instance[0]),
) )
} else { } else {
(self.tenant.clone(), self.resource.clone()) (self.tenant.clone(), format!("api://{}/", host))
}
}; };
let mut url = Url::parse("https://login.microsoftonline.com")?; let mut url = Url::parse("https://login.microsoftonline.com")?;
@ -139,7 +140,7 @@ impl ClientCredentials {
("client_secret", self.client_secret.expose_ref().to_string()), ("client_secret", self.client_secret.expose_ref().to_string()),
("grant_type", "client_credentials".into()), ("grant_type", "client_credentials".into()),
("tenant", authority), ("tenant", authority),
("scope", format!("{}.default", resource)), ("scope", format!("{}.default", scope)),
]) ])
.send_retry_default() .send_retry_default()
.await .await
@ -180,15 +181,17 @@ const MANAGED_IDENTITY_URL: &str =
impl ManagedIdentityCredentials { impl ManagedIdentityCredentials {
pub fn new(resource: String, multi_tenant_domain: Option<String>) -> Result<Self> { pub fn new(resource: String, multi_tenant_domain: Option<String>) -> Result<Self> {
let resource = if let Some(domain) = multi_tenant_domain.clone() { let resource = {
let resource_url = Url::parse(&resource)?; let resource_url = Url::parse(&resource)?;
let host = resource_url.host_str().ok_or_else(|| { let host = resource_url.host_str().ok_or_else(|| {
anyhow::format_err!("resource URL does not have a host string: {}", resource_url) anyhow::format_err!("resource URL does not have a host string: {}", resource_url)
})?; })?;
let instance: Vec<&str> = host.split('.').collect(); if let Some(domain) = multi_tenant_domain.clone() {
format!("https://{}/{}", domain, instance[0]) let instance: Vec<&str> = host.split('.').collect();
} else { format!("api://{}/{}", domain, instance[0])
resource } else {
format!("api://{}", host)
}
}; };
Ok(Self { Ok(Self {
@ -249,7 +252,7 @@ mod tests {
#[test] #[test]
fn test_managed_creds_with_valid_single_tenant() -> Result<()> { fn test_managed_creds_with_valid_single_tenant() -> Result<()> {
let resource = "https://host-26.azurewebsites.net"; let resource = "api://host-26.azurewebsites.net";
let managed_creds = ManagedIdentityCredentials::new(resource.to_string(), None)?; let managed_creds = ManagedIdentityCredentials::new(resource.to_string(), None)?;
@ -259,7 +262,7 @@ mod tests {
#[test] #[test]
fn test_managed_creds_with_valid_multi_tenant_domain() -> Result<()> { fn test_managed_creds_with_valid_multi_tenant_domain() -> Result<()> {
let resource = "https://host-26.azurewebsites.net"; let resource = "api://host-26.azurewebsites.net";
let multi_tenant_domain = "mycloud.contoso.com"; let multi_tenant_domain = "mycloud.contoso.com";
let managed_creds = ManagedIdentityCredentials::new( let managed_creds = ManagedIdentityCredentials::new(
@ -267,7 +270,7 @@ mod tests {
Some(multi_tenant_domain.to_string()), Some(multi_tenant_domain.to_string()),
)?; )?;
let expected = "https://mycloud.contoso.com/host-26"; let expected = "api://mycloud.contoso.com/host-26";
assert_eq!(managed_creds.resource, expected); assert_eq!(managed_creds.resource, expected);

View File

@ -177,10 +177,12 @@ class Backend:
if self.config.tenant_domain: if self.config.tenant_domain:
endpoint = urlparse(self.config.endpoint).netloc.split(".")[0] endpoint = urlparse(self.config.endpoint).netloc.split(".")[0]
scopes = [ scopes = [
"https://" + self.config.tenant_domain + "/" + endpoint + "/.default" f"api://{self.config.tenant_domain}/{endpoint}/.default",
f"https://{self.config.tenant_domain}/{endpoint}/.default",
] ]
else: else:
scopes = [self.config.endpoint + "/.default"] netloc = urlparse(self.config.endpoint).netloc
scopes = [f"api://{netloc}/.default", f"https://{netloc}/.default"]
if self.config.client_secret: if self.config.client_secret:
return self.client_secret(scopes) return self.client_secret(scopes)
@ -194,7 +196,15 @@ class Backend:
client_credential=self.config.client_secret, client_credential=self.config.client_secret,
token_cache=self.token_cache, token_cache=self.token_cache,
) )
result = self.app.acquire_token_for_client(scopes=scopes)
for scope in scopes:
result = self.app.acquire_token_for_client(scopes=[scope])
# AADSTS500011: The resource principal named ... was not found in the tenant named ...
# This error is caused by a by mismatch between the identifierUr and the scope provided in the request.
if "error" in result and "AADSTS500011" in result["error_description"]:
LOGGER.warning(f"failed to get access token with scope {scope}")
continue
if "error" in result: if "error" in result:
raise Exception( raise Exception(
"error: %s\n'%s'" "error: %s\n'%s'"
@ -210,26 +220,47 @@ class Backend:
token_cache=self.token_cache, token_cache=self.token_cache,
) )
accounts = self.app.get_accounts() for scope in scopes:
if accounts: accounts = self.app.get_accounts()
access_token = self.app.acquire_token_silent(scopes, account=accounts[0]) if accounts:
if access_token: access_token = self.app.acquire_token_silent(
return access_token [scope], account=accounts[0]
)
if access_token:
return access_token
LOGGER.info("Attempting interactive device login") for scope in scopes:
print("Please login", flush=True) LOGGER.info("Attempting interactive device login")
print("Please login", flush=True)
flow = self.app.initiate_device_flow(scopes=scopes) flow = self.app.initiate_device_flow(scopes=[scope])
check_msal_error(flow, ["user_code", "message"])
print(flow["message"], flush=True)
access_token = self.app.acquire_token_by_device_flow(flow) check_msal_error(flow, ["user_code", "message"])
check_msal_error(access_token, ["access_token"]) # setting the expiration time to allow us to retry the interactive login with a new scope
flow["expires_at"] = int(time.time()) + 90 # 90 seconds from now
print(flow["message"], flush=True)
LOGGER.info("Interactive device authentication succeeded") access_token = self.app.acquire_token_by_device_flow(flow)
print("Login succeeded", flush=True) # AADSTS70016: OAuth 2.0 device flow error. Authorization is pending
self.save_cache() # this happens when the intractive login request times out. This heppens when the login
return access_token # fails because of a scope mismatch.
if (
"error" in access_token
and "AADSTS70016" in access_token["error_description"]
):
LOGGER.warning(f"failed to get access token with scope {scope}")
continue
check_msal_error(access_token, ["access_token"])
LOGGER.info("Interactive device authentication succeeded")
print("Login succeeded", flush=True)
self.save_cache()
break
if access_token:
return access_token
else:
raise Exception("Failed to acquire token")
def request( def request(
self, self,

View File

@ -20,8 +20,8 @@
"app_func_issuer": { "app_func_issuer": {
"type": "string" "type": "string"
}, },
"app_func_audience": { "app_func_audiences": {
"type": "string" "type": "array"
}, },
"multi_tenant_domain": { "multi_tenant_domain": {
"type": "string" "type": "string"
@ -283,9 +283,7 @@
"clientSecret": "[parameters('clientSecret')]", "clientSecret": "[parameters('clientSecret')]",
"issuer": "[parameters('app_func_issuer')]", "issuer": "[parameters('app_func_issuer')]",
"defaultProvider": "AzureActiveDirectory", "defaultProvider": "AzureActiveDirectory",
"allowedAudiences": [ "allowedAudiences": "[parameters('app_func_audiences')]",
"[parameters('app_func_audience')]"
],
"isAadAutoProvisioned": false "isAadAutoProvisioned": false
} }
}, },

View File

@ -310,7 +310,7 @@ class Client:
params = ApplicationCreateParameters( params = ApplicationCreateParameters(
display_name=self.application_name, display_name=self.application_name,
identifier_uris=[url], identifier_uris=[f"api://{self.application_name}.azurewebsites.net"],
reply_urls=[url + "/.auth/login/aad/callback"], reply_urls=[url + "/.auth/login/aad/callback"],
optional_claims=OptionalClaims(id_token=[], access_token=[]), optional_claims=OptionalClaims(id_token=[], access_token=[]),
required_resource_access=[ required_resource_access=[
@ -362,6 +362,22 @@ class Client:
else: else:
app = existing[0] app = existing[0]
if self.multi_tenant_domain:
api_id = "api://%s/%s" % (
self.multi_tenant_domain,
self.application_name,
)
else:
api_id = "api://%s.azurewebsites.net" % self.application_name
if api_id not in app.identifier_uris:
identifier_uris = app.identifier_uris
identifier_uris.append(api_id)
client.applications.patch(
app.object_id,
ApplicationUpdateParameters(identifier_uris=identifier_uris),
)
existing_role_values = [app_role.value for app_role in app.app_roles] existing_role_values = [app_role.value for app_role in app.app_roles]
has_missing_roles = any( has_missing_roles = any(
[role.value not in existing_role_values for role in app_roles] [role.value not in existing_role_values for role in app_roles]
@ -383,23 +399,12 @@ class Client:
) )
if self.multi_tenant_domain and app.sign_in_audience == "AzureADMyOrg": if self.multi_tenant_domain and app.sign_in_audience == "AzureADMyOrg":
url = "https://%s/%s" % (
self.multi_tenant_domain,
self.application_name,
)
client.applications.patch(
app.object_id, ApplicationUpdateParameters(identifier_uris=[url])
)
set_app_audience(app.object_id, "AzureADMultipleOrgs") set_app_audience(app.object_id, "AzureADMultipleOrgs")
elif ( elif (
not self.multi_tenant_domain not self.multi_tenant_domain
and app.sign_in_audience == "AzureADMultipleOrgs" and app.sign_in_audience == "AzureADMultipleOrgs"
): ):
set_app_audience(app.object_id, "AzureADMyOrg") set_app_audience(app.object_id, "AzureADMyOrg")
url = "https://%s.azurewebsites.net" % self.application_name
client.applications.patch(
app.object_id, ApplicationUpdateParameters(identifier_uris=[url])
)
else: else:
logger.debug("No change to App Registration signInAudence setting") logger.debug("No change to App Registration signInAudence setting")
@ -471,20 +476,31 @@ class Client:
if self.multi_tenant_domain: if self.multi_tenant_domain:
# clear the value in the Issuer Url field: # clear the value in the Issuer Url field:
# https://docs.microsoft.com/en-us/sharepoint/dev/spfx/use-aadhttpclient-enterpriseapi-multitenant # https://docs.microsoft.com/en-us/sharepoint/dev/spfx/use-aadhttpclient-enterpriseapi-multitenant
app_func_audience = "https://%s/%s" % ( app_func_audiences = [
self.multi_tenant_domain, "api://%s/%s"
self.application_name, % (
) self.multi_tenant_domain,
self.application_name,
),
"https://%s/%s"
% (
self.multi_tenant_domain,
self.application_name,
),
]
app_func_issuer = "" app_func_issuer = ""
multi_tenant_domain = {"value": self.multi_tenant_domain} multi_tenant_domain = {"value": self.multi_tenant_domain}
else: else:
app_func_audience = "https://%s.azurewebsites.net" % self.application_name app_func_audiences = [
"api://%s.azurewebsites.net" % self.application_name,
"https://%s.azurewebsites.net" % self.application_name,
]
tenant_oid = str(self.cli_config["authority"]).split("/")[-1] tenant_oid = str(self.cli_config["authority"]).split("/")[-1]
app_func_issuer = "https://sts.windows.net/%s/" % tenant_oid app_func_issuer = "https://sts.windows.net/%s/" % tenant_oid
multi_tenant_domain = {"value": ""} multi_tenant_domain = {"value": ""}
params = { params = {
"app_func_audience": {"value": app_func_audience}, "app_func_audiences": {"value": app_func_audiences},
"name": {"value": self.application_name}, "name": {"value": self.application_name},
"owner": {"value": self.owner}, "owner": {"value": self.owner},
"clientId": {"value": self.results["client_id"]}, "clientId": {"value": self.results["client_id"]},