Commit e5fe68e2 authored by webchick's avatar webchick

Issue #1425330 by swentel, grendzy, davereid, wojtha, c960657: Fixed Apply...

Issue #1425330 by swentel, grendzy, davereid, wojtha, c960657: Fixed Apply Aggregator and OpenID fixes from DRUPAL-SA-CORE-2012-001.
parent 28b68ad7
......@@ -37,7 +37,7 @@ function aggregator_view() {
($feed->checked && $feed->refresh ? t('%time left', array('%time' => format_interval($feed->checked + $feed->refresh - REQUEST_TIME))) : t('never')),
l(t('edit'), "admin/config/services/aggregator/edit/feed/$feed->fid"),
l(t('remove items'), "admin/config/services/aggregator/remove/$feed->fid"),
l(t('update items'), "admin/config/services/aggregator/update/$feed->fid"),
l(t('update items'), "admin/config/services/aggregator/update/$feed->fid", array('query' => array('token' => drupal_get_token("aggregator/update/$feed->fid")))),
);
}
$output .= theme('table', array('header' => $header, 'rows' => $rows, 'empty' => t('No feeds available. <a href="@link">Add feed</a>.', array('@link' => url('admin/config/services/aggregator/add/feed')))));
......@@ -421,6 +421,9 @@ function _aggregator_parse_opml($opml) {
* @see aggregator_menu()
*/
function aggregator_admin_refresh_feed($feed) {
if (!isset($_GET['token']) || !drupal_valid_token($_GET['token'], 'aggregator/update/' . $feed->fid)) {
return MENU_ACCESS_DENIED;
}
aggregator_refresh($feed);
drupal_goto('admin/config/services/aggregator');
}
......
......@@ -92,8 +92,13 @@ class AggregatorTestCase extends DrupalWebTestCase {
$this->drupalGet($feed->url);
$this->assertResponse(200, t('!url is reachable.', array('!url' => $feed->url)));
// Refresh the feed (simulated link click).
// Attempt to access the update link directly without an access token.
$this->drupalGet('admin/config/services/aggregator/update/' . $feed->fid);
$this->assertResponse(403);
// Refresh the feed (simulated link click).
$this->drupalGet('admin/config/services/aggregator');
$this->clickLink('update items');
// Ensure we have the right number of items.
$result = db_query('SELECT iid FROM {aggregator_item} WHERE fid = :fid', array(':fid' => $feed->fid));
......@@ -531,8 +536,8 @@ class UpdateFeedItemTestCase extends AggregatorTestCase {
$this->assertRaw(t('The feed %name has been added.', array('%name' => $edit['title'])), t('The feed !name has been added.', array('!name' => $edit['title'])));
$feed = db_query("SELECT * FROM {aggregator_feed} WHERE url = :url", array(':url' => $edit['url']))->fetchObject();
$this->drupalGet('admin/config/services/aggregator/update/' . $feed->fid);
aggregator_refresh($feed);
$before = db_query('SELECT timestamp FROM {aggregator_item} WHERE fid = :fid', array(':fid' => $feed->fid))->fetchField();
// Sleep for 3 second.
......@@ -546,10 +551,9 @@ class UpdateFeedItemTestCase extends AggregatorTestCase {
'modified' => 0,
))
->execute();
$this->drupalGet('admin/config/services/aggregator/update/' . $feed->fid);
aggregator_refresh($feed);
$after = db_query('SELECT timestamp FROM {aggregator_item} WHERE fid = :fid', array(':fid' => $feed->fid))->fetchField();
$this->assertTrue($before === $after, t('Publish timestamp of feed item was not updated (!before === !after)', array('!before' => $before, '!after' => $after)));
}
}
......
......@@ -610,18 +610,31 @@ function _openid_get_params($str) {
* @param $fallback_prefix
* An optional prefix that will be used in case no prefix is found for the
* target extension namespace.
* @param $only_signed
* Return only keys that are included in the message signature in openid.sig.
* Unsigned fields may have been modified or added by other parties than the
* OpenID Provider.
*
* @return
* An associative array containing all the parameters in the response message
* that belong to the extension. The keys are stripped from their namespace
* prefix.
*
* @see http://openid.net/specs/openid-authentication-2_0.html#extensions
*/
function openid_extract_namespace($response, $extension_namespace, $fallback_prefix = NULL) {
function openid_extract_namespace($response, $extension_namespace, $fallback_prefix = NULL, $only_signed = FALSE) {
$signed_keys = explode(',', $response['openid.signed']);
// Find the namespace prefix.
$prefix = $fallback_prefix;
foreach ($response as $key => $value) {
if ($value == $extension_namespace && preg_match('/^openid\.ns\.([^.]+)$/', $key, $matches)) {
$prefix = $matches[1];
if ($only_signed && !in_array('ns.' . $matches[1], $signed_keys)) {
// The namespace was defined but was not signed as required. In this
// case we do not fall back to $fallback_prefix.
$prefix = NULL;
}
break;
}
}
......@@ -634,7 +647,9 @@ function openid_extract_namespace($response, $extension_namespace, $fallback_pre
foreach ($response as $key => $value) {
if (preg_match('/^openid\.' . $prefix . '\.(.+)$/', $key, $matches)) {
$local_key = $matches[1];
$output[$local_key] = $value;
if (!$only_signed || in_array($prefix . '.' . $local_key, $signed_keys)) {
$output[$local_key] = $value;
}
}
}
......
......@@ -185,10 +185,15 @@ function openid_form_user_register_form_alter(&$form, &$form_state) {
$response = $_SESSION['openid']['response'];
// Extract Simple Registration keys from the response.
$sreg_values = openid_extract_namespace($response, OPENID_NS_SREG, 'sreg');
// Extract Attribute Exchanges keys from the response.
$ax_values = openid_extract_namespace($response, OPENID_NS_AX, 'ax');
// Extract Simple Registration keys from the response. We only include
// signed keys as required by OpenID Simple Registration Extension 1.0,
// section 4.
$sreg_values = openid_extract_namespace($response, OPENID_NS_SREG, 'sreg', TRUE);
// Extract Attribute Exchanges keys from the response. We only include
// signed keys. This is not required by the specification, but it is
// recommended by Google, see
// http://googlecode.blogspot.com/2011/05/security-advisory-to-websites-using.html
$ax_values = openid_extract_namespace($response, OPENID_NS_AX, 'ax', TRUE);
if (!empty($sreg_values['nickname'])) {
// Use the nickname returned by Simple Registration if available.
......
......@@ -364,17 +364,49 @@ class OpenIDFunctionalTestCase extends OpenIDWebTestCase {
// Use a User-supplied Identity that is the URL of an XRDS document.
$identity = url('openid-test/yadis/xrds', array('absolute' => TRUE));
// Do not sign all mandatory fields (e.g. assoc_handle).
// Respond with an invalid signature.
variable_set('openid_test_response', array('openid.sig' => 'this-is-an-invalid-signature'));
$this->submitLoginForm($identity);
$this->assertRaw('OpenID login failed.');
// Do not sign the mandatory field openid.assoc_handle.
variable_set('openid_test_response', array('openid.signed' => 'op_endpoint,claimed_id,identity,return_to,response_nonce'));
$this->submitLoginForm($identity);
$this->assertRaw('OpenID login failed.');
// Sign all mandatory fields and some custom fields.
variable_set('openid_test_response', array('openid.foo' => 'bar', 'openid.signed' => 'op_endpoint,claimed_id,identity,return_to,response_nonce,assoc_handle,foo'));
// Sign all mandatory fields and a custom field.
$keys_to_sign = array('op_endpoint', 'claimed_id', 'identity', 'return_to', 'response_nonce', 'assoc_handle', 'foo');
$association = new stdClass();
$association->mac_key = variable_get('mac_key');
$response = array(
'openid.op_endpoint' => url('openid-test/endpoint', array('absolute' => TRUE)),
'openid.claimed_id' => $identity,
'openid.identity' => $identity,
'openid.return_to' => url('openid/authenticate', array('absolute' => TRUE)),
'openid.response_nonce' => _openid_nonce(),
'openid.assoc_handle' => 'openid-test',
'openid.foo' => 123,
'openid.signed' => implode(',', $keys_to_sign),
);
$response['openid.sig'] = _openid_signature($association, $response, $keys_to_sign);
variable_set('openid_test_response', $response);
$this->submitLoginForm($identity);
$this->assertNoRaw('OpenID login failed.');
}
$this->assertFieldByName('name', '', t('No username was supplied by provider.'));
$this->assertFieldByName('mail', '', t('No e-mail address was supplied by provider.'));
// Check that unsigned SREG fields are ignored.
$response = array(
'openid.signed' => 'op_endpoint,claimed_id,identity,return_to,response_nonce,assoc_handle,sreg.nickname',
'openid.sreg.nickname' => 'john',
'openid.sreg.email' => 'john@example.com',
);
variable_set('openid_test_response', $response);
$this->submitLoginForm($identity);
$this->assertNoRaw('OpenID login failed.');
$this->assertFieldByName('name', 'john', t('Username was supplied by provider.'));
$this->assertFieldByName('mail', '', t('E-mail address supplied by provider was ignored.'));
}
}
/**
......@@ -707,4 +739,41 @@ class OpenIDUnitTest extends DrupalWebTestCase {
$this->assertEqual(openid_normalize('http://example.com/path#fragment'), 'http://example.com/path', t('openid_normalize() correctly normalized a URL with a fragment.'));
}
/**
* Test openid_extract_namespace().
*/
function testOpenidExtractNamespace() {
$response = array(
'openid.sreg.nickname' => 'john',
'openid.ns.ext1' => OPENID_NS_SREG,
'openid.ext1.nickname' => 'george',
'openid.ext1.email' => 'george@example.com',
'openid.ns.ext2' => 'http://example.com/ns/ext2',
'openid.ext2.foo' => '123',
'openid.ext2.bar' => '456',
'openid.signed' => 'sreg.nickname,ns.ext1,ext1.email,ext2.foo',
);
$values = openid_extract_namespace($response, 'http://example.com/ns/dummy', NULL, FALSE);
$this->assertEqual($values, array(), t('Nothing found for unused namespace.'));
$values = openid_extract_namespace($response, 'http://example.com/ns/dummy', 'sreg', FALSE);
$this->assertEqual($values, array('nickname' => 'john'), t('Value found for fallback prefix.'));
$values = openid_extract_namespace($response, OPENID_NS_SREG, 'sreg', FALSE);
$this->assertEqual($values, array('nickname' => 'george', 'email' => 'george@example.com'), t('Namespace takes precedence over fallback prefix.'));
// ext1.email is signed, but ext1.nickname is not.
$values = openid_extract_namespace($response, OPENID_NS_SREG, 'sreg', TRUE);
$this->assertEqual($values, array('email' => 'george@example.com'), t('Unsigned namespaced fields ignored.'));
$values = openid_extract_namespace($response, 'http://example.com/ns/ext2', 'sreg', FALSE);
$this->assertEqual($values, array('foo' => '123', 'bar' => '456'), t('Unsigned fields found.'));
// ext2.foo and ext2.bar are ignored, because ns.ext2 is not signed. The
// fallback prefix is not used, because the namespace is specified.
$values = openid_extract_namespace($response, 'http://example.com/ns/ext2', 'sreg', TRUE);
$this->assertEqual($values, array(), t('Unsigned fields ignored.'));
}
}
......@@ -325,9 +325,7 @@ function _openid_test_endpoint_authenticate() {
// Generate unique identifier for this authentication.
$nonce = _openid_nonce();
// Generate response containing the user's identity. The openid.sreg.xxx
// entries contain profile data stored by the OpenID Provider (see OpenID
// Simple Registration Extension 1.0).
// Generate response containing the user's identity.
$response = variable_get('openid_test_response', array()) + array(
'openid.ns' => OPENID_NS_2_0,
'openid.mode' => 'id_res',
......@@ -337,14 +335,27 @@ function _openid_test_endpoint_authenticate() {
'openid.return_to' => $_REQUEST['openid_return_to'],
'openid.response_nonce' => $nonce,
'openid.assoc_handle' => 'openid-test',
'openid.signed' => 'op_endpoint,claimed_id,identity,return_to,response_nonce,assoc_handle',
);
if (isset($response['openid.signed'])) {
$keys_to_sign = explode(',', $response['openid.signed']);
}
else {
// Unless openid.signed is explicitly defined, all keys are signed.
$keys_to_sign = array();
foreach ($response as $key => $value) {
// Strip off the "openid." prefix.
$keys_to_sign[] = substr($key, 7);
}
$response['openid.signed'] = implode(',', $keys_to_sign);
}
// Sign the message using the MAC key that was exchanged during association.
$association = new stdClass();
$association->mac_key = variable_get('mac_key');
$keys_to_sign = explode(',', $response['openid.signed']);
$response['openid.sig'] = _openid_signature($association, $response, $keys_to_sign);
if (!isset($response['openid.sig'])) {
$response['openid.sig'] = _openid_signature($association, $response, $keys_to_sign);
}
// Put the signed message into the query string of a URL supplied by the
// Relying Party, and redirect the user.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment