Skip to content

{Role} az ad sp create-for-rbac: Fix "One or more properties contains invalid values"#17433

Merged
jiasli merged 1 commit intoAzure:devfrom
jiasli:sp-rbac
Apr 6, 2021
Merged

{Role} az ad sp create-for-rbac: Fix "One or more properties contains invalid values"#17433
jiasli merged 1 commit intoAzure:devfrom
jiasli:sp-rbac

Conversation

@jiasli
Copy link
Copy Markdown
Member

@jiasli jiasli commented Mar 24, 2021 •
edited
Loading

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:

azure-cli/src/azure-cli/azure/cli/command_modules/role/custom.py

Line 1414 in 50ccb24

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:

azure-cli/src/azure-cli/azure/cli/command_modules/role/custom.py

Line 1418 in 50ccb24

name = existing_sps[0].service_principal_names[0]

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

azure-cli/src/azure-cli/azure/cli/command_modules/role/custom.py

Lines 1437 to 1440 in 50ccb24

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:

azure-cli/src/azure-cli/azure/cli/command_modules/role/custom.py

Line 784 in 50ccb24

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:

azure-cli/src/azure-cli/azure/cli/command_modules/role/custom.py

Lines 820 to 822 in 50ccb24

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

azure-cli/src/azure-cli/azure/cli/command_modules/role/custom.py

Line 1414 in 50ccb24

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

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

azure-cli/src/azure-cli/azure/cli/command_modules/role/custom.py

Line 1418 in 50ccb24

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

jiasli
jiasli commented Mar 24, 2021
View reviewed changes
src/azure-cli/azure/cli/command_modules/role/custom.py
result = client.applications.list(filter=(' and '.join(sub_filters)))
if sub_filters or include_all:
return result
return list(result)
Copy link
Copy Markdown
Member Author

@jiasli jiasli Mar 24, 2021

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

azure-cli/src/azure-cli/azure/cli/command_modules/role/custom.py

Line 782 in 50ccb24

if existing_apps:

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
Copy Markdown
Collaborator

yonzhan commented Mar 24, 2021

Role

@jiasli
Copy link
Copy Markdown
Member Author

jiasli commented Mar 26, 2021

/azp run

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Mar 26, 2021

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

Reviewers

@evelyn-ys evelyn-ys evelyn-ys approved these changes

@fengzhou-msft fengzhou-msft Awaiting requested review from fengzhou-msft

@jsntcy jsntcy Awaiting requested review from jsntcy

Assignees

@jiasli jiasli

Labels

None yet

Projects

None yet

Milestone

S185

Development

Successfully merging this pull request may close these issues.

3 participants

@jiasli @yonzhan @evelyn-ys