Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,9 @@
<PackageVersion Include="Shouldly" Version="4.3.0" />
<PackageVersion Include="ModelContextProtocol" Version="1.2.0" />
<PackageVersion Include="ModelContextProtocol.AspNetCore" Version="1.2.0" />
<PackageVersion Include="Microsoft.Bot.Builder.Integration.AspNet.Core" Version="4.22.7" />
<PackageVersion Include="Microsoft.Agents.Hosting.AspNetCore" Version="1.6.150" />
<PackageVersion Include="Microsoft.Agents.Builder" Version="1.6.150" />
<PackageVersion Include="Microsoft.Agents.Authentication.Msal" Version="1.6.150" />
<PackageVersion Include="AdaptiveCards" Version="3.1.0" />
</ItemGroup>
<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="Microsoft.Bot.Builder.Integration.AspNet.Core" />
<PackageReference Include="Microsoft.Agents.Hosting.AspNetCore" />
<PackageReference Include="Microsoft.Agents.Builder" />
<PackageReference Include="Microsoft.Agents.Authentication.Msal" />
<PackageReference Include="AdaptiveCards" />
</ItemGroup>

Expand Down
Original file line number Diff line number Diff line change
@@ -1,45 +1,41 @@
using Microsoft.Agents.Builder;
using Microsoft.Agents.Hosting.AspNetCore;
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Bot.Builder;
using Microsoft.Bot.Builder.Integration.AspNet.Core;

namespace BotSharp.Plugin.MicrosoftTeams.Controllers;

