diff --git a/src/Analyzers/Analyzers/src/StartupAnalyzer.Diagnostics.cs b/src/Analyzers/Analyzers/src/StartupAnalyzer.Diagnostics.cs index 3185c19f34..0ba7239936 100644 --- a/src/Analyzers/Analyzers/src/StartupAnalyzer.Diagnostics.cs +++ b/src/Analyzers/Analyzers/src/StartupAnalyzer.Diagnostics.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// 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.Collections.Immutable; @@ -19,6 +19,7 @@ namespace Microsoft.AspNetCore.Analyzers { // ASP BuildServiceProviderShouldNotCalledInConfigureServicesMethod, + IncorrectlyConfiguredAuthorizationMiddleware, // MVC UnsupportedUseMvcWithEndpointRouting, @@ -42,6 +43,15 @@ namespace Microsoft.AspNetCore.Analyzers DiagnosticSeverity.Warning, isEnabledByDefault: true, helpLinkUri: "https://aka.ms/YJggeFn"); + + internal readonly static DiagnosticDescriptor IncorrectlyConfiguredAuthorizationMiddleware = new DiagnosticDescriptor( + "ASP0001", + "Authorization middleware is incorrectly configured.", + "The call to UseAuthorization should appear between app.UseRouting() and app.UseEndpoints(..) for authorization to be correctly evaluated.", + "Usage", + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + helpLinkUri: "https://aka.ms/AA64fv1"); } } } diff --git a/src/Analyzers/Analyzers/src/StartupAnalyzer.cs b/src/Analyzers/Analyzers/src/StartupAnalyzer.cs index 6deb7dd53e..54d3984bb2 100644 --- a/src/Analyzers/Analyzers/src/StartupAnalyzer.cs +++ b/src/Analyzers/Analyzers/src/StartupAnalyzer.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// 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; @@ -82,6 +82,7 @@ namespace Microsoft.AspNetCore.Analyzers var analysis = builder.Build(); new UseMvcAnalyzer(analysis).AnalyzeSymbol(context); new BuildServiceProviderValidator(analysis).AnalyzeSymbol(context); + new UseAuthorizationAnalyzer(analysis).AnalyzeSymbol(context); }); }, SymbolKind.NamedType); diff --git a/src/Analyzers/Analyzers/src/UseAuthorizationAnalyzer.cs b/src/Analyzers/Analyzers/src/UseAuthorizationAnalyzer.cs new file mode 100644 index 0000000000..6d790ddb47 --- /dev/null +++ b/src/Analyzers/Analyzers/src/UseAuthorizationAnalyzer.cs @@ -0,0 +1,82 @@ +// 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.Diagnostics; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Microsoft.AspNetCore.Analyzers +{ + internal class UseAuthorizationAnalyzer + { + private readonly StartupAnalysis _context; + + public UseAuthorizationAnalyzer(StartupAnalysis context) + { + _context = context; + } + + public void AnalyzeSymbol(SymbolAnalysisContext context) + { + Debug.Assert(context.Symbol.Kind == SymbolKind.NamedType); + Debug.Assert(StartupFacts.IsStartupClass(_context.StartupSymbols, (INamedTypeSymbol)context.Symbol)); + + var type = (INamedTypeSymbol)context.Symbol; + + foreach (var middlewareAnalysis in _context.GetRelatedAnalyses(type)) + { + MiddlewareItem? useAuthorizationItem = default; + MiddlewareItem? useRoutingItem = default; + + var length = middlewareAnalysis.Middleware.Length; + for (var i = length - 1; i >= 0; i-- ) + { + var middlewareItem = middlewareAnalysis.Middleware[i]; + var middleware = middlewareItem.UseMethod.Name; + + if (middleware == "UseAuthorization") + { + if (useRoutingItem != null && useAuthorizationItem == null) + { + // This looks like + // + // app.UseAuthorization(); + // ... + // app.UseRouting(); + // app.UseEndpoints(...); + + context.ReportDiagnostic(Diagnostic.Create( + StartupAnalyzer.Diagnostics.IncorrectlyConfiguredAuthorizationMiddleware, + middlewareItem.Operation.Syntax.GetLocation(), + middlewareItem.UseMethod.Name)); + } + + useAuthorizationItem = middlewareItem; + } + else if (middleware == "UseEndpoints") + { + if (useAuthorizationItem != null) + { + // This configuration looks like + // + // app.UseRouting(); + // app.UseEndpoints(...); + // ... + // app.UseAuthorization(); + // + + context.ReportDiagnostic(Diagnostic.Create( + StartupAnalyzer.Diagnostics.IncorrectlyConfiguredAuthorizationMiddleware, + useAuthorizationItem.Operation.Syntax.GetLocation(), + middlewareItem.UseMethod.Name)); + } + } + else if (middleware == "UseRouting") + { + useRoutingItem = middlewareItem; + } + } + } + } + } +} diff --git a/src/Analyzers/Analyzers/test/StartupAnalyzerTest.cs b/src/Analyzers/Analyzers/test/StartupAnalyzerTest.cs index a03cf6cdf2..00877ee2d3 100644 --- a/src/Analyzers/Analyzers/test/StartupAnalyzerTest.cs +++ b/src/Analyzers/Analyzers/test/StartupAnalyzerTest.cs @@ -162,7 +162,7 @@ namespace Microsoft.AspNetCore.Analyzers Assert.Collection( middlewareAnalysis.Middleware, - item => Assert.Equal("UseAuthorization", item.UseMethod.Name), + item => Assert.Equal("UseStaticFiles", item.UseMethod.Name), item => Assert.Equal("UseMiddleware", item.UseMethod.Name), item => Assert.Equal("UseMvc", item.UseMethod.Name), item => Assert.Equal("UseRouting", item.UseMethod.Name), @@ -228,5 +228,90 @@ namespace Microsoft.AspNetCore.Analyzers AnalyzerAssert.DiagnosticLocation(source.MarkerLocations["MM1"], diagnostic.Location); }); } + + [Fact] + public async Task StartupAnalyzer_UseAuthorizationConfiguredCorrectly_ReportsNoDiagnostics() + { + // Arrange + var source = Read(nameof(TestFiles.StartupAnalyzerTest.UseAuthConfiguredCorrectly)); + + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source.Source); + + // Assert + var middlewareAnalysis = Assert.Single(Analyses.OfType()); + Assert.NotEmpty(middlewareAnalysis.Middleware); + Assert.Empty(diagnostics); + } + + [Fact] + public async Task StartupAnalyzer_UseAuthorizationInvokedMultipleTimesInEndpointRoutingBlock_ReportsNoDiagnostics() + { + // Arrange + var source = Read(nameof(TestFiles.StartupAnalyzerTest.UseAuthMultipleTimes)); + + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source.Source); + + // Assert + var middlewareAnalysis = Assert.Single(Analyses.OfType()); + Assert.NotEmpty(middlewareAnalysis.Middleware); + Assert.Empty(diagnostics); + } + + [Fact] + public async Task StartupAnalyzer_UseAuthorizationConfiguredBeforeUseRouting_ReportsDiagnostics() + { + // Arrange + var source = Read(nameof(TestFiles.StartupAnalyzerTest.UseAuthBeforeUseRouting)); + + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source.Source); + + // Assert + var middlewareAnalysis = Assert.Single(Analyses.OfType()); + Assert.NotEmpty(middlewareAnalysis.Middleware); + Assert.Collection(diagnostics, + diagnostic => + { + Assert.Same(StartupAnalyzer.Diagnostics.IncorrectlyConfiguredAuthorizationMiddleware, diagnostic.Descriptor); + AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location); + }); + } + + [Fact] + public async Task StartupAnalyzer_UseAuthorizationConfiguredAfterUseEndpoints_ReportsDiagnostics() + { + // Arrange + var source = Read(nameof(TestFiles.StartupAnalyzerTest.UseAuthAfterUseEndpoints)); + + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source.Source); + + // Assert + var middlewareAnalysis = Assert.Single(Analyses.OfType()); + Assert.NotEmpty(middlewareAnalysis.Middleware); + Assert.Collection(diagnostics, + diagnostic => + { + Assert.Same(StartupAnalyzer.Diagnostics.IncorrectlyConfiguredAuthorizationMiddleware, diagnostic.Descriptor); + AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location); + }); + } + + [Fact] + public async Task StartupAnalyzer_MultipleUseAuthorization_ReportsNoDiagnostics() + { + // Arrange + var source = Read(nameof(TestFiles.StartupAnalyzerTest.UseAuthFallbackPolicy)); + + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source.Source); + + // Assert + var middlewareAnalysis = Assert.Single(Analyses.OfType()); + Assert.NotEmpty(middlewareAnalysis.Middleware); + Assert.Empty(diagnostics); + } } } diff --git a/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcAndConfiguredRoutes.cs b/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcAndConfiguredRoutes.cs index 89ab994d92..044043751e 100644 --- a/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcAndConfiguredRoutes.cs +++ b/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcAndConfiguredRoutes.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// 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.Builder; diff --git a/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcMultiple.cs b/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcMultiple.cs index c63cb42813..b93b48a9dd 100644 --- a/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcMultiple.cs +++ b/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcMultiple.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// 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.Authorization; @@ -18,7 +18,7 @@ namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest { /*MM1*/app.UseMvcWithDefaultRoute(); - app.UseAuthorization(); + app.UseStaticFiles(); app.UseMiddleware(); /*MM2*/app.UseMvc(); diff --git a/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcWithOtherMiddleware.cs b/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcWithOtherMiddleware.cs index 58005b87f8..841112acfc 100644 --- a/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcWithOtherMiddleware.cs +++ b/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcWithOtherMiddleware.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// 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.Authorization; @@ -16,7 +16,7 @@ namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest public void Configure(IApplicationBuilder app) { - app.UseAuthorization(); + app.UseStaticFiles(); app.UseMiddleware(); /*MM*/app.UseMvc(); diff --git a/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/UseAuthAfterUseEndpoints.cs b/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/UseAuthAfterUseEndpoints.cs new file mode 100644 index 0000000000..91fa7e7231 --- /dev/null +++ b/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/UseAuthAfterUseEndpoints.cs @@ -0,0 +1,17 @@ +// 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.Builder; + +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{ + public class UseAuthAfterUseEndpoints + { + public void Configure(IApplicationBuilder app) + { + app.UseRouting(); + app.UseEndpoints(r => { }); + /*MM*/app.UseAuthorization(); + } + } +} diff --git a/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/UseAuthBeforeUseRouting.cs b/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/UseAuthBeforeUseRouting.cs new file mode 100644 index 0000000000..82aebceb80 --- /dev/null +++ b/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/UseAuthBeforeUseRouting.cs @@ -0,0 +1,18 @@ +// 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.Builder; + +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{ + public class UseAuthBeforeUseRouting + { + public void Configure(IApplicationBuilder app) + { + app.UseFileServer(); + /*MM*/app.UseAuthorization(); + app.UseRouting(); + app.UseEndpoints(r => { }); + } + } +} diff --git a/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/UseAuthConfiguredCorrectly.cs b/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/UseAuthConfiguredCorrectly.cs new file mode 100644 index 0000000000..f34550833a --- /dev/null +++ b/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/UseAuthConfiguredCorrectly.cs @@ -0,0 +1,17 @@ +// 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.Builder; + +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{ + public class UseAuthConfiguredCorrectly + { + public void Configure(IApplicationBuilder app) + { + app.UseRouting(); + app.UseAuthorization(); + app.UseEndpoints(r => { }); + } + } +} diff --git a/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/UseAuthFallbackPolicy.cs b/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/UseAuthFallbackPolicy.cs new file mode 100644 index 0000000000..fde1ffb367 --- /dev/null +++ b/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/UseAuthFallbackPolicy.cs @@ -0,0 +1,22 @@ +// 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.Builder; + +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{ + public class UseAuthFallbackPolicy + { + public void Configure(IApplicationBuilder app) + { + // This sort of setup would be useful if the user wants to use Auth for non-endpoint content to be handled using the Fallback policy, while + // using the second instance for regular endpoint routing based auth. We do not want to produce a warning in this case. + app.UseAuthorization(); + app.UseStaticFiles(); + + app.UseRouting(); + app.UseAuthorization(); + app.UseEndpoints(r => { }); + } + } +} diff --git a/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/UseAuthMultipleTimes.cs b/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/UseAuthMultipleTimes.cs new file mode 100644 index 0000000000..a99ae8cad8 --- /dev/null +++ b/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/UseAuthMultipleTimes.cs @@ -0,0 +1,18 @@ +// 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.Builder; + +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{ + public class UseAuthMultipleTimes + { + public void Configure(IApplicationBuilder app) + { + app.UseRouting(); + app.UseAuthorization(); + app.UseAuthorization(); + app.UseEndpoints(r => { }); + } + } +} diff --git a/src/Components/test/testassets/TestServer/CorsStartup.cs b/src/Components/test/testassets/TestServer/CorsStartup.cs index 419940b88e..a4d91a84aa 100644 --- a/src/Components/test/testassets/TestServer/CorsStartup.cs +++ b/src/Components/test/testassets/TestServer/CorsStartup.cs @@ -21,7 +21,16 @@ namespace TestServer services.AddMvc(); services.AddCors(options => { - options.AddPolicy("AllowAll", _ => { /* Controlled below */ }); + // It's not enough just to return "Access-Control-Allow-Origin: *", because + // browsers don't allow wildcards in conjunction with credentials. So we must + // specify explicitly which origin we want to allow. + + options.AddPolicy("AllowAll", policy => policy + .SetIsOriginAllowed(host => host.StartsWith("http://localhost:") || host.StartsWith("http://127.0.0.1:")) + .AllowAnyHeader() + .WithExposedHeaders("MyCustomHeader") + .AllowAnyMethod() + .AllowCredentials()); }); } @@ -33,18 +42,6 @@ namespace TestServer app.UseDeveloperExceptionPage(); } - // It's not enough just to return "Access-Control-Allow-Origin: *", because - // browsers don't allow wildcards in conjunction with credentials. So we must - // specify explicitly which origin we want to allow. - app.UseCors(policy => - { - policy.SetIsOriginAllowed(host => host.StartsWith("http://localhost:") || host.StartsWith("http://127.0.0.1:")) - .AllowAnyHeader() - .WithExposedHeaders("MyCustomHeader") - .AllowAnyMethod() - .AllowCredentials(); - }); - // Mount the server-side Blazor app on /subdir app.Map("/subdir", app => { @@ -52,12 +49,14 @@ namespace TestServer app.UseClientSideBlazorFiles(); app.UseRouting(); + + app.UseCors(); + app.UseEndpoints(endpoints => { endpoints.MapControllers(); endpoints.MapFallbackToClientSideBlazor("index.html"); }); - }); } } diff --git a/src/Components/test/testassets/TestServer/Startup.cs b/src/Components/test/testassets/TestServer/Startup.cs index eed79f5cba..f110edfadd 100644 --- a/src/Components/test/testassets/TestServer/Startup.cs +++ b/src/Components/test/testassets/TestServer/Startup.cs @@ -1,6 +1,7 @@ using System.IO; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; @@ -68,9 +69,7 @@ namespace TestServer "); var content = writer.ToString(); ctx.Response.ContentLength = content.Length; - using var responseWriter = new StreamWriter(ctx.Response.Body); - await responseWriter.WriteAsync(content); - await responseWriter.FlushAsync(); + await ctx.Response.WriteAsync(content); }); } } diff --git a/src/Http/HttpAbstractions.sln b/src/Http/HttpAbstractions.sln index 10cac06923..e2595c4f40 100644 --- a/src/Http/HttpAbstractions.sln +++ b/src/Http/HttpAbstractions.sln @@ -113,6 +113,10 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.Server EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.WebUtilities.Performance", "WebUtilities\perf\Microsoft.AspNetCore.WebUtilities.Performance\Microsoft.AspNetCore.WebUtilities.Performance.csproj", "{21AC56E7-4E77-4B0E-B63E-C8E836E4D14E}" EndProject +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.Authorization.Policy", "..\Security\Authorization\Policy\src\Microsoft.AspNetCore.Authorization.Policy.csproj", "{8BCAA9EC-0ACD-435C-BF8A-8C843499FF7B}" +EndProject +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.Cors", "..\Middleware\CORS\src\Microsoft.AspNetCore.Cors.csproj", "{09168958-FD5B-4D25-8FBF-75E2C80D903B}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -603,6 +607,30 @@ Global {21AC56E7-4E77-4B0E-B63E-C8E836E4D14E}.Release|x64.Build.0 = Release|Any CPU {21AC56E7-4E77-4B0E-B63E-C8E836E4D14E}.Release|x86.ActiveCfg = Release|Any CPU {21AC56E7-4E77-4B0E-B63E-C8E836E4D14E}.Release|x86.Build.0 = Release|Any CPU + {8BCAA9EC-0ACD-435C-BF8A-8C843499FF7B}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {8BCAA9EC-0ACD-435C-BF8A-8C843499FF7B}.Debug|Any CPU.Build.0 = Debug|Any CPU + {8BCAA9EC-0ACD-435C-BF8A-8C843499FF7B}.Debug|x64.ActiveCfg = Debug|Any CPU + {8BCAA9EC-0ACD-435C-BF8A-8C843499FF7B}.Debug|x64.Build.0 = Debug|Any CPU + {8BCAA9EC-0ACD-435C-BF8A-8C843499FF7B}.Debug|x86.ActiveCfg = Debug|Any CPU + {8BCAA9EC-0ACD-435C-BF8A-8C843499FF7B}.Debug|x86.Build.0 = Debug|Any CPU + {8BCAA9EC-0ACD-435C-BF8A-8C843499FF7B}.Release|Any CPU.ActiveCfg = Release|Any CPU + {8BCAA9EC-0ACD-435C-BF8A-8C843499FF7B}.Release|Any CPU.Build.0 = Release|Any CPU + {8BCAA9EC-0ACD-435C-BF8A-8C843499FF7B}.Release|x64.ActiveCfg = Release|Any CPU + {8BCAA9EC-0ACD-435C-BF8A-8C843499FF7B}.Release|x64.Build.0 = Release|Any CPU + {8BCAA9EC-0ACD-435C-BF8A-8C843499FF7B}.Release|x86.ActiveCfg = Release|Any CPU + {8BCAA9EC-0ACD-435C-BF8A-8C843499FF7B}.Release|x86.Build.0 = Release|Any CPU + {09168958-FD5B-4D25-8FBF-75E2C80D903B}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {09168958-FD5B-4D25-8FBF-75E2C80D903B}.Debug|Any CPU.Build.0 = Debug|Any CPU + {09168958-FD5B-4D25-8FBF-75E2C80D903B}.Debug|x64.ActiveCfg = Debug|Any CPU + {09168958-FD5B-4D25-8FBF-75E2C80D903B}.Debug|x64.Build.0 = Debug|Any CPU + {09168958-FD5B-4D25-8FBF-75E2C80D903B}.Debug|x86.ActiveCfg = Debug|Any CPU + {09168958-FD5B-4D25-8FBF-75E2C80D903B}.Debug|x86.Build.0 = Debug|Any CPU + {09168958-FD5B-4D25-8FBF-75E2C80D903B}.Release|Any CPU.ActiveCfg = Release|Any CPU + {09168958-FD5B-4D25-8FBF-75E2C80D903B}.Release|Any CPU.Build.0 = Release|Any CPU + {09168958-FD5B-4D25-8FBF-75E2C80D903B}.Release|x64.ActiveCfg = Release|Any CPU + {09168958-FD5B-4D25-8FBF-75E2C80D903B}.Release|x64.Build.0 = Release|Any CPU + {09168958-FD5B-4D25-8FBF-75E2C80D903B}.Release|x86.ActiveCfg = Release|Any CPU + {09168958-FD5B-4D25-8FBF-75E2C80D903B}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -651,6 +679,8 @@ Global {611794D2-EF3A-422A-A077-23E61C7ADE49} = {793FFE24-138A-4C3D-81AB-18D625E36230} {1062FCDE-E145-40EC-B175-FDBCAA0C59A0} = {793FFE24-138A-4C3D-81AB-18D625E36230} {21AC56E7-4E77-4B0E-B63E-C8E836E4D14E} = {80A090C8-ED02-4DE3-875A-30DCCDBD84BA} + {8BCAA9EC-0ACD-435C-BF8A-8C843499FF7B} = {793FFE24-138A-4C3D-81AB-18D625E36230} + {09168958-FD5B-4D25-8FBF-75E2C80D903B} = {793FFE24-138A-4C3D-81AB-18D625E36230} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {85B5E151-2E9D-419C-83DD-0DDCF446C83A} diff --git a/src/Http/Routing/src/EndpointMiddleware.cs b/src/Http/Routing/src/EndpointMiddleware.cs index a2101c40cd..9b06b673ad 100644 --- a/src/Http/Routing/src/EndpointMiddleware.cs +++ b/src/Http/Routing/src/EndpointMiddleware.cs @@ -6,7 +6,6 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Cors.Infrastructure; using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Http.Features; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -14,8 +13,8 @@ namespace Microsoft.AspNetCore.Routing { internal sealed class EndpointMiddleware { - internal const string AuthorizationMiddlewareInvokedKey = "__AuthorizationMiddlewareInvoked"; - internal const string CorsMiddlewareInvokedKey = "__CorsMiddlewareInvoked"; + internal const string AuthorizationMiddlewareInvokedKey = "__AuthorizationMiddlewareWithEndpointInvoked"; + internal const string CorsMiddlewareInvokedKey = "__CorsMiddlewareWithEndpointInvoked"; private readonly ILogger _logger; private readonly RequestDelegate _next; @@ -91,7 +90,7 @@ namespace Microsoft.AspNetCore.Routing throw new InvalidOperationException($"Endpoint {endpoint.DisplayName} contains authorization metadata, " + "but a middleware was not found that supports authorization." + Environment.NewLine + - "Configure your application startup by adding app.UseAuthorization() inside the call to Configure(..) in the application startup code."); + "Configure your application startup by adding app.UseAuthorization() inside the call to Configure(..) in the application startup code. The call to app.UseAuthorization() must appear between app.UseRouting() and app.UseEndpoints(...)."); } private static void ThrowMissingCorsMiddlewareException(Endpoint endpoint) @@ -99,7 +98,7 @@ namespace Microsoft.AspNetCore.Routing throw new InvalidOperationException($"Endpoint {endpoint.DisplayName} contains CORS metadata, " + "but a middleware was not found that supports CORS." + Environment.NewLine + - "Configure your application startup by adding app.UseCors() inside the call to Configure(..) in the application startup code."); + "Configure your application startup by adding app.UseCors() inside the call to Configure(..) in the application startup code. The call to app.UseAuthorization() must appear between app.UseRouting() and app.UseEndpoints(...)."); } private static class Log diff --git a/src/Http/Routing/test/FunctionalTests/EndpointRoutingIntegrationTest.cs b/src/Http/Routing/test/FunctionalTests/EndpointRoutingIntegrationTest.cs new file mode 100644 index 0000000000..232d648714 --- /dev/null +++ b/src/Http/Routing/test/FunctionalTests/EndpointRoutingIntegrationTest.cs @@ -0,0 +1,240 @@ +// 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; +using System.Net; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; +using Xunit; + +namespace Microsoft.AspNetCore.Routing.FunctionalTests +{ + public class EndpointRoutingIntegrationTest + { + private static readonly RequestDelegate TestDelegate = async context => await Task.Yield(); + private static readonly string AuthErrorMessage = "Endpoint / contains authorization metadata, but a middleware was not found that supports authorization." + + Environment.NewLine + + "Configure your application startup by adding app.UseAuthorization() inside the call to Configure(..) in the application startup code. " + + "The call to app.UseAuthorization() must appear between app.UseRouting() and app.UseEndpoints(...)."; + + private static readonly string CORSErrorMessage = "Endpoint / contains CORS metadata, but a middleware was not found that supports CORS." + + Environment.NewLine + + "Configure your application startup by adding app.UseCors() inside the call to Configure(..) in the application startup code. " + + "The call to app.UseAuthorization() must appear between app.UseRouting() and app.UseEndpoints(...)."; + + [Fact] + public async Task AuthorizationMiddleware_WhenNoAuthMetadataIsConfigured() + { + // Arrange + var builder = new WebHostBuilder(); + builder.Configure(app => + { + app.UseRouting(); + app.UseAuthorization(); + app.UseEndpoints(b => b.Map("/", TestDelegate)); + }) + .ConfigureServices(services => + { + services.AddAuthorization(); + services.AddRouting(); + }); + + using var server = new TestServer(builder); + + var response = await server.CreateRequest("/").SendAsync("GET"); + + response.EnsureSuccessStatusCode(); + } + + [Fact] + public async Task AuthorizationMiddleware_WhenEndpointIsNotFound() + { + // Arrange + var builder = new WebHostBuilder(); + builder.Configure(app => + { + app.UseRouting(); + app.UseAuthorization(); + app.UseEndpoints(b => b.Map("/", TestDelegate)); + }) + .ConfigureServices(services => + { + services.AddAuthorization(); + services.AddRouting(); + }); + + using var server = new TestServer(builder); + + var response = await server.CreateRequest("/not-found").SendAsync("GET"); + + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + + [Fact] + public async Task AuthorizationMiddleware_WithAuthorizedEndpoint() + { + // Arrange + var builder = new WebHostBuilder(); + builder.Configure(app => + { + app.UseRouting(); + app.UseAuthorization(); + app.UseEndpoints(b => b.Map("/", TestDelegate).RequireAuthorization()); + }) + .ConfigureServices(services => + { + services.AddAuthorization(options => options.DefaultPolicy = new AuthorizationPolicyBuilder().RequireAssertion(_ => true).Build()); + services.AddRouting(); + }); + + using var server = new TestServer(builder); + + var response = await server.CreateRequest("/").SendAsync("GET"); + + response.EnsureSuccessStatusCode(); + } + + [Fact] + public async Task AuthorizationMiddleware_NotConfigured_Throws() + { + // Arrange + var builder = new WebHostBuilder(); + builder.Configure(app => + { + app.UseRouting(); + app.UseEndpoints(b => b.Map("/", TestDelegate).RequireAuthorization()); + + }) + .ConfigureServices(services => + { + services.AddAuthorization(options => options.DefaultPolicy = new AuthorizationPolicyBuilder().RequireAssertion(_ => true).Build()); + services.AddRouting(); + }); + + using var server = new TestServer(builder); + + var ex = await Assert.ThrowsAsync(() => server.CreateRequest("/").SendAsync("GET")); + Assert.Equal(AuthErrorMessage, ex.Message); + } + + [Fact] + public async Task AuthorizationMiddleware_NotConfigured_WhenEndpointIsNotFound() + { + // Arrange + var builder = new WebHostBuilder(); + builder.Configure(app => + { + app.UseRouting(); + app.UseEndpoints(b => b.Map("/", TestDelegate).RequireAuthorization()); + }) + .ConfigureServices(services => + { + services.AddRouting(); + }); + + using var server = new TestServer(builder); + + var response = await server.CreateRequest("/not-found").SendAsync("GET"); + + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + + [Fact] + public async Task AuthorizationMiddleware_ConfiguredBeforeRouting_Throws() + { + // Arrange + var builder = new WebHostBuilder(); + builder.Configure(app => + { + app.UseAuthorization(); + app.UseRouting(); + app.UseEndpoints(b => b.Map("/", TestDelegate).RequireAuthorization()); + }) + .ConfigureServices(services => + { + services.AddAuthorization(options => options.DefaultPolicy = new AuthorizationPolicyBuilder().RequireAssertion(_ => true).Build()); + services.AddRouting(); + }); + + using var server = new TestServer(builder); + + var ex = await Assert.ThrowsAsync(() => server.CreateRequest("/").SendAsync("GET")); + Assert.Equal(AuthErrorMessage, ex.Message); + } + + [Fact] + public async Task AuthorizationMiddleware_ConfiguredAfterRouting_Throws() + { + // Arrange + var builder = new WebHostBuilder(); + builder.Configure(app => + { + app.UseRouting(); + app.UseEndpoints(b => b.Map("/", TestDelegate).RequireAuthorization()); + app.UseAuthorization(); + }) + .ConfigureServices(services => + { + services.AddAuthorization(options => options.DefaultPolicy = new AuthorizationPolicyBuilder().RequireAssertion(_ => true).Build()); + services.AddRouting(); + }); + + using var server = new TestServer(builder); + + var ex = await Assert.ThrowsAsync(() => server.CreateRequest("/").SendAsync("GET")); + Assert.Equal(AuthErrorMessage, ex.Message); + } + + [Fact] + public async Task CorsMiddleware_WithCorsEndpoint() + { + // Arrange + var builder = new WebHostBuilder(); + builder.Configure(app => + { + app.UseRouting(); + app.UseCors(); + app.UseEndpoints(b => b.Map("/", TestDelegate).RequireCors(policy => policy.AllowAnyOrigin())); + }) + .ConfigureServices(services => + { + services.AddCors(); + services.AddRouting(); + }); + + using var server = new TestServer(builder); + + var response = await server.CreateRequest("/").SendAsync("PUT"); + + response.EnsureSuccessStatusCode(); + } + + [Fact] + public async Task CorsMiddleware_ConfiguredBeforeRouting_Throws() + { + // Arrange + var builder = new WebHostBuilder(); + builder.Configure(app => + { + app.UseCors(); + app.UseRouting(); + app.UseEndpoints(b => b.Map("/", TestDelegate).RequireCors(policy => policy.AllowAnyOrigin())); + }) + .ConfigureServices(services => + { + services.AddCors(); + services.AddRouting(); + }); + + using var server = new TestServer(builder); + + var ex = await Assert.ThrowsAsync(() => server.CreateRequest("/").SendAsync("GET")); + Assert.Equal(CORSErrorMessage, ex.Message); + } + } +} diff --git a/src/Http/Routing/test/FunctionalTests/Microsoft.AspNetCore.Routing.FunctionalTests.csproj b/src/Http/Routing/test/FunctionalTests/Microsoft.AspNetCore.Routing.FunctionalTests.csproj index 571658d907..0fd19466b6 100644 --- a/src/Http/Routing/test/FunctionalTests/Microsoft.AspNetCore.Routing.FunctionalTests.csproj +++ b/src/Http/Routing/test/FunctionalTests/Microsoft.AspNetCore.Routing.FunctionalTests.csproj @@ -10,6 +10,8 @@ + + diff --git a/src/Http/Routing/test/UnitTests/EndpointMiddlewareTest.cs b/src/Http/Routing/test/UnitTests/EndpointMiddlewareTest.cs index 1467425222..7a638e7e8c 100644 --- a/src/Http/Routing/test/UnitTests/EndpointMiddlewareTest.cs +++ b/src/Http/Routing/test/UnitTests/EndpointMiddlewareTest.cs @@ -101,7 +101,8 @@ namespace Microsoft.AspNetCore.Routing // Arrange var expected = "Endpoint Test contains authorization metadata, but a middleware was not found that supports authorization." + Environment.NewLine + - "Configure your application startup by adding app.UseAuthorization() inside the call to Configure(..) in the application startup code."; + "Configure your application startup by adding app.UseAuthorization() inside the call to Configure(..) in the application startup code. " + + "The call to app.UseAuthorization() must appear between app.UseRouting() and app.UseEndpoints(...)."; var httpContext = new DefaultHttpContext { RequestServices = new ServiceProvider() @@ -197,7 +198,8 @@ namespace Microsoft.AspNetCore.Routing // Arrange var expected = "Endpoint Test contains CORS metadata, but a middleware was not found that supports CORS." + Environment.NewLine + - "Configure your application startup by adding app.UseCors() inside the call to Configure(..) in the application startup code."; + "Configure your application startup by adding app.UseCors() inside the call to Configure(..) in the application startup code. " + + "The call to app.UseAuthorization() must appear between app.UseRouting() and app.UseEndpoints(...)."; var httpContext = new DefaultHttpContext { RequestServices = new ServiceProvider() diff --git a/src/Middleware/CORS/CORS.slnf b/src/Middleware/CORS/CORS.slnf new file mode 100644 index 0000000000..4112bb1c18 --- /dev/null +++ b/src/Middleware/CORS/CORS.slnf @@ -0,0 +1,10 @@ +{ + "solution": { + "path": "D:\\work\\aspnetcore\\src\\Middleware\\Middleware.sln", + "projects": [ + "CORS\\src\\Microsoft.AspNetCore.Cors.csproj", + "CORS\\test\\UnitTests\\Microsoft.AspNetCore.Cors.Test.csproj", + "CORS\\test\\testassets\\CorsMiddlewareWebSite\\CorsMiddlewareWebSite.csproj" + ] + } +} \ No newline at end of file diff --git a/src/Middleware/CORS/src/Infrastructure/CorsMiddleware.cs b/src/Middleware/CORS/src/Infrastructure/CorsMiddleware.cs index ed3f743dfc..02e5c7a0d1 100644 --- a/src/Middleware/CORS/src/Infrastructure/CorsMiddleware.cs +++ b/src/Middleware/CORS/src/Infrastructure/CorsMiddleware.cs @@ -14,8 +14,8 @@ namespace Microsoft.AspNetCore.Cors.Infrastructure public class CorsMiddleware { // Property key is used by other systems, e.g. MVC, to check if CORS middleware has run - private const string CorsMiddlewareInvokedKey = "__CorsMiddlewareInvoked"; - private static readonly object CorsMiddlewareInvokedValue = new object(); + private const string CorsMiddlewareWithEndpointInvokedKey = "__CorsMiddlewareWithEndpointInvoked"; + private static readonly object CorsMiddlewareWithEndpointInvokedValue = new object(); private readonly Func OnResponseStartingDelegate = OnResponseStarting; private readonly RequestDelegate _next; @@ -116,14 +116,6 @@ namespace Microsoft.AspNetCore.Cors.Infrastructure /// public Task Invoke(HttpContext context, ICorsPolicyProvider corsPolicyProvider) { - // Flag to indicate to other systems, that CORS middleware was run for this request - context.Items[CorsMiddlewareInvokedKey] = CorsMiddlewareInvokedValue; - - if (!context.Request.Headers.ContainsKey(CorsConstants.Origin)) - { - return _next(context); - } - // CORS policy resolution rules: // // 1. If there is an endpoint with IDisableCorsAttribute then CORS is not run @@ -131,9 +123,20 @@ namespace Microsoft.AspNetCore.Cors.Infrastructure // there is an endpoint with IEnableCorsAttribute that has a policy name then // fetch policy by name, prioritizing it above policy on middleware // 3. If there is no policy on middleware then use name on middleware - var endpoint = context.GetEndpoint(); + if (endpoint != null) + { + // EndpointRoutingMiddleware uses this flag to check if the CORS middleware processed CORS metadata on the endpoint. + // The CORS middleware can only make this claim if it observes an actual endpoint. + context.Items[CorsMiddlewareWithEndpointInvokedKey] = CorsMiddlewareWithEndpointInvokedValue; + } + + if (!context.Request.Headers.ContainsKey(CorsConstants.Origin)) + { + return _next(context); + } + // Get the most significant CORS metadata for the endpoint // For backwards compatibility reasons this is then downcast to Enable/Disable metadata var corsMetadata = endpoint?.Metadata.GetMetadata(); diff --git a/src/Middleware/CORS/test/UnitTests/CorsMiddlewareTests.cs b/src/Middleware/CORS/test/UnitTests/CorsMiddlewareTests.cs index ebd44aa2d0..4b53a43a5b 100644 --- a/src/Middleware/CORS/test/UnitTests/CorsMiddlewareTests.cs +++ b/src/Middleware/CORS/test/UnitTests/CorsMiddlewareTests.cs @@ -918,7 +918,7 @@ namespace Microsoft.AspNetCore.Cors.Infrastructure await middleware.Invoke(httpContext, mockProvider); // Assert - Assert.Contains(httpContext.Items, item => string.Equals(item.Key as string, "__CorsMiddlewareInvoked")); + Assert.Contains(httpContext.Items, item => string.Equals(item.Key as string, "__CorsMiddlewareWithEndpointInvoked")); } [Fact] @@ -936,12 +936,37 @@ namespace Microsoft.AspNetCore.Cors.Infrastructure "DefaultPolicyName"); var httpContext = new DefaultHttpContext(); + httpContext.SetEndpoint(new Endpoint(c => Task.CompletedTask, new EndpointMetadataCollection(new EnableCorsAttribute("MetadataPolicyName"), new DisableCorsAttribute()), "Test endpoint")); // Act await middleware.Invoke(httpContext, mockProvider); // Assert - Assert.Contains(httpContext.Items, item => string.Equals(item.Key as string, "__CorsMiddlewareInvoked")); + Assert.Contains(httpContext.Items, item => string.Equals(item.Key as string, "__CorsMiddlewareWithEndpointInvoked")); + } + + [Fact] + public async Task Invoke_WithoutEndpoint_InvokeFlagSet() + { + // Arrange + var corsService = Mock.Of(); + var mockProvider = Mock.Of(); + var loggerFactory = NullLoggerFactory.Instance; + + var middleware = new CorsMiddleware( + Mock.Of(), + corsService, + loggerFactory, + "DefaultPolicyName"); + + var httpContext = new DefaultHttpContext(); + httpContext.Request.Headers.Add(CorsConstants.Origin, new[] { "http://example.com" }); + + // Act + await middleware.Invoke(httpContext, mockProvider); + + // Assert + Assert.DoesNotContain(httpContext.Items, item => string.Equals(item.Key as string, "__CorsMiddlewareWithEndpointInvoked")); } } } diff --git a/src/Security/Authorization/Policy/src/AuthorizationMiddleware.cs b/src/Security/Authorization/Policy/src/AuthorizationMiddleware.cs index 184209557a..686374d829 100644 --- a/src/Security/Authorization/Policy/src/AuthorizationMiddleware.cs +++ b/src/Security/Authorization/Policy/src/AuthorizationMiddleware.cs @@ -13,9 +13,9 @@ namespace Microsoft.AspNetCore.Authorization { public class AuthorizationMiddleware { - // Property key is used by other systems, e.g. MVC, to check if authorization middleware has run - private const string AuthorizationMiddlewareInvokedKey = "__AuthorizationMiddlewareInvoked"; - private static readonly object AuthorizationMiddlewareInvokedValue = new object(); + // Property key is used by Endpoint routing to determine if Authorization has run + private const string AuthorizationMiddlewareInvokedWithEndpointKey = "__AuthorizationMiddlewareWithEndpointInvoked"; + private static readonly object AuthorizationMiddlewareWithEndpointInvokedValue = new object(); private readonly RequestDelegate _next; private readonly IAuthorizationPolicyProvider _policyProvider; @@ -35,8 +35,12 @@ namespace Microsoft.AspNetCore.Authorization var endpoint = context.GetEndpoint(); - // Flag to indicate to other systems, e.g. MVC, that authorization middleware was run for this request - context.Items[AuthorizationMiddlewareInvokedKey] = AuthorizationMiddlewareInvokedValue; + if (endpoint != null) + { + // EndpointRoutingMiddleware uses this flag to check if the Authorization middleware processed auth metadata on the endpoint. + // The Authorization middleware can only make this claim if it observes an actual endpoint. + context.Items[AuthorizationMiddlewareInvokedWithEndpointKey] = AuthorizationMiddlewareWithEndpointInvokedValue; + } // IMPORTANT: Changes to authorization logic should be mirrored in MVC's AuthorizeFilter var authorizeData = endpoint?.Metadata.GetOrderedMetadata() ?? Array.Empty();