Skip to content
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

expose dex's allowedGroups config option #3611

Closed
2 tasks
bcmdarroch opened this issue May 6, 2020 · 4 comments
Closed
2 tasks

expose dex's allowedGroups config option #3611

bcmdarroch opened this issue May 6, 2020 · 4 comments
Assignees
Labels
auth-team anything that needs to be on the auth team board customer-reported issues reported by customers documentation Anything related to the Automate docs. emergent

Comments

@bcmdarroch
Copy link
Contributor

bcmdarroch commented May 6, 2020

User Story

Dex recently added a feature that allows specifying certain SAML groups to allow. Exposing this will allow customers to reduce the number of groups passed around in requests.

Definition of Done

  • user can set config/submit a patch that specifies which groups should be allowed to authenticate and returned in dex's claims
  • document it

Demo Script / Repro Steps

[dex.v1.sys.connectors.saml]
groups_attr = "groups"
allowed_groups = ["IT", "ChefAdmins"]

chef-automate config patch saml.toml

Reference

dexidp/dex#1544

@bcmdarroch bcmdarroch added auth-team anything that needs to be on the auth team board emergent needs-triage and removed needs-triage labels May 6, 2020
@srenatus
Copy link
Contributor

srenatus commented May 7, 2020

habitat-sh/core-plans#3311 the plan is up to date ☑️

@srenatus srenatus self-assigned this May 7, 2020
@susanev susanev added the customer-reported issues reported by customers label May 8, 2020
@srenatus
Copy link
Contributor

Dex recently added a feature that allows specifying certain SAML groups to allow. Exposing this will allow customers to reduce the number of groups passed around in requests.

So... it just occurred to me that exposing the setting alone will not unbreak A2 for people with too many (SAML) groups. See this test case, and how ident is not changed in the relevant code section.

Hence exposing this setting will allow people to restrict the set of users that can login via SAML, to those being a member of the right groups, only. Any groups that are included in the SAML response for the user will be included in the ID token. It's not the equivalent of the LDAP group filter.

To really subset the incoming groups to a few, another code change to dex would be required.

@srenatus
Copy link
Contributor

Thinking about this more, what it gives us is a second way to restrict access to (SAML) users, which would be bypassing our IAM system. The same thing could be achieved by having a policy that grants access only to the chosen "allowed" SAML groups, I think.

If that assessment is correct, I'd rather not expose this config setting. At least not given the current code, non-filtering. I'll look into what it would take to filter upstream (dex).

@srenatus
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auth-team anything that needs to be on the auth team board customer-reported issues reported by customers documentation Anything related to the Automate docs. emergent
Projects
None yet
Development

No branches or pull requests

4 participants