Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
26 changes: 15 additions & 11 deletions src/azure-cli/azure/cli/command_modules/role/_help.py
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,16 @@
The output includes credentials that you must protect. Be sure that you do not include these credentials
in your code or check the credentials into your source control. As an alternative, consider using
[managed identities](https://aka.ms/azadsp-managed-identities) if available to avoid the need to use credentials.


By default, this command assigns the 'Contributor' role to the service principal at the subscription scope.
To reduce your risk of a compromised service principal, use --skip-assignment to avoid creating a role assignment,
then assign a more specific role and narrow the scope to a resource or resource group.
See [steps to add a role assignment](https://aka.ms/azadsp-more) for more information.


WARNING: In a future release, this command will NOT create a 'Contributor' role assignment by default.
If needed, use the --role argument to explicitly create a role assignment.
parameters:
- name: --name -n
short-summary: A URI to use as the logic name. It doesn't need to exist. If not present, CLI will generate one.
Expand All @@ -410,19 +420,13 @@
short-summary: Role of the service principal.
examples:
- name: Create with a default role assignment.
text: >
az ad sp create-for-rbac
text: az ad sp create-for-rbac
- name: Create using a custom name, and with a default assignment.
text: >
az ad sp create-for-rbac -n "MyApp"
text: az ad sp create-for-rbac -n "MyApp"
- name: Create without a default assignment.
text: >
az ad sp create-for-rbac --skip-assignment
- name: Create with customized contributor assignments.
text: |
az ad sp create-for-rbac -n "MyApp" --role contributor \\
--scopes /subscriptions/{SubID}/resourceGroups/{ResourceGroup1} \\
/subscriptions/{SubID}/resourceGroups/{ResourceGroup2}
text: az ad sp create-for-rbac --skip-assignment
- name: Create with a Contributor role assignments on specified scope.
text: az ad sp create-for-rbac -n "MyApp" --role Contributor --scopes /subscriptions/{SubID}/resourceGroups/{ResourceGroup1} /subscriptions/{SubID}/resourceGroups/{ResourceGroup2}
- name: Create using a self-signed certificate.
text: az ad sp create-for-rbac --create-cert
- name: Create using a self-signed certificate, and store it within KeyVault.
Expand Down
12 changes: 10 additions & 2 deletions src/azure-cli/azure/cli/command_modules/role/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@
"The output includes credentials that you must protect. Be sure that you do not include these credentials in "
"your code or check the credentials into your source control. For more information, see https://aka.ms/azadsp-cli")

ROLE_ASSIGNMENT_CREATE_WARNING = (
"In a future release, this command will NOT create a 'Contributor' role assignment by default. "
"If needed, use the --role argument to explicitly create a role assignment."
)

logger = get_logger(__name__)

# pylint: disable=too-many-lines
Expand Down Expand Up @@ -1401,7 +1406,7 @@ def _validate_app_dates(app_start_date, app_end_date, cert_start_date, cert_end_
# pylint: disable=inconsistent-return-statements
def create_service_principal_for_rbac(
# pylint:disable=too-many-statements,too-many-locals, too-many-branches
cmd, name=None, years=None, create_cert=False, cert=None, scopes=None, role='Contributor',
cmd, name=None, years=None, create_cert=False, cert=None, scopes=None, role=None,
Copy link
Member Author

Choose a reason for hiding this comment

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

Need to detect whether role is explicitly set and show the warning if not.

If role='Contributor' is used, it won't be possible to detect if role is provided by the user or as the default value.

See https://stackoverflow.com/a/14749388

Copy link
Member Author

Choose a reason for hiding this comment

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

Another option is to check role.is_default, and show the warning message accordingly. If role is assigned a default value, its type will be knack.validators.DefaultStr which has is_default field indicating whether it is the default value.

show_auth_for_sdk=None, skip_assignment=False, keyvault=None):
import time

Expand Down Expand Up @@ -1483,8 +1488,11 @@ def create_service_principal_for_rbac(

# retry while server replication is done
if not skip_assignment:
if not role:
role = "Contributor"
logger.warning(ROLE_ASSIGNMENT_CREATE_WARNING)
for scope in scopes:
logger.warning('Creating a role assignment under the scope of "%s"', scope)
logger.warning("Creating '%s' role assignment under scope '%s'", role, scope)
Copy link
Member Author

@jiasli jiasli Nov 30, 2020

Choose a reason for hiding this comment

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

Echo the role name to be more explicit and informative.

for retry_time in range(0, _RETRY_TIMES):
try:
_create_role_assignment(cmd.cli_ctx, role, sp_oid, None, scope, resolve_assignee=False)
Expand Down