Skip to content
This repository was archived by the owner on Jan 13, 2022. It is now read-only.

Enforce HTTPS #1049

Closed
shengfa opened this issue Aug 30, 2018 · 7 comments · Fixed by #1084
Closed

Enforce HTTPS #1049

shengfa opened this issue Aug 30, 2018 · 7 comments · Fixed by #1084

Comments

@shengfa
Copy link

shengfa commented Aug 30, 2018

I'm using these code:
$helper = $this->facebook->getRedirectLoginHelper();
$accessToken = $helper->getAccessToken();

In getAccessToken function has $redirectUrl = FacebookUrlManipulator::removeParamsFromUrl($redirectUrl, ['code', 'state']);
If my Facebook App has turn on "Enforce HTTPS" mode, this $redirectUrl will be something like https://donmain.com/facebook/callback?enforce_https=1
This will cause Facebook return an error about "domain" or "Valid OAuth Redirect URIs".

Shouldn't remove it?

@gianpaolodn
Copy link

gianpaolodn commented Aug 30, 2018

Login via facebook suddenly stopped working on our website yesterday night. Had to disable "enforce https" in the setting panel as a temporary work around.

this is the error you get:
Error validating verification code. Please make sure your redirect_uri is identical to the one you used in the OAuth dialog request

@BloodElf
Copy link

Same problem, how can i fix it?

@gianpaolodn
Copy link

gianpaolodn commented Aug 30, 2018

@BloodElf if you can't disable "enforce_https" in the app setting panel, modify the code:

$redirectUrl = FacebookUrlManipulator::removeParamsFromUrl($redirectUrl, ['code', 'state']);

to

$redirectUrl = FacebookUrlManipulator::removeParamsFromUrl($redirectUrl, ['code', 'state', 'enforce_https']);

in this class: FacebookRedirectLoginHelper at line 226.

It should workaround the issue, even tho is fb that should NOT send that param at all

@BloodElf
Copy link

@gianpaolodn It works! Thank you very much!

@70nyIT
Copy link

70nyIT commented Aug 30, 2018

confirm, same issue. Solved the same way, adding 'enforce_https'

@yguedidi
Copy link
Contributor

@gianpaolodn thanks! Can you open a PR with the fix?

@yguedidi
Copy link
Contributor

@gianpaolodn in fact, there is already a PR for that: #1054

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants