-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Clarify the description of REST vs OCS in accordance to sugestions discussed #12264
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 14 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
1e63339
Started to fix unclear statements about OCS and REST
christianlupus 6a9fb9d
Fix some sphinx build watrnings
christianlupus 9b817ad
Apply suggestions from code review
christianlupus 5fd856f
Fix OCS description after some research
christianlupus 64453ed
Adding CORS back into the mix
christianlupus 5e0e926
Apply suggestions from code review
christianlupus 2fdeb0b
Restructure significant part of the controller documentation
christianlupus 86d6bae
Add basic introduction to CORS in security introduction
christianlupus 08a8920
Add some paragraph on GET vs other verbs in HTTP
christianlupus 845e062
Adding some text on historic routing with REST routes
christianlupus 146e624
Add MDN links
christianlupus bda070d
Fix warning in RST code
christianlupus 5203eaa
Fix typo
christianlupus 3071dbf
Reword external app to non-browser
christianlupus 34756e2
Apply suggestions from code review
christianlupus 044c4d7
Merge remote-tracking branch 'origin/master' into fix/clarification-r…
christianlupus 66d1f61
Fix issue in config afer merging master
christianlupus 179d774
Added content from suggestions
christianlupus File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,8 @@ REST APIs | |
|
||
.. sectionauthor:: Bernhard Posselt <[email protected]> | ||
|
||
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]`` 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>`_. | ||
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 <https://developer.mozilla.org/en-US/docs/Web/HTTP/Access_control_CORS>`_. | ||
christianlupus marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
.. 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:: | ||
provokateurin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
/index.php/apps/myapp/api/1.0/resource | ||
|
||
|
@@ -79,3 +81,147 @@ 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 <ocscontroller>`. | ||
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 <controller_html_responses>`. | ||
|
||
State-of-the-Art methods and comparison | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
The following combinations of attributes might be relevant for various scenarios: | ||
|
||
#. Plain frontend route: ``Controller`` class | ||
#. 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. | ||
christianlupus marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
.. warning:: | ||
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. | ||
christianlupus marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
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. | ||
christianlupus marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
- *Access from non-browser* is if the user accesses the resource or page not using the default browser. | ||
Thus can be e.g. a script using CURL or an Android app. | ||
christianlupus marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
- *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 <controller_html_responses>`, disabling CSRF is considered fine. | ||
|
||
.. list-table:: Comparison of different API types | ||
:header-rows: 1 | ||
:align: center | ||
|
||
* - Description | ||
- ``Controller`` class | ||
- ``OCSController`` class | ||
- ``OCSController`` class & ``CORS`` on method | ||
* - URL prefix (relative to server) | ||
- ``/apps/<appid>/`` | ||
- ``/ocs/v2.php/apps/<appid>/`` | ||
- ``/ocs/v2.php/apps/<appid>/`` | ||
* - Access from web frontend | ||
- yes | ||
- yes | ||
- yes | ||
* - Access from non-browser | ||
- partial [#]_ | ||
- yes | ||
- yes | ||
provokateurin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* - Access from external website | ||
- --- | ||
- --- | ||
christianlupus marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
- yes | ||
christianlupus marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* - Encapsulated data | ||
- 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. | ||
christianlupus marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
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. | ||
christianlupus marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
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. | ||
|
||
|
||
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. | ||
christianlupus marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
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/<appid>/`` | ||
- ``/apps/<appid>/`` | ||
- ``/apps/<appid>/`` | ||
* - Access from web frontend | ||
- yes | ||
- yes (CSRF risk) | ||
- yes (CSRF risk) | ||
* - Access from non-browser | ||
- --- | ||
- yes | ||
- yes | ||
* - Access from external website | ||
- --- | ||
- --- | ||
christianlupus marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
- yes | ||
* - Encapsulated data | ||
- no | ||
- no | ||
- no |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.