ThreeMammals / Ocelot

.NET API Gateway
https://www.nuget.org/packages/Ocelot
MIT License
8.28k stars 1.63k forks source link

AllowedScopes does not match string array in JWT scope claim #913

Open fsmullinslc opened 5 years ago

fsmullinslc commented 5 years ago

Expected Behavior / New Feature

Expected AllowedScopes to parse scope claim and match one or more values in string array similar to RouteClaimsRequirement. For example JWT scope claim value has two scopes, space delimited by whitespace: Values.Read Values.Write

Actual Behavior / Motivation for New Feature

ScopesAuthoriser compares the entire claim value to the AllowedScopes array causing it to fail to match either scope. It looks like you addressed a similar issue in RouteClaimsRequirement. Is that the workaround for this issue? Or, can AllowedScopes be modified to use that behavior?

Steps to Reproduce the Problem

  1. Generate a JWT with "scp" claim containing two or more scopes delimited by whitespace
  2. Send JWT to Ocelot for route with a single AllowedScope
  3. Allowed scope is not matched even though the scope exists in the claim

Specifications

jords1987 commented 4 years ago

Hi, @fsmullinslc @marctalary will this feature be supported in upcoming versions?

fsmullin commented 4 years ago

Hi @jords1987 @marctalary, I am not part of the development team that manages the pull requests. I worked around the issue by using dependency injection. I added a function that is called from ConfigureServices:

        protected void CustomizeOcelot(IServiceCollection services)
        {
            services.RemoveAll<IScopesAuthoriser>();
            services.TryAddSingleton<IScopesAuthoriser, DelimitedScopesAuthorizer>();
        }

Here is the source to the class that will parse the delimited scopes. Hopefully this helps you.

using System.Collections.Generic;
using System.Linq;
using System.Security.Claims;
using Ocelot.Authorisation;
using Ocelot.Infrastructure.Claims.Parser;
using Ocelot.Responses;

namespace MyGateway.Custom.Claims
{
    public class DelimitedScopesAuthorizer : IScopesAuthoriser
    {
        private readonly IClaimsParser _claimsParser;
        private readonly string _scope = "scope";
        private readonly string _msScope = "http://schemas.microsoft.com/identity/claims/scope";

        public DelimitedScopesAuthorizer(IClaimsParser claimsParser)
        {
            _claimsParser = claimsParser;
        }

        public Response<bool> Authorise(ClaimsPrincipal claimsPrincipal, List<string> routeAllowedScopes)
        {
            if (routeAllowedScopes == null || routeAllowedScopes.Count == 0)
            {
                return new OkResponse<bool>(true);
            }

            var values = _claimsParser.GetValuesByClaimType(claimsPrincipal.Claims, _msScope);
            if (!values.IsError && !values.Data.Any())
            {
                values = _claimsParser.GetValuesByClaimType(claimsPrincipal.Claims, _scope);
            }

            if (values.IsError)
            {
                return new ErrorResponse<bool>(values.Errors);
            }

            List<string> userScopes = new List<string>();
            foreach (var item in values.Data)
            {
                if (item.Contains(' '))
                {
                    var scopes = item.Split(' ').ToList();
                    userScopes.AddRange(scopes);
                }
                else
                {
                    userScopes.Add(item);
                }
            }

            var matchesScopes = routeAllowedScopes.Intersect(userScopes).ToList();

            if (matchesScopes.Count == 0)
            {
                return new ErrorResponse<bool>(
                    new ScopeNotAuthorisedError($"no one user scope: '{string.Join(",", userScopes)}' match with some allowed scope: '{string.Join(",", routeAllowedScopes)}'"));
            }

            return new OkResponse<bool>(true);
        }
    }
}
jords1987 commented 4 years ago

@fsmullinslc thanks for sharing, I decided to just live with it and put single allowed scope and an allowed scope that reflects how the token looks but this may be another option. Maybe @TomPallister would know if this feature will be merged?

kaktuspalme commented 4 years ago

I have exactly the same problem. I get the following error message from ocelot:

 warn: Ocelot.Authorisation.Middleware.AuthorisationMiddleware[0]
      requestId: 0HLT9B5KIFEDS:00000001, previousRequestId: no previous request id, message: no one user scope: 'openid profile email read write' match with some allowed scope: 'read' 

I looked into ScopesAuthoriser.cs and it looks like Ocelot doesn't split the scopes claim and so just compares the whole string.

Therefore I forked ScopesAuthoriser locally and changed: var userScopes = values.Data; to var userScopes = values.Data[0].Split(" ");

To let ocelot use my ScopesAuthoriser i added the following line to the startup services.TryAddSingleton<IScopesAuthoriser, ModifiedScopesAuthoriser>();

But I hope it gets fixed upstream and I can delete my modified implementation.

MeysamS commented 1 year ago

"AuthenticationOptions": { "AuthenticationProviderKey": "Bearer", "AllowedScopes": [ "chk-j" ] },

but values.Data is empty !!

raman-m commented 1 year ago

The issue has been accepted due to opened PR #1478