[RBAC] BREAKING CHANGE: fix issue #11883 az role assignment create scope default to subscription if empty #11977
Conversation
…ope default to subscription if empty
…ope default to subscription if empty
…to fix/role_assignment
…ope default to subscription if empty
…to fix/role_assignment
| @@ -3,6 +3,10 @@ | |||
| Release History | |||
| =============== | |||
|
|
|||
| **RBAC** | |||
|
|
|||
| * [BREAKING CHANGE] Fix issue #11883 az role assignment create scope default to subscription if empty | |||
There was a problem hiding this comment.
Please follow the history convention like:
Fix #11883:
az role assignment create: scope defaults to subscription if not specified
| namespace.resource_group_name = None # drop configured defaults | ||
|
|
||
| if not namespace.show_all and not namespace.scope and not namespace.resource_group_name: | ||
| raise CLIError('usage error: please specify at least one of "--all", "--scope" and "--resource-group".') |
There was a problem hiding this comment.
We usually use single quotes in error messages.
There was a problem hiding this comment.
I searched similar error message in our code base, seems we usually use nothing in such case. The error message will be at least one of --scope and --resource-group in the new PR.
| if not namespace.show_all and not namespace.scope and not namespace.resource_group_name: | ||
| raise CLIError('usage error: please specify at least one of "--all", "--scope" and "--resource-group".') |
There was a problem hiding this comment.
Why the behavior of az role assignment list is also changed? There doesn't seem to be any security concern in this command.
Also, it is also a breaking change to force one of --all, --scope and -resource-group, which needs to go to the history.
| resource_group = namespace.resource_group_name | ||
| if namespace.scope and resource_group and getattr(resource_group, 'is_default', None): | ||
| namespace.resource_group_name = None # drop configured defaults |
There was a problem hiding this comment.
This is a duplicate of L90-L92. I'd suggested to extract it to a function like drop_default_group.
|
I think the security concern in the original issue #11883 is when We can error out here: The error can be something like: if scope == "":
raise CLIError('Invalid scope. Please use --help to view the valid format.')One interesting thing I noticed is that the role assignment REST API does support empty scope. In that case, the create call is PUT https://management.azure.com/providers/Microsoft.Authorization/roleAssignments/a5b85ff6-2fbe-454d-bbc3-6a90f9c54d62?api-version=2018-09-01-previewAnd the response is {
"canDelegate": null,
"id": "/providers/Microsoft.Authorization/roleAssignments/a5b85ff6-2fbe-454d-bbc3-6a90f9c54d62",
"name": "a5b85ff6-2fbe-454d-bbc3-6a90f9c54d62",
"principalId": "",
"principalName": "",
"principalType": "User",
"roleDefinitionId": "/subscriptions/0b1f6471-1bf0-4dda-aec3-cb9272f09590/providers/Microsoft.Authorization/roleDefinitions/b24988ac-6180-42a0-ab88-20f7382dd24c",
"roleDefinitionName": "Contributor",
"scope": "/",
"type": "Microsoft.Authorization/roleAssignments"
}Notice Anyway, let's error out for empty scope |
This checklist is used to make sure that common guidelines for a pull request are followed.
The PR has modified HISTORY.rst describing any customer-facing, functional changes. Note that this does not include changes only to help content. (see Modifying change log).
I adhere to the Command Guidelines.