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

Whitespace wildcards in AD group search #1790

Closed
alondhe opened this issue Feb 17, 2021 · 5 comments · Fixed by #2277
Closed

Whitespace wildcards in AD group search #1790

alondhe opened this issue Feb 17, 2021 · 5 comments · Fixed by #2277
Assignees
Labels
Milestone

Comments

@alondhe
Copy link
Contributor

alondhe commented Feb 17, 2021

Expected behavior

When searching for groups in the Import LDAP function, search results return.

Actual behavior

Due to the usage of WhitespaceWildcardsFilter, the search string has wildcards on both sides, which causes the search to time out in larger AD systems.

https://github.com/OHDSI/WebAPI/blob/master/src/main/java/org/ohdsi/webapi/user/importer/providers/AbstractLdapProvider.java#L31

Steps to reproduce behavior

Connect to AD, import LDAP groups, click on a role and try to search for an AD group using a part of the full group name.

I worked around this by making a modified version of the WhitespaceWildcardsFilter class that just adds the wildcard at the end. I do think perhaps this field shouldn't add wildcards, rather, let the user (which is most likely an admin) explicitly state the search string (with or without wildcards).

@alondhe alondhe added the bug label Feb 17, 2021
@chrisknoll
Copy link
Collaborator

I agree with this, we shouldn't assume wildcard filtering on behalf of the WebAPI configuration manager. We should just document the use of wildcards if needed.

@alondhe
Copy link
Contributor Author

alondhe commented Aug 9, 2021

Well, the current design uses wildcard search and multi-select so that you can assign multiple AD groups to a role. But this presents challenges even beyond the performance hit of doing the full wildcard search; suppose your AD groups have different naming conventions, you couldn't multi-select them.

I think the cleanest approach is to have a string field that is comma separated. WebAPI can then perform a check on each group to see if they exist. Then move to the import steps.

@leeevans
Copy link
Contributor

@konstjar I spoke with @gklebanov to ask if the fix for this bug can be incorporated into the next release of Atlas/WebAPI and he suggested tagging you.

This bug is still negatively impacting organizations with large LDAP registries who want to use Atlas LDAP authentication and import users from LDAP into Atlas.

@alondhe suggested a code fix at the top of this issue:

"I worked around this by making a modified version of the WhitespaceWildcardsFilter class that just adds the wildcard at the end. I do think perhaps this field shouldn't add wildcards, rather, let the user (which is most likely an admin) explicitly state the search string (with or without wildcards)."

@konstjar konstjar added this to the v2.13.1 milestone Mar 28, 2023
@konstjar
Copy link
Contributor

@alex-odysseus Please assign someone from team to work on this

@leeevans
Copy link
Contributor

thanks @konstjar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Review Complete
Development

Successfully merging a pull request may close this issue.

7 participants