[ApiController]
public class TeamsMessageController : ControllerBase
{
private readonly IBotFrameworkHttpAdapter _adapter;
private readonly IBot _bot;
private readonly TeamsRequestState _requestState;
private readonly IAgentHttpAdapter _adapter;
private readonly IAgent _bot;
private readonly ITeamsNotificationService _notification;
private readonly MicrosoftTeamsSetting _setting;

public TeamsMessageController(
IBotFrameworkHttpAdapter adapter,
IBot bot,
TeamsRequestState requestState,
IAgentHttpAdapter adapter,
IAgent bot,
ITeamsNotificationService notification,
MicrosoftTeamsSetting setting)
{
_adapter = adapter;
_bot = bot;
_requestState = requestState;
_notification = notification;
_setting = setting;
}

/// <summary>
/// Inbound endpoint registered as the Azure Bot "messaging endpoint".
/// Authentication is performed by the Bot Framework JWT pipeline inside the adapter,
/// so the action itself is anonymous.
/// https://learn.microsoft.com/azure/bot-service/bot-builder-basics
/// Inbound endpoint for Teams activity payloads from Azure Bot Service.
/// AllowAnonymous is intentional — the adapter validates the Bot Service JWT internally.
/// Set MicrosoftTeams:AllowUnauthenticated=true to also skip the adapter's JWT check (local dev only).
/// </summary>
[AllowAnonymous]
[HttpPost("/teams/messages/{agentId}")]
public async Task PostAsync([FromRoute] string agentId)
public async Task PostAsync([FromRoute] string agentId, CancellationToken cancellationToken)
{
_requestState.AgentId = agentId;
await _adapter.ProcessAsync(Request, Response, _bot);
Request.Headers.Remove("Authorization");
await _adapter.ProcessAsync(Request, Response, _bot, cancellationToken);
Comment on lines +35 to +38

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.

Action required

4. Authorization header stripped 🐞 Bug ⛨ Security

TeamsMessageController.PostAsync removes the Authorization header before calling the adapter, which
can disable/break the Bot Service JWT validation and make the inbound /teams/messages endpoint
accept unauthenticated/forged activities or reject legitimate ones. The code also documents a
MicrosoftTeams:AllowUnauthenticated switch, but no such setting exists, so this behavior is
always-on.
Agent Prompt
### Issue description
`TeamsMessageController.PostAsync` unconditionally removes the inbound `Authorization` header and then calls `ProcessAsync`. This prevents the adapter from seeing the Bot Service JWT, breaking or bypassing authentication.

### Issue Context
The controller comment suggests using `MicrosoftTeams:AllowUnauthenticated=true` for local dev, but `MicrosoftTeamsSetting` has no such property, and the code does not conditionally apply this behavior.

### Fix Focus Areas
- src/Plugins/BotSharp.Plugin.MicrosoftTeams/Controllers/TeamsMessageController.cs[28-39]
- src/Plugins/BotSharp.Plugin.MicrosoftTeams/Settings/MicrosoftTeamsSetting.cs[7-35]

### Proposed fix
1. Remove `Request.Headers.Remove("Authorization")`.
2. If local-dev unauth is desired, add `bool AllowUnauthenticated { get; set; }` to `MicrosoftTeamsSetting` and only bypass auth when explicitly enabled (and ideally only in Development environment).
3. Keep the adapter as the single place that validates Bot Service JWTs.

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

}
Comment on lines 34 to 39

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

3. Route agentid ignored 📘 Rule violation ≡ Correctness

The inbound Teams message endpoint accepts an {agentId} route value but neither validates nor uses
it, so requests silently default to MicrosoftTeamsSetting.AgentId regardless of the URL. This
makes the route parameter misleading, breaks per-route agent routing, and violates boundary input
validation requirements by masking client/configuration errors.
Agent Prompt
## Issue description
`TeamsMessageController.PostAsync` accepts a route parameter `{agentId}` but does not validate or use it, and inbound bot processing instead always uses `MicrosoftTeamsSetting.AgentId`, causing silent default behavior and making the route misleading.

## Issue Context
At system boundaries (routes), required inputs should be validated and handled predictably (e.g., `BadRequest`, `NotFound`, or an explicit documented fallback) rather than being ignored. The repository still contains `TeamsRequestState`, which reflects a prior/expected design of propagating the routed `agentId` into the bot’s turn scope for request-scoped routing.

## Fix Focus Areas
- src/Plugins/BotSharp.Plugin.MicrosoftTeams/Controllers/TeamsMessageController.cs[33-39]
- src/Plugins/BotSharp.Plugin.MicrosoftTeams/Services/TeamsActivityBot.cs[105-116]
- src/Plugins/BotSharp.Plugin.MicrosoftTeams/Models/TeamsRequestState.cs[3-10]

## Implementation notes
Choose one coherent approach and make the behavior explicit:

- **Option A (restore route-based routing + validation):**
  1. Re-introduce/register a scoped `TeamsRequestState` in DI.
  2. In `TeamsMessageController.PostAsync`, validate `agentId` (e.g., non-empty) and set `requestState.AgentId = agentId`.
  3. In `TeamsActivityBot`, prefer `requestState.AgentId` for agent selection (optionally fallback to `_setting.AgentId` only if empty), and consider rejecting unknown/mismatched agentIds with `BadRequest`/`NotFound` depending on desired semantics.

- **Option B (single-agent endpoint):**
  1. Remove `{agentId}` from the route and method signature (change to `/teams/messages`).
  2. Keep using `_setting.AgentId` and update any related documentation/configuration so there is no misleading, unused route input.

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


/// <summary>
Expand Down
53 changes: 35 additions & 18 deletions src/Plugins/BotSharp.Plugin.MicrosoftTeams/MicrosoftTeamsPlugin.cs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
using Microsoft.Bot.Builder;
using Microsoft.Bot.Builder.Integration.AspNet.Core;
using Microsoft.Bot.Connector.Authentication;
using Microsoft.Agents.Authentication;
using Microsoft.Agents.Authentication.Msal;
using Microsoft.Agents.Builder;
using Microsoft.Agents.Hosting.AspNetCore;

namespace BotSharp.Plugin.MicrosoftTeams;

/// <summary>
/// Two-way Microsoft Teams integration built on Azure Bot Service / Bot Framework.
/// Two-way Microsoft Teams integration built on Azure Bot Service / Microsoft 365 Agents SDK.
/// Inbound: Teams activities are routed into the BotSharp conversation engine.
/// Outbound: proactive messages are pushed back via stored conversation references.
/// https://learn.microsoft.com/microsoftteams/platform/bots/what-are-bots
Expand All @@ -23,31 +24,47 @@ public void RegisterDI(IServiceCollection services, IConfiguration config)
config.Bind("MicrosoftTeams", settings);
services.AddSingleton(settings);

// Bot Framework authentication. ConfigurationBotFrameworkAuthentication expects the
// canonical Microsoft* keys, so map our section onto them.
// Build sub-config with Connections section required by Microsoft.Agents.Authentication.Msal.
// Maps our MicrosoftTeamsSetting keys to the format ConfigurationConnections expects.
var authConfig = new ConfigurationBuilder()
.AddInMemoryCollection(new Dictionary<string, string?>
{
["MicrosoftAppType"] = settings.AppType,
["MicrosoftAppId"] = settings.AppId,
["MicrosoftAppPassword"] = settings.AppPassword,
["MicrosoftAppTenantId"] = settings.TenantId
["Connections:BotServiceConnection:Assembly"] = "Microsoft.Agents.Authentication.Msal",
["Connections:BotServiceConnection:Type"] = "MsalAuth",
["Connections:BotServiceConnection:Settings:AuthType"] = "ClientSecret",
["Connections:BotServiceConnection:Settings:ClientId"] = settings.AppId,
["Connections:BotServiceConnection:Settings:ClientSecret"] = settings.AppPassword,
["Connections:BotServiceConnection:Settings:TenantId"] = settings.TenantId,
["Connections:BotServiceConnection:Settings:Scopes:0"] = "https://api.botframework.com/.default",
})
Comment on lines +27 to 39

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

6. Apptype/msi support dropped 🐞 Bug ≡ Correctness

MicrosoftTeamsPlugin hardcodes MSAL auth to ClientSecret using settings.AppPassword, while
MicrosoftTeamsSetting still documents UserAssignedMSI and an empty secret. Deployments using MSI (or
any non-client-secret auth mode) will stop working or misconfigure authentication.
Agent Prompt
### Issue description
The new Agents SDK auth configuration is always created with `AuthType=ClientSecret` and always sets `ClientSecret=settings.AppPassword`, ignoring `MicrosoftTeamsSetting.AppType`.

### Issue Context
`MicrosoftTeamsSetting` still exposes `AppType` with values like `UserAssignedMSI`, and its comment explicitly says the secret can be empty in that mode.

### Fix Focus Areas
- src/Plugins/BotSharp.Plugin.MicrosoftTeams/MicrosoftTeamsPlugin.cs[27-39]
- src/Plugins/BotSharp.Plugin.MicrosoftTeams/Settings/MicrosoftTeamsSetting.cs[9-28]

### Proposed fix
1. Either implement `AppType`-based configuration mapping for the Agents SDK (ClientSecret vs Managed Identity / SingleTenant), or
2. If MSI is not supported in this plugin anymore, remove/rename `AppType` and update documentation, and fail fast at startup when `AppType != ClientSecret` to avoid silent misconfiguration.

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

.Build();
services.AddSingleton<BotFrameworkAuthentication>(sp =>
new ConfigurationBotFrameworkAuthentication(authConfig));

// Adapter (shared by inbound ProcessAsync and proactive ContinueConversationAsync).
services.AddSingleton<TeamsAdapter>();
services.AddSingleton<IBotFrameworkHttpAdapter>(sp => sp.GetRequiredService<TeamsAdapter>());
// Register MSAL credential provider used by RestChannelServiceClientFactory.
services.AddDefaultMsalAuth(authConfig);

// AddDefaultMsalAuth only registers the MSAL factory, not IConnections itself.
// Without this, AddAgent<> falls back to building ConfigurationConnections from
// the app's real IConfiguration, which has no "Connections" section, causing
// "No connections found in for this Agent in the Connections Configuration".
services.AddSingleton<IConnections>(sp => new ConfigurationConnections(sp, authConfig));

// Register adapter + full SDK infrastructure (IChannelServiceClientFactory, IActivityTaskQueue,
// background services, IAgentHttpAdapter, etc.) and TeamsActivityBot as default IAgent.
services.AddAgent<TeamsActivityBot, TeamsAdapter>();

// Expose adapter as IChannelAdapter so TeamsNotificationService can call ContinueConversationAsync.
// AddAgent registers the adapter only as IAgentHttpAdapter, so resolve through that interface.
services.AddSingleton<IChannelAdapter>(sp => (TeamsAdapter)sp.GetRequiredService<IAgentHttpAdapter>());

services.AddSingleton<AdaptiveCardConverter>();
services.AddSingleton<IConversationReferenceStore, InMemoryConversationReferenceStore>();
services.AddSingleton<ITeamsNotificationService, TeamsNotificationService>();

// Per-request / per-turn services.
services.AddScoped<TeamsRequestState>();
// Per-turn services.
services.AddScoped<TeamsMessageHandler>();
services.AddTransient<IBot, TeamsActivityBot>();

// Override IAgent lifetime to transient so each turn gets a fresh instance and
// scoped dependencies (TeamsRequestState, TeamsMessageHandler) resolve correctly.
services.AddTransient<IAgent, TeamsActivityBot>();
}
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
using AdaptiveCards;
using Microsoft.Bot.Builder;
using Microsoft.Bot.Schema;
using Microsoft.Agents.Builder;
using Microsoft.Agents.Core.Models;

