-
Notifications
You must be signed in to change notification settings - Fork 159
Add additional authority endpoints to OAUTH2 #912
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
Comments
@ricardozanini @JBBianchi @matthias-pichler-warrify @fjtirado What do you guys think? Can I proceed with it and open a PR? |
Is this in the case where the openId connect configuration lives on a different domain than e.g. the token endpoint? |
Yes, exactly! |
Ok got it. What would a proposed solution look like? Because I guess making On a related note: since OpenID Connect and OAuth2 are separate specifications and many OAuth APIs don't have an OIDC config shouldn't there be a way to specify e.g the token uri directly? And from the fields Would it make sense to either:
|
I am actually kind of struggling with the lacking support for oauth right now. I think a solution should at least cover the following cases: Open ID Connectone can definitely specify a well-known uri to a open id config. document: {}
use:
authentications:
exampleOIDC:
oauth2:
openIdConnect: https://example.com/.well-known/openid-configuration
trustedAuthorities:
- example.net
grant: client-credentials
client:
id: workflow-runtime
secret: "**********"
# audience, scopes, etc Question @cdavernas is the domain enough to whitelist additional endpoints? Token endpointdocument: {}
use:
authentications:
exampleOIDC:
oauth2:
tokenEndpoint: https://example.com/oauth2/token
grant: client-credentials
client:
id: workflow-runtime
secret: "**********"
# audience, scopes, etc Open Questions
|
@matthias-pichler-warrify |
Yeah the issue is that I don't think the runtime has a way to figure out which one is supported by any given endpoint. So I think the author has to specify which is desired. So my question is what should this config look like? For example Amazon Cognito support both Auth0 supports both and there exists basically any combination imaginable out there |
In my personal and professional use cases, I actually use both
Good catch! Yes, we should probably update the
I believe it should, yes, though I never had the need to whitelist, so the question would be better addressed to @jaliyaudagedara!
I believe it should, yes.
Yes, IMHO it should be configurable too!
We probably should support the legacy |
Agreed. My only concern is that these are going to be a lot of options then 🙈 and also kind of hard/opinionated to choose a default
I didn't even know that one 😅 |
Ok, I forgot we are the client, not the server ;) |
To summarize, I believe we should add the following properties:
These are just suggestions, of course. I'm convinced we can come up with a better wording! |
I don't think that's really a problem, is it? As long as we provide default values, we can ensure that most users will be able to declare OAUTH/OIDC auth kind of brievly.
As far as I know, the defaults are well defined, aren't they? My understanding is that the default encoding is
It's an awesome one, though, I highly recommend you take a look at it! Amongst other things, it allows a client to act on behalf of a user, which is priceless, especially in the context of a workflow execution (ex: a user "starts" the workflow, then the runtime requests a new token to act on behalf of her) |
From the OAuth2 RFC 6749 it seems like For encoding form encoded seems to be the default. But yeah defaults should be fine actually 👍
I think there should be one array listing allow listed domains/uris and a separate field specifying the openid config uri. Because otherwise it would look weird and be hard to determine where to find the config
something like this would make sense yeah 👍 I think maybe
This should probably be called something else and also use the official names for client authentication from the OpenID docs maybe For other grants |
@matthias-pichler-warrify @cdavernas great conversation here. Can we summarize again and move on with a PR? Personally, I'd go for a new And I agree that we should stick with the names recognized by users. |
@matthias-pichler-warrify Would you be so kind to summarize your key take-aways, so that I can address the issue in the best way? |
What would you like to be added:
Add a new property to OAUTH2 authentication, used to define additional token endpoints in order to address serverlessworkflow/synapse#342
Why is this needed:
As explained by @jaliyaudagedara in serverlessworkflow/synapse#342, this is a requirement for many librairies when facing authority mismatch
The text was updated successfully, but these errors were encountered: