Open dpacierpnik opened 7 years ago
If this proposal is only about granting groups to static users, I'm against it. We're trying really hard to make sure dex isn't a user management system and the static users are only there for minimal bootstrapping. Static users were never intended to be fully featured.
I think I'd be interested in a proposal that did allow us to define groups for any backend. Of main use case is to do async group syncing instead of doing lookups at the time of the login. A groups configuration that used the gRPC endpoints would let us start to prototype something like that without building it into the LDAP or SAML connectors.
I understand your motivations. And I'm aware that static passwords are not intended for production use at all (user passwords are very poorly secured and so on). However, since this configuration is only useful for POCs, in my opinion, it would be nice to have a possibility to fastly verify some more advanced scenarios of use of DEX (for example with DEX on Kubernetes with RBAC enabled, with group-based roles) without need to have real connector with groups support. Even example user (Kilgore Trout) has a group assigned, so why you can not play with users defined statically with some groups for testing purposes ? It is very simple change, and can make DEX testing and evaluation easier right now.
Defining groups for any backend for sure can be useful feature, and maybe we will be also interested in such solution in the future.
If you think, that current improvement is not in line with your development plans, feel free to close it.
Any updates on this? Does the static user configuration support groups now for POC?
I'm also looking at doing a POC w. Dex + Kubernetes, where I need something ... more than mr. Trout in 'authors'.
Perhaps groups could be auto-generated from some part of the static name? That way there would be some groups to test with, but still keeping the feature obviously useless for any production use:
Any of these would (at least from my current use-case) allow testing of group-membership for differentiated accesses in Kubernetes.
I can see two possible approaches here:
Both approaches allow local users to assert group membership, which is useful for PoCs and test/dev environments.
Having general Dex-defined groups is a bigger change. It would require new entities to exist in the YAML file, storage and gRPC admin interfaces. Potentially it's very powerful though: e.g. you could make a Dex group which contains a mix of Google users and Microsoft users.
It would be particularly useful where the upstream IDP doesn't provide usable groups for your purpose (such as when using Google without a GSuite managed domain), or you don't trust the third party to assign group membership and you wish to keep your own control of this.
Is it worth creating as a separate ticket for this?
Aside: possibly relates to #1191, which proposes making "local" a fully-fledged connector. That seems to make sense to me - even "mock" is a full connector.
I have made a little proof-of-concept for Dex defined groups, which you can find in this branch: https://github.com/candlerb/dex/tree/dex-groups - commit https://github.com/candlerb/dex/commit/6a67e7c453a101a7ec3d07fde7b1d8f97b878cc8
It lets you assign additional groups to arbitrary IDP users and groups:
memberships:
- connector: local
subject: 08a8684b-db88-4b73-90a9-3cd1661f5466
groups: [administrators, support]
- connector: mock
subject: authors
is_group: true
groups: [support]
This is currently only done statically, but could be added to the Storage and gRPC interfaces.
You can make a group out of a mixture of individual users or groups across multiple IDPs. In the above example, one local user plus everyone in the "authors" group from the "mock" connector are all added to the "support" group.
This let you make groups of Google users without having a hosted domain - or you can mix other Gmail users with your hosted domain, mix Google and Microsoft users into a group etc.
Implementation-wise: I'm not sure I've hooked the group updating in the right place (especially for refresh actions). But I'm interested in any feedback.
Ideally it would be combined with prefixes for IDP groups (#918) to avoid all risk of overlap. Then perhaps also add per-client filtering of groups against a regexp, so you can return just the groups of interest to that particular client (e.g. return the Dex-managed groups without the IDP ones)
I am wondering now: rather than just applying arbitrary extra groups, what about arbitrary extra claims? You could then have:
membership:
- connector: local:
subject: 08a8684b-db88-4b73-90a9-3cd1661f5466
claims:
groups: [administrators, support]
ssh_principals: "root,admin,mail"
(In the case of groups I'd expect it to be additive)
Would these claims then make it into the JWT? Then please add roles as well :)
am wondering now: rather than just applying arbitrary extra groups, what about arbitrary extra claims?
I just thought of that in another issue: https://github.com/dexidp/dex/issues/1182#issuecomment-649695644.
I am just testing Dex right now and, because I can't impersonate real users on my LDAP server :wink: I would like to test out how user groups work with the local password storage. This is just for testing, but it is a very useful feature for testing that I would appreciate having in Dex. As it stands I have to stand up some sort of real connector that I can run on my local machine just to test this out.
Simple group support would be very useful for testing. We'd like to be able to use Dex as a component of a system that gets testing in a CI pipeline, and in which groups are important. We can't use the mock connector (as it stands) because it has one user in one group, which isn't sufficient for our test cases. We could use the static username/password support, if it also had groups, which it does not.
The alternative is that we spin up an LDAP server (or I suppose an OpenID Connect server) as part of our tests, and we can do that, obviously. But it would be simpler and easier if we could give Dex a static set of users and groups.
I was also wondering about being able to add arbitrary extra claims. I was wondering about the concept of being having an arbitrary interceptor (interceptors) that you could configure. My guess was somewhere like finalizeLogin. There could be a chain of interceptors that the claims were passed to. This could be a hook for local groups. But also, if you implemented an interceptor that made an HTTP POST, you can basically do what you want. You could look the user up in a database and add role claims, if that's what you wanted. Perhaps I'll have a play.
just leaving a note that for PoC testing I have used https://github.com/rroemhild/docker-test-openldap in the past (not with dex but with other systems) I do not see an obvious reason it should not work with dex.
@jtnord just saying, but you'd better adjust "nofile" rlimit in your image to prevent memory leak. Openldap has some strange magic, which makes it eat a lot of memory and work slowly if aforementioned rlimit is very high (like the value that docker sets by default).
@jtnord just saying, but you'd better adjust "nofile" rlimit in your image to prevent memory leak. Openldap has some strange magic, which makes it eat a lot of memory and work slowly if aforementioned rlimit is very high (like the value that docker sets by default).
:+1: I had to do ulimit -n 1024
or it could use a lot of memory.
Following on from thoughts in #1182, I am now thinking that custom claims ought to be set per Dex client (i.e. audience), not globally, as each custom claim only probably makes sense to a particular target application.
However, groups are different. I can see a case for having global user-to-group and group-to-group mappings. For example, you might map an individual user into a particular group once, and then give all members of that group application-specific claims for various different apps.
This would also be useful to combine similar groups which have different names. For example, say you have some users in Azure Active Directory, where the group name is some UUID that you don't control, and wish to return the same group name as some Google users who have a named group.
Given that you have multiple upstream connectors, there's currently a risk of groups with the same names but different meanings - Dex doesn't qualify the group claims with the connector name. So taking this further, DEX could have its own concept of groups which are explicitly mapped to connector groups:
Connector groups >----- DEX groups -----< Audience groups
What I am thinking is that:
baz: qux
or group: [admins]
In this case, end-user applications will see only Dex groups, not raw upstream IDP groups. Dex becomes the source of truth about which groups are used and for what. If an upstream IDP adds a new group, it won't be passed through until the Dex admin has enabled it (and there's an opportunity to add a comment to describe this group).
This gives more fine-grained control of which groups are allowed, how they are combined, and how they are presented to client apps. That works for me, but for people who still want raw connector group claims to be passed through unchanged, that could be added as an option too (at the client level).
I assume this is a more generic approach to how groups are handled and not related to the static password "connector" (which I believe this issue is about).
My first thought: I'm a bit reluctant to add first class support for mapping groups, primarily because connectors are already full of redundant features that are often not even useful to everyone, but has been added because some weird enterprise use case requires it.
So my gut reaction to this is to try and solve it with a middleware layer that we've been working on: #1841 You can use it to map/drop/allow/deny groups. WDYT?
I certainly agree this doesn't belong in individual connectors, this is something core/plugin/middleware indeed. I'll comment on the other ticket.
I've just had a testing use case where this would have been quite handy. While I see why making the password connector (actually, not a real connector yet (?): #1960) more suitable for production use cases is a bad idea, I'm not sure we are supposed to protect people from shooting themselves in the leg, at least not at this level.
So for the short term, I think we should add groups support to the password DB.
For the long term, I think the password DB should eventually be removed from Dex and replaced by an external password connector. Whether it should be a generic password gRPC proto to verify credentials or more like a plugin (#1907) is not clear. A gRPC based connector is probably easier to implement, but building the foundation of a plugin system would be more future proof.
So for the short term, I think we should add groups support to the password DB.
If it could be done in a simple way which allows creating groups across accounts from multiple connectors, I think that would be more generally useful. However I am not opposed in principle to groups in password DB; it would make it more like "any other" upstream.
For the long term, I think the password DB should eventually be removed from Dex and replaced by an external password connector. Whether it should be a generic password gRPC proto to verify credentials or more like a plugin (#1907) is not clear.
Isn't that basically what the LDAP connector is? (*)
Without inventing another protocol, another option is that you implement a standalone OpenIDC IDP, and point Dex's oidc connector at it. That is better than any type of remote password checker, because it can integrate whatever 2FA mechanisms it likes without having to pollute Dex with that sort of stuff. Given that Dex removed the account registration flow, I think that's the direction Dex is moving in - as the aggregator of IDPs, not an IDP itself.
Does anyone know of a lightweight standalone OpenIDC IDP with its own user database? Preferably with U2F support, or integration with an existing OTP/2FA library?
(*) The protocol is standardized on-the-wire, and the "bind" operation is widely abused as a password oracle. Many years ago I wrote an LDAP server framework in ruby - if nothing else, it demonstrates that providing the basic operations is fairly straightforward.
If it could be done in a simple way which allows creating groups across accounts from multiple connectors, I think that would be more generally useful.
I don't see Dex going into that direction TBH. That could change in the future, but we have more pressing matters at the moment.
Isn't that basically what the LDAP connector is? (*)
A generic password connector with an interchangeable backend would make it easier to integrate custom, password based solutions into Dex.
Also, as this issue also suggests, people don't want to setup an LDAP server for a testing environment, so a dummy password connector implementation bundled with Dex (either as an internal or an external implementation) still makes sense in my opinion.
If we can find a simple enough solution with relatively minimal configuration overhead, we could probably solve this with documentation though.
Also, as this issue also suggests, people don't want to setup an LDAP server for a testing environment
Here's an instant LDAP server (in go) which can be used for testing: https://github.com/glauth/glauth
Thanks, I'll give it a try. We migh be able to improve our LDAP integration tests using it.
Without inventing another protocol, another option is that you implement a standalone OpenIDC IDP, and point Dex's oidc connector at it. That is better than any type of remote password checker, because it can integrate whatever 2FA mechanisms it likes without having to pollute Dex with that sort of stuff. Given that Dex removed the account registration flow, I think that's the direction Dex is moving in - as the aggregator of IDPs, not an IDP itself.
I've been thinking about this: the problem is that even a thin OIDC relay for LDAP would have to implement a lot of parts of the specification whereas a simple interface that's purely for password verification only is much simpler to implement (though the 2FA issue is a valid point).
Also, as this issue also suggests, people don't want to setup an LDAP server for a testing environment
Here's an instant LDAP server (in go) which can be used for testing: https://github.com/glauth/glauth
I looked at glauth: it feels like an overkill for our use case to be honest, but I guess we can go with it for now.
We could use https://github.com/vjeantet/ldapserver to implement a very simple LDAP server.
@sagikazarmark wrote:
I've been thinking about this: the problem is that even a thin OIDC relay for LDAP would have to implement a lot of parts of the specification whereas a simple interface that's purely for password verification only is much simpler to implement (though the 2FA issue is a valid point).
That's true. This OIDC "relay" would be a small standalone IDP, and it would generate the web page that prompts for the username and password. On the flip side, this is functionality which could be moved out of Dex, if so desired.
If they were separate programs, the LDAP backend and PasswordDB backend would have to duplicate this code. They'd also have to run on different ports than Dex (although they could still use the same certificate). Or you could stick Dex and these other providers behind a reverse proxy to make them appear as a single site.
If the mock connector were also a tiny OIDC endpoint, it would have the advantage of exercising the OIDC protocol exchange for real.
Is there any chance that this feature will be implemented?
Hello @AljoschaP. Adding more user attributes to static passwords, e.g., groups, preferred user names, and so on, is not in our roadmap. Please consider alternatives such as deploying an LDAP server or an OIDC provider along with Dex for now.
Hello @nabokihms :-) Thanks a lot for your quick response. I'll take a look at https://github.com/glauth/glauth.
BTW: Thank you for your awesome work :+1:
Hello @AljoschaP. Adding more user attributes to static passwords, e.g., groups, preferred user names, and so on, is not in our roadmap. Please consider alternatives such as deploying an LDAP server or an OIDC provider along with Dex for now.
I was hoping to use groups with static users in dex for some CI and local setups where our application uses groups to make authorization decisions. Adding LDAP would be quite cumbersome for such a setup, and dex is 99% of the way there for what we need in an IDP for local dev setups and CI.
Would you be willing to reconsider?
As far as implementation complexity goes, this is probably like 5-10 lines of very simple code, mostly just adding fields. The burden of maintenance is low, and I find the risk of security related issues low. This would improve UX, and satisfy a common request (as evidenced by the age of this issue and the comments).
I understand the argument that dex does not want to be general purpose user store, but I'd argue that a few more fields wouldn't change that by much.
I also needed to have groups to static passwords for a project - as mentioned by various people in this issue. More or less, dex is lightweight, great for developing against even if it isn't used ultimately in production; static passwords are similarly useful, but often times one wants to be able to differentiate how a service behaves based on group membership presented to service.
I have a small branch that provides the group support, particularly from a config file though I guess I should add it to all the storage options.
Does anyone still want this? It was a minor change. I appreciate that dex isn't intended to be a identity provider, but on the other hand, it is much easier to do it this way than trying to also spin up an ldap, etc.
@venezia I briefly checked your PR, and there is one thing about the implementation I'd like to change - groups should not be the property of a user.
What is the most common flow?
Groups should be managed separately. Each group knows it's members. Let's say, in addition to users there are group objects in the Dex database. Admins can interact with them via gRPC API: create groups, delete groups, add and remove members.
This approach makes more sense to me.
@nabokihms I totally get your feedback. This ticket is 6 years old - and perhaps I had misunderstood some of the comments, but I felt that there wasn't a consensus of whether or not dex really wanted to have full-fledged group support in its internal store (logins being persisted to storage vs federated to an upstream identity provider) while there were users (like me) who perhaps leverage upstream identity providers (and their groups) in "production" yet wanted/needed group support for the local store for doing development/ci-cd/etc.
Time marches on, people come and go, and perhaps it is time to think about this again.
If you're OK with group support (or at least groups appearing in the claims on a JWT) for logins created either through the static configuration or through the local storage, i'm more than happy to create a more full-fledged implementation of groups within dex.
The branch I mentioned was merely a branch to help me with a current need (and in case other people had a similar need) - it wasn't intended to be a PR, just a conversation item ... which may have worked :)
Let me know and thanks for the reply!
@venezia Your implementation of groups looks quite straight forward. If we get it rebased I don't see why it shouldn't be added to dex main line. I'll have just tested your commit applied to the latest from dex master branch. It works as expected. Super how a such small change can give so much value. Thanks.
@nabokihms Regarding your comment on groups and users. I think this that will be an over-engineering of the implementation. There is no need for creating groups and managing add/remove user permissions. The static users will anyhow have to specify which groups the user belongs to. It really does not matter if the several users are member of the same group as there is no api for fetching users that belongs to a group (and there should not be such api in dex).
Hi @ErikLundJensen , any idea when @venezia's changes could be available on master ? It would be awesome to have this feature in order to test oidc providers locally using staticPasswords. Thank you.
Same need here. I'm using dex for simple PoC where having groups as part of the staticPasswords
could be really useful for testing and PoC. I don't want either to run an LDAP or another provider just to get this simple support. TBH I don't understand why a feature that could be really useful for so many people and for testing is on hold. I do understand that full featured groups support is not on the roadmap but having simple/static support that can help a lot of users IMHO should be something to consider.
I'm more than willing to write an implementation - just wanted to make sure it is desired, etc.
@venezia thanks for the changes. I used them to have a working setup :pray:
Any if this is going to be available any time soon? I could really use this feature.
we are using dex together with local development and e2e tests. It would be wonderful to have group support for local development as well. Currently we are doing hacks in application code to make it work with in similar way than production idp (which is not dex).
We would like to configure groups for statically configured users. Currently it is not possible as static passwords do not support this attribute.
Pull request provided: https://github.com/coreos/dex/pull/1079