[RFC] generic OIDC provider #38
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
I'm thinking about patching the (now stale) upstream PR for generic OIDC integration into my hydra, and since I'd prefer upstreaming my patches I wanted to ask whether you'd want to merge that or if I should keep it as a patch (both are fine, the latter just gives me more leeway in how I implement a few things)?
So I kinda want to request some comments on the upstream PR to gauge whether you'd want to merge the feature (both in general, and about smaller changes within the feature).
Specifically I would (opposed to the comments in the PR) throw in:
While I understand the upstream rationale that OIDC is authentication and not authorisation on a high level, hydra's account-level permissions are literally roles it makes sense to me to just enable that use-case.
Anyone who needs more fine-grained control can go and add some OpenFGA integration or whatnot, but optional OIDC roles support should be fine for most people (heck it's what other software like Grafana uses too).
Adding the already-present roles as pre-defined "groups" for use by OIDC also provides room for expansion by allowing to define custom groups in hydra which can then be used by OIDC.
For context this is how SAML auth with OVH works (i.e. using your own SAML IdP to authenticate with OVH's web interface for server management): they accept three pre-defined Groups for admin, restricted, and read-only, and then allow adding additional groups which when present in the IdP information also apply, however the predefined groups cannot be removed, disabled, or changed (they're kinda built for ADFS as far as I understand).
The same could work in hydra by matching IdP provided roles with the hydra roles, and picking the most privileged one of those, and if someone wants to build that, adding custom groups to hydra later on which map either to existing groups or fine-grained authorization although I'd actually think that OpenFGA would be the way to go if anyone needs that.
I can also factor out some of the patch to merge of course.
To be honest it just irks me that there is OIDC integration for specific providers (like GitHub) but not a generic implementation ~.~
This probably won't be done tomorrow, since I have more urgent things to do (basically just a heads-up that I will be implementing this in the weeks to come), so there's plenty of time to throw in some comments (or say you don't want this upstreamed, either is fine by me).
The upstream PR is something I hacked together with @lheckemann. If Lix would've existed back then, the PR would be open against this repo. So long story short, yes this is something I'd really like to see. Not only for myself, I've been told that https://git.lix.systems/the-distro/infra is also interested in this.
While we're planning to finish this, I'm a little short on time. So if you don't want to wait, go ahead! If we're about to resume work, I'd let you know smh to avoid duplicated efforts.
You know, the fun thing is: Linus and I had a quite long discussion about this when we were working on this and I'm the one who was (and still is) in favor adding minimal roles support. I think I even cited Grafana (or something like that) as another example 🙃
Personally, I'd probably merge this with the rationale "good enough". Unless we get too many people disagreeing.
The other suggestions sound very reasonable to me.
If you're very motivated, you may also want to check if we can deduplicate the OIDC parts for GitHub/Google somehow.