[Quince testing] <add course mode from django admin>: <Getting 403 error while adding course mode from django admin> #325
Comments
|
It should be noted that the "csrftoken" appears to be correctly set (value =
The "csrfmiddlewaretoken" request parameter is also included in the POST request, but it has a different value from the cookie (
Maybe this discrepancy is causing the 403 response? In any case, the solution should NOT be to ignore CSRF parameters for the LMS host (via |
|
Not familiar with the way the tutor is set up but this may help. |
|
@iamsobanjaved the request is not coming from a subdomain. The request is coming from the same host as the LMS. |
|
Can we set |
|
Sure. There you go:
|
|
I believe this is failing with any operation made from the Django Admin, here is an example of when I tried to edit a user: |
|
label: quince testing |
mariajgrimaldi
added
release blocker
|
For the record, I am unable to reproduce the issue in development mode. ( EDIT: I am also unable to reproduce the issue locally, with local.overhang.io as the LMS_HOST. I suspect the error may be due to HTTPS. EDIT2: I can confirm that the issue is fixed in HTTP mode ( EDIT3: here is the error log from the lms container: |
|
I tried a different POST from the LMS and it's also failing: But POST requests from Studio are OK: |
|
I'll write here my investigations. In Turns out that the The call to What is strange to me is that this middleware has not changed for many years... And it's been enabled in the LMS for a long time. So maybe this is a false lead? I need to log out. If someone wants to pick up from there, be my guest 🤗 |
|
Yeah, that cors_csrf middleware is kind of scary. I filed a DEPR for it a couple of days ago (https://discuss.openedx.org/t/deprecation-removal-cors-csrf-middleware-and-utilities-edx-platform-33627/11577) after I saw that code during review of openedx/edx-platform#33226. |
|
There are two candidate solutions. It's important to realize that both are temporary, so let's not close this issue.
I implemented solution 2 so that testing can resume. But we must realise that the issue is caused by the upgrade to Django 4.2 in the Quince branch. This backport DOES NOT EXIST in master. So we are in a strange situation where Quince runs Django 4.2, but master runs Django 3. This is highly unusual. |
|
In Django 4. x, a new check for the CSRF middleware was introduced: |
|
It looks like in edx.org already added the host to |
I think you're right. But I'm not sure that disabling CSRF for the LMS is the right choice. |
I believe it's the order of the middleware that's causing this. Note that in the CMS django's csrf middleware runs before In particular, in the CMS, once |
|
👏 👏 👏 @zawan-ila 👏 👏 👏 Nailed it! Following your suggestion, I added to the LMS production settings: Lo and behold, this resolves the issue. Thus, the right fix would be to move the Django middleware up in the LMS common settings. @zawan-ila would you like to open a PR or should I do it? Once the fix is in master we should also backport it to Quince. Thank you so much for researching this @zawan-ila, we were truly out of our depth here. |
|
Bravo!
|
|
@regisb Feel free to open any pull requests as you see appropriate. I will not be making one :-) |
|
Who wants to review this? openedx/edx-platform#33727 |
|
@regisb: I'll do it. |
POST requests to the LMS are failing systematically when HTTPS is enabled. This issue is observed in the Quince release branch. Here is the root cause analysis: - CorsCSRFMiddleware overrides the `is_secure` attribute by setting it to "false". - CorsCSRFMiddleware calls the parent `process_view` method, from the CsrfViewMiddleware. - CsrfViewMiddleware checks the Origin header, including the scheme. It is equal to "https://LMSHOST". But because the request is not considered secure, the expected origin is "http://LMSHOST". - The check fails with "Origin checking failed" We resolve this issue by running the CsrfViewMiddleware *before* the custom CorsCSRFMiddleware. After a successful check of the CsrfViewMiddleware, the request has the "csrf_processing_done = True" attribute, and CorsCSRFMiddleware is short-circuited. This issue did not happen in the following environments: - in Palm because the CsrfViewMiddleware did not check the "Origin" header in Django 3. - in the Studio, because the Studio already runs the CsrfViewMiddleware before the CorsCSRFMiddleware. - in the master branch because the master branch does not yet run on Django 4. But the issue will happen in the master branch without this proposed change. To bypass this issue in the master branch, it was proposed that we add "https://LMSHOST" to CSRF_TRUSTED_ORIGINS. This would effectily bypass CSRF checking entirely for all requests that originate from the LMS. Such a solution would not be acceptable, as we would lose the security guarantees offered by CSRF. See discussion: openedx/wg-build-test-release#325
POST requests to the LMS are failing systematically when HTTPS is enabled. This issue is observed in the Quince release branch. Here is the root cause analysis: - CorsCSRFMiddleware overrides the `is_secure` attribute by setting it to "false". - CorsCSRFMiddleware calls the parent `process_view` method, from the CsrfViewMiddleware. - CsrfViewMiddleware checks the Origin header, including the scheme. It is equal to "https://LMSHOST". But because the request is not considered secure, the expected origin is "http://LMSHOST". - The check fails with "Origin checking failed" We resolve this issue by running the CsrfViewMiddleware *before* the custom CorsCSRFMiddleware. After a successful check of the CsrfViewMiddleware, the request has the "csrf_processing_done = True" attribute, and CorsCSRFMiddleware is short-circuited. This issue did not happen in the following environments: - in Palm because the CsrfViewMiddleware did not check the "Origin" header in Django 3. - in the Studio, because the Studio already runs the CsrfViewMiddleware before the CorsCSRFMiddleware. - in the master branch because the master branch does not yet run on Django 4. But the issue will happen in the master branch without this proposed change. To bypass this issue in the master branch, it was proposed that we add "https://LMSHOST" to CSRF_TRUSTED_ORIGINS. This would effectily bypass CSRF checking entirely for all requests that originate from the LMS. Such a solution would not be acceptable, as we would lose the security guarantees offered by CSRF. See discussion: openedx/wg-build-test-release#325
POST requests to the LMS are failing systematically when HTTPS is enabled. This issue is observed in the Quince release branch. Here is the root cause analysis: - CorsCSRFMiddleware overrides the `is_secure` attribute by setting it to "false". - CorsCSRFMiddleware calls the parent `process_view` method, from the CsrfViewMiddleware. - CsrfViewMiddleware checks the Origin header, including the scheme. It is equal to "https://LMSHOST". But because the request is not considered secure, the expected origin is "http://LMSHOST". - The check fails with "Origin checking failed" We resolve this issue by running the CsrfViewMiddleware *before* the custom CorsCSRFMiddleware. After a successful check of the CsrfViewMiddleware, the request has the "csrf_processing_done = True" attribute, and CorsCSRFMiddleware is short-circuited. This issue did not happen in the following environments: - in Palm because the CsrfViewMiddleware did not check the "Origin" header in Django 3. - in the Studio, because the Studio already runs the CsrfViewMiddleware before the CorsCSRFMiddleware. - in the master branch because the master branch does not yet run on Django 4. But the issue will happen in the master branch without this proposed change. To bypass this issue in the master branch, it was proposed that we add "https://LMSHOST" to CSRF_TRUSTED_ORIGINS. This would effectily bypass CSRF checking entirely for all requests that originate from the LMS. Such a solution would not be acceptable, as we would lose the security guarantees offered by CSRF. See discussion: openedx/wg-build-test-release#325
POST requests to the LMS are failing systematically when HTTPS is enabled. This issue is observed in the Quince release branch. Here is the root cause analysis: - CorsCSRFMiddleware overrides the `is_secure` attribute by setting it to "false". - CorsCSRFMiddleware calls the parent `process_view` method, from the CsrfViewMiddleware. - CsrfViewMiddleware checks the Origin header, including the scheme. It is equal to "https://LMSHOST". But because the request is not considered secure, the expected origin is "http://LMSHOST". - The check fails with "Origin checking failed" We resolve this issue by running the CsrfViewMiddleware *before* the custom CorsCSRFMiddleware. After a successful check of the CsrfViewMiddleware, the request has the "csrf_processing_done = True" attribute, and CorsCSRFMiddleware is short-circuited. This issue did not happen in the following environments: - in Palm because the CsrfViewMiddleware did not check the "Origin" header in Django 3. - in the Studio, because the Studio already runs the CsrfViewMiddleware before the CorsCSRFMiddleware. - in the master branch because the master branch does not yet run on Django 4. But the issue will happen in the master branch without this proposed change. To bypass this issue in the master branch, it was proposed that we add "https://LMSHOST" to CSRF_TRUSTED_ORIGINS. This would effectily bypass CSRF checking entirely for all requests that originate from the LMS. Such a solution would not be acceptable, as we would lose the security guarantees offered by CSRF. See discussion: openedx/wg-build-test-release#325
|
This was fixed by the PR linked above. |
github-project-automation
bot
moved this from Backlog
to Done
in Build-Test-Release Working Group
|
@mariajgrimaldi Verified. Thanks. |
Release
quince
Expected behavior
The user should be able to add course mode without any issues.
Actual behavior
I get 403 when I try to save course mode.
Steps to reproduce
Additional information
The text was updated successfully, but these errors were encountered: