Skip to content

Commit

Permalink
Upgraded to version 3.0.1.
Browse files Browse the repository at this point in the history 
Unit tests can be run from the command line using:

  phpunit --colors --coverage-html coverage --verbose --stderr --bootstrap tests/bootstrap.php tests/tests.php

Changes:
+ Added a new bootstrap file (as bootstrap.php) that helps the unit tests run more smoothly.
+ Allow for the possibility that session_start has already been called prior to construction of a Facebook class.
+ Updated the app-secret unit test to confirm that Desktop applications require a user access token to get insights.
+ Make sure that current URLs like /example.php?a=b&c=&d retain their structure (don't strip or introduce an equals
  sign for valueless GET params), and added unit tests to exercise this.
+ CSRF state is now managed using the persistent store instead of cookies.
  • Loading branch information
jcain committed Jun 1, 2011
1 parent 56fe8b7 commit ab2d46d
Show file tree
Hide file tree
Showing 4 changed files with 134 additions and 50 deletions.
66 changes: 39 additions & 27 deletions src/base_facebook.php
View file
Expand Up @@ -120,7 +120,7 @@ abstract class BaseFacebook
/**
* Version.
*/
const VERSION = '3.0.0';
const VERSION = '3.0.1';

/**
* Default options for curl.
Expand Down Expand Up @@ -206,8 +206,10 @@ public function __construct($config) {
if (isset($config['fileUpload'])) {
$this->setFileUploadSupport($config['fileUpload']);
}
if (isset($_COOKIE[$this->getCSRFTokenCookieName()])) {
$this->state = $_COOKIE[$this->getCSRFTokenCookieName()];

$state = $this->getPersistentData('state');
if (!empty($state)) {
$this->state = $this->getPersistentData('state');
}
}

Expand Down Expand Up @@ -533,7 +535,7 @@ protected function getCode() {

// CSRF state has done its job, so clear it
$this->state = null;
unset($_COOKIE[$this->getCSRFTokenCookieName()]);
$this->clearPersistentData('state');
return $_REQUEST['code'];
} else {
self::errorLog('CSRF state token does not match one provided.');
Expand Down Expand Up @@ -575,18 +577,14 @@ protected function getApplicationAccessToken() {
}

/**
* Lays down a CSRF state token for this process. We
* only generate a new CSRF token if one isn't currently
* circulating in the domain's cookie jar.
* Lays down a CSRF state token for this process.
*
* @return void
*/
protected function establishCSRFTokenState() {
if ($this->state === null) {
$this->state = md5(uniqid(mt_rand(), true));
setcookie($name = $this->getCSRFTokenCookieName(),
$value = $this->state,
$expires = time() + 3600); // sticks for an hour
$this->setPersistentData('state', $this->state);
}
}

Expand Down Expand Up @@ -772,15 +770,6 @@ protected function makeRequest($url, $params, $ch=null) {
return $result;
}

/**
* The name of the cookie housing the CSRF protection value.
*
* @return String the cookie name
*/
protected function getCSRFTokenCookieName() {
return 'fbcsrf_'.$this->getAppId();
}

/**
* Parses a signed_request and validates the signature.
*
Expand Down Expand Up @@ -923,16 +912,19 @@ protected function getCurrentUrl() {
$currentUrl = $protocol . $_SERVER['HTTP_HOST'] . $_SERVER['REQUEST_URI'];
$parts = parse_url($currentUrl);

// drop known fb params
$query = '';
if (!empty($parts['query'])) {
$params = array();
parse_str($parts['query'], $params);
foreach(self::$DROP_QUERY_PARAMS as $key) {
unset($params[$key]);
// drop known fb params
$params = explode('&', $parts['query']);
$retained_params = array();
foreach ($params as $param) {
if ($this->shouldRetainParam($param)) {
$retained_params[] = $param;
}
}
if (!empty($params)) {
$query = '?' . http_build_query($params, null, '&');

if (!empty($retained_params)) {
$query = '?'.implode($retained_params, '&');
}
}

Expand All @@ -947,6 +939,25 @@ protected function getCurrentUrl() {
return $protocol . $parts['host'] . $port . $parts['path'] . $query;
}

/**
* Returns true if and only if the key or key/value pair should
* be retained as part of the query string. This amounts to
* a brute-force search of the very small list of Facebook-specific
* params that should be stripped out.
*
* @param String a key or key/value pair within a URL's query (e.g.
* 'foo=a', 'foo=', or 'foo'.
*/
protected function shouldRetainParam($param) {
foreach (self::$DROP_QUERY_PARAMS as $drop_query_param) {
if (strpos($param, $drop_query_param.'=') === 0) {
return false;
}
}

return true;
}

