[SE-3111] Retrieve handler url from backend instead of frontend for audio xblock

[toxinu]

This pull request is about updating the audio xblock to use labxchange handler URL instead of directly using the edx since it is not possible because of security (CORS) issues.

Current problem:

The new URL that should be used:

Jira tickets

• SE-3111

Testing instructions

• Update your devstack to use this branch (toxinu/audio-block-follow-up)
• Create an audio asset in labxchange
• Add a transcript on it
• Check that the url called to retrieve the transcripts is labxchange api (which will request edx api) so that we don't get any CORS error (only visible on stage and production but not locally, that's why I didn't spot it before)

Reviewer

• @s0b0lev

[s0b0lev]

@toxinu :+1: looks good

• [x] I have read through the code
• [x] Test it following instruction

[bradenmacdonald]

@toxinu This shouldn't be necessary. XBlock JavaScript runtime should always be able to call handlers using the normal runtime API. It's an important part of how I designed the XBlock sandboxing here. XBlocks can't rely on LabXchange to forward requests, since they aren't always running in LabXchange, for example.

The error shown is that "No 'Access-Control-Allow-Origin' header is present on the requested resource" but you can see here that the XBlock handler endpoint does include that header: https://github.com/edx/edx-platform/blob/77d63c57f0d85f77b84863416748b49b4cbfd79e/openedx/core/djangoapps/xblock/rest_api/views.py#L135-L137

I'd like to understand what the root cause is here before putting in place a workaround/hack. If we deploy this, you may just find another error instead:

One way that CORS error can happen is if the xblock handler raises an exception, because then Django will return a normal response and won't get to the part of the view where it sets the Access-Control-Allow-Origin header. You need to inspect the network traffic or access the handler endpoint directly to see what the actual error is.

If you need help, give me a link to an asset on LabXchange showing this error.

[toxinu]

@toxinu This shouldn't be necessary. XBlock JavaScript runtime should always be able to call handlers using the normal runtime API. It's an important part of how I designed the XBlock sandboxing here. XBlocks can't rely on LabXchange to forward requests, since they aren't always running in LabXchange, for example.

The error shown is that "No 'Access-Control-Allow-Origin' header is present on the requested resource" but you can see here that the XBlock handler endpoint does include that header: https://github.com/edx/edx-platform/blob/77d63c57f0d85f77b84863416748b49b4cbfd79e/openedx/core/djangoapps/xblock/rest_api/views.py#L135-L137

I'd like to understand what the root cause is here before putting in place a workaround/hack. If we deploy this, you may just find another error instead:

One way that CORS error can happen is if the xblock handler raises an exception, because then Django will return a normal response and won't get to the part of the view where it sets the Access-Control-Allow-Origin header. You need to inspect the network traffic or access the handler endpoint directly to see what the actual error is.

If you need help, give me a link to an asset on LabXchange showing this error.

Yep I understand but since I am not able to test or debug anything and this is blocked since too long time I decided to go with a workaround that will probably work.

You can test it on https://labxchange.org, create an audio asset, add a transcript, view the audio asset.

Pre-flight OPTIONS

Post request

Console error

[bradenmacdonald]

@toxinu OK, I used this handy extension to see what happens without CORS, and it turns out that it does work. However, this is still a workaround, and I don't like merging workarounds because someone else is just going to run into the same problem on another XBlock in the future. We should always try to fix the root cause instead of just hacking around it. Also, in this case, calling the LX backend just slows down the request; going directly to the LMS will be faster.

The real problem here is twofold:

1. The XBlock handler sets Access-Control-Allow-Origin: *, but when we do a POST request, the browser first does an OPTIONS preflight request, and that doesn't get handled by the view; it gets intercepted by the CORS middleware which just returns an empty response with no CORS headers. The solution (to fix OPTIONS+POST requests) is to use a CORS signal handler to whitelist the xblock_handler API endpoint from CORS:

from corsheaders.signals import check_request_enabled

def cors_allow_xblock_handler(sender, request, **kwargs):
    """
    Sandboxed XBlocks need to be able to call XBlock handlers via POST,
    from a different domain. See views.py for details and how security is
    enforced.
    Per the corsheaders docs, a signal is the only way to achieve this for
    just a specific view/URL.
    """
    return request.path.startswith('/api/xblock/v2/xblocks/') and '/handler/ in request.path

check_request_enabled.connect(cors_allow_xblock_handler)

If we put that code into openedx/core/djangoapps/xblock/rest_api somewhere and make sure it gets run on startup, it should solve this problem (for LabXchange and Content Libraries, and for all XBlocks, not just Audio).

2. The Audio XBlock is using a POST request (with the lang in body data), when it should be using a GET request (with the lang in the query string. We want this data to be cached; it's not unique user-generated data, so it should be a GET request.

[bradenmacdonald]

CC @arbrandes in case you run into issues with XBlock POST handlers not working while testing things in content libraries ^

[arbrandes]

@bradenmacdonald, this is awesome. I've run into this very issue a few times in the past, but could never figure out WTH was going on. Makes total sense. Thanks!

[toxinu]

Thanks for the explanation @bradenmacdonald, that makes sense. 👍🏻

[bradenmacdonald]

@toxinu Will you be able to submit a follow-up fix to edx-platform? I want to make sure we get the root cause fixed now, sounds like it has been affecting several people and multiple projects :/ I can do the core committer review and get it merged.

We should also revert this change because it will actually make the request slightly slower. I'd recommend changing it from POST to GET at the same time.

[toxinu]

@bradenmacdonald Yes, I am planning to do it during next sprint. Thanks for your suggestion, it helps a lot. 🙇🏻