Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

Commit

Permalink
Move to using api:// for AAD Application "identifier uris" (#1243)
Browse files Browse the repository at this point in the history
  • Loading branch information
@stishkin
stishkin authored Sep 17, 2021
1 parent 3f891e5 commit 2e267a8
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 64 deletions.
39 changes: 21 additions & 18 deletions src/agent/onefuzz/src/auth.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,19 +111,20 @@ impl ClientCredentials {
}

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 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")?;
Expand All @@ -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
Expand Down Expand Up @@ -180,15 +181,17 @@ const MANAGED_IDENTITY_URL: &str =

impl ManagedIdentityCredentials {
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 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 {
Expand Down Expand Up @@ -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)?;

Expand All @@ -259,15 +262,15 @@ 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(
resource.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);

Expand Down
77 changes: 54 additions & 23 deletions src/cli/onefuzz/backend.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,10 +177,12 @@ def get_access_token(self) -> Any:
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)
Expand All @@ -194,7 +196,15 @@ def client_secret(self, scopes: List[str]) -> Any:
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'"
Expand All @@ -210,26 +220,47 @@ def device_login(self, scopes: List[str]) -> Any:
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

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)

access_token = self.app.acquire_token_by_device_flow(flow)
check_msal_error(access_token, ["access_token"])

LOGGER.info("Interactive device authentication succeeded")
print("Login succeeded", flush=True)
self.save_cache()
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

for scope in scopes:
LOGGER.info("Attempting interactive device login")
print("Please login", flush=True)

flow = self.app.initiate_device_flow(scopes=[scope])

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)

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,
Expand Down
8 changes: 3 additions & 5 deletions src/deployment/azuredeploy.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
"app_func_issuer": {
"type": "string"
},
"app_func_audience": {
"type": "string"
"app_func_audiences": {
"type": "array"
},
"multi_tenant_domain": {
"type": "string"
Expand Down Expand Up @@ -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
}
},
Expand Down
52 changes: 34 additions & 18 deletions src/deployment/deploy.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ def setup_rbac(self) -> None:

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=[
Expand Down Expand Up @@ -362,6 +362,22 @@ def try_sp_create() -> None:

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]
Expand All @@ -383,23 +399,12 @@ def try_sp_create() -> None:
)

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")

Expand Down Expand Up @@ -471,20 +476,31 @@ def deploy_template(self) -> None:
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"]},
Expand Down

0 comments on commit 2e267a8

Please sign in to comment.