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

Add LDAPIdentityProvider.spec.groupSearch.userAttributeForFilter #1534

Merged
merged 11 commits into from
May 31, 2023

Conversation

cfryanr
Copy link
Member

@cfryanr cfryanr commented May 23, 2023

Addresses #1533.

Release note:

Added new `LDAPIdentityProvider.spec.groupSearch.userAttributeForFilter` and 
`ActiveDirectoryIdentityProvider.spec.groupSearch.userAttributeForFilter` configuration options.
The additional flexibility for LDAP and AD group searches introduced by this new configuration option can
be used to find groups in new ways, such as finding groups defined using the `posixGroup` schema.
For backwards compatibility, the group search defaults to the old behavior when this new option is not set.
For more details, see the API documentation https://github.com/vmware-tanzu/pinniped/blob/main/generated/1.27/README.adoc#ldapidentityprovidergroupsearch.

@cfryanr cfryanr marked this pull request as draft May 23, 2023 23:53
@codecov
Copy link

codecov bot commented May 24, 2023

Codecov Report

Merging #1534 (d004859) into main (33cc973) will increase coverage by 0.03%.
The diff coverage is 95.74%.

@@            Coverage Diff             @@
##             main    #1534      +/-   ##
==========================================
+ Coverage   75.39%   75.42%   +0.03%     
==========================================
  Files         165      165              
  Lines       14942    14968      +26     
==========================================
+ Hits        11265    11290      +25     
- Misses       3383     3385       +2     
+ Partials      294      293       -1     
Impacted Files Coverage Δ
...onfig/ldapupstreamwatcher/ldap_upstream_watcher.go 75.86% <71.42%> (-0.90%) ⬇️
...streamwatcher/active_directory_upstream_watcher.go 89.59% <100.00%> (+0.11%) ⬆️
internal/upstreamldap/upstreamldap.go 86.13% <100.00%> (+0.53%) ⬆️

... and 1 file with indirect coverage changes

Add the field to the tmpl file and run codegen.
Also update the count of the fields of our APIs in an integration test.
@cfryanr cfryanr force-pushed the ldap_userAttributeForFilter branch from ae4d6ac to bad5e60 Compare May 25, 2023 16:52
Load the setting in the controller.
Use the setting during authentication and during refreshes.
@cfryanr
Copy link
Member Author

cfryanr commented May 25, 2023

I manually tested the code committed so far on this WIP PR and it works.

Next, need to write some integration tests, probably in supervisor_login_test.go, which:

  1. Needs to work when run locally after using prepare-for-integration-tests.sh.
  2. Needs to work in CI on Kind clusters with the openldap server running in the same cluster.
  3. Needs to work in CI when run against JumpCloud's LDAP, or needs to be skipped in that case.

Some small tests could also be added to ldap_client_test.go.

@cfryanr
Copy link
Member Author

cfryanr commented May 26, 2023

Integration tests added!

@cfryanr cfryanr changed the title WIP: Add LDAPIdentityProvider.spec.groupSearch.userAttributeForFilter Add LDAPIdentityProvider.spec.groupSearch.userAttributeForFilter May 26, 2023
@cfryanr cfryanr marked this pull request as ready for review May 26, 2023 18:48
@cfryanr cfryanr marked this pull request as draft May 26, 2023 20:30
Also adds new integration test env var to support the new test:
PINNIPED_TEST_LDAP_EXPECTED_DIRECT_POSIX_GROUPS_CN
@cfryanr cfryanr force-pushed the ldap_userAttributeForFilter branch from 66c8649 to 552ecea Compare May 31, 2023 17:31
…Filter

Add the field to the tmpl file and run codegen.
Also update the count of the fields of our APIs in an integration test.
…earches

- Load the setting in the controller.
- The LDAP auth code is shared between AD and LDAP,
  so no new changes there in this commit.
@joshuatcasey joshuatcasey marked this pull request as ready for review May 31, 2023 19:40
@cfryanr cfryanr merged commit 5fa2992 into main May 31, 2023
@cfryanr cfryanr deleted the ldap_userAttributeForFilter branch May 31, 2023 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants