Update the registration logic to print manual steps when adal authentication fails (#447)

Mitigate the deployment issue related to the conditional access policy.
The registration logic is updated to use the old rbac python library when possible. 
The deployment will print some manual step for operations that cannot be automated

src/deployment/deploy.py

@@ -67,7 +67,6 @@ from registration import (
     add_application_password,
     assign_scaleset_role,
     authorize_application,
-    get_application,
     register_application,
     update_pool_registration,
 )
@@ -329,10 +328,11 @@ class Client:
 
         (password_id, password) = self.create_password(app.object_id)
 
-        onefuzz_cli_app_uuid = uuid.UUID(ONEFUZZ_CLI_APP)
+        cli_app = client.applications.list(filter="appId eq '%s'" % ONEFUZZ_CLI_APP)
-        cli_app = get_application(onefuzz_cli_app_uuid)
 
-        if cli_app is None:
+        onefuzz_cli_app_uuid = uuid.UUID(ONEFUZZ_CLI_APP)
+
+        if not cli_app:
             logger.info(
                 "Could not find the default CLI application under the current "
                 "subscription, creating a new one"

src/deployment/registration.py

@@ -12,6 +12,7 @@ from enum import Enum
 from typing import Any, Dict, List, NamedTuple, Optional, Tuple
 from uuid import UUID, uuid4
 
+import adal  # type: ignore
 import requests
 from azure.common.client_factory import get_client_from_cli_profile
 from azure.common.credentials import get_cli_profile
@@ -19,12 +20,21 @@ from azure.graphrbac import GraphRbacManagementClient
 from azure.graphrbac.models import (
     Application,
     ApplicationCreateParameters,
+    ApplicationUpdateParameters,
+    AppRole,
+    PasswordCredential,
     RequiredResourceAccess,
     ResourceAccess,
+    ServicePrincipal,
 )
 from functional import seq
 from msrest.serialization import TZ_UTC
 
+FIX_URL = (
+    "https://ms.portal.azure.com/#blade/Microsoft_AAD_RegisteredApps/"
+    "ApplicationMenuBlade/ProtectAnAPI/appId/%s/isMSAApp/"
+)
+
 logger = logging.getLogger("deploy")
 
 
@@ -70,6 +80,13 @@ def query_microsoft_graph(
         )
 
 
+def get_graph_client() -> GraphRbacManagementClient:
+    client: GraphRbacManagementClient = get_client_from_cli_profile(
+        GraphRbacManagementClient
+    )
+    return client
+
+
 class ApplicationInfo(NamedTuple):
     client_id: UUID
     client_secret: str
@@ -85,7 +102,7 @@ def register_application(
     registration_name: str, onefuzz_instance_name: str, approle: OnefuzzAppRole
 ) -> ApplicationInfo:
     logger.info("retrieving the application registration %s" % registration_name)
-    client = get_client_from_cli_profile(GraphRbacManagementClient)
+    client = get_graph_client()
     apps: List[Application] = list(
         client.applications.list(filter="displayName eq '%s'" % registration_name)
     )
@@ -132,7 +149,7 @@ def create_application_credential(application_name: str) -> str:
     """ Add a new password to the application registration """
 
     logger.info("creating application credential for '%s'" % application_name)
-    client = get_client_from_cli_profile(GraphRbacManagementClient)
+    client = get_graph_client()
     apps: List[Application] = list(
         client.applications.list(filter="displayName eq '%s'" % application_name)
     )
@@ -148,7 +165,7 @@ def create_application_registration(
 ) -> Application:
     """ Create an application registration """
 
-    client = get_client_from_cli_profile(GraphRbacManagementClient)
+    client = get_graph_client()
     apps: List[Application] = list(
         client.applications.list(filter="displayName eq '%s'" % onefuzz_instance_name)
     )
@@ -189,22 +206,16 @@ def create_application_registration(
         atttempts = atttempts - 1
         try:
             time.sleep(5)
-            query_microsoft_graph(
+
-                method="PATCH",
+            client = get_graph_client()
-                resource="applications/%s" % registered_app.object_id,
+            update_param = ApplicationUpdateParameters(
-                body={
+                reply_urls=["https://%s.azurewebsites.net" % onefuzz_instance_name]
-                    "publicClient": {
-                        "redirectUris": [
-                            "https://%s.azurewebsites.net" % onefuzz_instance_name
-                        ]
-                    },
-                    "isFallbackPublicClient": True,
-                },
             )
+            client.applications.patch(registered_app.object_id, update_param)
+
             break
-        except GraphQueryError as err:
+        except Exception:
-            if err.status_code == 404:
+            continue
-                continue
 
     authorize_application(UUID(registered_app.app_id), UUID(app.app_id))
     return registered_app
@@ -232,6 +243,26 @@ def add_application_password(app_object_id: UUID) -> Tuple[str, str]:
         raise Exception("unable to create password")
 
 
+def add_application_password_legacy(app_object_id: UUID) -> Tuple[str, str]:
+    key = str(uuid4())
+    password = str(uuid4())
+
+    client = get_graph_client()
+    password_cred = [
+        PasswordCredential(
+            start_date="%s" % datetime.now(TZ_UTC).strftime("%Y-%m-%dT%H:%M.%fZ"),
+            end_date="%s"
+            % (datetime.now(TZ_UTC) + timedelta(days=365)).strftime(
+                "%Y-%m-%dT%H:%M.%fZ"
+            ),
+            key_id=key,
+            value=password,
+        )
+    ]
+    client.applications.update_password_credentials(app_object_id, password_cred)
+    return (key, password)
+
+
 def add_application_password_impl(app_object_id: UUID) -> Tuple[str, str]:
     key = uuid4()
     password_request = {
@@ -245,13 +276,15 @@ def add_application_password_impl(app_object_id: UUID) -> Tuple[str, str]:
         }
     }
 
-    password: Dict = query_microsoft_graph(
+    try:
-        method="POST",
+        password: Dict = query_microsoft_graph(
-        resource="applications/%s/addPassword" % app_object_id,
+            method="POST",
-        body=password_request,
+            resource="applications/%s/addPassword" % app_object_id,
-    )
+            body=password_request,
-
+        )
-    return (str(key), password["secretText"])
+        return (str(key), password["secretText"])
+    except adal.AdalError:
+        return add_application_password_legacy(app_object_id)
 
 
 def get_application(app_id: UUID) -> Optional[Any]:
@@ -271,40 +304,46 @@ def authorize_application(
     onefuzz_app_id: UUID,
     permissions: List[str] = ["user_impersonation"],
 ) -> None:
-    onefuzz_app = get_application(onefuzz_app_id)
+    try:
-    if onefuzz_app is None:
+        onefuzz_app = get_application(onefuzz_app_id)
-        logger.error("Application '%s' not found" % onefuzz_app_id)
+        if onefuzz_app is None:
-        return
+            logger.error("Application '%s' not found", onefuzz_app_id)
+            return
 
-    scopes = seq(onefuzz_app["api"]["oauth2PermissionScopes"]).filter(
+        scopes = seq(onefuzz_app["api"]["oauth2PermissionScopes"]).filter(
-        lambda scope: scope["value"] in permissions
+            lambda scope: scope["value"] in permissions
-    )
-
-    existing_preAuthorizedApplications = (
-        seq(onefuzz_app["api"]["preAuthorizedApplications"])
-        .map(
-            lambda paa: seq(paa["delegatedPermissionIds"]).map(
-                lambda permission_id: (paa["appId"], permission_id)
-            )
         )
-        .flatten()
-    )
 
-    preAuthorizedApplications = (
+        existing_preAuthorizedApplications = (
-        scopes.map(lambda s: (str(registration_app_id), s["id"]))
+            seq(onefuzz_app["api"]["preAuthorizedApplications"])
-        .union(existing_preAuthorizedApplications)
+            .map(
-        .distinct()
+                lambda paa: seq(paa["delegatedPermissionIds"]).map(
-        .group_by_key()
+                    lambda permission_id: (paa["appId"], permission_id)
-        .map(lambda data: {"appId": data[0], "delegatedPermissionIds": data[1]})
+                )
-    )
+            )
+            .flatten()
+        )
 
-    query_microsoft_graph(
+        preAuthorizedApplications = (
-        method="PATCH",
+            scopes.map(lambda s: (str(registration_app_id), s["id"]))
-        resource="applications/%s" % onefuzz_app["id"],
+            .union(existing_preAuthorizedApplications)
-        body={
+            .distinct()
-            "api": {"preAuthorizedApplications": preAuthorizedApplications.to_list()}
+            .group_by_key()
-        },
+            .map(lambda data: {"appId": data[0], "delegatedPermissionIds": data[1]})
-    )
+        )
+
+        query_microsoft_graph(
+            method="PATCH",
+            resource="applications/%s" % onefuzz_app["id"],
+            body={
+                "api": {
+                    "preAuthorizedApplications": preAuthorizedApplications.to_list()
+                }
+            },
+        )
+    except adal.AdalError:
+        logger.warning("*** Browse to: %s", FIX_URL % onefuzz_app_id)
+        logger.warning("*** Then add the client application %s", registration_app_id)
 
 
 def create_and_display_registration(
@@ -330,78 +369,138 @@ def update_pool_registration(onefuzz_instance_name: str) -> None:
     )
 
 
-def assign_scaleset_role(onefuzz_instance_name: str, scaleset_name: str) -> None:
+def assign_scaleset_role_manually(
-    """
+    onefuzz_instance_name: str, scaleset_name: str
-    Allows the nodes in the scaleset to access the service by assigning
+) -> None:
-    their managed identity to the ManagedNode Role
-    """
 
-    onefuzz_service_appId = query_microsoft_graph(
+    client = get_graph_client()
-        method="GET",
+    apps: List[Application] = list(
-        resource="applications",
+        client.applications.list(filter="displayName eq '%s'" % onefuzz_instance_name)
-        params={
-            "$filter": "displayName eq '%s'" % onefuzz_instance_name,
-            "$select": "appId",
-        },
     )
 
-    if len(onefuzz_service_appId["value"]) == 0:
+    if not apps:
         raise Exception("onefuzz app registration not found")
-    appId = onefuzz_service_appId["value"][0]["appId"]
 
-    onefuzz_service_principals = query_microsoft_graph(
+    app = apps[0]
-        method="GET",
+    appId = app.app_id
-        resource="servicePrincipals",
+
-        params={"$filter": "appId eq '%s'" % appId},
+    onefuzz_service_principals: List[ServicePrincipal] = list(
+        client.service_principals.list(filter="appId eq '%s'" % appId)
     )
 
-    if len(onefuzz_service_principals["value"]) == 0:
+    if not onefuzz_service_principals:
         raise Exception("onefuzz app service principal not found")
-    onefuzz_service_principal = onefuzz_service_principals["value"][0]
+    onefuzz_service_principal = onefuzz_service_principals[0]
 
-    scaleset_service_principals = query_microsoft_graph(
+    scaleset_service_principals: List[ServicePrincipal] = list(
-        method="GET",
+        client.service_principals.list(filter="displayName eq '%s'" % scaleset_name)
-        resource="servicePrincipals",
-        params={"$filter": "displayName eq '%s'" % scaleset_name},
     )
-    if len(scaleset_service_principals["value"]) == 0:
+
+    if not scaleset_service_principals:
         raise Exception("scaleset service principal not found")
-    scaleset_service_principal = scaleset_service_principals["value"][0]
+    scaleset_service_principal = scaleset_service_principals[0]
 
-    managed_node_role = (
+    scaleset_service_principal.app_roles
-        seq(onefuzz_service_principal["appRoles"])
+    app_roles: List[AppRole] = [
-        .filter(lambda x: x["value"] == OnefuzzAppRole.ManagedNode.value)
+        role for role in app.app_roles if role.value == OnefuzzAppRole.ManagedNode.value
-        .head_option()
+    ]
-    )
 
-    if not managed_node_role:
+    if not app_roles:
         raise Exception(
             "ManagedNode role not found in the OneFuzz application "
             "registration. Please redeploy the instance"
         )
 
-    assignments = query_microsoft_graph(
+    body = '{ "principalId": "%s", "resourceId": "%s", "appRoleId": "%s"}' % (
-        method="GET",
+        scaleset_service_principal.object_id,
-        resource="servicePrincipals/%s/appRoleAssignments"
+        onefuzz_service_principal.object_id,
-        % scaleset_service_principal["id"],
+        app_roles[0].id,
     )
 
-    # check if the role is already assigned
+    query = (
-    role_assigned = seq(assignments["value"]).find(
+        "az rest --method post --url https://graph.microsoft.com/v1.0/servicePrincipals/%s/appRoleAssignedTo --body '%s' --headers \"Content-Type\"=application/json"
-        lambda assignment: assignment["appRoleId"] == managed_node_role["id"]
+        % (scaleset_service_principal.object_id, body)
     )
-    if not role_assigned:
+
-        query_microsoft_graph(
+    logger.warning(
-            method="POST",
+        "execute the following query in the azure portal bash shell : \n%s" % query
-            resource="servicePrincipals/%s/appRoleAssignedTo"
+    )
-            % scaleset_service_principal["id"],
+
-            body={
+
-                "principalId": scaleset_service_principal["id"],
+def assign_scaleset_role(onefuzz_instance_name: str, scaleset_name: str) -> None:
-                "resourceId": onefuzz_service_principal["id"],
+    """
-                "appRoleId": managed_node_role["id"],
+    Allows the nodes in the scaleset to access the service by assigning
+    their managed identity to the ManagedNode Role
+    """
+    try:
+        onefuzz_service_appId = query_microsoft_graph(
+            method="GET",
+            resource="applications",
+            params={
+                "$filter": "displayName eq '%s'" % onefuzz_instance_name,
+                "$select": "appId",
             },
         )
 
+        if len(onefuzz_service_appId["value"]) == 0:
+            raise Exception("onefuzz app registration not found")
+        appId = onefuzz_service_appId["value"][0]["appId"]
+
+        onefuzz_service_principals = query_microsoft_graph(
+            method="GET",
+            resource="servicePrincipals",
+            params={"$filter": "appId eq '%s'" % appId},
+        )
+
+        if len(onefuzz_service_principals["value"]) == 0:
+            raise Exception("onefuzz app service principal not found")
+        onefuzz_service_principal = onefuzz_service_principals["value"][0]
+
+        scaleset_service_principals = query_microsoft_graph(
+            method="GET",
+            resource="servicePrincipals",
+            params={"$filter": "displayName eq '%s'" % scaleset_name},
+        )
+        if len(scaleset_service_principals["value"]) == 0:
+            raise Exception("scaleset service principal not found")
+        scaleset_service_principal = scaleset_service_principals["value"][0]
+
+        managed_node_role = (
+            seq(onefuzz_service_principal["appRoles"])
+            .filter(lambda x: x["value"] == OnefuzzAppRole.ManagedNode.value)
+            .head_option()
+        )
+
+        if not managed_node_role:
+            raise Exception(
+                "ManagedNode role not found in the OneFuzz application "
+                "registration. Please redeploy the instance"
+            )
+
+        assignments = query_microsoft_graph(
+            method="GET",
+            resource="servicePrincipals/%s/appRoleAssignments"
+            % scaleset_service_principal["id"],
+        )
+
+        # check if the role is already assigned
+        role_assigned = seq(assignments["value"]).find(
+            lambda assignment: assignment["appRoleId"] == managed_node_role["id"]
+        )
+        if not role_assigned:
+            query_microsoft_graph(
+                method="POST",
+                resource="servicePrincipals/%s/appRoleAssignedTo"
+                % scaleset_service_principal["id"],
+                body={
+                    "principalId": scaleset_service_principal["id"],
+                    "resourceId": onefuzz_service_principal["id"],
+                    "appRoleId": managed_node_role["id"],
+                },
+            )
+    except adal.AdalError:
+        assign_scaleset_role_manually(onefuzz_instance_name, scaleset_name)
+
 
 def main() -> None:
     formatter = argparse.ArgumentDefaultsHelpFormatter

src/deployment/requirements.txt

@@ -12,3 +12,4 @@ azure-storage-queue==12.1.3
 cryptography<3.0.0,>=2.3.1
 pyfunctional==1.4.2
 pyopenssl==19.1.0
+adal~=1.2.5