diff --git a/src/Analyzers/Analyzers/src/UseAuthorizationAnalyzer.cs b/src/Analyzers/Analyzers/src/UseAuthorizationAnalyzer.cs index 6d790ddb47..dbfa9a3dcb 100644 --- a/src/Analyzers/Analyzers/src/UseAuthorizationAnalyzer.cs +++ b/src/Analyzers/Analyzers/src/UseAuthorizationAnalyzer.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Diagnostics; +using System.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Diagnostics; @@ -27,6 +28,7 @@ namespace Microsoft.AspNetCore.Analyzers { MiddlewareItem? useAuthorizationItem = default; MiddlewareItem? useRoutingItem = default; + MiddlewareItem? useEndpoint = default; var length = middlewareAnalysis.Middleware.Length; for (var i = length - 1; i >= 0; i-- ) @@ -70,9 +72,24 @@ namespace Microsoft.AspNetCore.Analyzers useAuthorizationItem.Operation.Syntax.GetLocation(), middlewareItem.UseMethod.Name)); } + + useEndpoint = middlewareItem; } else if (middleware == "UseRouting") { + if (useEndpoint is null) + { + // We're likely here because the middleware uses an expression chain e.g. + // app.UseRouting() + // .UseAuthorization() + // .UseEndpoints(..)); + // This analyzer expects MiddlewareItem instances to appear in the order in which they appear in source + // which unfortunately isn't true for chained calls (the operations appear in reverse order). + // We'll avoid doing any analysis in this event and rely on the runtime guardrails. + // We'll use https://github.com/aspnet/AspNetCore/issues/16648 to track addressing this in a future milestone + return; + } + useRoutingItem = middlewareItem; } } diff --git a/src/Analyzers/Analyzers/test/StartupAnalyzerTest.cs b/src/Analyzers/Analyzers/test/StartupAnalyzerTest.cs index 00877ee2d3..fc635ef7f2 100644 --- a/src/Analyzers/Analyzers/test/StartupAnalyzerTest.cs +++ b/src/Analyzers/Analyzers/test/StartupAnalyzerTest.cs @@ -244,6 +244,22 @@ namespace Microsoft.AspNetCore.Analyzers Assert.Empty(diagnostics); } + [Fact] + public async Task StartupAnalyzer_UseAuthorizationConfiguredAsAChain_ReportsNoDiagnostics() + { + // Regression test for https://github.com/aspnet/AspNetCore/issues/15203 + // Arrange + var source = Read(nameof(TestFiles.StartupAnalyzerTest.UseAuthConfiguredCorrectlyChained)); + + // 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() { @@ -279,6 +295,23 @@ namespace Microsoft.AspNetCore.Analyzers }); } + [Fact] + public async Task StartupAnalyzer_UseAuthorizationConfiguredBeforeUseRoutingChained_ReportsDiagnostics() + { + // This one asserts a false negative for https://github.com/aspnet/AspNetCore/issues/15203. + // We don't correctly identify chained calls, this test verifies the behavior. + // Arrange + var source = Read(nameof(TestFiles.StartupAnalyzerTest.UseAuthBeforeUseRoutingChained)); + + // 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_UseAuthorizationConfiguredAfterUseEndpoints_ReportsDiagnostics() { diff --git a/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/UseAuthBeforeUseRoutingChained.cs b/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/UseAuthBeforeUseRoutingChained.cs new file mode 100644 index 0000000000..d76ecd0997 --- /dev/null +++ b/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/UseAuthBeforeUseRoutingChained.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 UseAuthBeforeUseRoutingChained + { + public void Configure(IApplicationBuilder app) + { + app.UseFileServer() + .UseAuthorization() + .UseRouting() + .UseEndpoints(r => { }); + } + } +} diff --git a/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/UseAuthConfiguredCorrectlyChained.cs b/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/UseAuthConfiguredCorrectlyChained.cs new file mode 100644 index 0000000000..c7d170e750 --- /dev/null +++ b/src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/UseAuthConfiguredCorrectlyChained.cs @@ -0,0 +1,16 @@ +// 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 UseAuthConfiguredCorrectlyChained + { + public void Configure(IApplicationBuilder app) + { + app.UseRouting() + .UseAuthorization() + .UseEndpoints(r => { }); + } + } +}