Closed MayorSheFF closed 9 months ago
I made some changes locally to solve it, but I just want to discuss if it is necessary for everyone and maybe someone is currently trying to solve too. If it is necessary and noone is doing it, I can create a pull request.
I've gotten around this by setting the provider key to a policy scheme instead of directly to the authentication scheme, and doing custom logic on the selector to determine which method to use.
config:
"AuthenticationProviderKey": "AnyAuthenticated"
policy scheme:
builder.Services
.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
.AddPolicyScheme("AnyAuthenticated", null, p =>
{
p.ForwardDefaultSelector = context =>
{
if (context.Request.Headers.ContainsKey("X-Use-Example-Auth"))
{
return "ExampleAuthScheme";
}
return JwtBearerDefaults.AuthenticationScheme;
};
})
.AddJwtBearer()
.AddExampleScheme()
But I'm not the happiest with this approach and would prefer having support for an array in the config, something like:
"AuthenticationProviderKeys": [ "Bearer", "ExampleAuthScheme" ]
I am still having these changes. Can someone from owners look at it and accept if it is OK?
Hi @MayorSheFF could you please share the code snippet
Hi, @sivapriyanc.
This function was added into the AuthenticationMiddleware
class.
private async Task<AuthenticateResult> AuthenticateAsync(HttpContext httpContext, DownstreamRoute route)
{
AuthenticateResult result = null;
if (!string.IsNullOrWhiteSpace(route.AuthenticationOptions.AuthenticationProviderKey))
{
result = await httpContext.AuthenticateAsync(route.AuthenticationOptions.AuthenticationProviderKey);
if (result.Succeeded)
{
return result;
}
}
IEnumerable<string> authenticationProviderKeys = route
.AuthenticationOptions
.AuthenticationProviderKeys
?.Where(apk => !string.IsNullOrWhiteSpace(apk))
?? Array.Empty<string>();
foreach (var authenticationProviderKey in authenticationProviderKeys)
{
result = await httpContext.AuthenticateAsync(authenticationProviderKey);
if (result.Succeeded)
{
break;
}
}
return result;
}
Then, it is using in Invoke
method, namely var result = await AuthenticateAsync(httpContext, downstreamRoute);
row.
public async Task Invoke(HttpContext httpContext)
{
var downstreamRoute = httpContext.Items.DownstreamRoute();
if (httpContext.Request.Method.ToUpper() != "OPTIONS" && IsAuthenticatedRoute(downstreamRoute))
{
Logger.LogInformation($"{httpContext.Request.Path} is an authenticated route. {MiddlewareName} checking if client is authenticated");
var result = await AuthenticateAsync(httpContext, downstreamRoute);
httpContext.User = result.Principal;
if (httpContext.User.Identity.IsAuthenticated)
{
Logger.LogInformation($"Client has been authenticated for {httpContext.Request.Path}");
await _next.Invoke(httpContext);
}
else
{
var error = new UnauthenticatedError(
$"Request for authenticated route {httpContext.Request.Path} by {httpContext.User.Identity.Name} was unauthenticated");
Logger.LogWarning($"Client has NOT been authenticated for {httpContext.Request.Path} and pipeline error set. {error}");
httpContext.Items.SetError(error);
}
}
else
{
Logger.LogInformation($"No authentication needed for {httpContext.Request.Path}");
await _next.Invoke(httpContext);
}
}
The following classes were changed too. It allows to get values from the configuration.
AuthenticationOptions
class;AuthenticationOptionsBuilder
class;AuthenticationOptionsCreator
class;FileAuthenticationOptions
class;RouteOptionsCreator
class.Hi Igor! Welcome to Ocelot world! 😄
@MayorSheFF commented on May 30, 2023
Why not to create a PR?
@MayorSheFF commented on Mar 13, 2023:
I am still having these changes. Can someone from owners look at it and accept if it is OK?
Tom, as the owner, still manages this repo, but he is super busy. I'm not owner sure, but I will accept the issue after a discussion, and provide code review for your PR(s). Let me to fix your code blocks wrapping. And let's start a discussion...
@MayorSheFF commented on Jun 2, 2022:
At the moment, Route can contain one AuthenticationProviderKey value only.
Correct!
So, if we want to handle Route for different authentication providers, we have to specify two Routes and make Route path unique.
So, it looks like a hack! 😺 I cannot get your user scenario about the downservice you're routing the traffic to! Why do you need multiple auth-providers for one endpoint/route? Are you going to hack the system? 🤣 Just give me the reason please!
In theory your current JWT-token can be expired and for a some reason you cannot generate new token. In this case you are not the owner of endpoint (not able to support the downservice), or you cannot generate new tokens because your access to downservice is limited or forbidden, and/or your credentials (user/pwd) were changed/deleted, and/or the service has changed access policy by closing authentication endpoint (aka auth-method).
Practically, you are requesting absolutely new feature because current bearer-tokens auth approach is not aligned to your needs. Browse to the Authentication docs please, and you will see the following supported auth approaches:
I see you need more control on authentication stage, and you don't like current default ASP.NET bearer-tokens approach. In this case you could try Identity Server approach where you have 100% of control on authentication.
Finally, I would ask you to share your user scenario having multiple auth-approaches for one route. Please note, that your splitting route into multiple routes with different auth-approaches is absolutely correct. Otherwise supporting multiple providers by one downservice app will be strange and risky. But in theory it is possible for sure.
Sometime it is not so ease to specify unique paths for the same endpoint.
Sometimes
means your user's case is rare! You cannot define multiple routes for the same endpoint (downservice path). In this case Ocelot starting will fail with an exception of bad config in ocelot.json file.
Once again,
Waiting an interesting feedback from you...
- Could you share your user scenario please?
Our scenario was that we needed to support both JWT Token authentication as well as API Key authentication for connected services or applications for all endpoints in the system.
My example code above works to change schemes based on the incoming header value, but it is not ideal.
Hi @raman-m,
Why not to create a PR?
I would be very glad to push my local branch to GitHub, but I have the following issue.
Remote: Permission to ThreeMammals/Ocelot.git denied to MayorSheFF.
Error encountered while pushing to the remote repository: Git failed with a fatal error.
Git failed with a fatal error.
unable to access 'https://github.com/ThreeMammals/Ocelot.git/': The requested URL returned error: 403
Maybe I make something wrong.
Provide details on configs, downservices, and paths of a downservice you're defining routes for, plz?
{
"Routes": [
{
"AuthenticationOptions": {
"AuthenticationProviderKeys": [
"Bearer",
"BearerB2B"
]
},
"DownstreamHostAndPorts": [
{
"Host": "servicehost",
"Port": 8000
}
],
"DownstreamPathTemplate": "/{anyurl}",
"DownstreamScheme": "http",
"UpstreamPathTemplate": "/service/{anyurl}"
}
]
}
Could you share your user scenario please?
We have microservices which have to be reachable for two kind of our users: our customers and our internal users. These two groups are in different Azure Active Directories. So, you can imagine these two Azure Active Directories as two different Identity Servers. By means of several AuthenticationProviderKeys you will be able to realize authentication with these Azure Active Directories without a lot of complementary implementation. I believe it is enough explanation. Should I add something?
@MayorSheFF commented on May 31, 2023:
I would be very glad to push my local branch to GitHub, but I have the following issue.
Remote: Permission to ThreeMammals/Ocelot.git denied to MayorSheFF. Error encountered while pushing to the remote repository: Git failed with a fatal error. Git failed with a fatal error. unable to access 'https://github.com/ThreeMammals/Ocelot.git/': The requested URL returned error: 403
Maybe I make something wrong.
Yes, you may! You need to create a fork of Ocelot in your account! Read these articles which will help you to make new forked repository:
As soon as you've created a fork you will be able to create feature branches, and create PR(s) for feature delivery to upstream (parent, source, base) repository, which is ThreeMammals/Ocelot repo.
@MayorSheFF
We have microservices which have to be reachable for two kind of our users: our customers and our internal users. These two groups are in different Azure Active Directories. So, you can imagine these two Azure Active Directories as two different Identity Servers. By means of several AuthenticationProviderKeys you will be able to realize authentication with these Azure Active Directories without a lot of complementary implementation. I believe it is enough explanation. Should I add something?
Well... It seems I understood your goal for such feature. But... I see 3 ways of evolution of your project:
I would recommend to discuss that with Software Architect of your project. I don't know either Azure is able to "merge" 2+ Identity Servers. or not. I believe Azure can merge, but I think a 3rd Active Directory service should be deployed. So, I cannot imagine the best possible solution here. Once again, I believe it should be possible to merge 2 Identity Servers and/or 2 Azure Active Directories.
Igor,
In my opinion, the easiest solution for your user's scenario will be the following:
What about this solution?
Hi, @raman-m.
You need to create a fork of Ocelot in your account!
I will try to look at it today. Thank you for your advice.
@raman-m,
I will answer on all your suggestions what your described in two last messages.
On the figure below you can see very rough architecture how it was before these changes and how it is after these changes (new implementation).
Just imagine. Before our application had the internal users only. So, these internal users send requests to our microservices by means of different clients (web, mobile, desktop etc.). Then, business came to developers and infrastructures and ask for giving an ability other users to use the microservices. At that moment, external users appeared in our system. But the main question for business is how to do it with less resources: money, time, development, delivery etc. and leave our architecture transparent. Starting from this point, I will ask you the following question. Will you implement all your suggestions and in the same time adhere business requirements? Our answer was: "We can extend our API Gateway / Ocelot to handle two identity providers." So, we extended Ocelot to authenticate requests via several identity providers. It took about 3 days. OK. Let's say 5 days (1 working week). Maybe 4 days if you are in the country where 1 working week = 4 days. So, these 5 days are development, test and delivery. Also, it is a good solution from architecture perspective.
Maybe it is a very rough description, but the general idea is like that.
In my standpoint, this implementation can be useful for other people too who will run into the same questions. Due to it, I suggested Ocelot team to look at it.
@MayorSheFF commented on Jun 1, 2023
🆗 I see... Having multiple identity services in the project is the real challenge! 🤣 How did you solve the problem? Is your solution based on Ocelot AuthenticationMiddleware overriding to eliminate the problem of single provider key?!
You could also split Ocelot gateway into 2 instances which of them should support one provider key. But this approach requires more hardware resources and DevOps.
Also, as @clintonbale suggested on Jun 27, 2022, you can setup Active Directory services to generate special custom header, forward that HTTP header to Ocelot gateway, and reuse Clinton's solution services setup to recognize AD user and auth provider key.
So, what solution is in production now?
@MayorSheFF Igor,
Can I ask your help to search for the similar issues in backlog please?
Use keywords: AuthenticationProviderKey
, Authentication
, etc.
We could attach them to your current open PR if those issues will be relevant to provider keys...
Hi all,
What is the status of this PR https://github.com/ThreeMammals/Ocelot/pull/1870 ?
Do you need help with testing ?
We are waiting for this feature and we are wondering if we can wait for you to release that or we need to implement the way around it.
Is it the matter of week/month ?
@dkornel Hi Kornel!
What is the status of this PR https://github.com/ThreeMammals/Ocelot/pull/1870 ?
In Progress. The author doesn't develop his PR anymore. As a Pair Programming participant I am finishing the PR to get "Code Complete" status.
Do you need help with testing ?
Sorry? How could you help us? Unit tests are ready. I'm finishing acceptance testing... They are most complicated part of the PR. Will you write some interesting user scenarios? Pay attention I work in the author's feature branch. I have access as repo Maintainer. But you cannot push to forked repo branch. Asking the author to add you as collaborator probably will take to much time, finally with no reply from the author. Also, I will not move the code to head repo branch because it is too late.
We are waiting for this feature and we are wondering if we can wait for you to release that or we need to implement the way around it. Is it the matter of week/month ?
Week. No more!
@dkornel If you need this feature ASAP then there are 2 possible ways
@clintonbale commented on Jun 27, 2022:
config:
"AuthenticationProviderKey": "AnyAuthenticated"
...
But I'm not the happiest with this approach and would prefer having support for an array in the config, something like:
"AuthenticationProviderKeys": [ "Bearer", "ExampleAuthScheme" ]
Clinton,
Why did you propose the new property as extra one for AuthenticationProviderKey
?
Why array-like approach?
Maybe 👇
"AuthenticationProviderKey": "Bearer ExampleAuthScheme",
Will not this simple convention work when you write all schemes in old property, separating by whitespace?
If extend the idea of using old property, we can propose any delimiter approach 👉
"AuthenticationProviderKey": "Bearer ExampleAuthScheme",
"AuthenticationProviderKey": "Bearer, ExampleAuthScheme",
"AuthenticationProviderKey": "Bearer | ExampleAuthScheme",
"AuthenticationProviderKey": "Bearer+Bearer2+ExampleAuthScheme",
So, having non-alpha character in the value will instruct the validator and logic that we are running into multiple auth schemes workflow.
I am not a fan of the new AuthenticationProviderKeys
property in plural form of old one.
@raman-m
We had a team discussion, that we will need this feature, and we need to assess if we have time to wait for this PR to land in the ocelot release or we should rather start developing our solution. We don't need this feature now but in a month.
Sorry? How could you help us? Unit tests are ready. I'm finishing acceptance testing... They are most complicated part of the PR. Will you write some interesting user scenarios?
I kindly ask if you need help because if the PR stucked, I could talk with manager and dedicate c# developer for help (explaining that time of building our solution > time of supporting this PR).
No offence.
@dkornel 🆗 Wait for our official release please next week! This feature will be merged next week before the release for sure. We're finishing of testing in Prod env of our team member now...
Any contributions and interesting PRs are welcome! But seems we needn't your urgent help here. The #1870 PR was stuck because I was on sick leave for 2+ weeks. 😉
@raman-m commented on Jan 26
Plural approach makes sense because it is listing multiple keys. But for backwards compatibility the singular property should not be removed of course.
You could have both properties maybe? AuthenticationProviderKey
could get/set AuthenticationProviderKeys[0]
. Only one property allowed to be set at a time at the config level.
@clintonbale Good proposal...
But for backwards compatibility the singular property should not be removed of course.
Agree.
As a team, we've decided to go with the current design. So, this is the same you've recommended.
And we will mark old AuthenticationProviderKey
property with [Obsolete]
attribute.
New Feature
Opportunity to support several AuthenticationProviderKey values in the Route.
Motivation for New Feature
At the moment, Route can contain one AuthenticationProviderKey value only. So, if we want to handle Route for different authentication providers, we have to specify two Routes and make Route path unique. Sometime it is not so easy to specify unique paths for the same endpoint.
Update by Maintainer on Jan 8, 2024 👇
Useful Links
Related
446
1002