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.

Sign up for GitHub

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

Jump to bottom

Add basic edit ldap auth test & actually fix #16252 #16465

Merged
merged 6 commits into from
Jul 20, 2021
Merged

Add basic edit ldap auth test & actually fix #16252 #16465

merged 6 commits into from
Jul 20, 2021

Conversation

zeripath
Copy link
Contributor

@zeripath zeripath commented Jul 16, 2021 •
edited
Loading

One of the reasons why #16447 was needed and why #16268 was needed in the first place was because it appears that editing ldap configuration doesn't get tested.

This PR therefore adds a basic test that will run the edit pipeline.

In doing so it's now clear that #16447 and #16268 aren't actually solving #16252. It turns out that what actually happens is that is that the bytes are actually double encoded.

This PR now changes the json unmarshal wrapper to handle this double encode.

Fix #16252
Fix #16447
Fix #16268

Signed-off-by: Andrew Thornton art27@cantab.net

@zeripath zeripath added this to the 1.16.0 milestone Jul 16, 2021
@zeripath
Copy link
Contributor Author

zeripath commented Jul 16, 2021 •
edited
Loading

I've marked it backport because we should probably put this into 1.15. I think we don't have to have this in 1.14 - as hopefully we'll be able to stop dev on that soon.

This unfortunately needs to be completely backported.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 16, 2021
@zeripath zeripath force-pushed the add-ldap-configuration-edit-tests branch from 31115d1 to 88d3a18 Compare July 16, 2021 17:26
@zeripath
Add basic edit ldap auth test & actually fix go-gitea#16252
699bd42 
One of the reasons why go-gitea#16447 was needed and why go-gitea#16268 was needed in
the first place was because it appears that editing ldap configuration
doesn't get tested.

This PR therefore adds a basic test that will run the edit pipeline.

In doing so it's now clear that go-gitea#16447 and go-gitea#16268 aren't actually
solving go-gitea#16252. It turns out that what actually happens is that is that
the bytes are actually double encoded.

This PR now changes the json unmarshal wrapper to handle this double
encode.

Fix go-gitea#16252

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath force-pushed the add-ldap-configuration-edit-tests branch from 88d3a18 to 699bd42 Compare July 16, 2021 20:49
@zeripath zeripath changed the title Add basic edit ldap auth test Add basic edit ldap auth test & actually fix #16252 Jul 16, 2021
@zeripath zeripath mentioned this pull request Jul 17, 2021
Merged
12 tasks
@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 Jul 17, 2021
@codecov-commenter
Copy link

codecov-commenter commented Jul 18, 2021 •
edited
Loading

Codecov Report

Merging #16465 (237d0e5) into main (2344007) will increase coverage by 0.07%.
The diff coverage is 28.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #16465      +/-   ##
==========================================
+ Coverage   45.42%   45.50%   +0.07%     
==========================================
  Files         717      717              
  Lines       84184    84198      +14     
==========================================
+ Hits        38240    38313      +73     
+ Misses      39795    39728      -67     
- Partials     6149     6157       +8
Impacted Files Coverage Δ
models/login_source.go 34.50% <10.00%> (+5.86%) ⬆️
models/repo_unit.go 88.09% <100.00%> (ø)
models/gpg_key_common.go 59.67% <0.00%> (-4.84%) ⬇️
models/unit.go 41.09% <0.00%> (-2.74%) ⬇️
modules/queue/workerpool.go 53.81% <0.00%> (-1.53%) ⬇️
modules/git/blame.go 65.71% <0.00%> (-1.43%) ⬇️
routers/web/repo/view.go 40.95% <0.00%> (-0.58%) ⬇️
modules/queue/unique_queue_disk_channel.go 48.63% <0.00%> (+1.36%) ⬆️
services/pull/patch.go 55.93% <0.00%> (+1.69%) ⬆️
modules/process/manager.go 75.30% <0.00%> (+2.46%) ⬆️
... and 4 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 2344007...237d0e5. Read the comment docs.

@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 Jul 20, 2021
@lafriks lafriks merged commit 2635778 into go-gitea:main Jul 20, 2021
@lafriks
Copy link
Member

lafriks commented Jul 20, 2021

Please send backport

@zeripath zeripath deleted the add-ldap-configuration-edit-tests branch July 20, 2021 11:45
zeripath added a commit to zeripath/gitea that referenced this pull request Jul 20, 2021
@zeripath
Add basic edit ldap auth test & actually fix go-gitea#16252 (go-gitea…
e4d3479 
…#16465)

Backport go-gitea#16465

One of the reasons why go-gitea#16447 was needed and why go-gitea#16268 was needed in
the first place was because it appears that editing ldap configuration
doesn't get tested.

This PR therefore adds a basic test that will run the edit pipeline.

In doing so it's now clear that go-gitea#16447 and go-gitea#16268 aren't actually
solving go-gitea#16252. It turns out that what actually happens is that is that
the bytes are actually double encoded.

This PR now changes the json unmarshal wrapper to handle this double
encode.

Fix go-gitea#16252

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath mentioned this pull request Jul 20, 2021
Merged
zeripath added a commit to zeripath/gitea that referenced this pull request Jul 20, 2021
@zeripath
Add basic edit ldap auth test & actually fix go-gitea#16252 (go-gitea…
e50309a 
…#16465)

