From a94325710cc80b23bc8dedf0d521ddf44f1f9c7e Mon Sep 17 00:00:00 2001 From: Oleksandr Didyk Date: Thu, 30 Jan 2025 16:56:11 +0000 Subject: [PATCH] Merged PR 47213: Fix missing authorization for API endpoints Resolves https://dev.azure.com/dnceng/internal/_workitems/7374 Adds back authorization for `/api/*` endpoints that was accidentally removed with the BAR token removal in [82d66a](https://github.com/dotnet/arcade-services/commit/82d66a0a6c0069c5f743f054e0101b7df97b2bf6#diff-063c785b35bfb14175032f6678838d7b6816789a51a057b9ae063ee42251f553L132-L137) ---- #### AI description (iteration 1) #### PR Classification Bug fix #### PR Summary This pull request addresses missing authorization for API endpoints by updating the authentication and authorization configurations. - `AuthenticationConfiguration.cs`: Added new policies for API and web authorization, updated existing policy names, and adjusted authentication schemes. - `ApiRedirection.cs`: Changed the authorization policy used in the redirection logic. - `PcsStartup.cs`: Updated folder authorization policy and enforced API authorization on controllers. Related work items: #7374 --- .../Configuration/ApiRedirection.cs | 2 +- .../AuthenticationConfiguration.cs | 20 ++++++++++++++----- .../PcsStartup.cs | 4 +++- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/ProductConstructionService/ProductConstructionService.Api/Configuration/ApiRedirection.cs b/src/ProductConstructionService/ProductConstructionService.Api/Configuration/ApiRedirection.cs index 8a56e3839a..f7ee0bd020 100644 --- a/src/ProductConstructionService/ProductConstructionService.Api/Configuration/ApiRedirection.cs +++ b/src/ProductConstructionService/ProductConstructionService.Api/Configuration/ApiRedirection.cs @@ -190,7 +190,7 @@ public static async Task IsAuthenticated(this HttpContext context) } var authService = context.RequestServices.GetRequiredService(); - AuthorizationResult result = await authService.AuthorizeAsync(success.Ticket!.Principal, AuthenticationConfiguration.MsftAuthorizationPolicyName); + AuthorizationResult result = await authService.AuthorizeAsync(success.Ticket!.Principal, AuthenticationConfiguration.WebAuthorizationPolicyName); if (!result.Succeeded) { context.Response.StatusCode = (int)HttpStatusCode.Forbidden; diff --git a/src/ProductConstructionService/ProductConstructionService.Api/Configuration/AuthenticationConfiguration.cs b/src/ProductConstructionService/ProductConstructionService.Api/Configuration/AuthenticationConfiguration.cs index 63d9d5747f..e6d9b1dae4 100644 --- a/src/ProductConstructionService/ProductConstructionService.Api/Configuration/AuthenticationConfiguration.cs +++ b/src/ProductConstructionService/ProductConstructionService.Api/Configuration/AuthenticationConfiguration.cs @@ -9,15 +9,16 @@ namespace ProductConstructionService.Api.Configuration; internal static class AuthenticationConfiguration { - public const string EntraAuthorizationPolicyName = "Entra"; - public const string MsftAuthorizationPolicyName = "msft"; + public const string EntraAuthorizationSchemeName = "Entra"; + public const string ApiAuthorizationPolicyName = "MsftApi"; + public const string WebAuthorizationPolicyName = "MsftWeb"; public const string AdminAuthorizationPolicyName = "RequireAdminAccess"; public const string AccountSignInRoute = "/Account/SignIn"; public static readonly string[] AuthenticationSchemes = [ - EntraAuthorizationPolicyName, + EntraAuthorizationSchemeName, OpenIdConnectDefaults.AuthenticationScheme, ]; @@ -54,7 +55,7 @@ public static void ConfigureAuthServices(this IServiceCollection services, IConf var openIdAuth = services.AddAuthentication(OpenIdConnectDefaults.AuthenticationScheme); openIdAuth - .AddMicrosoftIdentityWebApi(entraAuthConfig, EntraAuthorizationPolicyName); + .AddMicrosoftIdentityWebApi(entraAuthConfig, EntraAuthorizationSchemeName); openIdAuth .AddMicrosoftIdentityWebApp(options => @@ -88,12 +89,21 @@ public static void ConfigureAuthServices(this IServiceCollection services, IConf services .AddAuthorizationBuilder() - .AddPolicy(MsftAuthorizationPolicyName, policy => + .AddDefaultPolicy(WebAuthorizationPolicyName, policy => { policy.AddAuthenticationSchemes(AuthenticationSchemes); policy.RequireAuthenticatedUser(); policy.RequireRole(userRole); }) + .AddPolicy(ApiAuthorizationPolicyName, policy => + { + // Cookie scheme for BarViz, Entra JWT for Darc and other clients + // The order matters here as the last scheme's Forbid() handler is used for processing authentication failures + // Since cookie scheme returns 200 with the auth exception in the body, Entra should be used instead as it 401s + policy.AddAuthenticationSchemes([CookieAuthenticationDefaults.AuthenticationScheme, EntraAuthorizationSchemeName]); + policy.RequireAuthenticatedUser(); + policy.RequireRole(userRole); + }) .AddPolicy(AdminAuthorizationPolicyName, policy => { policy.AddAuthenticationSchemes(AuthenticationSchemes); diff --git a/src/ProductConstructionService/ProductConstructionService.Api/PcsStartup.cs b/src/ProductConstructionService/ProductConstructionService.Api/PcsStartup.cs index 276c4a146e..34aa82be1a 100644 --- a/src/ProductConstructionService/ProductConstructionService.Api/PcsStartup.cs +++ b/src/ProductConstructionService/ProductConstructionService.Api/PcsStartup.cs @@ -248,7 +248,7 @@ internal static async Task ConfigurePcs( builder.Services.AddRazorPages( options => { - options.Conventions.AuthorizeFolder("/", AuthenticationConfiguration.MsftAuthorizationPolicyName); + options.Conventions.AuthorizeFolder("/", AuthenticationConfiguration.WebAuthorizationPolicyName); options.Conventions.AllowAnonymousToPage("/Error"); }) .AddGitHubWebHooks() @@ -297,6 +297,8 @@ public static void ConfigureApi(this IApplicationBuilder app, bool isDevelopment app.UseEndpoints(e => { var controllers = e.MapControllers(); + controllers.RequireAuthorization(AuthenticationConfiguration.ApiAuthorizationPolicyName); + if (isDevelopment) { controllers.AllowAnonymous();