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 basic edit ldap auth test & actually fix #16252 (#16465) #16494

Merged
merged 4 commits into from
Jul 22, 2021

Conversation

zeripath
Copy link
Contributor

@zeripath zeripath commented Jul 20, 2021

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
Fix #16447
Fix #16288

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

…#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 added this to the 1.15.0 milestone Jul 20, 2021
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Jul 20, 2021
@6543 6543 added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Jul 22, 2021
host, _ := doc.Find(`input[name="host"]`).Attr("value")
assert.Equal(t, host, getLDAPServerHost())
binddn, _ := doc.Find(`input[name="bind_dn"]`).Attr("value")
assert.Equal(t, binddn, "uid=gitea,ou=service,dc=planetexpress,dc=com")
Copy link
Member

Choose a reason for hiding this comment

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

Switch the expected and actual parameters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a backport so the problem needed to have been caught in the original PR.

Copy link
Member

Choose a reason for hiding this comment

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

oh, right

@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 22, 2021
@lafriks lafriks merged commit 0b06b20 into go-gitea:release/v1.15 Jul 22, 2021
@zeripath zeripath deleted the backport-16465-v1.15 branch July 22, 2021 14:39
@zeripath
Copy link
Contributor Author

zeripath commented Jul 22, 2021

Fixes #16448

@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.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants