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

5 participants
@lafriks
Copy link
Member

lafriks commented Jan 23, 2019

Should fix #5808

lafriks added some commits Jan 23, 2019

@lafriks lafriks added the kind/bug label Jan 23, 2019

@lafriks lafriks added this to the 1.8.0 milestone Jan 23, 2019

@lafriks lafriks referenced this pull request Jan 23, 2019

Closed

LDAP problem #5808

2 of 2 tasks complete

@bkcsoft bkcsoft added the lgtm/need 1 label Jan 23, 2019

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jan 23, 2019

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

This comment has been minimized.

Copy link
Member Author

lafriks commented Jan 23, 2019

LGTM is stuck :)

@bkcsoft bkcsoft added lgtm/done and removed lgtm/need 1 labels Jan 23, 2019

@zeripath

This comment has been minimized.

Copy link
Contributor

zeripath commented Jan 23, 2019

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

@lafriks

This comment has been minimized.

Copy link
Member Author

lafriks commented Jan 23, 2019

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

@zeripath

This comment has been minimized.

Copy link
Contributor

zeripath commented Jan 23, 2019

Fair enough.

@lafriks lafriks merged commit 331c912 into go-gitea:master Jan 23, 2019

2 checks passed

approvals/lgtm this commit looks good
continuous-integration/drone/pr the build was successful
Details

@lafriks lafriks deleted the lafriks:fix/update-ldap branch Jan 23, 2019

lafriks added a commit to lafriks/gitea that referenced this pull request Jan 23, 2019

Request for public keys only if LDAP attribute is set (go-gitea#5816)
* Update go-ldap dependency

* Request for public keys only if attribute is set

lafriks added a commit that referenced this pull request Jan 24, 2019

Request for public keys only if LDAP attribute is set (#5816) (#5819)
* Update go-ldap dependency

* Request for public keys only if attribute is set

bmackinney added a commit to bmackinney/gitea that referenced this pull request Jan 27, 2019

Request for public keys only if LDAP attribute is set (go-gitea#5816)
* Update go-ldap dependency

* Request for public keys only if attribute is set
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment