Skip to content
This repository was archived by the owner on Jan 5, 2026. It is now read-only.
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
7 changes: 4 additions & 3 deletions libraries/Microsoft.Bot.Builder/BotFrameworkAdapter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public class BotFrameworkAdapter : BotAdapter, IAdapterIntegration, IExtendedUse
private readonly ConcurrentDictionary<string, ConnectorClient> _connectorClients = new ConcurrentDictionary<string, ConnectorClient>();

// Cache for OAuthClient to speed up OAuth operations
// _oAuthClients is a cache using [appId + oAuthCredentialAppId]
// _oAuthClients is a cache using [appId + oAuthCredentialAppId + oAuthEndpoint]
private readonly ConcurrentDictionary<string, OAuthClient> _oAuthClients = new ConcurrentDictionary<string, OAuthClient>();

/// <summary>
Expand Down Expand Up @@ -1396,7 +1396,6 @@ protected virtual async Task<OAuthClient> CreateOAuthApiClientAsync(ITurnContext

var appId = GetBotAppId(turnContext);

var clientKey = $"{appId}:{oAuthAppCredentials?.MicrosoftAppId}";
var oAuthScope = GetBotFrameworkOAuthScope();

var appCredentials = oAuthAppCredentials ?? await GetAppCredentialsAsync(appId, oAuthScope).ConfigureAwait(false);
Expand All @@ -1408,6 +1407,8 @@ protected virtual async Task<OAuthClient> CreateOAuthApiClientAsync(ITurnContext
OAuthClientConfig.EmulateOAuthCards = true;
}

var oAuthEndpoint = OAuthClientConfig.OAuthEndpoint;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The assumption is that we should not be calling these from different threads as we are still caching a static variable. A context switch before this line could result in incorrect endpoint being used if two threads set it to different values. I wonder if the approach is to do better documentation or improve synchronization here? Thoughts?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, there will always be a race when using a static like this. As long as we're setting it globally via an FCB I don't think it will matter. For this PR, I wanted to minimize changes and get something that will allow us to change it globally. If we end up needing to set the value on a per-call basis then we'll need another PR to redesign the API and remove the static.

var clientKey = $"{appId}:{oAuthAppCredentials?.MicrosoftAppId}:{oAuthEndpoint}";
var oAuthClient = _oAuthClients.GetOrAdd(clientKey, (key) =>
{
OAuthClient oAuthClientInner;
Expand All @@ -1419,7 +1420,7 @@ protected virtual async Task<OAuthClient> CreateOAuthApiClientAsync(ITurnContext
}
else
{
oAuthClientInner = new OAuthClient(new Uri(OAuthClientConfig.OAuthEndpoint), appCredentials);
oAuthClientInner = new OAuthClient(new Uri(oAuthEndpoint), appCredentials);
}

return oAuthClientInner;
Expand Down
46 changes: 46 additions & 0 deletions tests/Microsoft.Bot.Builder.Tests/BotFrameworkAdapterOAuthTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,47 @@ namespace Microsoft.Bot.Builder.Tests
{
public class BotFrameworkAdapterOAuthTests
{
[Fact]
public async Task CreateOAuthClientWithDifferentEndpoints()
{
var mockAppCredentials = new MockAppCredentials();
var mockCredentialProvider = new Mock<ICredentialProvider>();
var adapter = new MockAdapter(mockCredentialProvider.Object, () => new TokenResponse());

var claimsIdentity = new ClaimsIdentity();
claimsIdentity.AddClaim(new Claim(AuthenticationConstants.AudienceClaim, "AppId"));

var turnStateCollection = new TurnContextStateCollection();
turnStateCollection.Add(BotAdapter.BotIdentityKey, claimsIdentity);

var turnContext = new Mock<ITurnContext>();
turnContext.SetupGet(x => x.Activity).Returns(new Activity());
turnContext.SetupGet(x => x.TurnState).Returns(turnStateCollection);

var oauthEndpoint1 = "https://foo.com";
OAuthClientConfig.OAuthEndpoint = oauthEndpoint1;
var client = await adapter.CallCreateOAuthApiClientAsync(turnContext.Object, mockAppCredentials);
Assert.NotNull(client);
Assert.Equal(new Uri(oauthEndpoint1), client.BaseUri);
Assert.Same(mockAppCredentials, client.Credentials);

// client2 should come from the cache so it should be the same object
var client2 = await adapter.CallCreateOAuthApiClientAsync(turnContext.Object, mockAppCredentials);
Assert.NotNull(client2);
Assert.Same(client, client2);
Assert.Equal(new Uri(oauthEndpoint1), client2.BaseUri);
Assert.Same(mockAppCredentials, client2.Credentials);

// Changing the OAuthEndpoint should result in a different OAuthClient
var oauthEndpoint2 = "https://bar.com";
OAuthClientConfig.OAuthEndpoint = oauthEndpoint2;
var client3 = await adapter.CallCreateOAuthApiClientAsync(turnContext.Object, mockAppCredentials);
Assert.NotNull(client3);
Assert.NotSame(client3, client);
Assert.Equal(new Uri(oauthEndpoint2), client3.BaseUri);
Assert.Same(mockAppCredentials, client3.Credentials);
}

[Fact]
public async Task BotFrameworkAdapterFindsOAuthCards()
{
Expand Down Expand Up @@ -345,6 +386,11 @@ public override Task<TokenResponse> GetUserTokenAsync(ITurnContext turnContext,
{
return Task.FromResult(_getUserTokenAction());
}

public Task<OAuthClient> CallCreateOAuthApiClientAsync(ITurnContext turnContext, AppCredentials oAuthAppCredentials)
{
return CreateOAuthApiClientAsync(turnContext, oAuthAppCredentials);
}
}

private class MockLogger : ILogger<BotFrameworkHttpAdapterBase>
Expand Down
23 changes: 23 additions & 0 deletions tests/Microsoft.Bot.Builder.Tests/MockAppCredentials.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using System.Net.Http;
using Microsoft.Bot.Connector.Authentication;
using Microsoft.Extensions.Logging;

namespace Microsoft.Bot.Builder.Tests
{
public class MockAppCredentials : AppCredentials
{
public MockAppCredentials(string channelAuthTenant = null, HttpClient customHttpClient = null, ILogger logger = null)
: base(channelAuthTenant, customHttpClient, logger)
{
}

protected override Lazy<AdalAuthenticator> BuildAuthenticator()
{
return new Lazy<AdalAuthenticator>();
}
}
}