From ba07fb5d00f74fa0927b64bda669dbd3b2f5954f Mon Sep 17 00:00:00 2001 From: Tracy Boehrer Date: Mon, 24 Jun 2024 11:13:16 -0500 Subject: [PATCH 1/5] Added ManagedIdentity --- ...ation_service_client_credential_factory.py | 60 ++++++++++++++++--- .../botframework/connector/auth/__init__.py | 2 + .../auth/managedidentity_app_credentials.py | 54 +++++++++++++++++ ...ntity_service_client_credential_factory.py | 39 ++++++++++++ .../botframework-connector/requirements.txt | 2 +- libraries/botframework-connector/setup.py | 2 +- 6 files changed, 150 insertions(+), 9 deletions(-) create mode 100644 libraries/botframework-connector/botframework/connector/auth/managedidentity_app_credentials.py create mode 100644 libraries/botframework-connector/botframework/connector/auth/managedidentity_service_client_credential_factory.py diff --git a/libraries/botbuilder-integration-aiohttp/botbuilder/integration/aiohttp/configuration_service_client_credential_factory.py b/libraries/botbuilder-integration-aiohttp/botbuilder/integration/aiohttp/configuration_service_client_credential_factory.py index 34e7c7644..557875530 100644 --- a/libraries/botbuilder-integration-aiohttp/botbuilder/integration/aiohttp/configuration_service_client_credential_factory.py +++ b/libraries/botbuilder-integration-aiohttp/botbuilder/integration/aiohttp/configuration_service_client_credential_factory.py @@ -4,18 +4,24 @@ from logging import Logger from typing import Any -from botframework.connector.auth import PasswordServiceClientCredentialFactory +from msrest.authentication import Authentication +from botframework.connector.auth import PasswordServiceClientCredentialFactory +from botframework.connector.auth import ManagedIdentityServiceClientCredentialsFactory +from botframework.connector.auth import ServiceClientCredentialsFactory class ConfigurationServiceClientCredentialFactory( - PasswordServiceClientCredentialFactory + ServiceClientCredentialsFactory ): def __init__(self, configuration: Any, *, logger: Logger = None) -> None: + self._inner = None + app_type = ( configuration.APP_TYPE if hasattr(configuration, "APP_TYPE") else "MultiTenant" - ) + ).lower() + app_id = configuration.APP_ID if hasattr(configuration, "APP_ID") else None app_password = ( configuration.APP_PASSWORD @@ -24,10 +30,23 @@ def __init__(self, configuration: Any, *, logger: Logger = None) -> None: ) app_tenantid = None - if app_type == "UserAssignedMsi": - raise Exception("UserAssignedMsi APP_TYPE is not supported") + if app_type == "userassignedmsi": + if not app_id: + raise Exception("Property 'APP_ID' is expected in configuration object") + + app_tenantid = ( + configuration.APP_TENANTID + if hasattr(configuration, "APP_TENANTID") + else None + ) + if not app_tenantid: + raise Exception( + "Property 'APP_TENANTID' is expected in configuration object" + ) + + self._inner = ManagedIdentityServiceClientCredentialsFactory(app_id, logger=logger) - if app_type == "SingleTenant": + elif app_type == "singletenant": app_tenantid = ( configuration.APP_TENANTID if hasattr(configuration, "APP_TENANTID") @@ -45,4 +64,31 @@ def __init__(self, configuration: Any, *, logger: Logger = None) -> None: "Property 'APP_TENANTID' is expected in configuration object" ) - super().__init__(app_id, app_password, app_tenantid, logger=logger) + self._inner = PasswordServiceClientCredentialFactory(app_id, app_password, app_tenantid, logger=logger) + + # Default to MultiTenant + else: + if not app_id: + raise Exception("Property 'APP_ID' is expected in configuration object") + if not app_password: + raise Exception( + "Property 'APP_PASSWORD' is expected in configuration object" + ) + + self._inner = PasswordServiceClientCredentialFactory(app_id, app_password, None, logger=logger) + + async def is_valid_app_id(self, app_id: str) -> bool: + return self._inner.is_valid_app_id(app_id) + + async def is_authentication_disabled(self) -> bool: + return self._inner.is_authentication_disabled() + + async def create_credentials( + self, + app_id: str, + oauth_scope: str, + login_endpoint: str, + validate_authority: bool, + ) -> Authentication: + return self._inner.create_credentials(app_id, oauth_scope, login_endpoint, validate_authority) + diff --git a/libraries/botframework-connector/botframework/connector/auth/__init__.py b/libraries/botframework-connector/botframework/connector/auth/__init__.py index fd34db01a..8747a03c8 100644 --- a/libraries/botframework-connector/botframework/connector/auth/__init__.py +++ b/libraries/botframework-connector/botframework/connector/auth/__init__.py @@ -27,3 +27,5 @@ from .service_client_credentials_factory import * from .user_token_client import * from .authentication_configuration import * +from .managedidentity_app_credentials import * +from .managedidentity_service_client_credential_factory import * diff --git a/libraries/botframework-connector/botframework/connector/auth/managedidentity_app_credentials.py b/libraries/botframework-connector/botframework/connector/auth/managedidentity_app_credentials.py new file mode 100644 index 000000000..8c9a531d0 --- /dev/null +++ b/libraries/botframework-connector/botframework/connector/auth/managedidentity_app_credentials.py @@ -0,0 +1,54 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +from abc import ABC + +import msal +import requests + +from .app_credentials import AppCredentials +from .microsoft_app_credentials import MicrosoftAppCredentials + + +class ManagedIdentityAppCredentials(AppCredentials, ABC): + """ + AppCredentials implementation using application ID and password. + """ + + global_token_cache = msal.TokenCache() + + def __init__(self, app_id: str, oauth_scope: str = None): + # super will set proper scope and endpoint. + super().__init__( + app_id=app_id, + oauth_scope=oauth_scope, + ) + + self.app = None + + @staticmethod + def empty(): + return MicrosoftAppCredentials("", "") + + def get_access_token(self, force_refresh: bool = False) -> str: + """ + Implementation of AppCredentials.get_token. + :return: The access token for the given app id and password. + """ + + # Firstly, looks up a token from cache + # Since we are looking for token for the current app, NOT for an end user, + # notice we give account parameter as None. + auth_token = self.__get_msal_app().acquire_token_for_client( + resource=self.oauth_scope + ) + return auth_token["access_token"] + + def __get_msal_app(self): + if not self.app: + self.app = msal.ManagedIdentityClient( + self.microsoft_app_id, + http_client=requests.Session(), + token_cache=ManagedIdentityAppCredentials.global_token_cache, + ) + return self.app diff --git a/libraries/botframework-connector/botframework/connector/auth/managedidentity_service_client_credential_factory.py b/libraries/botframework-connector/botframework/connector/auth/managedidentity_service_client_credential_factory.py new file mode 100644 index 000000000..61bf2a12b --- /dev/null +++ b/libraries/botframework-connector/botframework/connector/auth/managedidentity_service_client_credential_factory.py @@ -0,0 +1,39 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +from logging import Logger + +from msrest.authentication import Authentication + +from .managedidentity_app_credentials import ManagedIdentityAppCredentials +from .microsoft_app_credentials import MicrosoftAppCredentials +from .service_client_credentials_factory import ServiceClientCredentialsFactory + + +class ManagedIdentityServiceClientCredentialsFactory(ServiceClientCredentialsFactory): + def __init__(self, app_id: str = None, *, logger: Logger = None) -> None: + self.app_id = app_id + self._logger = logger + + async def is_valid_app_id(self, app_id: str) -> bool: + return app_id == self.app_id + + async def is_authentication_disabled(self) -> bool: + return not self.app_id + + async def create_credentials( + self, + app_id: str, + oauth_scope: str, + login_endpoint: str, + validate_authority: bool, + ) -> Authentication: + if await self.is_authentication_disabled(): + return MicrosoftAppCredentials.empty() + + if not await self.is_valid_app_id(app_id): + raise Exception("Invalid app_id") + + credentials = ManagedIdentityAppCredentials(app_id, oauth_scope) + + return credentials diff --git a/libraries/botframework-connector/requirements.txt b/libraries/botframework-connector/requirements.txt index a762e8813..3c789ac2c 100644 --- a/libraries/botframework-connector/requirements.txt +++ b/libraries/botframework-connector/requirements.txt @@ -3,4 +3,4 @@ botbuilder-schema==4.16.0 requests==2.32.0 PyJWT==2.4.0 cryptography==42.0.4 -msal==1.* +msal>=1.29.0 diff --git a/libraries/botframework-connector/setup.py b/libraries/botframework-connector/setup.py index ca4b72025..ad269fe60 100644 --- a/libraries/botframework-connector/setup.py +++ b/libraries/botframework-connector/setup.py @@ -11,7 +11,7 @@ # "requests>=2.23.0,<2.26", "PyJWT>=2.4.0", "botbuilder-schema==4.16.0", - "msal==1.*", + "msal>=1.29.0", ] root = os.path.abspath(os.path.dirname(__file__)) From 8f1952aedc704d17c7ba496b450a3102c0883a63 Mon Sep 17 00:00:00 2001 From: Tracy Boehrer Date: Mon, 24 Jun 2024 11:39:22 -0500 Subject: [PATCH 2/5] Missing ConfigurationServiceClientCredentialFactory awaits --- .../configuration_service_client_credential_factory.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libraries/botbuilder-integration-aiohttp/botbuilder/integration/aiohttp/configuration_service_client_credential_factory.py b/libraries/botbuilder-integration-aiohttp/botbuilder/integration/aiohttp/configuration_service_client_credential_factory.py index 557875530..9af6661b2 100644 --- a/libraries/botbuilder-integration-aiohttp/botbuilder/integration/aiohttp/configuration_service_client_credential_factory.py +++ b/libraries/botbuilder-integration-aiohttp/botbuilder/integration/aiohttp/configuration_service_client_credential_factory.py @@ -78,10 +78,10 @@ def __init__(self, configuration: Any, *, logger: Logger = None) -> None: self._inner = PasswordServiceClientCredentialFactory(app_id, app_password, None, logger=logger) async def is_valid_app_id(self, app_id: str) -> bool: - return self._inner.is_valid_app_id(app_id) + return await self._inner.is_valid_app_id(app_id) async def is_authentication_disabled(self) -> bool: - return self._inner.is_authentication_disabled() + return await self._inner.is_authentication_disabled() async def create_credentials( self, @@ -90,5 +90,5 @@ async def create_credentials( login_endpoint: str, validate_authority: bool, ) -> Authentication: - return self._inner.create_credentials(app_id, oauth_scope, login_endpoint, validate_authority) + return await self._inner.create_credentials(app_id, oauth_scope, login_endpoint, validate_authority) From 0688f8d168fe150654686fcc64ddaa18bcbc4db0 Mon Sep 17 00:00:00 2001 From: Tracy Boehrer Date: Mon, 24 Jun 2024 12:44:47 -0500 Subject: [PATCH 3/5] ManagedIdentityAppCredentials needs ManagedIdentity dict --- .../connector/auth/managedidentity_app_credentials.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libraries/botframework-connector/botframework/connector/auth/managedidentity_app_credentials.py b/libraries/botframework-connector/botframework/connector/auth/managedidentity_app_credentials.py index 8c9a531d0..568eb19e2 100644 --- a/libraries/botframework-connector/botframework/connector/auth/managedidentity_app_credentials.py +++ b/libraries/botframework-connector/botframework/connector/auth/managedidentity_app_credentials.py @@ -24,6 +24,8 @@ def __init__(self, app_id: str, oauth_scope: str = None): oauth_scope=oauth_scope, ) + self._managed_identity = {"ManagedIdentityIdType": "ClientId", "Id": app_id} + self.app = None @staticmethod @@ -47,7 +49,7 @@ def get_access_token(self, force_refresh: bool = False) -> str: def __get_msal_app(self): if not self.app: self.app = msal.ManagedIdentityClient( - self.microsoft_app_id, + self._managed_identity, http_client=requests.Session(), token_cache=ManagedIdentityAppCredentials.global_token_cache, ) From b0ba624f18f22fc01b99f2b4d3b0ac005ae6e477 Mon Sep 17 00:00:00 2001 From: Tracy Boehrer Date: Tue, 25 Jun 2024 10:19:08 -0500 Subject: [PATCH 4/5] Added missing PermissionError descriptions --- .../core/channel_service_handler.py | 2 +- .../integration/aiohttp/cloud_adapter.py | 2 +- ...ation_service_client_credential_factory.py | 26 ++++++++++++------- ...ameterized_bot_framework_authentication.py | 12 ++++----- .../connector/auth/jwt_token_validation.py | 2 +- 5 files changed, 25 insertions(+), 19 deletions(-) diff --git a/libraries/botbuilder-core/botbuilder/core/channel_service_handler.py b/libraries/botbuilder-core/botbuilder/core/channel_service_handler.py index 8de4da56d..834dbce08 100644 --- a/libraries/botbuilder-core/botbuilder/core/channel_service_handler.py +++ b/libraries/botbuilder-core/botbuilder/core/channel_service_handler.py @@ -504,7 +504,7 @@ async def _authenticate(self, auth_header: str) -> ClaimsIdentity: ) if not is_auth_disabled: # No auth header. Auth is required. Request is not authorized. - raise PermissionError() + raise PermissionError("Authorization is required but has been disabled.") # In the scenario where Auth is disabled, we still want to have the # IsAuthenticated flag set in the ClaimsIdentity. To do this requires diff --git a/libraries/botbuilder-integration-aiohttp/botbuilder/integration/aiohttp/cloud_adapter.py b/libraries/botbuilder-integration-aiohttp/botbuilder/integration/aiohttp/cloud_adapter.py index 0aa2ba8af..0f9131871 100644 --- a/libraries/botbuilder-integration-aiohttp/botbuilder/integration/aiohttp/cloud_adapter.py +++ b/libraries/botbuilder-integration-aiohttp/botbuilder/integration/aiohttp/cloud_adapter.py @@ -107,7 +107,7 @@ async def process( return Response(status=201) else: raise HTTPMethodNotAllowed - except (HTTPUnauthorized, PermissionError) as _: + except PermissionError: raise HTTPUnauthorized async def _connect( diff --git a/libraries/botbuilder-integration-aiohttp/botbuilder/integration/aiohttp/configuration_service_client_credential_factory.py b/libraries/botbuilder-integration-aiohttp/botbuilder/integration/aiohttp/configuration_service_client_credential_factory.py index 9af6661b2..6379e16b6 100644 --- a/libraries/botbuilder-integration-aiohttp/botbuilder/integration/aiohttp/configuration_service_client_credential_factory.py +++ b/libraries/botbuilder-integration-aiohttp/botbuilder/integration/aiohttp/configuration_service_client_credential_factory.py @@ -10,9 +10,8 @@ from botframework.connector.auth import ManagedIdentityServiceClientCredentialsFactory from botframework.connector.auth import ServiceClientCredentialsFactory -class ConfigurationServiceClientCredentialFactory( - ServiceClientCredentialsFactory -): + +class ConfigurationServiceClientCredentialFactory(ServiceClientCredentialsFactory): def __init__(self, configuration: Any, *, logger: Logger = None) -> None: self._inner = None @@ -43,8 +42,10 @@ def __init__(self, configuration: Any, *, logger: Logger = None) -> None: raise Exception( "Property 'APP_TENANTID' is expected in configuration object" ) - - self._inner = ManagedIdentityServiceClientCredentialsFactory(app_id, logger=logger) + + self._inner = ManagedIdentityServiceClientCredentialsFactory( + app_id, logger=logger + ) elif app_type == "singletenant": app_tenantid = ( @@ -64,7 +65,9 @@ def __init__(self, configuration: Any, *, logger: Logger = None) -> None: "Property 'APP_TENANTID' is expected in configuration object" ) - self._inner = PasswordServiceClientCredentialFactory(app_id, app_password, app_tenantid, logger=logger) + self._inner = PasswordServiceClientCredentialFactory( + app_id, app_password, app_tenantid, logger=logger + ) # Default to MultiTenant else: @@ -74,8 +77,10 @@ def __init__(self, configuration: Any, *, logger: Logger = None) -> None: raise Exception( "Property 'APP_PASSWORD' is expected in configuration object" ) - - self._inner = PasswordServiceClientCredentialFactory(app_id, app_password, None, logger=logger) + + self._inner = PasswordServiceClientCredentialFactory( + app_id, app_password, None, logger=logger + ) async def is_valid_app_id(self, app_id: str) -> bool: return await self._inner.is_valid_app_id(app_id) @@ -90,5 +95,6 @@ async def create_credentials( login_endpoint: str, validate_authority: bool, ) -> Authentication: - return await self._inner.create_credentials(app_id, oauth_scope, login_endpoint, validate_authority) - + return await self._inner.create_credentials( + app_id, oauth_scope, login_endpoint, validate_authority + ) diff --git a/libraries/botframework-connector/botframework/connector/auth/_parameterized_bot_framework_authentication.py b/libraries/botframework-connector/botframework/connector/auth/_parameterized_bot_framework_authentication.py index 1388094fe..3419c2099 100644 --- a/libraries/botframework-connector/botframework/connector/auth/_parameterized_bot_framework_authentication.py +++ b/libraries/botframework-connector/botframework/connector/auth/_parameterized_bot_framework_authentication.py @@ -473,11 +473,11 @@ async def _government_channel_validation_validate_identity( ): if identity is None: # No valid identity. Not Authorized. - raise PermissionError() + raise PermissionError("Identity missing") if not identity.is_authenticated: # The token is in some way invalid. Not Authorized. - raise PermissionError() + raise PermissionError("Invalid token") # Now check that the AppID in the claim set matches # what we're looking for. Note that in a multi-tenant bot, this value @@ -487,12 +487,12 @@ async def _government_channel_validation_validate_identity( # Look for the "aud" claim, but only if issued from the Bot Framework issuer = identity.get_claim_value(AuthenticationConstants.ISSUER_CLAIM) if issuer != self._to_bot_from_channel_token_issuer: - raise PermissionError() + raise PermissionError("'iss' claim missing") app_id = identity.get_claim_value(AuthenticationConstants.AUDIENCE_CLAIM) if not app_id: # The relevant audience Claim MUST be present. Not Authorized. - raise PermissionError() + raise PermissionError("'aud' claim missing") # The AppId from the claim in the token must match the AppId specified by the developer. # In this case, the token is destined for the app, so we find the app ID in the audience claim. @@ -507,8 +507,8 @@ async def _government_channel_validation_validate_identity( ) if not service_url_claim: # Claim must be present. Not Authorized. - raise PermissionError() + raise PermissionError("'serviceurl' claim missing") if service_url_claim != service_url: # Claim must match. Not Authorized. - raise PermissionError() + raise PermissionError("Invalid 'serviceurl' claim") diff --git a/libraries/botframework-connector/botframework/connector/auth/jwt_token_validation.py b/libraries/botframework-connector/botframework/connector/auth/jwt_token_validation.py index 659559f14..a0e937156 100644 --- a/libraries/botframework-connector/botframework/connector/auth/jwt_token_validation.py +++ b/libraries/botframework-connector/botframework/connector/auth/jwt_token_validation.py @@ -46,7 +46,7 @@ async def authenticate_request( auth_is_disabled = await credentials.is_authentication_disabled() if not auth_is_disabled: # No Auth Header. Auth is required. Request is not authorized. - raise PermissionError("Unauthorized Access. Request is not authorized") + raise PermissionError("Required Authorization token was not supplied") # Check if the activity is for a skill call and is coming from the Emulator. try: From 22837ed6374e9d5a0d3a8f0d1b92a0d0054c19a8 Mon Sep 17 00:00:00 2001 From: Tracy Boehrer Date: Thu, 27 Jun 2024 10:45:48 -0500 Subject: [PATCH 5/5] Black reformatting in botbuilder-core --- .../botbuilder/core/channel_service_handler.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libraries/botbuilder-core/botbuilder/core/channel_service_handler.py b/libraries/botbuilder-core/botbuilder/core/channel_service_handler.py index 834dbce08..2b819d00c 100644 --- a/libraries/botbuilder-core/botbuilder/core/channel_service_handler.py +++ b/libraries/botbuilder-core/botbuilder/core/channel_service_handler.py @@ -504,7 +504,9 @@ async def _authenticate(self, auth_header: str) -> ClaimsIdentity: ) if not is_auth_disabled: # No auth header. Auth is required. Request is not authorized. - raise PermissionError("Authorization is required but has been disabled.") + raise PermissionError( + "Authorization is required but has been disabled." + ) # In the scenario where Auth is disabled, we still want to have the # IsAuthenticated flag set in the ClaimsIdentity. To do this requires