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

Request for public keys only if LDAP attribute is set #5816

Merged
merged 3 commits into from
Jan 23, 2019

Conversation

lafriks
Copy link
Member

@lafriks lafriks commented Jan 23, 2019

Should fix #5808

@lafriks lafriks added this to the 1.8.0 milestone Jan 23, 2019
@lafriks lafriks mentioned this pull request Jan 23, 2019
2 tasks
@bkcsoft bkcsoft added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Jan 23, 2019
@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@b9f8737). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff            @@
##             master   #5816   +/-   ##
========================================
  Coverage          ?   37.9%           
========================================
  Files             ?     328           
  Lines             ?   48205           
  Branches          ?       0           
========================================
  Hits              ?   18273           
  Misses            ?   27301           
  Partials          ?    2631
Impacted Files Coverage Δ
modules/auth/ldap/ldap.go 54.4% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b9f8737...f3aa1d7. Read the comment docs.

@lafriks
Copy link
Member Author

lafriks commented Jan 23, 2019

LGTM is stuck :)

@bkcsoft bkcsoft added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jan 23, 2019
@zeripath
Copy link
Contributor

Just one thing - could we adjust the LDAP tests to account for this case so we don't get stung by this again.

@lafriks
Copy link
Member Author

lafriks commented Jan 23, 2019

@zeripath it's not something really that could be reproduced with tests imho

@zeripath
Copy link
Contributor

Fair enough.

@lafriks lafriks merged commit 331c912 into go-gitea:master Jan 23, 2019
@lafriks lafriks deleted the fix/update-ldap branch January 23, 2019 23:25
lafriks added a commit to lafriks-fork/gitea that referenced this pull request Jan 23, 2019
* Update go-ldap dependency

* Request for public keys only if attribute is set
@lafriks lafriks added the backport/done All backports for this PR have been created label Jan 23, 2019
lafriks added a commit that referenced this pull request Jan 24, 2019
* Update go-ldap dependency

* Request for public keys only if attribute is set
bmackinney pushed a commit to bmackinney/gitea that referenced this pull request Jan 27, 2019
* Update go-ldap dependency

* Request for public keys only if attribute is set
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LDAP problem
5 participants