Skip to content

allow detour auth #1374

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.

Sign up for GitHub

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

Merged
iceljc merged 1 commit into from
Jul 7, 2026
+37 −2
Merged

allow detour auth #1374

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions src/Plugins/BotSharp.Plugin.MicrosoftTeams/MicrosoftTeamsPlugin.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
using Microsoft.Agents.Authentication;
using Microsoft.Agents.Authentication.Msal;
using Microsoft.Agents.Builder;
using Microsoft.Agents.Connector;
using Microsoft.Agents.Hosting.AspNetCore;
using System.Security.Claims;

namespace BotSharp.Plugin.MicrosoftTeams;

Expand Down Expand Up @@ -48,6 +50,17 @@ public void RegisterDI(IServiceCollection services, IConfiguration config)
// "No connections found in for this Agent in the Connections Configuration".
services.AddSingleton<IConnections>(sp => new ConfigurationConnections(sp, authConfig));

#if DEBUG
services.AddSingleton<IChannelServiceClientFactory>(sp =>
{
var inner = new RestChannelServiceClientFactory(
config,
sp.GetRequiredService<System.Net.Http.IHttpClientFactory>(),
sp.GetRequiredService<IConnections>());
return new AnonymousDebugChannelServiceClientFactory(inner);
});
#endif

// Register adapter + full SDK infrastructure (IChannelServiceClientFactory, IActivityTaskQueue,
// background services, IAgentHttpAdapter, etc.) and TeamsActivityBot as default IAgent.
services.AddAgent<TeamsActivityBot, TeamsAdapter>();
Expand All @@ -68,3 +81,22 @@ public void RegisterDI(IServiceCollection services, IConfiguration config)
services.AddTransient<IAgent, TeamsActivityBot>();
}
}

#if DEBUG
file class AnonymousDebugChannelServiceClientFactory(RestChannelServiceClientFactory inner) : IChannelServiceClientFactory
{
[Obsolete]
public Task<IConnectorClient> CreateConnectorClientAsync(ClaimsIdentity claimsIdentity, string serviceUrl, string audience, CancellationToken cancellationToken, IList<string>? scopes = null, bool useAnonymous = false)
=> inner.CreateConnectorClientAsync(claimsIdentity, serviceUrl, audience, cancellationToken, scopes, true);

// NOTE: inner's ITurnContext overload still branches on Activity.Recipient.Role=="agenticUser"
// regardless of useAnonymous, so route through the ClaimsIdentity overload to actually skip it.
public Task<IConnectorClient> CreateConnectorClientAsync(ITurnContext turnContext, string? audience = null, IList<string>? scopes = null, bool useAnonymous = false, CancellationToken cancellationToken = default)
#pragma warning disable CS0618 // intentionally using the legacy overload to bypass the agentic branch
=> inner.CreateConnectorClientAsync(turnContext.Identity, turnContext.Activity.ServiceUrl, audience, cancellationToken, scopes, true);
#pragma warning restore CS0618
Comment on lines +94 to +97

@qodo-code-review qodo-code-review Bot Jul 7, 2026

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.

Remediation recommended

1. Unvalidated serviceurl usage 📘 Rule violation ☼ Reliability

AnonymousDebugChannelServiceClientFactory.CreateConnectorClientAsync(ITurnContext ...) passes
turnContext.Activity.ServiceUrl (and turnContext.Identity) to connector creation without
null/empty/state checks. This can cause exception-driven control flow at an integration boundary
instead of a safe, deterministic failure.
Agent Prompt 
## Issue description
The debug `IChannelServiceClientFactory` implementation forwards `turnContext.Identity` and `turnContext.Activity.ServiceUrl` to `RestChannelServiceClientFactory` without validating they are present/valid, which can lead to exceptions at an integration boundary.

## Issue Context
Even though this is `#if DEBUG`, the compliance requirement expects boundary inputs (like URLs and request-derived fields) to be validated and to fail safely/deterministically.

## Fix Focus Areas
- src/Plugins/BotSharp.Plugin.MicrosoftTeams/MicrosoftTeamsPlugin.cs[94-97]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +94 to +97

@qodo-code-review qodo-code-review Bot Jul 7, 2026

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.

Remediation recommended

2. Nullable audience forwarded 🐞 Bug ≡ Correctness

AnonymousDebugChannelServiceClientFactory.CreateConnectorClientAsync(ITurnContext, ...) accepts
string? audience = null but forwards that value into a legacy overload that declares `string
audience, relying on the inner factory to tolerate null and risking failures when audience` is
omitted. This is a new nullability/contract mismatch introduced in the DEBUG detour path.
Agent Prompt 
### Issue description
`AnonymousDebugChannelServiceClientFactory.CreateConnectorClientAsync(ITurnContext ...)` takes `string? audience = null` and passes it to `RestChannelServiceClientFactory.CreateConnectorClientAsync(ClaimsIdentity, string, string audience, ...)`.

This violates the nullability contract of the legacy overload and makes connector creation depend on the inner implementation’s handling of `null` (which may fail or produce incorrect audience selection).

### Issue Context
This only executes under `#if DEBUG`, but it directly affects local/debug runs and test environments where the detour factory is registered.

### Fix Focus Areas
- src/Plugins/BotSharp.Plugin.MicrosoftTeams/MicrosoftTeamsPlugin.cs[94-97]

### Suggested fix
Update the ITurnContext overload to ensure a non-null audience is passed to the legacy overload. Options:
1) Coalesce to a safe default (preferably the same default the SDK would use when `audience` is null), e.g. derive it from configuration/settings, and only fall back to `string.Empty` if that is known to be accepted by the SDK.
2) Refactor the wrapper/constructor to accept whatever value you consider the default audience (e.g., AppId or configured audience) and apply `audience ??= <defaultAudience>` before calling the legacy overload.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


public Task<IUserTokenClient> CreateUserTokenClientAsync(ClaimsIdentity claimsIdentity, bool? useAnonymous = false, CancellationToken cancellationToken = default)
=> inner.CreateUserTokenClientAsync(claimsIdentity, true, cancellationToken);
}
#endif
7 changes: 5 additions & 2 deletions src/Plugins/BotSharp.Plugin.MicrosoftTeams/Services/TeamsActivityBot.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ public TeamsActivityBot(

public override async Task OnTurnAsync(ITurnContext turnContext, CancellationToken cancellationToken = default)
{
_httpContextAccessor.HttpContext ??= new DefaultHttpContext();

var aadId = GetUserId(turnContext.Activity);
if (!string.IsNullOrEmpty(aadId))
{
Expand Down Expand Up @@ -250,7 +252,8 @@ private async Task SendWelcomeAsync(ITurnContext turnContext, CancellationToken

/// <summary>
/// Populates HttpContext.User with the BotSharp user's claims so that IUserIdentity
/// resolves correctly for all downstream scoped services during this turn.
/// resolves correctly for all downstream scoped services during this turn. The HttpContext
/// is the turn-owned one created in <see cref="OnTurnAsync"/> (the SDK provides none).
/// </summary>
private void SetHttpContextUser(User user)
{
Expand All @@ -264,7 +267,7 @@ private void SetHttpContextUser(User user)
new Claim(ClaimTypes.Role, user.Role ?? string.Empty),
};
var identity = new ClaimsIdentity(claims, authenticationType: "Teams");
_httpContextAccessor.HttpContext!.User = new System.Security.Principal.GenericPrincipal(identity, roles: null);
_httpContextAccessor.HttpContext!.User = new ClaimsPrincipal(identity);
}

private async Task<User?> FindUserByEmailAsync(string email, CancellationToken cancellationToken)
Expand Down
Loading