Skip to content

Commit

Permalink
[BUGFIX] Prevent page cache poisoning
Browse files Browse the repository at this point in the history
Prevent page cache poisoning when using authentication providers
that are not session cookie based.
  • Loading branch information
Mateu Aguiló Bosch committed Sep 9, 2015
1 parent b298b78 commit 67a32c0
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 2 deletions.
8 changes: 8 additions & 0 deletions plugins/authentication/RestfulAuthenticationManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ public function getAccount(array $request = array(), $method = \RestfulInterface
if (!$account) {

if ($this->count() && !$this->getIsOptional()) {
// Allow caching pages for anonymous users.
drupal_page_is_cacheable(variable_get('restful_page_cache', FALSE));

// User didn't authenticate against any provider, so we throw an error.
throw new \RestfulUnauthorizedException('Bad credentials');
}
Expand All @@ -115,6 +118,11 @@ public function getAccount(array $request = array(), $method = \RestfulInterface
$this->setAccount($account);
}

// Disable page caching for security reasons so that an authenticated user
// response never gets into the page cache for anonymous users.
// This is necessary because the page cache system only looks at session
// cookies, but not at HTTP Basic Auth headers.
drupal_page_is_cacheable(!$account->uid && variable_get('restful_page_cache', FALSE));
return $account;
}

Expand Down
6 changes: 5 additions & 1 deletion restful.admin.inc
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,14 @@ function restful_admin_settings($form_state) {
);

$form['restful_cache']['restful_page_cache'] = array(
'#markup' => t('RESTful leverages page cache out of the box. !link to start caching responses. Status: <strong>@status</strong>.', array(
'#type' => 'checkbox',
'#title' => t('Page cache'),
'#description' => t('RESTful can leverage page cache, this will boost your performace for anonymous traffic. !link to start caching responses. Status: <strong>@status</strong>. <strong>CAUTION:</strong> If your resources are using authentication providers other than cookie, you will want to turn this off. Otherwise you may get cached anonymous values for your authenticated GET requests.', array(
'!link' => l(t('Enable page cache'), 'admin/config/development/performance'),
'@status' => variable_get('cache', FALSE) ? t('Enabled') : t('Disabled'),
)),
'#disabled' => !variable_get('cache', FALSE),
'#default_value' => variable_get('restful_page_cache', FALSE) && variable_get('cache', FALSE),
);

$form['restful_cache']['restful_render_cache'] = array(
Expand Down
81 changes: 80 additions & 1 deletion tests/RestfulRenderCacheTestCase.test
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* Contains RestfulRenderCacheTestCase
*/

class RestfulRenderCacheTestCase extends DrupalWebTestCase {
class RestfulRenderCacheTestCase extends \RestfulCurlBaseTestCase {

public static function getInfo() {
return array(
Expand Down Expand Up @@ -139,4 +139,83 @@ class RestfulRenderCacheTestCase extends DrupalWebTestCase {
$this->assertTrue($record->data, 'Cache key contains role information.');
}

/**
* Tests for SA 154563.
*/
public function testPageCache() {
// Enable page cache.
variable_set('cache', TRUE);
variable_set('restful_page_cache', TRUE);

// Make anonymous users not to be able to access content.
user_role_change_permissions(DRUPAL_ANONYMOUS_RID, array(
'access content' => FALSE,
));

// Create a new article.
$settings = array('type' => 'article');
$settings['title'] = 'Node title';
$node = $this->drupalCreateNode($settings);
$path = 'api/v1.0/test_articles/' . $node->nid;
$url = url($path, array('absolute' => TRUE));
$cache = _cache_get_object('cache_page');

// Create a user that can access content.
$account = $this->drupalCreateUser(array('access content'));

// 1. Test the cookie authentication.
// Log in the user (creating the cookie).
$this->drupalLogin($account);
// Access the created article creating a page cache entry.
$response = $this->httpRequest($path);
$this->assertEqual($response['code'], 200, 'Access granted for logged in user.');
// Pages are not cached if the request is originated in CLI.
// @see drupal_page_is_cacheable()
if (!drupal_is_cli()) {
// Make sure that there is not a page cache entry.
$this->assertFalse($cache->get($url), 'There should not be a cache entry for a authenticated user.');
}
// Log out the user.
$this->drupalLogout();
// Try to access the cached resource.
$response = $this->httpRequest($path);
// The user should get a 401.
$this->assertEqual($response['code'], 401, 'Access denied for anonymous user.');
if (!drupal_is_cli()) {
// Make sure that there is a page cache entry.
$this->assertTrue($cache->get($url), 'A page cache entry was created for an anonymous user.');
}
// Remove the cache entry.
$cache->clear($url);

// 2. Test the basic authentication.
$response = $this->httpRequest($path, \RestfulInterface::GET, NULL, array(
'Authorization' => 'Basic ' . drupal_base64_encode($account->name . ':' . $account->pass_raw),
));
$this->assertEqual($response['code'], 200, 'Access granted for logged in user.');
if (!drupal_is_cli()) {
// Make sure that there is a page cache entry.
$this->assertFalse($cache->get($url), 'A page cache entry was not created with basic auth.');
}

// Try to access the cached resource.
$response = $this->httpRequest($path);
// The user should get a 401.
$this->assertEqual($response['code'], 401, 'Access denied for anonymous user.');
if (!drupal_is_cli()) {
// Make sure that there is not a page cache entry.
$this->assertTrue($cache->get($url), 'A page cache entry was created for an anonymous user.');
}

// 3. Test that when restful_page_cache is off there is no page cache.
// Remove the cache entry.
$cache->clear($url);
variable_set('restful_page_cache', FALSE);
// Try to access the cached resource as anonymous users.
$this->httpRequest($path);
if (!drupal_is_cli()) {
$this->assertFalse($cache->get($url), 'A page cache entry was not created for an anonymous users when restful_page_cache is off.');
}
}

}

0 comments on commit 67a32c0

Please sign in to comment.