Security hardening: token validation and service URL improvements by corinagum · Pull Request #418 · microsoft/teams.net

corinagum · 2026-04-08T22:40:31Z

Summary

Security hardening for token validation, service URL handling, and development tooling.

Service URL validation: Validate inbound ServiceUrl against a known domain allowlist before
using it for outbound API calls. Configurable via AdditionalAllowedDomains and SkipServiceUrlValidation
on AppOptions.
Issuer validation: Log a warning when Entra token validation is configured without a tenant ID,
making the silent issuer validation skip visible.
DevTools: Prevent the DevTools plugin from starting in production environments.

Test plan

Unit tests for ServiceUrlValidator (20 tests: allowed domains, rejected domains, localhost,
empty/null, invalid URLs, additional domains)
E2E validated in Teams — no regressions
E2E validated DevTools blocked on ASPNETCORE_ENVIRONMENT=Production

rido-min · 2026-04-14T20:59:04Z

Configurable via AdditionalAllowedDomains and SkipServiceUrlValidation
on AppOptions

These two options seems overlaping, we could skip service UrlValidation by calling AdditionalAllowedDomains with a magic string such as *.

Wondering if we could override the addtional domains from config

…visisble

Copilot

Pull request overview

Security hardening across the Teams app runtime and ASP.NET Core plugins by validating inbound service URLs before using them for outbound Bot Framework calls, improving visibility into Entra issuer-validation configuration, and preventing DevTools from running in production.

Changes:

Add ServiceUrlValidator and enforce service URL allowlist validation during App.Process(...).
Extend CloudEnvironment with per-cloud allowed service URL hostnames and plumb AdditionalAllowedDomains from configuration into AppOptions.
Add production-environment blocking for DevTools and emit a warning when Entra validation is configured without a tenant ID.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
Tests/Microsoft.Teams.Apps.Tests/ServiceUrlValidatorTests.cs	Adds unit tests covering allowed/rejected service URL scenarios.
Libraries/Microsoft.Teams.Plugins/Microsoft.Teams.Plugins.AspNetCore/Extensions/TeamsValidationSettings.cs	Adds a warning path when tenant ID is missing for Entra issuer validation.
Libraries/Microsoft.Teams.Plugins/Microsoft.Teams.Plugins.AspNetCore.DevTools/DevToolsPlugin.cs	Blocks DevTools initialization in Production via `IHostEnvironment`.
Libraries/Microsoft.Teams.Extensions/Microsoft.Teams.Extensions.Configuration/Microsoft.Teams.Apps.Extensions/TeamsSettings.cs	Adds `AdditionalAllowedDomains` configuration and applies it to `AppOptions`.
Libraries/Microsoft.Teams.Apps/ServiceUrlValidator.cs	Introduces service URL hostname validation against cloud defaults + additional domains.
Libraries/Microsoft.Teams.Apps/AppOptions.cs	Adds `AdditionalAllowedDomains` option for runtime configuration.
Libraries/Microsoft.Teams.Apps/App.cs	Enforces service URL validation before creating `ConversationReference` used for outbound calls.
Libraries/Microsoft.Teams.Api/Auth/CloudEnvironment.cs	Adds `AllowedServiceUrls` and populates defaults per cloud; extends `WithOverrides`.

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

Copilot · 2026-04-17T19:11:17Z

