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
32 changes: 32 additions & 0 deletions src/azure-cli/azure/cli/command_modules/role/_help.py
Original file line number Diff line number Diff line change
Expand Up @@ -585,12 +585,44 @@
examples:
- name: Create role assignment for an assignee.
text: az role assignment create --assignee sp_name --role a_role
- name: Create role assignment for an assignee with description and condition.
text: >-

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What's the difference between >- and |?

@bim-msft bim-msft Sep 14, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@qwordy
image
In short: >- will replace all single \n to whitespace (especially, the final \n will be removed), and combine double \n to one.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I use >- to make the code more readable purposefully.

az role assignment create --role "Owner" --assignee "Jhon.Doe@Contoso.com"
--description "Role assignment foo to check on bar"
--condition "@Resource[Microsoft.Storage/storageAccounts/blobServices/containers:Name] stringEquals 'foo'"
--condition-version "2.0"
supported-profiles: latest
- name: Create a new role assignment for a user, group, or service principal. (autogenerated)
text: |
az role assignment create --assignee 00000000-0000-0000-0000-000000000000 --role "Storage Account Key Operator Service Role" --scope $id
crafted: true
"""


helps['role assignment update'] = """
type: command
short-summary: Update an existing role assignment for a user, group, or service principal.
examples:
- name: Update a role assignment from a JSON file.
text: az role assignment update --role-assignment assignment.json
- name: Update a role assignment from a JSON string. (Bash)
text: |
az role assignment update --role-assignment '{
"canDelegate": null,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we add parameters like --can-deleagate so that users can see help messages.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Taking JSON as an input is asked by the service team. I had the same suggestion as you to expose explicit arguments but the suggestion was rejected.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

CLI is yours. You fix it. Be brave. Go ahead.

"condition": "@Resource[Microsoft.Storage/storageAccounts/blobServices/containers:Name] stringEquals '"'"'foo'"'"'",
"conditionVersion": "2.0",
"description": "Role assignment foo to check on bar",
"id": "/subscriptions/00000001-0000-0000-0000-000000000000/resourceGroups/rg1/providers/Microsoft.Authorization/roleAssignments/3eabdd43-375b-4dbd-8dc4-04acd15ce56b",
"name": "3eabdd43-375b-4dbd-8dc4-04acd15ce56b",
"principalId": "00000002-0000-0000-0000-000000000000",
"principalType": "User",
"resourceGroup": "rg1",
"roleDefinitionId": "/subscriptions/00000001-0000-0000-0000-000000000000/providers/Microsoft.Authorization/roleDefinitions/acdd72a7-3385-48ef-bd42-f606fba81ae7",
"scope": "/subscriptions/00000001-0000-0000-0000-000000000000/resourceGroups/rg1",
"type": "Microsoft.Authorization/roleAssignments"
}'
"""

helps['role assignment delete'] = """
type: command
short-summary: Delete role assignments.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,21 @@ def create_role_definition(self, client, role_name, role_id, role_definition_inp
scope = role_definition_input['assignableScopes'][0]
return client.create_or_update(role_definition_id=role_id, scope=scope, role_definition=role_configuration)

def create_role_assignment(self, client, assignment_name, role_id, object_id, scope, assignee_principal_type=None):
def create_role_assignment(self, client, assignment_name, role_id, object_id, scope, assignee_principal_type=None,
description=None, condition=None, condition_version=None):
RoleAssignmentCreateParameters = get_sdk(
self.cli_ctx, ResourceType.MGMT_AUTHORIZATION,
'RoleAssignmentProperties' if self.old_api else 'RoleAssignmentCreateParameters',
mod='models', operation_group='role_assignments')
parameters = RoleAssignmentCreateParameters(role_definition_id=role_id, principal_id=object_id)
if assignee_principal_type:
parameters.principal_type = assignee_principal_type
if description:
parameters.description = description
if condition:
parameters.condition = condition
if condition_version:
parameters.condition_version = condition_version
return client.create(scope, assignment_name, parameters)

def get_role_property(self, obj, property_name): # pylint: disable=no-self-use
Expand Down
8 changes: 8 additions & 0 deletions src/azure-cli/azure/cli/command_modules/role/_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,9 @@ def load_arguments(self, _):
"use the object id and not the app id.")
c.argument('ids', nargs='+', help='space-separated role assignment ids')
c.argument('include_classic_administrators', arg_type=get_three_state_flag(), help='list default role assignments for subscription classic administrators, aka co-admins')
c.argument('description', min_api='2020-04-01-preview', help='Description of role assignment.')
c.argument('condition', min_api='2020-04-01-preview', help='Condition under which the user can be granted permission.')
c.argument('condition_version', min_api='2020-04-01-preview', help='Version of the condition syntax. If --condition is specified without --condition-version, default to 2.0.')

@jiasli jiasli Sep 15, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

How about adding this default value in the help message ?

Added. I didn't use the built-in default mechanism because it is a conditional default - only when --condition is specified.


