{Role} Add warning about using non-unique display names when creating service principal #32437
Conversation
️✔️AzureCLI-FullTest
|
|
Hi @ReaNAiveD, |
️✔️AzureCLI-BreakingChangeTest
|
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
microsoft-github-policy-service
bot
requested review from
calvinhzy and
yonzhan
There was a problem hiding this comment.
Pull Request Overview
This PR adds a warning about the risks of using non-unique display names when creating service principals with az ad sp create-for-rbac, addressing issue #30427.
- Adds a runtime warning message displayed when the command is executed
- Updates the help documentation with an IMPORTANT notice about the same issue
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/azure-cli/azure/cli/command_modules/role/custom.py | Adds logger.warning to alert users about display name non-uniqueness risks at runtime |
| src/azure-cli/azure/cli/command_modules/role/_help.py | Adds IMPORTANT section to command documentation explaining display name risks and recommending object ID or app ID usage |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ReaNAiveD
modified the milestones:
January 2026 (2026-01-06),
December 2025 (2025-12-02)
ReaNAiveD
changed the title
az ad sp create-for-rbac: Add warning about using non-unique display names
|
|
||
|
|
||
| def create_service_principal(cmd, identifier): | ||
| logger.warning("The `az ad sp create` command can modify an existing application or service principal " |
There was a problem hiding this comment.
Should we consider displaying the warning only when the command is overwriting an existing SP?
There was a problem hiding this comment.
Should we consider displaying the warning only when the command is overwriting an existing SP?
We already have a warning when overwriting an existing SP.
I have added the new warning before it.
| **IMPORTANT**: The `az ad sp create` command can modify an existing application or service principal if | ||
| another object shares the same **display name**. Display names aren't unique and can change, which | ||
| could result in credential loss or incorrect RBAC assignments. Use a **unique object ID or app ID** instead. |
There was a problem hiding this comment.
Can we consider a shorter message with a link to our conceptual docs where the user can learn about the issue and have some code to help finding the existing application / sp ?
| type: command | ||
| short-summary: Create a service principal. | ||
| long-summary: >- | ||
| **IMPORTANT**: The `az ad sp create` command can modify an existing application or service principal if |
There was a problem hiding this comment.
Only az ad app create and az ad sp create-for-rbac can modify an existing application. az ad sp create will not.
| type: command | ||
| short-summary: Create a service principal. | ||
| long-summary: >- | ||
| **IMPORTANT**: The `az ad sp create` command can modify an existing application or service principal if |
There was a problem hiding this comment.
We usually don't use Markdown syntax besides `code`, as this will make the in-tool help hard to read.
There was a problem hiding this comment.
Changed to double quota, i.e. "az ad app create". Also removed the markdown symbol like *
| show_auth_in_json=None, skip_assignment=False, keyvault=None): | ||
| import time | ||
|
|
||
| logger.warning("The `az ad sp create-for-rbac` command can modify an existing application or service principal " |
There was a problem hiding this comment.
Same here. I don't really like the idea of showing this warning unconditionally.
There was a problem hiding this comment.
Move the warning to the create_appication method. The warning will only show when there is exact one existing app with the same display_name and before the existing warning Found an existing application instance: (id) %s. We will patch it.
jiasli
left a comment
jiasli
left a comment
There was a problem hiding this comment.
Please address the comments first.
ReaNAiveD
force-pushed
the
ad-sp-create-for-rbac-warning
branch
from
1f74b72 to
23fd0c0
Compare
| short-summary: Create an application. | ||
| long-summary: For more detailed documentation, see https://learn.microsoft.com/graph/api/resources/application | ||
| long-summary: >- | ||
| IMPORTANT: The "az ad app create" command can modify an existing application or service principal if |
There was a problem hiding this comment.
We usually use WARNING: instead of IMPORTANT:. See #16081
| long-summary: >- | ||
| IMPORTANT: The "az ad app create" command can modify an existing application or service principal if | ||
| another object shares the same display name. Display names aren't unique and can change, which | ||
| could result in credential loss or incorrect RBAC assignments. Use a unique object ID or app ID instead. |
There was a problem hiding this comment.
It is only possible for the user to choose a unique display name. The user cannot choose to use a unique object ID or app ID. The object ID or app ID is generated by the MS Graph service.
There was a problem hiding this comment.
Removed the sentence Use a unique object ID or app ID instead.
| IMPORTANT: The "az ad app create" command can modify an existing application or service principal if | ||
| another object shares the same display name. Display names aren't unique and can change, which | ||
| could result in credential loss or incorrect RBAC assignments. Use a unique object ID or app ID instead. | ||
| For more details, see https://go.microsoft.com/fwlink/?linkid=2342455. |
There was a problem hiding this comment.
This link doesn't give more details than this help message:
There was a problem hiding this comment.
Yes. It might help to reach out to Mike and Alex to see if they can add more context to the documentation.
| For more details, see https://go.microsoft.com/fwlink/?linkid=2342455. | ||
| For more detailed documentation, see https://learn.microsoft.com/graph/api/resources/application |
There was a problem hiding this comment.
It's better to describe what documentation - the Microsoft Entra application, not this az ad app create command:
| For more detailed documentation, see https://learn.microsoft.com/graph/api/resources/application | |
| For more detailed documentation on Microsoft Entra application, see https://learn.microsoft.com/graph/api/resources/application |
| helps['ad sp create'] = """ | ||
| type: command | ||
| short-summary: Create a service principal. | ||
| long-summary: |
There was a problem hiding this comment.
The long summary should not be empty.
| logger.warning("IMPORTANT: The \"az %s\" command can modify an existing application or service principal " | ||
| "if another object shares the same display name. " | ||
| "Display names aren't unique and can change, " | ||
| "which could result in credential loss or incorrect RBAC assignments. " | ||
| "Use a unique object ID or app ID instead. For more details, " | ||
| "see https://go.microsoft.com/fwlink/?linkid=2342455.", cmd.name) |
There was a problem hiding this comment.
Since len(existing_apps) == 1 already evaluates to True, that means there is already an application that shares the same display name, so using conditional sentence (can or if) is not unnecessary.
It's better to combine this warning message with L624 and say:
Please notice that display names aren't unique, which could result in credential loss or incorrect RBAC assignments. To create a new application, use a unique display name instead. For more details, see https://go.microsoft.com/fwlink/?linkid=2342455.
There was a problem hiding this comment.
I’ve updated the warning as suggested. But I also reserved the warning line indicating the existing application ID to ensure clarity.
…play names in `az ad sp create-for-rbac`
…`az ad sp create-for-rbac`
…sp create-for-rbac` commands for non-unique display names
… in `az ad app create` and `az ad sp create-for-rbac` commands
…nd is overwriting an existing SP
ReaNAiveD
force-pushed
the
ad-sp-create-for-rbac-warning
branch
from
8a28faf to
75b7dca
Compare
ReaNAiveD
modified the milestones:
December 2025 (2025-12-02),
January 2026 (2026-01-13)
5 participants
Related command
az ad app createaz ad sp create-for-rbacDescription
Add a in-tool warning about using non-unique display names to create a service principal.
For more details, see #30427
Testing Guide
History Notes
{Role}
az ad app create: Add warning about using non-unique display names inaz ad app create{Role}
az ad sp create-for-rbac: Add warning about using non-unique display names inaz ad sp create-for-rbacThis checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.