{Graph} Refactor role module for Microsoft Graph migration

src/azure-cli-testsdk/azure/cli/testsdk/__init__.py

@@ -12,14 +12,14 @@
 from .checkers import (JMESPathCheck, JMESPathCheckExists, JMESPathCheckGreaterThan, NoneCheck, StringCheck,
                        StringContainCheck)
 from .decorators import api_version_constraint
-from .utilities import create_random_name, AADGraphUserReplacer
+from .utilities import create_random_name, MSGraphUserReplacer
 from .patches import MOCKED_USER_NAME
 
 __all__ = ['ScenarioTest', 'LiveScenarioTest', 'ResourceGroupPreparer', 'StorageAccountPreparer',
            'RoleBasedServicePrincipalPreparer', 'ManagedApplicationPreparer', 'CliTestError', 'JMESPathCheck',
            'JMESPathCheckExists', 'NoneCheck', 'live_only', 'record_only', 'StringCheck', 'StringContainCheck',
            'get_sha1_hash', 'KeyVaultPreparer', 'JMESPathCheckGreaterThan', 'api_version_constraint',
-           'create_random_name', 'MOCKED_USER_NAME', 'AADGraphUserReplacer', 'LocalContextScenarioTest',
+           'create_random_name', 'MOCKED_USER_NAME', 'MSGraphUserReplacer', 'LocalContextScenarioTest',
            'VirtualNetworkPreparer', 'VnetNicPreparer']
 
 

src/azure-cli-testsdk/azure/cli/testsdk/base.py

@@ -24,7 +24,7 @@
                       patch_progress_controller, patch_get_current_system_username)
 from .exceptions import CliExecutionError
 from .utilities import (find_recording_dir, StorageAccountKeyReplacer, GraphClientPasswordReplacer,
-                        MicrosoftGraphClientPasswordReplacer, AADAuthRequestFilter)
+                        MSGraphClientPasswordReplacer, AADAuthRequestFilter)
 from .reverse_dependency import get_dummy_cli
 
 logger = logging.getLogger('azure.cli.testsdk')
@@ -87,7 +87,7 @@ def __init__(self, method_name, config_file=None, recording_name=None,
         self.kwargs = {}
         self.test_guid_count = 0
         self._processors_to_reset = [StorageAccountKeyReplacer(), GraphClientPasswordReplacer(),
-                                     MicrosoftGraphClientPasswordReplacer()]
+                                     MSGraphClientPasswordReplacer()]
         default_recording_processors = [
             SubscriptionRecordingProcessor(MOCKED_SUBSCRIPTION_ID),
             AADAuthRequestFilter(),

src/azure-cli-testsdk/azure/cli/testsdk/utilities.py

@@ -48,14 +48,8 @@ def force_progress_logging():
     az_logger.handlers[0].level = old_az_level
 
 
-def _py3_byte_to_str(byte_or_str):
-    import logging
-    logger = logging.getLogger()
-    logger.warning(type(byte_or_str))
-    try:
-        return str(byte_or_str, 'utf-8') if isinstance(byte_or_str, bytes) else byte_or_str
-    except TypeError:  # python 2 doesn't allow decoding through str
-        return str(byte_or_str)
+def _byte_to_str(byte_or_str):
+    return str(byte_or_str, 'utf-8') if isinstance(byte_or_str, bytes) else byte_or_str
 
 
 class StorageAccountKeyReplacer(RecordingProcessor):
@@ -81,7 +75,7 @@ def process_request(self, request):  # pylint: disable=no-self-use
             pass
         for candidate in self._candidates:
             if request.body:
-                body_string = _py3_byte_to_str(request.body)
+                body_string = _byte_to_str(request.body)
                 request.body = body_string.replace(candidate, self.KEY_REPLACEMENT)
         return request
 
@@ -99,7 +93,7 @@ def process_response(self, response):
         for candidate in self._candidates:
             if response['body']['string']:
                 body = response['body']['string']
-                response['body']['string'] = _py3_byte_to_str(body)
+                response['body']['string'] = _byte_to_str(body)
                 response['body']['string'] = response['body']['string'].replace(candidate, self.KEY_REPLACEMENT)
         return response
 
@@ -127,11 +121,11 @@ def process_request(self, request):  # pylint: disable=no-self-use
             pattern = r"[^/]+/applications$"
             if re.search(pattern, request.path, re.I) and request.method.lower() == 'post':
                 self._activated = True
-                body = _py3_byte_to_str(request.body)
+                body = _byte_to_str(request.body)
                 body = json.loads(body)
                 for password_cred in body['passwordCredentials']:
                     if password_cred['value']:
-                        body_string = _py3_byte_to_str(request.body)
+                        body_string = _byte_to_str(request.body)
                         request.body = body_string.replace(password_cred['value'], self.PWD_REPLACEMENT)
 
         except (AttributeError, KeyError):
@@ -157,7 +151,7 @@ def process_response(self, response):
         return response
 
 
-class MicrosoftGraphClientPasswordReplacer(RecordingProcessor):
+class MSGraphClientPasswordReplacer(RecordingProcessor):
     """Replace 'secretText' property in 'addPassword' API's response."""
 
     PWD_REPLACEMENT = 'replaced-microsoft-graph-password'
@@ -169,7 +163,7 @@ def reset(self):
         self._activated = False
 
     def process_request(self, request):
-        if request.body and self.PWD_REPLACEMENT in _py3_byte_to_str(request.body):
+        if request.body and self.PWD_REPLACEMENT in _byte_to_str(request.body):
             return request
         if request.path.endswith('/addPassword') and request.method.lower() == 'post':
             self._activated = True
@@ -188,18 +182,17 @@ def process_response(self, response):
         return response
 
 
-class AADGraphUserReplacer:
+class MSGraphUserReplacer:
     def __init__(self, test_user, mock_user):
         self.test_user = test_user
         self.mock_user = mock_user
 
     def process_request(self, request):
-        test_user_encoded = self.test_user.replace('@', '%40')
-        if test_user_encoded in request.uri:
-            request.uri = request.uri.replace(test_user_encoded, self.mock_user.replace('@', '%40'))
+        if self.test_user in request.uri:
+            request.uri = request.uri.replace(self.test_user, self.mock_user)
 
         if request.body:
-            body = _py3_byte_to_str(request.body)
+            body = _byte_to_str(request.body)
             if self.test_user in body:
                 request.body = body.replace(self.test_user, self.mock_user)
 

src/azure-cli/azure/cli/command_modules/batch/tests/latest/recording_processors.py

@@ -19,12 +19,12 @@ class StorageSASReplacer(RecordingProcessor):
     def process_request(self, request):
         request.uri = self._replace_sas(request.uri)
         if request.body:
-            request.body = self._replace_sas(_py3_byte_to_str(request.body))
+            request.body = self._replace_sas(_byte_to_str(request.body))
         return request
 
     def process_response(self, response):
         if is_text_payload(response) and response["body"]["string"]:
-            body_string = _py3_byte_to_str(response["body"]["string"])
+            body_string = _byte_to_str(response["body"]["string"])
             response["body"]["string"] = self._replace_sas(body_string)
         return response
 
@@ -64,7 +64,7 @@ def process_request(self, request):
         body = None
 
         if is_text_payload(request) and request.body:
-            body_string = _py3_byte_to_str(request.body)
+            body_string = _byte_to_str(request.body)
             body = json.loads(body_string)
 
         pattern = r"/providers/Microsoft\.Batch/batchAccounts/[^/]+/(list|regenerate)Keys$"
@@ -85,7 +85,7 @@ def process_request(self, request):
 
     def process_response(self, response):
         if is_text_payload(response) and response['body']['string']:
-            body_string = _py3_byte_to_str(response['body']['string'])
+            body_string = _byte_to_str(response['body']['string'])
 
             if self._activated:
                 body = json.loads(body_string)
@@ -137,7 +137,7 @@ def get_key(self):
         return "".join([self.KEY_PREFIX, str(self._key_index), self.KEY_SUFFIX])
 
 
-def _py3_byte_to_str(byte_or_str):
+def _byte_to_str(byte_or_str):
     try:
         return str(byte_or_str, 'utf-8') if isinstance(byte_or_str, bytes) else byte_or_str
     except TypeError:  # python 2 doesn't allow decoding through str

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

@@ -17,17 +17,6 @@ def _auth_client_factory(cli_ctx, scope=None):
 
 
 def _graph_client_factory(cli_ctx, **_):
-    from ._graph_client import GraphClient
+    from .msgrpah import GraphClient
     client = GraphClient(cli_ctx)
     return client
-
-    from azure.cli.core._profile import Profile
-    from azure.cli.core.commands.client_factory import configure_common_settings
-    from azure.graphrbac import GraphRbacManagementClient
-    profile = Profile(cli_ctx=cli_ctx)
-    cred, _, tenant_id = profile.get_login_credentials(
-        resource=cli_ctx.cloud.endpoints.active_directory_graph_resource_id)
-    client = GraphRbacManagementClient(cred, tenant_id,
-                                       base_url=cli_ctx.cloud.endpoints.active_directory_graph_resource_id)
-    configure_common_settings(cli_ctx, client)
-    return client

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

@@ -6,7 +6,6 @@
 # pylint: disable=line-too-long
 
 from knack.arguments import CLIArgumentType
-from azure.graphrbac.models import ConsentType
 
 from azure.cli.core.commands.parameters import get_enum_type, get_three_state_flag
 from azure.cli.core.commands.validators import validate_file_or_dict
@@ -133,9 +132,9 @@ def load_arguments(self, _):
         c.argument('scope', nargs='*',
                    help='A space-separated list of the claim values for delegated permissions which should be included '
                         'in access tokens for the resource application (the API). '
-                        "For example, openid User.Read GroupMember.Read.All. "
+                        'For example, openid User.Read GroupMember.Read.All. '
                         'Each claim value should match the value field of one of the delegated permissions defined by '
-                        'the API, listed in the publishedPermissionScopes property of the resource service principal.')
+                        'the API, listed in the oauth2PermissionScopes property of the resource service principal.')
         c.argument('consent_type', arg_type=get_enum_type(["AllPrincipals", "Principal"]), default="AllPrincipals",
                    help="Indicates whether authorization is granted for the client application to impersonate all "
                         "users or only a specific user. 'AllPrincipals' indicates authorization to impersonate all "

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

@@ -45,46 +45,12 @@ def _transform_graph_object(result):
     return _transform_graph_object
 
 
-def transform_graph_objects_with_cred(result):
-    # here we will convert utf16 encoded custom key id back to the plain text
-    # we will handle single object from "show" cmd, object list from "list" cmd, and cred object itself
-    if not result:
-        return result
-    from msrest.paging import Paged
-    from azure.graphrbac.models import PasswordCredential
-
-    def _patch_creds(creds):
-        for c in creds:
-            custom_key_id = getattr(c, 'custom_key_identifier', None)
-            if custom_key_id:
-                try:
-                    c.custom_key_identifier = custom_key_id.decode('utf-16')
-                except Exception:  # pylint: disable=broad-except
-                    c.custom_key_identifier = None
-        return creds
-
-    singular = False
-    if isinstance(result, Paged):
-        result = list(result)
-
-    if not isinstance(result, list):
-        singular = True
-        result = [result]
-
-    for r in result:
-        if getattr(r, 'password_credentials', None):
-            _patch_creds(r.password_credentials)
-
-        if isinstance(r, PasswordCredential):
-            _patch_creds([r])
-    return result[0] if singular else result
-
-
 def graph_err_handler(ex):
-    from azure.graphrbac.models import GraphErrorException
-    if isinstance(ex, GraphErrorException):
+    # Convert GraphError to CLIError that can be printed
+    from .msgrpah import GraphError
+    if isinstance(ex, GraphError):
         from knack.util import CLIError
-        raise CLIError(ex.message)
+        raise CLIError(ex)
     raise ex
 
 
@@ -115,10 +81,10 @@ def load_command_table(self, _):
         g.custom_command('list-changelogs', 'list_role_assignment_change_logs')
 
     with self.command_group('ad app', client_factory=get_graph_client, resource_type=PROFILE_TYPE,
-                            exception_handler=graph_err_handler, transform=transform_graph_objects_with_cred) as g:
+                            exception_handler=graph_err_handler) as g:
         g.custom_command('create', 'create_application')
         g.custom_command('delete', 'delete_application')
-        g.custom_command('list', 'list_apps', table_transformer=get_graph_object_transformer('app'))
+        g.custom_command('list', 'list_applications', table_transformer=get_graph_object_transformer('app'))
         g.custom_show_command('show', 'show_application')
         g.custom_command('permission grant', 'grant_application')
         g.custom_command('permission list', 'list_permissions')
@@ -138,11 +104,11 @@ def load_command_table(self, _):
         g.custom_command('add', 'add_application_owner')
         g.custom_command('remove', 'remove_application_owner')
 
-    with self.command_group('ad sp', client_factory=get_graph_client, resource_type=PROFILE_TYPE, exception_handler=graph_err_handler,
-                            transform=transform_graph_objects_with_cred) as g:
+    with self.command_group('ad sp', client_factory=get_graph_client, resource_type=PROFILE_TYPE,
+                            exception_handler=graph_err_handler) as g:
         g.custom_command('create', 'create_service_principal')
         g.custom_command('delete', 'delete_service_principal')
-        g.custom_command('list', 'list_sps', table_transformer=get_graph_object_transformer('sp'))
+        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)
@@ -151,7 +117,7 @@ def load_command_table(self, _):
         g.custom_command('list', 'list_service_principal_owners')
 
     # RBAC related
-    with self.command_group('ad sp', client_factory=get_graph_client, exception_handler=graph_err_handler, transform=transform_graph_objects_with_cred) as g:
+    with self.command_group('ad sp', client_factory=get_graph_client, exception_handler=graph_err_handler) as g:
         g.custom_command('create-for-rbac', 'create_service_principal_for_rbac')
         g.custom_command('credential reset', 'reset_service_principal_credential')
         g.custom_command('credential list', 'list_service_principal_credentials')
@@ -165,8 +131,8 @@ def load_command_table(self, _):
         g.custom_command('create', 'create_user')
         g.custom_command('update', 'update_user')
 
-    with self.command_group('ad signed-in-user', client_factory=get_graph_client, exception_handler=graph_err_handler,
-                            transform=transform_graph_objects_with_cred) as g:
+    with self.command_group('ad signed-in-user', client_factory=get_graph_client,
+                            exception_handler=graph_err_handler) as g:
         g.custom_command('show', 'show_signed_in_user')
         g.custom_command('list-owned-objects', 'list_owned_objects')