Throw when UseAuthorization is incorrectly configured

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 @@ static Diagnostics()
                 {
                     // ASP
                     BuildServiceProviderShouldNotCalledInConfigureServicesMethod,
+                    IncorrectlyConfiguredAuthorizationMiddleware,
 
                     // MVC
                     UnsupportedUseMvcWithEndpointRouting,
@@ -42,6 +43,15 @@ static Diagnostics()
                 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");
         }
     }
 }

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 @@ private void OnCompilationStart(CompilationStartAnalysisContext context)
                     var analysis = builder.Build();
                     new UseMvcAnalyzer(analysis).AnalyzeSymbol(context);
                     new BuildServiceProviderValidator(analysis).AnalyzeSymbol(context);
+                    new UseAuthorizationAnalyzer(analysis).AnalyzeSymbol(context);
                 });
 
             }, SymbolKind.NamedType);

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<MiddlewareAnalysis>(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;
+                    }
+                }
+            }
+        }
+    }
+}

src/Analyzers/Analyzers/test/StartupAnalyzerTest.cs

@@ -162,7 +162,7 @@ public async Task StartupAnalyzer_MvcOptionsAnalysis_MultipleMiddleware()
 
             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 @@ public async Task StartupAnalyzer_ServicesAnalysis_CallBuildServiceProvider()
                     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<MiddlewareAnalysis>());
+            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<MiddlewareAnalysis>());
+            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<MiddlewareAnalysis>());
+            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<MiddlewareAnalysis>());
+            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<MiddlewareAnalysis>());
+            Assert.NotEmpty(middlewareAnalysis.Middleware);
+            Assert.Empty(diagnostics);
+        }
     }
 }

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;

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 @@ public void Configure(IApplicationBuilder app)
         {
             /*MM1*/app.UseMvcWithDefaultRoute();
 
-            app.UseAuthorization();
+            app.UseStaticFiles();
             app.UseMiddleware<AuthorizationMiddleware>();
 
             /*MM2*/app.UseMvc();

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 @@ public void ConfigureServices(IServiceCollection services)
 
         public void Configure(IApplicationBuilder app)
         {
-            app.UseAuthorization();
+            app.UseStaticFiles();
             app.UseMiddleware<AuthorizationMiddleware>();
 
             /*MM*/app.UseMvc();

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();
+        }
+    }
+}

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 => { });
+        }
+    }
+}

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 => { });
+        }
+    }
+}

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 => { });
+        }
+    }
+}

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 => { });
+        }
+    }
+}