nextcloud
Adapted Webfinger and Nodeinfo for Nginx
I guess they were missing?! Another thing is that without them all checks are being passed - while adding them, it can results in more security and setup warnings!
@elgorro You have to sign the commits locally (git commit -s, the -s option is important), only then the test will run further/could pass.
@jonathanmmm Sry had an Issue where a strange "|C" was behind my email, I couldn't get rid off. Finally I did a local reset. In the following commit I forget to readd the files. So they are "null" I guess.... :/ Somehow the automatismn acceppted it. Please reopen.
@elgorro Can't reopen, have no rights. Maybe look at other pull requests that got through and add someone from there as review.
There was a dynamic redirect before that probably caught this case as well:
# Anything else is dynamically handled by Nextcloud
location ^~ /.well-known { return 301 /index.php$uri; }
But it was removed by this PR https://github.com/nextcloud/documentation/pull/6221
From this, it would make sense for me to add this. @MichaIng can you help us to clarify?
I even found an older pull request which does the same thing: https://github.com/nextcloud/documentation/pull/7130
I can try to replicate that/if the warnings appear with current suggested config. I personally still use selective redirects as well, but only since I don't want to have everything redirected which Nextcloud may or may not support (see my comments in PR where this was changed). But in case we should fix the intended "redirect everything else" directive instead of adding selective redirects.
There was a dynamic redirect before that probably caught this case as well:
Exactly, it was switched from "redirect specific well-known URLs to Nextcloud endpoint" to "exclude specific well-known URLs and redirect everything else to Nextcloud endpoint", now in line 108, which coveres webfinger and nodeinfo already, or does this not work in your case @elgorro?
It however renders these additional directives redundant. Also, if any change is done, it should be done for the subdirectory config as well: https://github.com/nextcloud/documentation/blob/master/admin_manual/installation/nginx-subdir.conf.sample
@MichaIng In my case I can confirm it works - I guess especially since PR #6221 (thanks for the info @tflidd) I had "major" problems with the instance - but had before "issues" with webfinger aswell.
The whole nodeinfo thing is a bit trickier! There was the error displayed in NC and this "fixed" it. Whats happening behind I cannot garentee or have the knowledge to test the sh!t out it.
Furthermore the real "issue" was as said before - that after adding these entries, NC is showing more "important" errors. I've no clue whats blocking it!
I could add the missing sub-directory entires, but someoneelse has to test them:).
Not sure whether I correctly understand: When you use the exact current example Nginx config, you still see both warnings? I will be able to try replicating next week, for both, root and sub directory configs.
@MichaIng No, with this "new" config it resolves all warnings - but (at least for me) I only had to fix more significant issues after. So the whole NC instance is much better now. I can't say if you will experience same isssues after, as I was before running two years with the "old" config.
I only had to fix more significant issues after.
Could you give more details which issues you needed to resolve? If the new config causes other warnings/issues, then it should be fixed, of course. But at least I cannot replicate any, aside of the HSTS header which you need to uncomment in the config, if wanted.
To everyone: Btw, I do not see any reason nowadays to not use HSTS, if HTTPS is used already. Shall we uncomment the HSTS header by default in these examples, but remove preload instead? preload is quite a different topic since one cannot easily remove the hostname from this list once it was added. Also one needs to register the hostname manually, so adding the header attribute alone usually has no effect. IMO it is out of scope, but we may add a short comment about HSTS preloading instead, mentioning https://hstspreload.org/ and the header attribute (or not, the website explains everything very well).
Question: the current version (without this PR) works regarding the webfinger and nodeinfo redirect? In this case we can close this PR and all other related PRs.
Not sure if you see other issues with the current nginx config, in this case, we could perhaps create an overview issue and then link individual PR. Just to keep it clean and clear.
Hi and thanks for your pull request :+1:
Webfinger and Nodeinfo should be forwarded to index.php.
