Skip to content

Commit 29a1d50

Browse files
pranavkmmkArtakMSFT
pranavkm
authored and
mkArtakMSFT
committed
Throw when UseAuthorization is incorrectly configured
* Update AuthZ & Cors middlewares to only set endpoint routing metadata when executing in the context of endpoint routing * Add analyzers for incorrect UseAuth use Fixes #14049
1 parent 51ae61b commit 29a1d50

23 files changed

+633
-50
lines changed

‎src/Analyzers/Analyzers/src/StartupAnalyzer.Diagnostics.cs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System.Collections.Immutable;
@@ -19,6 +19,7 @@ static Diagnostics()
1919
{
2020
// ASP
2121
BuildServiceProviderShouldNotCalledInConfigureServicesMethod,
22+
IncorrectlyConfiguredAuthorizationMiddleware,
2223

2324
// MVC
2425
UnsupportedUseMvcWithEndpointRouting,
@@ -42,6 +43,15 @@ static Diagnostics()
4243
DiagnosticSeverity.Warning,
4344
isEnabledByDefault: true,
4445
helpLinkUri: "https://aka.ms/YJggeFn");
46+
47+
internal readonly static DiagnosticDescriptor IncorrectlyConfiguredAuthorizationMiddleware = new DiagnosticDescriptor(
48+
"ASP0001",
49+
"Authorization middleware is incorrectly configured.",
50+
"The call to UseAuthorization should appear between app.UseRouting() and app.UseEndpoints(..) for authorization to be correctly evaluated.",
51+
"Usage",
52+
DiagnosticSeverity.Warning,
53+
isEnabledByDefault: true,
54+
helpLinkUri: "https://aka.ms/AA64fv1");
4555
}
4656
}
4757
}

‎src/Analyzers/Analyzers/src/StartupAnalyzer.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
@@ -82,6 +82,7 @@ private void OnCompilationStart(CompilationStartAnalysisContext context)
8282
var analysis = builder.Build();
8383
new UseMvcAnalyzer(analysis).AnalyzeSymbol(context);
8484
new BuildServiceProviderValidator(analysis).AnalyzeSymbol(context);
85+
new UseAuthorizationAnalyzer(analysis).AnalyzeSymbol(context);
8586
});
8687

8788
}, SymbolKind.NamedType);

‎src/Analyzers/Analyzers/src/UseAuthorizationAnalyzer.cs

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using System.Diagnostics;
5+
using Microsoft.CodeAnalysis;
6+
using Microsoft.CodeAnalysis.Diagnostics;
7+
8+
namespace Microsoft.AspNetCore.Analyzers
9+
{
10+
internal class UseAuthorizationAnalyzer
11+
{
12+
private readonly StartupAnalysis _context;
13+
14+
public UseAuthorizationAnalyzer(StartupAnalysis context)
15+
{
16+
_context = context;
17+
}
18+
19+
public void AnalyzeSymbol(SymbolAnalysisContext context)
20+
{
21+
Debug.Assert(context.Symbol.Kind == SymbolKind.NamedType);
22+
Debug.Assert(StartupFacts.IsStartupClass(_context.StartupSymbols, (INamedTypeSymbol)context.Symbol));
23+
24+
var type = (INamedTypeSymbol)context.Symbol;
25+
26+
foreach (var middlewareAnalysis in _context.GetRelatedAnalyses<MiddlewareAnalysis>(type))
27+
{
28+
MiddlewareItem? useAuthorizationItem = default;
29+
MiddlewareItem? useRoutingItem = default;
30+
31+
var length = middlewareAnalysis.Middleware.Length;
32+
for (var i = length - 1; i >= 0; i-- )
33+
{
34+
var middlewareItem = middlewareAnalysis.Middleware[i];
35+
var middleware = middlewareItem.UseMethod.Name;
36+
37+
if (middleware == "UseAuthorization")
38+
{
39+
if (useRoutingItem != null && useAuthorizationItem == null)
40+
{
41+
// This looks like
42+
//
43+
// app.UseAuthorization();
44+
// ...
45+
// app.UseRouting();
46+
// app.UseEndpoints(...);
47+
48+
context.ReportDiagnostic(Diagnostic.Create(
49+
StartupAnalyzer.Diagnostics.IncorrectlyConfiguredAuthorizationMiddleware,
50+
middlewareItem.Operation.Syntax.GetLocation(),
51+
middlewareItem.UseMethod.Name));
52+
}
53+
54+
useAuthorizationItem = middlewareItem;
55+
}
56+
else if (middleware == "UseEndpoints")
57+
{
58+
if (useAuthorizationItem != null)
59+
{
60+
// This configuration looks like
61+
//
62+
// app.UseRouting();
63+
// app.UseEndpoints(...);
64+
// ...
65+
// app.UseAuthorization();
66+
//
67+
68+
context.ReportDiagnostic(Diagnostic.Create(
69+
StartupAnalyzer.Diagnostics.IncorrectlyConfiguredAuthorizationMiddleware,
70+
useAuthorizationItem.Operation.Syntax.GetLocation(),
71+
middlewareItem.UseMethod.Name));
72+
}
73+
}
74+
else if (middleware == "UseRouting")
75+
{
76+
useRoutingItem = middlewareItem;
77+
}
78+
}
79+
}
80+
}
81+
}
82+
}

‎src/Analyzers/Analyzers/test/StartupAnalyzerTest.cs

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ public async Task StartupAnalyzer_MvcOptionsAnalysis_MultipleMiddleware()
162162

163163
Assert.Collection(
164164
middlewareAnalysis.Middleware,
165-
item => Assert.Equal("UseAuthorization", item.UseMethod.Name),
165+
item => Assert.Equal("UseStaticFiles", item.UseMethod.Name),
166166
item => Assert.Equal("UseMiddleware", item.UseMethod.Name),
167167
item => Assert.Equal("UseMvc", item.UseMethod.Name),
168168
item => Assert.Equal("UseRouting", item.UseMethod.Name),
@@ -228,5 +228,90 @@ public async Task StartupAnalyzer_ServicesAnalysis_CallBuildServiceProvider()
228228
AnalyzerAssert.DiagnosticLocation(source.MarkerLocations["MM1"], diagnostic.Location);
229229
});
230230
}
231+
232+
[Fact]
233+
public async Task StartupAnalyzer_UseAuthorizationConfiguredCorrectly_ReportsNoDiagnostics()
234+
{
235+
// Arrange
236+
var source = Read(nameof(TestFiles.StartupAnalyzerTest.UseAuthConfiguredCorrectly));
237+
238+
// Act
239+
var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
240+
241+
// Assert
242+
var middlewareAnalysis = Assert.Single(Analyses.OfType<MiddlewareAnalysis>());
243+
Assert.NotEmpty(middlewareAnalysis.Middleware);
244+
Assert.Empty(diagnostics);
245+
}
246+
247+
[Fact]
248+
public async Task StartupAnalyzer_UseAuthorizationInvokedMultipleTimesInEndpointRoutingBlock_ReportsNoDiagnostics()
249+
{
250+
// Arrange
251+
var source = Read(nameof(TestFiles.StartupAnalyzerTest.UseAuthMultipleTimes));
252+
253+
// Act
254+
var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
255+
256+
// Assert
257+
var middlewareAnalysis = Assert.Single(Analyses.OfType<MiddlewareAnalysis>());
258+
Assert.NotEmpty(middlewareAnalysis.Middleware);
259+
Assert.Empty(diagnostics);
260+
}
261+
262+
[Fact]
263+
public async Task StartupAnalyzer_UseAuthorizationConfiguredBeforeUseRouting_ReportsDiagnostics()
264+
{
265+
// Arrange
266+
var source = Read(nameof(TestFiles.StartupAnalyzerTest.UseAuthBeforeUseRouting));
267+
268+
// Act
269+
var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
270+
271+
// Assert
272+
var middlewareAnalysis = Assert.Single(Analyses.OfType<MiddlewareAnalysis>());
273+
Assert.NotEmpty(middlewareAnalysis.Middleware);
274+
Assert.Collection(diagnostics,
275+
diagnostic =>
276+
{
277+
Assert.Same(StartupAnalyzer.Diagnostics.IncorrectlyConfiguredAuthorizationMiddleware, diagnostic.Descriptor);
278+
AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location);
279+
});
280+
}
281+
282+
[Fact]
283+
public async Task StartupAnalyzer_UseAuthorizationConfiguredAfterUseEndpoints_ReportsDiagnostics()
284+
{
285+
// Arrange
286+
var source = Read(nameof(TestFiles.StartupAnalyzerTest.UseAuthAfterUseEndpoints));
287+
288+
// Act
289+
var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
290+
291+
// Assert
292+
var middlewareAnalysis = Assert.Single(Analyses.OfType<MiddlewareAnalysis>());
293+
Assert.NotEmpty(middlewareAnalysis.Middleware);
294+
Assert.Collection(diagnostics,
295+
diagnostic =>
296+
{
297+
Assert.Same(StartupAnalyzer.Diagnostics.IncorrectlyConfiguredAuthorizationMiddleware, diagnostic.Descriptor);
298+
AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location);
299+
});
300+
}
301+
302+
[Fact]
303+
public async Task StartupAnalyzer_MultipleUseAuthorization_ReportsNoDiagnostics()
304+
{
305+
// Arrange
306+
var source = Read(nameof(TestFiles.StartupAnalyzerTest.UseAuthFallbackPolicy));
307+
308+
// Act
309+
var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
310+
311+
// Assert
312+
var middlewareAnalysis = Assert.Single(Analyses.OfType<MiddlewareAnalysis>());
313+
Assert.NotEmpty(middlewareAnalysis.Middleware);
314+
Assert.Empty(diagnostics);
315+
}
231316
}
232317
}

‎src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcAndConfiguredRoutes.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using Microsoft.AspNetCore.Builder;

‎src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcMultiple.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using Microsoft.AspNetCore.Authorization;
@@ -18,7 +18,7 @@ public void Configure(IApplicationBuilder app)
1818
{
1919
/*MM1*/app.UseMvcWithDefaultRoute();
2020

21-
app.UseAuthorization();
21+
app.UseStaticFiles();
2222
app.UseMiddleware<AuthorizationMiddleware>();
2323

2424
/*MM2*/app.UseMvc();

‎src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcWithOtherMiddleware.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using Microsoft.AspNetCore.Authorization;
@@ -16,7 +16,7 @@ public void ConfigureServices(IServiceCollection services)
1616

1717
public void Configure(IApplicationBuilder app)
1818
{
19-
app.UseAuthorization();
19+
app.UseStaticFiles();
2020
app.UseMiddleware<AuthorizationMiddleware>();
2121

2222
/*MM*/app.UseMvc();

‎src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/UseAuthAfterUseEndpoints.cs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using Microsoft.AspNetCore.Builder;
5+
6+
namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest
7+
{
8+
public class UseAuthAfterUseEndpoints
9+
{
10+
public void Configure(IApplicationBuilder app)
11+
{
12+
app.UseRouting();
13+
app.UseEndpoints(r => { });
14+
/*MM*/app.UseAuthorization();
15+
}
16+
}
17+
}

‎src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/UseAuthBeforeUseRouting.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using Microsoft.AspNetCore.Builder;
5+
6+
namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest
7+
{
8+
public class UseAuthBeforeUseRouting
9+
{
10+
public void Configure(IApplicationBuilder app)
11+
{
12+
app.UseFileServer();
13+
/*MM*/app.UseAuthorization();
14+
app.UseRouting();
15+
app.UseEndpoints(r => { });
16+
}
17+
}
18+
}

‎src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/UseAuthConfiguredCorrectly.cs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using Microsoft.AspNetCore.Builder;
5+
6+
namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest
7+
{
8+
public class UseAuthConfiguredCorrectly
9+
{
10+
public void Configure(IApplicationBuilder app)
11+
{
12+
app.UseRouting();
13+
app.UseAuthorization();
14+
app.UseEndpoints(r => { });
15+
}
16+
}
17+
}

‎src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/UseAuthFallbackPolicy.cs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using Microsoft.AspNetCore.Builder;
5+
6+
namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest
7+
{
8+
public class UseAuthFallbackPolicy
9+
{
10+
public void Configure(IApplicationBuilder app)
11+
{
12+
// 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
13+
// using the second instance for regular endpoint routing based auth. We do not want to produce a warning in this case.
14+
app.UseAuthorization();
15+
app.UseStaticFiles();
16+
17+
app.UseRouting();
18+
app.UseAuthorization();
19+
app.UseEndpoints(r => { });
20+
}
21+
}
22+
}

‎src/Analyzers/Analyzers/test/TestFiles/StartupAnalyzerTest/UseAuthMultipleTimes.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using Microsoft.AspNetCore.Builder;
5+
6+
namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest
7+
{
8+
public class UseAuthMultipleTimes
9+
{
10+
public void Configure(IApplicationBuilder app)
11+
{
12+
app.UseRouting();
13+
app.UseAuthorization();
14+
app.UseAuthorization();
15+
app.UseEndpoints(r => { });
16+
}
17+
}
18+
}

0 commit comments

Comments
 (0)