SAML: whitelist groups?

[srenatus]

So, for LDAP through the group filter, and for some other connectors (like github, gitlab, bitbucket), we support whitelisting a bunch of groups -- so beyond authentication, only the groups listed will be considered...

❓ What do you think, should the SAML connector have a similar feature?

[kenperkins]

This is also something I'd love to know if we can do. Would contributions on this topic be welcomed?

[srenatus]

Would contributions on this topic be welcomed?

Sure! 😃 At this point, however, I wonder if we shouldn't take a step back and change how connectors that supply dex with groups work. We've got the group filtering logic in a few connectors -- maybe it should be refactored out of them, in to the handler instead? Then, it would be straightforward to extend this feature to all connectors in a homogenous way. What do you think?

[kenperkins]

Any time we can provide a generalized solution that works across connectors, that would be great. Something like a group filtering interface that some connectors could then implement?

[srenatus]

I'd thought that the connectors would expose a bit of their configuration -- the whitelisted groups -- and the handler code could, after the identity has been provided by the connector, run the filtering steps for the identity's groups against that list...

But I haven't looked deeper into this, maybe there's a more elegant way 😄

[kenperkins]

I took a look at the group filtering in GitHub and LDAP, and there's a fair amount of complexity in both of those relative to SAML. GitHub has two tiers: Orgs and Teams, LDAP has a group search with optional filter.

Perhaps you're thinking that after the connector specific logic to "get" the groups for a given user, you could have a handler that filters based on a normalized representation of the groups? Is that worth the complexity?

Regardless of which direction you want to go, I decided to implement the SAML whitelist in #1544 anyway as it's simple and straight forward and won't be a breaking change.

[srenatus]

I'm thinking about adding another flag, like filterGroups: bool to actually exclude the non-whitelisted groups... 🤔