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

feat: ignore cors during connectivity check #833

Merged
merged 1 commit into from
Sep 9, 2024

Conversation

matthewhilton
Copy link
Contributor

@matthewhilton matthewhilton commented Sep 9, 2024

The issue
Regarding the connectivity check, its purpose is to detect if a url is accessible and if it is, redirect to the idp login page.

I found through some testing that the CORS policy enforcement was likely causing lots of false negatives i.e. saying it couldn't connect when the user actually could.

The fix
We can use fetch() with no-cors mode, this returns an opaque response i.e. no http code, body, etc.. it only returns yes or no did it connect which is exactly what we need.

See https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch#making_cross-origin_requests for info re no-cors

Testing
Manual testing - accessible site resolves the fetch promise, inaccessible site returns error where previously a accessible site would return an error because of cors

@Peterburnett
Copy link
Contributor

Approved once CI is green

@Peterburnett Peterburnett merged commit 3aa5842 into MOODLE_39_STABLE Sep 9, 2024
17 of 23 checks passed
@Peterburnett Peterburnett deleted the fix-connectivity-check branch September 9, 2024 04:37
@Peterburnett
Copy link
Contributor

#816 (comment)

Matt has noted here that the CI issue is a long term issue, and I've confirmed PHPUnit is clean. This is as good as it can be without tackling the behat fails directly. Approving and merging.

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 this pull request may close these issues.

2 participants