Allow accessing CORS routes on ApiControllers if a proper CSRF token is provided

[juliusknorr]

The current implementation always requires basic auth when @CORS annotated routes are accessed. That leads to apps implementing basically two endpoints that have the same implementation but one with @CORS @NoCSRFRequired and one without those.

This PR would basically allow apps to provide one API endpoint for both internal usage in Nextcloud as well as external API calls, but the CORSMiddleware doesn't block the request if a valid CSRF token is provided.

I think this should cover the CSRF attack vectors that are described in

server/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php

Lines 85 to 86 in b801d27

	// ensure that @CORS annotated API routes are not used in conjunction
	// with session authentication since this enables CSRF attack vectors

or am I missing something?

---

The second commit is just for checking if username and password are actually provided so that the type hint added in

server/lib/public/User/Events/BeforeUserLoggedInEvent.php

Line 46 in 1b46621

public function __construct(string $username, string $password) {

doesn't cause a TypeError to be thrown but when calling logClientIn in the CORSMiddleware.

[nickvergessen]

And just using the OCS api internally as well does not work because?

[juliusknorr]

And just using the OCS api internally as well does not work because?

Yes that would also work of course. But this is not documented anywhere, while the app tutorial explains how the CORS routes work. Personally I also don't see any benefit in wrapping my apps API routes in to the OCS wrapper - is there any?

If we want to encourage app developers to use OCS then we need to explain that better.

[nickvergessen]

is there any?

Well it makes sure thet stuff works with mobile apps for free :)

That is exactly why we built OCS and OCS-Resources even. In Talk the full API is OCS. I think this should just be the way to go instead.

[juliusknorr]

That is exactly why we built OCS and OCS-Resources even. In Talk the full API is OCS. I think this should just be the way to go instead.

Could you provide some more insights on the decision back then? As to me the current way that API access is limited to OCS endpoints feels more like an artificial barrier, so app developers are pinned to the way OCS responses are structured.

Just one thing I encountered as of today is that with the current way OCSResponses are implemented it is not possible to emit a proper OCS response in a middleware that catches exceptions in order to convert them into responses. I use that approach for catching exceptions during controller execution in deck so the API returns a standardized error response and there is less duplicated code for the exception handling in the various controller methods.

[rullzer]

That is exactly why we built OCS and OCS-Resources even. In Talk the full API is OCS. I think this should just be the way to go instead.

Could you provide some more insights on the decision back then? As to me the current way that API access is limited to OCS endpoints feels more like an artificial barrier, so app developers are pinned to the way OCS responses are structured.

Just one thing I encountered as of today is that with the current way OCSResponses are implemented it is not possible to emit a proper OCS response in a middleware that catches exceptions in order to convert them into responses. I use that approach for catching exceptions during controller execution in deck so the API returns a standardized error response and there is less duplicated code for the exception handling in the various controller methods.

Then we'd need to fix that. However the OCS endpoints are public by definition. So i would propose to first fix cors there.

[juliusknorr]

Ok, so I guess we don't do that then.

[juliusknorr]

Could you provide some more insights on the decision back then? As to me the current way that API access is limited to OCS endpoints feels more like an artificial barrier, so app developers are pinned to the way OCS responses are structured.

I'd still be interested in some reasoning about the OCS enforcement, I'd be happy to make that clearer in our developer docs then.