Workaround for Windows Live authentication #658

Merged
merged 6 commits into from Oct 29, 2016

Projects

None yet

3 participants

@mourjan
Contributor
mourjan commented Jul 1, 2016

A while ago, Microsoft enforced their oauth security by disallowing redirect url to contain any parameters.
Hence the following redirect url does not work:
http://mywebsite.com/path_to_hybridauth/?hauth.done=Live

A workaound created by @tohweesiang was to provide another redirect url format in the case of Live authentication, so the redirect url will become like:
http://mywebsite.com/path_to_hybridauth/live.php

If this pull request is accepted than Hybridauth's documentation for Windows Live should be changed as well,

regards

mourjan added some commits Jul 1, 2016
@mourjan mourjan Create live.php
Windows Live authentication does not allow redirect URLs to contain any parameters, 
Therefore live.php is an alternative to index.php which initializes $_REQUEST['hauth_done'] = 'Live'; before proceeding with authentication.
As a result The redirect URL to be used in Windows Live authentication settings will become:
http://mywebsite.com/path_to_hybridauth/live.php
instead of:
http://mywebsite.com/path_to_hybridauth/?hauth.done=Live
751a5f5
@mourjan mourjan Special case handling of Live authentication
Applied a workaround to solve the fact that Windows Live does not allow parameters in redirect URLs,
causing failure in this case:
http://mywebsite.com/path_to_hybridauth/?hauth.done=Live
So if the case is Live authentication, then redirect url should be
http://mywebsite.com/path_to_hybridauth/live.php
0719c16
@mourjan mourjan Update composer.json 67d1f24
@mourjan mourjan Fix for cases where $mode value is an empty string in redirect function e9a99e6
@StorytellerCZ StorytellerCZ commented on the diff Jul 8, 2016
composer.json
@@ -1,5 +1,5 @@
{
- "name": "hybridauth/hybridauth",
+ "name": "mourjan/hybridauth",
@StorytellerCZ
StorytellerCZ Jul 8, 2016 Contributor

Please fix this back.

@mourjan
mourjan Jul 8, 2016 Contributor

Done..

am I supposed to submit another Pull Request?

Bassel

On Fri, Jul 8, 2016 at 4:37 AM, Jan Dvorak notifications@github.com wrote:

In composer.json
#658 (comment):

@@ -1,5 +1,5 @@
{

  • "name": "hybridauth/hybridauth",
  • "name": "mourjan/hybridauth",

Please fix this back.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/hybridauth/hybridauth/pull/658/files/e9a99e685ec1b7bc8f3a93ef82172cadaf7d0f57#r70015151,
or mute the thread
https://github.com/notifications/unsubscribe/AFzgxk6J2tf0xy3MVCi-RitIknLKP2VHks5qTanpgaJpZM4JDFP3
.

@StorytellerCZ
StorytellerCZ Sep 5, 2016 edited Contributor

Just pushing to this branch will do.

@StorytellerCZ StorytellerCZ added this to the 2.x milestone Jul 8, 2016
@christ0ph3r christ0ph3r commented on the diff Jul 9, 2016
hybridauth/Hybrid/Auth.php
@@ -352,6 +352,9 @@ public static function logoutAllProviders() {
* @param string $mode PHP|JS
*/
public static function redirect($url, $mode = "PHP") {
+ if(!$mode){
+ $mode = 'PHP';
@christ0ph3r
christ0ph3r Jul 9, 2016 Collaborator

I am not sure if this is needed. Can you explain why it is?

<?php 

function redirect($url, $mode = "PHP") {
  echo $mode;
}

redirect('https://test.com/');
?>

will output PHP because the second parameter is already set

$mode = "PHP"

so I do not understand why check it again.

@christ0ph3r
christ0ph3r Jul 9, 2016 Collaborator

Nevermind. I understand now. So incase its empty this is fallback.

@mourjan
mourjan Jul 10, 2016 Contributor

Exactly, in some cases $mode is passed as an empty string, and in this case $mode will not be initialized by 'PHP' value since it is not null

@StorytellerCZ StorytellerCZ merged commit 02ae0cd into hybridauth:master Oct 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment