edx-platform icon indicating copy to clipboard operation
edx-platform copied to clipboard

[DEPR]: cors_csrf middleware and utilities

Open timmc-edx opened this issue 1 year ago • 2 comments

Proposal Date

2023-10-31

Target Ticket Acceptance Date

2023-11-15

Earliest Open edX Named Release Without This Functionality

Redwood - 2024-04

Rationale

cors_csrf (the code under openedx/core/djangoapps/cors_csrf/ in edx-platform) is intended as an extension of the Django CSRF mechanism that distributes the CSRF cookie to all subdomains rather than just the site's exact domain, allowing for trusted cross-origin calls.

Problems:

  • CSRF was always intended as a same-site protection mechanism, and the standard way to permit trusted cross-site requests is CORS, which we use elsewhere in edxapp. Using this custom extension of the CSRF middleware makes it harder to perform security analysis on edxapp and the related IDAs.
  • This mechanism of distributing the CSRF cookie to all subdomains (e.g. .edx.org) is overly broad, and would allow unintended subdomains to make authorized calls to edxapp.
  • The middleware and other code in the cors_csrf Django app relies on undocumented internals of Django's csrf middleware, which recently lead to difficulties in upgrading to Django 4.x.

Removal

Everything under openedx/core/djangoapps/cors_csrf/ would be removed, as well as any calls to those utilities.

Replacement

Calls to cors_csrf, including decorators, should be replaced with code that uses existing CORS utilities.

Deprecation

The cors_csrf utilities could emit DeprecationWarnings pending full removal. If needed, it could be left in this state for one full release (Redwood), with removal occurring in the next release after that.

Migration

No response

Additional Info

No response


Discourse post: https://discuss.openedx.org/t/deprecation-removal-cors-csrf-middleware-and-utilities-edx-platform-33627/11577

timmc-edx avatar Oct 31 '23 14:10 timmc-edx

@timmc-edx Are you or someone else in the security working group planning on pushing this forward? We are trying to assign owners to active DEPR tickets.

dianakhuang avatar Apr 04 '24 15:04 dianakhuang

No, unfortunately not at this time.

timmc-edx avatar Apr 04 '24 15:04 timmc-edx