Backport go-gitea#16465

One of the reasons why go-gitea#16447 was needed and why go-gitea#16268 was needed in
the first place was because it appears that editing ldap configuration
doesn't get tested.

This PR therefore adds a basic test that will run the edit pipeline.

In doing so it's now clear that go-gitea#16447 and go-gitea#16268 aren't actually
solving go-gitea#16252. It turns out that what actually happens is that is that
the bytes are actually double encoded.

This PR now changes the json unmarshal wrapper to handle this double
encode.

Fix go-gitea#16252

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath mentioned this pull request Jul 20, 2021
Merged
@zeripath zeripath added the backport/done All backports for this PR have been created label Jul 21, 2021
lafriks pushed a commit that referenced this pull request Jul 22, 2021
@zeripath @6543
Add basic edit ldap auth test & actually fix #16252 (#16465) (#16495)
6a3c785 
Backport #16465

One of the reasons why #16447 was needed and why #16268 was needed in
the first place was because it appears that editing ldap configuration
doesn't get tested.

This PR therefore adds a basic test that will run the edit pipeline.

In doing so it's now clear that #16447 and #16268 aren't actually
solving #16252. It turns out that what actually happens is that is that
the bytes are actually double encoded.

This PR now changes the json unmarshal wrapper to handle this double
encode.

Fix #16252

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: 6543 <6543@obermui.de>
lafriks pushed a commit that referenced this pull request Jul 22, 2021
@zeripath @6543
Add basic edit ldap auth test & actually fix #16252 (#16465) (#16494)
0b06b20 
Backport #16465

One of the reasons why #16447 was needed and why #16268 was needed in
the first place was because it appears that editing ldap configuration
doesn't get tested.

This PR therefore adds a basic test that will run the edit pipeline.

In doing so it's now clear that #16447 and #16268 aren't actually
solving #16252. It turns out that what actually happens is that is that
the bytes are actually double encoded.

This PR now changes the json unmarshal wrapper to handle this double
encode.

Fix #16252

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: 6543 <6543@obermui.de>
@zeripath zeripath mentioned this pull request Jul 22, 2021
Merged
@zeripath zeripath mentioned this pull request Jul 22, 2021
Merged
zeripath added a commit to zeripath/gitea that referenced this pull request Aug 4, 2021
@zeripath
Changelog for 1.14.6
53d48e4 
[1.14.6](https://github.com/go-gitea/gitea/releases/tag/v1.14.6) - 2021-08-04

* SECURITY
  * Bump github.com/markbates/goth from v1.67.1 to v1.68.0 (go-gitea#16538) (go-gitea#16540)
  * Switch to maintained JWT lib (go-gitea#16532) (go-gitea#16535)
  * Upgrade to latest version of golang-jwt (as forked for 1.14) (go-gitea#16590) (go-gitea#16607)
* BUGFIXES
  * Add basic edit ldap auth test & actually fix go-gitea#16252 (go-gitea#16465) (go-gitea#16495)
  * Make cancel from CatFileBatch and CatFileBatchCheck wait for the command to end (go-gitea#16479) (go-gitea#16481)

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath mentioned this pull request Aug 4, 2021
Merged
zeripath added a commit that referenced this pull request Aug 5, 2021
@zeripath
Changelog for 1.14.6 (#16619)
bb054fd 
## [1.14.6](https://github.com/go-gitea/gitea/releases/tag/v1.14.6) - 2021-08-04

* SECURITY
  * Bump github.com/markbates/goth from v1.67.1 to v1.68.0 (#16538) (#16540)
  * Switch to maintained JWT lib (#16532) (#16535)
  * Upgrade to latest version of golang-jwt (as forked for 1.14) (#16590) (#16607)
* BUGFIXES
  * Add basic edit ldap auth test & actually fix #16252 (#16465) (#16495)
  * Make cancel from CatFileBatch and CatFileBatchCheck wait for the command to end (#16479) (#16481)

Signed-off-by: Andrew Thornton <art27@cantab.net>
AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this pull request Aug 10, 2021
@zeripath @6543 @AbdulrhmnGhanem
Add basic edit ldap auth test & actually fix go-gitea#16252 (go-gitea…
207fb61 
…#16465)

One of the reasons why go-gitea#16447 was needed and why go-gitea#16268 was needed in
the first place was because it appears that editing ldap configuration
doesn't get tested.

This PR therefore adds a basic test that will run the edit pipeline.

In doing so it's now clear that go-gitea#16447 and go-gitea#16268 aren't actually
solving go-gitea#16252. It turns out that what actually happens is that is that
the bytes are actually double encoded.

This PR now changes the json unmarshal wrapper to handle this double
encode.

Fix go-gitea#16252

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: 6543 <6543@obermui.de>
@go-gitea go-gitea locked and limited conversation to collaborators Oct 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lafriks lafriks lafriks approved these changes

@6543 6543 6543 approved these changes

Assignees
No one assigned
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 type/testing
Projects
None yet
Milestone
1.16.0
Development

Successfully merging this pull request may close these issues.

Change LDAP - PANIC: runtime error: invalid memory address or nil pointer dereference
5 participants
@zeripath @codecov-commenter @lafriks @6543 @GiteaBot