Update Nginx config according to feedback

[MorrisJobke]

cc @josh4trunks @BernieO

ref #1280

[MorrisJobke]

/backport to stable15

[josh4trunks]

@MorrisJobke I wasnt sure if return 301 was better in this case, was wondering if @BernieO could chime in.

But, if we are going the return 301 route, then I think it makes sense to just go all the way and use 2 "location =". Give me a few minutes and I'll write up what I think could work.

[BernieO]

I chose a rewrite, because 301 are stored in the Browsers cache. I thought a good idea would be to not fill up the Browsers caches with 301 redirects to everything under /oc(m|s)-provider (dunno, whether much is there or not, though)
I agree that the .* at the end of the line is unnecessary
Literal matches like suggested by @josh4trunks would also be possible - so only two 301 redirects would be stored in the Browers cache.

EDIT: actually: if Nextcloud resides in a subfolder - will there also be requests for e.g. /oc(m|s)-provider/index.php (in the root directory)? Then literal matches ( location = ) won‘t work and a regular expression location match is needed.

[josh4trunks]

Makes sense. Then I suggest the configuration below. Could someone test to confirm? Thank you

location = /ocm-provider {
  return 301 $scheme://$host/nextcloud/ocm-provider/index.php;
}
location = /ocs-provider {
  return 301 $scheme://$host/nextcloud/ocs-provider/index.php;
}

[BernieO]

A trailing slash at the end of the location is needed - otherwise the warning in the admin panel does not disappear.
So it needs to be (I just tested this and this works):

location = /ocm-provider/ {
  return 301 $scheme://$host/nextcloud/ocm-provider/index.php;
}
location = /ocs-provider/ {
  return 301 $scheme://$host/nextcloud/ocs-provider/index.php;
}

Let me know, if I should I open a new pull request?

[josh4trunks]

Makes sense. I wonder if in practice clients using these endpoint use /ocm-provider/ or /ocm-provider/index.php

If either is supposed to work, then maybe we should keep this as a rewrite.

[BernieO]

If either is supposed to work, then maybe we should keep this as a rewrite.

I agree - this is what I meant with my EDIT above.

[josh4trunks]

Hmm, I do not think location = /ocm-provider/ would work if a client tries to GET /ocm-provder/index.php
My guess is it only works if you have index index.php or maybe some type of try_files defined.

@BernieO are you saying you can GET /ocm-provider/index.php with location = /ocm-provider/ and it properly serves /nextcloud/ocs-provider/index.php?

[josh4trunks]

Now that I familiarized myself further with the config (been a while since I looked at it!), I realize we already tried to handle this with the below location.

        location ~ ^\/nextcloud\/(?:updater|ocs-provider|ocm-provider)(?:$|\/) {
            try_files $uri/ =404;
            index index.php;
        }

I am wondering if the admin panel is just not handling the webroot correctly in its test? The only reason we need to define the /.well-known locations is because they are part of a specs outside of Nextcloud. Any ownCloud/Nextcloud clients should be able to know how to handle a non-standard webroot properly once you tell them your instance is located at https://example.com/nextcloud.

So, my hypothesis is, this is a bug in the admin panel, which should be fixed in its code. In that case #1280 would not be necessary. Please correct me if you find a mistake in my logic.

[skjnldsv]

Fine for me as well

[josh4trunks]

Before we move forward with this PR, can we confirm if the spec requires that /ocm-provider and /ocs-provider work, when the Nextcloud server is hosted under a webroot (ex. https://www.example.com/nextcloud). I suspect this is a bug with the code in the Admin Panel.

[BernieO]

@BernieO are you saying you can GET /ocm-provider/index.php with location = /ocm-provider/ and it properly serves /nextcloud/ocm-provider/index.php?

No, that doesn't work. When using an exact location match (location = /ocm-provider/ {), a request for /ocm-provider/index.php will result in a 404 (unless there are other directives in the config aside from the nextcloud-subdir one).
When using the “=” modifier, nginx tests for an exact match of location and URI.

If a request for /ocm-provider/index.php needs to deliver /nextcloud/ocm-provider/index.php we probably need to stick to the regex-rewrite like:

location ~ ^\/(?:ocm-provider|ocs-provider) {
        rewrite ^ /nextcloud$request_uri;
}

But I agree with @josh4trunks that we need to check first whether the ocm-/ocs-specs require that /ocm-provider/ and /ocs-provider/ do have to work, if Nextcloud resides in a subdir of the webroot.
I understand his logic:

So, my hypothesis is, this is a bug in the admin panel, which should be fixed in its code. In that case #1280 would not be necessary. Please correct me if you find a mistake in my logic.

[MorrisJobke]

Before we move forward with this PR, can we confirm if the spec requires that /ocm-provider and /ocs-provider work, when the Nextcloud server is hosted under a webroot (ex. https://www.example.com/nextcloud). I suspect this is a bug with the code in the Admin Panel.

Yes it works fine if installed to the web root. Also the checks just return fine.

The code for the check is in here:

https://github.com/nextcloud/server/blob/faef05730a2fca6ca019f645685ad6a08839d71c/settings/js/admin.js#L254-L255

This was added in nextcloud/server#14255 and @schiessle defined it there like this:

nextcloud/server#14247 (comment)

So it seems that the standard requires it to be served with the trailing slash.

@schiessle Could you elaborate on this one?

[MorrisJobke]

Also the /ocm-provider/ is mostly for API usage - it's for the OpenCloudMesh compatibility and thus is often used for federation between instances and not by the instance itself. So this is not that often cached in any browser at all.

[josh4trunks]

@MorrisJobke @schiessle
If a ownCloud/Nextcloud instance is located at https://www.example.com/nextcloud, which URL should ocm-provider and ocs-provider be located at? Thanks

1. https://www.example.com/nextcloud/ocm-provider and
   https://www.example.com/nextcloud/ocs-provider
   or...
2. https://www.example.com/ocm-provider and
   https://www.example.com/ocs-provider

[MorrisJobke]

3. https://www.example.com/ocm-provider and
   https://www.example.com/nextcloud/ocs-provider

[MorrisJobke]

3. https://www.example.com/ocm-provider and
   https://www.example.com/nextcloud/ocs-provider

Right, @schiessle ?

[josh4trunks]

Ok, would https://www.example.com/ocm-provider/index.php or https://www.example.com/nextcloud/ocs-provider/index.php ever be expected to work?

[MorrisJobke]

Sorry - it seems to be number 2. But let's wait for @schiessle to clarify on this. The setup check does it at least for 2.

[josh4trunks]

Ok, sounds good.

[josh4trunks]

@schiessle when you have a chance to help clarify what URL ocm-provider & ocs-provider are expected to be at on a server hosted at https://www.example.com/nextcloud we can do the rest =]

[josh4trunks]

My understanding is this can be closed. Thanks everyone
See #1333

[MorrisJobke]

My understanding is this can be closed. Thanks everyone

Correct 👍