Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
fsmullinslc opened this issue Jun 4, 2019 · 6 comments · May be fixed by #1478
Open

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

fsmullinslc opened this issue Jun 4, 2019 · 6 comments · May be fixed by #1478
Assignees
Labels
accepted Bug or feature would be accepted as a PR or is being worked on bug Identified as a potential bug proposal Proposal for a new functionality in Ocelot

Comments

@fsmullinslc
Copy link

fsmullinslc commented Jun 4, 2019

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

  • Version: Ocelot 13.5.0 and 19.0.2
  • Platform: .NET Core 2.2 and .NET 7
@jords1987
Copy link

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

@fsmullin
Copy link

fsmullin commented Nov 26, 2019

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
Copy link

@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
Copy link

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.

@mehyaa mehyaa linked a pull request May 31, 2021 that will close this issue
@MeysamS
Copy link

MeysamS commented Nov 20, 2022

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

but values.Data is empty !!

@raman-m raman-m added bug Identified as a potential bug proposal Proposal for a new functionality in Ocelot accepted Bug or feature would be accepted as a PR or is being worked on labels Aug 24, 2023
@raman-m
Copy link
Member

raman-m commented Aug 24, 2023

The issue has been accepted due to opened PR #1478

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Bug or feature would be accepted as a PR or is being worked on bug Identified as a potential bug proposal Proposal for a new functionality in Ocelot
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants