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.

Sign up for GitHub

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

Jump to bottom

Allow users to configure group search filter and attributes (group_search_filter and group_attributes config) #168

Merged
merged 2 commits into from
Sep 16, 2024
Merged

Allow users to configure group search filter and attributes (group_search_filter and group_attributes config) #168

merged 2 commits into from
Sep 16, 2024

Conversation

kinow
Copy link
Contributor

@kinow kinow commented Jul 9, 2020

This change allowed me to connect to another server that was rejecting the connection due to only member being in its schema. Other attributes caused it to raise an exception, failing the authentication.

Closes #62
Closes #133
Related to #145

@welcome
Copy link

welcome bot commented Jul 9, 2020

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

kinow
kinow commented Jul 9, 2020
View reviewed changes
ldapauthenticator/tests/test_ldapauthenticator.py Outdated
@@ -100,3 +100,14 @@ async def test_ldap_auth_state_attributes(authenticator):
)
assert authorized["name"] == "fry"
assert authorized["auth_state"] == {"employeeType": ["Delivery boy"]}

async def test_ldap_auth_state_attributes2(authenticator):
authenticator.group_filter = "(cn=ship_crew)"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Played with this value, with the Docker container (same used in CI), causing it to fail with different values. Also tested locally with a different docker image that contains a more restrict schema in OpenLDAP, allowing only member in the search.

@kinow kinow force-pushed the configure-group-search branch 3 times, most recently from 31bdcf5 to 51f6a39 Compare July 9, 2020 03:49
@ordlucas
Copy link

Bumping this PR, any reason this hasn't been merged? This would fix the issues we've been having getting this configured for my organization's LDAP server.

@mananpreetsingh
Copy link

👍 Can this be merged, waiting for this option to fix group restriction using LDAP.

@mananpreetsingh
Copy link

@manics FYI ^

@mananpreetsingh
Copy link

Hi It's been more than a year, wonder if there is some blockers to fix this?

@consideRatio consideRatio mentioned this pull request Jun 4, 2023
Closed
12 tasks
consideRatio
consideRatio approved these changes Sep 15, 2024
View reviewed changes
Copy link
Member

@consideRatio consideRatio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wieee nice @kinow! I greatly appreciate that you have linked various issues etc as well from the PR description btw - helps a lot with triaging!! I arrived here from linked issues :)

@manics what do you think, ok to merge? It would feel good to have another pair of eyes on this.

@consideRatio
Copy link
Member

@kinow do you think this is good to merge still btw?

@consideRatio consideRatio changed the title Allow users to configure group search filter and attributes Allow users to configure group search filter and attributes (group_filter and group_attributes config) Sep 15, 2024
consideRatio
consideRatio reviewed Sep 15, 2024
View reviewed changes
README.md Outdated Show resolved Hide resolved
@kinow
Copy link
Contributor Author

kinow commented Sep 16, 2024

@kinow do you think this is good to merge still btw?

I think so! Let me just address your suggestion, and rebase it.

@kinow kinow force-pushed the configure-group-search branch 2 times, most recently from 8e218e8 to a797922 Compare September 16, 2024 08:54
@kinow
Copy link
Contributor Author

kinow commented Sep 16, 2024

Will look at the broken tests later

@consideRatio
Copy link
Member

Will look at the broken tests later

The test failures are unrelated and tracked by #246, as long as the jupyterhub 4 tests pass we are good!

@consideRatio consideRatio changed the title Allow users to configure group search filter and attributes (group_filter and group_attributes config) Allow users to configure group search filter and attributes (group_search_filter and group_attributes config) Sep 16, 2024
@consideRatio consideRatio merged commit f990420 into jupyterhub:main Sep 16, 2024
5 of 7 checks passed
@consideRatio
Copy link
Member

Thank you @kinow!!!

@kinow
Copy link
Contributor Author

kinow commented Sep 16, 2024

Will look at the broken tests later

The test failures are unrelated and tracked by #246, as long as the jupyterhub 4 tests pass we are good!

Thanks @consideRatio !!

@kinow kinow deleted the configure-group-search branch September 16, 2024 11:48
@consideRatio consideRatio mentioned this pull request Sep 16, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@consideRatio consideRatio consideRatio approved these changes

@manics manics manics left review comments

Assignees
No one assigned
Labels
new
Projects
None yet
Milestone
No milestone
5 participants
@kinow @ordlucas @mananpreetsingh @consideRatio @manics