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

feat(settings): Migrate .well-known tests to SetupCheck #43939

Merged
merged 4 commits into from
Mar 7, 2024

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Mar 1, 2024

Summary

  1. This adds a generic request method to IClient that allow arbitrary HTTP methods (e.g. PROPFIND).
  2. Migrates the legacy frontend tests for .well-known URLs to SetupCheck

Checklist

@susnux susnux added enhancement 3. to review Waiting for reviews php Pull requests that update Php code labels Mar 1, 2024
@susnux susnux added this to the Nextcloud 29 milestone Mar 1, 2024
@susnux susnux requested review from nickvergessen, come-nc, a team, ArtificialOwl and sorbaugh and removed request for a team March 1, 2024 15:43
@susnux susnux force-pushed the enh/migrate-frontend-setupcheck-v2 branch 2 times, most recently from f5c3416 to 9af575a Compare March 1, 2024 18:48
@susnux susnux force-pushed the enh/migrate-frontend-setupcheck-v2 branch 2 times, most recently from 7d34665 to edbf943 Compare March 1, 2024 20:06
@come-nc
Copy link
Contributor

come-nc commented Mar 7, 2024

@nickvergessen Swapped the attribute order to match Guzzle.

* @return IResponse
* @throws \Exception If the request could not get completed
*/
public function request(string $method, string $uri, array $options = []): IResponse {
Copy link
Member

Choose a reason for hiding this comment

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

Should add consts for the methods at some point to document which are supported (and whether lowercase or uppercase is expected)

susnux and others added 4 commits March 7, 2024 14:06
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Co-authored-by: Côme Chilliet <come.chilliet@nextcloud.com>
Co-authored-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc come-nc force-pushed the enh/migrate-frontend-setupcheck-v2 branch from 9065c29 to 82fbab4 Compare March 7, 2024 13:06
@come-nc come-nc enabled auto-merge March 7, 2024 13:42
@come-nc come-nc merged commit bfcbffa into master Mar 7, 2024
160 checks passed
@come-nc come-nc deleted the enh/migrate-frontend-setupcheck-v2 branch March 7, 2024 14:10
@blizzz blizzz mentioned this pull request Mar 7, 2024
MichaIng added a commit that referenced this pull request Jun 24, 2024
#43939 moved the CalDAV/CardDAV redirect checks from the frontend to a new backend API.

Since the backend does not send an authentication header, checking for the expected response code 207 of the DAV endpoint does not work anymore, hence the URL of the last redirect is checked instead.

This URL is expected to contain a trailing slash, which was not required before, since the DAV endpoint works properly without it (when authenticated).

While a trailing slash in the redirect does no harm, it causes many setups to throw an admin panel warning, while in fact the redirects work properly. Furthermore, the proposed "/.well-known/carddav" => "/remote.php/dav/" redirect leads to double slashes, when doing a request to "/.well-known/carddav/", which seems more wrong then right.

This change makes the trailing slash optional, hence old and adjusted setups won't throw the warning anymore, and the DAV endpoint works well in both cases.

Signed-off-by: MichaIng <micha@dietpi.com>
backportbot bot pushed a commit that referenced this pull request Jun 25, 2024
#43939 moved the CalDAV/CardDAV redirect checks from the frontend to a new backend API.

Since the backend does not send an authentication header, checking for the expected response code 207 of the DAV endpoint does not work anymore, hence the URL of the last redirect is checked instead.

This URL is expected to contain a trailing slash, which was not required before, since the DAV endpoint works properly without it (when authenticated).

While a trailing slash in the redirect does no harm, it causes many setups to throw an admin panel warning, while in fact the redirects work properly. Furthermore, the proposed "/.well-known/carddav" => "/remote.php/dav/" redirect leads to double slashes, when doing a request to "/.well-known/carddav/", which seems more wrong then right.

This change makes the trailing slash optional, hence old and adjusted setups won't throw the warning anymore, and the DAV endpoint works well in both cases.

Signed-off-by: MichaIng <micha@dietpi.com>
susnux pushed a commit that referenced this pull request Jun 25, 2024
#43939 moved the CalDAV/CardDAV redirect checks from the frontend to a new backend API.

Since the backend does not send an authentication header, checking for the expected response code 207 of the DAV endpoint does not work anymore, hence the URL of the last redirect is checked instead.

This URL is expected to contain a trailing slash, which was not required before, since the DAV endpoint works properly without it (when authenticated).

While a trailing slash in the redirect does no harm, it causes many setups to throw an admin panel warning, while in fact the redirects work properly. Furthermore, the proposed "/.well-known/carddav" => "/remote.php/dav/" redirect leads to double slashes, when doing a request to "/.well-known/carddav/", which seems more wrong then right.

This change makes the trailing slash optional, hence old and adjusted setups won't throw the warning anymore, and the DAV endpoint works well in both cases.

Signed-off-by: MichaIng <micha@dietpi.com>
SebastianKrupinski pushed a commit that referenced this pull request Jun 25, 2024
#43939 moved the CalDAV/CardDAV redirect checks from the frontend to a new backend API.

Since the backend does not send an authentication header, checking for the expected response code 207 of the DAV endpoint does not work anymore, hence the URL of the last redirect is checked instead.

This URL is expected to contain a trailing slash, which was not required before, since the DAV endpoint works properly without it (when authenticated).

While a trailing slash in the redirect does no harm, it causes many setups to throw an admin panel warning, while in fact the redirects work properly. Furthermore, the proposed "/.well-known/carddav" => "/remote.php/dav/" redirect leads to double slashes, when doing a request to "/.well-known/carddav/", which seems more wrong then right.

This change makes the trailing slash optional, hence old and adjusted setups won't throw the warning anymore, and the DAV endpoint works well in both cases.

Signed-off-by: MichaIng <micha@dietpi.com>
MichaIng added a commit that referenced this pull request Jul 9, 2024
#43939 moved the CalDAV/CardDAV redirect checks from the frontend to a new backend API.

Since the backend does not send an authentication header, checking for the expected response code 207 of the DAV endpoint does not work anymore, hence the URL of the last redirect is checked instead.

This URL is expected to contain a trailing slash, which was not required before, since the DAV endpoint works properly without it (when authenticated).

While a trailing slash in the redirect does no harm, it causes many setups to throw an admin panel warning, while in fact the redirects work properly. Furthermore, the proposed "/.well-known/carddav" => "/remote.php/dav/" redirect leads to double slashes, when doing a request to "/.well-known/carddav/", which seems more wrong then right.

This change makes the trailing slash optional, hence old and adjusted setups won't throw the warning anymore, and the DAV endpoint works well in both cases.

Signed-off-by: MichaIng <micha@dietpi.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement php Pull requests that update Php code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants