[AD] az ad app/sp update: Add --parameters to replace generic update arguments

src/azure-cli/azure/cli/command_modules/role/_help.py

@@ -261,65 +261,66 @@
 type: command
 short-summary: Update an application.
 examples:
-  - name: update a native application with delegated permission of "access the AAD directory as the signed-in user"
-    text: |
-        az ad app update --id e042ec79-34cd-498f-9d9f-123456781234 --required-resource-accesses @manifest.json
-        ("manifest.json" contains the following content)
-        [{
-            "resourceAppId": "00000002-0000-0000-c000-000000000000",
-            "resourceAccess": [
-                {
-                    "id": "a42657d6-7f20-40e3-b6f0-cee03008a62a",
-                    "type": "Scope"
-                }
-           ]
-        }]
-  - name: declare an application role
-    text: |
-        az ad app update --id e042ec79-34cd-498f-9d9f-123456781234 --app-roles @manifest.json
-        ("manifest.json" contains the following content)
-        [{
-            "allowedMemberTypes": [
-              "User"
-            ],
-            "description": "Approvers can mark documents as approved",
-            "displayName": "Approver",
-            "isEnabled": "true",
-            "value": "approver"
-        }]
-  - name: update optional claims
-    text: |
-        az ad app update --id e042ec79-34cd-498f-9d9f-123456781234 --optional-claims @manifest.json
-        ("manifest.json" contains the following content)
-        {
-            "idToken": [
-                {
-                    "name": "auth_time",
-                    "essential": false
-                }
-            ],
-            "accessToken": [
-                {
-                    "name": "ipaddr",
-                    "essential": false
-                }
-            ],
-            "saml2Token": [
-                {
-                    "name": "upn",
-                    "essential": false
-                },
-                {
-                    "name": "extension_ab603c56068041afb2f6832e2a17e237_skypeId",
-                    "source": "user",
-                    "essential": false
-                }
-            ]
-        }
-  - name: update an application's group membership claims to "All"
-    text: >
-        az ad app update --id e042ec79-34cd-498f-9d9f-123456781234 --set groupMembershipClaims=All
-
+- name: Update a native application with delegated permission of "access the AAD directory as the signed-in user"
+  text: |
+      az ad app update --id e042ec79-34cd-498f-9d9f-123456781234 --required-resource-accesses @manifest.json
+      ("manifest.json" contains the following content)
+      [{
+          "resourceAppId": "00000002-0000-0000-c000-000000000000",
+          "resourceAccess": [
+              {
+                  "id": "a42657d6-7f20-40e3-b6f0-cee03008a62a",
+                  "type": "Scope"
+              }
+         ]
+      }]
+- name: Declare an application role
+  text: |
+      az ad app update --id e042ec79-34cd-498f-9d9f-123456781234 --app-roles @manifest.json
+      ("manifest.json" contains the following content)
+      [{
+          "allowedMemberTypes": [
+            "User"
+          ],
+          "description": "Approvers can mark documents as approved",
+          "displayName": "Approver",
+          "isEnabled": "true",
+          "value": "approver"
+      }]
+- name: Update optional claims
+  text: |
+      az ad app update --id e042ec79-34cd-498f-9d9f-123456781234 --optional-claims @manifest.json
+      ("manifest.json" contains the following content)
+      {
+          "idToken": [
+              {
+                  "name": "auth_time",
+                  "essential": false
+              }
+          ],
+          "accessToken": [
+              {
+                  "name": "ipaddr",
+                  "essential": false
+              }
+          ],
+          "saml2Token": [
+              {
+                  "name": "upn",
+                  "essential": false
+              },
+              {
+                  "name": "extension_ab603c56068041afb2f6832e2a17e237_skypeId",
+                  "source": "user",
+                  "essential": false
+              }
+          ]
+      }
+- name: Update an application's groupMembershipClaims property to "All"
+  text: az ad app update --id e042ec79-34cd-498f-9d9f-123456781234 --set groupMembershipClaims=All
+- name: Update an application's groupMembershipClaims property to "All" with JSON (Bash)
+  text: |
+    az ad app update --id e042ec79-34cd-498f-9d9f-123456781234 --parameters '{"groupMembershipClaims": "All"}'
 """
 
 helps['ad app federated-credential'] = """
@@ -627,9 +628,11 @@
 type: command
 short-summary: Update a service principal
 examples:
-  - name: update a service principal (autogenerated)
-    text: az ad sp update --id 00000000-0000-0000-0000-000000000000 --set groupMembershipClaims=All
-    crafted: true
+- name: Update a service principal's appRoleAssignmentRequired property to true
+  text: az ad sp update --id 00000000-0000-0000-0000-000000000000 --set appRoleAssignmentRequired=true
+- name: Update a service principal's appRoleAssignmentRequired property to true with JSON (Bash)
+  text: >
+    az ad sp update --id 00000000-0000-0000-0000-000000000000 --parameters '{"appRoleAssignmentRequired": true}'
 """
 
 helps['ad user'] = """

src/azure-cli/azure/cli/command_modules/role/_params.py

@@ -219,6 +219,14 @@ def load_arguments(self, _):
             c.argument('key_id', help='credential key id')
             c.argument('cert', action='store_true', help='a certificate based credential')
 
+    # Generic Update
+    for item in ['ad app update', 'ad sp update']:
+        with self.argument_context(item) as c:
+            c.argument('properties_to_set', arg_group='Generic Update', options_list=['--set'], nargs='+',
+                       help='Set properties. Space-separated list of {name}={string or JSON}.')
+            c.argument('parameters', arg_group='Generic Update', type=validate_file_or_dict, is_experimental=True,
+                       help='Parameters for update operation. ' + JSON_PROPERTY_HELP)
+
     with self.argument_context('ad') as c:
         c.argument('display_name', help='object\'s display name or its prefix')
         c.argument('identifier_uri', help='graph application identifier, must be in uri format')

src/azure-cli/azure/cli/command_modules/role/commands.py

@@ -81,16 +81,14 @@ def load_command_table(self, _):
         g.custom_command('create', 'create_application')
         g.custom_command('delete', 'delete_application')
         g.custom_command('list', 'list_applications', table_transformer=get_graph_object_transformer('app'))
+        g.custom_command('update', 'update_application')
         g.custom_show_command('show', 'show_application')
         g.custom_command('permission grant', 'grant_application')
         g.custom_command('permission list', 'list_permissions')
         g.custom_command('permission add', 'add_permission')
         g.custom_command('permission delete', 'delete_permission')
         g.custom_command('permission list-grants', 'list_permission_grants')
         g.custom_command('permission admin-consent', 'admin_consent')
-        g.generic_update_command('update', setter_name='patch_application', setter_type=role_custom,
-                                 getter_name='show_application', getter_type=role_custom,
-                                 custom_func_name='update_application', custom_func_type=role_custom)
         g.custom_command('credential reset', 'reset_application_credential')
         g.custom_command('credential list', 'list_application_credentials')
         g.custom_command('credential delete', 'delete_application_credential')
@@ -112,9 +110,7 @@ def load_command_table(self, _):
         g.custom_command('delete', 'delete_service_principal')
         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,
-                                 custom_func_name='update_service_principal', custom_func_type=role_custom)
+        g.custom_command('update', 'update_service_principal')
 
     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')

src/azure-cli/azure/cli/command_modules/role/custom.py

@@ -625,8 +625,9 @@ def create_application(cmd, client, display_name, identifier_uris=None,
         if len(existing_apps) == 1:
             logger.warning("Found an existing application instance: (id) %s. We will patch it.",
                            existing_apps[0][ID])
-            body = update_application(
-                existing_apps[0], display_name=display_name, identifier_uris=identifier_uris,
+            update_application(
+                client, existing_apps[0][ID],
+                display_name=display_name, identifier_uris=identifier_uris,
                 is_fallback_public_client=is_fallback_public_client, sign_in_audience=sign_in_audience,
                 # keyCredentials
                 key_value=key_value, key_type=key_type, key_usage=key_usage,
@@ -642,7 +643,6 @@ def create_application(cmd, client, display_name, identifier_uris=None,
                 app_roles=app_roles,
                 optional_claims=optional_claims,
                 required_resource_accesses=required_resource_accesses)
-            patch_application(cmd, existing_apps[0][ID], body)
 
             # no need to resolve identifierUris or appId. Just use id.
             return client.application_get(existing_apps[0][ID])
@@ -680,7 +680,7 @@ def create_application(cmd, client, display_name, identifier_uris=None,
     return result
 
 
-def update_application(instance, display_name=None, identifier_uris=None,  # pylint: disable=unused-argument
+def update_application(client, identifier, display_name=None, identifier_uris=None,
                        is_fallback_public_client=None, sign_in_audience=None,
                        # keyCredentials
                        key_value=None, key_type=None, key_usage=None, start_date=None, end_date=None,
@@ -691,8 +691,11 @@ def update_application(instance, display_name=None, identifier_uris=None,  # pyl
                        # publicClient
                        public_client_redirect_uris=None,
                        # JSON properties
-                       app_roles=None, optional_claims=None, required_resource_accesses=None):
+                       app_roles=None, optional_claims=None, required_resource_accesses=None,
+                       # Generic update
+                       parameters=None, properties_to_set=None):
     body = {}
+    _update_patch_body(body, parameters, properties_to_set)
 
     key_credentials = None
     if key_value:
@@ -714,14 +717,8 @@ def update_application(instance, display_name=None, identifier_uris=None,  # pyl
         # JSON properties
         app_roles=app_roles, optional_claims=optional_claims, required_resource_accesses=required_resource_accesses
     )
-
-    return body
-
-
-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_update(object_id, parameters)
+    object_id = _resolve_application(client, identifier)
+    return client.application_update(object_id, body)
 
 
 def show_application(client, identifier):
@@ -1013,15 +1010,14 @@ def create_service_principal(cmd, identifier):
     return _create_service_principal(cmd.cli_ctx, identifier)
 
 
-def update_service_principal(instance):  # pylint: disable=unused-argument
+def update_service_principal(client, identifier,
+                             # Generic update
+                             parameters=None, properties_to_set=None):
     # 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, identifier)
-    return graph_client.service_principal_update(object_id, parameters)
+    body = {}
+    _update_patch_body(body, parameters, properties_to_set)
+    object_id = _resolve_service_principal(client, identifier)
+    return client.service_principal_update(object_id, body)
 
 
 def _create_service_principal(cli_ctx, identifier, resolve_app=True):
@@ -1989,3 +1985,26 @@ def _get_member_groups(get_member_group_func, identifier, security_enabled_only)
         "securityEnabledOnly": security_enabled_only
     }
     return get_member_group_func(identifier, body)
+
+
+def _update_patch_body(body, parameters, properties_to_set):
+    # --parameters
+    if parameters:
+        body.update(parameters)
+
+    # --set
+    if properties_to_set:
+        for exp in properties_to_set:
+            try:
+                key, value = exp.split('=', 1)
+            except ValueError as ex:
+                from azure.cli.core.azclierror import ArgumentUsageError
+                raise ArgumentUsageError('Usage error: Please set properties with space-separated list of '
+                                         '{name}={string or JSON}, such as `--set displayName=myapp`') from ex
+            # Try parsing value as JSON
+            try:
+                value = shell_safe_json_parse(value)
+            except:  # pylint:disable=bare-except
+                pass
+            body[key] = value
+    return body