{Role} az ad sp create-for-rbac: Stop setting identifierUris on a…

…pp (#18312)

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

@@ -52,8 +52,8 @@ def show_subscription(cmd, subscription=None, show_auth_for_sdk=None):
     profile = Profile(cli_ctx=cmd.cli_ctx)
 
     if show_auth_for_sdk:
-        from azure.cli.command_modules.role.custom import CREDENTIAL_WARNING_MESSAGE
-        logger.warning(CREDENTIAL_WARNING_MESSAGE)
+        from azure.cli.command_modules.role.custom import CREDENTIAL_WARNING
+        logger.warning(CREDENTIAL_WARNING)
         # sdk-auth file should be in json format all the time, hence the print
         print(json.dumps(profile.get_sp_auth_info(subscription), indent=2))
         return

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

@@ -399,7 +399,7 @@
     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.
+    short-summary: Display name of the service principal. If not present, default to azure-cli-%Y-%m-%d-%H-%M-%S where the suffix is the time of creation.
   - name: --cert
     short-summary: Certificate to use for credentials.
     long-summary: When used with `--keyvault,` indicates the name of the cert to use or create. Otherwise, supply a PEM or DER formatted public certificate string. Use `@{path}` to load from a file. Do not include private key info.

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

@@ -5,11 +5,11 @@
 
 import base64
 import datetime
+import itertools
 import json
-import re
 import os
+import re
 import uuid
-import itertools
 from dateutil.relativedelta import relativedelta
 import dateutil.parser
 
@@ -33,7 +33,7 @@
 from ._client_factory import _auth_client_factory, _graph_client_factory
 from ._multi_api_adaptor import MultiAPIAdaptor
 
-CREDENTIAL_WARNING_MESSAGE = (
+CREDENTIAL_WARNING = (
     "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")
 
@@ -42,6 +42,9 @@
     "If needed, use the --role argument to explicitly create a role assignment."
 )
 
+NAME_DEPRECATION_WARNING = \
+    "'name' property in the output is deprecated and will be removed in the future. Use 'appId' instead."
+
 logger = get_logger(__name__)
 
 # pylint: disable=too-many-lines
@@ -1400,48 +1403,30 @@ def create_service_principal_for_rbac(
     role_client = _auth_client_factory(cmd.cli_ctx).role_assignments
     scopes = scopes or ['/subscriptions/' + role_client.config.subscription_id]
     years = years or 1
-    sp_oid = None
     _RETRY_TIMES = 36
-    app_display_name, existing_sps = None, None
-    if name:
-        if '://' not in name:
-            prefix = "http://"
-            app_display_name = name
-            # replace space, /, \ with - to make it a valid URI
-            name = name.replace(' ', '-').replace('/', '-').replace('\\', '-')
-            logger.warning('Changing "%s" to a valid URI of "%s%s", which is the required format'
-                           ' used for service principal names', name, prefix, name)
-            name = prefix + name  # normalize be a valid graph service principal name
-        else:
-            app_display_name = name.split('://', 1)[-1]
+    existing_sps = None
 
-    if name:
-        query_exp = 'servicePrincipalNames/any(x:x eq \'{}\')'.format(name)
+    if not name:
+        # No name is provided, create a new one
+        app_display_name = 'azure-cli-' + datetime.datetime.utcnow().strftime('%Y-%m-%d-%H-%M-%S')
+    else:
+        app_display_name = name
+        # patch existing app with the same displayName to make the command idempotent
+        query_exp = "displayName eq '{}'".format(name)
         existing_sps = list(graph_client.service_principals.list(filter=query_exp))
-        if existing_sps:
-            app_display_name = existing_sps[0].display_name
 
     app_start_date = datetime.datetime.now(TZ_UTC)
     app_end_date = app_start_date + relativedelta(years=years or 1)
 
-    app_display_name = app_display_name or ('azure-cli-' +
-                                            app_start_date.strftime('%Y-%m-%d-%H-%M-%S'))
-    if name is None:
-        name = 'http://' + app_display_name  # just a valid uri, no need to exist
-
     password, public_cert_string, cert_file, cert_start_date, cert_end_date = \
         _process_service_principal_creds(cmd.cli_ctx, years, app_start_date, app_end_date, cert, create_cert,
                                          None, keyvault)
 
     app_start_date, app_end_date, cert_start_date, cert_end_date = \
         _validate_app_dates(app_start_date, app_end_date, cert_start_date, cert_end_date)
 
-    # replace space, /, \ with - to make it a valid URI
-    homepage = 'https://' + app_display_name.replace(' ', '-').replace('/', '-').replace('\\', '-')
     aad_application = create_application(cmd,
                                          display_name=app_display_name,
-                                         homepage=homepage,
-                                         identifier_uris=[name],
                                          available_to_other_tenants=False,
                                          password=password,
                                          key_value=public_cert_string,
@@ -1469,9 +1454,9 @@ def create_service_principal_for_rbac(
                     time.sleep(5)
                 else:
                     logger.warning(
-                        "Creating service principal failed for appid '%s'. Trace followed:\n%s",
-                        name, ex.response.headers if hasattr(ex,
-                                                             'response') else ex)  # pylint: disable=no-member
+                        "Creating service principal failed for '%s'. Trace followed:\n%s",
+                        app_id, ex.response.headers
+                        if hasattr(ex, 'response') else ex)  # pylint: disable=no-member
                     raise
     sp_oid = aad_sp.object_id
 
@@ -1504,7 +1489,8 @@ def create_service_principal_for_rbac(
                                        ex.response.headers)  # pylint: disable=no-member
                     raise
 
-    logger.warning(CREDENTIAL_WARNING_MESSAGE)
+    logger.warning(CREDENTIAL_WARNING)
+    logger.warning(NAME_DEPRECATION_WARNING)
 
     if show_auth_for_sdk:
         from azure.cli.core._profile import Profile
@@ -1518,7 +1504,7 @@ def create_service_principal_for_rbac(
     result = {
         'appId': app_id,
         'password': password,
-        'name': name,
+        'name': app_id,
         'displayName': app_display_name,
         'tenant': graph_client.config.tenant_id
     }
@@ -1770,7 +1756,7 @@ def reset_service_principal_credential(cmd, name, password=None, create_cert=Fal
     if cert_file:
         result['fileWithCertAndPrivateKey'] = cert_file
 
-    logger.warning(CREDENTIAL_WARNING_MESSAGE)
+    logger.warning(CREDENTIAL_WARNING)
     return result