-
Notifications
You must be signed in to change notification settings - Fork 11
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
Support a default provider for OIDC + new example #373
Conversation
m-mohr
commented
Apr 7, 2021
- Support a default client for OIDC
- Added a new example, also showing the default_clients
issue title should say "default provider" instead of "default client"? |
Yes, indeed. |
not ideal that you can not enforce that at most one provider can be flagged as default. Alternatives:
|
Yes, that's always the issue (see also default_plan for example). You can't really provide a good way of doing it consistently. If you do "default_provider", then you can't check whether the value actually exists. Similarly for the priority, where you can't check whether all fields may be 0 for example. |
@soxofaan Would you prefer the alternative of the first array element being the default? I think that's the only option that would lead to a bit more consistency. |
Yes, "first of list" would be my preference. It introduces no real API changes, except for documenting this "convention". It would also work automatically for backends that just specify a single OIDC provider. Second preference would be new "default_provider" field because if there is an inconsistency then it will fail in a clear way. If you have inconsistency with the boolean "default" flags, then some implementations might still accept that (because they pick the first match they find). |
I'll update this PR to work with the array convention. |
PR has been updated. This really feels much better integrated, it also works out of the box with the Web Editor implementation. A review would be appreciated. |
First provider under `/credentials/oidc` is default provider see Open-EO/openeo-api#373