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

Add check for LDAP group membership #10869

Merged
merged 11 commits into from
Sep 10, 2020
Merged

Conversation

jolheiser
Copy link
Member

This is a port of gogs/gogs#4398

The only changes made by myself are:

  • Add locales
  • Add some JS to the UI

Otherwise all code credit goes to @aboron

Resolves #10829

@jolheiser jolheiser added the type/enhancement An improvement of existing functionality label Mar 28, 2020
@jolheiser jolheiser added this to the 1.12.0 milestone Mar 28, 2020
@jolheiser jolheiser requested a review from lafriks March 28, 2020 20:34
routers/repo/compare.go Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 28, 2020
modules/auth/ldap/ldap.go Outdated Show resolved Hide resolved
modules/auth/ldap/ldap.go Outdated Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Mar 29, 2020

Codecov Report

Merging #10869 into master will increase coverage by 0.00%.
The diff coverage is 14.51%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #10869    +/-   ##
========================================
  Coverage   43.39%   43.40%            
========================================
  Files         593      593            
  Lines       83276    83392   +116     
========================================
+ Hits        36141    36198    +57     
- Misses      42643    42696    +53     
- Partials     4492     4498     +6     
Impacted Files Coverage Δ
modules/auth/auth_form.go 100.00% <ø> (ø)
routers/repo/compare.go 40.80% <ø> (-0.14%) ⬇️
modules/auth/ldap/ldap.go 46.41% <7.01%> (-7.85%) ⬇️
routers/admin/auths.go 32.33% <100.00%> (+1.14%) ⬆️
models/notification.go 62.47% <0.00%> (-2.25%) ⬇️
modules/queue/unique_queue_disk_channel.go 53.84% <0.00%> (-1.93%) ⬇️
services/pull/pull.go 34.70% <0.00%> (-1.18%) ⬇️
modules/queue/queue_wrapped.go 6.28% <0.00%> (-0.19%) ⬇️
models/repo.go 51.55% <0.00%> (+0.13%) ⬆️
models/repo_list.go 77.50% <0.00%> (+0.47%) ⬆️
... and 10 more

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 052bff0...047bccf. Read the comment docs.

@zeripath
Copy link
Contributor

I think we should take another look at #8814 which appears to do the same thing but also updates the cli commands and includes tests

@jolheiser
Copy link
Member Author

I think we should take another look at #8814 which appears to do the same thing but also updates the cli commands and includes tests

That's fine with me, however I don't know if I have the time to resolve those conflicts at the moment.
Should I close this one or continue?

@jolheiser
Copy link
Member Author

Closing in favor of #8814

@jolheiser jolheiser closed this Mar 30, 2020
@zeripath
Copy link
Contributor

I think we should reopen this and get it merged as #8814 appears to be stuck.

@zeripath zeripath reopened this Aug 28, 2020
@codecov-commenter
Copy link

codecov-commenter commented Aug 28, 2020

Codecov Report

Merging #10869 into master will decrease coverage by 0.00%.
The diff coverage is 14.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10869      +/-   ##
==========================================
- Coverage   43.24%   43.24%   -0.01%     
==========================================
  Files         650      650              
  Lines       72050    72099      +49     
==========================================
+ Hits        31157    31176      +19     
- Misses      35851    35877      +26     
- Partials     5042     5046       +4     
Impacted Files Coverage Δ
modules/auth/auth_form.go 100.00% <ø> (ø)
modules/auth/ldap/ldap.go 46.81% <4.44%> (-8.79%) ⬇️
routers/admin/auths.go 33.72% <100.00%> (+1.30%) ⬆️
modules/util/timer.go 42.85% <0.00%> (-42.86%) ⬇️
modules/indexer/stats/db.go 43.47% <0.00%> (-17.40%) ⬇️
modules/indexer/stats/queue.go 64.70% <0.00%> (-11.77%) ⬇️
modules/log/event.go 57.54% <0.00%> (-1.89%) ⬇️
models/gpg_key.go 55.40% <0.00%> (+0.58%) ⬆️
routers/repo/view.go 38.11% <0.00%> (+0.64%) ⬆️
services/mirror/mirror.go 19.93% <0.00%> (+1.35%) ⬆️
... and 5 more

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 4c42fce...81ec437. Read the comment docs.

jolheiser and others added 5 commits August 28, 2020 13:41
Signed-off-by: jolheiser <john.olheiser@gmail.com>
Signed-off-by: jolheiser <john.olheiser@gmail.com>
Signed-off-by: jolheiser <john.olheiser@gmail.com>
Signed-off-by: jolheiser <john.olheiser@gmail.com>
Co-Authored-By: Lauris BH <lauris@nix.lv>
Co-Authored-By: zeripath <art27@cantab.net>
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Aug 28, 2020
Copy link
Member

@lafriks lafriks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment

modules/auth/ldap/ldap.go Outdated Show resolved Hide resolved
@zeripath zeripath modified the milestones: 1.12.0, 1.13.0 Sep 5, 2020
@GiteaBot GiteaBot 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 Sep 10, 2020
@zeripath zeripath merged commit c3e8c94 into go-gitea:master Sep 10, 2020
@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
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LDAP User Filter
8 participants