Skip to content

Conversation

@jiasli
Copy link
Member

@jiasli jiasli commented Mar 24, 2021

Fix IcM 232405459: Running az ad sp create-for-rbac results in failure

Symptom

az ad sp create-for-rbac fails with:

Error: One or more properties contains invalid values.

Cause

For example:

ad sp create-for-rbac -n sp-prod --skip-assignment

CLI first checks the existence of the Service Principal by searching for Service Principal whose servicePrincipalNames contains http://sp-prod:

query_exp = 'servicePrincipalNames/any(x:x eq \'{}\')'.format(name)

Normally, the result will look like:

"servicePrincipalNames": [
  "http://sp-prod",
  "1f440000-0000-0000-0000-000000000000"
]

Under some unknown cases, in AAD's response of a Service Principal, the order is reversed:

"servicePrincipalNames":[
  "1f440000-0000-0000-0000-000000000000",
  "http://sp-prod"
]

So CLI uses the 1st item 1f440000-0000-0000-0000-000000000000 as name:

name = existing_sps[0].service_principal_names[0]

Then CLI uses name (1f440000-0000-0000-0000-000000000000) as identifierUri to create an Application:

aad_application = create_application(cmd,
display_name=app_display_name,
homepage=homepage,
identifier_uris=[name],

It first checks whether there is an Application whose identifierUris contains 1f440000-0000-0000-0000-000000000000:

existing_apps = [x for x in existing_apps if set(identifier_uris).issubset(set(x.identifier_uris))]

This will certainly fail as 1f440000-0000-0000-0000-000000000000 is the appId, not identifierUri:

> az ad app show --id 1f440000-0000-0000-0000-000000000000
{
  "appId": "1f440000-0000-0000-0000-000000000000",
  ...
  "identifierUris": [
    "http://sp-prod"
  ],
  ...

So CLI will use 1f440000-0000-0000-0000-000000000000 as a identifierUri to create a new Application:

app_create_param = ApplicationCreateParameters(available_to_other_tenants=available_to_other_tenants,
display_name=display_name,
identifier_uris=identifier_uris,

This is why PUT is called, instead of PATCH. This will certainly fail as 1f440000-0000-0000-0000-000000000000 is not a valid identifierUri (should start with http://).

Change

As the query for searching Service Principal already uses name (http://sp-prod):

query_exp = 'servicePrincipalNames/any(x:x eq \'{}\')'.format(name)

there is simply no need to re-set name to the first item:

name = existing_sps[0].service_principal_names[0]

In other words, name already has correct value (http://sp-prod).

Temporary mitigation

As the Service Principal's response is not always consistent, the user should delete the old Application with az ad app delete and run az ad sp create-for-rbac from scratch (without existing Service Principal and Application.)

result = client.applications.list(filter=(' and '.join(sub_filters)))
if sub_filters or include_all:
return result
return list(result)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

result returned by client.applications.list is an Iterator. It is later used by

which will always evaluate to True.

result should be converted to a list before being evaluated by if.

Example:

it = iter([])
print(bool(it))
# True

@jiasli jiasli self-assigned this Mar 24, 2021
@jiasli jiasli added this to the S185 milestone Mar 24, 2021
@yonzhan
Copy link
Collaborator

yonzhan commented Mar 24, 2021

Role

@jiasli
Copy link
Member Author

jiasli commented Mar 26, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@jiasli jiasli merged commit 6225ca5 into Azure:dev Apr 6, 2021
@jiasli jiasli deleted the sp-rbac branch April 6, 2021 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants