-
Notifications
You must be signed in to change notification settings - Fork 16
Fix UnauthorizedAccessException with skipAuth #297
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
1c276ae
953fa50
2cb8d52
0eb2ae0
bfb6b37
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 | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,37 @@ | ||||||||||||||||||||||||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||||||||||||||||||||||||
| // Licensed under the MIT License. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| namespace Microsoft.Teams.Api.Auth; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||
| /// A fallback token used when no authentication is provided (e.g., skipAuth mode). | ||||||||||||||||||||||||
| /// Mirrors the behavior of Python and TypeScript SDKs. | ||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||
| public class AnonymousToken : IToken | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| public string? AppId => string.Empty; | ||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| public string? AppDisplayName => string.Empty; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| public string? TenantId => string.Empty; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| public string ServiceUrl { get; } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| public CallerType From => CallerType.Azure; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| public string FromId => string.Empty; | ||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| public DateTime? Expiration => null; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| public bool IsExpired => false; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| public IEnumerable<string> Scopes => []; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| public AnonymousToken(string serviceUrl) | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| // Ensure serviceUrl has trailing slash for consistency | ||||||||||||||||||||||||
| ServiceUrl = serviceUrl.EndsWith('/') ? serviceUrl : serviceUrl + '/'; | ||||||||||||||||||||||||
|
Comment on lines
+32
to
+33
|
||||||||||||||||||||||||
| // Ensure serviceUrl has trailing slash for consistency | |
| ServiceUrl = serviceUrl.EndsWith('/') ? serviceUrl : serviceUrl + '/'; | |
| // Use a default service URL if the provided value is null, empty, or whitespace | |
| var normalizedServiceUrl = string.IsNullOrWhiteSpace(serviceUrl) | |
| ? "https://smba.trafficmanager.net/teams" | |
| : serviceUrl; | |
| // Ensure serviceUrl has trailing slash for consistency | |
| ServiceUrl = normalizedServiceUrl.EndsWith('/') | |
| ? normalizedServiceUrl | |
| : normalizedServiceUrl + '/'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -184,6 +184,10 @@ public async Task<IResult> Do(HttpContext httpContext, CancellationToken cancell | |
| return Results.BadRequest("Missing activity"); | ||
| } | ||
|
|
||
| // If no token was extracted, create an anonymous token with serviceUrl from the activity (or default) | ||
| // This matches Python/TypeScript SDK behavior for skipAuth scenarios | ||
| IToken resolvedToken = (IToken?)token ?? new AnonymousToken(activity.ServiceUrl ?? "https://smba.trafficmanager.net/teams"); | ||
|
||
|
|
||
| var data = new Dictionary<string, object?> | ||
| { | ||
| ["Request.TraceId"] = httpContext.TraceIdentifier | ||
|
|
@@ -200,7 +204,7 @@ public async Task<IResult> Do(HttpContext httpContext, CancellationToken cancell | |
|
|
||
| var res = await Do(new ActivityEvent() | ||
| { | ||
| Token = token, | ||
| Token = resolvedToken, | ||
| Activity = activity, | ||
| Extra = data, | ||
| Services = httpContext.RequestServices | ||
|
|
@@ -235,9 +239,13 @@ await Events( | |
| } | ||
| } | ||
|
|
||
| public JsonWebToken ExtractToken(HttpRequest httpRequest) | ||
| public JsonWebToken? ExtractToken(HttpRequest httpRequest) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Return-type change from |
||
| { | ||
| var authHeader = httpRequest.Headers.Authorization.FirstOrDefault() ?? throw new UnauthorizedAccessException(); | ||
| var authHeader = httpRequest.Headers.Authorization.FirstOrDefault(); | ||
| if (string.IsNullOrEmpty(authHeader)) | ||
| { | ||
| return null; | ||
| } | ||
| return new JsonWebToken(authHeader.Replace("Bearer ", "")); | ||
| } | ||
|
|
||
|
|
||
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.
Net-new in this PR with no subclasses anywhere in the repo, and not declared
sealed. Since narrowing accessibility becomes a breaking change after ship, this is the right moment to decide: I'd suggestinternal sealed class AnonymousToken : IToken. The class summary frames it as "internal SDK behavior matching Python and TypeScript SDKs", which doesn't require a public type.