Skip to content

Commit 1065b55

Browse files
blackchoeysinghk97
blackchoey
authored and
singhk97
committed
[C#] refactor: fix warnings and issues in code (#944)
## Linked issues closes: #861 ## Details 1. Fix warnings in the code 2. Add cancellation token parameter to interface 3. Mark bot/message extension/adaptive card base class as internal - we can change them to public in the future if we want to developers to leverage them. For now, they should be internal implementation. 4. Add `Async` to function names 5. Remove `TeamsSsoAdaptiveCardAuthentication` class as it's not supported temporary 6. Fix a bug that cannot read token from MSAL OBO token cache by using the `ILongRunningWebApi` interface 7. Fix a bug that token exchange route selector may throw exception ## 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 (we use [TypeDoc](https://typedoc.org/) to document our code) - 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
1 parent 45ffbe0 commit 1065b55

19 files changed

+127
-100
lines changed

‎dotnet/packages/Microsoft.TeamsAI/Microsoft.TeamsAI.Tests/Application/Authentication/AuthenticationManagerTests.cs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ public async void Test_SignIn_DefaultHandler()
2323
var turnState = await TurnStateConfig.GetTurnStateWithConversationStateAsync(turnContext);
2424

2525
// act
26-
var response = await authManager.SignUserIn(turnContext, turnState);
26+
var response = await authManager.SignUserInAsync(turnContext, turnState);
2727

2828
// assert
2929
Assert.Equal(SignInStatus.Complete, response.Status);
@@ -48,7 +48,7 @@ public async void Test_SignIn_SpecificHandler()
4848
var turnState = await TurnStateConfig.GetTurnStateWithConversationStateAsync(turnContext);
4949

5050
// act
51-
var response = await authManager.SignUserIn(turnContext, turnState, "sharepoint");
51+
var response = await authManager.SignUserInAsync(turnContext, turnState, "sharepoint");
5252

5353
// assert
5454
Assert.Equal(SignInStatus.Complete, response.Status);
@@ -72,7 +72,7 @@ public async void Test_SignIn_Pending()
7272
var turnState = await TurnStateConfig.GetTurnStateWithConversationStateAsync(turnContext);
7373

7474
// act
75-
var response = await authManager.SignUserIn(turnContext, turnState);
75+
var response = await authManager.SignUserInAsync(turnContext, turnState);
7676

7777
// assert
7878
Assert.Equal(SignInStatus.Pending, response.Status);
@@ -99,7 +99,7 @@ public async void Test_SignOut_DefaultHandler()
9999
};
100100

101101
// act
102-
await authManager.SignOutUser(turnContext, turnState);
102+
await authManager.SignOutUserAsync(turnContext, turnState);
103103

104104
// assert
105105
Assert.False(turnState.Temp.AuthTokens.ContainsKey("graph"));
@@ -126,7 +126,7 @@ public async void Test_SignOut_SpecificHandler()
126126
};
127127

128128
// act
129-
await authManager.SignOutUser(turnContext, turnState, "sharepoint");
129+
await authManager.SignOutUserAsync(turnContext, turnState, "sharepoint");
130130

131131
// assert
132132
Assert.False(turnState.Temp.AuthTokens.ContainsKey("sharepoint"));
@@ -147,7 +147,7 @@ public async void Test_IsValidActivity_DefaultHandler()
147147
var turnContext = MockTurnContext();
148148

149149
// act
150-
var validActivity = await authManager.IsValidActivity(turnContext);
150+
var validActivity = await authManager.IsValidActivityAsync(turnContext);
151151

152152
// assert
153153
Assert.True(validActivity);
@@ -167,7 +167,7 @@ public async void Test_IsValidActivity_SpecificHandler()
167167
var turnContext = MockTurnContext();
168168

169169
// act
170-
var validActivity = await authManager.IsValidActivity(turnContext, "sharepoint");
170+
var validActivity = await authManager.IsValidActivityAsync(turnContext, "sharepoint");
171171

172172
// assert
173173
Assert.True(validActivity);

‎dotnet/packages/Microsoft.TeamsAI/Microsoft.TeamsAI.Tests/Application/Authentication/Bot/BotAuthenticationBaseTests.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,16 @@ public MockedBotAuthentication(Application<TState> app, string name, List<Dialog
2121
_throwExceptionWhenContinue = throwExceptionWhenContinue;
2222
}
2323

24-
public override Task<DialogTurnResult> ContinueDialog(ITurnContext context, TState state, string dialogStateProperty)
24+
public override Task<DialogTurnResult> ContinueDialog(ITurnContext context, TState state, string dialogStateProperty, CancellationToken cancellationToken = default)
2525
{
2626
if (_throwExceptionWhenContinue)
2727
{
28-
throw new Exception("mocked error");
28+
throw new TeamsAIAuthException("mocked error");
2929
}
3030
return Task.FromResult(_continueDialogResult);
3131
}
3232

33-
public override Task<DialogTurnResult> RunDialog(ITurnContext context, TState state, string dialogStateProperty)
33+
public override Task<DialogTurnResult> RunDialog(ITurnContext context, TState state, string dialogStateProperty, CancellationToken cancellationToken = default)
3434
{
3535
var result = _runDialogResult.FirstOrDefault();
3636
if (result == null)
@@ -113,7 +113,7 @@ public async void Test_Authenticate_Complete()
113113
{
114114
// arrange
115115
var app = new Application<TurnState>(new ApplicationOptions<TurnState>());
116-
var botAuth = new MockedBotAuthentication<TurnState>(app, "test", runDialogResult: new List<DialogTurnResult>() { new DialogTurnResult(DialogTurnStatus.Complete, new TokenResponse(token: "test token")) });
116+
var botAuth = new MockedBotAuthentication<TurnState>(app, "test", runDialogResult: new List<DialogTurnResult>() { new(DialogTurnStatus.Complete, new TokenResponse(token: "test token")) });
117117
var context = MockTurnContext();
118118
var state = await TurnStateConfig.GetTurnStateWithConversationStateAsync(context);
119119

@@ -130,7 +130,7 @@ public async void Test_Authenticate_CompleteWithoutToken()
130130
{
131131
// arrange
132132
var app = new Application<TurnState>(new ApplicationOptions<TurnState>());
133-
var botAuth = new MockedBotAuthentication<TurnState>(app, "test", runDialogResult: new List<DialogTurnResult>() { new DialogTurnResult(DialogTurnStatus.Complete) });
133+
var botAuth = new MockedBotAuthentication<TurnState>(app, "test", runDialogResult: new List<DialogTurnResult>() { new(DialogTurnStatus.Complete) });
134134
var context = MockTurnContext();
135135
var state = await TurnStateConfig.GetTurnStateWithConversationStateAsync(context);
136136

@@ -161,7 +161,7 @@ public async void Test_HandleSignInActivity_Complete()
161161
messageText = context.Activity.Text;
162162
return Task.CompletedTask;
163163
});
164-
botAuth.OnUserSignInFailure((context, state, exception) => { throw new Exception("sign in failure handler should not be called"); });
164+
botAuth.OnUserSignInFailure((context, state, exception) => { throw new TeamsAIAuthException("sign in failure handler should not be called"); });
165165

166166
// act
167167
await botAuth.HandleSignInActivity(context, state, new CancellationToken());
@@ -180,7 +180,7 @@ public async void Test_HandleSignInActivity_CompleteWithoutToken()
180180
var context = MockTurnContext();
181181
var state = await TurnStateConfig.GetTurnStateWithConversationStateAsync(context);
182182
TeamsAIAuthException? authException = null;
183-
botAuth.OnUserSignInSuccess((context, state) => { throw new Exception("sign in success handler should not be called"); });
183+
botAuth.OnUserSignInSuccess((context, state) => { throw new TeamsAIAuthException("sign in success handler should not be called"); });
184184
botAuth.OnUserSignInFailure((context, state, exception) => { authException = exception; return Task.CompletedTask; });
185185

186186
// act
@@ -200,7 +200,7 @@ public async void Test_HandleSignInActivity_ThrowException()
200200
var context = MockTurnContext();
201201
var state = await TurnStateConfig.GetTurnStateWithConversationStateAsync(context);
202202
TeamsAIAuthException? authException = null;
203-
botAuth.OnUserSignInSuccess((context, state) => { throw new Exception("sign in success handler should not be called"); });
203+
botAuth.OnUserSignInSuccess((context, state) => { throw new TeamsAIAuthException("sign in success handler should not be called"); });
204204
botAuth.OnUserSignInFailure((context, state, exception) => { authException = exception; return Task.CompletedTask; });
205205

206206
// act

‎dotnet/packages/Microsoft.TeamsAI/Microsoft.TeamsAI.Tests/Application/Authentication/MessageExtensions/MessageExtensionsAuthenticationBaseTests.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using Microsoft.Bot.Schema;
44
using Microsoft.Bot.Schema.Teams;
55
using Microsoft.Teams.AI.Tests.TestUtils;
6+
using Microsoft.Teams.AI.Exceptions;
67

78
namespace Microsoft.Teams.AI.Tests.Application.Authentication.MessageExtensions
89
{
@@ -26,7 +27,7 @@ public override Task<TokenResponse> HandleUserSignIn(ITurnContext context, strin
2627
{
2728
if (_signInResponse == null)
2829
{
29-
throw new Exception("HandlerUserSignIn failed");
30+
throw new TeamsAIAuthException("HandlerUserSignIn failed");
3031
}
3132
return Task.FromResult(_signInResponse);
3233
}
@@ -35,7 +36,7 @@ public override Task<TokenResponse> HandleSsoTokenExchange(ITurnContext context)
3536
{
3637
if (_tokenExchangeResponse == null)
3738
{
38-
throw new Exception("HandleSsoTokenExchange failed");
39+
throw new TeamsAIAuthException("HandleSsoTokenExchange failed");
3940
}
4041
return Task.FromResult(_tokenExchangeResponse);
4142
}

‎dotnet/packages/Microsoft.TeamsAI/Microsoft.TeamsAI.Tests/Application/Authentication/MockedAuthentication.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ public void Initialize(Application<TState> app, string name, IStorage? storage =
2323
return;
2424
}
2525

26-
public Task<bool> IsValidActivity(ITurnContext context)
26+
public Task<bool> IsValidActivityAsync(ITurnContext context)
2727
{
2828
return Task.FromResult(_validActivity);
2929
}
@@ -38,7 +38,7 @@ public IAuthentication<TState> OnUserSignInSuccess(Func<ITurnContext, TState, Ta
3838
return this;
3939
}
4040

41-
public Task<SignInResponse> SignInUser(ITurnContext context, TState state)
41+
public Task<SignInResponse> SignInUserAsync(ITurnContext context, TState state, CancellationToken cancellationToken = default)
4242
{
4343
var result = new SignInResponse(_mockedStatus);
4444
if (_mockedStatus == SignInStatus.Complete)
@@ -48,7 +48,7 @@ public Task<SignInResponse> SignInUser(ITurnContext context, TState state)
4848
return Task.FromResult(result);
4949
}
5050

51-
public Task SignOutUser(ITurnContext context, TState state)
51+
public Task SignOutUserAsync(ITurnContext context, TState state, CancellationToken cancellationToken = default)
5252
{
5353
return Task.CompletedTask;
5454
}

‎dotnet/packages/Microsoft.TeamsAI/Microsoft.TeamsAI/Application/Application.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -846,9 +846,9 @@ private async Task _OnTurnAsync(ITurnContext turnContext, CancellationToken canc
846846
if (Authentication != null && _startSignIn != null && await _startSignIn(turnContext, cancellationToken))
847847
{
848848
// Should skip activity that does not support sign-in
849-
if (await Authentication.IsValidActivity(turnContext))
849+
if (await Authentication.IsValidActivityAsync(turnContext))
850850
{
851-
SignInResponse response = await Authentication.SignUserIn(turnContext, turnState);
851+
SignInResponse response = await Authentication.SignUserInAsync(turnContext, turnState);
852852
if (response.Status == SignInStatus.Pending)
853853
{
854854
// Requires user action, save state and stop processing current activity

‎dotnet/packages/Microsoft.TeamsAI/Microsoft.TeamsAI/Application/Authentication/AdaptiveCards/AdaptiveCardsAuthenticationBase.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ namespace Microsoft.Teams.AI
66
/// <summary>
77
/// Base class for adaptive card authentication that handles common logic
88
/// </summary>
9-
public abstract class AdaptiveCardsAuthenticationBase
9+
internal abstract class AdaptiveCardsAuthenticationBase
1010
{
1111
/// <summary>
1212
/// Authenticate current user

‎dotnet/packages/Microsoft.TeamsAI/Microsoft.TeamsAI/Application/Authentication/AdaptiveCards/OAuthAdaptiveCardsAuthentication.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
/// <summary>
44
/// Handles authentication for Adaptive Cards in Teams using OAuth Connection.
55
/// </summary>
6-
public class OAuthAdaptiveCardsAuthentication : AdaptiveCardsAuthenticationBase
6+
internal class OAuthAdaptiveCardsAuthentication : AdaptiveCardsAuthenticationBase
77
{
88
}
99
}

‎dotnet/packages/Microsoft.TeamsAI/Microsoft.TeamsAI/Application/Authentication/AdaptiveCards/TeamsSsoAdaptiveCardsAuthentication.cs

Lines changed: 0 additions & 9 deletions
This file was deleted.

‎dotnet/packages/Microsoft.TeamsAI/Microsoft.TeamsAI/Application/Authentication/AuthenticationManager.cs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,16 +38,17 @@ public AuthenticationManager(AuthenticationOptions<TState> options)
3838
/// <param name="context">The turn context</param>
3939
/// <param name="state">The turn state</param>
4040
/// <param name="handlerName">Optional. The name of the authentication handler to use. If not specified, the default handler name is used.</param>
41+
/// <param name="cancellationToken">The cancellation token</param>
4142
/// <returns>The sign in response</returns>
42-
public async Task<SignInResponse> SignUserIn(ITurnContext context, TState state, string? handlerName = null)
43+
public async Task<SignInResponse> SignUserInAsync(ITurnContext context, TState state, string? handlerName = null, CancellationToken cancellationToken = default)
4344
{
4445
if (handlerName == null)
4546
{
4647
handlerName = _default;
4748
}
4849

4950
IAuthentication<TState> auth = Get(handlerName);
50-
SignInResponse response = await auth.SignInUser(context, state);
51+
SignInResponse response = await auth.SignInUserAsync(context, state, cancellationToken);
5152
if (response.Status == SignInStatus.Complete)
5253
{
5354
AuthUtilities.SetTokenInState(state, handlerName, response.Token!);
@@ -61,15 +62,16 @@ public async Task<SignInResponse> SignUserIn(ITurnContext context, TState state,
6162
/// <param name="context">The turn context</param>
6263
/// <param name="state">The turn state</param>
6364
/// <param name="handlerName">Optional. The name of the authentication handler to use. If not specified, the default handler name is used.</param>
64-
public async Task SignOutUser(ITurnContext context, TState state, string? handlerName = null)
65+
/// <param name="cancellationToken">The cancellation token</param>
66+
public async Task SignOutUserAsync(ITurnContext context, TState state, string? handlerName = null, CancellationToken cancellationToken = default)
6567
{
6668
if (handlerName == null)
6769
{
6870
handlerName = _default;
6971
}
7072

7173
IAuthentication<TState> auth = Get(handlerName);
72-
await auth.SignOutUser(context, state);
74+
await auth.SignOutUserAsync(context, state, cancellationToken);
7375
AuthUtilities.DeleteTokenFromState(state, handlerName);
7476
}
7577

@@ -79,15 +81,15 @@ public async Task SignOutUser(ITurnContext context, TState state, string? handle
7981
/// <param name="context">Current turn context.</param>
8082
/// <param name="handlerName">Optional. The name of the authentication handler to use. If not specified, the default handler name is used.</param>
8183
/// <returns>True if current activity supports authentication. Otherwise, false.</returns>
82-
public async Task<bool> IsValidActivity(ITurnContext context, string? handlerName = null)
84+
public async Task<bool> IsValidActivityAsync(ITurnContext context, string? handlerName = null)
8385
{
8486
if (handlerName == null)
8587
{
8688
handlerName = _default;
8789
}
8890

8991
IAuthentication<TState> auth = Get(handlerName);
90-
return await auth.IsValidActivity(context);
92+
return await auth.IsValidActivityAsync(context);
9193
}
9294

9395
/// <summary>

‎dotnet/packages/Microsoft.TeamsAI/Microsoft.TeamsAI/Application/Authentication/Bot/BotAuthenticationBase.cs

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ namespace Microsoft.Teams.AI
99
/// <summary>
1010
/// Base class for bot authentication that handles common logic
1111
/// </summary>
12-
public abstract class BotAuthenticationBase<TState>
12+
internal abstract class BotAuthenticationBase<TState>
1313
where TState : TurnState, new()
1414
{
1515
/// <summary>
@@ -47,21 +47,22 @@ public BotAuthenticationBase(Application<TState> app, string name, IStorage? sto
4747
app.AddRoute(this.VerifyStateRouteSelector, async (context, state, cancellationToken) =>
4848
{
4949
await this.HandleSignInActivity(context, state, cancellationToken);
50-
});
50+
}, true);
5151

5252
app.AddRoute(this.TokenExchangeRouteSelector, async (context, state, cancellationToken) =>
5353
{
5454
await this.HandleSignInActivity(context, state, cancellationToken);
55-
});
55+
}, true);
5656
}
5757

5858
/// <summary>
5959
/// Authenticate current user
6060
/// </summary>
6161
/// <param name="context">The turn context</param>
6262
/// <param name="state">The turn state</param>
63+
/// <param name="cancellationToken">The cancellation token</param>
6364
/// <returns>The sign in response</returns>
64-
public async Task<SignInResponse> AuthenticateAsync(ITurnContext context, TState state)
65+
public async Task<SignInResponse> AuthenticateAsync(ITurnContext context, TState state, CancellationToken cancellationToken = default)
6566
{
6667
// Get property names to use
6768
string userAuthStatePropertyName = GetUserAuthStatePropertyName(context);
@@ -76,7 +77,7 @@ public async Task<SignInResponse> AuthenticateAsync(ITurnContext context, TState
7677
});
7778
}
7879

79-
DialogTurnResult result = await RunDialog(context, state, userDialogStatePropertyName);
80+
DialogTurnResult result = await RunDialog(context, state, userDialogStatePropertyName, cancellationToken);
8081
if (result.Status == DialogTurnStatus.Complete)
8182
{
8283
// Delete user auth state
@@ -87,7 +88,7 @@ public async Task<SignInResponse> AuthenticateAsync(ITurnContext context, TState
8788
// Completed dialog without a token.
8889
// This could mean the user declined the consent prompt in the previous turn.
8990
// Retry authentication flow again.
90-
return await AuthenticateAsync(context, state);
91+
return await AuthenticateAsync(context, state, cancellationToken);
9192
}
9293
else
9394
{
@@ -125,7 +126,7 @@ public async Task HandleSignInActivity(ITurnContext context, TState state, Cance
125126
try
126127
{
127128
string userDialogStatePropertyName = GetUserDialogStatePropertyName(context);
128-
DialogTurnResult result = await ContinueDialog(context, state, userDialogStatePropertyName);
129+
DialogTurnResult result = await ContinueDialog(context, state, userDialogStatePropertyName, cancellationToken);
129130

130131
if (result.Status == DialogTurnStatus.Complete)
131132
{
@@ -259,16 +260,18 @@ private void DeleteAuthFlowState(ITurnContext context, TState state)
259260
/// <param name="context">The turn context</param>
260261
/// <param name="state">The turn state</param>
261262
/// <param name="dialogStateProperty">The property name for storing dialog state.</param>
263+
/// <param name="cancellationToken">The cancellation token</param>
262264
/// <returns>Dialog turn result that contains token if sign in success</returns>
263-
public abstract Task<DialogTurnResult> RunDialog(ITurnContext context, TState state, string dialogStateProperty);
265+
public abstract Task<DialogTurnResult> RunDialog(ITurnContext context, TState state, string dialogStateProperty, CancellationToken cancellationToken = default);
264266

265267
/// <summary>
266268
/// Continue the authentication dialog.
267269
/// </summary>
268270
/// <param name="context">The turn context</param>
269271
/// <param name="state">The turn state</param>
270272
/// <param name="dialogStateProperty">The property name for storing dialog state.</param>
273+
/// <param name="cancellationToken">The cancellation token</param>
271274
/// <returns>Dialog turn result that contains token if sign in success</returns>
272-
public abstract Task<DialogTurnResult> ContinueDialog(ITurnContext context, TState state, string dialogStateProperty);
275+
public abstract Task<DialogTurnResult> ContinueDialog(ITurnContext context, TState state, string dialogStateProperty, CancellationToken cancellationToken = default);
273276
}
274277
}

0 commit comments

Comments
 (0)