-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Move user password verification after checking his groups on ldap auth #19587
Conversation
Signed-off-by: Gwilherm Folliot <gwilherm55fo@gmail.com>
This comment was marked as off-topic.
This comment was marked as off-topic.
This feels like possible misuse of |
I don't think this is a misuse because since the checkbox |
I can not understand why this patch works. If |
The difference is when the group is fetched it call the method Here are the openldap logs without the patch, at the send it said that it bindint the user
But with my patch the group search is done before the user bind and there is not anymore error 32
|
I see the purpose and how it works. LGTM. However, that's quite strange in your case the a user can not read their own attributes .......... |
* giteaofficial/main: Fix broken TR on cherrypick page (go-gitea#19599) Use correct context in `routers/web` (go-gitea#19597) Use for a repo action one database transaction (go-gitea#19576) Only set CanColorStdout / CanColorStderr to true if the stdout/stderr is a terminal (go-gitea#19581) Don't fetch Mirror when it's migrating (go-gitea#19588) Move user password verification after checking his groups on ldap auth (go-gitea#19587)
go-gitea#19587) In case the binded user can not access its own attributes. Signed-off-by: Gwilherm Folliot <gwilherm55fo@gmail.com> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This PR move the user password verifications after groups checking.
In my LDAP setup i noticed that groups check always failed because the bind dn changed to be the user one, or the user don't have the permission to see his roles