Skip to content
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

CORS issue while fetch assets #30

Closed
igobranco opened this issue Feb 15, 2024 · 10 comments
Closed

CORS issue while fetch assets #30

igobranco opened this issue Feb 15, 2024 · 10 comments

Comments

@igobranco
Copy link
Contributor

igobranco commented Feb 15, 2024

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 tunapanda/h5p-standalone#151 but also a PR tunapanda/h5p-standalone#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
Copy link

yagouam commented Feb 20, 2024

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

@mariajgrimaldi
Copy link
Contributor

mariajgrimaldi commented Feb 21, 2024

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 added a commit to fccn/h5pxblock that referenced this issue Feb 21, 2024
The h5p-standalone added credentials mode to be 'include' that breaks when loading h5p
content from an external storage (S3).
So the current solution is to use an older version of the h5p-standalone version (v3.5.1).
fixes edly-io#30
relates to tunapanda/h5p-standalone#151
@igobranco
Copy link
Contributor Author

@ziafazal @mariajgrimaldi and @yagouam I've opened a PR #31 with my fix.

@mariajgrimaldi
Copy link
Contributor

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

@ziafazal
Copy link
Contributor

@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
Copy link
Contributor

mariajgrimaldi commented Feb 21, 2024

This issue looks like it's been happening for some time: master...open-craft:h5pxblock:farhaan/cors-issue-method

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

@ziafazal
Copy link
Contributor

@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
Copy link

@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
Copy link
Contributor

mariajgrimaldi commented Feb 22, 2024

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
Copy link
Contributor Author

@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...

igobranco added a commit to fccn/h5pxblock that referenced this issue Jul 18, 2024
Upgrade h5p-standalone to `3.7.0` version.
Release notes: https://github.com/tunapanda/h5p-standalone/releases/tag/v3.7.0
Bug Fixes:
- CORS issue while fetch assets
- library not use assetsRequestFetchOptions

tunapanda/h5p-standalone#151
edly-io#30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants