From bd8d425cee90df2ea2326d2e03901813c698514c Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Mon, 7 Jun 2021 11:02:45 -0700 Subject: [PATCH 01/64] migrate to msgraph --- src/deployment/registration.py | 143 ++++++++++++++++----------------- 1 file changed, 69 insertions(+), 74 deletions(-) diff --git a/src/deployment/registration.py b/src/deployment/registration.py index 8d4a82c969..6791914ae8 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -12,6 +12,8 @@ from typing import Any, Callable, Dict, List, NamedTuple, Optional, Tuple, TypeVar from uuid import UUID, uuid4 +# https://github.com/microsoftgraph/msgraph-sdk-python-core + import requests from azure.cli.core.azclierror import AuthenticationError from azure.common.client_factory import get_client_from_cli_profile @@ -141,39 +143,29 @@ def register_application( subscription_id: str, ) -> ApplicationInfo: logger.info("retrieving the application registration %s" % registration_name) - client = get_graph_client(subscription_id) - apps: List[Application] = list( - client.applications.list(filter="displayName eq '%s'" % registration_name) - ) - if len(apps) == 0: + app = get_application(display_name=registration_name) + + if not app: logger.info("No existing registration found. creating a new one") app = create_application_registration( onefuzz_instance_name, registration_name, approle, subscription_id ) else: - app = apps[0] logger.info( "Found existing application objectId '%s' - appid '%s'" - % (app.object_id, app.app_id) + % (app.object_id, app["appId"] ) ) - onefuzz_apps: List[Application] = list( - client.applications.list(filter="displayName eq '%s'" % onefuzz_instance_name) - ) + onefuzz_app = get_application(display_name=onefuzz_instance_name) - if len(onefuzz_apps) == 0: + if not(onefuzz_app): raise Exception("onefuzz app not found") - onefuzz_app = onefuzz_apps[0] - pre_authorized_applications = ( - onefuzz_app.pre_authorized_applications - if onefuzz_app.pre_authorized_applications is not None - else [] - ) + pre_authorized_applications = onefuzz_app["apiApplication"]["preAuthorizedApplications"] - if app.app_id not in [app.app_id for app in pre_authorized_applications]: - authorize_application(UUID(app.app_id), UUID(onefuzz_app.app_id)) + if app["appId"] not in [app["appId"] for app in pre_authorized_applications]: + authorize_application(UUID(app["appId"]), UUID(onefuzz_app["appId"])) password = create_application_credential(registration_name, subscription_id) @@ -188,14 +180,12 @@ def create_application_credential(application_name: str, subscription_id: str) - """Add a new password to the application registration""" logger.info("creating application credential for '%s'" % application_name) - client = get_graph_client(subscription_id) - apps: List[Application] = list( - client.applications.list(filter="displayName eq '%s'" % application_name) - ) + app = get_application(display_name=application_name) - app: Application = apps[0] + if not app: + raise Exception("app not found") - (_, password) = add_application_password(app.object_id, subscription_id) + (_, password) = add_application_password(app["objectId"], subscription_id) return str(password) @@ -204,36 +194,42 @@ def create_application_registration( ) -> Application: """Create an application registration""" - client = get_graph_client(subscription_id) - apps: List[Application] = list( - client.applications.list(filter="displayName eq '%s'" % onefuzz_instance_name) - ) + app = get_application(display_name=onefuzz_instance_name) + + if not app: + raise Exception("onefuzz app registration not found") - app = apps[0] resource_access = [ - ResourceAccess(id=role.id, type="Role") - for role in app.app_roles - if role.value == approle.value + { "id": "guid", "type": "string" } + for role in app["appRoles"] + if role["value"] == approle.value ] - params = ApplicationCreateParameters( - is_device_only_auth_supported=True, - display_name=name, - identifier_uris=[], - password_credentials=[], - required_resource_access=( - [ - RequiredResourceAccess( - resource_access=resource_access, - resource_app_id=app.app_id, - ) - ] - if len(resource_access) > 0 - else [] - ), - ) - registered_app: Application = client.applications.create(params) + params = { + "isDeviceOnlyAuthSupported": True, + "appRoles": [{"@odata.type": "microsoft.graph.appRole"}], + "displayName": name, + "publicClient": { + "redirectUris": ["https://%s.azurewebsites.net" % onefuzz_instance_name] + }, + "requiredResourceAccess": ( + [ + { + "resourceAccess": resource_access, + "resourceAppId":app["appId"], + } + ] + if len(resource_access) > 0 + else [] + ), + } + + registered_app:Dict = query_microsoft_graph( + method="POST", + resource="applications", + body=params, + ) logger.info("creating service principal") service_principal_params = ServicePrincipalCreateParameters( @@ -243,29 +239,20 @@ def create_application_registration( app_id=registered_app.app_id, ) - client.service_principals.create(service_principal_params) - - atttempts = 5 - while True: - if atttempts < 0: - raise Exception( - "Unable to create application registration, Please try again" - ) - - atttempts = atttempts - 1 - try: - time.sleep(5) - - update_param = ApplicationUpdateParameters( - reply_urls=["https://%s.azurewebsites.net" % onefuzz_instance_name] - ) - client.applications.patch(registered_app.object_id, update_param) + service_principal_params = { + "accountEnabled": True, + "appRoleAssignmentRequired": False, + "servicePrincipalType": "Application", + "appId": registered_app["appId"], + } - break - except Exception: - continue + query_microsoft_graph( + method="POST", + resource="applications/servicePrincipals", + body=service_principal_params, + ) - authorize_application(UUID(registered_app.app_id), UUID(app.app_id)) + authorize_application(UUID(registered_app["appId"]), UUID(app["appId"])) assign_app_role( onefuzz_instance_name, name, subscription_id, OnefuzzAppRole.ManagedNode ) @@ -333,11 +320,19 @@ def add_application_password_impl( return add_application_password_legacy(app_object_id, subscription_id) -def get_application(app_id: UUID) -> Optional[Any]: +def get_application(app_id: Optional[UUID]=None, display_name: Optional[str]=None) -> Optional[Any]: + filters = [] + if app_id: + filters.append("appId eq '%s'" % app_id) + if display_name: + filters.append("displayName eq '%s'" % display_name) + + filter_str = " and ".join(filters) + apps: Dict = query_microsoft_graph( method="GET", resource="applications", - params={"$filter": "appId eq '%s'" % app_id}, + params=filter_str, ) if len(apps["value"]) == 0: return None @@ -351,7 +346,7 @@ def authorize_application( permissions: List[str] = ["user_impersonation"], ) -> None: try: - onefuzz_app = get_application(onefuzz_app_id) + onefuzz_app = get_application(app_id = onefuzz_app_id) if onefuzz_app is None: logger.error("Application '%s' not found", onefuzz_app_id) return From 1ad0f5a28aae462398c569fb9e59c548ea7551d0 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Mon, 7 Jun 2021 14:13:15 -0700 Subject: [PATCH 02/64] add subscription id to query_microsoft_graph --- src/deployment/deploy.py | 4 ++-- src/deployment/registration.py | 29 ++++++++++++++++++++++------- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index 11e1c95993..acfc914ae1 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -389,12 +389,12 @@ def try_sp_create() -> None: client.applications.patch( app.object_id, ApplicationUpdateParameters(identifier_uris=[url]) ) - set_app_audience(app.object_id, "AzureADMultipleOrgs") + set_app_audience(app.object_id, "AzureADMultipleOrgs", subscription_id=self.get_subscription_id()) elif ( not self.multi_tenant_domain and app.sign_in_audience == "AzureADMultipleOrgs" ): - set_app_audience(app.object_id, "AzureADMyOrg") + set_app_audience(app.object_id, "AzureADMyOrg", subscription_id=self.get_subscription_id()) url = "https://%s.azurewebsites.net" % self.application_name client.applications.patch( app.object_id, ApplicationUpdateParameters(identifier_uris=[url]) diff --git a/src/deployment/registration.py b/src/deployment/registration.py index 6791914ae8..0151006148 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -5,6 +5,7 @@ import argparse import logging +from re import S, sub import time import urllib.parse from datetime import datetime, timedelta @@ -53,10 +54,12 @@ def query_microsoft_graph( resource: str, params: Optional[Dict] = None, body: Optional[Dict] = None, + subscription: Optional[str] = None, ) -> Any: profile = get_cli_profile() (token_type, access_token, _), _, _ = profile.get_raw_token( - resource="https://graph.microsoft.com" + resource="https://graph.microsoft.com", + subscription=subscription ) url = urllib.parse.urljoin("https://graph.microsoft.com/v1.0/", resource) headers = { @@ -144,7 +147,7 @@ def register_application( ) -> ApplicationInfo: logger.info("retrieving the application registration %s" % registration_name) - app = get_application(display_name=registration_name) + app = get_application(display_name=registration_name, subscription_id=subscription_id) if not app: logger.info("No existing registration found. creating a new one") @@ -157,7 +160,7 @@ def register_application( % (app.object_id, app["appId"] ) ) - onefuzz_app = get_application(display_name=onefuzz_instance_name) + onefuzz_app = get_application(display_name=onefuzz_instance_name, subscription_id=subscription_id) if not(onefuzz_app): raise Exception("onefuzz app not found") @@ -194,7 +197,7 @@ def create_application_registration( ) -> Application: """Create an application registration""" - app = get_application(display_name=onefuzz_instance_name) + app = get_application(display_name=onefuzz_instance_name, subscription_id=subscription_id) if not app: raise Exception("onefuzz app registration not found") @@ -229,6 +232,7 @@ def create_application_registration( method="POST", resource="applications", body=params, + subscription=subscription_id ) logger.info("creating service principal") @@ -250,9 +254,10 @@ def create_application_registration( method="POST", resource="applications/servicePrincipals", body=service_principal_params, + subscription=subscription_id ) - authorize_application(UUID(registered_app["appId"]), UUID(app["appId"])) + authorize_application(UUID(registered_app["appId"]), UUID(app["appId"]), subscription_id) assign_app_role( onefuzz_instance_name, name, subscription_id, OnefuzzAppRole.ManagedNode ) @@ -314,13 +319,14 @@ def add_application_password_impl( method="POST", resource="applications/%s/addPassword" % app_object_id, body=password_request, + subscription=subscription_id ) return (str(key), password["secretText"]) except AuthenticationError: return add_application_password_legacy(app_object_id, subscription_id) -def get_application(app_id: Optional[UUID]=None, display_name: Optional[str]=None) -> Optional[Any]: +def get_application(app_id: Optional[UUID]=None, display_name: Optional[str]=None, subscription_id: Optional[str] = None) -> Optional[Any]: filters = [] if app_id: filters.append("appId eq '%s'" % app_id) @@ -333,6 +339,7 @@ def get_application(app_id: Optional[UUID]=None, display_name: Optional[str]=Non method="GET", resource="applications", params=filter_str, + subscription=subscription_id ) if len(apps["value"]) == 0: return None @@ -344,6 +351,7 @@ def authorize_application( registration_app_id: UUID, onefuzz_app_id: UUID, permissions: List[str] = ["user_impersonation"], + subscription_id: Optional[str] = None ) -> None: try: onefuzz_app = get_application(app_id = onefuzz_app_id) @@ -384,6 +392,7 @@ def add_preauthorized_app() -> None: "preAuthorizedApplications": preAuthorizedApplications.to_list() } }, + subscription=subscription_id ) retry(add_preauthorized_app, "authorize application") @@ -501,6 +510,7 @@ def assign_app_role( "$filter": "displayName eq '%s'" % onefuzz_instance_name, "$select": "appId", }, + subscription=subscription_id ) if len(onefuzz_service_appId["value"]) == 0: raise Exception("onefuzz app registration not found") @@ -509,6 +519,7 @@ def assign_app_role( method="GET", resource="servicePrincipals", params={"$filter": "appId eq '%s'" % appId}, + subscription=subscription_id ) if len(onefuzz_service_principals["value"]) == 0: @@ -518,6 +529,7 @@ def assign_app_role( method="GET", resource="servicePrincipals", params={"$filter": "displayName eq '%s'" % application_name}, + subscription=subscription_id ) if len(scaleset_service_principals["value"]) == 0: raise Exception("scaleset service principal not found") @@ -537,6 +549,7 @@ def assign_app_role( method="GET", resource="servicePrincipals/%s/appRoleAssignments" % scaleset_service_principal["id"], + subscription=subscription_id ) # check if the role is already assigned @@ -553,6 +566,7 @@ def assign_app_role( "resourceId": onefuzz_service_principal["id"], "appRoleId": managed_node_role["id"], }, + subscription=subscription_id ) except AuthenticationError: assign_app_role_manually( @@ -560,7 +574,7 @@ def assign_app_role( ) -def set_app_audience(objectId: str, audience: str) -> None: +def set_app_audience(objectId: str, audience: str, subscription_id: Optional[str]=None) -> None: # typical audience values: AzureADMyOrg, AzureADMultipleOrgs http_body = {"signInAudience": audience} try: @@ -568,6 +582,7 @@ def set_app_audience(objectId: str, audience: str) -> None: method="PATCH", resource="applications/%s" % objectId, body=http_body, + subscription=subscription_id ) except GraphQueryError: query = ( From ac0f4005f6f0523ae8ed1f4c23e881d642e7e5b2 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 8 Jun 2021 14:06:16 -0700 Subject: [PATCH 03/64] migrating remaingin references --- .../__app__/onefuzzlib/azure/creds.py | 49 ++++++++++++++++--- src/deployment/deploy.py | 7 +-- src/deployment/registration.py | 12 +++-- 3 files changed, 55 insertions(+), 13 deletions(-) diff --git a/src/api-service/__app__/onefuzzlib/azure/creds.py b/src/api-service/__app__/onefuzzlib/azure/creds.py index e3669f1317..2dc0a25440 100644 --- a/src/api-service/__app__/onefuzzlib/azure/creds.py +++ b/src/api-service/__app__/onefuzzlib/azure/creds.py @@ -4,13 +4,16 @@ # Licensed under the MIT License. import os -from typing import Any, List +import urllib.parse +from typing import Any, Dict, List, Optional from uuid import UUID +import requests from azure.graphrbac import GraphRbacManagementClient from azure.graphrbac.models import CheckGroupMembershipParameters from azure.identity import DefaultAzureCredential from azure.keyvault.secrets import SecretClient +from azure.mgmt import subscription from azure.mgmt.resource import ResourceManagementClient from azure.mgmt.subscription import SubscriptionClient from memoization import cached @@ -101,13 +104,47 @@ def get_graph_client() -> GraphRbacManagementClient: return GraphRbacManagementClient(get_msi(), get_subscription()) +def query_microsoft_graph( + method: str, + resource: str, + params: Optional[Dict] = None, + body: Optional[Dict] = None, +) -> Any: + auth = get_msi() + access_token = auth.token["access_token"] + token_type = auth.token["token_type"] + + url = urllib.parse.urljoin("https://graph.microsoft.com/v1.0/", resource) + headers = { + "Authorization": "%s %s" % (token_type, access_token), + "Content-Type": "application/json", + } + response = requests.request( + method=method, url=url, headers=headers, params=params, json=body + ) + + response.status_code + + if 200 <= response.status_code < 300: + try: + return response.json() + except ValueError: + return None + else: + error_text = str(response.content, encoding="utf-8", errors="backslashreplace") + raise Exception( + "request did not succeed: HTTP %s - %s" + % (response.status_code, error_text), + response.status_code, + ) + + def is_member_of(group_id: str, member_id: str) -> bool: - client = get_graph_client() - return bool( - client.groups.is_member_of( - CheckGroupMembershipParameters(group_id=group_id, member_id=member_id) - ).value + body = {"groupIds": [group_id]} + response = query_microsoft_graph( + method="POST", resource=f"users/{member_id}/checkMemberGroups", body=body ) + return group_id in response["value"] @cached diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index acfc914ae1..0de31f73a5 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -67,7 +67,7 @@ add_application_password, assign_app_role, authorize_application, - get_graph_client, + get_tenant_id, register_application, set_app_audience, update_pool_registration, @@ -437,10 +437,11 @@ def try_sp_create() -> None: if self.multi_tenant_domain: authority = COMMON_AUTHORITY else: - onefuzz_client = get_graph_client(self.get_subscription_id()) + + tenant_id = get_tenant_id(self.get_subscription_id()) authority = ( "https://login.microsoftonline.com/%s" - % onefuzz_client.config.tenant_id + % tenant_id ) self.cli_config = { "client_id": onefuzz_cli_app.app_id, diff --git a/src/deployment/registration.py b/src/deployment/registration.py index 0151006148..0ad6958c44 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -22,12 +22,8 @@ from azure.graphrbac import GraphRbacManagementClient from azure.graphrbac.models import ( Application, - ApplicationCreateParameters, - ApplicationUpdateParameters, AppRole, PasswordCredential, - RequiredResourceAccess, - ResourceAccess, ServicePrincipal, ServicePrincipalCreateParameters, ) @@ -85,6 +81,14 @@ def query_microsoft_graph( response.status_code, ) +def get_tenant_id(subscription_id: Optional[str]=None) -> str: + result = query_microsoft_graph( + method="GET", + resource="organization", + subscription=subscription_id + ) + result["value"]["id"] + OperationResult = TypeVar("OperationResult") From 4902528dbd55d88ca90f4d1a8512eb02e0c191f6 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 8 Jun 2021 15:06:42 -0700 Subject: [PATCH 04/64] formatting --- src/deployment/deploy.py | 17 ++-- src/deployment/registration.py | 151 +++++++++++++++++++-------------- 2 files changed, 96 insertions(+), 72 deletions(-) diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index 0de31f73a5..36d2acd1cc 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -389,12 +389,20 @@ def try_sp_create() -> None: client.applications.patch( app.object_id, ApplicationUpdateParameters(identifier_uris=[url]) ) - set_app_audience(app.object_id, "AzureADMultipleOrgs", subscription_id=self.get_subscription_id()) + set_app_audience( + app.object_id, + "AzureADMultipleOrgs", + subscription_id=self.get_subscription_id(), + ) elif ( not self.multi_tenant_domain and app.sign_in_audience == "AzureADMultipleOrgs" ): - set_app_audience(app.object_id, "AzureADMyOrg", subscription_id=self.get_subscription_id()) + set_app_audience( + app.object_id, + "AzureADMyOrg", + subscription_id=self.get_subscription_id(), + ) url = "https://%s.azurewebsites.net" % self.application_name client.applications.patch( app.object_id, ApplicationUpdateParameters(identifier_uris=[url]) @@ -439,10 +447,7 @@ def try_sp_create() -> None: else: tenant_id = get_tenant_id(self.get_subscription_id()) - authority = ( - "https://login.microsoftonline.com/%s" - % tenant_id - ) + authority = "https://login.microsoftonline.com/%s" % tenant_id self.cli_config = { "client_id": onefuzz_cli_app.app_id, "authority": authority, diff --git a/src/deployment/registration.py b/src/deployment/registration.py index 0ad6958c44..8ae5207783 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -5,16 +5,14 @@ import argparse import logging -from re import S, sub import time import urllib.parse from datetime import datetime, timedelta from enum import Enum -from typing import Any, Callable, Dict, List, NamedTuple, Optional, Tuple, TypeVar +from re import S, sub +from typing import Any, Callable, Dict, List, NamedTuple, Optional, Tuple, TypeVar, cast from uuid import UUID, uuid4 -# https://github.com/microsoftgraph/msgraph-sdk-python-core - import requests from azure.cli.core.azclierror import AuthenticationError from azure.common.client_factory import get_client_from_cli_profile @@ -30,6 +28,9 @@ from functional import seq from msrest.serialization import TZ_UTC +# https://github.com/microsoftgraph/msgraph-sdk-python-core + + FIX_URL = ( "https://ms.portal.azure.com/#blade/Microsoft_AAD_RegisteredApps/" "ApplicationMenuBlade/ProtectAnAPI/appId/%s/isMSAApp/" @@ -54,8 +55,7 @@ def query_microsoft_graph( ) -> Any: profile = get_cli_profile() (token_type, access_token, _), _, _ = profile.get_raw_token( - resource="https://graph.microsoft.com", - subscription=subscription + resource="https://graph.microsoft.com", subscription=subscription ) url = urllib.parse.urljoin("https://graph.microsoft.com/v1.0/", resource) headers = { @@ -81,13 +81,12 @@ def query_microsoft_graph( response.status_code, ) -def get_tenant_id(subscription_id: Optional[str]=None) -> str: + +def get_tenant_id(subscription_id: Optional[str] = None) -> str: result = query_microsoft_graph( - method="GET", - resource="organization", - subscription=subscription_id - ) - result["value"]["id"] + method="GET", resource="organization", subscription=subscription_id + ) + return cast(str, result["value"]["id"]) OperationResult = TypeVar("OperationResult") @@ -151,7 +150,9 @@ def register_application( ) -> ApplicationInfo: logger.info("retrieving the application registration %s" % registration_name) - app = get_application(display_name=registration_name, subscription_id=subscription_id) + app = get_application( + display_name=registration_name, subscription_id=subscription_id + ) if not app: logger.info("No existing registration found. creating a new one") @@ -161,25 +162,30 @@ def register_application( else: logger.info( "Found existing application objectId '%s' - appid '%s'" - % (app.object_id, app["appId"] ) + % (app.object_id, app["appId"]) ) - onefuzz_app = get_application(display_name=onefuzz_instance_name, subscription_id=subscription_id) + onefuzz_app = get_application( + display_name=onefuzz_instance_name, subscription_id=subscription_id + ) - if not(onefuzz_app): + if not (onefuzz_app): raise Exception("onefuzz app not found") - pre_authorized_applications = onefuzz_app["apiApplication"]["preAuthorizedApplications"] + pre_authorized_applications = onefuzz_app["apiApplication"][ + "preAuthorizedApplications" + ] if app["appId"] not in [app["appId"] for app in pre_authorized_applications]: authorize_application(UUID(app["appId"]), UUID(onefuzz_app["appId"])) password = create_application_credential(registration_name, subscription_id) + tenant_id = get_tenant_id(subscription_id=subscription_id) return ApplicationInfo( client_id=app.app_id, client_secret=password, - authority=("https://login.microsoftonline.com/%s" % client.config.tenant_id), + authority=("https://login.microsoftonline.com/%s" % tenant_id), ) @@ -201,42 +207,43 @@ def create_application_registration( ) -> Application: """Create an application registration""" - app = get_application(display_name=onefuzz_instance_name, subscription_id=subscription_id) + app = get_application( + display_name=onefuzz_instance_name, subscription_id=subscription_id + ) if not app: raise Exception("onefuzz app registration not found") resource_access = [ - { "id": "guid", "type": "string" } + {"id": "guid", "type": "string"} for role in app["appRoles"] if role["value"] == approle.value ] - params = { - "isDeviceOnlyAuthSupported": True, - "appRoles": [{"@odata.type": "microsoft.graph.appRole"}], - "displayName": name, - "publicClient": { - "redirectUris": ["https://%s.azurewebsites.net" % onefuzz_instance_name] - }, - "requiredResourceAccess": ( - [ - { - "resourceAccess": resource_access, - "resourceAppId":app["appId"], - } - ] - if len(resource_access) > 0 - else [] - ), - } + "isDeviceOnlyAuthSupported": True, + "appRoles": [{"@odata.type": "microsoft.graph.appRole"}], + "displayName": name, + "publicClient": { + "redirectUris": ["https://%s.azurewebsites.net" % onefuzz_instance_name] + }, + "requiredResourceAccess": ( + [ + { + "resourceAccess": resource_access, + "resourceAppId": app["appId"], + } + ] + if len(resource_access) > 0 + else [] + ), + } - registered_app:Dict = query_microsoft_graph( + registered_app: Dict = query_microsoft_graph( method="POST", resource="applications", body=params, - subscription=subscription_id + subscription=subscription_id, ) logger.info("creating service principal") @@ -244,24 +251,28 @@ def create_application_registration( account_enabled=True, app_role_assignment_required=False, service_principal_type="Application", - app_id=registered_app.app_id, + app_id=registered_app["appId"], ) service_principal_params = { - "accountEnabled": True, - "appRoleAssignmentRequired": False, - "servicePrincipalType": "Application", - "appId": registered_app["appId"], - } + "accountEnabled": True, + "appRoleAssignmentRequired": False, + "servicePrincipalType": "Application", + "appId": registered_app["appId"], + } query_microsoft_graph( - method="POST", - resource="applications/servicePrincipals", - body=service_principal_params, - subscription=subscription_id - ) + method="POST", + resource="applications/servicePrincipals", + body=service_principal_params, + subscription=subscription_id, + ) - authorize_application(UUID(registered_app["appId"]), UUID(app["appId"]), subscription_id) + authorize_application( + UUID(registered_app["appId"]), + UUID(app["appId"]), + subscription_id=subscription_id, + ) assign_app_role( onefuzz_instance_name, name, subscription_id, OnefuzzAppRole.ManagedNode ) @@ -323,14 +334,18 @@ def add_application_password_impl( method="POST", resource="applications/%s/addPassword" % app_object_id, body=password_request, - subscription=subscription_id + subscription=subscription_id, ) return (str(key), password["secretText"]) except AuthenticationError: return add_application_password_legacy(app_object_id, subscription_id) -def get_application(app_id: Optional[UUID]=None, display_name: Optional[str]=None, subscription_id: Optional[str] = None) -> Optional[Any]: +def get_application( + app_id: Optional[UUID] = None, + display_name: Optional[str] = None, + subscription_id: Optional[str] = None, +) -> Optional[Any]: filters = [] if app_id: filters.append("appId eq '%s'" % app_id) @@ -342,8 +357,10 @@ def get_application(app_id: Optional[UUID]=None, display_name: Optional[str]=Non apps: Dict = query_microsoft_graph( method="GET", resource="applications", - params=filter_str, - subscription=subscription_id + params={ + "$filter": filter_str, + }, + subscription=subscription_id, ) if len(apps["value"]) == 0: return None @@ -355,10 +372,10 @@ def authorize_application( registration_app_id: UUID, onefuzz_app_id: UUID, permissions: List[str] = ["user_impersonation"], - subscription_id: Optional[str] = None + subscription_id: Optional[str] = None, ) -> None: try: - onefuzz_app = get_application(app_id = onefuzz_app_id) + onefuzz_app = get_application(app_id=onefuzz_app_id) if onefuzz_app is None: logger.error("Application '%s' not found", onefuzz_app_id) return @@ -396,7 +413,7 @@ def add_preauthorized_app() -> None: "preAuthorizedApplications": preAuthorizedApplications.to_list() } }, - subscription=subscription_id + subscription=subscription_id, ) retry(add_preauthorized_app, "authorize application") @@ -514,7 +531,7 @@ def assign_app_role( "$filter": "displayName eq '%s'" % onefuzz_instance_name, "$select": "appId", }, - subscription=subscription_id + subscription=subscription_id, ) if len(onefuzz_service_appId["value"]) == 0: raise Exception("onefuzz app registration not found") @@ -523,7 +540,7 @@ def assign_app_role( method="GET", resource="servicePrincipals", params={"$filter": "appId eq '%s'" % appId}, - subscription=subscription_id + subscription=subscription_id, ) if len(onefuzz_service_principals["value"]) == 0: @@ -533,7 +550,7 @@ def assign_app_role( method="GET", resource="servicePrincipals", params={"$filter": "displayName eq '%s'" % application_name}, - subscription=subscription_id + subscription=subscription_id, ) if len(scaleset_service_principals["value"]) == 0: raise Exception("scaleset service principal not found") @@ -553,7 +570,7 @@ def assign_app_role( method="GET", resource="servicePrincipals/%s/appRoleAssignments" % scaleset_service_principal["id"], - subscription=subscription_id + subscription=subscription_id, ) # check if the role is already assigned @@ -570,7 +587,7 @@ def assign_app_role( "resourceId": onefuzz_service_principal["id"], "appRoleId": managed_node_role["id"], }, - subscription=subscription_id + subscription=subscription_id, ) except AuthenticationError: assign_app_role_manually( @@ -578,7 +595,9 @@ def assign_app_role( ) -def set_app_audience(objectId: str, audience: str, subscription_id: Optional[str]=None) -> None: +def set_app_audience( + objectId: str, audience: str, subscription_id: Optional[str] = None +) -> None: # typical audience values: AzureADMyOrg, AzureADMultipleOrgs http_body = {"signInAudience": audience} try: @@ -586,7 +605,7 @@ def set_app_audience(objectId: str, audience: str, subscription_id: Optional[str method="PATCH", resource="applications/%s" % objectId, body=http_body, - subscription=subscription_id + subscription=subscription_id, ) except GraphQueryError: query = ( From a4379c8346375d7e7042b18a60d0ac08a71e5147 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 8 Jun 2021 15:54:15 -0700 Subject: [PATCH 05/64] adding missing dependencies --- src/cli/requirements.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cli/requirements.txt b/src/cli/requirements.txt index 9259bf2a5d..1703a8e64f 100644 --- a/src/cli/requirements.txt +++ b/src/cli/requirements.txt @@ -18,3 +18,5 @@ cryptography<3.4,>=3.2 pyjwt==1.7.1 # onefuzztypes version is set during build onefuzztypes==0.0.0 +six==1.16.0 +requests==2.25.1 \ No newline at end of file From e1fa0ad9cca32827bfb70079dedec4a57e2ee5dd Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Wed, 9 Jun 2021 12:12:15 -0700 Subject: [PATCH 06/64] flake fix --- src/api-service/__app__/onefuzzlib/azure/creds.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/api-service/__app__/onefuzzlib/azure/creds.py b/src/api-service/__app__/onefuzzlib/azure/creds.py index 2dc0a25440..5f89d5b7bb 100644 --- a/src/api-service/__app__/onefuzzlib/azure/creds.py +++ b/src/api-service/__app__/onefuzzlib/azure/creds.py @@ -10,10 +10,8 @@ import requests from azure.graphrbac import GraphRbacManagementClient -from azure.graphrbac.models import CheckGroupMembershipParameters from azure.identity import DefaultAzureCredential from azure.keyvault.secrets import SecretClient -from azure.mgmt import subscription from azure.mgmt.resource import ResourceManagementClient from azure.mgmt.subscription import SubscriptionClient from memoization import cached From c130d6d04718dcf3affbef0477d8f9b1e4aeb98e Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Wed, 9 Jun 2021 13:28:31 -0700 Subject: [PATCH 07/64] fix get_tenant_id --- src/deployment/registration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/deployment/registration.py b/src/deployment/registration.py index 8ae5207783..06ad5afb52 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -86,7 +86,7 @@ def get_tenant_id(subscription_id: Optional[str] = None) -> str: result = query_microsoft_graph( method="GET", resource="organization", subscription=subscription_id ) - return cast(str, result["value"]["id"]) + return cast(str, result["value"][0]["id"]) OperationResult = TypeVar("OperationResult") From 681867e35ff97cba4af4dc1c0e464b33e543a3c9 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Wed, 9 Jun 2021 18:39:36 -0700 Subject: [PATCH 08/64] cleanup --- src/cli/requirements.txt | 4 +--- src/deployment/registration.py | 2 -- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/cli/requirements.txt b/src/cli/requirements.txt index 1703a8e64f..e06400e77b 100644 --- a/src/cli/requirements.txt +++ b/src/cli/requirements.txt @@ -17,6 +17,4 @@ cryptography<3.4,>=3.2 # PyJWT needs to be pinned to the version used by azure-cli-core pyjwt==1.7.1 # onefuzztypes version is set during build -onefuzztypes==0.0.0 -six==1.16.0 -requests==2.25.1 \ No newline at end of file +onefuzztypes==0.0.0 \ No newline at end of file diff --git a/src/deployment/registration.py b/src/deployment/registration.py index 06ad5afb52..58b046e1e6 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -28,8 +28,6 @@ from functional import seq from msrest.serialization import TZ_UTC -# https://github.com/microsoftgraph/msgraph-sdk-python-core - FIX_URL = ( "https://ms.portal.azure.com/#blade/Microsoft_AAD_RegisteredApps/" From 82300244b1ef54fe5390417447e852f3df50edce Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Mon, 14 Jun 2021 08:44:39 -0700 Subject: [PATCH 09/64] formatting --- src/deployment/registration.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/deployment/registration.py b/src/deployment/registration.py index 58b046e1e6..1a0c5adcb9 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -28,7 +28,6 @@ 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/" From 0403485d474c236ecc84bca752e3e2fb86ac9e03 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Wed, 16 Jun 2021 12:31:10 -0700 Subject: [PATCH 10/64] migrate application creation in deploy.py --- .../__app__/onefuzzlib/azure/creds.py | 6 - src/deployment/deploy.py | 183 ++++++++------ src/deployment/registration.py | 229 +++++------------- 3 files changed, 164 insertions(+), 254 deletions(-) diff --git a/src/api-service/__app__/onefuzzlib/azure/creds.py b/src/api-service/__app__/onefuzzlib/azure/creds.py index 5f89d5b7bb..68a75db943 100644 --- a/src/api-service/__app__/onefuzzlib/azure/creds.py +++ b/src/api-service/__app__/onefuzzlib/azure/creds.py @@ -9,7 +9,6 @@ from uuid import UUID import requests -from azure.graphrbac import GraphRbacManagementClient from azure.identity import DefaultAzureCredential from azure.keyvault.secrets import SecretClient from azure.mgmt.resource import ResourceManagementClient @@ -97,11 +96,6 @@ def get_regions() -> List[Region]: return sorted([Region(x.name) for x in locations]) -@cached -def get_graph_client() -> GraphRbacManagementClient: - return GraphRbacManagementClient(get_msi(), get_subscription()) - - def query_microsoft_graph( method: str, resource: str, diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index 36d2acd1cc..3e641df05c 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -23,7 +23,6 @@ from azure.common.credentials import get_cli_profile from azure.core.exceptions import ResourceExistsError from azure.cosmosdb.table.tableservice import TableService -from azure.graphrbac import GraphRbacManagementClient from azure.graphrbac.models import ( Application, ApplicationCreateParameters, @@ -68,9 +67,11 @@ assign_app_role, authorize_application, get_tenant_id, + query_microsoft_graph, register_application, set_app_audience, update_pool_registration, + get_application ) # Found by manually assigning the User.Read permission to application @@ -260,43 +261,34 @@ def setup_rbac(self) -> None: logger.info("using existing client application") return - client = get_client_from_cli_profile( - GraphRbacManagementClient, subscription_id=self.get_subscription_id() + + app = get_application( + display_name=self.application_name, subscription_id=self.get_subscription_id() ) - logger.info("checking if RBAC already exists") - try: - existing = list( - client.applications.list( - filter="displayName eq '%s'" % self.application_name - ) - ) - except GraphErrorException: - logger.error("unable to query RBAC. Provide client_id and client_secret") - sys.exit(1) app_roles = [ - AppRole( - allowed_member_types=["Application"], - display_name=OnefuzzAppRole.CliClient.value, - id=str(uuid.uuid4()), - is_enabled=True, - description="Allows access from the CLI.", - value=OnefuzzAppRole.CliClient.value, - ), - AppRole( - allowed_member_types=["Application"], - display_name=OnefuzzAppRole.ManagedNode.value, - id=str(uuid.uuid4()), - is_enabled=True, - description="Allow access from a lab machine.", - value=OnefuzzAppRole.ManagedNode.value, - ), + { + "allowedMemberTypes": ["Application"], + "description": "Allows access from the CLI.", + "displayName": OnefuzzAppRole.CliClient.value, + "id": str(uuid.uuid4()), + "isEnabled": True, + "value": OnefuzzAppRole.CliClient.value + }, + { + "allowedMemberTypes": ["Application"], + "description": "Allow access from a lab machine.", + "displayName": OnefuzzAppRole.ManagedNode.value, + "id": str(uuid.uuid4()), + "isEnabled": True, + "value": OnefuzzAppRole.ManagedNode.value, + } ] - app: Optional[Application] = None + # app: Optional[Application] = None - if not existing: + if not app: logger.info("creating Application registration") if self.multi_tenant_domain: @@ -307,37 +299,49 @@ def setup_rbac(self) -> None: else: url = "https://%s.azurewebsites.net" % self.application_name - params = ApplicationCreateParameters( - display_name=self.application_name, - identifier_uris=[url], - reply_urls=[url + "/.auth/login/aad/callback"], - optional_claims=OptionalClaims(id_token=[], access_token=[]), - required_resource_access=[ - RequiredResourceAccess( - resource_access=[ - ResourceAccess(id=USER_READ_PERMISSION, type="Scope") - ], - resource_app_id=MICROSOFT_GRAPH_APP_ID, - ) - ], - app_roles=app_roles, - ) - - app = client.applications.create(params) + params = { + "displayName": self.application_name, + "identifierUris": [url], + "appRoles": app_roles, + "requiredResourceAccess": + [ + { + "resourceAccess": { + "id": USER_READ_PERMISSION, + "type": "Scope" + }, + "resourceAppId": MICROSOFT_GRAPH_APP_ID, + } + ], + } + + + app:Dict = query_microsoft_graph( + method="POST", + resource="applications", + body=params, + subscription=self.get_subscription_id(), + ) logger.info("creating service principal") - service_principal_params = ServicePrincipalCreateParameters( - account_enabled=True, - app_role_assignment_required=False, - service_principal_type="Application", - app_id=app.app_id, - ) + + service_principal_params = { + "accountEnabled": True, + "appRoleAssignmentRequired": False, + "servicePrincipalType": "Application", + "appId": app["appId"], + } def try_sp_create() -> None: error: Optional[Exception] = None for _ in range(10): try: - client.service_principals.create(service_principal_params) + query_microsoft_graph( + method="POST", + resource="applications/servicePrincipals", + body=service_principal_params, + subscription=self.get_subscription_id(), + ) return except GraphErrorException as err: # work around timing issue when creating service principal @@ -360,25 +364,36 @@ def try_sp_create() -> None: try_sp_create() else: - app = existing[0] - existing_role_values = [app_role.value for app_role in app.app_roles] + + existing_role_values = [app_role["value"] for app_role in app["appRoles"]] has_missing_roles = any( - [role.value not in existing_role_values for role in app_roles] + [role["value"] not in existing_role_values for role in app_roles] ) if has_missing_roles: # disabling the existing app role first to allow the update # this is a requirement to update the application roles - for role in app.app_roles: - role.is_enabled = False + for role in app["appRoles"]: + role["isEnabled"] = False + - client.applications.patch( - app.object_id, ApplicationUpdateParameters(app_roles=app.app_roles) + query_microsoft_graph( + method="PATCH", + resource=f"applications/{app['id']}", + body={ + "appRoles": app["AppRoles"] + }, + subscription=self.get_subscription_id(), ) # overriding the list of app roles - client.applications.patch( - app.object_id, ApplicationUpdateParameters(app_roles=app_roles) + query_microsoft_graph( + method="PATCH", + resource=f"applications/{app['id']}", + body={ + "appRoles": app_roles + }, + subscription=self.get_subscription_id(), ) if self.multi_tenant_domain and app.sign_in_audience == "AzureADMyOrg": @@ -386,9 +401,15 @@ def try_sp_create() -> None: self.multi_tenant_domain, self.application_name, ) - client.applications.patch( - app.object_id, ApplicationUpdateParameters(identifier_uris=[url]) + query_microsoft_graph( + method="PATCH", + resource=f"applications/{app['id']}", + body={ + "identifierUris": [url] + }, + subscription=self.get_subscription_id(), ) + set_app_audience( app.object_id, "AzureADMultipleOrgs", @@ -404,22 +425,24 @@ def try_sp_create() -> None: subscription_id=self.get_subscription_id(), ) url = "https://%s.azurewebsites.net" % self.application_name - client.applications.patch( - app.object_id, ApplicationUpdateParameters(identifier_uris=[url]) + query_microsoft_graph( + method="PATCH", + resource=f"applications/{app['id']}", + body={ + "identifierUris": [url] + }, + subscription=self.get_subscription_id(), ) else: logger.debug("No change to App Registration signInAudence setting") - creds = list(client.applications.list_password_credentials(app.object_id)) - client.applications.update_password_credentials(app.object_id, creds) (password_id, password) = self.create_password(app.object_id) - cli_app = list( - client.applications.list(filter="appId eq '%s'" % ONEFUZZ_CLI_APP) - ) - if len(cli_app) == 0: + cli_app = get_application(app_id=ONEFUZZ_CLI_APP, subscription_id=self.get_subscription_id()) + + if not cli_app: logger.info( "Could not find the default CLI application under the current " "subscription, creating a new one" @@ -440,8 +463,8 @@ def try_sp_create() -> None: } else: - onefuzz_cli_app = cli_app[0] - authorize_application(uuid.UUID(onefuzz_cli_app.app_id), app.app_id) + onefuzz_cli_app = cli_app + authorize_application(uuid.UUID(onefuzz_cli_app["appId"]), app["appId"]) if self.multi_tenant_domain: authority = COMMON_AUTHORITY else: @@ -449,18 +472,18 @@ def try_sp_create() -> None: tenant_id = get_tenant_id(self.get_subscription_id()) authority = "https://login.microsoftonline.com/%s" % tenant_id self.cli_config = { - "client_id": onefuzz_cli_app.app_id, + "client_id": onefuzz_cli_app["appId"], "authority": authority, } - self.results["client_id"] = app.app_id + self.results["client_id"] = app["appId"] self.results["client_secret"] = password # Log `client_secret` for consumption by CI. if self.log_service_principal: - logger.info("client_id: %s client_secret: %s", app.app_id, password) + logger.info("client_id: %s client_secret: %s", app["appId"], password) else: - logger.debug("client_id: %s client_secret: %s", app.app_id, password) + logger.debug("client_id: %s client_secret: %s", app["appId"], password) def deploy_template(self) -> None: logger.info("deploying arm template: %s", self.arm_template) diff --git a/src/deployment/registration.py b/src/deployment/registration.py index 1a0c5adcb9..2f71d3622a 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -17,7 +17,6 @@ from azure.cli.core.azclierror import AuthenticationError from azure.common.client_factory import get_client_from_cli_profile from azure.common.credentials import get_cli_profile -from azure.graphrbac import GraphRbacManagementClient from azure.graphrbac.models import ( Application, AppRole, @@ -120,14 +119,6 @@ def retry( raise Exception(f"failed '{description}'") -def get_graph_client(subscription_id: str) -> GraphRbacManagementClient: - client: GraphRbacManagementClient = get_client_from_cli_profile( - GraphRbacManagementClient, - subscription_id=subscription_id, - ) - return client - - class ApplicationInfo(NamedTuple): client_id: UUID client_secret: str @@ -219,7 +210,6 @@ def create_application_registration( params = { "isDeviceOnlyAuthSupported": True, - "appRoles": [{"@odata.type": "microsoft.graph.appRole"}], "displayName": name, "publicClient": { "redirectUris": ["https://%s.azurewebsites.net" % onefuzz_instance_name] @@ -244,12 +234,6 @@ def create_application_registration( ) logger.info("creating service principal") - service_principal_params = ServicePrincipalCreateParameters( - account_enabled=True, - app_role_assignment_required=False, - service_principal_type="Application", - app_id=registered_app["appId"], - ) service_principal_params = { "accountEnabled": True, @@ -289,27 +273,6 @@ def create_password() -> Tuple[str, str]: return retry(create_password, "create password") -def add_application_password_legacy( - app_object_id: UUID, subscription_id: str -) -> Tuple[str, str]: - key = str(uuid4()) - password = str(uuid4()) - - client = get_graph_client(subscription_id) - 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, subscription_id: str @@ -326,16 +289,14 @@ def add_application_password_impl( } } - try: - password: Dict = query_microsoft_graph( - method="POST", - resource="applications/%s/addPassword" % app_object_id, - body=password_request, - subscription=subscription_id, - ) - return (str(key), password["secretText"]) - except AuthenticationError: - return add_application_password_legacy(app_object_id, subscription_id) + + password: Dict = query_microsoft_graph( + method="POST", + resource="applications/%s/addPassword" % app_object_id, + body=password_request, + subscription=subscription_id, + ) + return (str(key), password["secretText"]) def get_application( @@ -446,151 +407,83 @@ def update_pool_registration(onefuzz_instance_name: str, subscription_id: str) - subscription_id, ) - -def assign_app_role_manually( +def assign_app_role( onefuzz_instance_name: str, application_name: str, subscription_id: str, app_role: OnefuzzAppRole, ) -> None: + """ + Allows the application to access the service by assigning + their managed identity to the provided App Role + """ - client = get_graph_client(subscription_id) - apps: List[Application] = list( - client.applications.list(filter="displayName eq '%s'" % onefuzz_instance_name) + onefuzz_service_appId = query_microsoft_graph( + method="GET", + resource="applications", + params={ + "$filter": "displayName eq '%s'" % onefuzz_instance_name, + "$select": "appId", + }, + subscription=subscription_id, ) - - if not apps: + if len(onefuzz_service_appId["value"]) == 0: raise Exception("onefuzz app registration not found") - - app = apps[0] - appId = app.app_id - - onefuzz_service_principals: List[ServicePrincipal] = list( - client.service_principals.list(filter="appId eq '%s'" % appId) + appId = onefuzz_service_appId["value"][0]["appId"] + onefuzz_service_principals = query_microsoft_graph( + method="GET", + resource="servicePrincipals", + params={"$filter": "appId eq '%s'" % appId}, + subscription=subscription_id, ) - if not onefuzz_service_principals: + if len(onefuzz_service_principals["value"]) == 0: raise Exception("onefuzz app service principal not found") - onefuzz_service_principal = onefuzz_service_principals[0] - - scaleset_service_principals: List[ServicePrincipal] = list( - client.service_principals.list(filter="displayName eq '%s'" % application_name) + onefuzz_service_principal = onefuzz_service_principals["value"][0] + scaleset_service_principals = query_microsoft_graph( + method="GET", + resource="servicePrincipals", + params={"$filter": "displayName eq '%s'" % application_name}, + subscription=subscription_id, ) - - if not scaleset_service_principals: + if len(scaleset_service_principals["value"]) == 0: raise Exception("scaleset service principal not found") - scaleset_service_principal = scaleset_service_principals[0] - - scaleset_service_principal.app_roles - app_roles: List[AppRole] = [ - role for role in app.app_roles if role.value == app_role.value - ] + scaleset_service_principal = scaleset_service_principals["value"][0] + managed_node_role = ( + seq(onefuzz_service_principal["appRoles"]) + .filter(lambda x: x["value"] == app_role.value) + .head_option() + ) - if not app_roles: + if not managed_node_role: raise Exception( - "ManagedNode role not found in the OneFuzz application " + f"{app_role.value} role not found in the OneFuzz application " "registration. Please redeploy the instance" ) - - body = '{ "principalId": "%s", "resourceId": "%s", "appRoleId": "%s"}' % ( - scaleset_service_principal.object_id, - onefuzz_service_principal.object_id, - app_roles[0].id, - ) - - query = ( - "az rest --method post --url " - "https://graph.microsoft.com/v1.0/servicePrincipals/%s/appRoleAssignedTo " - "--body '%s' --headers \"Content-Type\"=application/json" - % (scaleset_service_principal.object_id, body) + assignments = query_microsoft_graph( + method="GET", + resource="servicePrincipals/%s/appRoleAssignments" + % scaleset_service_principal["id"], + subscription=subscription_id, ) - logger.warning( - "execute the following query in the azure portal bash shell : \n%s" % query + # check if the role is already assigned + role_assigned = seq(assignments["value"]).find( + lambda assignment: assignment["appRoleId"] == managed_node_role["id"] ) - - -def assign_app_role( - onefuzz_instance_name: str, - application_name: str, - subscription_id: str, - app_role: OnefuzzAppRole, -) -> None: - """ - Allows the application to access the service by assigning - their managed identity to the provided App Role - """ - try: - onefuzz_service_appId = query_microsoft_graph( - method="GET", - resource="applications", - params={ - "$filter": "displayName eq '%s'" % onefuzz_instance_name, - "$select": "appId", - }, - subscription=subscription_id, - ) - 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}, - subscription=subscription_id, - ) - - 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'" % application_name}, - subscription=subscription_id, - ) - 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"] == app_role.value) - .head_option() - ) - - if not managed_node_role: - raise Exception( - f"{app_role.value} role not found in the OneFuzz application " - "registration. Please redeploy the instance" - ) - assignments = query_microsoft_graph( - method="GET", - resource="servicePrincipals/%s/appRoleAssignments" + 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"], + }, subscription=subscription_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"], - }, - subscription=subscription_id, - ) - except AuthenticationError: - assign_app_role_manually( - onefuzz_instance_name, application_name, subscription_id, app_role - ) - def set_app_audience( objectId: str, audience: str, subscription_id: Optional[str] = None From 62455631e26add0f390b726de8970d8ad0160233 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Wed, 16 Jun 2021 12:37:57 -0700 Subject: [PATCH 11/64] foramt --- src/deployment/deploy.py | 79 ++++++++++++++-------------------- src/deployment/registration.py | 3 +- 2 files changed, 33 insertions(+), 49 deletions(-) diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index 3e641df05c..505d0b9d2f 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -71,7 +71,7 @@ register_application, set_app_audience, update_pool_registration, - get_application + get_application, ) # Found by manually assigning the User.Read permission to application @@ -261,12 +261,11 @@ def setup_rbac(self) -> None: logger.info("using existing client application") return - app = get_application( - display_name=self.application_name, subscription_id=self.get_subscription_id() + display_name=self.application_name, + subscription_id=self.get_subscription_id(), ) - app_roles = [ { "allowedMemberTypes": ["Application"], @@ -274,7 +273,7 @@ def setup_rbac(self) -> None: "displayName": OnefuzzAppRole.CliClient.value, "id": str(uuid.uuid4()), "isEnabled": True, - "value": OnefuzzAppRole.CliClient.value + "value": OnefuzzAppRole.CliClient.value, }, { "allowedMemberTypes": ["Application"], @@ -283,7 +282,7 @@ def setup_rbac(self) -> None: "id": str(uuid.uuid4()), "isEnabled": True, "value": OnefuzzAppRole.ManagedNode.value, - } + }, ] # app: Optional[Application] = None @@ -303,25 +302,20 @@ def setup_rbac(self) -> None: "displayName": self.application_name, "identifierUris": [url], "appRoles": app_roles, - "requiredResourceAccess": - [ - { - "resourceAccess": { - "id": USER_READ_PERMISSION, - "type": "Scope" - }, - "resourceAppId": MICROSOFT_GRAPH_APP_ID, - } - ], - } - - - app:Dict = query_microsoft_graph( - method="POST", - resource="applications", - body=params, - subscription=self.get_subscription_id(), - ) + "requiredResourceAccess": [ + { + "resourceAccess": {"id": USER_READ_PERMISSION, "type": "Scope"}, + "resourceAppId": MICROSOFT_GRAPH_APP_ID, + } + ], + } + + app: Dict = query_microsoft_graph( + method="POST", + resource="applications", + body=params, + subscription=self.get_subscription_id(), + ) logger.info("creating service principal") @@ -376,13 +370,10 @@ def try_sp_create() -> None: for role in app["appRoles"]: role["isEnabled"] = False - query_microsoft_graph( method="PATCH", resource=f"applications/{app['id']}", - body={ - "appRoles": app["AppRoles"] - }, + body={"appRoles": app["AppRoles"]}, subscription=self.get_subscription_id(), ) @@ -390,9 +381,7 @@ def try_sp_create() -> None: query_microsoft_graph( method="PATCH", resource=f"applications/{app['id']}", - body={ - "appRoles": app_roles - }, + body={"appRoles": app_roles}, subscription=self.get_subscription_id(), ) @@ -402,12 +391,10 @@ def try_sp_create() -> None: self.application_name, ) query_microsoft_graph( - method="PATCH", - resource=f"applications/{app['id']}", - body={ - "identifierUris": [url] - }, - subscription=self.get_subscription_id(), + method="PATCH", + resource=f"applications/{app['id']}", + body={"identifierUris": [url]}, + subscription=self.get_subscription_id(), ) set_app_audience( @@ -426,21 +413,19 @@ def try_sp_create() -> None: ) url = "https://%s.azurewebsites.net" % self.application_name query_microsoft_graph( - method="PATCH", - resource=f"applications/{app['id']}", - body={ - "identifierUris": [url] - }, - subscription=self.get_subscription_id(), + method="PATCH", + resource=f"applications/{app['id']}", + body={"identifierUris": [url]}, + subscription=self.get_subscription_id(), ) else: logger.debug("No change to App Registration signInAudence setting") - (password_id, password) = self.create_password(app.object_id) - - cli_app = get_application(app_id=ONEFUZZ_CLI_APP, subscription_id=self.get_subscription_id()) + cli_app = get_application( + app_id=ONEFUZZ_CLI_APP, subscription_id=self.get_subscription_id() + ) if not cli_app: logger.info( diff --git a/src/deployment/registration.py b/src/deployment/registration.py index 2f71d3622a..8f29ab7007 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -273,7 +273,6 @@ def create_password() -> Tuple[str, str]: return retry(create_password, "create password") - def add_application_password_impl( app_object_id: UUID, subscription_id: str ) -> Tuple[str, str]: @@ -289,7 +288,6 @@ def add_application_password_impl( } } - password: Dict = query_microsoft_graph( method="POST", resource="applications/%s/addPassword" % app_object_id, @@ -407,6 +405,7 @@ def update_pool_registration(onefuzz_instance_name: str, subscription_id: str) - subscription_id, ) + def assign_app_role( onefuzz_instance_name: str, application_name: str, From 431200d3de9a64e1618ae57b163e825ba9875114 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Wed, 16 Jun 2021 12:58:38 -0700 Subject: [PATCH 12/64] mypy fix --- src/deployment/deploy.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index 505d0b9d2f..82069620a8 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -310,7 +310,7 @@ def setup_rbac(self) -> None: ], } - app: Dict = query_microsoft_graph( + app = query_microsoft_graph( method="POST", resource="applications", body=params, @@ -424,7 +424,7 @@ def try_sp_create() -> None: (password_id, password) = self.create_password(app.object_id) cli_app = get_application( - app_id=ONEFUZZ_CLI_APP, subscription_id=self.get_subscription_id() + app_id=uuid.UUID(ONEFUZZ_CLI_APP), subscription_id=self.get_subscription_id() ) if not cli_app: From 71bceef4fde1b8ec3b028aab62b107145fdec90c Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Wed, 16 Jun 2021 13:25:55 -0700 Subject: [PATCH 13/64] isort --- src/deployment/deploy.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index 82069620a8..d35b1121d0 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -58,20 +58,19 @@ generate_container_sas, ) from azure.storage.queue import QueueServiceClient -from msrest.serialization import TZ_UTC - from data_migration import migrate +from msrest.serialization import TZ_UTC from registration import ( OnefuzzAppRole, add_application_password, assign_app_role, authorize_application, + get_application, get_tenant_id, query_microsoft_graph, register_application, set_app_audience, update_pool_registration, - get_application, ) # Found by manually assigning the User.Read permission to application From 7e1b93c18e2f02c3a4eb4b557d22c82d787feb90 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Wed, 16 Jun 2021 13:36:47 -0700 Subject: [PATCH 14/64] isort --- src/deployment/deploy.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index d35b1121d0..e49471bb06 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -58,8 +58,9 @@ generate_container_sas, ) from azure.storage.queue import QueueServiceClient -from data_migration import migrate from msrest.serialization import TZ_UTC + +from data_migration import migrate from registration import ( OnefuzzAppRole, add_application_password, From 7ba55defc47c009736e98f106482b9163fe2b410 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Wed, 16 Jun 2021 13:55:35 -0700 Subject: [PATCH 15/64] format --- src/deployment/deploy.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index e49471bb06..0a2ac4a6ff 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -424,7 +424,8 @@ def try_sp_create() -> None: (password_id, password) = self.create_password(app.object_id) cli_app = get_application( - app_id=uuid.UUID(ONEFUZZ_CLI_APP), subscription_id=self.get_subscription_id() + app_id=uuid.UUID(ONEFUZZ_CLI_APP), + subscription_id=self.get_subscription_id(), ) if not cli_app: From 09107aba15c89b5a38eb3545aee81e87de7e46f3 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Wed, 16 Jun 2021 17:01:38 -0700 Subject: [PATCH 16/64] bug fixes --- src/deployment/deploy.py | 34 +++++++++++------------- src/deployment/registration.py | 48 ++++++++++++++++++++++------------ 2 files changed, 47 insertions(+), 35 deletions(-) diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index 0a2ac4a6ff..131ae80da9 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -23,17 +23,7 @@ from azure.common.credentials import get_cli_profile from azure.core.exceptions import ResourceExistsError from azure.cosmosdb.table.tableservice import TableService -from azure.graphrbac.models import ( - Application, - ApplicationCreateParameters, - ApplicationUpdateParameters, - AppRole, - GraphErrorException, - OptionalClaims, - RequiredResourceAccess, - ResourceAccess, - ServicePrincipalCreateParameters, -) +from azure.graphrbac.models import GraphErrorException from azure.mgmt.applicationinsights import ApplicationInsightsManagementClient from azure.mgmt.applicationinsights.models import ( ApplicationInsightsComponentExportRequest, @@ -249,7 +239,9 @@ def check_region(self) -> None: sys.exit(1) def create_password(self, object_id: UUID) -> Tuple[str, str]: - return add_application_password(object_id, self.get_subscription_id()) + return add_application_password( + "cli_password", object_id, self.get_subscription_id() + ) def setup_rbac(self) -> None: """ @@ -301,10 +293,14 @@ def setup_rbac(self) -> None: params = { "displayName": self.application_name, "identifierUris": [url], + "signInAudience": "AzureADMyOrg", "appRoles": app_roles, + "web": {"redirectUris": [f"{url}/.auth/login/aad/callback"]}, "requiredResourceAccess": [ { - "resourceAccess": {"id": USER_READ_PERMISSION, "type": "Scope"}, + "resourceAccess": [ + {"id": USER_READ_PERMISSION, "type": "Scope"} + ], "resourceAppId": MICROSOFT_GRAPH_APP_ID, } ], @@ -332,7 +328,7 @@ def try_sp_create() -> None: try: query_microsoft_graph( method="POST", - resource="applications/servicePrincipals", + resource="servicePrincipals", body=service_principal_params, subscription=self.get_subscription_id(), ) @@ -385,7 +381,7 @@ def try_sp_create() -> None: subscription=self.get_subscription_id(), ) - if self.multi_tenant_domain and app.sign_in_audience == "AzureADMyOrg": + if self.multi_tenant_domain and app["signInAudience"] == "AzureADMyOrg": url = "https://%s/%s" % ( self.multi_tenant_domain, self.application_name, @@ -398,16 +394,16 @@ def try_sp_create() -> None: ) set_app_audience( - app.object_id, + app["id"], "AzureADMultipleOrgs", subscription_id=self.get_subscription_id(), ) elif ( not self.multi_tenant_domain - and app.sign_in_audience == "AzureADMultipleOrgs" + and app["signInAudience"] == "AzureADMultipleOrgs" ): set_app_audience( - app.object_id, + app["id"], "AzureADMyOrg", subscription_id=self.get_subscription_id(), ) @@ -421,7 +417,7 @@ def try_sp_create() -> None: else: logger.debug("No change to App Registration signInAudence setting") - (password_id, password) = self.create_password(app.object_id) + (password_id, password) = self.create_password(app["id"]) cli_app = get_application( app_id=uuid.UUID(ONEFUZZ_CLI_APP), diff --git a/src/deployment/registration.py b/src/deployment/registration.py index 8f29ab7007..5e996ab3dc 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -9,21 +9,13 @@ import urllib.parse from datetime import datetime, timedelta from enum import Enum -from re import S, sub from typing import Any, Callable, Dict, List, NamedTuple, Optional, Tuple, TypeVar, cast from uuid import UUID, uuid4 import requests from azure.cli.core.azclierror import AuthenticationError -from azure.common.client_factory import get_client_from_cli_profile from azure.common.credentials import get_cli_profile -from azure.graphrbac.models import ( - Application, - AppRole, - PasswordCredential, - ServicePrincipal, - ServicePrincipalCreateParameters, -) +from azure.graphrbac.models import Application from functional import seq from msrest.serialization import TZ_UTC @@ -150,7 +142,7 @@ def register_application( else: logger.info( "Found existing application objectId '%s' - appid '%s'" - % (app.object_id, app["appId"]) + % (app["id"], app["appId"]) ) onefuzz_app = get_application( @@ -171,7 +163,7 @@ def register_application( tenant_id = get_tenant_id(subscription_id=subscription_id) return ApplicationInfo( - client_id=app.app_id, + client_id=app["id"], client_secret=password, authority=("https://login.microsoftonline.com/%s" % tenant_id), ) @@ -186,7 +178,9 @@ def create_application_credential(application_name: str, subscription_id: str) - if not app: raise Exception("app not found") - (_, password) = add_application_password(app["objectId"], subscription_id) + (_, password) = add_application_password( + f"{application_name}_password", app["objectId"], subscription_id + ) return str(password) @@ -244,7 +238,7 @@ def create_application_registration( query_microsoft_graph( method="POST", - resource="applications/servicePrincipals", + resource="servicePrincipals", body=service_principal_params, subscription=subscription_id, ) @@ -261,10 +255,12 @@ def create_application_registration( def add_application_password( - app_object_id: UUID, subscription_id: str + password_name: str, app_object_id: UUID, subscription_id: str ) -> Tuple[str, str]: def create_password() -> Tuple[str, str]: - password = add_application_password_impl(app_object_id, subscription_id) + password = add_application_password_impl( + password_name, app_object_id, subscription_id + ) logger.info("app password created") return password @@ -274,8 +270,28 @@ def create_password() -> Tuple[str, str]: def add_application_password_impl( - app_object_id: UUID, subscription_id: str + password_name: str, app_object_id: UUID, subscription_id: str ) -> Tuple[str, str]: + + app = query_microsoft_graph( + method="GET", + resource="applications/%s" % app_object_id, + subscription=subscription_id, + ) + + passwords = [ + x for x in app["passwordCredentials"] if x["displayName"] == password_name + ] + + if len(passwords) > 0: + key_id = passwords[0]["keyId"] + query_microsoft_graph( + method="POST", + resource="applications/%s/removePassword" % app_object_id, + body={"keyId": key_id}, + subscription=subscription_id, + ) + key = uuid4() password_request = { "passwordCredential": { From 73a135fbb7f4c02f7c35c6318d0337b61ea412a6 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Wed, 16 Jun 2021 17:12:49 -0700 Subject: [PATCH 17/64] specify the correct signInAudience --- src/deployment/deploy.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index 131ae80da9..0432c8dfb9 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -287,13 +287,15 @@ def setup_rbac(self) -> None: self.multi_tenant_domain, self.application_name, ) + signInAudience = "AzureADMultipleOrgs" else: url = "https://%s.azurewebsites.net" % self.application_name + signInAudience = "AzureADMyOrg" params = { "displayName": self.application_name, "identifierUris": [url], - "signInAudience": "AzureADMyOrg", + "signInAudience": signInAudience, "appRoles": app_roles, "web": {"redirectUris": [f"{url}/.auth/login/aad/callback"]}, "requiredResourceAccess": [ From be18ab9217636152aa5c848490bb712d861ae193 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Thu, 17 Jun 2021 11:57:37 -0700 Subject: [PATCH 18/64] fix backing service principal creation fix preauthorized application --- src/deployment/deploy.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index 0432c8dfb9..463ba77e9b 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -23,7 +23,6 @@ from azure.common.credentials import get_cli_profile from azure.core.exceptions import ResourceExistsError from azure.cosmosdb.table.tableservice import TableService -from azure.graphrbac.models import GraphErrorException from azure.mgmt.applicationinsights import ApplicationInsightsManagementClient from azure.mgmt.applicationinsights.models import ( ApplicationInsightsComponentExportRequest, @@ -52,6 +51,7 @@ from data_migration import migrate from registration import ( + GraphQueryError, OnefuzzAppRole, add_application_password, assign_app_role, @@ -297,6 +297,20 @@ def setup_rbac(self) -> None: "identifierUris": [url], "signInAudience": signInAudience, "appRoles": app_roles, + "api": { + "oauth2PermissionScopes": [ + { + "adminConsentDescription": f"Allow the application to access {self.application_name} on behalf of the signed-in user.", + "adminConsentDisplayName": f"Access {self.application_name}", + "id": str(uuid.uuid4()), + "isEnabled": True, + "type": "User", + "userConsentDescription": f"Allow the application to access {self.application_name} on your behalf.", + "userConsentDisplayName": f"Access {self.application_name}", + "value": "user_impersonation", + } + ] + }, "web": {"redirectUris": [f"{url}/.auth/login/aad/callback"]}, "requiredResourceAccess": [ { @@ -335,7 +349,7 @@ def try_sp_create() -> None: subscription=self.get_subscription_id(), ) return - except GraphErrorException as err: + except GraphQueryError as err: # work around timing issue when creating service principal # https://github.com/Azure/azure-cli/issues/14767 if ( From 295c56e1dd76e95bf6114198c0c5fe769211a20f Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Thu, 17 Jun 2021 13:29:15 -0700 Subject: [PATCH 19/64] remove remaining references to graphrbac --- src/api-service/__app__/onefuzzlib/request.py | 3 +-- src/api-service/__app__/requirements.txt | 1 - src/deployment/registration.py | 3 +-- src/deployment/requirements.txt | 1 - 4 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/api-service/__app__/onefuzzlib/request.py b/src/api-service/__app__/onefuzzlib/request.py index ebaad6d79a..e5e8d65ed5 100644 --- a/src/api-service/__app__/onefuzzlib/request.py +++ b/src/api-service/__app__/onefuzzlib/request.py @@ -10,7 +10,6 @@ from uuid import UUID from azure.functions import HttpRequest, HttpResponse -from azure.graphrbac.models import GraphErrorException from onefuzztypes.enums import ErrorCode from onefuzztypes.models import Error from onefuzztypes.responses import BaseResponse @@ -35,7 +34,7 @@ def check_access(req: HttpRequest) -> Optional[Error]: member_id = req.headers["x-ms-client-principal-id"] try: result = is_member_of(group_id, member_id) - except GraphErrorException: + except Exception: return Error( code=ErrorCode.UNAUTHORIZED, errors=["unable to interact with graph"] ) diff --git a/src/api-service/__app__/requirements.txt b/src/api-service/__app__/requirements.txt index 2a9869ebc3..76a45154e3 100644 --- a/src/api-service/__app__/requirements.txt +++ b/src/api-service/__app__/requirements.txt @@ -4,7 +4,6 @@ azure-cosmosdb-nspkg==2.0.2 azure-cosmosdb-table==1.0.6 azure-devops==6.0.0b4 azure-functions==1.5.0 -azure-graphrbac~=0.61.1 azure-identity==1.6.0 azure-keyvault-keys~=4.3.1 azure-keyvault-secrets~=4.2.0 diff --git a/src/deployment/registration.py b/src/deployment/registration.py index 5e996ab3dc..f39447e925 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -15,7 +15,6 @@ import requests from azure.cli.core.azclierror import AuthenticationError from azure.common.credentials import get_cli_profile -from azure.graphrbac.models import Application from functional import seq from msrest.serialization import TZ_UTC @@ -186,7 +185,7 @@ def create_application_credential(application_name: str, subscription_id: str) - def create_application_registration( onefuzz_instance_name: str, name: str, approle: OnefuzzAppRole, subscription_id: str -) -> Application: +) -> Any: """Create an application registration""" app = get_application( diff --git a/src/deployment/requirements.txt b/src/deployment/requirements.txt index 3779de2cff..77ea638718 100644 --- a/src/deployment/requirements.txt +++ b/src/deployment/requirements.txt @@ -1,7 +1,6 @@ azure-cli-core==2.23.0 azure-cli==2.23.0 azure-cosmosdb-table==1.0.6 -azure-graphrbac==0.60.0 azure-mgmt-eventgrid==3.0.0rc9 azure-mgmt-resource==12.1.0 azure-mgmt-storage==17.1.0 From 9c471590534a8112986c4cdd88887fc1aa7f836a Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Fri, 18 Jun 2021 11:05:04 -0700 Subject: [PATCH 20/64] fix ms graph authentication --- src/api-service/__app__/onefuzzlib/azure/creds.py | 10 ++++++---- src/api-service/__app__/onefuzzlib/request.py | 5 +++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/api-service/__app__/onefuzzlib/azure/creds.py b/src/api-service/__app__/onefuzzlib/azure/creds.py index 68a75db943..ae1dcc9fd2 100644 --- a/src/api-service/__app__/onefuzzlib/azure/creds.py +++ b/src/api-service/__app__/onefuzzlib/azure/creds.py @@ -14,7 +14,7 @@ from azure.mgmt.resource import ResourceManagementClient from azure.mgmt.subscription import SubscriptionClient from memoization import cached -from msrestazure.azure_active_directory import MSIAuthentication +from msrestazure.azure_active_directory import AZURE_PUBLIC_CLOUD, MSIAuthentication from msrestazure.tools import parse_resource_id from onefuzztypes.primitives import Container, Region @@ -22,10 +22,12 @@ @cached -def get_msi() -> MSIAuthentication: +def get_ms_graph_msi() -> MSIAuthentication: allow_more_workers() reduce_logging() - return MSIAuthentication() + return MSIAuthentication( + resource=AZURE_PUBLIC_CLOUD.endpoints.microsoft_graph_resource_id + ) @cached @@ -102,7 +104,7 @@ def query_microsoft_graph( params: Optional[Dict] = None, body: Optional[Dict] = None, ) -> Any: - auth = get_msi() + auth = get_ms_graph_msi() access_token = auth.token["access_token"] token_type = auth.token["token_type"] diff --git a/src/api-service/__app__/onefuzzlib/request.py b/src/api-service/__app__/onefuzzlib/request.py index e5e8d65ed5..34c9e6844e 100644 --- a/src/api-service/__app__/onefuzzlib/request.py +++ b/src/api-service/__app__/onefuzzlib/request.py @@ -34,9 +34,10 @@ def check_access(req: HttpRequest) -> Optional[Error]: member_id = req.headers["x-ms-client-principal-id"] try: result = is_member_of(group_id, member_id) - except Exception: + except Exception as e: return Error( - code=ErrorCode.UNAUTHORIZED, errors=["unable to interact with graph"] + code=ErrorCode.UNAUTHORIZED, + errors=["unable to interact with graph", str(e)], ) if not result: logging.error("unauthorized access: %s is not in %s", member_id, group_id) From 128c7f8201c7068139f39f8b1ee33325f8fb9d33 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Fri, 9 Jul 2021 13:53:43 -0700 Subject: [PATCH 21/64] formatting --- src/deployment/deploy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index e7f9cce06a..7a8539f253 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -121,7 +121,7 @@ def __init__( multi_tenant_domain: str, upgrade: bool, subscription_id: Optional[str], - admins: List[UUID] + admins: List[UUID], ): self.subscription_id = subscription_id self.resource_group = resource_group From c3e5b38aaf5330b47d13c86cc4c703dda29c6e76 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 20 Jul 2021 13:29:30 -0700 Subject: [PATCH 22/64] fix typo --- src/deployment/registration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/deployment/registration.py b/src/deployment/registration.py index f39447e925..db0605f18d 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -151,7 +151,7 @@ def register_application( if not (onefuzz_app): raise Exception("onefuzz app not found") - pre_authorized_applications = onefuzz_app["apiApplication"][ + pre_authorized_applications = onefuzz_app["api"][ "preAuthorizedApplications" ] From 174cd86f4fcc9831b7eb7e2a21a666545846dbca Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 20 Jul 2021 13:51:46 -0700 Subject: [PATCH 23/64] format --- src/deployment/registration.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/deployment/registration.py b/src/deployment/registration.py index db0605f18d..687a3c98d1 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -151,9 +151,7 @@ def register_application( if not (onefuzz_app): raise Exception("onefuzz app not found") - pre_authorized_applications = onefuzz_app["api"][ - "preAuthorizedApplications" - ] + pre_authorized_applications = onefuzz_app["api"]["preAuthorizedApplications"] if app["appId"] not in [app["appId"] for app in pre_authorized_applications]: authorize_application(UUID(app["appId"]), UUID(onefuzz_app["appId"])) From 088dc4db4bf4e731c20cf117ca54e1cdb43e0e70 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 20 Jul 2021 14:22:13 -0700 Subject: [PATCH 24/64] deployment fix --- src/deployment/registration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/deployment/registration.py b/src/deployment/registration.py index 687a3c98d1..b5fe948361 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -176,7 +176,7 @@ def create_application_credential(application_name: str, subscription_id: str) - raise Exception("app not found") (_, password) = add_application_password( - f"{application_name}_password", app["objectId"], subscription_id + f"{application_name}_password", app["id"], subscription_id ) return str(password) From b402038e0737f9903d684628b94ed4a29aa12077 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 20 Jul 2021 17:16:22 -0700 Subject: [PATCH 25/64] set implicitGrantSettings in the deployment --- src/deployment/deploy.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index 7a8539f253..7b0f706b7a 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -314,7 +314,13 @@ def setup_rbac(self) -> None: } ] }, - "web": {"redirectUris": [f"{url}/.auth/login/aad/callback"]}, + "web": { + "implicitGrantSettings": { + "enableAccessTokenIssuance": False, + "enableIdTokenIssuance": True + }, + "redirectUris": [f"{url}/.auth/login/aad/callback"] + }, "requiredResourceAccess": [ { "resourceAccess": [ From 9996678273af833fed7c6f950e97b0882848d695 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 20 Jul 2021 18:11:01 -0700 Subject: [PATCH 26/64] format --- src/deployment/deploy.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index 7b0f706b7a..1481d61b43 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -315,12 +315,12 @@ def setup_rbac(self) -> None: ] }, "web": { - "implicitGrantSettings": { - "enableAccessTokenIssuance": False, - "enableIdTokenIssuance": True - }, - "redirectUris": [f"{url}/.auth/login/aad/callback"] + "implicitGrantSettings": { + "enableAccessTokenIssuance": False, + "enableIdTokenIssuance": True, }, + "redirectUris": [f"{url}/.auth/login/aad/callback"], + }, "requiredResourceAccess": [ { "resourceAccess": [ From 998829d1c2f8b65627a1cdda62bc76051248230f Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 27 Jul 2021 11:41:02 -0700 Subject: [PATCH 27/64] fix deployment --- src/deployment/registration.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/deployment/registration.py b/src/deployment/registration.py index b5fe948361..ba4cef2c19 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -160,7 +160,7 @@ def register_application( tenant_id = get_tenant_id(subscription_id=subscription_id) return ApplicationInfo( - client_id=app["id"], + client_id=app["appId"], client_secret=password, authority=("https://login.microsoftonline.com/%s" % tenant_id), ) @@ -194,7 +194,7 @@ def create_application_registration( raise Exception("onefuzz app registration not found") resource_access = [ - {"id": "guid", "type": "string"} + {"id": role["id"], "type": "Scope"} for role in app["appRoles"] if role["value"] == approle.value ] @@ -205,6 +205,7 @@ def create_application_registration( "publicClient": { "redirectUris": ["https://%s.azurewebsites.net" % onefuzz_instance_name] }, + "isFallbackPublicClient": True, "requiredResourceAccess": ( [ { From 63cde940adf34888f62c6d09af2effa35ae15b41 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 27 Jul 2021 16:07:17 -0700 Subject: [PATCH 28/64] fix graph authentication on the server --- src/api-service/__app__/onefuzzlib/azure/creds.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/api-service/__app__/onefuzzlib/azure/creds.py b/src/api-service/__app__/onefuzzlib/azure/creds.py index ae1dcc9fd2..d64d418039 100644 --- a/src/api-service/__app__/onefuzzlib/azure/creds.py +++ b/src/api-service/__app__/onefuzzlib/azure/creds.py @@ -104,13 +104,15 @@ def query_microsoft_graph( params: Optional[Dict] = None, body: Optional[Dict] = None, ) -> Any: - auth = get_ms_graph_msi() - access_token = auth.token["access_token"] - token_type = auth.token["token_type"] + cred = get_identity() + access_token = cred.get_token("https://graph.microsoft.com/.default") + # auth = get_ms_graph_msi() + # access_token = auth.token["access_token"] + token_type = "Bearer" url = urllib.parse.urljoin("https://graph.microsoft.com/v1.0/", resource) headers = { - "Authorization": "%s %s" % (token_type, access_token), + "Authorization": "%s %s" % (token_type , access_token.token ), "Content-Type": "application/json", } response = requests.request( From 93ee8c9fb785ed146f3ab8eb17f666d14ba30a26 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Thu, 29 Jul 2021 11:21:55 -0700 Subject: [PATCH 29/64] use the current cli logged in account to retrive the backend token cache --- src/cli/onefuzz/backend.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/cli/onefuzz/backend.py b/src/cli/onefuzz/backend.py index 77d33bf7e8..dda5db76ea 100644 --- a/src/cli/onefuzz/backend.py +++ b/src/cli/onefuzz/backend.py @@ -26,6 +26,7 @@ ) from urllib.parse import urlparse, urlunparse from uuid import UUID +from azure.common.credentials import get_cli_profile import msal import requests @@ -185,7 +186,13 @@ def device_login(self, scopes: List[str]) -> Any: token_cache=self.token_cache, ) - accounts = self.app.get_accounts() + profile = get_cli_profile() + current_user = profile.get_current_account_user() + if current_user: + accounts = self.app.get_accounts(current_user) + else: + accounts = self.app.get_accounts() + if accounts: access_token = self.app.acquire_token_silent(scopes, account=accounts[0]) if access_token: From e9372860c0d0e179aa40bdc43318942dee06e6ec Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Thu, 29 Jul 2021 12:10:42 -0700 Subject: [PATCH 30/64] assign the the msgraph app role permissions to the web app during the deployment --- src/deployment/azuredeploy.json | 5 +++ src/deployment/deploy.py | 17 +++++++++- src/deployment/registration.py | 57 +++++++++++++++++++++++++++++++-- 3 files changed, 76 insertions(+), 3 deletions(-) diff --git a/src/deployment/azuredeploy.json b/src/deployment/azuredeploy.json index 127fd3383d..69189cc9d1 100644 --- a/src/deployment/azuredeploy.json +++ b/src/deployment/azuredeploy.json @@ -821,6 +821,11 @@ "scaleset-identity": { "type": "string", "value": "[variables('scaleset_identity')]" + }, + "webapp-identity" : { + "type": "string", + + "value": "[reference(resourceId('Microsoft.Web/sites', parameters('name')), '2018-02-01', 'Full').identity.principalId]" } } } diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index 1481d61b43..6c1999985a 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -54,6 +54,7 @@ GraphQueryError, OnefuzzAppRole, add_application_password, + assign_instance_app_role, assign_app_role, authorize_application, get_application, @@ -580,13 +581,26 @@ def assign_scaleset_identity_role(self) -> None: logger.info("Upgrading: skipping assignment of the managed identity role") return logger.info("assigning the user managed identity role") - assign_app_role( + assign_instance_app_role( self.application_name, self.results["deploy"]["scaleset-identity"]["value"], self.get_subscription_id(), OnefuzzAppRole.ManagedNode, ) + def assign_app_permissions(self) -> None: + if self.upgrade: + logger.info("Upgrading: skipping assignment of msgraph permissions") + return + logger.info("assigning msgraph permissions") + + assign_app_role( + principal_id=self.results["deploy"]["webapp-identity"]["value"], + application_id=MICROSOFT_GRAPH_APP_ID, + role_names=["GroupMember.Read.All"], + subscription_id=self.get_subscription_id(), + ) + def apply_migrations(self) -> None: name = self.results["deploy"]["func-name"]["value"] key = self.results["deploy"]["func-key"]["value"] @@ -954,6 +968,7 @@ def main() -> None: ("rbac", Client.setup_rbac), ("arm", Client.deploy_template), ("assign_scaleset_identity_role", Client.assign_scaleset_identity_role), + ("assign_app_permissions", Client.assign_app_permissions), ] full_deployment_states = rbac_only_states + [ diff --git a/src/deployment/registration.py b/src/deployment/registration.py index ba4cef2c19..26a1291f4f 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -246,7 +246,7 @@ def create_application_registration( UUID(app["appId"]), subscription_id=subscription_id, ) - assign_app_role( + assign_instance_app_role( onefuzz_instance_name, name, subscription_id, OnefuzzAppRole.ManagedNode ) return registered_app @@ -421,6 +421,59 @@ def update_pool_registration(onefuzz_instance_name: str, subscription_id: str) - def assign_app_role( + principal_id: str, + application_id: str, + role_names: List[str], + subscription_id: str, +): + application_registration = query_microsoft_graph( + method="GET", + resource="applications", + params={ + "$filter": f"displayName eq '{application_id}'", + "$select": "appId", + }, + subscription=subscription_id, + ) + + roles = seq(application_registration["appRoles"]).find( + lambda role: role["value"] in role_names + ) + if len(roles) < len(role_names): + existing_roles = [role["value"] for role in roles] + missing_roles = [ + role_name for role_name in role_names if role_name not in existing_roles + ] + raise Exception( + f"The following roles could not be found in appId '{application_id}': {missing_roles}" + ) + + expected_role_ids = [role["id"] for role in roles] + assignments = query_microsoft_graph( + method="GET", + resource=f"servicePrincipals/{principal_id}/appRoleAssignments", + subscription=subscription_id, + ) + assigned_role_ids = [assignment["appRoleId"] for assignment in assignments["value"]] + missing_assignments = [ + id for id in expected_role_ids if id not in assigned_role_ids + ] + + if missing_assignments: + for app_role_id in missing_assignments: + query_microsoft_graph( + method="POST", + resource=f"servicePrincipals/{principal_id}/appRoleAssignedTo", + body={ + "principalId": principal_id, + "resourceId": application_registration["id"], + "appRoleId": app_role_id, + }, + subscription=subscription_id, + ) + + +def assign_instance_app_role( onefuzz_instance_name: str, application_name: str, subscription_id: str, @@ -589,7 +642,7 @@ def main() -> None: args.subscription_id, ) elif args.command == "assign_scaleset_role": - assign_app_role( + assign_instance_app_role( onefuzz_instance_name, args.scaleset_name, args.subscription_id, From 642c37b605958ab05ff760c723a11235e7903bcb Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Thu, 29 Jul 2021 12:12:39 -0700 Subject: [PATCH 31/64] formatting --- src/cli/onefuzz/backend.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cli/onefuzz/backend.py b/src/cli/onefuzz/backend.py index dda5db76ea..a1cd1ed92e 100644 --- a/src/cli/onefuzz/backend.py +++ b/src/cli/onefuzz/backend.py @@ -26,10 +26,10 @@ ) from urllib.parse import urlparse, urlunparse from uuid import UUID -from azure.common.credentials import get_cli_profile import msal import requests +from azure.common.credentials import get_cli_profile from azure.storage.blob import ContainerClient from pydantic import BaseModel, Field from tenacity import RetryCallState, retry From 398af21ad554eacf8fdc4d1c5ab350e3f278d133 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Thu, 29 Jul 2021 12:38:27 -0700 Subject: [PATCH 32/64] fix build --- src/api-service/__app__/onefuzzlib/azure/creds.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/api-service/__app__/onefuzzlib/azure/creds.py b/src/api-service/__app__/onefuzzlib/azure/creds.py index 7806ab22a4..4b892b2187 100644 --- a/src/api-service/__app__/onefuzzlib/azure/creds.py +++ b/src/api-service/__app__/onefuzzlib/azure/creds.py @@ -25,12 +25,10 @@ @cached -def get_ms_graph_msi() -> MSIAuthentication: +def get_msi() -> MSIAuthentication: allow_more_workers() reduce_logging() - return MSIAuthentication( - resource=AZURE_PUBLIC_CLOUD.endpoints.microsoft_graph_resource_id - ) + return MSIAuthentication() @cached @@ -109,8 +107,6 @@ def query_microsoft_graph( ) -> Any: cred = get_identity() access_token = cred.get_token("https://graph.microsoft.com/.default") - # auth = get_ms_graph_msi() - # access_token = auth.token["access_token"] token_type = "Bearer" url = urllib.parse.urljoin("https://graph.microsoft.com/v1.0/", resource) From b58a04d1c341c222c03cafd62d6760aad9509c8f Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Thu, 29 Jul 2021 12:51:51 -0700 Subject: [PATCH 33/64] build fix --- src/api-service/__app__/onefuzzlib/azure/creds.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api-service/__app__/onefuzzlib/azure/creds.py b/src/api-service/__app__/onefuzzlib/azure/creds.py index 4b892b2187..791022efd0 100644 --- a/src/api-service/__app__/onefuzzlib/azure/creds.py +++ b/src/api-service/__app__/onefuzzlib/azure/creds.py @@ -17,7 +17,7 @@ from azure.mgmt.resource import ResourceManagementClient from azure.mgmt.subscription import SubscriptionClient from memoization import cached -from msrestazure.azure_active_directory import AZURE_PUBLIC_CLOUD, MSIAuthentication +from msrestazure.azure_active_directory import MSIAuthentication from msrestazure.tools import parse_resource_id from onefuzztypes.primitives import Container, Region From c4902392df6c8fd172d12d6c64e241d6d3f5af12 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Thu, 29 Jul 2021 13:08:30 -0700 Subject: [PATCH 34/64] fix bandit issue --- src/api-service/__app__/onefuzzlib/azure/creds.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/api-service/__app__/onefuzzlib/azure/creds.py b/src/api-service/__app__/onefuzzlib/azure/creds.py index 791022efd0..1d758b1b85 100644 --- a/src/api-service/__app__/onefuzzlib/azure/creds.py +++ b/src/api-service/__app__/onefuzzlib/azure/creds.py @@ -107,11 +107,10 @@ def query_microsoft_graph( ) -> Any: cred = get_identity() access_token = cred.get_token("https://graph.microsoft.com/.default") - token_type = "Bearer" url = urllib.parse.urljoin("https://graph.microsoft.com/v1.0/", resource) headers = { - "Authorization": "%s %s" % (token_type, access_token.token), + "Authorization": "Bearer %s" % access_token.token, "Content-Type": "application/json", } response = requests.request( From 79b050d2e2629f151d06007a57eb3ee2ec6576b9 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Thu, 29 Jul 2021 13:27:05 -0700 Subject: [PATCH 35/64] mypy fix --- src/deployment/registration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/deployment/registration.py b/src/deployment/registration.py index 26a1291f4f..7d68782666 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -425,7 +425,7 @@ def assign_app_role( application_id: str, role_names: List[str], subscription_id: str, -): +) -> None: application_registration = query_microsoft_graph( method="GET", resource="applications", From 92ad8e9649cd1e9677167073e57a482ecca63263 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Thu, 29 Jul 2021 14:06:32 -0700 Subject: [PATCH 36/64] isort --- src/deployment/deploy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index 8f59d69683..ee012e2378 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -54,8 +54,8 @@ GraphQueryError, OnefuzzAppRole, add_application_password, - assign_instance_app_role, assign_app_role, + assign_instance_app_role, authorize_application, get_application, get_tenant_id, From 4f9b98f684b1a6a66a6705d9ea77a20ad1d2ee34 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Thu, 29 Jul 2021 17:47:33 -0700 Subject: [PATCH 37/64] deploy fixes --- src/deployment/deploy.py | 2 +- src/deployment/registration.py | 22 ++++++++++++---------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index ee012e2378..9bfa307ebf 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -597,7 +597,7 @@ def assign_app_permissions(self) -> None: assign_app_role( principal_id=self.results["deploy"]["webapp-identity"]["value"], application_id=MICROSOFT_GRAPH_APP_ID, - role_names=["GroupMember.Read.All"], + role_names=["GroupMember.Read.All", "User.Read.All"], subscription_id=self.get_subscription_id(), ) diff --git a/src/deployment/registration.py b/src/deployment/registration.py index 7d68782666..43b6bc36d4 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -247,7 +247,7 @@ def create_application_registration( subscription_id=subscription_id, ) assign_instance_app_role( - onefuzz_instance_name, name, subscription_id, OnefuzzAppRole.ManagedNode + onefuzz_instance_name, name, subscription_id, approle ) return registered_app @@ -290,10 +290,9 @@ def add_application_password_impl( subscription=subscription_id, ) - key = uuid4() password_request = { "passwordCredential": { - "displayName": "%s" % key, + "displayName": "%s" % password_name, "startDateTime": "%s" % datetime.now(TZ_UTC).strftime("%Y-%m-%dT%H:%M.%fZ"), "endDateTime": "%s" % (datetime.now(TZ_UTC) + timedelta(days=365)).strftime( @@ -308,7 +307,7 @@ def add_application_password_impl( body=password_request, subscription=subscription_id, ) - return (str(key), password["secretText"]) + return (password_name, password["secretText"]) def get_application( @@ -428,17 +427,20 @@ def assign_app_role( ) -> None: application_registration = query_microsoft_graph( method="GET", - resource="applications", + resource="servicePrincipals", params={ - "$filter": f"displayName eq '{application_id}'", - "$select": "appId", + "$filter": f"appId eq '{application_id}'", }, subscription=subscription_id, ) + if len(application_registration["value"]) == 0: + raise Exception(f"appid '{application_id}' was not found:") + app = application_registration["value"][0] - roles = seq(application_registration["appRoles"]).find( - lambda role: role["value"] in role_names + roles = ( + seq(app["appRoles"]).filter(lambda role: role["value"] in role_names).to_list() ) + if len(roles) < len(role_names): existing_roles = [role["value"] for role in roles] missing_roles = [ @@ -466,7 +468,7 @@ def assign_app_role( resource=f"servicePrincipals/{principal_id}/appRoleAssignedTo", body={ "principalId": principal_id, - "resourceId": application_registration["id"], + "resourceId": app["id"], "appRoleId": app_role_id, }, subscription=subscription_id, From 66d3bed747bd84322e0c4d97e1c52e87ac24de43 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Fri, 30 Jul 2021 13:00:28 -0700 Subject: [PATCH 38/64] formatting --- src/deployment/registration.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/deployment/registration.py b/src/deployment/registration.py index 43b6bc36d4..a95e9f66cb 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -246,9 +246,7 @@ def create_application_registration( UUID(app["appId"]), subscription_id=subscription_id, ) - assign_instance_app_role( - onefuzz_instance_name, name, subscription_id, approle - ) + assign_instance_app_role(onefuzz_instance_name, name, subscription_id, approle) return registered_app From 7ca5d9b65e75e2cb30696947601ce4f151bef02d Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Fri, 6 Aug 2021 08:22:31 -0700 Subject: [PATCH 39/64] remove assign_app_permissions --- src/deployment/deploy.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index 9bfa307ebf..ad571d7140 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -969,7 +969,6 @@ def main() -> None: ("rbac", Client.setup_rbac), ("arm", Client.deploy_template), ("assign_scaleset_identity_role", Client.assign_scaleset_identity_role), - ("assign_app_permissions", Client.assign_app_permissions), ] full_deployment_states = rbac_only_states + [ From e4adb979fa6494f9f7817cea3e0e9a05ea50e38e Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Mon, 30 Aug 2021 10:03:28 -0700 Subject: [PATCH 40/64] mypy fix --- src/deployment/registration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/deployment/registration.py b/src/deployment/registration.py index 7916dc7099..11098622e0 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -259,7 +259,7 @@ def create_application_registration( def add_application_password( password_name: str, app_object_id: UUID, subscription_id: str ) -> Tuple[str, str]: - def create_password() -> Tuple[str, str]: + def create_password(data: Any) -> Tuple[str, str]: password = add_application_password_impl( password_name, app_object_id, subscription_id ) From 9622d3a04022c7ec17d9244879ee739f9c142e80 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Mon, 27 Sep 2021 12:11:24 -0700 Subject: [PATCH 41/64] build fix --- src/cli/onefuzz/backend.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/cli/onefuzz/backend.py b/src/cli/onefuzz/backend.py index fa13a46426..c413e62ce3 100644 --- a/src/cli/onefuzz/backend.py +++ b/src/cli/onefuzz/backend.py @@ -29,7 +29,6 @@ import msal import requests -from azure.common.credentials import get_cli_profile from azure.storage.blob import ContainerClient from pydantic import BaseModel, Field from tenacity import RetryCallState, retry From 3582046c67100df653320c9a72b5029b9461b888 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Mon, 27 Sep 2021 12:28:35 -0700 Subject: [PATCH 42/64] mypy fix --- src/deployment/deploy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index d3664ca191..d2760d25f7 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -317,7 +317,7 @@ def setup_rbac(self) -> None: params = { "displayName": self.application_name, "identifierUris": [self.get_identifier_url()], - "signInAudience": signInAudience, + "signInAudience": self.get_signin_audience(), "appRoles": app_roles, "api": { "oauth2PermissionScopes": [ From 9825cae929f42e566d8874c04a65b66344d224e5 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Mon, 27 Sep 2021 13:01:22 -0700 Subject: [PATCH 43/64] format --- src/deployment/deploy.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index d2760d25f7..fc24c318cc 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -45,9 +45,8 @@ ContainerSasPermissions, generate_container_sas, ) -from msrest.serialization import TZ_UTC - from data_migration import migrate +from msrest.serialization import TZ_UTC from registration import ( GraphQueryError, OnefuzzAppRole, @@ -449,7 +448,9 @@ def try_sp_create() -> None: and app["signInAudience"] == "AzureADMultipleOrgs" ): set_app_audience( - app.object_id, "AzureADMyOrg", subscription=self.get_subscription_id() + app.object_id, + "AzureADMyOrg", + subscription_id=self.get_subscription_id(), ) else: logger.debug("No change to App Registration signInAudence setting") From a4a5123de5b13fd8e56ba94119af08495b035aa5 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Mon, 27 Sep 2021 13:18:52 -0700 Subject: [PATCH 44/64] formatting --- src/deployment/deploy.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index fc24c318cc..e95491fd83 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -45,8 +45,9 @@ ContainerSasPermissions, generate_container_sas, ) -from data_migration import migrate from msrest.serialization import TZ_UTC + +from data_migration import migrate from registration import ( GraphQueryError, OnefuzzAppRole, From 6e0f8cae2aa23018970d7c9988305bcfd0f8aeef Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 28 Sep 2021 10:15:17 -0700 Subject: [PATCH 45/64] flake fix --- src/deployment/registration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/deployment/registration.py b/src/deployment/registration.py index 11098622e0..0c7c79e7e7 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -11,7 +11,7 @@ from datetime import datetime, timedelta from enum import Enum from typing import Any, Callable, Dict, List, NamedTuple, Optional, Tuple, TypeVar, cast -from uuid import UUID, uuid4 +from uuid import UUID import requests from azure.cli.core.azclierror import AuthenticationError From 1db50925bb880e3f09c704f87a67c4f6ebe5d163 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 28 Sep 2021 13:17:53 -0700 Subject: [PATCH 46/64] remove webapp identity permission assignment --- src/deployment/azuredeploy.json | 5 ----- src/deployment/deploy.py | 13 ------------- 2 files changed, 18 deletions(-) diff --git a/src/deployment/azuredeploy.json b/src/deployment/azuredeploy.json index 81d5b52e1e..a46f2752fc 100644 --- a/src/deployment/azuredeploy.json +++ b/src/deployment/azuredeploy.json @@ -883,11 +883,6 @@ "type": "string", "value": "[variables('scaleset_identity')]" }, - "webapp-identity" : { - "type": "string", - - "value": "[reference(resourceId('Microsoft.Web/sites', parameters('name')), '2018-02-01', 'Full').identity.principalId]" - }, "tenant_id": { "type": "string", "value": "[subscription().tenantId]" diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index e95491fd83..7af92e906a 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -594,19 +594,6 @@ def assign_scaleset_identity_role(self) -> None: OnefuzzAppRole.ManagedNode, ) - def assign_app_permissions(self) -> None: - if self.upgrade: - logger.info("Upgrading: skipping assignment of msgraph permissions") - return - logger.info("assigning msgraph permissions") - - assign_app_role( - principal_id=self.results["deploy"]["webapp-identity"]["value"], - application_id=MICROSOFT_GRAPH_APP_ID, - role_names=["GroupMember.Read.All", "User.Read.All"], - subscription_id=self.get_subscription_id(), - ) - def apply_migrations(self) -> None: name = self.results["deploy"]["func-name"]["value"] key = self.results["deploy"]["func-key"]["value"] From 3550bfc256d6b64f4d47b1d3955116f83f33d25d Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 28 Sep 2021 13:36:46 -0700 Subject: [PATCH 47/64] remove unused reference to assign_app_role --- src/deployment/deploy.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index 7af92e906a..96b9aed2d4 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -52,7 +52,6 @@ GraphQueryError, OnefuzzAppRole, add_application_password, - assign_app_role, assign_instance_app_role, authorize_application, get_application, From 7e4a104e4bd4151376c65ac13a45da269366c46b Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 5 Oct 2021 12:39:23 -0700 Subject: [PATCH 48/64] remove manual registration message --- src/deployment/registration.py | 120 +++++++++++++++------------------ 1 file changed, 56 insertions(+), 64 deletions(-) diff --git a/src/deployment/registration.py b/src/deployment/registration.py index 0c7c79e7e7..42ceee1843 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -14,16 +14,10 @@ from uuid import UUID import requests -from azure.cli.core.azclierror import AuthenticationError from azure.common.credentials import get_cli_profile 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") @@ -347,71 +341,69 @@ def authorize_application( permissions: List[str] = ["user_impersonation"], subscription_id: Optional[str] = None, ) -> None: - try: - onefuzz_app = get_application(app_id=onefuzz_app_id) - if onefuzz_app is None: - logger.error("Application '%s' not found", onefuzz_app_id) - return + onefuzz_app = get_application( + app_id=onefuzz_app_id, subscription_id=subscription_id + ) + if onefuzz_app is None: + logger.error("Application '%s' not found", onefuzz_app_id) + return - scopes = seq(onefuzz_app["api"]["oauth2PermissionScopes"]).filter( - lambda scope: scope["value"] in permissions - ) + scopes = seq(onefuzz_app["api"]["oauth2PermissionScopes"]).filter( + 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) - ) + existing_preAuthorizedApplications = ( + seq(onefuzz_app["api"]["preAuthorizedApplications"]) + .map( + lambda paa: seq(paa["delegatedPermissionIds"]).map( + lambda permission_id: (paa["appId"], permission_id) ) - .flatten() ) + .flatten() + ) - preAuthorizedApplications = ( - scopes.map(lambda s: (str(registration_app_id), s["id"])) - .union(existing_preAuthorizedApplications) - .distinct() - .group_by_key() - .map(lambda data: {"appId": data[0], "delegatedPermissionIds": data[1]}) - ) + preAuthorizedApplications = ( + scopes.map(lambda s: (str(registration_app_id), s["id"])) + .union(existing_preAuthorizedApplications) + .distinct() + .group_by_key() + .map(lambda data: {"appId": data[0], "delegatedPermissionIds": data[1]}) + ) - onefuzz_app_id = onefuzz_app["id"] - - def add_preauthorized_app(app_list: List[Dict]) -> None: - try: - query_microsoft_graph( - method="PATCH", - resource="applications/%s" % onefuzz_app_id, - body={"api": {"preAuthorizedApplications": app_list}}, - ) - except GraphQueryError as e: - m = re.search( - "Property PreAuthorizedApplication references " - "applications (.*) that cannot be found.", - e.message, - ) - if m: - invalid_app_id = m.group(1) - if invalid_app_id: - for app in app_list: - if app["appId"] == invalid_app_id: - logger.warning( - f"removing invalid id {invalid_app_id} " - "for the next request" - ) - app_list.remove(app) - - raise e - - retry( - add_preauthorized_app, - "authorize application", - data=preAuthorizedApplications.to_list(), - ) + onefuzz_app_id = onefuzz_app["id"] - except AuthenticationError: - logger.warning("*** Browse to: %s", FIX_URL % onefuzz_app_id) - logger.warning("*** Then add the client application %s", registration_app_id) + def add_preauthorized_app(app_list: List[Dict]) -> None: + try: + query_microsoft_graph( + method="PATCH", + resource="applications/%s" % onefuzz_app_id, + body={"api": {"preAuthorizedApplications": app_list}}, + subscription=subscription_id, + ) + except GraphQueryError as e: + m = re.search( + "Property PreAuthorizedApplication references " + "applications (.*) that cannot be found.", + e.message, + ) + if m: + invalid_app_id = m.group(1) + if invalid_app_id: + for app in app_list: + if app["appId"] == invalid_app_id: + logger.warning( + f"removing invalid id {invalid_app_id} " + "for the next request" + ) + app_list.remove(app) + + raise e + + retry( + add_preauthorized_app, + "authorize application", + data=preAuthorizedApplications.to_list(), + ) def create_and_display_registration( From 8a4944247de2ca49e221b82f7feacb3a971ae62a Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 5 Oct 2021 16:40:36 -0700 Subject: [PATCH 49/64] fixing name and logging --- src/deployment/registration.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/deployment/registration.py b/src/deployment/registration.py index 42ceee1843..02973aa140 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -526,15 +526,15 @@ def assign_instance_app_role( 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( + application_service_principals = query_microsoft_graph( method="GET", resource="servicePrincipals", params={"$filter": "displayName eq '%s'" % application_name}, subscription=subscription_id, ) - if len(scaleset_service_principals["value"]) == 0: - raise Exception("scaleset service principal not found") - scaleset_service_principal = scaleset_service_principals["value"][0] + if len(application_service_principals["value"]) == 0: + raise Exception(f"application '{application_name}' service principal not found") + application_service_principal = application_service_principals["value"][0] managed_node_role = ( seq(onefuzz_service_principal["appRoles"]) .filter(lambda x: x["value"] == app_role.value) @@ -549,7 +549,7 @@ def assign_instance_app_role( assignments = query_microsoft_graph( method="GET", resource="servicePrincipals/%s/appRoleAssignments" - % scaleset_service_principal["id"], + % application_service_principal["id"], subscription=subscription_id, ) @@ -561,9 +561,9 @@ def assign_instance_app_role( query_microsoft_graph( method="POST", resource="servicePrincipals/%s/appRoleAssignedTo" - % scaleset_service_principal["id"], + % application_service_principal["id"], body={ - "principalId": scaleset_service_principal["id"], + "principalId": application_service_principal["id"], "resourceId": onefuzz_service_principal["id"], "appRoleId": managed_node_role["id"], }, From ceee50eed5859b72e3f37e8e64e4f54afb646850 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Mon, 11 Oct 2021 10:56:41 -0700 Subject: [PATCH 50/64] address PR coments --- src/deployment/registration.py | 56 +++++++++++++++++++++++----------- 1 file changed, 39 insertions(+), 17 deletions(-) diff --git a/src/deployment/registration.py b/src/deployment/registration.py index 02973aa140..fddc67a9d6 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -20,6 +20,9 @@ logger = logging.getLogger("deploy") +GRAPH_RESOURCE = "https://graph.microsoft.com" +GRAPH_RESOURCE_ENDPOINT = "https://graph.microsoft.com/v0.1" + class GraphQueryError(Exception): def __init__(self, message: str, status_code: int) -> None: @@ -27,7 +30,7 @@ def __init__(self, message: str, status_code: int) -> None: self.message = message self.status_code = status_code - +## Queries microsoft graph api and return def query_microsoft_graph( method: str, resource: str, @@ -37,9 +40,9 @@ def query_microsoft_graph( ) -> Any: profile = get_cli_profile() (token_type, access_token, _), _, _ = profile.get_raw_token( - resource="https://graph.microsoft.com", subscription=subscription + resource=GRAPH_RESOURCE, subscription=subscription ) - url = urllib.parse.urljoin("https://graph.microsoft.com/v1.0/", resource) + url = urllib.parse.urljoin(f"{GRAPH_RESOURCE_ENDPOINT}/", resource) headers = { "Authorization": "%s %s" % (token_type, access_token), "Content-Type": "application/json", @@ -47,28 +50,44 @@ def query_microsoft_graph( response = requests.request( method=method, url=url, headers=headers, params=params, json=body ) - - response.status_code - if 200 <= response.status_code < 300: - try: + if response.content and response.content.strip(): return response.json() - except ValueError: + else: return None else: error_text = str(response.content, encoding="utf-8", errors="backslashreplace") raise GraphQueryError( - "request did not succeed: HTTP %s - %s" - % (response.status_code, error_text), + f"request did not succeed: HTTP {response.status_code} - {error_text}", response.status_code, ) -def get_tenant_id(subscription_id: Optional[str] = None) -> str: +def query_microsoft_graph_list( + method: str, + resource: str, + params: Optional[Dict] = None, + body: Optional[Dict] = None, + subscription: Optional[str] = None, +) -> List[Dict]: result = query_microsoft_graph( + method, + resource, + params, + body, + subscription, + ) + if result and result["value"]: + return cast(List[Dict], result["value"]) + else: + raise Exception("Expected data containing a list of values") + + +def get_tenant_id(subscription_id: Optional[str] = None) -> str: + result = query_microsoft_graph_list( method="GET", resource="organization", subscription=subscription_id ) - return cast(str, result["value"][0]["id"]) + return cast(str, result["id"]) OperationResult = TypeVar("OperationResult") @@ -218,7 +237,7 @@ def create_application_registration( ), } - registered_app: Dict = query_microsoft_graph( + registered_app = query_microsoft_graph( method="POST", resource="applications", body=params, @@ -321,7 +340,7 @@ def get_application( filter_str = " and ".join(filters) - apps: Dict = query_microsoft_graph( + apps = query_microsoft_graph( method="GET", resource="applications", params={ @@ -329,10 +348,13 @@ def get_application( }, subscription=subscription_id, ) - if len(apps["value"]) == 0: + number_of_apps = len(apps["value"]) + if number_of_apps == 0: return None - - return apps["value"][0] + elif number_of_apps == 1: + return apps["value"][0] + else: + raise Exception(f"Found {number_of_apps} application matching filter: '{filter_str}'") def authorize_application( From 07f6586c2568e80e78d39c9bec7beed908ca4f8d Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Mon, 11 Oct 2021 11:12:02 -0700 Subject: [PATCH 51/64] address PR comments --- .../__app__/onefuzzlib/azure/creds.py | 54 +++++++++++---- src/api-service/__app__/onefuzzlib/request.py | 6 +- src/deployment/deploy.py | 1 + src/deployment/registration.py | 69 ++++++++++--------- 4 files changed, 83 insertions(+), 47 deletions(-) diff --git a/src/api-service/__app__/onefuzzlib/azure/creds.py b/src/api-service/__app__/onefuzzlib/azure/creds.py index 1d758b1b85..4f863656f8 100644 --- a/src/api-service/__app__/onefuzzlib/azure/creds.py +++ b/src/api-service/__app__/onefuzzlib/azure/creds.py @@ -23,6 +23,10 @@ from .monkeypatch import allow_more_workers, reduce_logging +## https://docs.microsoft.com/en-us/graph/api/overview?view=graph-rest-1.0 +GRAPH_RESOURCE = "https://graph.microsoft.com" +GRAPH_RESOURCE_ENDPOINT = "https://graph.microsoft.com/v1.0" + @cached def get_msi() -> MSIAuthentication: @@ -99,16 +103,23 @@ def get_regions() -> List[Region]: return sorted([Region(x.name) for x in locations]) +class GraphQueryError(Exception): + def __init__(self, message: str, status_code: Optional[int]) -> None: + super(GraphQueryError, self).__init__(message) + self.message = message + self.status_code = status_code + + def query_microsoft_graph( method: str, resource: str, params: Optional[Dict] = None, body: Optional[Dict] = None, -) -> Any: +) -> Dict: cred = get_identity() - access_token = cred.get_token("https://graph.microsoft.com/.default") + access_token = cred.get_token(f"{GRAPH_RESOURCE}/.default") - url = urllib.parse.urljoin("https://graph.microsoft.com/v1.0/", resource) + url = urllib.parse.urljoin(f"{GRAPH_RESOURCE_ENDPOINT}/", resource) headers = { "Authorization": "Bearer %s" % access_token.token, "Content-Type": "application/json", @@ -117,28 +128,45 @@ def query_microsoft_graph( method=method, url=url, headers=headers, params=params, json=body ) - response.status_code - if 200 <= response.status_code < 300: - try: + if response.content and response.content.strip(): return response.json() - except ValueError: - return None + else: + return {} else: error_text = str(response.content, encoding="utf-8", errors="backslashreplace") - raise Exception( - "request did not succeed: HTTP %s - %s" - % (response.status_code, error_text), + raise GraphQueryError( + f"request did not succeed: HTTP {response.status_code} - {error_text}", response.status_code, ) +def query_microsoft_graph_list( + method: str, + resource: str, + params: Optional[Dict] = None, + body: Optional[Dict] = None, + subscription: Optional[str] = None, +) -> List[Dict]: + result = query_microsoft_graph( + method, + resource, + params, + body, + subscription, + ) + if result["value"]: + return cast(List[Dict], result["value"]) + else: + raise GraphQueryError("Expected data containing a list of values", None) + + def is_member_of(group_id: str, member_id: str) -> bool: body = {"groupIds": [group_id]} - response = query_microsoft_graph( + response = query_microsoft_graph_list( method="POST", resource=f"users/{member_id}/checkMemberGroups", body=body ) - return group_id in response["value"] + return group_id in response @cached diff --git a/src/api-service/__app__/onefuzzlib/request.py b/src/api-service/__app__/onefuzzlib/request.py index 54552726dd..153e1007a6 100644 --- a/src/api-service/__app__/onefuzzlib/request.py +++ b/src/api-service/__app__/onefuzzlib/request.py @@ -15,7 +15,9 @@ from onefuzztypes.responses import BaseResponse from pydantic import ValidationError -from .azure.creds import is_member_of +from deployment.registration import GraphQueryError + +from .azure.creds import is_member_of, GraphQueryError from .orm import ModelMixin # We don't actually use these types at runtime at this time. Rather, @@ -34,7 +36,7 @@ def check_access(req: HttpRequest) -> Optional[Error]: member_id = req.headers["x-ms-client-principal-id"] try: result = is_member_of(group_id, member_id) - except Exception as e: + except GraphQueryError as e: return Error( code=ErrorCode.UNAUTHORIZED, errors=["unable to interact with graph", str(e)], diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index 2118839b0e..6b4458ad69 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -271,6 +271,7 @@ def get_identifier_url(self) -> str: return "api://%s.azurewebsites.net" % self.application_name def get_signin_audience(self) -> str: + # https://docs.microsoft.com/en-us/azure/active-directory/develop/supported-accounts-validation if self.multi_tenant_domain: return "AzureADMultipleOrgs" else: diff --git a/src/deployment/registration.py b/src/deployment/registration.py index fddc67a9d6..85034ff294 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -20,16 +20,18 @@ logger = logging.getLogger("deploy") +## https://docs.microsoft.com/en-us/graph/api/overview?view=graph-rest-1.0 GRAPH_RESOURCE = "https://graph.microsoft.com" -GRAPH_RESOURCE_ENDPOINT = "https://graph.microsoft.com/v0.1" +GRAPH_RESOURCE_ENDPOINT = "https://graph.microsoft.com/v1.0" class GraphQueryError(Exception): - def __init__(self, message: str, status_code: int) -> None: + def __init__(self, message: str, status_code: Optional[int]) -> None: super(GraphQueryError, self).__init__(message) self.message = message self.status_code = status_code + ## Queries microsoft graph api and return def query_microsoft_graph( method: str, @@ -37,7 +39,7 @@ def query_microsoft_graph( params: Optional[Dict] = None, body: Optional[Dict] = None, subscription: Optional[str] = None, -) -> Any: +) -> Dict: profile = get_cli_profile() (token_type, access_token, _), _, _ = profile.get_raw_token( resource=GRAPH_RESOURCE, subscription=subscription @@ -54,7 +56,7 @@ def query_microsoft_graph( if response.content and response.content.strip(): return response.json() else: - return None + return {} else: error_text = str(response.content, encoding="utf-8", errors="backslashreplace") raise GraphQueryError( @@ -71,23 +73,24 @@ def query_microsoft_graph_list( subscription: Optional[str] = None, ) -> List[Dict]: result = query_microsoft_graph( - method, - resource, - params, - body, - subscription, - ) - if result and result["value"]: + method, + resource, + params, + body, + subscription, + ) + if result["value"]: return cast(List[Dict], result["value"]) else: - raise Exception("Expected data containing a list of values") + raise GraphQueryError("Expected data containing a list of values", None) def get_tenant_id(subscription_id: Optional[str] = None) -> str: - result = query_microsoft_graph_list( - method="GET", resource="organization", subscription=subscription_id + profile = get_cli_profile() + _, tenant_id, _ = profile.get_raw_token( + resource=GRAPH_RESOURCE, subscription=subscription_id ) - return cast(str, result["id"]) + return tenant_id OperationResult = TypeVar("OperationResult") @@ -354,7 +357,9 @@ def get_application( elif number_of_apps == 1: return apps["value"][0] else: - raise Exception(f"Found {number_of_apps} application matching filter: '{filter_str}'") + raise Exception( + f"Found {number_of_apps} application matching filter: '{filter_str}'" + ) def authorize_application( @@ -465,7 +470,7 @@ def assign_app_role( role_names: List[str], subscription_id: str, ) -> None: - application_registration = query_microsoft_graph( + application_registrations = query_microsoft_graph_list( method="GET", resource="servicePrincipals", params={ @@ -473,9 +478,9 @@ def assign_app_role( }, subscription=subscription_id, ) - if len(application_registration["value"]) == 0: + if len(application_registrations) == 0: raise Exception(f"appid '{application_id}' was not found:") - app = application_registration["value"][0] + app = application_registrations[0] roles = ( seq(app["appRoles"]).filter(lambda role: role["value"] in role_names).to_list() @@ -491,12 +496,12 @@ def assign_app_role( ) expected_role_ids = [role["id"] for role in roles] - assignments = query_microsoft_graph( + assignments = query_microsoft_graph_list( method="GET", resource=f"servicePrincipals/{principal_id}/appRoleAssignments", subscription=subscription_id, ) - assigned_role_ids = [assignment["appRoleId"] for assignment in assignments["value"]] + assigned_role_ids = [assignment["appRoleId"] for assignment in assignments] missing_assignments = [ id for id in expected_role_ids if id not in assigned_role_ids ] @@ -526,7 +531,7 @@ def assign_instance_app_role( their managed identity to the provided App Role """ - onefuzz_service_appId = query_microsoft_graph( + onefuzz_service_appIds = query_microsoft_graph_list( method="GET", resource="applications", params={ @@ -535,28 +540,28 @@ def assign_instance_app_role( }, subscription=subscription_id, ) - if len(onefuzz_service_appId["value"]) == 0: + if len(onefuzz_service_appIds) == 0: raise Exception("onefuzz app registration not found") - appId = onefuzz_service_appId["value"][0]["appId"] - onefuzz_service_principals = query_microsoft_graph( + appId = onefuzz_service_appIds[0]["appId"] + onefuzz_service_principals = query_microsoft_graph_list( method="GET", resource="servicePrincipals", params={"$filter": "appId eq '%s'" % appId}, subscription=subscription_id, ) - if len(onefuzz_service_principals["value"]) == 0: + if len(onefuzz_service_principals) == 0: raise Exception("onefuzz app service principal not found") - onefuzz_service_principal = onefuzz_service_principals["value"][0] - application_service_principals = query_microsoft_graph( + onefuzz_service_principal = onefuzz_service_principals[0] + application_service_principals = query_microsoft_graph_list( method="GET", resource="servicePrincipals", params={"$filter": "displayName eq '%s'" % application_name}, subscription=subscription_id, ) - if len(application_service_principals["value"]) == 0: + if len(application_service_principals) == 0: raise Exception(f"application '{application_name}' service principal not found") - application_service_principal = application_service_principals["value"][0] + application_service_principal = application_service_principals[0] managed_node_role = ( seq(onefuzz_service_principal["appRoles"]) .filter(lambda x: x["value"] == app_role.value) @@ -568,7 +573,7 @@ def assign_instance_app_role( f"{app_role.value} role not found in the OneFuzz application " "registration. Please redeploy the instance" ) - assignments = query_microsoft_graph( + assignments = query_microsoft_graph_list( method="GET", resource="servicePrincipals/%s/appRoleAssignments" % application_service_principal["id"], @@ -576,7 +581,7 @@ def assign_instance_app_role( ) # check if the role is already assigned - role_assigned = seq(assignments["value"]).find( + role_assigned = seq(assignments).find( lambda assignment: assignment["appRoleId"] == managed_node_role["id"] ) if not role_assigned: From c6e3888cbd154ec138343bd2096734beecdc9390 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 12 Oct 2021 12:37:59 -0700 Subject: [PATCH 52/64] build fix --- src/api-service/__app__/onefuzzlib/request.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/api-service/__app__/onefuzzlib/request.py b/src/api-service/__app__/onefuzzlib/request.py index 153e1007a6..3a29c7302c 100644 --- a/src/api-service/__app__/onefuzzlib/request.py +++ b/src/api-service/__app__/onefuzzlib/request.py @@ -15,8 +15,6 @@ from onefuzztypes.responses import BaseResponse from pydantic import ValidationError -from deployment.registration import GraphQueryError - from .azure.creds import is_member_of, GraphQueryError from .orm import ModelMixin From 5bb126c9feaf774bf30ec92a9d4e1e97039badeb Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 12 Oct 2021 12:44:56 -0700 Subject: [PATCH 53/64] lint --- src/api-service/__app__/onefuzzlib/azure/creds.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api-service/__app__/onefuzzlib/azure/creds.py b/src/api-service/__app__/onefuzzlib/azure/creds.py index 4f863656f8..d23734d9b1 100644 --- a/src/api-service/__app__/onefuzzlib/azure/creds.py +++ b/src/api-service/__app__/onefuzzlib/azure/creds.py @@ -23,7 +23,7 @@ from .monkeypatch import allow_more_workers, reduce_logging -## https://docs.microsoft.com/en-us/graph/api/overview?view=graph-rest-1.0 +# https://docs.microsoft.com/en-us/graph/api/overview?view=graph-rest-1.0 GRAPH_RESOURCE = "https://graph.microsoft.com" GRAPH_RESOURCE_ENDPOINT = "https://graph.microsoft.com/v1.0" From 5e4053e0d51c6cedba021245a29d93cb72e7d905 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 12 Oct 2021 13:36:00 -0700 Subject: [PATCH 54/64] lint --- src/api-service/__app__/onefuzzlib/request.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api-service/__app__/onefuzzlib/request.py b/src/api-service/__app__/onefuzzlib/request.py index 3a29c7302c..0082c531a9 100644 --- a/src/api-service/__app__/onefuzzlib/request.py +++ b/src/api-service/__app__/onefuzzlib/request.py @@ -15,7 +15,7 @@ from onefuzztypes.responses import BaseResponse from pydantic import ValidationError -from .azure.creds import is_member_of, GraphQueryError +from .azure.creds import GraphQueryError, is_member_of from .orm import ModelMixin # We don't actually use these types at runtime at this time. Rather, From 82e8e1ab5c69526a94032eee3620f3e8257aaa2e Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 12 Oct 2021 13:59:24 -0700 Subject: [PATCH 55/64] mypy fix --- src/api-service/__app__/onefuzzlib/azure/creds.py | 4 +--- src/deployment/registration.py | 7 +++++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/api-service/__app__/onefuzzlib/azure/creds.py b/src/api-service/__app__/onefuzzlib/azure/creds.py index d23734d9b1..9854d1d9f0 100644 --- a/src/api-service/__app__/onefuzzlib/azure/creds.py +++ b/src/api-service/__app__/onefuzzlib/azure/creds.py @@ -130,7 +130,7 @@ def query_microsoft_graph( if 200 <= response.status_code < 300: if response.content and response.content.strip(): - return response.json() + return cast(Dict, response.json()) else: return {} else: @@ -146,14 +146,12 @@ def query_microsoft_graph_list( resource: str, params: Optional[Dict] = None, body: Optional[Dict] = None, - subscription: Optional[str] = None, ) -> List[Dict]: result = query_microsoft_graph( method, resource, params, body, - subscription, ) if result["value"]: return cast(List[Dict], result["value"]) diff --git a/src/deployment/registration.py b/src/deployment/registration.py index 85034ff294..85b526994c 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -54,7 +54,7 @@ def query_microsoft_graph( ) if 200 <= response.status_code < 300: if response.content and response.content.strip(): - return response.json() + return cast(Dict, response.json()) else: return {} else: @@ -90,7 +90,10 @@ def get_tenant_id(subscription_id: Optional[str] = None) -> str: _, tenant_id, _ = profile.get_raw_token( resource=GRAPH_RESOURCE, subscription=subscription_id ) - return tenant_id + if isinstance(tenant_id, str): + return tenant_id + else: + raise Exception(f"unable to retrive tenant_id for subscription {subscription_id}") OperationResult = TypeVar("OperationResult") From f30f8dbd4896efc354df0b23d164c89afb990b9f Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 12 Oct 2021 14:19:25 -0700 Subject: [PATCH 56/64] mypy fix --- src/deployment/deploy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index 6b4458ad69..44b2bf614c 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -449,7 +449,7 @@ def try_sp_create() -> None: and app["signInAudience"] == "AzureADMultipleOrgs" ): set_app_audience( - app.object_id, + app["id"], "AzureADMyOrg", subscription_id=self.get_subscription_id(), ) From 550aa25327ec72e15832a2b4cfe9ad832a5db7d1 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 12 Oct 2021 15:54:57 -0700 Subject: [PATCH 57/64] formatting --- src/deployment/registration.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/deployment/registration.py b/src/deployment/registration.py index 85b526994c..ee67a58c5d 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -93,7 +93,9 @@ def get_tenant_id(subscription_id: Optional[str] = None) -> str: if isinstance(tenant_id, str): return tenant_id else: - raise Exception(f"unable to retrive tenant_id for subscription {subscription_id}") + raise Exception( + f"unable to retrive tenant_id for subscription {subscription_id}" + ) OperationResult = TypeVar("OperationResult") From eaa5992cfd4e1bc77850a127afd26e607cb7e0d4 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Wed, 13 Oct 2021 10:05:10 -0700 Subject: [PATCH 58/64] address PR comments --- src/api-service/__app__/onefuzzlib/azure/creds.py | 14 +++++++++++--- src/deployment/registration.py | 14 +++++++++++--- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/api-service/__app__/onefuzzlib/azure/creds.py b/src/api-service/__app__/onefuzzlib/azure/creds.py index 9854d1d9f0..6385ae743d 100644 --- a/src/api-service/__app__/onefuzzlib/azure/creds.py +++ b/src/api-service/__app__/onefuzzlib/azure/creds.py @@ -130,7 +130,14 @@ def query_microsoft_graph( if 200 <= response.status_code < 300: if response.content and response.content.strip(): - return cast(Dict, response.json()) + json = response.json() + if isinstance(json, Dict): + return json + else: + raise GraphQueryError( + f"invalid data received expected a json object: HTTP {response.status_code} - {json}", + response.status_code, + ) else: return {} else: @@ -153,8 +160,9 @@ def query_microsoft_graph_list( params, body, ) - if result["value"]: - return cast(List[Dict], result["value"]) + value = result.get("value") + if isinstance(value, list): + return value else: raise GraphQueryError("Expected data containing a list of values", None) diff --git a/src/deployment/registration.py b/src/deployment/registration.py index ee67a58c5d..1faf6b6782 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -54,7 +54,14 @@ def query_microsoft_graph( ) if 200 <= response.status_code < 300: if response.content and response.content.strip(): - return cast(Dict, response.json()) + json = response.json() + if isinstance(json, Dict): + return json + else: + raise GraphQueryError( + f"invalid data received expected a json object: HTTP {response.status_code} - {json}", + response.status_code, + ) else: return {} else: @@ -79,8 +86,9 @@ def query_microsoft_graph_list( body, subscription, ) - if result["value"]: - return cast(List[Dict], result["value"]) + value = result.get("value") + if isinstance(value, list): + return value else: raise GraphQueryError("Expected data containing a list of values", None) From c75cd3ae6592e1278319da10c2e07e77eceab02b Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Wed, 13 Oct 2021 10:19:37 -0700 Subject: [PATCH 59/64] linting --- src/api-service/__app__/onefuzzlib/azure/creds.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/api-service/__app__/onefuzzlib/azure/creds.py b/src/api-service/__app__/onefuzzlib/azure/creds.py index 6385ae743d..42380b06d4 100644 --- a/src/api-service/__app__/onefuzzlib/azure/creds.py +++ b/src/api-service/__app__/onefuzzlib/azure/creds.py @@ -135,7 +135,8 @@ def query_microsoft_graph( return json else: raise GraphQueryError( - f"invalid data received expected a json object: HTTP {response.status_code} - {json}", + "invalid data expected a json object: HTTP" + f" {response.status_code} - {json}", response.status_code, ) else: From 1f0408b32dd463e98cde2b24c0b745d6777185a0 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Wed, 13 Oct 2021 15:22:32 -0700 Subject: [PATCH 60/64] lint --- src/deployment/registration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/deployment/registration.py b/src/deployment/registration.py index 1faf6b6782..fd7ba32136 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -10,7 +10,7 @@ import urllib.parse from datetime import datetime, timedelta from enum import Enum -from typing import Any, Callable, Dict, List, NamedTuple, Optional, Tuple, TypeVar, cast +from typing import Any, Callable, Dict, List, NamedTuple, Optional, Tuple, TypeVar from uuid import UUID import requests From 28b4dcf77a9934cbc7a857acf1813ee26ea97892 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 19 Oct 2021 13:16:02 -0700 Subject: [PATCH 61/64] remove ONEFUZZ_AAD_GROUP_ID check --- src/api-service/__app__/onefuzzlib/request.py | 26 +++++-------------- src/pytypes/onefuzztypes/enums.py | 1 + 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/src/api-service/__app__/onefuzzlib/request.py b/src/api-service/__app__/onefuzzlib/request.py index 0082c531a9..470ac429f0 100644 --- a/src/api-service/__app__/onefuzzlib/request.py +++ b/src/api-service/__app__/onefuzzlib/request.py @@ -15,7 +15,6 @@ from onefuzztypes.responses import BaseResponse from pydantic import ValidationError -from .azure.creds import GraphQueryError, is_member_of from .orm import ModelMixin # We don't actually use these types at runtime at this time. Rather, @@ -27,26 +26,15 @@ def check_access(req: HttpRequest) -> Optional[Error]: - if "ONEFUZZ_AAD_GROUP_ID" not in os.environ: - return None - - group_id = os.environ["ONEFUZZ_AAD_GROUP_ID"] - member_id = req.headers["x-ms-client-principal-id"] - try: - result = is_member_of(group_id, member_id) - except GraphQueryError as e: - return Error( - code=ErrorCode.UNAUTHORIZED, - errors=["unable to interact with graph", str(e)], - ) - if not result: - logging.error("unauthorized access: %s is not in %s", member_id, group_id) + if "ONEFUZZ_AAD_GROUP_ID" in os.environ: + message = "ONEFUZZ_AAD_GROUP_ID configuration not supported" + logging.error(message) return Error( - code=ErrorCode.UNAUTHORIZED, - errors=["not approved to use this instance of onefuzz"], + code=ErrorCode.INVALID_CONFIGURATION, + errors=[message], ) - - return None + else: + return None def ok( diff --git a/src/pytypes/onefuzztypes/enums.py b/src/pytypes/onefuzztypes/enums.py index 67796bd7fa..08fb8aef30 100644 --- a/src/pytypes/onefuzztypes/enums.py +++ b/src/pytypes/onefuzztypes/enums.py @@ -265,6 +265,7 @@ class ErrorCode(Enum): NOTIFICATION_FAILURE = 470 UNABLE_TO_UPDATE = 471 PROXY_FAILED = 472 + INVALID_CONFIGURATION = 473 class HeartbeatType(Enum): From e58c69e05f795a33a6c05758cf28d3f4e47ec0c9 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 19 Oct 2021 13:28:09 -0700 Subject: [PATCH 62/64] regenerate webhook_events.md --- docs/webhook_events.md | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/docs/webhook_events.md b/docs/webhook_events.md index 53964ed3b9..c4e065806e 100644 --- a/docs/webhook_events.md +++ b/docs/webhook_events.md @@ -1004,7 +1004,8 @@ Each event will be submitted via HTTP POST to the user provided URL. 469, 470, 471, - 472 + 472, + 473 ], "title": "ErrorCode" }, @@ -1615,7 +1616,8 @@ Each event will be submitted via HTTP POST to the user provided URL. 469, 470, 471, - 472 + 472, + 473 ], "title": "ErrorCode" } @@ -2501,7 +2503,8 @@ Each event will be submitted via HTTP POST to the user provided URL. 469, 470, 471, - 472 + 472, + 473 ], "title": "ErrorCode" } @@ -3190,7 +3193,8 @@ Each event will be submitted via HTTP POST to the user provided URL. 469, 470, 471, - 472 + 472, + 473 ], "title": "ErrorCode" }, @@ -5123,7 +5127,8 @@ Each event will be submitted via HTTP POST to the user provided URL. 469, 470, 471, - 472 + 472, + 473 ], "title": "ErrorCode" }, From 701fb69c92fa41893b3d79faf4b60144bc380345 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 19 Oct 2021 14:35:59 -0700 Subject: [PATCH 63/64] change return type of query_microsoft_graph_list --- src/api-service/__app__/onefuzzlib/azure/creds.py | 2 +- src/deployment/registration.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/api-service/__app__/onefuzzlib/azure/creds.py b/src/api-service/__app__/onefuzzlib/azure/creds.py index 42380b06d4..d946b6de5c 100644 --- a/src/api-service/__app__/onefuzzlib/azure/creds.py +++ b/src/api-service/__app__/onefuzzlib/azure/creds.py @@ -154,7 +154,7 @@ def query_microsoft_graph_list( resource: str, params: Optional[Dict] = None, body: Optional[Dict] = None, -) -> List[Dict]: +) -> List[Any]: result = query_microsoft_graph( method, resource, diff --git a/src/deployment/registration.py b/src/deployment/registration.py index fd7ba32136..c44aacaa8f 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -78,7 +78,7 @@ def query_microsoft_graph_list( params: Optional[Dict] = None, body: Optional[Dict] = None, subscription: Optional[str] = None, -) -> List[Dict]: +) -> List[Any]: result = query_microsoft_graph( method, resource, From 74a6d705964cfbecd8caf5a82c003f0ac22d12cd Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 19 Oct 2021 21:55:35 -0700 Subject: [PATCH 64/64] fix tenant_id --- src/deployment/registration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/deployment/registration.py b/src/deployment/registration.py index c44aacaa8f..b6edc06e24 100644 --- a/src/deployment/registration.py +++ b/src/deployment/registration.py @@ -95,7 +95,7 @@ def query_microsoft_graph_list( def get_tenant_id(subscription_id: Optional[str] = None) -> str: profile = get_cli_profile() - _, tenant_id, _ = profile.get_raw_token( + _, _, tenant_id = profile.get_raw_token( resource=GRAPH_RESOURCE, subscription=subscription_id ) if isinstance(tenant_id, str):