Skip to content

Commit

Permalink
[C#] fix: auth bug fixes (#1981)
Browse files Browse the repository at this point in the history
## Linked issues

closes: #1744 #1933

## Details

Fixed bug in both #1744 and #1933. 

The issue does not persist in either JS, or PY so the fix is only for
C#.

#### Change details
* Refactored `FilteredTeamsSSOTokenExchangeMiddleware` as the underlying
middleware was not being invoked correctly.
* The `app.adapter.Use` method was being called for every incomming
request (since the `Application` object is a transient in the Asp.NET
service collection).
* As for the issue in #1744 - The code is updated to use
`DateTime.UtcNow` by default.

## Attestation Checklist

- [x] My code follows the style guidelines of this project

- I have checked for/fixed spelling, linting, and other errors
- I have commented my code for clarity
- I have made corresponding changes to the documentation (updating the
doc strings in the code is sufficient)
- My changes generate no new warnings
- I have added tests that validates my changes, and provides sufficient
test coverage. I have tested with:
  - Local testing
  - E2E testing in Teams
- New and existing unit tests pass locally with my changes

### Additional information

> Feel free to add other relevant information below
  • Loading branch information
singhk97 authored Sep 4, 2024
1 parent 5fa361c commit 71222f9
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public void Test_ApplicationBuilder_CustomSetup()
Moderator = new TestModerator()
};
AuthenticationOptions<TurnState> authOptions = new();
authOptions.AddAuthentication("graph", new OAuthSettings());
authOptions.AddAuthentication("graph", new OAuthSettings() { ConnectionName = "graph-connection" });

// Act
var app = new ApplicationBuilder<TurnState>()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public void Test_Application_CustomSetup()
{
AutoSignIn = (context, cancellationToken) => Task.FromResult(false)
};
authenticationOptions.AddAuthentication("graph", new OAuthSettings());
authenticationOptions.AddAuthentication("graph", new OAuthSettings() { ConnectionName = "graph-connection" });
ApplicationOptions<TurnState> applicationOptions = new()
{
RemoveRecipientMention = removeRecipientMention,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ public async void Test_VerifyStateRouteSelector_ReturnsTrue()
((JObject)turnContext.Activity.Value).Add("settingName", SETTING_NAME);

var app = new TestApplication(new() { Adapter = testAdapter });
var botAuth = new TestOAuthBotAuthentication(app, new(), SETTING_NAME);
var botAuth = new TestOAuthBotAuthentication(app, new() { ConnectionName = "connectionName" }, SETTING_NAME);

// Act
var result = await botAuth.VerifyStateRouteSelector(turnContext, default);
Expand All @@ -143,7 +143,7 @@ public async void Test_VerifyStateRouteSelector_IncorrectActivity_ReturnsFalse()
((JObject)turnContext.Activity.Value).Add("settingName", SETTING_NAME);

var app = new TestApplication(new() { Adapter = testAdapter });
var botAuth = new TestOAuthBotAuthentication(app, new(), SETTING_NAME);
var botAuth = new TestOAuthBotAuthentication(app, new() { ConnectionName = "connectionName" }, SETTING_NAME);

// Act
var result = await botAuth.VerifyStateRouteSelector(turnContext, default);
Expand All @@ -164,7 +164,7 @@ public async void Test_VerifyStateRouteSelector_IncorrectInvokeName_ReturnsFalse
((JObject)turnContext.Activity.Value).Add("settingName", SETTING_NAME);

var app = new TestApplication(new() { Adapter = testAdapter });
var botAuth = new TestOAuthBotAuthentication(app, new(), SETTING_NAME);
var botAuth = new TestOAuthBotAuthentication(app, new() { ConnectionName = "connectionName" }, SETTING_NAME);

// Act
var result = await botAuth.VerifyStateRouteSelector(turnContext, default);
Expand All @@ -185,7 +185,7 @@ public async void Test_VerifyStateRouteSelector_IncorrectSettingName_ReturnsFals
((JObject)turnContext.Activity.Value).Add("settingName", "NOT SETTING_NAME");

var app = new TestApplication(new() { Adapter = testAdapter });
var botAuth = new TestOAuthBotAuthentication(app, new(), SETTING_NAME);
var botAuth = new TestOAuthBotAuthentication(app, new() { ConnectionName = "connectionName" }, SETTING_NAME);

// Act
var result = await botAuth.VerifyStateRouteSelector(turnContext, default);
Expand All @@ -206,7 +206,7 @@ public async void Test_TokenExchangeRouteSelector_ReturnsTrue()
((JObject)turnContext.Activity.Value).Add("settingName", SETTING_NAME);

var app = new TestApplication(new() { Adapter = testAdapter });
var botAuth = new TestOAuthBotAuthentication(app, new(), SETTING_NAME);
var botAuth = new TestOAuthBotAuthentication(app, new() { ConnectionName = "connectionName" }, SETTING_NAME);

// Act
var result = await botAuth.TokenExchangeRouteSelector(turnContext, default);
Expand All @@ -227,7 +227,7 @@ public async void Test_TokenExchangeRouteSelector_IncorrectActivity_ReturnsFalse
((JObject)turnContext.Activity.Value).Add("settingName", SETTING_NAME);

var app = new TestApplication(new() { Adapter = testAdapter });
var botAuth = new TestOAuthBotAuthentication(app, new(), SETTING_NAME);
var botAuth = new TestOAuthBotAuthentication(app, new() { ConnectionName = "connectionName" }, SETTING_NAME);

// Act
var result = await botAuth.TokenExchangeRouteSelector(turnContext, default);
Expand All @@ -248,7 +248,7 @@ public async void Test_TokenExchangeRouteSelector_IncorrectInvokeName_ReturnsFal
((JObject)turnContext.Activity.Value).Add("settingName", SETTING_NAME);

var app = new TestApplication(new() { Adapter = testAdapter });
var botAuth = new TestOAuthBotAuthentication(app, new(), SETTING_NAME);
var botAuth = new TestOAuthBotAuthentication(app, new() { ConnectionName = "connectionName" }, SETTING_NAME);

// Act
var result = await botAuth.TokenExchangeRouteSelector(turnContext, default);
Expand All @@ -269,7 +269,7 @@ public async void Test_TokenExchangeRouteSelector_IncorrectSettingName_ReturnsFa
((JObject)turnContext.Activity.Value).Add("settingName", "NOT SETTING_NAME");

var app = new TestApplication(new() { Adapter = testAdapter });
var botAuth = new TestOAuthBotAuthentication(app, new(), SETTING_NAME);
var botAuth = new TestOAuthBotAuthentication(app, new() { ConnectionName = "connectionName" }, SETTING_NAME);

// Act
var result = await botAuth.TokenExchangeRouteSelector(turnContext, default);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,37 +1,49 @@
using Microsoft.Bot.Builder;
using Microsoft.Bot.Builder.Teams;
using Microsoft.Bot.Connector;
using Microsoft.Bot.Schema;
using Microsoft.Teams.AI.Exceptions;
using Newtonsoft.Json.Linq;

namespace Microsoft.Teams.AI.Application.Authentication.Bot
{
internal class FilteredTeamsSSOTokenExchangeMiddleware : TeamsSSOTokenExchangeMiddleware
internal class FilteredTeamsSSOTokenExchangeMiddleware : IMiddleware
{
private string _oauthConnectionName;
private TeamsSSOTokenExchangeMiddleware tokenExchangeMiddleware;

public FilteredTeamsSSOTokenExchangeMiddleware(IStorage storage, string oauthConnectionName) : base(storage, oauthConnectionName)
public FilteredTeamsSSOTokenExchangeMiddleware(IStorage storage, string oauthConnectionName)
{
this.tokenExchangeMiddleware = new TeamsSSOTokenExchangeMiddleware(storage, oauthConnectionName);
this._oauthConnectionName = oauthConnectionName;
}

public new async Task OnTurnAsync(ITurnContext turnContext, NextDelegate next, CancellationToken cancellationToken = default)
public async Task OnTurnAsync(ITurnContext turnContext, NextDelegate next, CancellationToken cancellationToken = default)
{
if (string.Equals(Channels.Msteams, turnContext.Activity.ChannelId, StringComparison.OrdinalIgnoreCase)
&& string.Equals(SignInConstants.TokenExchangeOperationName, turnContext.Activity.Name, StringComparison.OrdinalIgnoreCase))
{
string? connectionName = _GetConnectionName(turnContext);

// If connection name matches then continue to the Teams SSO Token Exchange Middleware.
if (connectionName == this._oauthConnectionName)
{
await tokenExchangeMiddleware.OnTurnAsync(turnContext, next, cancellationToken).ConfigureAwait(false);
return;
}
}

await next(cancellationToken).ConfigureAwait(false);
}

private string? _GetConnectionName(ITurnContext turnContext)
{
JObject? obj = turnContext.Activity.Value as JObject;
if (obj == null)
{
throw new TeamsAIException("Excepted `turnContext.Activity.Value` to have `connectionName` property");
};
string? connectionName = obj.Value<string>("connectionName");

// If connection name matches then continue to the Teams SSO Token Exchange Middleware.
if (connectionName == this._oauthConnectionName)
{
await base.OnTurnAsync(turnContext, next, cancellationToken);
}
else
{
await next(cancellationToken);
}
return obj.Value<string>("connectionName");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ public OAuthBotAuthentication(Application<TState> app, OAuthSettings oauthSettin
this._oauthPrompt = new OAuthPrompt("OAuthPrompt", this._oauthSettings);

// Handles deduplication of token exchange event when using SSO with Bot Authentication
app.Adapter.Use(new FilteredTeamsSSOTokenExchangeMiddleware(storage ?? new MemoryStorage(), settingName));
if (!IsTokenExchangeMiddlewareRegistered(app))
{
app.Adapter.Use(new FilteredTeamsSSOTokenExchangeMiddleware(storage ?? new MemoryStorage(), oauthSettings.ConnectionName));
}
}

/// <summary>
Expand Down Expand Up @@ -115,9 +118,14 @@ public async Task<Attachment> CreateOAuthCard(ITurnContext context, Cancellation
};
}

protected async virtual Task<SignInResource> GetSignInResourceAsync(ITurnContext context, string connectionName, CancellationToken cancellationToken = default)
protected virtual async Task<SignInResource> GetSignInResourceAsync(ITurnContext context, string connectionName, CancellationToken cancellationToken = default)
{
return await UserTokenClientWrapper.GetSignInResourceAsync(context, this._oauthSettings.ConnectionName, cancellationToken);
}

private bool IsTokenExchangeMiddlewareRegistered(Application<TState> app)
{
return app.Adapter.MiddlewareSet.Where(middleWare => middleWare as FilteredTeamsSSOTokenExchangeMiddleware is not null).Count() > 0;
}
}
}

0 comments on commit 71222f9

Please sign in to comment.