-
Notifications
You must be signed in to change notification settings - Fork 12k
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
case-insensitive LDAP group comparison #9926
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking the time to contribute to Grafana.
One minor question about it.
pkg/login/ldap_user.go
Outdated
if member == group { | ||
return true | ||
} | ||
return strings.EqualFold(member, group) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure this is correct.
If the user is part of two groups it will always return based on the match of the first group. I don't think that's what you want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, I'll update the PR.
According to RFC2251 4.1.5, LDAP strings are case-insensitive. Disregard case when comparing group mappings.
Hey, can you re-reviews? I believe I've implemented requested changes. |
* grafana/master: Templating : return __empty__ value when all value return nothing to prevent elasticsearch syntaxe error (grafana#9701) http_server: All files in public/build have now a huge max-age (grafana#11536) fix: ldap unit test only error log when err is not nil rename alerting engine to service case-insensitive LDAP group comparison (grafana#9926) changelog: add notes about closing grafana#11813 docs: updated changelog fix XSS vulnerabilities in dashboard links (grafana#11813) PR: ux changes to grafana#11528 decrease length of auth_id column in user_auth table PR comments Make dashboard JSON editable
According to RFC2251 4.1.5, LDAP strings are case-insensitive. Disregard case when comparing group mappings.