Skip to content

Commit 34756e2

Browse files
christianlupusnickvergessenprovokateurin
authored
Apply suggestions from code review
Add modifications from reviews Co-authored-by: Joas Schilling <213943+nickvergessen@users.noreply.github.com> Co-authored-by: Kate <26026535+provokateurin@users.noreply.github.com> Signed-off-by: Christian Wolf <github@christianwolf.email>
1 parent 3071dbf commit 34756e2

3 files changed

Lines changed: 17 additions & 18 deletions

File tree

developer_manual/basics/controllers.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -564,7 +564,7 @@ Why does this work? Because the dispatcher sees that the controller did not retu
564564

565565
.. deprecated:: 30
566566

567-
Usage of classical controllers for data transmission should be avoided. Use OCS instead.
567+
Usage of "index.php"-controllers for data transmission should be avoided. Use OCS instead.
568568

569569

570570
Handling errors

developer_manual/digging_deeper/rest_apis.rst

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ REST APIs
77
.. sectionauthor:: Bernhard Posselt <dev@bernhard-posselt.com>
88

99
Offering a RESTful API is not different from creating a :doc:`route <../basics/routing>` and :doc:`controllers <../basics/controllers>` for the web interface.
10-
It is recommended though to inherit from ApiController and add **@CORS** annotations to the methods so that `web applications will also be able to access the API <https://developer.mozilla.org/en-US/docs/Web/HTTP/Access_control_CORS>`_.
10+
It is recommended though to inherit from ApiController and add ``#[CORS]`` attribute to the methods so that `web applications will also be able to access the API <https://developer.mozilla.org/en-US/docs/Web/HTTP/Access_control_CORS>`_.
1111

1212
.. code-block:: php
1313
@@ -103,22 +103,21 @@ The following combinations of attributes might be relevant for various scenarios
103103
.. warning::
104104
Adding the ``#[NoCRSFRequired]`` attribute imposes a security risk.
105105
You should not add this to your controller methods unless you understand the implications and be sure that you absolutely need the attribute.
106-
Typically, you can instead use the ``OCS-APIRequest`` header for data requests, instead.
106+
Typically, you can use the ``OCS-APIRequest`` header for data requests instead, in order to satisfy the CSRF checks in your API requests.
107107

108108
.. warning::
109109
Adding the attribute ``#[CORS]`` alone is not sufficient to allow access using CORS with plain frontend routes.
110110
Without further measures, the CSRF checker would fail.
111111
So, enabling CORS for plain controllers is generally and highly discouraged.
112112

113-
You would have to disable the CSRF checker (one more security risk) or use the ``OCP-APIRequest`` header to successfully pass the checker.
113+
You would have to disable the CSRF checks (one more security risk) or use the ``OCP-APIRequest`` header or send a CSRF token to successfully pass the checks.
114114
The latter requires dedicated JS code on the importing page.
115115

116116
There are different ways a clients might interact with your APIs.
117117
These ways depend on your API configuration (what you allow) and on which route the request is finally made.
118118

119-
- *Access from web frontend* means the user is browses the Nextcloud web frontend with a browser.
120-
- *Access from non-browser* is if the user accesses the resource or page not using the default browser.
121-
Thus can be e.g. a script using CURL or an Android app.
119+
- *Access from web frontend* means the user is accessing the Nextcloud web frontend with a web browser.
120+
- *Access from non-browser* is if the user accesses the resource or page using something that is not a web browser, like an Android app or a curl command.```
122121
- *Access from external website* means that the user browses some third party web site and data from your Nextcloud server appears.
123122
The other website has to embed/load/use images, JSON data, or other resources from a URL pointing to the Nextcloud server, to be able to do this.
124123

@@ -147,8 +146,8 @@ These ways depend on your API configuration (what you allow) and on which route
147146
- yes
148147
- yes
149148
* - Access from external website
150-
- ---
151-
- ---
149+
- no
150+
- no
152151
- yes
153152
* - Encapsulated data
154153
- no
@@ -157,12 +156,12 @@ These ways depend on your API configuration (what you allow) and on which route
157156

158157
.. [#] The external app has to satisfy the CSRF checks.
159158
That is, you need to have the ``OCS-APIRequest`` HTTP request header set to ``true``.
160-
This is only possible for NC 30 onwards, older versions do not respect the header.
159+
This is only possible for Nextcloud 30 onwards, older versions do not respect the header.
161160
162161
Methods from ``Controller`` classes can return ``DataResponse`` objects similar to ``OCSController`` class methods.
163162
For methods of a ``Controller`` class, the data of this response is sent e.g. as JSON as you provide it.
164163
Basically, the output is very similar to what ``json_encode`` would do.
165-
In contrast, the OCS controller will encapsulate the data in an outer shell that provides some more (meta) information.
164+
In contrast, the ``OCSController`` will encapsulate the data in an outer shell that provides some more (meta) information.
166165
For example a status code (similar to the HTTP status code) is transmitted at top level.
167166
The actual data is transmitted in the ``data`` property.
168167

@@ -177,7 +176,7 @@ Historical options
177176
.. deprecated:: 30
178177
The information in this section are mainly for reference purposes. Do not use the approaches in new code.
179178

180-
Before NC server 30 the plain ``Controller`` classes' methods did not respect the ``OCS-APIRequest`` header.
179+
Before Nextcloud 30 the plain ``Controller`` classes' methods did not respect the ``OCS-APIRequest`` header.
181180
Thus, to provide access to this type of controller methods for external apps, it was necessary to use the ``#[NoCSRFRequired]`` attribute (or the corresponding ``@NoCSRFRequired`` annotation).
182181

183182
The following combinations of attributes were relevant for various scenarios:
@@ -214,12 +213,12 @@ The following combinations of attributes were relevant for various scenarios:
214213
- yes (CSRF risk)
215214
- yes (CSRF risk)
216215
* - Access from non-browser
217-
- ---
216+
- no
218217
- yes
219218
- yes
220219
* - Access from external website
221-
- ---
222-
- ---
220+
- no
221+
- no
223222
- yes
224223
* - Encapsulated data
225224
- no

developer_manual/prologue/security.rst

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,8 @@ CORS
262262

263263
`Cross-origin resource sharing (CORS) <https://en.wikipedia.org/wiki/Cross-origin_resource_sharing>`_ (see also on `MDN <https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS>`__) is a method implemented by browser to access resources from different domains at the same time.
264264
Assume, there is a website published on host A.
265-
The URL would for example be https://A/path/to/index.html.
266-
If there is a _different_ host B that serves a resource (e.g. an image file) as https://B/assets/image.jpg, the index file on host A could simply link to the image on B.
265+
The URL would for example be ``https://A/path/to/index.html``.
266+
If there is a _different_ host B that serves a resource (e.g. an image file) as ``https://B/assets/image.jpg``, the index file on host A could simply link to the image on B.
267267
However, to protect B and its property (the image), the browsers do not silently embed the image of B into the page of A.
268268
Instead, B is kindly asked by the browser if embedding is allowed (the so-called `preflight <https://developer.mozilla.org/en-US/docs/Glossary/Preflight_request>`_).
269269

@@ -273,7 +273,7 @@ These define, what the browser is to be allowed to do.
273273
Only if the destination server B confirms cross site resource sharing is allowed, the browser access the resource.
274274

275275
Basically, accessing foreign resources is not limited to embedding images.
276-
Using JavaScript, arbitrary XHR/Ajax requests can be directed at arbitrary other hosts.
276+
Using JavaScript, arbitrary XHR/Ajax requests can be directed at arbitrary other hosts, which might be used to call APIs that leak your data.
277277
There are some safety measurements in place (especially about cookie handling), but one has still to be careful not to leak information unwillingly.
278278
Especially, if the destination server B allows to sent credentials using ``Access-Control-Allow-Credentials: true``, cross site scripting is very critical.
279279
You need :ref:`CSRF protection <csrf_introduction>` in place or your users are at relatively high risk.

0 commit comments

Comments
 (0)