+using Microsoft.Teams.Api.Auth;
+
+namespace Microsoft.Teams.Apps.Tests;
+
+public class ServiceUrlValidatorTests
+{
+    // --- Public cloud ---
+
+    [Theory]
+    [InlineData("https://smba.trafficmanager.net/teams/")]
+    [InlineData("https://smba.trafficmanager.net/amer/")]
+    [InlineData("https://smba.onyx.prod.teams.trafficmanager.net")]
+    public void IsAllowed_AcceptsPublicCloudDomains(string serviceUrl)
+    {
+        Assert.True(ServiceUrlValidator.IsAllowed(serviceUrl, CloudEnvironment.Public));


This test file references ServiceUrlValidator but does not import the Microsoft.Teams.Apps namespace, and there is no global using Microsoft.Teams.Apps; in the test project. This will fail to compile; add the missing using (or fully-qualify the type).

Copilot · 2026-04-17T19:11:17Z

+    /// plus any additional domains provided by the caller.
+    /// Localhost is always allowed for local development.
+    /// </summary>
+    public static bool IsAllowed(string serviceUrl, CloudEnvironment cloud, IEnumerable<string>? additionalDomains = null)


IsAllowed takes a non-nullable string serviceUrl but the implementation explicitly handles null/empty. Consider changing the parameter type to string? to match behavior and avoid forcing callers/tests to use null-suppression.

Suggested change

public static bool IsAllowed(string serviceUrl, CloudEnvironment cloud, IEnumerable<string>? additionalDomains = null)

public static bool IsAllowed(string? serviceUrl, CloudEnvironment cloud, IEnumerable<string>? additionalDomains = null)

Copilot · 2026-04-17T19:11:17Z

+
+        if (hostname is "localhost" or "127.0.0.1")
+            return true;
+


Service URL validation currently does not enforce an https scheme (any absolute URI with an allowed host passes). Since this value is used for outbound calls, consider requiring uri.Scheme == https (except perhaps localhost during development) to avoid accepting insecure endpoints.

Suggested change

if (!string.Equals(uri.Scheme, Uri.UriSchemeHttps, StringComparison.OrdinalIgnoreCase))

return false;

Copilot · 2026-04-17T19:11:18Z

+        var serviceUrl = @event.Activity.ServiceUrl ?? @event.Token.ServiceUrl;
+        if (!ServiceUrlValidator.IsAllowed(serviceUrl, _cloud, _additionalAllowedDomains))
+        {
+            Logger.Warn($"Rejected service URL: {serviceUrl}");
+            throw new InvalidOperationException("Service URL is not from an allowed domain");
+        }


PR description mentions a SkipServiceUrlValidation option on AppOptions, but this change set only adds AdditionalAllowedDomains and enforces validation unconditionally in App.Process. Either add the documented option (and honor it here) or update the PR description to match the implementation.

Copilot · 2026-04-17T19:11:18Z

+
+        if (hostname is "localhost" or "127.0.0.1")
+            return true;
+
+        var additional = additionalDomains?.ToList() ?? [];


localhost/127.0.0.1 are always allowed, regardless of environment or additional-domain configuration. If the goal is SSRF hardening for production workloads, consider gating localhost allowance behind an explicit opt-in (e.g., additionalDomains contains localhost, or an environment/development flag).

Suggested change

if (hostname is "localhost" or "127.0.0.1")

return true;

var additional = additionalDomains?.ToList() ?? [];

var additional = additionalDomains?.ToList() ?? [];

if (hostname is "localhost" or "127.0.0.1")

return additional.Any(domain => hostname == domain.ToLowerInvariant());

Copilot · 2026-04-17T19:11:18Z

+        var additional = additionalDomains?.ToList() ?? [];
+        if (additional.Contains("*"))
+            return true;
+
+        // Check against cloud environment's allowed FQDNs
+        if (cloud.AllowedServiceUrls.Any(allowed => hostname == allowed.ToLowerInvariant()))
+            return true;
+
+        // Check against additional domains (suffix match)
+        return additional.Any(domain => hostname.EndsWith(domain.ToLowerInvariant()));
+    }


The additional-domain check uses a raw EndsWith(...) on the provided string. If a caller configures custom-channel.com (without a leading dot), hosts like evilcustom-channel.com would match. Consider treating entries without a leading dot as exact hostnames (or enforce a dot-boundary when doing suffix matches).

Copilot · 2026-04-17T19:11:18Z

        OpenIdMetadataUrl = openIdMetadataUrl;
        TokenIssuer = tokenIssuer;
        GraphScope = graphScope;
+        AllowedServiceUrls = allowedServiceUrls ?? [];


AllowedServiceUrls is typed as IReadOnlyList<string> but is assigned directly from a mutable string[]. Since arrays implement IReadOnlyList<T>, consumers can downcast and mutate the allowlist (including the static cloud instances). Consider wrapping/copying (e.g., Array.AsReadOnly(allowedServiceUrls.ToArray())) to keep the allowlist immutable.

Suggested change

AllowedServiceUrls = allowedServiceUrls ?? [];

AllowedServiceUrls = Array.AsReadOnly(allowedServiceUrls is null ? Array.Empty<string>() : (string[])allowedServiceUrls.Clone());

Copilot · 2026-04-17T19:11:19Z

+        var hostEnvironment = _services.GetService<IHostEnvironment>();
+        if (hostEnvironment?.IsProduction() == true)
+        {
+            throw new InvalidOperationException(
+                "Devtools plugin cannot be used in production environments. " +
+                "Remove the devtools plugin from your app configuration.");
+        }


DevToolsPlugin only blocks production when IHostEnvironment is available in DI. If it’s missing, the plugin will still start, which undermines the intended safeguard. Consider using GetRequiredService<IHostEnvironment>() (and failing closed) or another deterministic production check.

Copilot · 2026-04-17T19:11:19Z

+        else
+        {
+            // When no tenant ID is configured, issuer validation will be skipped.
+            // This accepts tokens from any Azure AD tenant.
+            System.Diagnostics.Trace.TraceWarning(
+                "No tenant ID provided for Entra token validation. " +
+                "Issuer validation will be skipped, accepting tokens from any tenant.");
+        }


This warning uses System.Diagnostics.Trace.TraceWarning, which often won’t flow into ASP.NET Core structured logging by default, so the message may not be visible in real deployments. Consider emitting the warning via ILogger (e.g., from HostApplicationBuilder.AddTeamsTokenAuthentication where logging is available) instead of Trace.

rido-min reviewed Apr 14, 2026

View reviewed changes

Comment thread Libraries/Microsoft.Teams.Apps/ServiceUrlValidator.cs Outdated

corinagum force-pushed the cg/security branch from 400052a to 21d76b7 Compare April 14, 2026 23:27

corinagum mentioned this pull request Apr 14, 2026

Docs: Service URL validation and DevTools production guard microsoft/teams-sdk#2743

Open

3 tasks

corinagum added 7 commits April 16, 2026 09:09

Log a warning when token validation has no tenantId, making the skip …

ecc642a

…visisble

Block DevTools from starting when in production env

9fae030

Adds a domain allowList that rejects serviceUrls not from known domains

4ac0616

Narrow scope for trafficmanager.net

36bedc2

Allow setting AdditionalAllowedDomains in appsettings

d39e6fb

Remove SkipServiceUrlValidation

e8a6b90

Source service urls from CloudEnvironment

d379743

corinagum force-pushed the cg/security branch from 21d76b7 to d379743 Compare April 16, 2026 16:42

Avoid repeating rejected service urls

73b5346

corinagum marked this pull request as ready for review April 17, 2026 19:05

Copilot AI review requested due to automatic review settings April 17, 2026 19:05

Copilot started reviewing on behalf of corinagum April 17, 2026 19:06 View session

Copilot AI reviewed Apr 17, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Security hardening: token validation and service URL improvements#418

Security hardening: token validation and service URL improvements#418
corinagum wants to merge 8 commits intomainfrom
cg/security

corinagum commented Apr 8, 2026

Uh oh!

rido-min commented Apr 14, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	public static bool IsAllowed(string serviceUrl, CloudEnvironment cloud, IEnumerable<string>? additionalDomains = null)
	public static bool IsAllowed(string? serviceUrl, CloudEnvironment cloud, IEnumerable<string>? additionalDomains = null)



	if (!string.Equals(uri.Scheme, Uri.UriSchemeHttps, StringComparison.OrdinalIgnoreCase))
	return false;

	AllowedServiceUrls = allowedServiceUrls ?? [];
	AllowedServiceUrls = Array.AsReadOnly(allowedServiceUrls is null ? Array.Empty<string>() : (string[])allowedServiceUrls.Clone());

Conversation

corinagum commented Apr 8, 2026

Summary

Test plan

Uh oh!

rido-min commented Apr 14, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants