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

Breaking change to HTTP POST API in v2.2.0 #1713

Closed
kingosticks opened this Issue Oct 8, 2018 · 4 comments

Comments

2 participants
@kingosticks
Member

kingosticks commented Oct 8, 2018

By requiring Content-Type: application/json to be set for all HTTP POST requests in #1668, I introduced a breaking change to users of the HTTP POST API in v2.2.0. As pointed out by @tkem here

Users that are unable to set this header would no longer be able to use the HTTP POST API. And even those that could, we should not introduce breaking changes in a feature release. I guess the correct thing to do is have the check configurable and disabled (i.e. the v2.1.0 behaviour) by default, but will a clear recommendation to enable it. At least until the next major release of Mopidy at which point a breaking change is reasonable.

@jodal

This comment has been minimized.

Member

jodal commented Oct 8, 2018

In the name of secure defaults, I'd like the current behavior to remain the default.

I'm open to adding a config value to turn the check off, so that the issue can be resolved on either the client side by adding the Content-Type: application/json header or on the server side, by turning off the check, whichever is easiest for the end user.

@jodal jodal added this to the v2.2.1 milestone Oct 8, 2018

@kingosticks

This comment has been minimized.

Member

kingosticks commented Oct 8, 2018

I'm happy with that. But it'll have to wait until tomorrow.

kingosticks added a commit to kingosticks/mopidy that referenced this issue Oct 11, 2018

http: Add config option to disable CSRF protection (Fixes: mopidy#1713)
This will remove the requirement to set a ``Content-Type: application/json``
header for JSON-RPC POST requests. It will also disable all same-origin
checks, effectively ignoring the ``allowed_origins`` config since requests
from any origin will be allowed. Lastly, all ``'Access-Control-Allow-*``
response headers will be suppressed.
@kingosticks

This comment has been minimized.

Member

kingosticks commented Oct 11, 2018

kingosticks added a commit to kingosticks/mopidy that referenced this issue Oct 11, 2018

http: Add config option to disable CSRF protection (Fixes: mopidy#1713)
This will remove the requirement to set a ``Content-Type: application/json``
header for JSON-RPC POST requests. It will also disable all same-origin
checks, effectively ignoring the ``allowed_origins`` config since requests
from any origin will be allowed. Lastly, all ``'Access-Control-Allow-*``
response headers will be suppressed.

kingosticks added a commit to kingosticks/mopidy that referenced this issue Oct 11, 2018

http: Add config option to control CSRF protection (Fixes: mopidy#1713)
Allows users to disable CSRF protection and revert to the HTTP server's
previous (less secure) behaviour. Users are advised to leave this config
value enabled if possible. However, if disabled this will:
  * Remove the requirement to set a ``Content-Type: application/json``
    header for JSON-RPC POST requests.
  * Disable all same-origin checks, effectively ignoring the ``allowed_origins``
    config since requests from any origin will be allowed.
  * Suppress all ``Access-Control-Allow-*`` response headers.

kingosticks added a commit to kingosticks/mopidy that referenced this issue Oct 15, 2018

http: Add config option to control CSRF protection (Fixes: mopidy#1713)
Allows users to disable CSRF protection and revert to the HTTP server's
previous (less secure) behaviour. Users are advised to leave this config
value enabled if possible. However, if disabled this will:
  * Remove the requirement to set a ``Content-Type: application/json``
    header for JSON-RPC POST requests.
  * Disable all same-origin checks, effectively ignoring the ``allowed_origins``
    config since requests from any origin will be allowed.
  * Suppress all ``Access-Control-Allow-*`` response headers.

kingosticks added a commit to kingosticks/mopidy that referenced this issue Oct 15, 2018

http: Add config option to control CSRF protection (Fixes: mopidy#1713)
Allows users to disable CSRF protection and revert to the HTTP server's
previous (less secure) behaviour. Users are advised to leave this config
value enabled if possible. However, if disabled this will:
  * Remove the requirement to set a ``Content-Type: application/json``
    header for JSON-RPC POST requests.
  * Disable all same-origin checks, effectively ignoring the ``allowed_origins``
    config since requests from any origin will be allowed.
  * Suppress all ``Access-Control-Allow-*`` response headers.

jodal added a commit that referenced this issue Oct 15, 2018

Merge pull request #1714 from kingosticks/feature/disable-http-csrf-c…
…onfig

http: Add config option to disable CSRF protection (Fixes: #1713)
@jodal

This comment has been minimized.

Member

jodal commented Oct 15, 2018

Fixed by #1714.

@jodal jodal closed this Oct 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment