From beb4db733cdd2c8dc082f61f6861a940041d8927 Mon Sep 17 00:00:00 2001
From: Patrick Radtke <patrick@cirrusidentity.com>
Date: Thu, 18 Jan 2018 12:04:13 -0800
Subject: [PATCH] authfacebook compatability with Facebook strict URI match
Per https://developers.facebook.com/docs/facebook-login/security/#strict_mode the
`state` parameter should be used for any state needed after the redirect, and the
redirect URI should remain constant.
---
modules/authfacebook/extlibinc/base_facebook.php | 2 +-
modules/authfacebook/lib/Auth/Source/Facebook.php | 2 +-
modules/authfacebook/lib/Facebook.php | 7 +++++++
modules/authfacebook/www/linkback.php | 12 ++++++++----
4 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/modules/authfacebook/extlibinc/base_facebook.php b/modules/authfacebook/extlibinc/base_facebook.php
index a5fd3e904..cd4536db1 100644
--- a/modules/authfacebook/extlibinc/base_facebook.php
+++ b/modules/authfacebook/extlibinc/base_facebook.php
@@ -698,7 +698,7 @@ abstract class BaseFacebook
$this->clearPersistentData('state');
return $_REQUEST['code'];
} else {
- self::errorLog('CSRF state token does not match one provided.');
+ self::errorLog('CSRF state token does not match one provided. ' . $this->state . '!=' . $_REQUEST['state']);
return false;
}
}
diff --git a/modules/authfacebook/lib/Auth/Source/Facebook.php b/modules/authfacebook/lib/Auth/Source/Facebook.php
index 1a85e2bcd..990659946 100644
--- a/modules/authfacebook/lib/Auth/Source/Facebook.php
+++ b/modules/authfacebook/lib/Auth/Source/Facebook.php
@@ -91,7 +91,7 @@ class sspmod_authfacebook_Auth_Source_Facebook extends SimpleSAML_Auth_Source {
$facebook = new sspmod_authfacebook_Facebook(array('appId' => $this->api_key, 'secret' => $this->secret), $state);
$facebook->destroySession();
- $linkback = SimpleSAML\Module::getModuleURL('authfacebook/linkback.php', array('AuthState' => $stateID));
+ $linkback = SimpleSAML\Module::getModuleURL('authfacebook/linkback.php');
$url = $facebook->getLoginUrl(array('redirect_uri' => $linkback, 'scope' => $this->req_perms));
SimpleSAML_Auth_State::saveState($state, self::STAGE_INIT);
diff --git a/modules/authfacebook/lib/Facebook.php b/modules/authfacebook/lib/Facebook.php
index 530853fbf..42153933c 100644
--- a/modules/authfacebook/lib/Facebook.php
+++ b/modules/authfacebook/lib/Facebook.php
@@ -146,4 +146,11 @@ class sspmod_authfacebook_Facebook extends BaseFacebook
}
return implode('_', $parts);
}
+
+ protected function establishCSRFTokenState() {
+ if ($this->state === null) {
+ $this->state = SimpleSAML_Auth_State::getStateId($this->ssp_state);
+ $this->setPersistentData('state', $this->state);
+ }
+ }
}
diff --git a/modules/authfacebook/www/linkback.php b/modules/authfacebook/www/linkback.php
index 6ca8855bc..94adb1672 100644
--- a/modules/authfacebook/www/linkback.php
+++ b/modules/authfacebook/www/linkback.php
@@ -3,11 +3,15 @@
/**
* Handle linkback() response from Facebook.
*/
-
-if (!array_key_exists('AuthState', $_REQUEST) || empty($_REQUEST['AuthState'])) {
- throw new SimpleSAML_Error_BadRequest('Missing state parameter on facebook linkback endpoint.');
+
+// For backwards compatability look for AuthState first
+if (array_key_exists('AuthState', $_REQUEST) && !empty($_REQUEST['AuthState'])) {
+ $state = SimpleSAML_Auth_State::loadState($_REQUEST['AuthState'], sspmod_authfacebook_Auth_Source_Facebook::STAGE_INIT);
+} elseif (array_key_exists('state', $_REQUEST) && !empty($_REQUEST['state'])) {
+ $state = SimpleSAML_Auth_State::loadState($_REQUEST['state'], sspmod_authfacebook_Auth_Source_Facebook::STAGE_INIT);
+} else {
+ throw new SimpleSAML_Error_BadRequest('Missing state parameter on facebook linkback endpoint.');
}
-$state = SimpleSAML_Auth_State::loadState($_REQUEST['AuthState'], sspmod_authfacebook_Auth_Source_Facebook::STAGE_INIT);
// Find authentication source
if (!array_key_exists(sspmod_authfacebook_Auth_Source_Facebook::AUTHID, $state)) {
--
GitLab