time_help = ('The {} of the query in the format of %Y-%m-%dT%H:%M:%SZ, e.g. 2000-12-31T12:59:59Z. Defaults to {}')
with self.argument_context('role assignment list-changelogs') as c:
Expand All @@ -198,6 +201,11 @@ def load_arguments(self, _):
c.argument('assignee_principal_type', min_api='2018-09-01-preview', arg_type=get_enum_type(PrincipalType),
help='use with --assignee-object-id to avoid errors caused by propagation latency in AAD Graph')

with self.argument_context('role assignment update') as c:
c.argument('role_assignment',
help='Description of an existing role assignment as JSON, or a path to a file containing a '
'JSON description.')

with self.argument_context('role assignment delete') as c:
c.argument('yes', options_list=['--yes', '-y'], action='store_true', help='Continue to delete all assignments under the subscription')

Expand Down
1 change: 1 addition & 0 deletions src/azure-cli/azure/cli/command_modules/role/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ def load_command_table(self, _):
g.custom_command('delete', 'delete_role_assignments', validator=process_assignment_namespace)
g.custom_command('list', 'list_role_assignments', validator=process_assignment_namespace, table_transformer=transform_assignment_list)
g.custom_command('create', 'create_role_assignment', validator=process_assignment_namespace)
g.custom_command('update', 'update_role_assignment', min_api='2020-04-01-preview')
g.custom_command('list-changelogs', 'list_role_assignment_change_logs')

with self.command_group('ad app', client_factory=get_graph_client_applications, resource_type=PROFILE_TYPE,
Expand Down
56 changes: 51 additions & 5 deletions src/azure-cli/azure/cli/command_modules/role/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,9 @@ def _create_update_role_definition(cmd, role_definition, for_update):
role_definition = shell_safe_json_parse(role_definition)

if not isinstance(role_definition, dict):
raise CLIError('Invalid role defintion. A valid dictionary JSON representation is expected.')
raise CLIError('Invalid role definition. A valid dictionary JSON representation is expected.')
# to workaround service defects, ensure property names are camel case
# e.g. AssignableScopes -> assignableScopes
names = [p for p in role_definition if p[:1].isupper()]
for n in names:
new_name = n[:1].lower() + n[1:]
Expand Down Expand Up @@ -129,25 +130,38 @@ def _search_role_definitions(cli_ctx, definitions_client, name, scopes, custom_r


def create_role_assignment(cmd, role, assignee=None, assignee_object_id=None, resource_group_name=None,
scope=None, assignee_principal_type=None):
scope=None, assignee_principal_type=None, description=None,
condition=None, condition_version=None):
"""Check parameters are provided correctly, then call _create_role_assignment."""
if bool(assignee) == bool(assignee_object_id):
raise CLIError('usage error: --assignee STRING | --assignee-object-id GUID')

if assignee_principal_type and not assignee_object_id:
raise CLIError('usage error: --assignee-object-id GUID [--assignee-principal-type]')

# If condition is set and condition-version is empty, condition-version defaults to "2.0".
if condition and not condition_version:
condition_version = "2.0"
Comment thread
arrownj marked this conversation as resolved.

# If condition-version is set, condition must be set as well.
if condition_version and not condition:
raise CLIError('usage error: When --condition-version is set, --condition must be set as well.')

try:
return _create_role_assignment(cmd.cli_ctx, role, assignee or assignee_object_id, resource_group_name, scope,
resolve_assignee=(not assignee_object_id),
assignee_principal_type=assignee_principal_type)
assignee_principal_type=assignee_principal_type, description=description,
condition=condition, condition_version=condition_version)
except Exception as ex: # pylint: disable=broad-except
if _error_caused_by_role_assignment_exists(ex): # for idempotent
return list_role_assignments(cmd, assignee, role, resource_group_name, scope)[0]
raise


def _create_role_assignment(cli_ctx, role, assignee, resource_group_name=None, scope=None,
resolve_assignee=True, assignee_principal_type=None):
resolve_assignee=True, assignee_principal_type=None, description=None,
condition=None, condition_version=None):
"""Prepare scope, role ID and resolve object ID from Graph API."""
factory = _auth_client_factory(cli_ctx, scope)
assignments_client = factory.role_assignments
definitions_client = factory.role_definitions
Expand All @@ -158,7 +172,8 @@ def _create_role_assignment(cli_ctx, role, assignee, resource_group_name=None, s
object_id = _resolve_object_id(cli_ctx, assignee) if resolve_assignee else assignee
worker = MultiAPIAdaptor(cli_ctx)
return worker.create_role_assignment(assignments_client, _gen_guid(), role_id, object_id, scope,
assignee_principal_type)
assignee_principal_type, description=description,
condition=condition, condition_version=condition_version)


