edly-io / h5pxblock

Xblock which provides ability to play H5P content in open edX
MIT License
13 stars 23 forks source link

CORS issue while fetch assets #30

Closed igobranco closed 6 months ago

igobranco commented 7 months ago

The error reported by the console is CORS policy: The value of the 'Access-Control-Allow-Origin' header in the response must not be the wildcard '*' when the request's credentials mode is 'include'.

The h5p-standalone version 3.6.0 introduced a new feature accept override of assets fetch function. On the code is the assetsRequestFetchOptions option. This feature has included the credentials property while fetching assets (using the Fetch API) with the include value instead of the default same-origin. This breaking change, I think, makes the loading of H5P content from a different domain not possible, like when using a S3 Bucket.

I've opened an issue to the h5p-standalone project https://github.com/tunapanda/h5p-standalone/issues/151 but also a PR https://github.com/tunapanda/h5p-standalone/issues/152 Meanwhile and temporary we at nau.edu.pt are using a forked branch with an applied hot fix that uses the older h5p-standalone v3.5.1 that doesn't have this issue.

yagouam commented 6 months ago

I'm experimenting the same issue when injecting this component into an OpenEdx instance. Hope it will be fixed soon.

mariajgrimaldi commented 6 months ago

Hello folks, we've been using the 3.6.0 for quite some time and it was working as expected. However, one of my teammates found this issue in their remote environments.

@ziafazal: this issue might be something affecting some of the H5P xblock adopters. Can we do something in the meantime? Like the hotfix @igobranco proposed.

igobranco commented 6 months ago

@ziafazal @mariajgrimaldi and @yagouam I've opened a PR https://github.com/edly-io/h5pxblock/pull/31 with my fix.

mariajgrimaldi commented 6 months ago

I'll deploy the fix in our environment to test it out. Thanks @igobranco!

ziafazal commented 6 months ago

@igobranco @mariajgrimaldi thanks for escalating this and relevant PR. BTW you upstream PR got approved too. Wondering if we can get a new version of h5p-standalone package that would be great.

mariajgrimaldi commented 6 months ago

This issue looks like it's been happening for some time: https://github.com/edly-io/h5pxblock/compare/master...open-craft:h5pxblock:farhaan/cors-issue-method

I'll include @farhaanbukhsh in this thread since he's the author of those commits.

ziafazal commented 6 months ago

@igobranco @mariajgrimaldi I have tested using H5P xblock's latest version with S3 storage and it works fine. I think issue stem from your S3 bucket's Cross-origin resource sharing (CORS) configuration. If you have set AllowedOrigins to * then you'll see CORS policy: The value of the 'Access-Control-Allow-Origin' header in the response must not be the wildcard '*' when the request's credentials mode is 'include'. You need to explicitly mention the origins you want to whitelist.

My S3 bucket's CORS

[
    {
        "AllowedHeaders": [
            "*"
        ],
        "AllowedMethods": [
            "HEAD",
            "GET",
            "PUT",
            "POST"
        ],
        "AllowedOrigins": [
            "http://apps.local.edly.io:2000",
            "http://local.edly.io:8000",
            "http://studio.local.edly.io:8001"
        ],
        "ExposeHeaders": []
    }
]
farhaanbukhsh commented 6 months ago

@ziafazal You are correct, and the reason I forked and made those changes is that we are using DO spaces and they have a bug while setting 'Access-Control-Allow-Origin', with S3 it should work fine.

mariajgrimaldi commented 6 months ago

I finally tested H5P again in an installation with MinIO, and it worked; that's the installation I mentioned here that's been working fine with 3.6.0. I'll share this solution with my teammates who have access to installations with S3 and other providers. Thank you folks.

igobranco commented 6 months ago

@ziafazal you are right.

In the past I've included configured the AllowedOrigins with * because we have a multi site instance, and I didn't want to manage all the LMS domains also inside the S3 Bucket CORS configuration. I've using a Ceph S3 and not AWS S3, and probably there is a similar behavior difference, like with the DO spaces referred by @farhaanbukhsh.

I've installed again the h5pxblock with the v0.2.9 with a similar CORS configuration of @ziafazal:

<CORSConfiguration xmlns="http://s3.amazonaws.com/doc/2006-03-01/">
    <CORSRule>
        <AllowedOrigin>https://lms.dev.nau.fccn.pt</AllowedOrigin>
        <AllowedOrigin>https://partner-site.dev.nau.fccn.pt</AllowedOrigin>
        <AllowedOrigin>https://studio.dev.nau.fccn.pt</AllowedOrigin>
        <AllowedMethod>GET</AllowedMethod>
        <AllowedMethod>POST</AllowedMethod>
        <AllowedMethod>DELETE</AllowedMethod>
        <AllowedMethod>PUT</AllowedMethod>
        <AllowedHeader>*</AllowedHeader>
    </CORSRule>
</CORSConfiguration>

Then I view this error:

Access to fetch at 'https://rgw.nau.fccn.pt/nau-development-edxapp-uploads/h5pxblockmedia/FCT/FCT-BARRAGENS-101/XXXXXXXXXXXXXXXX/h5p.json' from origin 'https://lms.dev.nau.fccn.pt' has been blocked by CORS policy: The value of the 'Access-Control-Allow-Credentials' header in the response is '' which must be 'true' when the request's credentials mode is 'include'.

I think this is an issue related to the difference implementation of AWS S3 and Ceph S3.

I understand that this xblock doesn't need a fix for a specific environment. For now I'm going to use my fork that uses the older h5p-standalone v3.5.1. And I hope that this issue will be fixed with the next h5p-standalone version. My PRs already have been approved...