Quick code review from @raman-m

src/Ocelot/Authorization/IRolesAuthorizer.cs

@@ -1,11 +1,9 @@
-using System.Collections.Generic;
+using Ocelot.Responses;
 using System.Security.Claims;
-using Ocelot.Responses;
 
-namespace Ocelot.Authorization
+namespace Ocelot.Authorization;
+
+public interface IRolesAuthorizer
 {
-    public interface IRolesAuthorizer
-    {
-        Response<bool> Authorize(ClaimsPrincipal claimsPrincipal, List<string> routeRequiredRole, string roleKey);
-    }
+    Response<bool> Authorize(ClaimsPrincipal claimsPrincipal, List<string> routeRequiredRole, string roleKey);
 }

src/Ocelot/Authorization/IScopesAuthorizer.cs

@@ -1,10 +1,10 @@
 using Ocelot.Responses;
 using System.Security.Claims;
 
-namespace Ocelot.Authorization
+namespace Ocelot.Authorization;
+
+public interface IScopesAuthorizer
 {
-    public interface IScopesAuthorizer
-    {
-        Response<bool> Authorize(ClaimsPrincipal claimsPrincipal, List<string> routeAllowedScopes, string scopeKey);
-    }
+    Response<bool> Authorize(ClaimsPrincipal claimsPrincipal, List<string> routeAllowedScopes,
+        string scopeKey);
 }

src/Ocelot/Authorization/Middleware/AuthorizationMiddleware.cs

@@ -28,16 +28,16 @@ public AuthorizationMiddleware(RequestDelegate next,
 
         // Note roles is a duplicate of scopes - should refactor based on type
         // Note scopes and roles are processed as OR
-        // todo create logic to process policies that we use in the API
+        // TODO: Create logic to process policies that we use in the API
         public async Task Invoke(HttpContext httpContext)
         {
             var downstreamRoute = httpContext.Items.DownstreamRoute();
-
+            var options = downstreamRoute.AuthenticationOptions;
             if (!IsOptionsHttpMethod(httpContext) && IsAuthenticatedRoute(downstreamRoute))
             {
                 Logger.LogInformation("route is authenticated scopes must be checked");
 
-                var authorized = _scopesAuthorizer.Authorize(httpContext.User, downstreamRoute.AuthenticationOptions.AllowedScopes, downstreamRoute.AuthenticationOptions.ScopeKey);
+                var authorized = _scopesAuthorizer.Authorize(httpContext.User, options.AllowedScopes, options.ScopeKey);
 
                 if (authorized.IsError)
                 {
@@ -64,7 +64,7 @@ public async Task Invoke(HttpContext httpContext)
             {
                 Logger.LogInformation("route and scope is authenticated role must be checked");
 
-                var authorizedRole = _rolesAuthorizer.Authorize(httpContext.User, downstreamRoute.AuthenticationOptions.RequiredRole, downstreamRoute.AuthenticationOptions.RoleKey);
+                var authorizedRole = _rolesAuthorizer.Authorize(httpContext.User, options.RequiredRole, options.RoleKey);
 
                 if (authorizedRole.IsError)
                 {

src/Ocelot/Authorization/RolesAuthorizer.cs

@@ -1,47 +1,44 @@
-using System.Collections.Generic;
-using System.Linq;
-using System.Security.Claims;
-using Ocelot.Infrastructure.Claims.Parser;
+using Ocelot.Infrastructure.Claims.Parser;
 using Ocelot.Responses;
+using System.Security.Claims;
 
-namespace Ocelot.Authorization
+namespace Ocelot.Authorization;
+
+public class RolesAuthorizer : IRolesAuthorizer
 {
-    public class RolesAuthorizer : IRolesAuthorizer
+    private readonly IClaimsParser _claimsParser;
+
+    public RolesAuthorizer(IClaimsParser claimsParser)
     {
-        private readonly IClaimsParser _claimsParser;
+        _claimsParser = claimsParser;
+    }
 
-        public RolesAuthorizer(IClaimsParser claimsParser)
+    public Response<bool> Authorize(ClaimsPrincipal claimsPrincipal, List<string> routeRequiredRole, string roleKey)
+    {
+        if (routeRequiredRole == null || routeRequiredRole.Count == 0)
         {
-            _claimsParser = claimsParser;
+            return new OkResponse<bool>(true);
         }
 
-        public Response<bool> Authorize(ClaimsPrincipal claimsPrincipal, List<string> routeRequiredRole, string roleKey)
-        {
-            if (routeRequiredRole == null || routeRequiredRole.Count == 0)
-            {
-                return new OkResponse<bool>(true);
-            }
+        roleKey ??= "role";
 
-            roleKey ??= "role";
+        var values = _claimsParser.GetValuesByClaimType(claimsPrincipal.Claims, roleKey);
 
-            var values = _claimsParser.GetValuesByClaimType(claimsPrincipal.Claims, roleKey);
-
-            if (values.IsError)
-            {
-                return new ErrorResponse<bool>(values.Errors);
-            }
-
-            var userRoles = values.Data;
+        if (values.IsError)
+        {
+            return new ErrorResponse<bool>(values.Errors);
+        }
 
-            var matchedRoles = routeRequiredRole.Intersect(userRoles).ToList(); // Note this is an OR
+        var userRoles = values.Data;
 
-            if (matchedRoles.Count == 0)
-            {
-                return new ErrorResponse<bool>(
-                    new ScopeNotAuthorizedError($"no one user role: '{string.Join(",", userRoles)}' match with some allowed role: '{string.Join(",", routeRequiredRole)}'"));
-            }
+        var matchedRoles = routeRequiredRole.Intersect(userRoles).ToList(); // Note this is an OR
 
-            return new OkResponse<bool>(true);
+        if (matchedRoles.Count == 0)
+        {
+            return new ErrorResponse<bool>(
+                new ScopeNotAuthorizedError($"no one user role: '{string.Join(",", userRoles)}' match with some allowed role: '{string.Join(",", routeRequiredRole)}'"));
         }
+
+        return new OkResponse<bool>(true);
     }
 }

src/Ocelot/Configuration/AuthenticationOptions.cs

@@ -1,5 +1,4 @@
 using Ocelot.Configuration.File;
-using System;
 
 namespace Ocelot.Configuration
 {

src/Ocelot/Configuration/Builder/AuthenticationOptionsBuilder.cs

@@ -1,5 +1,3 @@
-using System;
-
 namespace Ocelot.Configuration.Builder
 {
     public class AuthenticationOptionsBuilder

src/Ocelot/Configuration/Creator/AuthenticationOptionsCreator.cs

@@ -1,12 +1,8 @@
 using Ocelot.Configuration.File;
 
-namespace Ocelot.Configuration.Creator
+namespace Ocelot.Configuration.Creator;
+
+public class AuthenticationOptionsCreator : IAuthenticationOptionsCreator
 {
-    public class AuthenticationOptionsCreator : IAuthenticationOptionsCreator
-    {
-        public AuthenticationOptions Create(FileRoute route)
-        {
-            return new AuthenticationOptions(route.AuthenticationOptions);
-        }
-    }
+    public AuthenticationOptions Create(FileRoute route) => new(route.AuthenticationOptions);
 }

test/Ocelot.UnitTests/Infrastructure/RoleAuthorizerTests.cs

@@ -1,116 +1,109 @@
-using Moq;
 using Ocelot.Authorization;
-using Ocelot.Errors;
 using Ocelot.Infrastructure.Claims.Parser;
 using Ocelot.Responses;
-using Shouldly;
-using System.Collections.Generic;
 using System.Security.Claims;
-using TestStack.BDDfy;
-using Xunit;
 
-namespace Ocelot.UnitTests.Infrastructure
+namespace Ocelot.UnitTests.Infrastructure;
+
+public class RoleAuthorizerTests : UnitTest
 {
-    public class RoleAuthorizerTests
-    {
-        private readonly RolesAuthorizer _authorizer;
-        private readonly Mock<IClaimsParser> _parser;
-        private ClaimsPrincipal _principal;
-        private List<string> _requiredRole;
-        private Response<bool> _result;
+    private readonly RolesAuthorizer _authorizer;
+    private readonly Mock<IClaimsParser> _parser;
+    private ClaimsPrincipal _principal;
+    private List<string> _requiredRole;
+    private Response<bool> _result;
 
-        public RoleAuthorizerTests()
-        {
-            _parser = new Mock<IClaimsParser>();
-            _authorizer = new RolesAuthorizer(_parser.Object);
-        }
+    public RoleAuthorizerTests()
+    {
+        _parser = new Mock<IClaimsParser>();
+        _authorizer = new RolesAuthorizer(_parser.Object);
+    }
 
-        [Fact]
-        public void Should_return_ok_if_no_allowed_scopes()
-        {
-            this.Given(_ => GivenTheFollowing(new ClaimsPrincipal()))
-            .And(_ => GivenTheFollowing(new List<string>()))
-            .When(_ => WhenIAuthorize())
-            .Then(_ => ThenTheFollowingIsReturned(new OkResponse<bool>(true)))
-            .BDDfy();
-        }
+    [Fact]
+    public void Should_return_ok_if_no_allowed_scopes()
+    {
+        this.Given(_ => GivenTheFollowing(new ClaimsPrincipal()))
+        .And(_ => GivenTheFollowing(new List<string>()))
+        .When(_ => WhenIAuthorize())
+        .Then(_ => ThenTheFollowingIsReturned(new OkResponse<bool>(true)))
+        .BDDfy();
+    }
 
-        [Fact]
-        public void Should_return_ok_if_null_allowed_scopes()
-        {
-            this.Given(_ => GivenTheFollowing(new ClaimsPrincipal()))
-            .And(_ => GivenTheFollowing((List<string>)null))
-            .When(_ => WhenIAuthorize())
-            .Then(_ => ThenTheFollowingIsReturned(new OkResponse<bool>(true)))
-            .BDDfy();
-        }
+    [Fact]
+    public void Should_return_ok_if_null_allowed_scopes()
+    {
+        this.Given(_ => GivenTheFollowing(new ClaimsPrincipal()))
+        .And(_ => GivenTheFollowing((List<string>)null))
+        .When(_ => WhenIAuthorize())
+        .Then(_ => ThenTheFollowingIsReturned(new OkResponse<bool>(true)))
+        .BDDfy();
+    }
 
-        [Fact]
-        public void Should_return_error_if_claims_parser_returns_error()
-        {
-            var fakeError = new FakeError();
-            this.Given(_ => GivenTheFollowing(new ClaimsPrincipal()))
-            .And(_ => GivenTheParserReturns(new ErrorResponse<List<string>>(fakeError)))
-            .And(_ => GivenTheFollowing(new List<string>() { "doesntmatter" }))
-            .When(_ => WhenIAuthorize())
-            .Then(_ => ThenTheFollowingIsReturned(new ErrorResponse<bool>(fakeError)))
-            .BDDfy();
-        }
+    [Fact]
+    public void Should_return_error_if_claims_parser_returns_error()
+    {
+        var fakeError = new FakeError();
+        this.Given(_ => GivenTheFollowing(new ClaimsPrincipal()))
+        .And(_ => GivenTheParserReturns(new ErrorResponse<List<string>>(fakeError)))
+        .And(_ => GivenTheFollowing(new List<string>() { "doesntmatter" }))
+        .When(_ => WhenIAuthorize())
+        .Then(_ => ThenTheFollowingIsReturned(new ErrorResponse<bool>(fakeError)))
+        .BDDfy();
+    }
 
-        [Fact]
-        public void Should_match_role_and_return_ok_result()
-        {
-            var claimsPrincipal = new ClaimsPrincipal();
-            var requiredRole = new List<string>() { "someRole" };
+    [Fact]
+    public void Should_match_role_and_return_ok_result()
+    {
+        var claimsPrincipal = new ClaimsPrincipal();
+        var requiredRole = new List<string>() { "someRole" };
 
-            this.Given(_ => GivenTheFollowing(claimsPrincipal))
-            .And(_ => GivenTheParserReturns(new OkResponse<List<string>>(requiredRole)))
-            .And(_ => GivenTheFollowing(requiredRole))
-            .When(_ => WhenIAuthorize())
-            .Then(_ => ThenTheFollowingIsReturned(new OkResponse<bool>(true)))
-            .BDDfy();
-        }
+        this.Given(_ => GivenTheFollowing(claimsPrincipal))
+        .And(_ => GivenTheParserReturns(new OkResponse<List<string>>(requiredRole)))
+        .And(_ => GivenTheFollowing(requiredRole))
+        .When(_ => WhenIAuthorize())
+        .Then(_ => ThenTheFollowingIsReturned(new OkResponse<bool>(true)))
+        .BDDfy();
+    }
 
-        [Fact]
-        public void Should_not_match_role_and_return_error_result()
-        {
-            var fakeError = new FakeError();
-            var claimsPrincipal = new ClaimsPrincipal();
-            var requiredRole = new List<string>() { "someRole" };
-            var userRoles = new List<string>() { "anotherRole" };
+    [Fact]
+    public void Should_not_match_role_and_return_error_result()
+    {
+        var fakeError = new FakeError();
+        var claimsPrincipal = new ClaimsPrincipal();
+        var requiredRole = new List<string>() { "someRole" };
+        var userRoles = new List<string>() { "anotherRole" };
 
-            this.Given(_ => GivenTheFollowing(claimsPrincipal))
-            .And(_ => GivenTheParserReturns(new OkResponse<List<string>>(userRoles)))
-            .And(_ => GivenTheFollowing(requiredRole))
-            .When(_ => WhenIAuthorize())
-            .Then(_ => ThenTheFollowingIsReturned(new ErrorResponse<bool>(fakeError)))
-            .BDDfy();
-        }
+        this.Given(_ => GivenTheFollowing(claimsPrincipal))
+        .And(_ => GivenTheParserReturns(new OkResponse<List<string>>(userRoles)))
+        .And(_ => GivenTheFollowing(requiredRole))
+        .When(_ => WhenIAuthorize())
+        .Then(_ => ThenTheFollowingIsReturned(new ErrorResponse<bool>(fakeError)))
+        .BDDfy();
+    }
 
-        private void GivenTheParserReturns(Response<List<string>> response)
-        {
-            _parser.Setup(x => x.GetValuesByClaimType(It.IsAny<IEnumerable<Claim>>(), It.IsAny<string>())).Returns(response);
-        }
+    private void GivenTheParserReturns(Response<List<string>> response)
+    {
+        _parser.Setup(x => x.GetValuesByClaimType(It.IsAny<IEnumerable<Claim>>(), It.IsAny<string>())).Returns(response);
+    }
 
-        private void GivenTheFollowing(ClaimsPrincipal principal)
-        {
-            _principal = principal;
-        }
+    private void GivenTheFollowing(ClaimsPrincipal principal)
+    {
+        _principal = principal;
+    }
 
-        private void GivenTheFollowing(List<string> requiredRole)
-        {
-            _requiredRole = requiredRole;
-        }
+    private void GivenTheFollowing(List<string> requiredRole)
+    {
+        _requiredRole = requiredRole;
+    }
 
-        private void WhenIAuthorize()
-        {
-            _result = _authorizer.Authorize(_principal, _requiredRole, null);
-        }
+    private void WhenIAuthorize()
+    {
+        _result = _authorizer.Authorize(_principal, _requiredRole, null);
+    }
 
-        private void ThenTheFollowingIsReturned(Response<bool> expected)
-        {
-            _result.Data.ShouldBe(expected.Data);
-            _result.IsError.ShouldBe(expected.IsError);
-        }
+    private void ThenTheFollowingIsReturned(Response<bool> expected)
+    {
+        _result.Data.ShouldBe(expected.Data);
+        _result.IsError.ShouldBe(expected.IsError);
     }
 }