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

LDAPIdentityProvider could support group queries using an attributes from the authenticating user other than DN #1533

Closed
cfryanr opened this issue May 23, 2023 · 0 comments
Assignees
Labels
enhancement New feature or request estimate/L Estimated effort/complexity/risk is large state/accepted All done!

Comments

@cfryanr
Copy link
Member

cfryanr commented May 23, 2023

Is your feature request related to a problem? Please describe.

The group search capability of the Pinniped Supervisor's LDAPIdentityProvider CR could be more flexible.

For example, given an LDAP server in which users are defined like this:

dn: uid=pinny,ou=people,dc=example,dc=com
objectClass: inetOrgPerson
objectClass: posixAccount
objectClass: top
objectClass: inetLocalMailRecipient
cn: pinny
uid: pinny
mailRoutingAddress: pinny@pa-exch1.example.com
uidNumber: 123456789
gidNumber: 201
loginShell: /bin/bash
givenName: Pinny
sn: Seal
mail: pinny@example.com
homeDirectory: /home/pinny

And groups are defined like this:

dn: cn=my-admins,ou=group,dc=example,dc=com
objectClass: posixGroup
objectClass: top
cn: my-admins
gidNumber: 3000038
description: 1052335
memberUid: pinny
memberUid: seal
memberUid: walrus

Then I would like to be able to configure the LDAP group search query to compare the authenticating user's uid value to the group's memberUid attributes, e.g. (&(objectClass=posixGroup)(memberUid={})) where Pinniped would automatically replace the {} placeholder with the authenticating user's uid value. However, currently the LDAPIdentityProvider will only replace the {} placeholder with the user's dn (which is to enable member={} group searching). This is not currently configurable.

Describe the solution you'd like

Some way to tell the LDAPIdentityProvider's spec which attribute of the authenticating user's record should be used to fill in the {} placeholder of the group search filter setting. When this new setting is not provided, then it should default to the old behavior of using the dn to replace the placeholder to make it backwards compatible and to make sure that behavior does not change as the result of an upgrade to a version which includes this new feature.

One possibility would be to add a new option LDAPIdentityProvider.spec.groupSearch.userAttributeForFilter.

For example:

  # Specify how to search for the group membership of an end-user during login.
  groupSearch:

    # Specify the root of the group search.
    base: "dc=pinniped,dc=dev"

    # Specify the search filter which should be applied when searching for
    # groups for a user. "{}" will be replaced by the value of an attribute of
    # the user entry found as a result of the user search. Which attribute
    # depends on the value of userAttributeForFilter below.
    filter: "&(objectClass=posixGroup)(memberUid={})"

    # Specify the name of the attribute of the user entry found as a result
    # of the user search whose value will be used to replace the "{}"
    # placeholder in the filter above. When blank, this field will default to
    # behaving the same as when set to "dn".
    userAttributeForFilter: "uid"

    # Specify which fields from each group entry should be used upon
    # successful login.
    attributes:

      # Specify the name of the attribute in the LDAP entries whose value
      # shall become a group name in the user’s list of groups after a
      # successful authentication.
      groupName: "cn"

The same enhancement could be made to ActiveDirectoryIdentityProvider to maintain the symmetry between the configuration options of ActiveDirectoryIdentityProvider and LDAPIdentityProvider, which previously supported the exact same configuration options (with different defaults).

Describe alternatives you've considered

Also considered changing the filter syntax to allow the user to specify the name of the attribute inside the placeholder, e.g. &(objectClass=posixGroup)(memberUid={uid}). This allows a little more flexibility because multiple placeholders could be used to support replacing different user attributes into different parts of the filter. However, the practical usefulness for this seems limited and it adds extra complexity to the implementation.

Are you considering submitting a PR for this feature?

Yes.

  • How will this project improvement be tested? Unit and integration tests.
  • How does this change the current architecture? Doesn't.
  • How will this change be backwards compatible? Described above.
  • How will this feature be documented? Added to the auto-generated documentation of LDAPIdentityProvider. Could also be added to the examples shown on the web site for LDAP providers.

Notes

The existing group search settings are documented here.

@pinniped-ci-bot pinniped-ci-bot added enhancement New feature or request priority/backlog Prioritized for an upcoming iteration estimate/L Estimated effort/complexity/risk is large labels May 25, 2023
@pinniped-ci-bot pinniped-ci-bot added state/started Someone is working on it currently state/finished Code finished but not yet delivered state/delivered Ready for manual acceptance review and removed state/started Someone is working on it currently state/finished Code finished but not yet delivered labels May 25, 2023
@pinniped-ci-bot pinniped-ci-bot added state/accepted All done! and removed priority/backlog Prioritized for an upcoming iteration state/delivered Ready for manual acceptance review labels Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request estimate/L Estimated effort/complexity/risk is large state/accepted All done!
Projects
None yet
Development

No branches or pull requests

2 participants