-
Notifications
You must be signed in to change notification settings - Fork 241
Improvements #3478
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
Improvements #3478
Changes from all commits
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 |
|---|---|---|
| @@ -1,8 +1,10 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| using Microsoft.AspNetCore.Authorization; | ||
| using Microsoft.AspNetCore.Http.HttpResults; | ||
| using Microsoft.AspNetCore.Mvc; | ||
| using Microsoft.Extensions.Options; | ||
| using Microsoft.Identity.Abstractions; | ||
| using Microsoft.Identity.Web.Sidecar.Models; | ||
|
|
||
|
|
@@ -19,6 +21,7 @@ public static void AddDownstreamApiRequestEndpoints(this WebApplication app) | |
| ProducesProblem(401); | ||
| } | ||
|
|
||
| [AllowAnonymous] | ||
| private static async Task<Results<Ok<AuthorizationHeaderResult>, ProblemHttpResult>> AuthorizationHeaderAsync( | ||
| HttpContext httpContext, | ||
| [FromRoute] string apiName, | ||
|
|
@@ -27,9 +30,9 @@ private static async Task<Results<Ok<AuthorizationHeaderResult>, ProblemHttpResu | |
| [FromQuery] string? tenant, | ||
| [FromBody] DownstreamApiOptions? optionsOverride, | ||
| [FromServices] IAuthorizationHeaderProvider headerProvider, | ||
| [FromServices] IConfiguration configuration) | ||
| [FromServices] IOptionsMonitor<DownstreamApiOptions> optionsMonitor) | ||
|
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. It's better to get the options directly. and we'll be ready if we want to have a CalldownstreamApis endpoint |
||
| { | ||
| DownstreamApiOptions? options = configuration.GetSection($"DownstreamApi:{apiName}").Get<DownstreamApiOptions>(); | ||
| DownstreamApiOptions? options = optionsMonitor.Get(apiName); | ||
|
|
||
| if (options is null) | ||
| { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,8 +30,8 @@ private static Results<Ok<ValidateAuthorizationHeaderResult>, ProblemHttpResult> | |
| { | ||
| httpContext.VerifyUserHasAnyAcceptedScope(scopeRequiredByApi); | ||
| } | ||
| var claimsPrincipal = httpContext.User; | ||
| var token = claimsPrincipal.GetBootstrapToken() as JsonWebToken; | ||
|
|
||
| var token = httpContext.GetTokenUsedToCallWebAPI() as JsonWebToken; | ||
|
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. See also the simplication in program.cs |
||
|
|
||
| if (token is null) | ||
| { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |
| // Licensed under the MIT License. | ||
|
|
||
| using System.Diagnostics; | ||
| using System.IdentityModel.Tokens.Jwt; | ||
| using Microsoft.AspNetCore.Authentication.JwtBearer; | ||
| using Microsoft.Identity.Web; | ||
| using Microsoft.Identity.Web.Sidecar.Endpoints; | ||
|
|
@@ -21,26 +22,21 @@ public static void Main(string[] args) | |
| .EnableTokenAcquisitionToCallDownstreamApi() | ||
| .AddInMemoryTokenCaches(); | ||
|
|
||
| // Add the agent identities and downstream APIs | ||
| builder.Services.AddAgentIdentities() | ||
| .AddDownstreamApis(builder.Configuration.GetSection("DownstreamApis")); | ||
|
|
||
| builder.Services.AddHealthChecks(); | ||
|
|
||
| // Disable claims mapping. | ||
| JwtSecurityTokenHandler.DefaultMapInboundClaims = false; | ||
|
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. We don't want Wilson to transform the claims. |
||
| JsonWebTokenHandler.DefaultMapInboundClaims = false; | ||
| builder.Services.Configure<JwtBearerOptions>(JwtBearerDefaults.AuthenticationScheme, | ||
| options => | ||
| { | ||
| options.Events ??= new(); | ||
|
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. Simplication here as this is already done. |
||
| options.Events.OnTokenValidated = context => | ||
| { | ||
| Debug.Assert(context.SecurityToken is JsonWebToken, "Token should always be JsonWebToken"); | ||
| var token = (JsonWebToken)context.SecurityToken; | ||
|
|
||
| if (context.Principal?.Identities.FirstOrDefault() is null) | ||
| { | ||
| context.Fail("No principal or no identity"); | ||
| return Task.FromResult(context); | ||
| } | ||
|
|
||
| context.Principal.Identities.First().BootstrapContext = token.InnerToken is not null ? token.InnerToken : token; | ||
| return Task.FromResult(context); | ||
| }; | ||
| // Enable the right role claim type. | ||
| options.TokenValidationParameters.RoleClaimType = "roles"; | ||
|
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. important for AuthZ |
||
| options.TokenValidationParameters.NameClaimType = "sub"; | ||
| }); | ||
|
|
||
| builder.Services.AddAuthorization(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,10 +21,10 @@ For more info see https://aka.ms/dotnet-template-ms-identity-platform | |
| ], | ||
|
|
||
| "EnablePiiLogging": false, | ||
| "AllowWebApiToBeAuthorizedByACL": true, | ||
| "AllowWebApiToBeAuthorizedByACL": true | ||
| }, | ||
|
|
||
| "DownstreamApi": { | ||
| "DownstreamApis": { | ||
|
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. the usual convention |
||
| "me": { | ||
| "BaseUrl": "https://graph.microsoft.com/v1.0/", | ||
| "RelativePath": "me", | ||
|
|
@@ -38,7 +38,7 @@ For more info see https://aka.ms/dotnet-template-ms-identity-platform | |
| "Microsoft.AspNetCore": "Warning" | ||
| } | ||
| }, | ||
| "AllowedHosts": "*", | ||
| "AllowedHosts": "*" | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |
| // Licensed under the MIT License. | ||
|
|
||
| using System.Net.Http.Headers; | ||
| using System.Net.Http.Json; | ||
| using Microsoft.AspNetCore.Hosting; | ||
| using Microsoft.AspNetCore.Mvc.Testing; | ||
| using Microsoft.Extensions.Configuration; | ||
|
|
@@ -27,15 +28,26 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) | |
| }); | ||
| builder.ConfigureServices(services => | ||
| { | ||
| // Given we add the Json file after the initial configuration, and that | ||
| // downstream APIs are added to a IOptions, we need to re-add the downstream APIs | ||
| // with the new config | ||
| IConfiguration? configuration = services! | ||
| .First(s => s.ServiceType == typeof(IConfiguration)) | ||
| ?.ImplementationFactory | ||
| ?.Invoke(null!) as IConfiguration; | ||
|
|
||
| services!.AddDownstreamApis(configuration!.GetSection("DownstreamApis")); | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| public class ValidateEndpointTests : IClassFixture<SidecarApiFactory> | ||
| public class EndpointsE2ETests : IClassFixture<SidecarApiFactory> | ||
| { | ||
| private readonly SidecarApiFactory _factory; | ||
|
|
||
| public ValidateEndpointTests(SidecarApiFactory factory) => _factory = factory; | ||
| public EndpointsE2ETests(SidecarApiFactory factory) => _factory = factory; | ||
| string agentIdentity = "d84da24a-2ea2-42b8-b5ab-8637ec208024"; // Replace with the actual agent identity | ||
| string userUpn = "[email protected]"; // Replace with the actual user upn. | ||
|
|
||
| [Fact] | ||
| public async Task Validate_WhenBadTokenAsync() | ||
|
|
@@ -65,6 +77,38 @@ public async Task Validate_WhenGoodTokenAsync() | |
| Assert.NotEmpty(content); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task GetAuthorizationHeaderForAgentUserIdentityAuthenticated() | ||
| { | ||
| string agentIdentity = "d84da24a-2ea2-42b8-b5ab-8637ec208024"; // Replace with the actual agent identity | ||
| string userUpn = "[email protected]"; // Replace with the actual user upn. | ||
|
|
||
| // Getting a token to call the API. | ||
| string authorizationHeader = await GetAuthorizationHeaderToCallTheSideCarAsync(); | ||
|
|
||
| // Calling the API | ||
| var client = _factory.CreateClient(); | ||
|
|
||
| client.DefaultRequestHeaders.Authorization = AuthenticationHeaderValue.Parse(authorizationHeader); | ||
| var response = await client.PostAsync($"/AuthorizationHeader/MsGraph?agentidentity={agentIdentity}&agentUsername={userUpn}", null); | ||
| var content = await response.Content.ReadAsStringAsync(); | ||
| Assert.Equal(System.Net.HttpStatusCode.OK, response.StatusCode); | ||
|
|
||
| } | ||
|
|
||
| [Fact] | ||
| public async Task GetAuthorizationHeaderForAgentUserIdentityUnauthenticated() | ||
| { | ||
| // Calling the API | ||
| var client = _factory.CreateClient(); | ||
|
|
||
| var response = await client.PostAsync($"/AuthorizationHeader/MsGraph?agentidentity={agentIdentity}&agentUsername={userUpn}", null); | ||
| var content = await response.Content.ReadAsStringAsync(); | ||
| Assert.Equal(System.Net.HttpStatusCode.OK, response.StatusCode); | ||
|
|
||
| } | ||
|
|
||
|
|
||
| private static async Task<string> GetAuthorizationHeaderToCallTheSideCarAsync() | ||
| { | ||
| ServiceCollection services = new(); | ||
|
|
||
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.
OidcFic can't work with scoped token acquisition :-(