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

LDAP team retrieval using wrong credentials #833

Closed
gitblit opened this issue Aug 12, 2015 · 5 comments
Closed

LDAP team retrieval using wrong credentials #833

gitblit opened this issue Aug 12, 2015 · 5 comments

Comments

@gitblit
Copy link
Owner

@gitblit gitblit commented Aug 12, 2015

Originally reported on Google Code with ID 537

What steps will reproduce the problem?
Configure gitblit to use LDAP authorization and read teams from LDAP. Specify LDAP
manager credentials for gitblit to use.

What is the expected output? What do you see instead?
The expected behavior is for gitblit to check team memberships for a user by using
the manager account, not that user's account since that user may not have the privileges
to see team memberships.

What version of the product are you using? On what operating system?
1.6.2 on Jetty 9, CentOS 6.3

Please provide any additional information below.
Browsing the source code, I noticed that after binding with the manager account, gitblit
rebinds as the user trying to log in (the comment says this is to prevent an LDAP injection
attack). Team memberships are then read after this, while bound to the LDAP server
as the user trying to log in, not the manager. I believe this is wrong since the user
doesn't have to be authorized to read team memberships.

Reported by hrvoje@mail.maracic.net on 2014-11-22 01:21:32

@gitblit
Copy link
Owner Author

@gitblit gitblit commented Aug 12, 2015

https://github.com/gitblit/gitblit/pull/247

Reported by redsam42 on 2015-03-23 13:46:49

@gitblit gitblit self-assigned this Aug 12, 2015
@flaix flaix added this to the 1.9.0 milestone Dec 13, 2016
@flaix flaix added Status-Obsolete and removed Status-New labels Dec 13, 2016
@flaix
Copy link
Collaborator

@flaix flaix commented Dec 13, 2016

This is fixed in PR #1149

@flaix flaix self-assigned this Dec 14, 2016
@steveno
Copy link
Contributor

@steveno steveno commented Apr 15, 2017

If this issue was fixed in #1149, should this be closed?

@flaix
Copy link
Collaborator

@flaix flaix commented Apr 16, 2017

Maybe. =)
To be honest I don't know yet what the best workflow is with labels and status and milestones and releases.
This is labeled as "Queued", i.e. implemented but not released. Which means it will be closed when the 1.9.0 is released. Maybe that isn't the smartest workflow and it could also be closed now.
I know, it's not consistent right now, as there are other implemented issues that have been closed. It's a bit of experimenting with workflows to see what works best in the Github frame.

@flaix flaix added Status-Fixed and removed Status-Obsolete labels Nov 10, 2019
@flaix
Copy link
Collaborator

@flaix flaix commented Nov 10, 2019

Closing this as fixed. Turns out keeping it open is more irritating than helpful. Adding the issue to a yet-to-be released milestone serves as a better "Queued" status.

@flaix flaix closed this as completed Nov 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants