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

Login with github as backend fails if base url contains additional path segment #3622

Open
agairing opened this issue Apr 17, 2020 · 5 comments
Open

Comments

@agairing
Copy link

@agairing agairing commented Apr 17, 2020

Describe the bug
If you use github as backend the following basic setup is required:

Example:

The authentication and token creation with github is successfull but the communication between the auth popup and the original page fails.

To Reproduce

  • setup github auth backend with additional path segment
  • adjust base_url in config
  • try to login

Expected behavior
Login should work

Applicable Versions:

  • Netlify CMS version: app - 2.12.9, core: 2.24.4, cms: 2.10.45
  • Git provider: Github
  • OS: windows 10
  • Browser version Chrome 80

Additional context
It seems that the following check in netlify-auth.js is not working:
e.origin !== this.base_url
I think origin is always just the host without any path segment.

@erezrokah erezrokah added this to Triage in Priority Issues via automation Apr 19, 2020
@erezrokah erezrokah moved this from Triage to Ready For Development in Priority Issues Apr 19, 2020
@erezrokah
Copy link
Collaborator

@erezrokah erezrokah commented Apr 19, 2020

Thanks @agairing, looks like origin contains the schema and port too:
https://developer.mozilla.org/en-US/docs/Web/API/Window/postMessage#The_dispatched_event
We would gladly accept a PR for it (probably we just need to parse base_url and reconstruct without the path).

@agairing
Copy link
Author

@agairing agairing commented Apr 19, 2020

Ok, and I can try to create a PR. What about sth. like
this.base_url.indexOf(e.origin) !== 0
?

@erezrokah
Copy link
Collaborator

@erezrokah erezrokah commented Apr 20, 2020

How about e.origin !== new URL(this.base_url).origin ? It makes it clear what we're checking and is stricter.

@agairing
Copy link
Author

@agairing agairing commented Apr 24, 2020

Fine for me.
I just did the local setup and executed the tests before changing anything. 34 tests are failing on windows.
Example:
Expected: ObjectContaining {"path": "static/media/abc_def_eaco_.png"} Received: {"field": undefined, "fileObj": {}, "path": "static\\media\\abc_def_eaco_.png", "url": "displayURL"}

How should this be handled? At first glance, it seems that the test is not cross platform compatible.

@erezrokah
Copy link
Collaborator

@erezrokah erezrokah commented Apr 24, 2020

@agairing, thanks for letting us know. I opened an new issue for that #3655 and I'm surprised our CI doesn't fail it since it we run the tests on Widows too: https://github.com/netlify/netlify-cms/runs/609296943?check_suite_focus=true#step:6:1496.
I think you could just test the fix locally and submit it and I'll take care of the tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Priority Issues
Ready For Development
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.