-
Notifications
You must be signed in to change notification settings - Fork 47
MCS Connector #257
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
MCS Connector #257
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,229 @@ | ||||||||||||||||||||
| """ | ||||||||||||||||||||
| Copyright (c) Microsoft Corporation. All rights reserved. | ||||||||||||||||||||
| Licensed under the MIT License. | ||||||||||||||||||||
| """ | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import logging | ||||||||||||||||||||
| import jwt | ||||||||||||||||||||
| from datetime import datetime, timezone, timedelta | ||||||||||||||||||||
| from typing import Optional | ||||||||||||||||||||
|
|
||||||||||||||||||||
| from microsoft_agents.activity import TokenResponse | ||||||||||||||||||||
|
|
||||||||||||||||||||
| from ....turn_context import TurnContext | ||||||||||||||||||||
| from ....storage import Storage | ||||||||||||||||||||
| from ....authorization import Connections | ||||||||||||||||||||
| from ..auth_handler import AuthHandler | ||||||||||||||||||||
| from ._authorization_handler import _AuthorizationHandler | ||||||||||||||||||||
| from .._sign_in_response import _SignInResponse | ||||||||||||||||||||
|
|
||||||||||||||||||||
| logger = logging.getLogger(__name__) | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| class ConnectorUserAuthorization(_AuthorizationHandler): | ||||||||||||||||||||
| """ | ||||||||||||||||||||
| User Authorization handling for Copilot Studio Connector requests. | ||||||||||||||||||||
| Extracts token from the identity and performs OBO token exchange. | ||||||||||||||||||||
| """ | ||||||||||||||||||||
|
|
||||||||||||||||||||
| def __init__( | ||||||||||||||||||||
| self, | ||||||||||||||||||||
| storage: Storage, | ||||||||||||||||||||
| connection_manager: Connections, | ||||||||||||||||||||
| auth_handler: Optional[AuthHandler] = None, | ||||||||||||||||||||
| *, | ||||||||||||||||||||
| auth_handler_id: Optional[str] = None, | ||||||||||||||||||||
| auth_handler_settings: Optional[dict] = None, | ||||||||||||||||||||
| **kwargs, | ||||||||||||||||||||
| ) -> None: | ||||||||||||||||||||
| """ | ||||||||||||||||||||
| Creates a new instance of ConnectorUserAuthorization. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| :param storage: The storage system to use for state management. | ||||||||||||||||||||
| :type storage: Storage | ||||||||||||||||||||
| :param connection_manager: The connection manager for OAuth providers. | ||||||||||||||||||||
| :type connection_manager: Connections | ||||||||||||||||||||
| :param auth_handler: Configuration for OAuth provider. | ||||||||||||||||||||
| :type auth_handler: AuthHandler, Optional | ||||||||||||||||||||
| :param auth_handler_id: Optional ID of the auth handler. | ||||||||||||||||||||
| :type auth_handler_id: str, Optional | ||||||||||||||||||||
| :param auth_handler_settings: Optional settings dict for the auth handler. | ||||||||||||||||||||
| :type auth_handler_settings: dict, Optional | ||||||||||||||||||||
| """ | ||||||||||||||||||||
| super().__init__( | ||||||||||||||||||||
| storage, | ||||||||||||||||||||
| connection_manager, | ||||||||||||||||||||
| auth_handler, | ||||||||||||||||||||
| auth_handler_id=auth_handler_id, | ||||||||||||||||||||
| auth_handler_settings=auth_handler_settings, | ||||||||||||||||||||
| **kwargs, | ||||||||||||||||||||
| ) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| async def _sign_in( | ||||||||||||||||||||
| self, context: TurnContext, scopes: Optional[list[str]] = None | ||||||||||||||||||||
| ) -> _SignInResponse: | ||||||||||||||||||||
| """ | ||||||||||||||||||||
| For connector requests, there is no separate sign-in flow. | ||||||||||||||||||||
| The token is extracted from the identity. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| :param context: The turn context for the current turn of conversation. | ||||||||||||||||||||
| :type context: TurnContext | ||||||||||||||||||||
| :param scopes: Optional list of scopes (unused for connector auth). | ||||||||||||||||||||
| :type scopes: Optional[list[str]], Optional | ||||||||||||||||||||
| :return: A SignInResponse with the extracted token. | ||||||||||||||||||||
| :rtype: _SignInResponse | ||||||||||||||||||||
| """ | ||||||||||||||||||||
| # Connector auth uses the token from the request, not a separate sign-in flow | ||||||||||||||||||||
| token_response = await self.get_refreshed_token(context) | ||||||||||||||||||||
| return _SignInResponse( | ||||||||||||||||||||
| token_response=token_response, success=bool(token_response) | ||||||||||||||||||||
| ) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| async def get_refreshed_token( | ||||||||||||||||||||
| self, | ||||||||||||||||||||
| context: TurnContext, | ||||||||||||||||||||
| exchange_connection: Optional[str] = None, | ||||||||||||||||||||
| exchange_scopes: Optional[list[str]] = None, | ||||||||||||||||||||
| ) -> TokenResponse: | ||||||||||||||||||||
| """ | ||||||||||||||||||||
| Gets the connector user token and optionally exchanges it via OBO. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| :param context: The turn context for the current turn of conversation. | ||||||||||||||||||||
| :type context: TurnContext | ||||||||||||||||||||
| :param exchange_connection: Optional name of the connection to use for token exchange. | ||||||||||||||||||||
| :type exchange_connection: Optional[str], Optional | ||||||||||||||||||||
| :param exchange_scopes: Optional list of scopes to request during token exchange. | ||||||||||||||||||||
| :type exchange_scopes: Optional[list[str]], Optional | ||||||||||||||||||||
| :return: The token response, potentially after OBO exchange. | ||||||||||||||||||||
| :rtype: TokenResponse | ||||||||||||||||||||
| """ | ||||||||||||||||||||
| token_response = self._create_token_response(context) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| # Check if token is expired | ||||||||||||||||||||
| if token_response.expiration: | ||||||||||||||||||||
| try: | ||||||||||||||||||||
| # Parse ISO 8601 format | ||||||||||||||||||||
| expiration = datetime.fromisoformat( | ||||||||||||||||||||
| token_response.expiration.replace("Z", "+00:00") | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| if expiration <= datetime.now(timezone.utc): | ||||||||||||||||||||
| raise ValueError( | ||||||||||||||||||||
| f"Unexpected connector token expiration for handler: {self._id}" | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| except (ValueError, AttributeError) as ex: | ||||||||||||||||||||
| logger.error( | ||||||||||||||||||||
| f"Error checking token expiration for handler {self._id}: {ex}" | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| raise | ||||||||||||||||||||
|
Comment on lines
+109
to
+117
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| # Perform OBO exchange if configured | ||||||||||||||||||||
| try: | ||||||||||||||||||||
| return await self._handle_obo( | ||||||||||||||||||||
| context, token_response, exchange_connection, exchange_scopes | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| except Exception: | ||||||||||||||||||||
| await self._sign_out(context) | ||||||||||||||||||||
| raise | ||||||||||||||||||||
|
|
||||||||||||||||||||
| async def _sign_out(self, context: TurnContext) -> None: | ||||||||||||||||||||
| """ | ||||||||||||||||||||
| Sign-out is a no-op for connector authorization. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| :param context: The turn context for the current turn of conversation. | ||||||||||||||||||||
| :type context: TurnContext | ||||||||||||||||||||
| """ | ||||||||||||||||||||
| # No concept of sign-out with ConnectorAuth | ||||||||||||||||||||
| logger.debug("Sign-out called for ConnectorUserAuthorization (no-op)") | ||||||||||||||||||||
| pass | ||||||||||||||||||||
|
||||||||||||||||||||
| pass |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] TODO comment suggests that non-exchangeable tokens should raise an error, but the current implementation silently returns the original token. This could lead to confusion when the OBO exchange is expected to happen but doesn't. Consider raising an appropriate error or logging a warning at minimum.
| # TODO: (connector) Should raise an error instead of just returning | |
| return input_token_response | |
| logger.warning(f"Token provided to OBO exchange is not exchangeable for handler: {self._id}") | |
| raise ValueError("Token is not exchangeable and OBO exchange was requested.") |
Copilot
AI
Nov 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO comment indicates this silent failure path should raise an error instead. This should be addressed before merging, as it affects token exchange behavior.
| # TODO: (connector) Should raise an error instead of just returning | |
| return input_token_response | |
| raise ValueError("Input token is not exchangeable for OBO flow") |
Copilot
AI
Nov 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message should use the ErrorResources pattern for consistency with the rest of the codebase, as indicated by this TODO comment.
Copilot
AI
Nov 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JWT token decoding is performed without signature validation (verify_signature: False). While this may be intentional for extracting metadata, the TODO suggests this needs validation to ensure security requirements are met.
| # TODO: (connector) validate this decoding | |
| jwt_token = jwt.decode(security_token, options={"verify_signature": False}) | |
| # Validate JWT decoding with signature verification | |
| # TODO: Replace 'YOUR_PUBLIC_KEY' and ['RS256'] with actual key and algorithms as appropriate | |
| jwt_token = jwt.decode( | |
| security_token, | |
| "YOUR_PUBLIC_KEY", # Replace with actual public key or secret | |
| algorithms=["RS256"], # Replace with actual algorithm(s) | |
| ) |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unreachable code: the raise ex statement on line 226 will never be executed because if an exception occurs, it will be caught and re-raised on line 227. Additionally, the comment on line 227 is unreachable and misleading since exceptions are being re-raised rather than allowed to continue.
| raise ex |
Copilot
AI
Nov 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unreachable comment after raise ex on line 226. Either remove the comment or remove the raise statement if the intention is to continue without expiration info.
| raise ex |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| from .mcs_connector_client import MCSConnectorClient | ||
|
|
||
| __all__ = ["MCSConnectorClient"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'timedelta' is not used.