From e29c49516646227d5c6c1dcfa7c04993759c3067 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Fri, 14 Feb 2020 10:53:47 -0800 Subject: [PATCH] Transfer endpoint metadata from PageActionDescriptor to CompiledPageActionDescriptor (#19061) Fixes https://github.com/dotnet/aspnetcore/issues/17300 --- .../src/Abstractions/ActionDescriptor.cs | 1 + .../src/Infrastructure/DefaultPageLoader.cs | 20 ++++- .../DynamicPageEndpointMatcherPolicy.cs | 14 +++- .../Infrastructure/PageLoaderMatcherPolicy.cs | 11 ++- .../AuthMiddlewareUsingRequireAuthTest.cs | 80 +++++++++++++++++++ .../SecurityWebSite/StartupWithRequireAuth.cs | 44 ++++++++++ 6 files changed, 165 insertions(+), 5 deletions(-) create mode 100644 src/Mvc/test/Mvc.FunctionalTests/AuthMiddlewareUsingRequireAuthTest.cs create mode 100644 src/Mvc/test/WebSites/SecurityWebSite/StartupWithRequireAuth.cs diff --git a/src/Mvc/Mvc.Abstractions/src/Abstractions/ActionDescriptor.cs b/src/Mvc/Mvc.Abstractions/src/Abstractions/ActionDescriptor.cs index 443e05259a..c6200ba6ee 100644 --- a/src/Mvc/Mvc.Abstractions/src/Abstractions/ActionDescriptor.cs +++ b/src/Mvc/Mvc.Abstractions/src/Abstractions/ActionDescriptor.cs @@ -47,6 +47,7 @@ namespace Microsoft.AspNetCore.Mvc.Abstractions /// /// Gets or sets the endpoint metadata for this action. + /// This API is meant for infrastructure and should not be used by application code. /// public IList EndpointMetadata { get; set; } diff --git a/src/Mvc/Mvc.RazorPages/src/Infrastructure/DefaultPageLoader.cs b/src/Mvc/Mvc.RazorPages/src/Infrastructure/DefaultPageLoader.cs index 32472186e2..370df5baee 100644 --- a/src/Mvc/Mvc.RazorPages/src/Infrastructure/DefaultPageLoader.cs +++ b/src/Mvc/Mvc.RazorPages/src/Infrastructure/DefaultPageLoader.cs @@ -67,6 +67,9 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure } public override Task LoadAsync(PageActionDescriptor actionDescriptor) + => LoadAsync(actionDescriptor, EndpointMetadataCollection.Empty); + + internal Task LoadAsync(PageActionDescriptor actionDescriptor, EndpointMetadataCollection endpointMetadata) { if (actionDescriptor == null) { @@ -79,10 +82,10 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure return compiledDescriptorTask; } - return cache.GetOrAdd(actionDescriptor, LoadAsyncCore(actionDescriptor)); + return cache.GetOrAdd(actionDescriptor, LoadAsyncCore(actionDescriptor, endpointMetadata)); } - private async Task LoadAsyncCore(PageActionDescriptor actionDescriptor) + private async Task LoadAsyncCore(PageActionDescriptor actionDescriptor, EndpointMetadataCollection endpointMetadata) { var viewDescriptor = await Compiler.CompileAsync(actionDescriptor.RelativePath); var context = new PageApplicationModelProviderContext(actionDescriptor, viewDescriptor.Type.GetTypeInfo()); @@ -110,7 +113,18 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure routeNames: new HashSet(StringComparer.OrdinalIgnoreCase), action: compiled, routes: Array.Empty(), - conventions: Array.Empty>(), + conventions: new Action[] + { + b => + { + // Metadata from PageActionDescriptor is less significant than the one discovered from the compiled type. + // Consequently, we'll insert it at the beginning. + for (var i = endpointMetadata.Count - 1; i >=0; i--) + { + b.Metadata.Insert(0, endpointMetadata[i]); + } + }, + }, createInertEndpoints: false); // In some test scenarios there's no route so the endpoint isn't created. This is fine because diff --git a/src/Mvc/Mvc.RazorPages/src/Infrastructure/DynamicPageEndpointMatcherPolicy.cs b/src/Mvc/Mvc.RazorPages/src/Infrastructure/DynamicPageEndpointMatcherPolicy.cs index ea68558d4b..9f1faa2baa 100644 --- a/src/Mvc/Mvc.RazorPages/src/Infrastructure/DynamicPageEndpointMatcherPolicy.cs +++ b/src/Mvc/Mvc.RazorPages/src/Infrastructure/DynamicPageEndpointMatcherPolicy.cs @@ -160,7 +160,19 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure var loadedEndpoints = new List(endpoints); for (var j = 0; j < loadedEndpoints.Count; j++) { - var compiled = await _loader.LoadAsync(loadedEndpoints[j].Metadata.GetMetadata()); + var metadata = loadedEndpoints[j].Metadata; + var pageActionDescriptor = metadata.GetMetadata(); + + CompiledPageActionDescriptor compiled; + if (_loader is DefaultPageLoader defaultPageLoader) + { + compiled = await defaultPageLoader.LoadAsync(pageActionDescriptor, endpoint.Metadata); + } + else + { + compiled = await _loader.LoadAsync(pageActionDescriptor); + } + loadedEndpoints[j] = compiled.Endpoint; } diff --git a/src/Mvc/Mvc.RazorPages/src/Infrastructure/PageLoaderMatcherPolicy.cs b/src/Mvc/Mvc.RazorPages/src/Infrastructure/PageLoaderMatcherPolicy.cs index 3d2acb23e8..18c14db7c6 100644 --- a/src/Mvc/Mvc.RazorPages/src/Infrastructure/PageLoaderMatcherPolicy.cs +++ b/src/Mvc/Mvc.RazorPages/src/Infrastructure/PageLoaderMatcherPolicy.cs @@ -78,7 +78,16 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure { // We found an endpoint instance that has a PageActionDescriptor, but not a // CompiledPageActionDescriptor. Update the CandidateSet. - var compiled = _loader.LoadAsync(page); + Task compiled; + if (_loader is DefaultPageLoader defaultPageLoader) + { + compiled = defaultPageLoader.LoadAsync(page, endpoint.Metadata); + } + else + { + compiled = _loader.LoadAsync(page); + } + if (compiled.IsCompletedSuccessfully) { candidates.ReplaceEndpoint(i, compiled.Result.Endpoint, candidate.Values); diff --git a/src/Mvc/test/Mvc.FunctionalTests/AuthMiddlewareUsingRequireAuthTest.cs b/src/Mvc/test/Mvc.FunctionalTests/AuthMiddlewareUsingRequireAuthTest.cs new file mode 100644 index 0000000000..7c8e4fed40 --- /dev/null +++ b/src/Mvc/test/Mvc.FunctionalTests/AuthMiddlewareUsingRequireAuthTest.cs @@ -0,0 +1,80 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Linq; +using System.Net; +using System.Net.Http; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Hosting; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.FunctionalTests +{ + public class AuthMiddlewareUsingRequireAuthTest : IClassFixture> + { + public AuthMiddlewareUsingRequireAuthTest(MvcTestFixture fixture) + { + var factory = fixture.Factories.FirstOrDefault() ?? fixture.WithWebHostBuilder(ConfigureWebHostBuilder); + Client = factory.CreateDefaultClient(); + } + + private static void ConfigureWebHostBuilder(IWebHostBuilder builder) => + builder.UseStartup(); + + public HttpClient Client { get; } + + [Fact] + public async Task RequireAuthConfiguredGlobally_AppliesToControllers() + { + // Arrange + var action = "Home/Index"; + var response = await Client.GetAsync(action); + + await AssertAuthorizeResponse(response); + + // We should be able to login with ClaimA alone + var authCookie = await GetAuthCookieAsync("LoginClaimA"); + + var request = new HttpRequestMessage(HttpMethod.Get, action); + request.Headers.Add("Cookie", authCookie); + + response = await Client.SendAsync(request); + await response.AssertStatusCodeAsync(HttpStatusCode.OK); + } + + [Fact] + public async Task RequireAuthConfiguredGlobally_AppliesToRazorPages() + { + // Arrange + var action = "PagesHome"; + var response = await Client.GetAsync(action); + + await AssertAuthorizeResponse(response); + + // We should be able to login with ClaimA alone + var authCookie = await GetAuthCookieAsync("LoginClaimA"); + + var request = new HttpRequestMessage(HttpMethod.Get, action); + request.Headers.Add("Cookie", authCookie); + + response = await Client.SendAsync(request); + await response.AssertStatusCodeAsync(HttpStatusCode.OK); + } + + private async Task AssertAuthorizeResponse(HttpResponseMessage response) + { + await response.AssertStatusCodeAsync(HttpStatusCode.Redirect); + Assert.Equal("/Home/Login", response.Headers.Location.LocalPath); + } + + private async Task GetAuthCookieAsync(string action) + { + var response = await Client.PostAsync($"Login/{action}", null); + + await response.AssertStatusCodeAsync(HttpStatusCode.OK); + Assert.True(response.Headers.Contains("Set-Cookie")); + return response.Headers.GetValues("Set-Cookie").FirstOrDefault(); + } + } +} + diff --git a/src/Mvc/test/WebSites/SecurityWebSite/StartupWithRequireAuth.cs b/src/Mvc/test/WebSites/SecurityWebSite/StartupWithRequireAuth.cs new file mode 100644 index 0000000000..6f28bfdd3a --- /dev/null +++ b/src/Mvc/test/WebSites/SecurityWebSite/StartupWithRequireAuth.cs @@ -0,0 +1,44 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNetCore.Authentication.Cookies; +using Microsoft.AspNetCore.Authorization.Policy; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.DependencyInjection; + +namespace SecurityWebSite +{ + public class StartupWithRequireAuth + { + // This method gets called by the runtime. Use this method to add services to the container. + public void ConfigureServices(IServiceCollection services) + { + // Add framework services. + services.AddControllersWithViews().SetCompatibilityVersion(CompatibilityVersion.Latest); + services.AddRazorPages(); + services.AddAntiforgery(); + services.AddAuthentication(CookieAuthenticationDefaults.AuthenticationScheme).AddCookie(options => + { + options.LoginPath = "/Home/Login"; + options.LogoutPath = "/Home/Logout"; + }) + .AddCookie("Cookie2"); + } + + // This method gets called by the runtime. Use this method to configure the HTTP request pipeline. + public void Configure(IApplicationBuilder app) + { + app.UseRouting(); + + app.UseAuthentication(); + app.UseAuthorization(); + + app.UseEndpoints(endpoints => + { + endpoints.MapRazorPages().RequireAuthorization(); + endpoints.MapDefaultControllerRoute().RequireAuthorization(); + }); + } + } +}