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

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

Merged
merged 5 commits into from
Sep 17, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
39 changes: 21 additions & 18 deletions src/agent/onefuzz/src/auth.rs
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
5 changes: 3 additions & 2 deletions src/cli/onefuzz/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,10 +177,11 @@ 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"
"api://" + self.config.tenant_domain + "/" + endpoint + "/.default"
]
else:
scopes = [self.config.endpoint + "/.default"]
netloc = urlparse(self.config.endpoint).netloc
scopes = [f"api://{netloc}/.default"]

if self.config.client_secret:
return self.client_secret(scopes)
Expand Down
8 changes: 3 additions & 5 deletions src/deployment/azuredeploy.json
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
43 changes: 34 additions & 9 deletions src/deployment/deploy.py
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 @@ -387,8 +387,16 @@ def try_sp_create() -> None:
self.multi_tenant_domain,
self.application_name,
)
api_url = "api://%s/%s" % (
self.multi_tenant_domain,
self.application_name,
)
if not app.identifier_uris.contains(api_url):
chkeita marked this conversation as resolved.
Show resolved Hide resolved
app.identifier_uris.append(api_url)

client.applications.patch(
app.object_id, ApplicationUpdateParameters(identifier_uris=[url])
app.object_id,
ApplicationUpdateParameters(identifier_uris=app.identifier_uris),
)
set_app_audience(app.object_id, "AzureADMultipleOrgs")
elif (
Expand All @@ -397,8 +405,14 @@ def try_sp_create() -> None:
):
set_app_audience(app.object_id, "AzureADMyOrg")
url = "https://%s.azurewebsites.net" % self.application_name
api = "api://%s.azurewebsites.net" % self.application_name

if not app.identifier_uris.contains(api_url):
chkeita marked this conversation as resolved.
Show resolved Hide resolved
app.identifier_uris.append(api_url)

client.applications.patch(
app.object_id, ApplicationUpdateParameters(identifier_uris=[url])
app.object_id,
ApplicationUpdateParameters(identifier_uris=app.identifier_uris),
)
else:
logger.debug("No change to App Registration signInAudence setting")
Expand Down Expand Up @@ -471,20 +485,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