Skip to content

Normalize composite channels for all user token operations#457

Merged
rodrigobr-msft merged 2 commits into
microsoft:mainfrom
Carvalh0XYZ:fix/copilot-base-channel-token-refresh
Jul 13, 2026
Merged

Normalize composite channels for all user token operations#457
rodrigobr-msft merged 2 commits into
microsoft:mainfrom
Carvalh0XYZ:fix/copilot-base-channel-token-refresh

Conversation

@Carvalh0XYZ

@Carvalh0XYZ Carvalh0XYZ commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

TL;DR

Fixes #456.

M365 Copilot custom-engine activities arrive with a composite channel such as msteams:COPILOT. Bot Framework user-token storage is partitioned by its base channel (msteams). PR #392 normalized only the initial GetTokenOrSignInResource path; every other Python UserToken operation still sent the composite channel.

In a deployed agent this split caused a deterministic silent-SSO loop:

  1. base-channel GetTokenOrSignInResource found the resource;
  2. signin/tokenExchange stored the token under raw msteams:COPILOT;
  3. replay queried base msteams, found no token, and initiated sign-in again;
  4. one message produced 10 token-exchange invokes before the M365 signInInfo endpoint returned HTTP 429.

This PR follows the maintainer's suggested design and the .NET implementation: normalize at the UserToken boundary so every Bot Framework token operation uses the same base-channel partition.

Change

UserToken._base_channel_id() uses ChannelId(...).channel, and all six operations normalize before telemetry and HTTP query construction:

  • get_token
  • _get_token_or_sign_in_resource
  • get_aad_tokens
  • exchange_token
  • get_token_status
  • sign_out

This matches .NET UserTokenRestClient, which uses channelId?.Channel across these operations.

Safety invariants

  • The inbound Activity is not mutated.
  • OAuth flow state retains the original msteams:COPILOT value.
  • Replies, continuations, streaming, service URL, and conversation references retain the originating Copilot context.
  • Plain channels such as msteams are unchanged.
  • None, empty, whitespace-only, and malformed values are not broadened into an unscoped request.
  • Telemetry and HTTP use the same effective channel, matching .NET.

Regression coverage

The tests use a real local aiohttp server and execute all six operations, asserting every wire-level channelId is msteams. They also cover omitted optional channels and malformed/edge values.

Before this change, the composite-channel test observes msteams:COPILOT on those requests; after it, all six use msteams.

Validation

Run from a clean Python 3.12 editable-install environment:

  • Focused token/client tests: 139 passed
  • Full repository suite: 2984 passed, 88 skipped
  • Black on changed files: clean
  • Focused Flake8: clean
  • Independent read-only review: no blockers

Reviewer guide

  1. Review _base_channel_id() and the six call sites in user_token_client.py.
  2. Review the wire-level test covering all six operations.
  3. Compare the boundary with .NET UserTokenRestClient and the requested direction in [Bug] UserAuthorization refresh sends composite Copilot channel to GetToken #456.

Related

@Carvalh0XYZ Carvalh0XYZ requested a review from a team as a code owner July 9, 2026 21:10
Copilot AI review requested due to automatic review settings July 9, 2026 21:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes OAuth refresh/token operations for M365 Copilot Web activities that arrive with composite channelId values (e.g., msteams:COPILOT) by ensuring Bot Framework token API calls consistently use the base channel (msteams) while preserving the original composite channel in _FlowState for storage/continuation context.

Changes:

  • Normalize _OAuthFlow’s operational token channel_id to the base segment via split(":", 1)[0].
  • Add a focused regression test asserting get_user_token() calls UserToken.get_token(channel_id="msteams") while flow.flow_state.channel_id remains unchanged (msteams:COPILOT).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/_oauth/_oauth_flow.py Derives _OAuthFlow’s token-operation channel_id from the base channel while preserving the composite channel in stored flow state.
tests/hosting_core/_oauth/test_oauth_flow.py Adds regression coverage for get_user_token() with a composite channel ID.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/hosting_core/_oauth/test_oauth_flow.py Outdated
@Carvalh0XYZ

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

Copilot AI review requested due to automatic review settings July 9, 2026 21:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings July 10, 2026 19:15
@Carvalh0XYZ Carvalh0XYZ force-pushed the fix/copilot-base-channel-token-refresh branch from ac6aa94 to 51aa996 Compare July 10, 2026 19:15
@Carvalh0XYZ Carvalh0XYZ changed the title Use base channel for Copilot OAuth token operations Normalize composite channels for all user token operations Jul 10, 2026
@Carvalh0XYZ

Copy link
Copy Markdown
Contributor Author

Updated per the maintainer suggestion on #456: the implementation now normalizes at the UserToken boundary, matching .NET, and covers all six Bot Framework token operations with wire-level regression tests. The activity and OAuth flow state remain untouched. Full suite: 2984 passed, 88 skipped.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

@Carvalh0XYZ

Copy link
Copy Markdown
Contributor Author

Confirmed end-to-end in a deployed M365 Copilot custom-engine agent after resetting both the base and composite Bot Framework token partitions.

Fresh-user-path result

A brand-new Copilot conversation completed silent SSO with no sign-in card and no client restart:

  1. The inbound activity remained msteams:COPILOT.
  2. GetTokenOrSignInResource used activity.channel_id=msteams and succeeded.
  3. Exactly one signin/tokenExchange invoke arrived.
  4. UserToken.exchange_token used activity.channel_id=msteams and succeeded.
  5. Continuation replay ran exactly once.
  6. UserToken.get_token used activity.channel_id=msteams and succeeded.
  7. OBO, one downstream agent invocation, and Copilot delivery all succeeded.
  8. There were no failed spans and no Bot Framework 404 responses.

The original activity, reply context, and delivery telemetry continued to report msteams:COPILOT; only Bot Framework token operations used msteams.

The M365 client's signInInfo request returned HTTP 200 with throttle usage 1/10. Before normalization, the same clean-token test produced ten token-exchange/continuation attempts, exhausted the quota at 11/10, and ended in HTTP 429 because exchange stored under msteams:COPILOT while replay queried msteams.

Our telemetry pipeline duplicates stored span rows, so counts above were deduplicated by operation ID; the runtime console independently shows one Resending continuation activity entry.

This validates the boundary requested by the maintainer: normalize all Bot Framework token operations in UserToken, while leaving activity and OAuth flow state untouched. PR #457 now implements and tests that shape across all six token operations.

@rodrigobr-msft rodrigobr-msft left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will need to perform some testing before approving this, just to check off all the bases, but it looks good.

@rodrigobr-msft rodrigobr-msft left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for contributing!

Copilot AI review requested due to automatic review settings July 13, 2026 19:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@rodrigobr-msft rodrigobr-msft merged commit f5b35f9 into microsoft:main Jul 13, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] UserAuthorization refresh sends composite Copilot channel to GetToken

4 participants