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

fix(settings/SetupChecks): Don't follow redirects #46449

Closed
wants to merge 1 commit into from

Conversation

marcelklehr
Copy link
Member

Summary

In some cases the reverse proxy config redirects to login when accessing unknown URLs like .ocdata. Login, however, returns a HTTP 200, which is taken to be a failure by the datadirectory protection setup check.

  • Is this a good idea?

Checklist

@susnux
Copy link
Contributor

susnux commented Jul 11, 2024

I think not following is a bad idea as there could be a redirect that still resolves to the data directory.
Instead we should check for the final response body content?

@come-nc
Copy link
Contributor

come-nc commented Jul 11, 2024

I agree with Ferdinand it makes sense to follow redirects, at least for some tests.
But this test is too fragile and should be improved.

Maybe we should add content to .ocdata and check that the content is part of the response? (maybe it’s time to rename that file also)

@susnux
Copy link
Contributor

susnux commented Jul 30, 2024

See #46456

@susnux susnux closed this Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants