ThreeMammals / Ocelot

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

Multiple values for single key in RouteClaimsRequirement #746

Open Stians92 opened 5 years ago

Stians92 commented 5 years ago

New Feature

Allow claims to have an array value and not just a string value.

Motivation for New Feature

I have an application where I have multiple roles for my users. For endpoints that should only be reached by admins, I can use the following:

"RouteClaimsRequirement": {
    "Role": "Admin"
}

For endpoints that should only be reached by users, I can use the following:

"RouteClaimsRequirement": {
    "Role": "User"
}

For endpoints that should be reached by both users and admins I would like to use the following:

"RouteClaimsRequirement": {
    "Role": ["User", "Admin"]
}

When I tried adding this, all requests to the endpoint respond with a 404.

This should let the request go through if the request has a claim for the role user or admin. This is equivalent to the built-in asp .net core attribute: [Authorize(Roles = "User, Admin")]

https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.authorization.authorizeattribute.roles?view=aspnetcore-2.2

philproctor commented 5 years ago

I believe adding more sophisticated route claims requirements in general would help with this, will discuss with the team best approaches.

royayan1988 commented 5 years ago

Is this fixed?

kuzdu commented 5 years ago

I'm interested too. Does another workaround exist?

feugen24 commented 5 years ago

as workaround override authorisation middleware for claims or policy based with claims in policies as suggested here

arro000 commented 5 years ago

I've tried the override authorisation middleware method , but the claims are strictly converted in a Dictionary<string,string> format before the middleware was called ;

This format break the Route because the value cannot be converted into a string;

"RouteClaimsRequirement": {
    "Role": ["User", "Admin"]
}

This format also don't work because the Role key in dictionary will be ovverride with the second value;

"RouteClaimsRequirement": {
    "Role": "Admin"
        "Role": "User" 
}

so.. My personal solution will be Workaround example

"RouteClaimsRequirement": {
    "Role": "Admin , User"
}

In the authorisation middleware method , i will parse the string with a regex pattern to obtain the single role value:

  public async void Configure(IApplicationBuilder app, IHostingEnvironment env, ILoggerFactory loggerFactory)
        {
            var configuration = new OcelotPipelineConfiguration
            {
                AuthorizationMiddleware = async (ctx, next) =>
                {
                    if (this.Authorize(ctx))
                    {
                        await next.Invoke();

                    }
                    else {
                       // ctx.Errors.Add(new UnauthorisedError($"Fail to authorize"));
                    }

                }
            };
            .
            .
            .
            await app.UseOcelot(configuration);
      }

The logic of Authorize Method

 private bool Authorize(HttpContext ctx)
        {
           DownstreamRoute route = (DownstreamRoute)ctx.Items["DownstreamRoute"];
            string key = route.AuthenticationOptions.AuthenticationProviderKey;

            if (key == null || key == "") return true;
            if (route.RouteClaimsRequirement.Count == 0) return true;
            else
            {
            else {
                //flag for authorization
                bool auth = false;

                //where are stored the claims of the jwt token
                Claim[] claims = ctx.User.Claims.ToArray<Claim>();

                //where are stored the required claims for the route
                Dictionary<string, string> required = route.RouteClaimsRequirement;
                .
                .
                ((AUTHORIZATION LOGIC))
                .
                .
                return auth;
           }

remeber to add in the ConfigureService method

 services.AddAuthorization();
services.AddAuthentication()
                    .AddJwtBearer("TestKey", x =>
                    {
                      //  x.RequireHttpsMetadata = false;
                        x.TokenValidationParameters = tokenValidationParameters;
                    });

(I Still working on my Authorization logic that will implement the multiple claims with And/Or logic with regex of strings , but the claims data structure implemented with Dictionary<string , string> is very ugly and not very flexible)

//updated for last version of ocelot

arro000 commented 5 years ago

((AUTHORIZATION LOGIC)) Example

                Regex reor = new Regex(@"[^,\s+$ ][^\,]*[^,\s+$ ]");
                MatchCollection matches;

                Regex reand = new Regex(@"[^&\s+$ ][^\&]*[^&\s+$ ]");
                MatchCollection matchesand;
                int cont = 0;
                foreach (KeyValuePair<string, string> claim in required)
                {
                    matches = reor.Matches(claim.Value);
                    foreach (Match match in matches)
                    {
                        matchesand = reand.Matches(match.Value);
                        cont = 0;
                        foreach (Match m in matchesand)
                        {
                            foreach (Claim cl in claims)
                            {
                                if (cl.Type == claim.Key)
                                {
                                    if (cl.Value == m.Value)
                                    {
                                        cont++;
                                    }
                                }
                            }
                        }
                        if (cont == matchesand.Count)
                        {
                            return true;
                            // break;
                        }
                    }
                }

Example of Configuration

"RouteClaimsRequirement": {
    "Role": "User & IT , Admin"
}

Logic result :
IF((User and IT) or Admin)
hrishikeshtt commented 4 years ago

May I please know any update on this?

MuhammadSohaibSultan commented 4 years ago

by using @arro000 's trick I changed a few things to make it work for me on .Net Core 3.1 & Ocelot 16.0.1. My answer on Stackoverflow. https://stackoverflow.com/questions/60300349/how-to-check-claim-value-in-array-or-any-in-ocelot-gateway/62390542#62390542

pateljeel commented 3 years ago
tokenValidationParameters

What is tokenValidationParameters?

arro000 commented 3 years ago

Is the instance of object with the bearer authentication rules, in my case for example:


 var tokenValidationParameters = new TokenValidationParameters
            {
                ValidateIssuer = false,
                ValidateAudience = false,
                ValidateLifetime = true,
                ValidateIssuerSigningKey = true,

                IssuerSigningKey = signingKey
            };
pateljeel commented 3 years ago

((AUTHORIZATION LOGIC)) Example

                Regex reor = new Regex(@"[^,\s+$ ][^\,]*[^,\s+$ ]");
                MatchCollection matches;

                Regex reand = new Regex(@"[^&\s+$ ][^\&]*[^&\s+$ ]");
                MatchCollection matchesand;
                int cont=0;
                foreach (KeyValuePair<string,string> claim in required)
                {
                    matches = reor.Matches(claim.Value);
                    foreach (Match match in matches)
                    {
                        matchesand = reand.Matches(match.Value); 
              cont = 0;
                        foreach (Match m in matchesand)
                        {
                            foreach (Claim cl in claims) 
                            {
                                if (cl.Type == claim.Key)
                                {
                                    if (cl.Value == m.Value)
                                    {
                                        cont++;
                                    }
                                }
                            }
                        }
                        if (cont == matchesand.Count) 
                        {
              auth= true;
              break;
          }
                    }
                }

Example of Configuration

"RouteClaimsRequirement": { "Role": "User & IT , Admin" }

Logic result : IF((User and IT) or Admin)

With this change do you still need to add [Authorize(Roles = "User, Admin")] to your controllers?

eeefbal commented 3 years ago

Hi, is there any push new commit for this issue?

ntruongvux commented 3 years ago

@arro000 Should used PreAuthorizationMiddleware and AuthorizationMiddleware in OcelotPipelineConfiguration to check RequirementClaims?

default-kaas commented 3 years ago

I believe adding more sophisticated route claims requirements in general would help with this, will discuss with the team best approaches.

Is it clear when this enhancement will be added.

I am asking this question, because it has been over two years since this enhancement was requested.

default-kaas commented 3 years ago

I've tried the override authorisation middleware method , but the claims are strictly converted in a Dictionary<string,string> format before the middleware was called ;

This format break the Route because the value cannot be converted into a string;

"RouteClaimsRequirement": { "Role": ["User", "Admin"] } This format also don't work because the Role key in dictionary will be ovverride with the second value;

"RouteClaimsRequirement": { "Role": "Admin" "Role": "User" } so.. My personal solution will be Workaround example

"RouteClaimsRequirement": { "Role": "Admin , User" } In the authorisation middleware method , i will parse the string with a regex pattern to obtain the single role value:

  public async void Configure(IApplicationBuilder app, IHostingEnvironment env, ILoggerFactory loggerFactory)
        {
            var configuration = new OcelotPipelineConfiguration
            {
                AuthorisationMiddleware = async (ctx, next) =>
                {
                    if (this.Authorize(ctx))
                    {
                        await next.Invoke();

                    }
                    else {
                        ctx.Errors.Add(new UnauthorisedError($"Fail to authorize"));
                    }

                }
            };
            .
            .
            .
            await app.UseOcelot(configuration);
      }

The logic of Authorize Method

 private bool Authorize(DownstreamContext ctx)
        {
            if (ctx.DownstreamReRoute.AuthenticationOptions.AuthenticationProviderKey == null) return true;
            else {
                //flag for authorization
                bool auth = false;

                //where are stored the claims of the jwt token
                Claim[] claims = ctx.HttpContext.User.Claims.ToArray<Claim>();

                //where are stored the required claims for the route
                Dictionary<string, string> required = ctx.DownstreamReRoute.RouteClaimsRequirement;
                .
                .
                ((AUTHORIZATION LOGIC))
                .
                .
                return auth;
           }

remeber to add in the ConfigureService method

 services.AddAuthorization();
services.AddAuthentication()
                    .AddJwtBearer("TestKey", x =>
                    {
                      //  x.RequireHttpsMetadata = false;
                        x.TokenValidationParameters = tokenValidationParameters;
                    });

(I Still working on my Authorization logic that will implement the multiple claims with And/Or logic with regex of strings , but the claims data structure implemented with Dictionary<string , string> is very ugly and not very flexible)

From where do you get the AuthorisationMiddleware, DownstreamContext and UnauthorisedError.

arro000 commented 2 years ago

I made this script for ocelot version 13, I updated my comment making it compatible with the latest version 17 AuthorisationMiddleware replaced by AuthorizationMiddleware DownstreamContext = replaced by DownstreamRoute route = (DownstreamRoute)ctx.Items["DownstreamRoute"]; you can get route information from HttpContext UnauthorizedError from Ocelot.Authorization but i commented that line because HttpContext dosn't have Error resposne list like DownstreamContext

Simkiw commented 2 years ago

No? Still no update on the topic ? (if we can avoid the workaround, would be better)

SistemasInfinitos commented 2 years ago

Still no update on the topic ?

olssonp commented 2 years ago

Any progess on this issue? Ocelot works great for authorization regarding client apps (via audience, scopes etc.) but for anything user related via claims (roles etc) we seem to be lacking proper config tools. A fleshed out version of 'RouteClaimsRequirement' would be greatly appreciated.

raman-m commented 1 year ago

@Stians92 Mr. Stian,

Do you have any solutions to fix this your opened issue over 4 years ago? Pay attention that the issue reporter can prioritize solutions and resolve any discussions.

asrasya-kosha commented 5 months ago

Hi,

I was able to make a few changes on this and get this to work for static claims by updating the ClaimsAuthorizer.cs.

The changes are basically updating all the Dictionary<string, string> routeClaimsRequirement to Dictionary<string, string[]> routeClaimsRequirement and then updating the ClaimsAuthorizer.cs's Authorize() to

            public Response<bool> Authorize(
            ClaimsPrincipal claimsPrincipal,
            Dictionary<string, string[]> routeClaimsRequirement,
            List<PlaceholderNameAndValue> urlPathPlaceholderNameAndValues
        )
        {
            foreach (var required in routeClaimsRequirement)
            {
                var values = _claimsParser.GetValuesByClaimType(claimsPrincipal.Claims, required.Key);

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

                if (values.Data != null)
                {
                    // dynamic claim
                    var match = Regex.Match(required.Value, @"^{(?<variable>.+)}$"); <-- how this line is supposed to be changed?
                    ...........
                    ...........
                 }
                    else
                    {
                        // static claim
                        var authorized = required.Value.Any(x=> values.Data.Contains(x));
                        if (!authorized)
                        {
                            return new ErrorResponse<bool>(new ClaimValueNotAuthorizedError(
                                       $"claim value: {string.Join(", ", values.Data)} is not the same as required value: {required.Value} for type: {required.Key}"));
                        }
                    }
                }
                else
                {
                    return new ErrorResponse<bool>(new UserDoesNotHaveClaimError($"user does not have claim {required.Key}"));
                }
            }

            return new OkResponse<bool>(true);
        }

However, I am confused what should be the approach on the Dynamic claims? This doesn't seem to be applicable to the dynamic claim section, in my opinion.

I did change it to


var match = Regex.Match(required.Value!.FirstOrDefault() ?? string.Empty, "^{(?<variable>.+)}$");

which basically takes the first configured dynamic placeholder and moves on but am not sure if this is a good approach. Let me know your thoughts.

raman-m commented 5 months ago

@asrasya-kosha Will you open a PR?

asrasya-kosha commented 5 months ago

@raman-m yes, that is the intention. Thanks.

raman-m commented 5 months ago

@asrasya-kosha commented on Apr 1

You are assigned! Good luck and lots of inspiration!

pietrocarpinacci-dufercodev commented 1 month ago

Hi, is there any update about this feature? It is very interesting in my use case!

raman-m commented 1 month ago

Dear Pietro (@pietrocarpinacci-dufercodev), Asrasya (@asrasya-kosha), do you remember that you are assigned? 😉

I am eager to see your contributions. Should you submit a PR with robust code and tests, I will give the issue priority. Currently, there are no PRs linked to this longstanding issue. However, it could be expedited with professional contributions from the community.

Project Coordinator

pietrocarpinacci-dufercodev commented 1 month ago

Hi Raman (@raman-m), I'm sorry but right now me and my company don't have time to dedicate on this. Maybe in october...

Best Regards, Pietro

raman-m commented 1 month ago

@asrasya-kosha Asrasya, now it's your turn! 😉

asrasya-kosha commented 1 month ago

I did start working on this but lost track because of work commitments.I will try and devote some time in next two weeks.

asrasya-kosha commented 1 month ago

@raman-m PR #2131 available for review and discussion please. Let me know your comments and thoughts.

raman-m commented 1 month ago

@asrasya-kosha Fantastic, thank you, Asrasya! 🤩 The issue and PR have been included in the upcoming milestone, set for release after the current v23.3 Hotfixes. I will concentrate on the v23.3 Hotfixes now and will return to the PR afterward.