def list_role_assignments(cmd, assignee=None, role=None, resource_group_name=None,
Expand Down Expand Up @@ -231,6 +246,37 @@ def list_role_assignments(cmd, assignee=None, role=None, resource_group_name=Non
return results


def update_role_assignment(cmd, role_assignment):
# Try role_assignment as a file.
if os.path.exists(role_assignment):
role_assignment = get_file_json(role_assignment)
else:
role_assignment = shell_safe_json_parse(role_assignment)

# Updating role assignment is only supported after 2020-04-01-preview, so we don't need to use MultiAPIAdaptor.
from azure.cli.core.profiles import get_sdk

RoleAssignment = get_sdk(cmd.cli_ctx, ResourceType.MGMT_AUTHORIZATION, 'RoleAssignment', mod='models',
operation_group='role_assignments')
assignment = RoleAssignment.from_dict(role_assignment)
scope = assignment.scope
name = assignment.name

auth_client = _auth_client_factory(cmd.cli_ctx, scope)
assignments_client = auth_client.role_assignments

# Get the existing assignment to do some checks.
original_assignment = assignments_client.get(scope, name)

# Forbid condition version downgrading.
# This should be implemented on the service-side in the future.
if (assignment.condition_version and original_assignment.condition_version and
original_assignment.condition_version.startswith('2.') and assignment.condition_version.startswith('1.')):
raise CLIError("Condition version cannot be downgraded to '1.X'.")

return assignments_client.create(scope, name, parameters=assignment)


def _get_assignment_events(cli_ctx, start_time=None, end_time=None):
from azure.mgmt.monitor import MonitorManagementClient
from azure.cli.core.commands.client_factory import get_mgmt_service_client
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,79 @@ def test_role_assignment_create_using_principal_type(self, resource_group):
finally:
self.cmd('ad user delete --upn-or-object-id {upn}')

@ResourceGroupPreparer(name_prefix='cli_role_assign')
@AllowLargeResponse()
def test_role_assignment_create_update(self, resource_group):
if self.run_under_service_principal():
return # this test delete users which are beyond a SP's capacity, so quit...

with mock.patch('azure.cli.command_modules.role.custom._gen_guid', side_effect=self.create_guid):
user = self.create_random_name('testuser', 15)
self.kwargs.update({
'upn': user + '@azuresdkteam.onmicrosoft.com',
'rg': resource_group,
'description': "Role assignment foo to check on bar",
'condition': "@Resource[Microsoft.Storage/storageAccounts/blobServices/containers:Name] stringEquals 'foo'",
'condition_version': "2.0"
})

result = self.cmd('ad user create --display-name tester123 --password Test123456789 --user-principal-name {upn}').get_output_in_json()
self.kwargs['object_id'] = result['objectId']
try:
# Test create role assignment with description, condition and condition_version
self.cmd('role assignment create --assignee-object-id {object_id} --assignee-principal-type User --role reader -g {rg} '
# Include double quotes to tell shlex to treat arguments as a whole
'--description "{description}" '
'--condition "{condition}" --condition-version {condition_version}',
checks=[
self.check("description", "{description}"),
self.check("condition", "{condition}"),
self.check("conditionVersion", "{condition_version}")
])
self.cmd('role assignment delete -g {rg}')

# Test create role assignment with description, condition. condition_version defaults to 2.0
self.cmd('role assignment create --assignee-object-id {object_id} --assignee-principal-type User --role reader -g {rg} '
'--description "{description}" '
'--condition "{condition}"',
checks=[
self.check("description", "{description}"),
self.check("condition", "{condition}"),
self.check("conditionVersion", "2.0")
])
self.cmd('role assignment delete -g {rg}')

# Test error is raised if condition-version is set but condition is not
with self.assertRaisesRegex(CLIError, "--condition must be set"):
self.cmd('role assignment create --assignee-object-id {object_id} --assignee-principal-type User --role reader -g {rg} '
'--condition-version {condition_version}')

# Update
output = self.cmd('role assignment create --assignee-object-id {object_id} --assignee-principal-type User --role reader -g {rg} '
'--description "{description}" '
'--condition "{condition}" --condition-version {condition_version}').get_output_in_json()

updated_description = "Some updated description."
output['description'] = updated_description
import shlex
from ..util import escape_apply_kwargs
# The json contains both single (in description) and double quotes, use shlex to quote and escape it,
# then escape it again before being passed to self.cmd
output_json = escape_apply_kwargs(shlex.quote(json.dumps(output)))

self.cmd("role assignment update --role-assignment {}".format(output_json),
checks=[self.check("description", updated_description)])

with self.assertRaisesRegex(CLIError, "cannot be downgraded"):
output['conditionVersion'] = '1.0'
output_json = escape_apply_kwargs(shlex.quote(json.dumps(output)))
self.cmd("role assignment update --role-assignment {}".format(output_json))

self.cmd('role assignment delete -g {rg}')

finally:
self.cmd('ad user delete --upn-or-object-id {upn}')

@ResourceGroupPreparer(name_prefix='cli_role_assign')
@AllowLargeResponse()
def test_role_assignment_handle_conflicted_assignments(self, resource_group):
Expand Down
5 changes: 5 additions & 0 deletions src/azure-cli/azure/cli/command_modules/role/tests/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,8 @@ def cmd_with_retry(self, *args, sleep_duration=ROLE_COMMAND_SLEEP_DURATION, max_
else:
raise
return result


def escape_apply_kwargs(val):
"""Replace {} as {{}} so that val is preserved after _apply_kwargs."""
return val.replace('{', "{{").replace('}', "}}")