diff --git a/src/agent/onefuzz/src/auth.rs b/src/agent/onefuzz/src/auth.rs index 8f26414fd..c58383f36 100644 --- a/src/agent/onefuzz/src/auth.rs +++ b/src/agent/onefuzz/src/auth.rs @@ -111,19 +111,20 @@ impl ClientCredentials { } pub async fn access_token(&self) -> Result { - let (authority, resource) = if let Some(domain) = &self.multi_tenant_domain { + let (authority, scope) = { let url = Url::parse(&self.resource.clone())?; let host = url.host_str().ok_or_else(|| { anyhow::format_err!("resource URL does not have a host string: {}", url) })?; - - let instance: Vec<&str> = host.split('.').collect(); - ( - String::from("common"), - format!("https://{}/{}/", &domain, instance[0]), - ) - } else { - (self.tenant.clone(), self.resource.clone()) + if let Some(domain) = &self.multi_tenant_domain { + let instance: Vec<&str> = host.split('.').collect(); + ( + String::from("common"), + format!("api://{}/{}/", &domain, instance[0]), + ) + } else { + (self.tenant.clone(), format!("api://{}/", host)) + } }; let mut url = Url::parse("https://login.microsoftonline.com")?; @@ -139,7 +140,7 @@ impl ClientCredentials { ("client_secret", self.client_secret.expose_ref().to_string()), ("grant_type", "client_credentials".into()), ("tenant", authority), - ("scope", format!("{}.default", resource)), + ("scope", format!("{}.default", scope)), ]) .send_retry_default() .await @@ -180,15 +181,17 @@ const MANAGED_IDENTITY_URL: &str = impl ManagedIdentityCredentials { pub fn new(resource: String, multi_tenant_domain: Option) -> Result { - let resource = if let Some(domain) = multi_tenant_domain.clone() { + let resource = { let resource_url = Url::parse(&resource)?; let host = resource_url.host_str().ok_or_else(|| { anyhow::format_err!("resource URL does not have a host string: {}", resource_url) })?; - let instance: Vec<&str> = host.split('.').collect(); - format!("https://{}/{}", domain, instance[0]) - } else { - resource + if let Some(domain) = multi_tenant_domain.clone() { + let instance: Vec<&str> = host.split('.').collect(); + format!("api://{}/{}", domain, instance[0]) + } else { + format!("api://{}", host) + } }; Ok(Self { @@ -249,7 +252,7 @@ mod tests { #[test] 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)?; @@ -259,7 +262,7 @@ mod tests { #[test] 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 managed_creds = ManagedIdentityCredentials::new( @@ -267,7 +270,7 @@ mod tests { 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); diff --git a/src/cli/onefuzz/backend.py b/src/cli/onefuzz/backend.py index 3964234d2..c413e62ce 100644 --- a/src/cli/onefuzz/backend.py +++ b/src/cli/onefuzz/backend.py @@ -177,10 +177,12 @@ class Backend: if self.config.tenant_domain: endpoint = urlparse(self.config.endpoint).netloc.split(".")[0] 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: - 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: return self.client_secret(scopes) @@ -194,7 +196,15 @@ class Backend: client_credential=self.config.client_secret, 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: raise Exception( "error: %s\n'%s'" @@ -210,26 +220,47 @@ class Backend: token_cache=self.token_cache, ) - accounts = self.app.get_accounts() - if accounts: - access_token = self.app.acquire_token_silent(scopes, account=accounts[0]) - if access_token: - return access_token + for scope in scopes: + accounts = self.app.get_accounts() + if accounts: + access_token = self.app.acquire_token_silent( + [scope], account=accounts[0] + ) + if access_token: + return access_token - LOGGER.info("Attempting interactive device login") - print("Please login", flush=True) + for scope in scopes: + LOGGER.info("Attempting interactive device login") + print("Please login", flush=True) - flow = self.app.initiate_device_flow(scopes=scopes) - check_msal_error(flow, ["user_code", "message"]) - print(flow["message"], flush=True) + flow = self.app.initiate_device_flow(scopes=[scope]) - access_token = self.app.acquire_token_by_device_flow(flow) - check_msal_error(access_token, ["access_token"]) + check_msal_error(flow, ["user_code", "message"]) + # 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") - print("Login succeeded", flush=True) - self.save_cache() - return access_token + access_token = self.app.acquire_token_by_device_flow(flow) + # AADSTS70016: OAuth 2.0 device flow error. Authorization is pending + # this happens when the intractive login request times out. This heppens when the login + # 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( self, diff --git a/src/deployment/azuredeploy.json b/src/deployment/azuredeploy.json index 7d9eea5da..5b461511c 100644 --- a/src/deployment/azuredeploy.json +++ b/src/deployment/azuredeploy.json @@ -20,8 +20,8 @@ "app_func_issuer": { "type": "string" }, - "app_func_audience": { - "type": "string" + "app_func_audiences": { + "type": "array" }, "multi_tenant_domain": { "type": "string" @@ -283,9 +283,7 @@ "clientSecret": "[parameters('clientSecret')]", "issuer": "[parameters('app_func_issuer')]", "defaultProvider": "AzureActiveDirectory", - "allowedAudiences": [ - "[parameters('app_func_audience')]" - ], + "allowedAudiences": "[parameters('app_func_audiences')]", "isAadAutoProvisioned": false } }, diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index 17ab75389..0f428ae45 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -310,7 +310,7 @@ class Client: params = ApplicationCreateParameters( display_name=self.application_name, - identifier_uris=[url], + identifier_uris=[f"api://{self.application_name}.azurewebsites.net"], reply_urls=[url + "/.auth/login/aad/callback"], optional_claims=OptionalClaims(id_token=[], access_token=[]), required_resource_access=[ @@ -362,6 +362,22 @@ class Client: else: 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] has_missing_roles = any( [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": - 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") elif ( not self.multi_tenant_domain and app.sign_in_audience == "AzureADMultipleOrgs" ): 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: logger.debug("No change to App Registration signInAudence setting") @@ -471,20 +476,31 @@ class Client: if self.multi_tenant_domain: # clear the value in the Issuer Url field: # https://docs.microsoft.com/en-us/sharepoint/dev/spfx/use-aadhttpclient-enterpriseapi-multitenant - app_func_audience = "https://%s/%s" % ( - self.multi_tenant_domain, - self.application_name, - ) + app_func_audiences = [ + "api://%s/%s" + % ( + self.multi_tenant_domain, + self.application_name, + ), + "https://%s/%s" + % ( + self.multi_tenant_domain, + self.application_name, + ), + ] app_func_issuer = "" multi_tenant_domain = {"value": self.multi_tenant_domain} 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] app_func_issuer = "https://sts.windows.net/%s/" % tenant_oid multi_tenant_domain = {"value": ""} params = { - "app_func_audience": {"value": app_func_audience}, + "app_func_audiences": {"value": app_func_audiences}, "name": {"value": self.application_name}, "owner": {"value": self.owner}, "clientId": {"value": self.results["client_id"]},