/**
* Analyzes the supplied result to see if it was thrown
* because the access token is no longer valid. If that is
Expand Down Expand Up @@ -1004,7 +1015,7 @@ protected static function base64UrlDecode($input) {
}

/**
* Each of the following three methods should be overridden in
* Each of the following four methods should be overridden in
* a concrete subclass, as they are in the provided Facebook class.
* The Facebook class uses PHP sessions to provide a primitive
* persistent store, but another subclass--one that you implement--
Expand All @@ -1014,5 +1025,6 @@ protected static function base64UrlDecode($input) {
*/
abstract protected function setPersistentData($key, $value);
abstract protected function getPersistentData($key, $default = false);
abstract protected function clearPersistentData($key);
abstract protected function clearAllPersistentData();
}
26 changes: 18 additions & 8 deletions src/facebook.php
View file
Expand Up @@ -33,20 +33,21 @@ class Facebook extends BaseFacebook
* @see BaseFacebook::__construct in facebook.php
*/
public function __construct($config) {
parent::__construct($config);
if (!isset($_SESSION)) {
if (!session_id()) {
session_start();
}
parent::__construct($config);
}

protected static $kSupportedKeys =
array('state', 'code', 'access_token', 'user_id');

/**
* Provides the implementations of the inherited abstract
* methods. The implementation uses PHP sessions to maintain
* a store for user ids and access tokens.
* a store for authorization codes, user ids, CSRF states, and
* access tokens.
*/
protected static $kSupportedKeys =
array('code', 'access_token', 'user_id');

protected function setPersistentData($key, $value) {
if (!in_array($key, self::$kSupportedKeys)) {
self::errorLog('Unsupported key passed to setPersistentData.');
Expand All @@ -68,10 +69,19 @@ protected function getPersistentData($key, $default = false) {
$_SESSION[$session_var_name] : $default;
}

protected function clearPersistentData($key) {
if (!in_array($key, self::$kSupportedKeys)) {
self::errorLog('Unsupported key passed to clearPersistentData.');
return;
}

$session_var_name = $this->constructSessionVariableName($key);
unset($_SESSION[$session_var_name]);
}

protected function clearAllPersistentData() {
foreach (self::$kSupportedKeys as $key) {
$session_var_name = $this->constructSessionVariableName($key);
unset($_SESSION[$session_var_name]);
$this->clearPersistentData($key);
}
}

Expand Down
5 changes: 5 additions & 0 deletions tests/bootstrap.php
View file
@@ -0,0 +1,5 @@
<?php

$base = realpath(dirname(__FILE__) . '/..');
require "$base/src/base_facebook.php";
require "$base/src/facebook.php";
87 changes: 72 additions & 15 deletions tests/tests.php
View file
Expand Up @@ -94,8 +94,44 @@ public function testSetFileUploadSupport() {
'Expect file upload support to be on.');
}

public function testGetCurrentURL() {
$facebook = new FBGetCurrentURLFacebook(array(
'appId' => self::APP_ID,
'secret' => self::SECRET,
));

// fake the HPHP $_SERVER globals
$_SERVER['HTTP_HOST'] = 'www.test.com';
$_SERVER['REQUEST_URI'] = '/unit-tests.php?one=one&two=two&three=three';
$current_url = $facebook->publicGetCurrentUrl();
$this->assertEquals(
'http://www.test.com/unit-tests.php?one=one&two=two&three=three',
$current_url,
'getCurrentUrl function is changing the current URL');

// ensure structure of valueless GET params is retained (sometimes
// an = sign was present, and sometimes it was not)
// first test when equal signs are present
$_SERVER['HTTP_HOST'] = 'www.test.com';
$_SERVER['REQUEST_URI'] = '/unit-tests.php?one=&two=&three=';
$current_url = $facebook->publicGetCurrentUrl();
$this->assertEquals(
'http://www.test.com/unit-tests.php?one=&two=&three=',
$current_url,
'getCurrentUrl function is changing the current URL');

// now confirm that
$_SERVER['HTTP_HOST'] = 'www.test.com';
$_SERVER['REQUEST_URI'] = '/unit-tests.php?one&two&three';
$current_url = $facebook->publicGetCurrentUrl();
$this->assertEquals(
'http://www.test.com/unit-tests.php?one&two&three',
$current_url,
'getCurrentUrl function is changing the current URL');
}

public function testGetLoginURL() {
$facebook = new TransientFacebook(array(
$facebook = new Facebook(array(
'appId' => self::APP_ID,
'secret' => self::SECRET,
));
Expand All @@ -120,7 +156,7 @@ public function testGetLoginURL() {
}

public function testGetLoginURLWithExtraParams() {
$facebook = new TransientFacebook(array(
$facebook = new Facebook(array(
'appId' => self::APP_ID,
'secret' => self::SECRET,
));
Expand Down Expand Up @@ -148,30 +184,28 @@ public function testGetLoginURLWithExtraParams() {
}

public function testGetCodeWithValidCSRFState() {
$csrf_cookie_name = FBCode::constructCSRFTokenCookieName(self::APP_ID);
$_COOKIE[$csrf_cookie_name] = $this->generateMD5HashOfRandomValue();
$facebook = new FBCode(array(
'appId' => self::APP_ID,
'secret' => self::SECRET,
));

$facebook->setCSRFStateToken();
$code = $_REQUEST['code'] = $this->generateMD5HashOfRandomValue();
$_REQUEST['state'] = $_COOKIE[$csrf_cookie_name];
$_REQUEST['state'] = $facebook->getCSRFStateToken();
$this->assertEquals($code,
$facebook->publicGetCode(),
'Expect code to be pulled from $_REQUEST[\'code\']');
}

public function testGetCodeWithInvalidCSRFState() {
$csrf_cookie_name = FBCode::constructCSRFTokenCookieName(self::APP_ID);
$_COOKIE[$csrf_cookie_name] = $this->generateMD5HashOfRandomValue();
$facebook = new FBCode(array(
'appId' => self::APP_ID,
'secret' => self::SECRET,
));

$facebook->setCSRFStateToken();
$code = $_REQUEST['code'] = $this->generateMD5HashOfRandomValue();
$_REQUEST['state'] = $_COOKIE[$csrf_cookie_name]."forgery!!!";
$_REQUEST['state'] = $facebook->getCSRFStateToken().'forgery!!!';
$this->assertFalse($facebook->publicGetCode(),
'Expect getCode to fail, CSRF state should not match.');
}
Expand All @@ -183,7 +217,7 @@ public function testGetCodeWithMissingCSRFState() {
));

$code = $_REQUEST['code'] = $this->generateMD5HashOfRandomValue();
// don't set $_REQUEST['fbcsrf_<app-id>']
// intentionally don't set CSRF token at all
$this->assertFalse($facebook->publicGetCode(),
'Expect getCode to fail, CSRF state not sent back.');

Expand Down Expand Up @@ -562,9 +596,20 @@ public function testAppSecretCall() {
'appId' => self::APP_ID,
'secret' => self::SECRET,
));
$response = $facebook->api('/' . self::APP_ID . '/insights');
$this->assertTrue(count($response['data']) > 0,
'Expect some data back.');

$proper_exception_thrown = false;
try {
$response = $facebook->api('/' . self::APP_ID . '/insights');
$this->fail('Desktop applications need a user token for insights.');
} catch (FacebookApiException $e) {
$proper_exception_thrown =
strpos($e->getMessage(),
'Requires session when calling from a desktop app') !== false;
} catch (Exception $e) {}

$this->assertTrue($proper_exception_thrown,
'Incorrect exception type thrown when trying to gain '.
'insights for desktop app without a user access token.');
}

public function testBase64UrlEncode() {
Expand Down Expand Up @@ -734,6 +779,7 @@ protected function setPersistentData($key, $value) {}
protected function getPersistentData($key, $default = false) {
return $default;
}
protected function clearPersistentData($key) {}
protected function clearAllPersistentData() {}
}

Expand Down Expand Up @@ -762,18 +808,23 @@ class PersistentFBPublic extends Facebook {
public function publicParseSignedRequest($input) {
return $this->parseSignedRequest($input);
}

public function publicSetPersistentData($key, $value) {
$this->setPersistentData($key, $value);
}
}

class FBCode extends TransientFacebook {
class FBCode extends Facebook {
public function publicGetCode() {
return $this->getCode();
}

public static function constructCSRFTokenCookieName($app_id) {
return 'fbcsrf_'.$app_id;
public function setCSRFStateToken() {
$this->establishCSRFTokenState();
}

public function getCSRFStateToken() {
return $this->getPersistentData('state');
}
}

Expand All @@ -782,3 +833,9 @@ public function publicGetApplicationAccessToken() {
return $this->getApplicationAccessToken();
}
}

class FBGetCurrentURLFacebook extends TransientFacebook {
public function publicGetCurrentUrl() {
return $this->getCurrentUrl();
}
}

0 comments on commit ab2d46d

Please sign in to comment.