-
Notifications
You must be signed in to change notification settings - Fork 15
Account Linking Url: Combined Auth Flow #188
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
d9f0933
de69483
857a402
153f7c4
b1c59b4
b8f3dec
a38bbea
92bfe06
711fb51
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,7 @@ public class BotTokenClient : Client | |
| public static readonly string BotScope = "https://api.botframework.com/.default"; | ||
| public static readonly string GraphScope = "https://graph.microsoft.com/.default"; | ||
|
|
||
| public BotTokenClient() : this(default) | ||
| public BotTokenClient() : base() | ||
| { | ||
|
|
||
| } | ||
|
Comment on lines
+13
to
16
|
||
|
|
@@ -40,7 +40,7 @@ public virtual async Task<ITokenResponse> GetAsync(IHttpCredentials credentials, | |
| return await credentials.Resolve(http ?? _http, [BotScope], _cancellationToken); | ||
| } | ||
|
|
||
| public async Task<ITokenResponse> GetGraphAsync(IHttpCredentials credentials, IHttpClient? http = null) | ||
| public virtual async Task<ITokenResponse> GetGraphAsync(IHttpCredentials credentials, IHttpClient? http = null) | ||
| { | ||
| return await credentials.Resolve(http ?? _http, [GraphScope], _cancellationToken); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,8 +11,8 @@ namespace Microsoft.Teams.Api.Clients; | |
| public class ConversationClient : Client | ||
| { | ||
| public readonly string ServiceUrl; | ||
| public readonly ActivityClient Activities; | ||
| public readonly MemberClient Members; | ||
| public virtual ActivityClient Activities { get; } | ||
| public virtual MemberClient Members { get; } | ||
|
Comment on lines
+14
to
+15
|
||
|
|
||
| public ConversationClient(string serviceUrl, CancellationToken cancellationToken = default) : base(cancellationToken) | ||
| { | ||
|
|
@@ -42,7 +42,7 @@ public ConversationClient(string serviceUrl, IHttpClientFactory factory, Cancell | |
| Members = new MemberClient(serviceUrl, _http, cancellationToken); | ||
| } | ||
|
|
||
| public async Task<ConversationResource> CreateAsync(CreateRequest request) | ||
| public virtual async Task<ConversationResource> CreateAsync(CreateRequest request) | ||
| { | ||
| var req = HttpRequest.Post($"{ServiceUrl}v3/conversations", body: request); | ||
| var res = await _http.SendAsync<ConversationResource>(req, _cancellationToken); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,7 +7,12 @@ namespace Microsoft.Teams.Api.Clients; | |||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| public class UserClient : Client | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| public UserTokenClient Token { get; } | ||||||||||||||||||||||||||
| public virtual UserTokenClient Token { get; } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| public UserClient() : base() | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| Token = new UserTokenClient(_http, _cancellationToken); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
Comment on lines
+12
to
16
|
||||||||||||||||||||||||||
| public UserClient() : base() | |
| { | |
| Token = new UserTokenClient(_http, _cancellationToken); | |
| } |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This parameterless constructor calls base(), but the parent Client class does not have a parameterless constructor. This will cause a compilation error. Either add a parameterless constructor to the Client base class, or remove this constructor if it's not needed.
| public UserClient() : base() | |
| { | |
| Token = new UserTokenClient(_http, _cancellationToken); | |
| } | |
| public UserClient(CancellationToken cancellationToken = default) : base(cancellationToken) | |
| public UserClient(CancellationToken cancellationToken = default) : base(cancellationToken) | |
| { | |
| Token = new UserTokenClient(_http, cancellationToken); | |
| } | |
| public UserClient(IHttpClient client, CancellationToken cancellationToken = default) : base(client, cancellationToken) |
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -15,6 +15,11 @@ public class UserTokenClient : Client | |||||||||
| DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| public UserTokenClient() : base() | ||||||||||
| { | ||||||||||
|
|
||||||||||
| } | ||||||||||
|
Comment on lines
+18
to
+21
|
||||||||||
|
|
||||||||||
|
Comment on lines
+18
to
+22
|
||||||||||
| public UserTokenClient() : base() | |
| { | |
| } |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -7,14 +7,49 @@ namespace Microsoft.Teams.Apps; | |||||
|
|
||||||
| public class AppOptions | ||||||
| { | ||||||
| /// <summary> | ||||||
| /// The applications optional storage provider that allows | ||||||
| /// the application to access shared dependencies. | ||||||
| /// </summary> | ||||||
| public IServiceProvider? Provider { get; set; } | ||||||
|
|
||||||
| /// <summary> | ||||||
| /// The applications optional ILogger instance. | ||||||
| /// </summary> | ||||||
| public Common.Logging.ILogger? Logger { get; set; } | ||||||
|
|
||||||
| /// <summary> | ||||||
| /// The applications optional IStorage instance. | ||||||
| /// </summary> | ||||||
| public Common.Storage.IStorage<string, object>? Storage { get; set; } | ||||||
|
|
||||||
| /// <summary> | ||||||
| /// When provided, the application will use this <code>IHttpClient</code> instance | ||||||
| /// to send all http requests. | ||||||
| /// </summary> | ||||||
| public Common.Http.IHttpClient? Client { get; set; } | ||||||
|
|
||||||
| /// <summary> | ||||||
| /// When provided, the application will use this <code>IHttpClientFactory</code> to | ||||||
| /// initialize a new client whenever needed. | ||||||
| /// </summary> | ||||||
| public Common.Http.IHttpClientFactory? ClientFactory { get; set; } | ||||||
|
|
||||||
| /// <summary> | ||||||
| /// When provided, the application will use these credentials to resolve tokens it | ||||||
| /// uses to make API requests. | ||||||
| /// </summary> | ||||||
| public Common.Http.IHttpCredentials? Credentials { get; set; } | ||||||
|
|
||||||
| /// <summary> | ||||||
| /// A list of plugins to import into the application. | ||||||
| /// </summary> | ||||||
| public IList<IPlugin> Plugins { get; set; } = []; | ||||||
| public OAuthSettings OAuth { get; set; } = new OAuthSettings(); | ||||||
|
|
||||||
| /// <summary> | ||||||
| /// User <code>OAuth</code> settings for the deferred (User) auth flows. | ||||||
|
||||||
| /// User <code>OAuth</code> settings for the deferred (User) auth flows. | |
| /// User <code>OAuth</code> settings for the deferred (user) auth flows. |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -112,6 +112,15 @@ public partial interface IContext<TActivity> where TActivity : IActivity | |||||||||||||||||||
| /// </summary> | ||||||||||||||||||||
| public Task<object?> Next(); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||
| /// Called to continue the chain of route handlers using the specified context instance. | ||||||||||||||||||||
| /// Use this overload when you want to invoke the next handler with a different or wrapped | ||||||||||||||||||||
| /// <see cref="IContext{TActivity}"/> than the current one; if not called, no other handlers | ||||||||||||||||||||
| /// in the sequence will be executed. | ||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||
| /// <param name="context">The context to pass to the next handler in the chain.</param> | ||||||||||||||||||||
| public Task<object?> Next(IContext<TActivity> context); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||
|
Comment on lines
+116
to
124
|
||||||||||||||||||||
| /// Called to continue the chain of route handlers using the specified context instance. | |
| /// Use this overload when you want to invoke the next handler with a different or wrapped | |
| /// <see cref="IContext{TActivity}"/> than the current one; if not called, no other handlers | |
| /// in the sequence will be executed. | |
| /// </summary> | |
| /// <param name="context">The context to pass to the next handler in the chain.</param> | |
| public Task<object?> Next(IContext<TActivity> context); | |
| /// <summary> |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new Next(IContext<TActivity> context) overload appears redundant. It converts the passed context to IActivity type and calls OnNext, which is the same as what Next() does with the current context. In the test at line 70, context.Next(context) is called, which is functionally equivalent to just calling context.Next(). Consider removing this overload unless there's a specific use case where a different context instance needs to be passed.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,20 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| public class OAuthSettings(string? connectionName = "graph") | ||
| namespace Microsoft.Teams.Apps; | ||
|
|
||
| /// <summary> | ||
| /// Settings for Deferred (User) auth flows | ||
| /// </summary> | ||
| public class OAuthSettings | ||
| { | ||
| public string DefaultConnectionName { get; set; } = connectionName; | ||
| /// <summary> | ||
| /// The default connection name to use | ||
| /// </summary> | ||
| public string DefaultConnectionName { get; set; } = "graph"; | ||
|
|
||
| /// <summary> | ||
| /// Url used for client to perform tab auth and link the NAA account to the bot login account. | ||
| /// </summary> | ||
| public string? AccountLinkingUrl { get; set; } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This parameterless constructor calls
base(), but the parentClientclass does not have a parameterless constructor. This will cause a compilation error. The previous code was callingbase(default)which correctly called theClient(CancellationToken)constructor. Either revert this change tobase(default)or add a parameterless constructor to theClientbase class.