-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
opa support #1378
Comments
Bringing together two of my favorite projects -- that sure is tempting 😄 However, I'm afraid there's currently no place in Dex to plug something like this in: if there was a middleware structure for "do stuff to the user record after authenticating with the external IdP", having an OPA implementation (that either uses it as a library, or calls out to OPA standalone) would make a lot of sense. #1371 is a related issue, I suppose, insofar as that middleware (or plugin) system could satisfy that other use case as well. @ericchiang what do you think about a plugin system? @kfox1111 do you have an idea for a plugin interface that's generic enough to accommodate different implementations? |
Yeah, looks very related to #1371. though that issue is much more generic. Having a plugin system would be great, but might take more work. If all the plugin api did was http post the data as a json doc and get back the doc with any changes, that might be enough to plug it into opa and other custom services as well? |
Since dex is auth-n and opa is auth-z, I'd rather add more data to the token and have whatever's consuming the tokens continue to enforce the policy. (See #1371 (comment)) However since OPA is technically just a standard for a policy language, it seems fine to include it as part of dex's configuration if we can figure out the right place for that. |
I don't think @kfox1111 wanted to challenge that -- I would side with you on this, Dex shouldn't do fancy authz-related things. This proposal would only be about processing what goes into Dex's JWT ID token claims (and perhaps more specifically, the groups). And for that bit, Rego (OPA's policy language) would be a good fit. I suppose the question is Do we want a generic POST to some service, and deal with the response (where the service could be OPA) or Do we want to bake some OPA into Dex for this functionality? |
I think the answer depends partially on how much time we want to put into an implementation. Making it work with OPA's api is pretty trivial. Coming up with a very generic api might not be. I'd vote for now to just get OPA working and if something else shows up, and doesn't want to follow OPA's api, then we re-evaluate? |
I’ve been mulling over tackling this one to solve some of the issues in #1400 . I have a lot of love for both Dex and OPA, so seeing them together is pretty interesting to me. I do want to clarify a few things though. Firstly, @kfox1111 do you have any examples of the kind of things you’d like to achieve with this? That might help me grok the idea a bit better. I’m also a little confused on the “no authz” point. The original issue mentions “applied by sites such as blocking users with particular groups”, which sounds a lot like an authz decision to me. If we’re not using OPA to decide to issue a token and to just do things like add a list of groups to a token, it feels a little overkill to me. I’d also love to know peoples opinions on pointing to an OPA endpoint vs. embedding OPA in-process. The latter is maybe simpler for a user to approach (just point the config at a path), but is probably slightly more complex implementation-wise. The former keeps OPA out of vendor, but means running sidecars or the like. I think I’d be against doing something completely generic though, trying to get the design for that right up-front might be tough. |
related issue #1635 |
It would be very interesting if dex could pass off data to opa (https://www.openpolicyagent.org/docs/#what-is-policy-enablement) before signing the tokens. This would allow custom policy to be applied by sites such as blocking users with particular groups or patch groups from the upstream provider before dex relays them.
It takes in json and returns json, so it should be pretty easy to add support for.
The text was updated successfully, but these errors were encountered: