#746 Multiple values in static claims #2131
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,7 @@ public ClaimsAuthorizer(IClaimsParser claimsParser) | |
|
|
||
| public Response<bool> Authorize( | ||
| ClaimsPrincipal claimsPrincipal, | ||
| Dictionary<string, string[]> routeClaimsRequirement, | ||
| List<PlaceholderNameAndValue> urlPathPlaceholderNameAndValues | ||
| ) | ||
| { | ||
|
|
@@ -32,7 +32,7 @@ List<PlaceholderNameAndValue> urlPathPlaceholderNameAndValues | |
| if (values.Data != null) | ||
| { | ||
| // dynamic claim | ||
| var match = Regex.Match(required.Value!.FirstOrDefault() ?? string.Empty, "^{(?<variable>.+)}$"); | ||
|
There was a problem hiding this comment. That's not your code, I agree, Maybe we could use a compiled Regex and enhance security by specifying a timeout, 10 seconds... [GeneratedRegex("^{(?<variable>.+)}$", RegexOptions.Compiled, 10000)]
private static partial Regex DynamicClaimRegex();There was a problem hiding this comment. in GenerateRegex, RegexOptions.Compiled is ignored There was a problem hiding this comment. We have well known open PR by Mohsen who followed |
||
| if (match.Success) | ||
| { | ||
| var variableName = match.Captures[0].Value; | ||
|
|
@@ -67,7 +67,7 @@ List<PlaceholderNameAndValue> urlPathPlaceholderNameAndValues | |
| else | ||
| { | ||
| // static claim | ||
| var authorized = required.Value.Any(x=> values.Data.Contains(x)); | ||
| if (!authorized) | ||
| { | ||
| return new ErrorResponse<bool>(new ClaimValueNotAuthorizedError( | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -13,7 +13,7 @@ public class DownstreamRouteBuilder | |||||
| private bool _isAuthenticated; | ||||||
| private List<ClaimToThing> _claimsToHeaders; | ||||||
| private List<ClaimToThing> _claimToClaims; | ||||||
| private Dictionary<string, string[]> _routeClaimRequirement; | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| private bool _isAuthorized; | ||||||
| private List<ClaimToThing> _claimToQueries; | ||||||
| private List<ClaimToThing> _claimToDownstreamPath; | ||||||
|
|
@@ -126,7 +126,7 @@ public DownstreamRouteBuilder WithClaimsToClaims(List<ClaimToThing> input) | |||||
| return this; | ||||||
| } | ||||||
|
|
||||||
| public DownstreamRouteBuilder WithRouteClaimsRequirement(Dictionary<string, string[]> input) | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| { | ||||||
| _routeClaimRequirement = input; | ||||||
| return this; | ||||||
|
|
||||||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,4 @@ | ||||||||
| using Ocelot.Configuration.Creator; | ||||||||
| using Ocelot.Values; | ||||||||
|
|
||||||||
| namespace Ocelot.Configuration | ||||||||
|
|
@@ -23,7 +23,7 @@ public DownstreamRoute( | |||||||
| CacheOptions cacheOptions, | ||||||||
| LoadBalancerOptions loadBalancerOptions, | ||||||||
| RateLimitOptions rateLimitOptions, | ||||||||
| Dictionary<string, string[]> routeClaimsRequirement, | ||||||||
| List<ClaimToThing> claimsToQueries, | ||||||||
| List<ClaimToThing> claimsToHeaders, | ||||||||
| List<ClaimToThing> claimsToClaims, | ||||||||
|
|
@@ -99,7 +99,7 @@ public DownstreamRoute( | |||||||
| public CacheOptions CacheOptions { get; } | ||||||||
| public LoadBalancerOptions LoadBalancerOptions { get; } | ||||||||
| public RateLimitOptions RateLimitOptions { get; } | ||||||||
| public Dictionary<string, string[]> RouteClaimsRequirement { get; } | ||||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||||
| public List<ClaimToThing> ClaimsToQueries { get; } | ||||||||
| public List<ClaimToThing> ClaimsToHeaders { get; } | ||||||||
| public List<ClaimToThing> ClaimsToClaims { get; } | ||||||||
|
|
||||||||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,31 +1,31 @@ | ||||||
| using Ocelot.Configuration.Creator; | ||||||
|
|
||||||
| namespace Ocelot.Configuration.File | ||||||
| { | ||||||
| public class FileRoute : IRoute, ICloneable | ||||||
| { | ||||||
| public FileRoute() | ||||||
| { | ||||||
| AddClaimsToRequest = new Dictionary<string, string>(); | ||||||
| AddHeadersToRequest = new Dictionary<string, string>(); | ||||||
| AddQueriesToRequest = new Dictionary<string, string>(); | ||||||
| AuthenticationOptions = new FileAuthenticationOptions(); | ||||||
| ChangeDownstreamPathTemplate = new Dictionary<string, string>(); | ||||||
| DelegatingHandlers = new List<string>(); | ||||||
| DownstreamHeaderTransform = new Dictionary<string, string>(); | ||||||
| DownstreamHostAndPorts = new List<FileHostAndPort>(); | ||||||
| FileCacheOptions = new FileCacheOptions(); | ||||||
| HttpHandlerOptions = new FileHttpHandlerOptions(); | ||||||
| LoadBalancerOptions = new FileLoadBalancerOptions(); | ||||||
| Metadata = new Dictionary<string, string>(); | ||||||
| Priority = 1; | ||||||
| QoSOptions = new FileQoSOptions(); | ||||||
| RateLimitOptions = new FileRateLimitRule(); | ||||||
| RouteClaimsRequirement = new Dictionary<string, string[]>(); | ||||||
| SecurityOptions = new FileSecurityOptions(); | ||||||
| UpstreamHeaderTemplates = new Dictionary<string, string>(); | ||||||
| UpstreamHeaderTransform = new Dictionary<string, string>(); | ||||||
| UpstreamHttpMethod = new List<string>(); | ||||||
| } | ||||||
|
|
||||||
| public FileRoute(FileRoute from) | ||||||
|
|
@@ -44,30 +44,30 @@ public FileRoute(FileRoute from) | |||||
| public List<FileHostAndPort> DownstreamHostAndPorts { get; set; } | ||||||
| public string DownstreamHttpMethod { get; set; } | ||||||
| public string DownstreamHttpVersion { get; set; } | ||||||
|
|
||||||
| /// <summary>The <see cref="HttpVersionPolicy"/> enum specifies behaviors for selecting and negotiating the HTTP version for a request.</summary> | ||||||
| /// <value>A <see langword="string" /> value of defined <see cref="VersionPolicies"/> constants.</value> | ||||||
| /// <remarks> | ||||||
| /// Related to the <see cref="DownstreamHttpVersion"/> property. | ||||||
| /// <list type="bullet"> | ||||||
| /// <item><see href="https://learn.microsoft.com/en-us/dotnet/api/system.net.http.httpversionpolicy">HttpVersionPolicy Enum</see></item> | ||||||
| /// <item><see href="https://learn.microsoft.com/en-us/dotnet/api/system.net.httpversion">HttpVersion Class</see></item> | ||||||
| /// <item><see href="https://learn.microsoft.com/en-us/dotnet/api/system.net.http.httprequestmessage.versionpolicy">HttpRequestMessage.VersionPolicy Property</see></item> | ||||||
| /// </list> | ||||||
| /// </remarks> | ||||||
| public string DownstreamHttpVersionPolicy { get; set; } | ||||||
| public string DownstreamPathTemplate { get; set; } | ||||||
| public string DownstreamScheme { get; set; } | ||||||
| public FileCacheOptions FileCacheOptions { get; set; } | ||||||
| public FileHttpHandlerOptions HttpHandlerOptions { get; set; } | ||||||
| public string Key { get; set; } | ||||||
| public FileLoadBalancerOptions LoadBalancerOptions { get; set; } | ||||||
| public IDictionary<string, string> Metadata { get; set; } | ||||||
| public int Priority { get; set; } | ||||||
| public FileQoSOptions QoSOptions { get; set; } | ||||||
| public FileRateLimitRule RateLimitOptions { get; set; } | ||||||
| public string RequestIdKey { get; set; } | ||||||
| public Dictionary<string, string[]> RouteClaimsRequirement { get; set; } | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| public bool RouteIsCaseSensitive { get; set; } | ||||||
| public FileSecurityOptions SecurityOptions { get; set; } | ||||||
| public string ServiceName { get; set; } | ||||||
|
|
@@ -103,14 +103,14 @@ public static void DeepCopy(FileRoute from, FileRoute to) | |||||
| to.DownstreamHostAndPorts = from.DownstreamHostAndPorts.Select(x => new FileHostAndPort(x)).ToList(); | ||||||
| to.DownstreamHttpMethod = from.DownstreamHttpMethod; | ||||||
| to.DownstreamHttpVersion = from.DownstreamHttpVersion; | ||||||
| to.DownstreamHttpVersionPolicy = from.DownstreamHttpVersionPolicy; | ||||||
| to.DownstreamPathTemplate = from.DownstreamPathTemplate; | ||||||
| to.DownstreamScheme = from.DownstreamScheme; | ||||||
| to.FileCacheOptions = new(from.FileCacheOptions); | ||||||
| to.HttpHandlerOptions = new(from.HttpHandlerOptions); | ||||||
| to.Key = from.Key; | ||||||
| to.LoadBalancerOptions = new(from.LoadBalancerOptions); | ||||||
| to.Metadata = new Dictionary<string, string>(from.Metadata); | ||||||
| to.Priority = from.Priority; | ||||||
| to.QoSOptions = new(from.QoSOptions); | ||||||
| to.RateLimitOptions = new(from.RateLimitOptions); | ||||||
|
|
@@ -127,5 +127,5 @@ public static void DeepCopy(FileRoute from, FileRoute to) | |||||
| to.UpstreamHttpMethod = new(from.UpstreamHttpMethod); | ||||||
| to.UpstreamPathTemplate = from.UpstreamPathTemplate; | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,7 +75,7 @@ public void should_return_response_200_authorizing_route() | |
| }, | ||
| RouteClaimsRequirement = | ||
| { | ||
| {"UserType",new []{ "registered" }}, | ||
| }, | ||
| }, | ||
| }, | ||
|
|
@@ -134,7 +134,7 @@ public void should_return_response_403_authorizing_route() | |
| }, | ||
| RouteClaimsRequirement = | ||
| { | ||
| {"UserType",new []{ "registered" }}, | ||
| }, | ||
| }, | ||
| }, | ||
|
|
@@ -266,7 +266,7 @@ public void should_fix_issue_240() | |
| }, | ||
| RouteClaimsRequirement = | ||
| { | ||
| {"Role",new []{ "User" }}, | ||
| }, | ||
| }, | ||
| }, | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -64,15 +64,15 @@ private void GivenTheAuthServiceReturns(Response<bool> expected) | |
| _authService | ||
| .Setup(x => x.Authorize( | ||
| It.IsAny<ClaimsPrincipal>(), | ||
| It.IsAny<Dictionary<string, string[]>>(), | ||
| It.IsAny<List<PlaceholderNameAndValue>>())) | ||
| .Returns(expected); | ||
| } | ||
|
|
||
| private void ThenTheAuthServiceIsCalledCorrectly() | ||
| { | ||
| _authService.Verify( | ||
| x => x.Authorize(It.IsAny<ClaimsPrincipal>(), It.IsAny<Dictionary<string, string[]>>(), It.IsAny<List<PlaceholderNameAndValue>>()), | ||
| Times.Once); | ||
| } | ||
| } | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,16 @@ | ||
| using Ocelot.Authorization; | ||
| using Ocelot.DownstreamRouteFinder.UrlMatcher; | ||
| using Ocelot.Infrastructure.Claims.Parser; | ||
| using Ocelot.Responses; | ||
| using System.Security.Claims; | ||
|
|
||
| namespace Ocelot.UnitTests.Authorization | ||
| { | ||
| public class ClaimsAuthorizerTests : UnitTest | ||
| { | ||
| private readonly ClaimsAuthorizer _claimsAuthorizer; | ||
| private ClaimsPrincipal _claimsPrincipal; | ||
| private Dictionary<string, string[]> _requirement; | ||
| private List<PlaceholderNameAndValue> _urlPathPlaceholderNameAndValues; | ||
| private Response<bool> _result; | ||
|
|
||
|
|
@@ -26,9 +26,9 @@ public void should_authorize_user() | |
| { | ||
| new("UserType", "registered"), | ||
| })))) | ||
| .And(x => x.GivenARouteClaimsRequirement(new Dictionary<string, string[]> | ||
| { | ||
| {"UserType",new []{ "registered" }}, | ||
| })) | ||
| .When(x => x.WhenICallTheAuthorizer()) | ||
| .Then(x => x.ThenTheUserIsAuthorized()) | ||
|
|
@@ -42,9 +42,9 @@ public void should_authorize_dynamic_user() | |
| { | ||
| new("userid", "14"), | ||
| })))) | ||
| .And(x => x.GivenARouteClaimsRequirement(new Dictionary<string, string[]> | ||
| { | ||
| {"userid",new []{ "{userId}" }}, | ||
| })) | ||
| .And(x => x.GivenAPlaceHolderNameAndValueList(new List<PlaceholderNameAndValue> | ||
| { | ||
|
|
@@ -62,9 +62,9 @@ public void should_not_authorize_dynamic_user() | |
| { | ||
| new("userid", "15"), | ||
| })))) | ||
| .And(x => x.GivenARouteClaimsRequirement(new Dictionary<string, string[]> | ||
| { | ||
| {"userid", new []{ "{userId}" }}, | ||
| })) | ||
| .And(x => x.GivenAPlaceHolderNameAndValueList(new List<PlaceholderNameAndValue> | ||
| { | ||
|
|
@@ -74,7 +74,7 @@ public void should_not_authorize_dynamic_user() | |
| .Then(x => x.ThenTheUserIsntAuthorized()) | ||
| .BDDfy(); | ||
| } | ||
| [Fact] | ||
| public void should_authorize_user_multiple_claims_of_same_type() | ||
| { | ||
|
|
@@ -83,9 +83,25 @@ public void should_authorize_user_multiple_claims_of_same_type() | |
| new("UserType", "guest"), | ||
| new("UserType", "registered"), | ||
| })))) | ||
| .And(x => x.GivenARouteClaimsRequirement(new Dictionary<string, string[]> | ||
| { | ||
| {"UserType", new []{ "registered" }}, | ||
| })) | ||
| .When(x => x.WhenICallTheAuthorizer()) | ||
| .Then(x => x.ThenTheUserIsAuthorized()) | ||
| .BDDfy(); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void should_authorize_user_with_route_having_multiple_claims_requirement() | ||
| { | ||
| this.Given(x => x.GivenAClaimsPrincipal(new ClaimsPrincipal(new ClaimsIdentity(new List<Claim> | ||
| { | ||
| new("UserType", "registered"), | ||
| })))) | ||
| .And(x => x.GivenARouteClaimsRequirement(new Dictionary<string, string[]> | ||
| { | ||
| {"UserType", new []{ "non-admin","registered" }}, | ||
| })) | ||
| .When(x => x.WhenICallTheAuthorizer()) | ||
| .Then(x => x.ThenTheUserIsAuthorized()) | ||
|
|
@@ -96,9 +112,9 @@ public void should_authorize_user_multiple_claims_of_same_type() | |
| public void should_not_authorize_user() | ||
| { | ||
| this.Given(x => x.GivenAClaimsPrincipal(new ClaimsPrincipal(new ClaimsIdentity(new List<Claim>())))) | ||
| .And(x => x.GivenARouteClaimsRequirement(new Dictionary<string, string[]> | ||
| { | ||
| { "UserType",new []{ "registered" }}, | ||
| })) | ||
| .When(x => x.WhenICallTheAuthorizer()) | ||
| .Then(x => x.ThenTheUserIsntAuthorized()) | ||
|
|
@@ -110,7 +126,7 @@ private void GivenAClaimsPrincipal(ClaimsPrincipal claimsPrincipal) | |
| _claimsPrincipal = claimsPrincipal; | ||
| } | ||
|
|
||
| private void GivenARouteClaimsRequirement(Dictionary<string, string[]> requirement) | ||
| { | ||
| _requirement = requirement; | ||
| } | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,7 +32,7 @@ public DownstreamRouteExtensionsTests() | |
| new CacheOptions(0, null, null, null), | ||
| new LoadBalancerOptions(null, null, 0), | ||
| new RateLimitOptions(false, null, null, false, null, null, null, 0), | ||
| new Dictionary<string, string[]>(), | ||
| new List<ClaimToThing>(), | ||
| new List<ClaimToThing>(), | ||
| new List<ClaimToThing>(), | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.