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

Multiple values for single key in RouteClaimsRequirement #746

Open
Stians92 opened this issue Jan 14, 2019 · 31 comments · May be fixed by #2131
Open

Multiple values for single key in RouteClaimsRequirement #746

Stians92 opened this issue Jan 14, 2019 · 31 comments · May be fixed by #2131
Assignees
Labels
accepted Bug or feature would be accepted as a PR or is being worked on Authorization Ocelot feature: Authorization Autumn'24 Autumn 2024 release Configuration Ocelot feature: Configuration feature A new feature high High priority
Milestone

Comments

@Stians92
Copy link

Stians92 commented Jan 14, 2019

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 philproctor added feature A new feature proposal Proposal for a new functionality in Ocelot labels Jan 16, 2019
@philproctor
Copy link
Contributor

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

@royayan1988
Copy link

Is this fixed?

@kuzdu
Copy link

kuzdu commented Jun 8, 2019

I'm interested too. Does another workaround exist?

@feugen24
Copy link

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

@arro000
Copy link

arro000 commented Sep 6, 2019

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

arro000 commented Sep 6, 2019

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

hrishikeshtt commented May 4, 2020

May I please know any update on this?

@MuhammadSohaibSultan
Copy link

MuhammadSohaibSultan commented Jul 1, 2020

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

tokenValidationParameters

What is tokenValidationParameters?

@arro000
Copy link

arro000 commented Sep 25, 2020

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

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

eeefbal commented Jan 19, 2021

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

@ntruongvux
Copy link

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

@default-kaas
Copy link

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

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

arro000 commented Sep 27, 2021

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

Simkiw commented Jan 21, 2022

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

@SistemasInfinitos
Copy link

Still no update on the topic ?

@olssonp
Copy link

olssonp commented Jul 5, 2022

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

raman-m commented May 10, 2023

@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.

@raman-m raman-m unpinned this issue May 11, 2023
@raman-m raman-m added help wanted Not actively being worked on. If you plan to contribute, please drop a note. good first issue Should be pretty easy to do labels Jan 7, 2024
@asrasya-kosha
Copy link

asrasya-kosha commented Mar 30, 2024

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

raman-m commented Apr 1, 2024

@asrasya-kosha Will you open a PR?

@asrasya-kosha
Copy link

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

@raman-m raman-m added the medium effort Likely a few days of development effort label Apr 2, 2024
@raman-m raman-m assigned asrasya-kosha and unassigned Stians92 Apr 2, 2024
@raman-m
Copy link
Member

raman-m commented Apr 2, 2024

@asrasya-kosha commented on Apr 1

You are assigned!
Good luck and lots of inspiration!

@pietrocarpinacci-dufercodev

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

@raman-m
Copy link
Member

raman-m commented Jul 18, 2024

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

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

raman-m commented Jul 19, 2024

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

@asrasya-kosha
Copy link

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 asrasya-kosha linked a pull request Jul 31, 2024 that will close this issue
@asrasya-kosha
Copy link

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

@raman-m raman-m added accepted Bug or feature would be accepted as a PR or is being worked on Configuration Ocelot feature: Configuration Authorization Ocelot feature: Authorization Oct'24 October 2024 release and removed help wanted Not actively being worked on. If you plan to contribute, please drop a note. good first issue Should be pretty easy to do medium effort Likely a few days of development effort proposal Proposal for a new functionality in Ocelot labels Jul 31, 2024
@raman-m raman-m added this to the Summer'24 milestone Jul 31, 2024
@raman-m
Copy link
Member

raman-m commented Jul 31, 2024

@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.

@raman-m raman-m added the high High priority label Jul 31, 2024
@raman-m raman-m added Autumn'24 Autumn 2024 release and removed Oct'24 October 2024 release labels Oct 26, 2024
@raman-m raman-m modified the milestones: October'24, Autumn'24 Oct 26, 2024
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 Authorization Ocelot feature: Authorization Autumn'24 Autumn 2024 release Configuration Ocelot feature: Configuration feature A new feature high High priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.