Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AD] az ad app/sp update: Support generic update --set on root level #22798

Merged
merged 1 commit into from
Jun 13, 2022
Merged

[AD] az ad app/sp update: Support generic update --set on root level #22798

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions src/azure-cli/azure/cli/command_modules/role/_msgrpah/_graph_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ def application_delete(self, id):
result = self._send("DELETE", "/applications/{id}".format(id=id))
return result

def application_patch(self, id, body):
def application_update(self, id, body):
Copy link
Member Author

@jiasli jiasli Jun 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

AD Graph SDK uses verb "patch", instead of "update": azure.graphrbac.operations.applications_operations.ApplicationsOperations.patch. I guess it is used to differentiate with update_application of generic update.

We switch to use "update" to align with other update operations (such as azure.graphrbac.operations.users_operations.UsersOperations.update) and the API document https://docs.microsoft.com/en-us/graph/api/application-update.

# https://docs.microsoft.com/en-us/graph/api/application-update
result = self._send("PATCH", "/applications/{id}".format(id=id), body=body)
return result
Expand Down Expand Up @@ -146,7 +146,7 @@ def service_principal_delete(self, id):
result = self._send("DELETE", "/servicePrincipals/{id}".format(id=id))
return result

def service_principal_patch(self, id, body):
def service_principal_update(self, id, body):
# https://docs.microsoft.com/en-us/graph/api/serviceprincipal-update
result = self._send("PATCH", "/servicePrincipals/{id}".format(id=id), body=body)
return result
Expand Down Expand Up @@ -268,7 +268,7 @@ def user_delete(self, id_or_upn):
result = self._send("DELETE", "/users/{}".format(id_or_upn))
return result

def user_patch(self, id_or_upn, body):
def user_update(self, id_or_upn, body):
# https://docs.microsoft.com/graph/api/user-update
result = self._send("PATCH", "/users/{}".format(id_or_upn), body=body)
return result
Expand Down
3 changes: 2 additions & 1 deletion src/azure-cli/azure/cli/command_modules/role/commands.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ def load_command_table(self, _):
g.custom_command('list', 'list_service_principals', table_transformer=get_graph_object_transformer('sp'))
g.custom_show_command('show', 'show_service_principal')
g.generic_update_command('update', getter_name='show_service_principal', getter_type=role_custom,
setter_name='patch_service_principal', setter_type=role_custom)
setter_name='patch_service_principal', setter_type=role_custom,
custom_func_name='update_service_principal', custom_func_type=role_custom)

with self.command_group('ad sp owner', client_factory=get_graph_client, exception_handler=graph_err_handler) as g:
g.custom_command('list', 'list_service_principal_owners')
Expand Down
25 changes: 15 additions & 10 deletions src/azure-cli/azure/cli/command_modules/role/custom.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -721,7 +721,7 @@ def update_application(instance, display_name=None, identifier_uris=None, # pyl
def patch_application(cmd, identifier, parameters):
graph_client = _graph_client_factory(cmd.cli_ctx)
object_id = _resolve_application(graph_client, identifier)
return graph_client.application_patch(object_id, parameters)
return graph_client.application_update(object_id, parameters)


def show_application(client, identifier):
Expand Down Expand Up @@ -788,15 +788,15 @@ def reset_application_credential(cmd, client, identifier, create_cert=False, cer
raise CLIError("can't find an application matching '{}'".format(identifier))
result = _reset_credential(
cmd, app, client.application_add_password, client.application_remove_password,
client.application_patch, create_cert=create_cert, cert=cert, years=years,
client.application_update, create_cert=create_cert, cert=cert, years=years,
end_date=end_date, keyvault=keyvault, append=append, display_name=display_name)
result['tenant'] = client.tenant
return result


def delete_application_credential(cmd, client, identifier, key_id, cert=False): # pylint: disable=unused-argument
sp = show_application(client, identifier)
_delete_credential(sp, client.application_remove_password, client.application_patch,
_delete_credential(sp, client.application_remove_password, client.application_update,
key_id=key_id, cert=cert)


Expand Down Expand Up @@ -875,7 +875,7 @@ def add_permission(client, identifier, api, api_permissions):
}
required_resource_access_list.append(required_resource_access)
body = {'requiredResourceAccess': required_resource_access_list}
client.application_patch(application[ID], body)
client.application_update(application[ID], body)
logger.warning('Invoking `az ad app permission grant --id %s --api %s` is needed to make the '
'change effective', identifier, api)

Expand Down Expand Up @@ -916,7 +916,7 @@ def delete_permission(client, identifier, api, api_permissions=None):
required_resource_access_list.remove(existing_required_resource_access)

body = {'requiredResourceAccess': required_resource_access_list}
return client.application_patch(application[ID], body)
return client.application_update(application[ID], body)


def list_permissions(cmd, identifier):
Expand Down Expand Up @@ -983,10 +983,15 @@ def create_service_principal(cmd, identifier):
return _create_service_principal(cmd.cli_ctx, identifier)


def update_service_principal(instance): # pylint: disable=unused-argument
# Do not PATCH back properties retrieved with GET and leave everything else to generic update.
return {}


def patch_service_principal(cmd, identifier, parameters):
graph_client = _graph_client_factory(cmd.cli_ctx)
object_id = _resolve_service_principal(graph_client.service_principals, identifier)
return graph_client.service_principals.update(object_id, parameters)
object_id = _resolve_service_principal(graph_client, identifier)
return graph_client.service_principal_update(object_id, parameters)


def _create_service_principal(cli_ctx, identifier, resolve_app=True):
Expand Down Expand Up @@ -1058,7 +1063,7 @@ def reset_service_principal_credential(cmd, client, identifier, create_cert=Fals
result = _reset_credential(
cmd, sp,
client.service_principal_add_password, client.service_principal_remove_password,
client.service_principal_patch,
client.service_principal_update,
create_cert=create_cert, cert=cert, years=years,
end_date=end_date, keyvault=keyvault, append=append, display_name=display_name)
result['tenant'] = client.tenant
Expand All @@ -1068,7 +1073,7 @@ def reset_service_principal_credential(cmd, client, identifier, create_cert=Fals
def delete_service_principal_credential(cmd, client, # pylint: disable=unused-argument
identifier, key_id, cert=False):
sp = show_service_principal(client, identifier)
_delete_credential(sp, client.service_principal_remove_password, client.service_principal_patch,
_delete_credential(sp, client.service_principal_remove_password, client.service_principal_update,
key_id=key_id, cert=cert)


Expand Down Expand Up @@ -1801,7 +1806,7 @@ def update_user(client, upn_or_object_id, display_name=None, force_change_passwo
_set_user_properties(body, display_name=display_name, password=password, mail_nickname=mail_nickname,
force_change_password_next_sign_in=force_change_password_next_sign_in,
account_enabled=account_enabled)
return client.user_patch(upn_or_object_id, body)
return client.user_update(upn_or_object_id, body)


def _set_user_properties(body, **kwargs):
Expand Down
Loading