-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Enable multiple providers #926
Comments
I am definitely up for supporting this feature, it's been asked for a few times in the past and hopefully shouldn't add too much extra complexity. Some initial feedback on this proposal:
I would like this to become part of the new Also, I expect it to be a list in that so something like:
Where we have provider specific options (I'd like to minimise this) but they should become substructs of the provider struct, so for example: (this is how we are doing it in the session storage so let's copy that pattern)
I'd suggest adding an ID field as we have done in the
Why does each provider need its own cookie? I would expect a user to only need to be logged in to one provider at a time and only need one session with OAuth2 Proxy. If there are multiple cookied sessions, how do we determine which one the user wants to use to provide header information to the upstream?
We will want to add a new field to the sessionstate that contains the provider ID, this will enable us to perform the required actions (such as refreshing, checking validity) when a session is being loaded rather than being created. If the provider ID cannot be found (eg someone updated the configuration), then the session load should fail and the user should be logged out (clear cookie/session).
It may be good to gather a list of all the options that are currently affecting the providers and then we can discuss how we want the structure of the config to look, which can be dropped, which should be shared between all providers, which can be dropped as part of this refactor, etc.
I think it would be better to have a mandatory ID field as in the
@NickMeves is working on a PR to streamline this process at the moment. From my understanding this should be able to be implemented without affecting this too much, apart from the need to configure multiple providers in the jwt session loading middleware, but it should be relatively trivial
If we stick to just one session per user and don't conflate that with multiple providers, this question goes away 😉 |
@JoelSpeed I actually started a spike of cleaning up the OIDC provider & adding some features yesterday. When I was working claim -> session extraction, I thought about the OIDC provider specific claim-name options that are OIDC only fields now. I was thinking about making those global option on the ProviderData and making any providers that are compatible be have full default OIDC options & methods (claims + group extraction) at their disposal. I think Keycloak, Gitlab & Azure fall into this category? Sounds like you are in favor of this route as well? I branched off #869 so once that merged I'll get a draft PR up so this is easier to visualize. |
@JoelSpeed thank you for your comments. AlphaOptions + providerID + yaml structure sounds great.
This assumption led me to suggest having a single provider cookie that will store only the chosen provider after user selects it. Based on the information from the cookie the correct session chain will be loaded, i.e something like this:
Another usage of the same mechanism is in the callback:
And then (providers is of type map[string]providers.Provider):
Is this what you meant by 'multiple cookied sessions', or were we out of sync? and more importantly - how can we redeem the session (assuming it will be stored in sessionstate) if we don't have the provider chosen beforehand (as extracted above directly from the cookie)?
Mandatory for each provider: Optional for each provider: Provider specific:
|
Ignore for the moment this part of the code, @NickMeves is refactoring that bit quite a lot and removing some of this complexity. This section is about loading a session from headers instead of cookies. Cookies are what we need to focus on here.
Having a single cookie will store only the chosen provider, but I think that's desirable? As a user, I would expect when allowing multiple providers, that the user being logged in to any of my providers is sufficient to allow them access no?
I think we may not have been on the same page. It looks like you're creating a cookie which has the provider name in, then using that to check which provider? Could this information not be encoded into the The
Don't want to focus too much on this first as it might distract from the bigger picture, only thing I will say now is that the stuff related to Final thing to say is that this is obviously going to be a major project, it would be preferable if we could somehow break down the work into smaller PRs to make some of this easier to review. I guess the first thing to do if we are taking that approach is the structure of the configuration and the conversion logic, that would allow us to settle that before we do the main implementation |
@JoelSpeed yes, you are right the provider configs are a bit jumping the gun..
Just to be sure I understand - you suggest passing the chosen provider in the
Great, so how should we proceed? |
Yeah that was my suggestion, I think that is the idiomatic place to put this information when performing an oauth2 flow Have you got a link handy to the work Nick is doing? Would be good to have that linked into this conversation
This is exactly how I would start yes.
I think if we take the approach of renaming things in the validation method we should be able to keep the logical changes to a minimum and focus the first part of the review on just the design of the structure of the configuration. All changes outside of that discussion should just be renaming, conversion logic and validation logic. We could perhaps even divide some of that into smaller PRs if we wanted to, the first two steps could possibly be done without integrating into anything else, it could be done as additions only pretty much. If you were to extract the |
Sounds good to me! I see that alpha options #907 is merged, I'll pull it to my branch and get started:) Regarding ProviderID - I don't think we have an alternative of a unique ID like in Regarding passing the provider in state param - I am still a bit uncertain about the behaviour in all the cases that are not the oauth2 flow (all flows that require |
My suggestion was actually that we add a new ID field for this purpose, as we have in
Extracting a previously stored session should be a case of unmarshalling it into the struct. Once we extract it into the struct, we can then pass it to the validation based on the stored value of the providerID in the extracted session right? I think we need to separate the extraction from the validation that currently is tied together |
This issue has been inactive for 60 days. If the issue is still relevant please comment to re-activate the issue. If no action is taken within 7 days, the issue will be marked closed. |
Thank you for this amazing tool. |
Yup! We are looking to merge #947 soon and have it available in v7.2. I can't recall if that PR had the full functionality, or set the stage for structured configs which will be fully released in v8 (available now using alpha config flags) |
As far as I can see, the commit of #947 does not enable multiple providers by itself, instead it only enables the configuration of such setups. Are there any other issues where the next steps are handled or is this issue closed mistakenly? |
Yep this was closed by mistake, we have the first part implemented now (the configuration changes), next up will be the actually implementation that allows multiple providers IIUC |
Looking forward to try it out |
will this have cli support as well or only config files ? |
Due to the complexity of the structure, this will be config file only |
I literally would need this right now. My other option is to change my setup in such a way that I'm running two concurrent instances of oauth2-proxy. Since I'm using the helm chart this isn't a trivial task and it's something I would prefer to avoid if at all possible. I could possibly postpone this a bit if I could be confident that the release providing this functionality would be coming real soon (within a week or two). So, any guesses when v7.2 would be out with this functionality? |
It looks like this refactor got picked up and included in v7.1.2 already: https://github.com/oauth2-proxy/oauth2-proxy/blob/master/CHANGELOG.md I haven't dug in deeply, does it have the alpha-config hooks you need to try this out? Or were those not in the initial refactor PR? |
I ended up going around this issue via a hack(ish) solution but I will look into this a bit later when I have time. It would definitely be much better solution. Cheers for the link! |
Not stale |
Just to sanity check here, the current status of this is that we can configure multiple provider instances (e.g. multiple OIDC IdP's) with the alpha configuration options, but multiple provider instances is not yet supported, right? |
Yes, that's correct |
Is there a documentation somewhere about this ? I'm not finding anything in the doc (especially in https://oauth2-proxy.github.io/oauth2-proxy/docs/configuration/oauth_provider) |
It’s implied on the “alpha config” page by the fact that the “providers” option is an array, but as stated above currently only one provider is actually supported in the rest of the code outside of the alpha options YAML model. Multiple providers will never be supported in the legacy config style, if/when the structured config style graduates beyond being considered “alpha” I’m sure the options will be more fully documented at that point. |
And when will multiple provider instances be supported? If we can already configure it now. |
Unfortunately, no timeline does exist for when this is going to be supported. As of now you will just have to deploy multiple oauth2-proxy instances. |
This issue has been inactive for 60 days. If the issue is still relevant please comment to re-activate the issue. If no action is taken within 7 days, the issue will be marked closed. |
Still relevant - not stale Link: #2279 |
Hi. If I understand correctly, this would work as an workaround in this case? Multiple instances should be able to handle multiple providers? Can you provide an example of how it should be configured for single application? |
It works, but it requires to set up completely new instance of client-side aplication on k8s along with new oauth2-proxy container... I believe that setting up multiple providers on single insance of oauth2-proxy would be much cleaner solution. |
This would be a huge deal - I'm trying to find a way to allow users for front-end web apps to have access to the same api as a native application or a server running in a cloud environment. It doesn't seem possible without extensive additional infrastructure. |
Maybe this can help some of you? |
Hi, When will this feature be available to try out?Despite configuring two OIDC providers in the alpha configuration of oauth2-proxy, it seems that only the first provider is being recognized ? Is there a way to get this working? Any guidance or suggestions would be greatly appreciated. Currently my architecture is Keycloak configured as provider in oauth2-proxy , and Azure is added as Identity provider in Keycloak. Below is my configmap
Additionally, here are some logs from the oauth2-proxy container that might provide more context:
|
This issue has been inactive for 60 days. If the issue is still relevant please comment to re-activate the issue. If no action is taken within 7 days, the issue will be marked closed. |
This issue is still relevant. |
This issue has been inactive for 60 days. If the issue is still relevant please comment to re-activate the issue. If no action is taken within 7 days, the issue will be marked closed. |
Issue is still relevant. |
This issue is still relevant. |
Hello. Sorry for the offtopic, but there is another project oauth2-tier. It supports multiple oauth2 providers. |
Expected Behavior
Solution should support authentication via multiple providers and not only one, including support of multiple providers of the same type (i.e 2 different okta providers).
Current Behavior
Oauth2-proxy currently supports only 1 provider at a time (and all its provider related configuration is bound to it). Additionally, currently it is not trivial to ingest recurring structured configs (in our case providers) in an elegant way while supporting all possible ingestion methods.
Possible Solution
Tap into the big structural config refactoring (#532) to configure multiple providers in a YAML file and once provider is chosen pass it in a designated provider cookie through the chain.
Steps
providers map[string]providers.Provider
(instead of the singleprovider providers.Provider
) - a map from provider display name to provider struct.Additional comments / Caveats
skip-provider-button
will be irrelevant in case more than 1 provider is configured.http.DefaultClient)
)?providerCookie
to cookie store and somehow translate that into the ticket version on the Redis alternative..?The text was updated successfully, but these errors were encountered: