#913 Support RFC 8693 (OAuth 2.0 Token Exchange) for the "scope" claim in ScopesAuthorizer (#1478)

* ScopesAuthorizer refactoring
Scopes can be a space separated list in a single claim. Include this possibility on allowed scopes check.
* Refactoring
* Refactored scope authorization logic to match the intended feature
* Add unit tests for space-separated scope handling in ScopesAuthorizer
* Add acceptance tests for space-separated scope handling in AuthorizationTests
* Refactor ScopesAuthorizer
* Fix AuthorizationTests acceptance tests for space-separated scopes handling
* Added missing changes to AuthorizationTests
* Update ScopesAuthorizer.cs
* Refactor AuthorizationTests to simplify space-separated scope tests and remove redundant cases
* Update src/Ocelot/Authorization/ScopesAuthorizer.cs
* adding reference to rfc 8693, the scope claim is a json string
* Improve code coverage
* Code review by @raman-m
* Update docs
---------
Co-authored-by: Raman Maksimchuk, @raman-m <[email protected]>
Co-authored-by: Guillaume Gnaegi, @ggnaegi <[email protected]>

Author: 3 people

docs/features/authentication.rst

@@ -236,17 +236,26 @@ However, for the “my-service” service, authorization with the ``my-service``
 Allowed Scopes
 --------------
 
-  Option: ``AllowedScopes``
+  | Option: ``AllowedScopes``
+  | Middleware: :ref:`authorization-middleware`
 
 To set up authorization by scopes from the ``AllowedScopes`` collection, after successful authentication by the middleware and after claims have been transformed,
 the authorization middleware in Ocelot retrieves all user claims (from the token) of the '``scope``' type and ensures that the user has at least one of the scopes in the list.
 This provides a way to restrict access to a route on a per-scope basis.
 
-  **Note**: Since version `24.1`_, specifying global *allowed scopes* is exclusively supported.
-  Therefore, only a route-level scheme (i.e., the ``AuthenticationProviderKeys`` array) combined with a route-level ``AllowedScopes`` array can override the global ``AllowedScopes``.
-  Sure, to enable authentication, the ``AllowAnonymous`` option must be set to ``false`` or left undefined.
+.. note::
+  [#f5]_ Depending on the authentication provider, incoming tokens embed the '``scope``' claim value in the body either as an array or as a single space-separated string of multiple values.
+  For instance, :ref:`authentication-identity-server` use an array, whereas most :ref:`authentication-jwt-tokens` providers generate a space-separated list of scopes, in accordance with `RFC 8693`_, as stated in section "`4.2. "scope" (Scopes) Claim`_".
+  Since version `24.1`_, Ocelot supports `RFC 8693`_ (OAuth 2.0 Token Exchange) for the ``scope`` claim in the ``ScopesAuthorizer`` service, also known as the ``IScopesAuthorizer`` service in the DI container.
+
+.. note::
+  Starting with version `24.1`_, specifying global *allowed scopes* is exclusively supported.
+  Be cautious when overriding the global ``AllowedScopes`` array with a route-level ``AllowedScopes`` array;
+  a combination of the route scheme (``AuthenticationProviderKeys`` array) and its *allowed scopes* might be required, since new *allowed scopes* could belong to another authentication provider's security model.
   For more details, refer to the ":ref:`Configuration and AllowAnonymous <authentication-configuration>`" and ":ref:`Global Configuration <authentication-global-configuration>`" sections.
 
+.. _authentication-jwt-tokens:
+
 JWT Tokens
 ----------
 
@@ -380,12 +389,20 @@ Please open a "`Show and tell <https://github.com/ThreeMammals/Ocelot/discussion
 .. [#f2] The ":ref:`Multiple Authentication Schemes <authentication-multiple>`" feature was requested in issues `740`_, `1580`_ and delivered as a part of `23.0`_ release.
 .. [#f3] The global ":ref:`Configuration and AllowAnonymous <authentication-configuration>`" feature for static routes was requested in issues `842`_ and `1414`_, implemented in pull request `2114`_, and officially released in version `24.1`_.
 .. [#f4] The ":ref:`Global Configuration <authentication-global-configuration>`" feature for dynamic routes was requested in issues `585`_ and `2316`_, implemented in pull request `2336`_, and released in version `24.1`_.
+.. [#f5] The ":ref:`authentication-allowed-scopes`" feature fully supports `RFC 8693`_ (OAuth 2.0 Token Exchange) for the ``scope`` claim in the ``ScopesAuthorizer`` service, which is part of the :ref:`authorization-middleware`.
+  Refer to section `4.2. "scope" (Scopes) Claim`_.
+  This enhancement was requested in bug `913`_, fixed in pull request `1478`_, and the patch was rolled out as part of the `24.1`_ release.
+
+.. _RFC 8693: https://datatracker.ietf.org/doc/html/rfc8693
+.. _4.2. "scope" (Scopes) Claim: https://datatracker.ietf.org/doc/html/rfc8693#name-scope-scopes-claim
 
 .. _446: https://github.com/ThreeMammals/Ocelot/issues/446
 .. _585: https://github.com/ThreeMammals/Ocelot/issues/585
 .. _740: https://github.com/ThreeMammals/Ocelot/issues/740
 .. _842: https://github.com/ThreeMammals/Ocelot/issues/842
+.. _913: https://github.com/ThreeMammals/Ocelot/issues/913
 .. _1414: https://github.com/ThreeMammals/Ocelot/issues/1414
+.. _1478: https://github.com/ThreeMammals/Ocelot/pull/1478
 .. _1580: https://github.com/ThreeMammals/Ocelot/issues/1580
 .. _2114: https://github.com/ThreeMammals/Ocelot/pull/2114
 .. _2316: https://github.com/ThreeMammals/Ocelot/issues/2316

src/Ocelot/Authorization/AuthorizationMiddleware.cs

@@ -38,7 +38,7 @@ public async Task Invoke(HttpContext context)
                 return;
             }
 
-            if (!authorized.Data)
+            if (!authorized.Data) // TODO: Looks like this is never called due to the current ScopesAuthorizer design :D Definitely a good reason to refactor
             {
                 var error = new UnauthorizedError($"{context.User.Identity.Name} unable to access route {route.Name()}");
 #if DEBUG

src/Ocelot/Authorization/IScopesAuthorizer.cs

@@ -5,5 +5,21 @@ namespace Ocelot.Authorization;
 
 public interface IScopesAuthorizer
 {
+    /// <summary>
+    /// Checks that the <paramref name="claimsPrincipal"/> and its <see cref="ClaimsPrincipal.Claims"/> collection
+    /// contain at least one <see cref="ScopeClaim"/> value present in the <paramref name="routeAllowedScopes"/> list.
+    /// </summary>
+    /// <remarks>
+    /// Supports the RFC 8693 standard, allowing scope claim values as whitespace-separated strings.<br/>
+    /// RFC 8693 Docs: <see href="https://datatracker.ietf.org/doc/html/rfc8693">OAuth 2.0 Token Exchange</see> | <see href="https://datatracker.ietf.org/doc/html/rfc8693#name-scope-scopes-claim">4.2. "scope" (Scopes) Claim</see>.
+    /// </remarks>
+    /// <exception cref="ScopeNotAuthorizedError">If not authorized.</exception>
+    /// <param name="claimsPrincipal">Claims object from the current authentication provider's token.</param>
+    /// <param name="routeAllowedScopes">List of allowed scopes for the route.</param>
+    /// <returns><see langword="true"/> if any token scope claim value is in the allowed scopes; otherwise, <see langword="false"/>.</returns>
     Response<bool> Authorize(ClaimsPrincipal claimsPrincipal, List<string> routeAllowedScopes);
+
+    /// <summary>Gets the claim type for <c>scope</c>.</summary>
+    /// <value>A <see cref="string"/> representing the <c>scope</c>.</value>
+    string ScopeClaim { get; }
 }

src/Ocelot/Authorization/ScopeNotAuthorizedError.cs

@@ -1,11 +1,12 @@
-using Ocelot.Errors;
+using Microsoft.AspNetCore.Http;
+using Ocelot.Errors;
 
 namespace Ocelot.Authorization;
 
 public class ScopeNotAuthorizedError : Error
 {
     public ScopeNotAuthorizedError(string message)
-        : base(message, OcelotErrorCode.ScopeNotAuthorizedError, 403)
+        : base(message, OcelotErrorCode.ScopeNotAuthorizedError, StatusCodes.Status403Forbidden)
     {
     }
 }

src/Ocelot/Authorization/ScopesAuthorizer.cs

@@ -1,41 +1,54 @@
-using Ocelot.Infrastructure.Claims;
-using Ocelot.Responses;
+using Ocelot.Infrastructure.Claims;
+using Ocelot.Infrastructure.Extensions;
+using Ocelot.Responses;
 using System.Security.Claims;
 
 namespace Ocelot.Authorization;
 
 public class ScopesAuthorizer : IScopesAuthorizer
 {
-    private readonly IClaimsParser _claimsParser;
     public const string Scope = "scope";
+    public const char SpaceChar = (char)32;
+
+    private readonly IClaimsParser _claimsParser;
 
     public ScopesAuthorizer(IClaimsParser claimsParser)
     {
         _claimsParser = claimsParser;
     }
 
+    /// <inheritdoc/>
+    public string ScopeClaim => Scope;
+
+    /// <inheritdoc/>
     public Response<bool> Authorize(ClaimsPrincipal claimsPrincipal, List<string> routeAllowedScopes)
     {
         if (routeAllowedScopes == null || routeAllowedScopes.Count == 0)
         {
             return new OkResponse<bool>(true);
         }
 
-        var values = _claimsParser.GetValuesByClaimType(claimsPrincipal.Claims, Scope);
-
+        var values = _claimsParser.GetValuesByClaimType(claimsPrincipal.Claims, ScopeClaim);
         if (values.IsError)
         {
             return new ErrorResponse<bool>(values.Errors);
         }
 
-        var userScopes = values.Data;
+        IList<string> userScopes = values.Data;
 
-        var matchesScopes = routeAllowedScopes.Intersect(userScopes);
+        // There should not be more than one scope claim that has space-separated value by design
+        // Some providers use array value some space-separated value but not both
+        // https://datatracker.ietf.org/doc/html/rfc8693#name-scope-scopes-claim
+        if (userScopes.Count == 1 && userScopes[0].Contains(SpaceChar))
+        {
+            userScopes = userScopes[0].Split(SpaceChar, StringSplitOptions.RemoveEmptyEntries);
+        }
 
+        var matchesScopes = routeAllowedScopes.Intersect(userScopes);
         if (!matchesScopes.Any())
         {
             return new ErrorResponse<bool>(
-                new ScopeNotAuthorizedError($"no one user scope: '{string.Join(',', userScopes)}' match with some allowed scope: '{string.Join(',', routeAllowedScopes)}'"));
+                new ScopeNotAuthorizedError($"no one user scope: '{userScopes.Csv()}' match with some allowed scope: '{routeAllowedScopes.Csv()}'"));
         }
 
         return new OkResponse<bool>(true);

test/Ocelot.AcceptanceTests/Authentication/AuthenticationSteps.cs

@@ -101,9 +101,12 @@ public void WithJwtBearerAuthentication(IServiceCollection services, bool addOce
         app.MapGet("/connect", () => "Hello! Connected!");
         app.MapPost("/token", (AuthenticationTokenRequest model) =>
         {
-            if (!apiScopes.Contains(model.Scope))
+            // The signing server should be eligible to sign predefined claims as specified in its configuration.
+            // If an unknown scope or claim is requested for inclusion in a JWT, the server should reject the request.
+            // Therefore, the server configuration should be well-known to the client; otherwise, it poses a security risk.
+            if (!apiScopes.Intersect(model.Scopes.Split(' ')).Any())
             {
-                return Results.NotFound();
+                return Results.BadRequest();
             }
             var token = GenerateToken(url, model);
             return Results.Json(token);
@@ -144,7 +147,7 @@ public AuthenticationTokenRequest GivenAuthTokenRequest(string scope,
         {
             Audience = ocelotClient?.BaseAddress.Authority, // Ocelot DNS is token audience
             ApiSecret = testName, // "secret",
-            Scope = scope ?? OcelotScopes.Api,
+            Scopes = scope ?? OcelotScopes.Api,
             Claims = claims is null ? new() : new(claims),
             UserId = testName,
             UserName = testName,
@@ -279,7 +282,7 @@ public static BearerToken GenerateToken(string issuerUrl, AuthenticationTokenReq
             new(JwtRegisteredClaimNames.Sub, auth.UserId),
             new(JwtRegisteredClaimNames.Email, auth.UserName),
             new(JwtRegisteredClaimNames.Jti, Guid.NewGuid().ToString()),
-            new(ScopesAuthorizer.Scope, auth.Scope),
+            new(ScopesAuthorizer.Scope, auth.Scopes),
         };
         claims.AddRange(roleClaims);
         claims.AddRange(userClaims);

test/Ocelot.AcceptanceTests/Authentication/AuthenticationTokenRequest.cs

@@ -28,7 +28,7 @@ public string ApiSecret
     }
 
     [JsonInclude]
-    public string Scope { get; set; }
+    public string Scopes { get; set; }
 
     [JsonInclude]
     public List<KeyValuePair<string, string>> Claims { get; set; } = new();

test/Ocelot.AcceptanceTests/Authorization/AuthorizationTests.cs

@@ -1,5 +1,4 @@
 using Microsoft.AspNetCore.Authentication.JwtBearer;
-using Microsoft.VisualStudio.TestPlatform.ObjectModel;
 using Ocelot.AcceptanceTests.Authentication;
 using Ocelot.Configuration.File;
 using System.Runtime.CompilerServices;
@@ -271,10 +270,51 @@ public async Task Should_return_200_OK_with_global_allowed_scopes()
         await ThenTheResponseBodyAsync();
     }
 
-    private static void Void() { }
+    #region PR 1478
+    [Fact]
+    [Trait("Bug", "913")] // https://github.com/ThreeMammals/Ocelot/issues/913
+    [Trait("PR", "1478")] // https://github.com/ThreeMammals/Ocelot/pull/1478
+    public async Task Should_return_200_OK_with_space_separated_scope_match()
+    {
+        var port = PortFinder.GetRandomPort();
+        var route = GivenAuthRoute(port);
+        route.AuthenticationOptions.AllowedScopes = ["api", "api.read", "api.write"];
+        var configuration = GivenConfiguration(route);
+        GivenThereIsAConfiguration(configuration);
+        await GivenThereIsExternalJwtSigningService("api.read", "openid", "offline_access");
+        GivenThereIsAServiceRunningOn(port);
+        GivenOcelotIsRunning(WithJwtBearerAuthentication);
+        await GivenIHaveATokenWithScope("api.read openid offline_access");
+        GivenIHaveAddedATokenToMyRequest();
+        await WhenIGetUrlOnTheApiGateway("/");
+        ThenTheStatusCodeShouldBeOK();
+        await ThenTheResponseBodyAsync();
+    }
 
-    private async Task GivenIHaveATokenWithScope(string scope, [CallerMemberName] string testName = "")
-        => await GivenIHaveAToken(scope, null, JwtSigningServerUrl, null, testName);
-    private async Task GivenIHaveATokenWithClaims(IEnumerable<KeyValuePair<string, string>> claims, [CallerMemberName] string testName = "")
-        => await GivenIHaveAToken(OcelotScopes.Api, claims, JwtSigningServerUrl, null, testName);
+    [Fact]
+    [Trait("Bug", "913")]
+    [Trait("PR", "1478")]
+    public async Task Should_return_403_Forbidden_with_space_separated_scope_no_match()
+    {
+        var port = PortFinder.GetRandomPort();
+        var route = GivenAuthRoute(port);
+        route.AuthenticationOptions.AllowedScopes = ["admin", "superuser"];
+        var configuration = GivenConfiguration(route);
+        await GivenThereIsExternalJwtSigningService("api.read", "api.write", "openid");
+        GivenThereIsAServiceRunningOn(port);
+        GivenThereIsAConfiguration(configuration);
+        GivenOcelotIsRunning(WithJwtBearerAuthentication);
+        await GivenIHaveATokenWithScope("api.read api.write openid");
+        GivenIHaveAddedATokenToMyRequest();
+        await WhenIGetUrlOnTheApiGateway("/");
+        ThenTheStatusCodeShouldBe(HttpStatusCode.Forbidden);
+    }
+    #endregion PR 1478
+
+    private static void Void() { }
+    private const string DefaultAudience = null;
+    private Task<BearerToken> GivenIHaveATokenWithScope(string scope, [CallerMemberName] string testName = "")
+        => GivenIHaveAToken(scope, null, JwtSigningServerUrl, DefaultAudience, testName);
+    private Task<BearerToken> GivenIHaveATokenWithClaims(IEnumerable<KeyValuePair<string, string>> claims, [CallerMemberName] string testName = "")
+        => GivenIHaveAToken(OcelotScopes.Api, claims, JwtSigningServerUrl, DefaultAudience, testName);
 }

test/Ocelot.UnitTests/Authorization/AuthorizationMiddlewareTests.cs

@@ -30,8 +30,14 @@ public AuthorizationMiddlewareTests()
         _loggerFactory.Setup(x => x.CreateLogger<AuthorizationMiddleware>()).Returns(_logger.Object);
         _next = context => Task.CompletedTask;
         _middleware = new AuthorizationMiddleware(_next, _claimsAuthorizer.Object, _scopesAuthorizer.Object, _loggerFactory.Object);
+
+        _logger.Setup(x => x.LogWarning(It.IsAny<Func<string>>()))
+            .Callback<Func<string>>(_warnings.Add);
     }
 
+    private readonly List<Func<string>> _warnings = new();
+    private List<string> GetWarnings() => _warnings.Select(w => w()).ToList();
+
     [Fact]
     [Trait("Feat", "100")] // https://github.com/ThreeMammals/Ocelot/issues/100
     [Trait("PR", "104")] // https://github.com/ThreeMammals/Ocelot/pull/104
@@ -75,6 +81,39 @@ public async Task Should_call_authorization_service()
         ThenClaimsAuthorizerIsCalled();
     }
 
+    [Fact]
+    [Trait("Feat", "100")] // https://github.com/ThreeMammals/Ocelot/issues/100
+    [Trait("PR", "104")] // https://github.com/ThreeMammals/Ocelot/pull/104
+    [Trait("Release", "1.4.5")] // https://github.com/ThreeMammals/Ocelot/releases/tag/1.4.5
+    public async Task Invoke_RouteIsAuthenticated_WhenScopesAuthorizerError_ShouldUpsertErrors()
+    {
+        // Arrange
+        var route = new DownstreamRouteBuilder()
+            .WithAuthenticationOptions(new(new("authScheme")))
+            .WithUpstreamPathTemplate(new UpstreamPathTemplateBuilder().WithOriginalValue("/test").Build())
+            .Build();
+        var response = new ErrorResponse<bool>(new ScopeNotAuthorizedError("No match"));
+        GivenTheDownStreamRouteIs(new(), route);
+        GivenScopesAuthorizerReturns(response);
+
+        // Act
+        await _middleware.Invoke(_httpContext);
+
+        // Assert
+        ThenScopesAuthorizerIsCalled();
+#if DEBUG
+        _logger.Verify(x => x.LogWarning(It.IsAny<Func<string>>()), Times.Once);
+        var warnings = GetWarnings();
+        Assert.Contains($"The '/test' route encountered authorization errors due to user scopes:{Environment.NewLine}ScopeNotAuthorizedError: No match", warnings);
+#endif
+        var errors = _httpContext.Items.Errors();
+        Assert.NotEmpty(errors);
+        Assert.Contains(response.Errors[0], errors);
+        var actual = Assert.Single(errors);
+        Assert.Same(response.Errors[0], actual);
+        Assert.Equal("No match", actual.Message);
+    }
+
     private void GivenTheDownStreamRouteIs(List<PlaceholderNameAndValue> templatePlaceholderNameAndValues, DownstreamRoute downstreamRoute)
     {
         _httpContext.Items.UpsertTemplatePlaceholderNameAndValues(templatePlaceholderNameAndValues);