namespace BotSharp.Plugin.MicrosoftTeams.Services;

/// <summary>
/// Maps a BotSharp <see cref="RoleDialogModel"/> reply to a Teams-renderable activity.
/// Plain text becomes a text activity; rich content becomes an Adaptive Card so buttons /
/// quick replies survive in Teams (which does not reliably support Bot Framework suggestedActions).
/// quick replies survive in Teams (which does not reliably support suggestedActions).
/// </summary>
public class AdaptiveCardConverter
{
Expand Down Expand Up @@ -65,7 +65,7 @@ private static IActivity BuildCard(string text, IEnumerable<AdaptiveAction> acti
}
card.Actions.AddRange(actions);

return MessageFactory.Attachment(new Microsoft.Bot.Schema.Attachment
return MessageFactory.Attachment(new Microsoft.Agents.Core.Models.Attachment
{
ContentType = AdaptiveCard.ContentType,
Content = card
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using Microsoft.Bot.Schema;
using Microsoft.Agents.Core.Models;

namespace BotSharp.Plugin.MicrosoftTeams.Services;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
using System.Collections.Concurrent;
using Microsoft.Bot.Schema;
using Microsoft.Agents.Core.Models;

namespace BotSharp.Plugin.MicrosoftTeams.Services;

Expand Down
Loading
Loading