From c8441ab9bbd247ae23fb46e0d523f5d839bfa53e Mon Sep 17 00:00:00 2001 From: Christian Wolf Date: Thu, 3 Oct 2024 20:31:51 +0200 Subject: [PATCH 01/13] Started to fix unclear statements about OCS and REST Signed-off-by: Christian Wolf --- developer_manual/basics/controllers.rst | 8 ++- developer_manual/digging_deeper/rest_apis.rst | 69 ++++++++++++++++++- 2 files changed, 73 insertions(+), 4 deletions(-) diff --git a/developer_manual/basics/controllers.rst b/developer_manual/basics/controllers.rst index c8de6729c03..3ce160caeb4 100644 --- a/developer_manual/basics/controllers.rst +++ b/developer_manual/basics/controllers.rst @@ -448,10 +448,14 @@ The user only indirectly requested the data by user interaction with the fronten OCS ^^^ +In order to simplify exchange of data between the Nextcloud backend and any client (be it the web frontend or whatever else), the OCS API has been introduced. +Here, JSON and XML responders have been prepared and are installed without additional effort. + .. note:: - This is purely for compatibility reasons. If you are planning to offer an external API, go for a :ref:`REST APIs ` instead. + The usage of OCS is closely related to the usage of :doc:`../digging_deeper/rest_apis`. + Unless you have a clear use-case, it is advised to use OCS over pure REST. + A more detailed description can be found in :ref:`ocs-vs-rest`. -In order to simplify exchange of data between the Nextcloud backend and any client (be it the web frontend or whatever else), the OCS API has been introduced. To use OCS in your API you can use the **OCP\\AppFramework\\OCSController** base class and return your data in the form of a **DataResponse** in the following way: .. code-block:: php diff --git a/developer_manual/digging_deeper/rest_apis.rst b/developer_manual/digging_deeper/rest_apis.rst index 8a634f24027..354e70ee557 100644 --- a/developer_manual/digging_deeper/rest_apis.rst +++ b/developer_manual/digging_deeper/rest_apis.rst @@ -6,7 +6,8 @@ REST APIs .. sectionauthor:: Bernhard Posselt -Offering a RESTful API is not different from creating a :doc:`route <../basics/routing>` and :doc:`controllers <../basics/controllers>` for the web interface. 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 `_. +Offering a RESTful API is not different from creating a :doc:`route <../basics/routing>` and :doc:`controllers <../basics/controllers>` for the web interface. +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 `_. .. code-block:: php @@ -44,7 +45,8 @@ CORS also needs a separate URL for the preflighted **OPTIONS** request that can ) -Keep in mind that multiple apps will likely depend on the API interface once it is published and they will move at different speeds to react to changes implemented in the API. Therefore it is recommended to version the API in the URL to not break existing apps when backwards incompatible changes are introduced:: +Keep in mind that multiple apps will likely depend on the API interface once it is published and they will move at different speeds to react to changes implemented in the API. +Therefore it is recommended to version the API in the URL to not break existing apps when backwards incompatible changes are introduced:: /index.php/apps/myapp/api/1.0/resource @@ -79,3 +81,66 @@ To add an additional method or header or allow less headers, simply pass additio } } + +.. _ocs-vs-rest: + +Relation of REST and OCS +------------------------ + +There is a close relationship between REST APIs and :ref:`OCS `. +Both provide a way to transmit data between the backend of the app in the Nextcloud server and some frontend. + +The following combinations of attributes might be useful for various scenarios: + +#. Plain frontend route: No special attribute related to CSRF or CORS +#. Plain Frontend with CRSF checks disabled: Add the ``#[NoCSRFRequired]`` attribute +#. REST route with CORS enabled: Add the ``#[CORS]`` attribute +#. OCS-based route + +There are different ways a clients might interact with your APIs. +These ways depend on your API configuration (what you allow) and on which route the request is finally made. + +- *Access from web frontend* means the user is browses the Nextcloud web frontend with a browser. +- *Access from an external app* indicates that the user is not using the normal browser (as logged in) but directly navigates a certain URL. + This can be in a browser tab or an external program (like an Android app or simply a curl command line). +- *Access from external website* means that the user browses some third party web site and *magically* data from your app appears. + Technically, the other website would embed/load/use images, JSON data, or other resources from a URL pointing to the Nextcloud server. + +.. list-table:: Comparision of different API types + :header-rows: 1 + :align: center + + * - Description + - 1 (plain) + - 2 (no CSRF) + - 3 (CORS) + - 4 (OCS) + * - URL prefix (relative to server) + - ``/apps//`` + - ``/apps//`` + - ``/apps//`` + - ``/ocs/v2.php/apps//`` + * - Access from web frontend + - yes + - yes (CSRF risk) + - yes (CSRF risk) + - yes + * - Access from external app + - --- (CSRF protection blocks) + - yes + - yes + - yes + * - Access from external web page + - --- + - --- + - yes + - yes + * - Transmitted data type + - plain data + - plain data + - plain data + - encapsulated data + +As a rule of thumb one can conclude that OCS provides a good way to handle most use cases. +The only exception to this is if you want to provide an API for external usage where you have to comply with an externally defined API scheme. +Here, the encapsulation introduced in OCS might be in your way. From 0b3b355b1b6094911082144b9341808ddb31bd72 Mon Sep 17 00:00:00 2001 From: Christian Wolf Date: Thu, 3 Oct 2024 20:32:08 +0200 Subject: [PATCH 02/13] Fix some sphinx build watrnings Signed-off-by: Christian Wolf --- developer_manual/conf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/developer_manual/conf.py b/developer_manual/conf.py index 7770ac004b1..d8f7f37a838 100644 --- a/developer_manual/conf.py +++ b/developer_manual/conf.py @@ -183,7 +183,7 @@ #'pointsize': '10pt', # Additional stuff for the LaTeX preamble. -'preamble': '\extrafloats{100}\maxdeadcycles=500\DeclareUnicodeCharacter{274C}{\sffamily X}', +'preamble': '\\extrafloats{100}\\maxdeadcycles=500\\DeclareUnicodeCharacter{274C}{\\sffamily X}', } # Grouping the document tree into LaTeX files. List of tuples From affa35594e139fa086ac4eeca8a08c914234e9b8 Mon Sep 17 00:00:00 2001 From: Christian Wolf Date: Fri, 4 Oct 2024 00:04:53 +0200 Subject: [PATCH 03/13] Apply suggestions from code review Co-authored-by: Kate <26026535+provokateurin@users.noreply.github.com> Signed-off-by: Christian Wolf --- developer_manual/digging_deeper/rest_apis.rst | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/developer_manual/digging_deeper/rest_apis.rst b/developer_manual/digging_deeper/rest_apis.rst index 354e70ee557..9594609b6fd 100644 --- a/developer_manual/digging_deeper/rest_apis.rst +++ b/developer_manual/digging_deeper/rest_apis.rst @@ -92,10 +92,10 @@ Both provide a way to transmit data between the backend of the app in the Nextcl The following combinations of attributes might be useful for various scenarios: -#. Plain frontend route: No special attribute related to CSRF or CORS +#. Plain frontend route: ``Controller`` class #. Plain Frontend with CRSF checks disabled: Add the ``#[NoCSRFRequired]`` attribute -#. REST route with CORS enabled: Add the ``#[CORS]`` attribute -#. OCS-based route +#. REST route with CORS enabled: ``Controller`` class and ``#[CORS]`` attribute on the route +#. OCS-based route: ``OCSController`` class There are different ways a clients might interact with your APIs. These ways depend on your API configuration (what you allow) and on which route the request is finally made. @@ -134,13 +134,13 @@ These ways depend on your API configuration (what you allow) and on which route - --- - --- - yes - - yes + - no * - Transmitted data type - plain data - plain data - plain data - - encapsulated data + - encapsulated data (JSON or XML) -As a rule of thumb one can conclude that OCS provides a good way to handle most use cases. +As a rule of thumb one can conclude that OCS provides a good way to handle most use cases including sufficient security checks. The only exception to this is if you want to provide an API for external usage where you have to comply with an externally defined API scheme. -Here, the encapsulation introduced in OCS might be in your way. +Here, the encapsulation introduced in OCS and CSRF checks might be in your way. From 33976b1d763855f56fa5c9b369532b0f564e9863 Mon Sep 17 00:00:00 2001 From: Christian Wolf Date: Mon, 7 Oct 2024 23:58:02 +0200 Subject: [PATCH 04/13] Fix OCS description after some research Signed-off-by: Christian Wolf --- developer_manual/digging_deeper/rest_apis.rst | 50 +++++++++++-------- 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/developer_manual/digging_deeper/rest_apis.rst b/developer_manual/digging_deeper/rest_apis.rst index 9594609b6fd..f8eb89a847e 100644 --- a/developer_manual/digging_deeper/rest_apis.rst +++ b/developer_manual/digging_deeper/rest_apis.rst @@ -90,57 +90,63 @@ Relation of REST and OCS There is a close relationship between REST APIs and :ref:`OCS `. Both provide a way to transmit data between the backend of the app in the Nextcloud server and some frontend. -The following combinations of attributes might be useful for various scenarios: +The following combinations of attributes might be relevant for various scenarios: #. Plain frontend route: ``Controller`` class -#. Plain Frontend with CRSF checks disabled: Add the ``#[NoCSRFRequired]`` attribute -#. REST route with CORS enabled: ``Controller`` class and ``#[CORS]`` attribute on the route +#. Plain Frontend with CRSF checks disabled: ``Controller`` class and ``#[NoCSRFRequired]`` attribute on the method #. OCS-based route: ``OCSController`` class +#. OCS-based route with CSRF disabled: ``OCSController`` class and ``#[NoCSRFRequired]`` attribute on the method + +.. warning:: + Adding the ``#[NoCRSFRequired]`` attribute imposes a security risk. + You should not add this to your controller methods unless you understand the implications and be sure that you absolutely need the attribute. There are different ways a clients might interact with your APIs. These ways depend on your API configuration (what you allow) and on which route the request is finally made. - *Access from web frontend* means the user is browses the Nextcloud web frontend with a browser. - *Access from an external app* indicates that the user is not using the normal browser (as logged in) but directly navigates a certain URL. - This can be in a browser tab or an external program (like an Android app or simply a curl command line). -- *Access from external website* means that the user browses some third party web site and *magically* data from your app appears. - Technically, the other website would embed/load/use images, JSON data, or other resources from a URL pointing to the Nextcloud server. + This can be in a new browser tab or an external program (like an Android app or simply a curl command line). -.. list-table:: Comparision of different API types +.. list-table:: Comparison of different API types :header-rows: 1 :align: center * - Description - 1 (plain) - - 2 (no CSRF) - - 3 (CORS) + - 2 (w/o CSRF) - 4 (OCS) + - 4 (OCS w/o CSRF) * - URL prefix (relative to server) - - ``/apps//`` - ``/apps//`` - ``/apps//`` - ``/ocs/v2.php/apps//`` + - ``/ocs/v2.php/apps//`` * - Access from web frontend - yes - yes (CSRF risk) - - yes (CSRF risk) - yes + - yes (CSRF risk) * - Access from external app - - --- (CSRF protection blocks) - - yes - - yes - - yes - * - Access from external web page - - --- - --- - yes + - yes (with header [#]_) + - yes + * - Encapsulated data - no - * - Transmitted data type - - plain data - - plain data - - plain data - - encapsulated data (JSON or XML) + - no + - yes (JSON or XML) + - yes (JSON or XML) + +Methods from ``Controller`` classes can return ``DataResponse`` objects similar to ``OCSController`` class methods. +For methods of a ``Controller`` class, the data of this response is sent e.g. as JSON as you provide it. +Basically, the output is very similar to what ``json_encode`` would do. +In contrast, the OCS controller will encapsulate the data in an outer shell that provides some more (meta) information. +For example a status code (similar to the HTTP status code) is transmitted at top level. +The actual data is transmitted in the ``data`` property. As a rule of thumb one can conclude that OCS provides a good way to handle most use cases including sufficient security checks. The only exception to this is if you want to provide an API for external usage where you have to comply with an externally defined API scheme. Here, the encapsulation introduced in OCS and CSRF checks might be in your way. + +.. [#] The OCS controller needs the request header ``OCS-APIREQUEST`` to be set to ``true``. From 96fcdc34278c1bb043bd91d96affe17c994a03fb Mon Sep 17 00:00:00 2001 From: Christian Wolf Date: Thu, 10 Oct 2024 00:36:14 +0200 Subject: [PATCH 05/13] Adding CORS back into the mix Signed-off-by: Christian Wolf --- developer_manual/digging_deeper/rest_apis.rst | 31 ++++++++++++++++--- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/developer_manual/digging_deeper/rest_apis.rst b/developer_manual/digging_deeper/rest_apis.rst index f8eb89a847e..f4ff8697cae 100644 --- a/developer_manual/digging_deeper/rest_apis.rst +++ b/developer_manual/digging_deeper/rest_apis.rst @@ -93,20 +93,29 @@ Both provide a way to transmit data between the backend of the app in the Nextcl The following combinations of attributes might be relevant for various scenarios: #. Plain frontend route: ``Controller`` class -#. Plain Frontend with CRSF checks disabled: ``Controller`` class and ``#[NoCSRFRequired]`` attribute on the method +#. Plain frontend with CRSF checks disabled: ``Controller`` class and ``#[NoCSRFRequired]`` attribute on the method +#. REST route with CORS enabled: ``Controller`` class and ``#[CORS]`` and ``#[NoCSRFRequired]`` attributes on the route #. OCS-based route: ``OCSController`` class -#. OCS-based route with CSRF disabled: ``OCSController`` class and ``#[NoCSRFRequired]`` attribute on the method +#. OCS-based route with CORS enabled: ``OCSController`` class and ``#[CORS]`` attribute on the method .. warning:: Adding the ``#[NoCRSFRequired]`` attribute imposes a security risk. You should not add this to your controller methods unless you understand the implications and be sure that you absolutely need the attribute. +.. warning:: + Adding the attribute ``#[CORS]`` alone is not sufficient to allow access using CORS. + The CSRF checker will typically fail, so enabling CORS enforces you to disable the CSRF checker as well. + Although the disabled CSRF checker in itself is a security issue to consider, adding CORS opens up this even more. + You should make sure, that you understand the implications completely when enabling CORS and do so only when there is a good use case. + There are different ways a clients might interact with your APIs. These ways depend on your API configuration (what you allow) and on which route the request is finally made. - *Access from web frontend* means the user is browses the Nextcloud web frontend with a browser. - *Access from an external app* indicates that the user is not using the normal browser (as logged in) but directly navigates a certain URL. This can be in a new browser tab or an external program (like an Android app or simply a curl command line). +- *Access from external website* means that the user browses some third party web site and *magically* data from your app appears. + Technically, the other website would embed/load/use images, JSON data, or other resources from a URL pointing to the Nextcloud server. .. list-table:: Comparison of different API types :header-rows: 1 @@ -115,9 +124,11 @@ These ways depend on your API configuration (what you allow) and on which route * - Description - 1 (plain) - 2 (w/o CSRF) + - 3 (CORS) - 4 (OCS) - - 4 (OCS w/o CSRF) + - 5 (OCS+CORS) * - URL prefix (relative to server) + - ``/apps//`` - ``/apps//`` - ``/apps//`` - ``/ocs/v2.php/apps//`` @@ -125,14 +136,23 @@ These ways depend on your API configuration (what you allow) and on which route * - Access from web frontend - yes - yes (CSRF risk) - - yes - yes (CSRF risk) + - yes + - yes (CSRF risk [#]_) * - Access from external app - --- - yes + - yes - yes (with header [#]_) - yes + * - Access from external website + - --- + - --- + - yes + - --- + - yes * - Encapsulated data + - no - no - no - yes (JSON or XML) @@ -149,4 +169,7 @@ As a rule of thumb one can conclude that OCS provides a good way to handle most The only exception to this is if you want to provide an API for external usage where you have to comply with an externally defined API scheme. Here, the encapsulation introduced in OCS and CSRF checks might be in your way. +.. [#] Only if you have set ``#[NoCSRFRequired]``. + OCS controllers have other CSRF checks in place that might with CORS without disabling the CSRF checks completely. + Using the ``OCS-APIREQUEST`` header is also a CSRF protection but is compatible with CORS. .. [#] The OCS controller needs the request header ``OCS-APIREQUEST`` to be set to ``true``. From 9a356e0ea67aaa0d9c9c538a5ff68384063ab320 Mon Sep 17 00:00:00 2001 From: Christian Wolf Date: Wed, 30 Oct 2024 19:34:02 +0100 Subject: [PATCH 06/13] Apply suggestions from code review Fix typos and other errors in the code as suggested by review process Co-authored-by: Kate <26026535+provokateurin@users.noreply.github.com> Signed-off-by: Christian Wolf --- developer_manual/digging_deeper/rest_apis.rst | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/developer_manual/digging_deeper/rest_apis.rst b/developer_manual/digging_deeper/rest_apis.rst index f4ff8697cae..38e785915d6 100644 --- a/developer_manual/digging_deeper/rest_apis.rst +++ b/developer_manual/digging_deeper/rest_apis.rst @@ -94,9 +94,9 @@ The following combinations of attributes might be relevant for various scenarios #. Plain frontend route: ``Controller`` class #. Plain frontend with CRSF checks disabled: ``Controller`` class and ``#[NoCSRFRequired]`` attribute on the method -#. REST route with CORS enabled: ``Controller`` class and ``#[CORS]`` and ``#[NoCSRFRequired]`` attributes on the route -#. OCS-based route: ``OCSController`` class -#. OCS-based route with CORS enabled: ``OCSController`` class and ``#[CORS]`` attribute on the method +#. Plain frontend route with CORS enabled: ``Controller`` class and ``#[CORS]`` and ``#[NoCSRFRequired]`` attributes on the route +#. OCS route: ``OCSController`` class +#. OCS route with CORS enabled: ``OCSController`` class and ``#[CORS]`` attribute on the method .. warning:: Adding the ``#[NoCRSFRequired]`` attribute imposes a security risk. @@ -114,8 +114,8 @@ These ways depend on your API configuration (what you allow) and on which route - *Access from web frontend* means the user is browses the Nextcloud web frontend with a browser. - *Access from an external app* indicates that the user is not using the normal browser (as logged in) but directly navigates a certain URL. This can be in a new browser tab or an external program (like an Android app or simply a curl command line). -- *Access from external website* means that the user browses some third party web site and *magically* data from your app appears. - Technically, the other website would embed/load/use images, JSON data, or other resources from a URL pointing to the Nextcloud server. +- *Access from external website* means that the user browses some third party web site and data from your Nextcloud server appears. + 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. .. list-table:: Comparison of different API types :header-rows: 1 @@ -170,6 +170,6 @@ The only exception to this is if you want to provide an API for external usage w Here, the encapsulation introduced in OCS and CSRF checks might be in your way. .. [#] Only if you have set ``#[NoCSRFRequired]``. - OCS controllers have other CSRF checks in place that might with CORS without disabling the CSRF checks completely. - Using the ``OCS-APIREQUEST`` header is also a CSRF protection but is compatible with CORS. -.. [#] The OCS controller needs the request header ``OCS-APIREQUEST`` to be set to ``true``. + OCS controllers have other CSRF checks in place that work with CORS without disabling the CSRF checks completely. + Using the ``OCS-APIRequest`` header is a CSRF protection which is compatible with CORS. +.. [#] The OCS controller needs the request header ``OCS-APIRequest`` to be set to ``true``. From 8f4c3c1c82479dd59bf46976bc54a48099805220 Mon Sep 17 00:00:00 2001 From: Christian Wolf Date: Mon, 3 Mar 2025 18:15:35 +0100 Subject: [PATCH 07/13] Restructure significant part of the controller documentation Signed-off-by: Christian Wolf --- developer_manual/basics/controllers.rst | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/developer_manual/basics/controllers.rst b/developer_manual/basics/controllers.rst index 3ce160caeb4..6a1155f695e 100644 --- a/developer_manual/basics/controllers.rst +++ b/developer_manual/basics/controllers.rst @@ -393,6 +393,10 @@ A :doc:`template ` can be rendered by returning a TemplateR } +Showing a template is the only exception to the rule to :ref:`not disable CSRF checks `: +The user might type the URL directly (or use a browser bookmark or similar) to navigate to a HTML template. +Therefore, usage of the ``#[NoCSRFRequired]`` attribute (see :ref:`below`) is acceptable in this context. + Public page templates ^^^^^^^^^^^^^^^^^^^^^ @@ -434,6 +438,9 @@ A ``OCP\\AppFramework\\Http\\Template\\SimpleMenuAction`` will be a link with an developers can implement their own types of menu renderings by adding a custom class implementing the ``OCP\\AppFramework\\Http\\Template\\IMenuAction`` interface. +As the public template is also some HTML template, the same argumentation as for :ref:`regular templates` regarding the CSRF checks hold true: +The usage of ``#[NoCSRFRequired]`` for public pages is considered acceptable and is actually needed to visit the page without an active account. + Data-based responses -------------------- @@ -513,6 +520,10 @@ Now your method will be reachable via ``/ocs/v2.php/apps//api/v JSON ^^^^ +.. warning:: + The usage of standard controller to access content data like JSON (no HTML) is considered legacy. + Better use :ref:`OCS ` for this type of requests. + Returning JSON is simple, just pass an array to a JSONResponse: .. code-block:: php @@ -551,6 +562,11 @@ Because returning JSON is such a common task, there's even a shorter way to do t Why does this work? Because the dispatcher sees that the controller did not return a subclass of a Response and asks the controller to turn the value into a Response. That's where responders come in. +.. deprecated:: 30 + + Usage of classical controllers for data transmission should be avoided. Use OCS instead. + + Handling errors ^^^^^^^^^^^^^^^ From fda691f3a7cdd01fb98d90eaf217d7ae63a1232c Mon Sep 17 00:00:00 2001 From: Christian Wolf Date: Mon, 20 Jan 2025 16:06:38 +0100 Subject: [PATCH 08/13] Add basic introduction to CORS in security introduction Signed-off-by: Christian Wolf --- developer_manual/prologue/security.rst | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/developer_manual/prologue/security.rst b/developer_manual/prologue/security.rst index 856a938f703..c9c1081b185 100644 --- a/developer_manual/prologue/security.rst +++ b/developer_manual/prologue/security.rst @@ -213,6 +213,8 @@ Sensitive data exposure Always store user data or configuration files in safe locations, e.g. **nextcloud/data/** and not in the webroot where they can be accessed by anyone using a web browser. +.. _csrf_introduction: + Cross site request forgery -------------------------- @@ -250,6 +252,28 @@ Always validate the URL before redirecting if the requested URL is on the same d `_ is a method impleneted by browser to access resources from different domains at the same time. +Assume, there is a website published on host A. +The URL would for example be https://A/path/to/index.html. +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. +However, to protect B and its property (the image), the browsers do not silently embed the image of B into the page of A. +Instead, B is kindly asked by the browser if embedding is allowed (the so-called `preflight `_). + +To do so, there is a first request made to the resource on B with the ``OPTIONS`` HTTP command/verb. +The server only answers with the headers as specified and adds ``Access-Control-*`` headers. +These define, what the browser is to be allowed to do. +Only if the destination server B confirms cross site resource sharing is allowed, the browser access the resource. + +Basically, accessing foreign resources is not limited to embedding images. +Using JavaScript, arbitrary XHR/Ajax requests can be directed at arbitrary other hosts. +There are some safety measurements in place (especially about cookie handling), but one has still to be careful not to leak information unwillingly. +Especially, if the destination server B allows to sent credentials using ``Access-Control-Allow-Credentials: true``, cross site scripting is very critical. +You need :ref:`CSRF protection ` in place or your users are at relatively high risk. + Getting help ------------ From 82e612e1e29a3e7429bf0f3a4846d548b4320d84 Mon Sep 17 00:00:00 2001 From: Christian Wolf Date: Mon, 20 Jan 2025 17:33:28 +0100 Subject: [PATCH 09/13] Add some paragraph on GET vs other verbs in HTTP Signed-off-by: Christian Wolf --- developer_manual/prologue/security.rst | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/developer_manual/prologue/security.rst b/developer_manual/prologue/security.rst index c9c1081b185..c68d8fa2ff3 100644 --- a/developer_manual/prologue/security.rst +++ b/developer_manual/prologue/security.rst @@ -229,7 +229,11 @@ To prevent CSRF in an app, be sure to call the following method at the top of al Date: Mon, 20 Jan 2025 17:34:45 +0100 Subject: [PATCH 10/13] Adding some text on historic routing with REST routes Signed-off-by: Christian Wolf --- developer_manual/digging_deeper/rest_apis.rst | 110 +++++++++++++----- 1 file changed, 81 insertions(+), 29 deletions(-) diff --git a/developer_manual/digging_deeper/rest_apis.rst b/developer_manual/digging_deeper/rest_apis.rst index 38e785915d6..77d0e05c2d7 100644 --- a/developer_manual/digging_deeper/rest_apis.rst +++ b/developer_manual/digging_deeper/rest_apis.rst @@ -89,75 +89,76 @@ Relation of REST and OCS There is a close relationship between REST APIs and :ref:`OCS `. Both provide a way to transmit data between the backend of the app in the Nextcloud server and some frontend. +This is explicitly not about :ref:`HTML template responses `. + +State-of-the-Art methods and comparison +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ The following combinations of attributes might be relevant for various scenarios: #. Plain frontend route: ``Controller`` class -#. Plain frontend with CRSF checks disabled: ``Controller`` class and ``#[NoCSRFRequired]`` attribute on the method -#. Plain frontend route with CORS enabled: ``Controller`` class and ``#[CORS]`` and ``#[NoCSRFRequired]`` attributes on the route #. OCS route: ``OCSController`` class #. OCS route with CORS enabled: ``OCSController`` class and ``#[CORS]`` attribute on the method .. warning:: Adding the ``#[NoCRSFRequired]`` attribute imposes a security risk. You should not add this to your controller methods unless you understand the implications and be sure that you absolutely need the attribute. + Typically, you can instead use the ``OCS-APIRequest`` header for data requests, instead. .. warning:: - Adding the attribute ``#[CORS]`` alone is not sufficient to allow access using CORS. - The CSRF checker will typically fail, so enabling CORS enforces you to disable the CSRF checker as well. - Although the disabled CSRF checker in itself is a security issue to consider, adding CORS opens up this even more. - You should make sure, that you understand the implications completely when enabling CORS and do so only when there is a good use case. + Adding the attribute ``#[CORS]`` alone is not sufficient to allow access using CORS with plain frontend routes. + Without further measures, the CSRF checker would fail. + So, enabling CORS for plain controllers is generally and highly discouraged. + + You would have to disable the CSRF checker (one more security risk) or use the ``OCP-APIRequest`` header to successfully pass the checker. + The latter requires dedicated JS code on the importing page. There are different ways a clients might interact with your APIs. These ways depend on your API configuration (what you allow) and on which route the request is finally made. - *Access from web frontend* means the user is browses the Nextcloud web frontend with a browser. -- *Access from an external app* indicates that the user is not using the normal browser (as logged in) but directly navigates a certain URL. - This can be in a new browser tab or an external program (like an Android app or simply a curl command line). +- *Access from an external app* indicates that the user is not using the normal browser (as logged in) but directly navigates a certain URL directly. + This is typically an external program (like an Android app or simply a curl command line). - *Access from external website* means that the user browses some third party web site and data from your Nextcloud server appears. 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. +.. hint:: + The discussion here is for data requests only. + If you think of controller :ref:`methods serving (HTML) templates `, disabling CSRF is considered fine. + .. list-table:: Comparison of different API types :header-rows: 1 :align: center * - Description - - 1 (plain) - - 2 (w/o CSRF) - - 3 (CORS) - - 4 (OCS) - - 5 (OCS+CORS) + - ``Controller`` class + - ``OCSController`` class + - ``OCSController`` class & ``CORS`` on method * - URL prefix (relative to server) - - ``/apps//`` - - ``/apps//`` - ``/apps//`` - ``/ocs/v2.php/apps//`` - ``/ocs/v2.php/apps//`` * - Access from web frontend - yes - - yes (CSRF risk) - - yes (CSRF risk) - yes - - yes (CSRF risk [#]_) - * - Access from external app - - --- - yes + * - Access from external app + - partial [#]_ - yes - - yes (with header [#]_) - yes * - Access from external website - --- - --- - yes - - --- - - yes * - Encapsulated data - - no - - no - no - yes (JSON or XML) - yes (JSON or XML) +.. [#] The external app has to satisfy the CSRF checks. + That is, you need to have the ``OCS-APIRequest`` HTTP request header set to ``true``. + This is only possible for NC 30 onwards, older versions do not respect the header. + Methods from ``Controller`` classes can return ``DataResponse`` objects similar to ``OCSController`` class methods. For methods of a ``Controller`` class, the data of this response is sent e.g. as JSON as you provide it. Basically, the output is very similar to what ``json_encode`` would do. @@ -169,7 +170,58 @@ As a rule of thumb one can conclude that OCS provides a good way to handle most The only exception to this is if you want to provide an API for external usage where you have to comply with an externally defined API scheme. Here, the encapsulation introduced in OCS and CSRF checks might be in your way. -.. [#] Only if you have set ``#[NoCSRFRequired]``. - OCS controllers have other CSRF checks in place that work with CORS without disabling the CSRF checks completely. - Using the ``OCS-APIRequest`` header is a CSRF protection which is compatible with CORS. -.. [#] The OCS controller needs the request header ``OCS-APIRequest`` to be set to ``true``. + +Historical options +~~~~~~~~~~~~~~~~~~ + +.. deprecated:: 30 + The information in this section are mainly for reference purposes. Do not use the approaches in new code. + +Before NC server 30 the plain ``Controller`` classes' methods did not respect the ``OCS-APIRequest`` header. +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). + +The following combinations of attributes were relevant for various scenarios: + +#. Plain frontend route: ``Controller`` class +#. Plain frontend with CRSF checks disabled: ``Controller`` class and ``#[NoCSRFRequired]`` attribute on the method +#. Plain frontend route with CORS enabled: ``Controller`` class and ``#[CORS]`` and ``#[NoCSRFRequired]`` attributes on the route +#. OCS route: ``OCSController`` class +#. OCS route with CORS enabled: ``OCSController`` class and ``#[CORS]`` attribute on the method + +.. hint:: + The two scenarios involving the ``OCSController`` have not changed and, thus, the state-of-the-art documentation as noted above still holds true. + Thus, these options are not reconsidered here again for simplicity reasons and to get the overall view more crisp. + + The warnings about not using ``NoCSRFRequired`` and ``CORS`` as mentioned in the state-of-the-art section holds true here as well. + +.. list-table:: Comparison of different API types + :header-rows: 1 + :align: center + + * - | Description + - | ``Controller`` class + - | ``Controller`` class with + | ``NoCSRFRequired`` on method + - | ``Controller`` class with + | ``NoCSRFRequired`` and ``CORS`` + | on method + * - URL prefix (relative to server) + - ``/apps//`` + - ``/apps//`` + - ``/apps//`` + * - Access from web frontend + - yes + - yes (CSRF risk) + - yes (CSRF risk) + * - Access from external app + - --- + - yes + - yes + * - Access from external website + - --- + - --- + - yes + * - Encapsulated data + - no + - no + - no From 2281f606042dd3c3e49ca8f15f1a743d52495c5c Mon Sep 17 00:00:00 2001 From: Christian Wolf Date: Mon, 20 Jan 2025 17:38:20 +0100 Subject: [PATCH 11/13] Add MDN links Signed-off-by: Christian Wolf --- developer_manual/prologue/security.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/developer_manual/prologue/security.rst b/developer_manual/prologue/security.rst index c68d8fa2ff3..0cca32e3ac2 100644 --- a/developer_manual/prologue/security.rst +++ b/developer_manual/prologue/security.rst @@ -218,7 +218,7 @@ Always store user data or configuration files in safe locations, e.g. **nextclou Cross site request forgery -------------------------- -Using `CSRF `_ one can trick a user into executing a request that they did not want to make. Thus every POST and GET request needs to be protected against it. The only places where no CSRF checks are needed are in the main template, which is rendering the application, or in externally callable interfaces. +Using `CSRF `_ (see also on `MDN `_) one can trick a user into executing a request that they did not want to make. Thus every POST and GET request needs to be protected against it. The only places where no CSRF checks are needed are in the main template, which is rendering the application, or in externally callable interfaces. .. note:: Submitting a form is also a POST/GET request! @@ -260,7 +260,7 @@ Always validate the URL before redirecting if the requested URL is on the same d CORS ---- -`Cross-origin resource sharing (CORS) `_ is a method impleneted by browser to access resources from different domains at the same time. +`Cross-origin resource sharing (CORS) `_ (see also on `MDN `_) is a method impleneted by browser to access resources from different domains at the same time. Assume, there is a website published on host A. The URL would for example be https://A/path/to/index.html. 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. From 65800e5e1e7642e0bc96800901c3851a5756c96c Mon Sep 17 00:00:00 2001 From: Christian Wolf Date: Mon, 20 Jan 2025 17:53:25 +0100 Subject: [PATCH 12/13] Fix warning in RST code Signed-off-by: Christian Wolf --- developer_manual/prologue/security.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/developer_manual/prologue/security.rst b/developer_manual/prologue/security.rst index 0cca32e3ac2..8c5ab35d617 100644 --- a/developer_manual/prologue/security.rst +++ b/developer_manual/prologue/security.rst @@ -218,7 +218,7 @@ Always store user data or configuration files in safe locations, e.g. **nextclou Cross site request forgery -------------------------- -Using `CSRF `_ (see also on `MDN `_) one can trick a user into executing a request that they did not want to make. Thus every POST and GET request needs to be protected against it. The only places where no CSRF checks are needed are in the main template, which is rendering the application, or in externally callable interfaces. +Using `CSRF `_ (see also on `MDN `__) one can trick a user into executing a request that they did not want to make. Thus every POST and GET request needs to be protected against it. The only places where no CSRF checks are needed are in the main template, which is rendering the application, or in externally callable interfaces. .. note:: Submitting a form is also a POST/GET request! @@ -260,7 +260,7 @@ Always validate the URL before redirecting if the requested URL is on the same d CORS ---- -`Cross-origin resource sharing (CORS) `_ (see also on `MDN `_) is a method impleneted by browser to access resources from different domains at the same time. +`Cross-origin resource sharing (CORS) `_ (see also on `MDN `__) is a method impleneted by browser to access resources from different domains at the same time. Assume, there is a website published on host A. The URL would for example be https://A/path/to/index.html. 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. From b0084a7a4332b75b8a37c18593e04110afc9aba9 Mon Sep 17 00:00:00 2001 From: Christian Wolf Date: Mon, 20 Jan 2025 18:38:57 +0100 Subject: [PATCH 13/13] Fix typo Signed-off-by: Christian Wolf --- developer_manual/prologue/security.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/developer_manual/prologue/security.rst b/developer_manual/prologue/security.rst index 8c5ab35d617..6840454eb56 100644 --- a/developer_manual/prologue/security.rst +++ b/developer_manual/prologue/security.rst @@ -260,7 +260,7 @@ Always validate the URL before redirecting if the requested URL is on the same d CORS ---- -`Cross-origin resource sharing (CORS) `_ (see also on `MDN `__) is a method impleneted by browser to access resources from different domains at the same time. +`Cross-origin resource sharing (CORS) `_ (see also on `MDN `__) is a method implemented by browser to access resources from different domains at the same time. Assume, there is a website published on host A. The URL would for example be https://A/path/to/index.html. 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.