Commit 44e8ffd5 authored by xjm's avatar xjm

Issue #2432585 by almaudoh, znerol, dawehner, cpj: Improve authentication...

Issue #2432585 by almaudoh, znerol, dawehner, cpj: Improve authentication manager service construction to support custom global service providers
parent 4f697081
......@@ -20,7 +20,7 @@
*
* If no provider set an active user then the user is set to anonymous.
*/
class AuthenticationManager implements AuthenticationProviderInterface, AuthenticationProviderFilterInterface, AuthenticationProviderChallengeInterface {
class AuthenticationManager implements AuthenticationProviderInterface, AuthenticationProviderFilterInterface, AuthenticationProviderChallengeInterface, AuthenticationManagerInterface {
/**
* Array of all registered authentication providers, keyed by ID.
......@@ -65,44 +65,23 @@ class AuthenticationManager implements AuthenticationProviderInterface, Authenti
protected $globalProviders;
/**
* Constructs an authentication manager.
*
* @todo Revisit service construction. Especially write a custom compiler pass
* which is capable of collecting, sorting and injecting all providers
* (including global/vs non global), filters and challengers on compile
* time in https://www.drupal.org/node/2432585.
*
* @param array $global_providers
* List of global providers, keyed by the provier ID.
*/
public function __construct($global_providers = ['cookie' => TRUE]) {
$this->globalProviders = $global_providers;
}
/**
* Adds a provider to the array of registered providers.
*
* @param \Drupal\Core\Authentication\AuthenticationProviderInterface $provider
* The provider object.
* @param string $id
* Identifier of the provider.
* @param int $priority
* The providers priority.
* {@inheritdoc}
*/
public function addProvider(AuthenticationProviderInterface $provider, $id, $priority = 0) {
// Remove the 'authentication.' prefix from the provider ID.
$id = substr($id, 15);
$this->providers[$id] = $provider;
$this->providerOrders[$priority][$id] = $provider;
public function addProvider(AuthenticationProviderInterface $provider, $provider_id, $priority = 0, $global = FALSE) {
$this->providers[$provider_id] = $provider;
$this->providerOrders[$priority][$provider_id] = $provider;
// Force the builders to be re-sorted.
$this->sortedProviders = NULL;
if ($provider instanceof AuthenticationProviderFilterInterface) {
$this->filters[$id] = $provider;
$this->filters[$provider_id] = $provider;
}
if ($provider instanceof AuthenticationProviderChallengeInterface) {
$this->challengers[$id] = $provider;
$this->challengers[$provider_id] = $provider;
}
if ($global) {
$this->globalProviders[$provider_id] = TRUE;
}
}
......
<?php
/**
* @file
* Contains \Drupal\Core\Authentication\AuthenticationManagerInterface
*/
namespace Drupal\Core\Authentication;
/**
* Defines an interface for authentication managers.
*/
interface AuthenticationManagerInterface {
/**
* Adds a provider to the array of registered providers.
*
* @param \Drupal\Core\Authentication\AuthenticationProviderInterface $provider
* The provider object.
* @param string $provider_id
* Identifier of the provider.
* @param int $priority
* (optional) The provider's priority.
* @param bool $global
* (optional) TRUE if the provider is to be applied globally on all routes.
* Defaults to FALSE.
*/
public function addProvider(AuthenticationProviderInterface $provider, $provider_id, $priority = 0, $global = FALSE);
}
......@@ -94,16 +94,20 @@ public function process(ContainerBuilder $container) {
$interface_pos = 0;
$id_pos = NULL;
$priority_pos = NULL;
$extra_params = [];
foreach ($params as $pos => $param) {
if ($param->getClass()) {
$interface = $param->getClass();
}
if ($param->getName() == 'id') {
else if ($param->getName() === 'id') {
$id_pos = $pos;
}
if ($param->getName() == 'priority') {
else if ($param->getName() === 'priority') {
$priority_pos = $pos;
}
else {
$extra_params[$param->getName()] = $pos;
}
}
// Determine the ID.
......@@ -118,6 +122,7 @@ public function process(ContainerBuilder $container) {
// Find all tagged handlers.
$handlers = array();
$extra_arguments = array();
foreach ($container->findTaggedServiceIds($tag) as $id => $attributes) {
// Validate the interface.
$handler = $container->getDefinition($id);
......@@ -125,6 +130,10 @@ public function process(ContainerBuilder $container) {
throw new LogicException("Service '$id' for consumer '$consumer_id' does not implement $interface.");
}
$handlers[$id] = isset($attributes[0]['priority']) ? $attributes[0]['priority'] : 0;
// Keep track of other tagged handlers arguments.
foreach ($extra_params as $name => $pos) {
$extra_arguments[$id][$pos] = isset($attributes[0][$name]) ? $attributes[0][$name] : $params[$pos]->getDefaultValue();
}
}
if (empty($handlers)) {
if ($required) {
......@@ -146,6 +155,11 @@ public function process(ContainerBuilder $container) {
if (isset($id_pos)) {
$arguments[$id_pos] = $id;
}
// Add in extra arguments.
if (isset($extra_arguments[$id])) {
// Place extra arguments in their right positions.
$arguments += $extra_arguments[$id];
}
// Sort the arguments by position.
ksort($arguments);
$consumer->addMethodCall($method_name, $arguments);
......
services:
authentication.basic_auth:
basic_auth.authentication.basic_auth:
class: Drupal\basic_auth\Authentication\Provider\BasicAuth
arguments: ['@config.factory', '@user.auth', '@flood', '@entity.manager']
tags:
- { name: authentication_provider, priority: 100 }
- { name: authentication_provider, provider_id: 'basic_auth', priority: 100 }
basic_auth.page_cache_request_policy.disallow_basic_auth_requests:
class: Drupal\basic_auth\PageCache\DisallowBasicAuthRequests
public: false
......
services:
authentication.early_translation_test:
early_translation_test.authentication.early_translation_test:
class: Drupal\early_translation_test\Auth
arguments: ['@entity.manager']
tags:
- { name: authentication_provider, priority: 100 }
- { name: authentication_provider, provider_id: 'early_translation_test', priority: 100 }
......@@ -15,11 +15,11 @@ services:
class: Drupal\user\Access\LoginStatusCheck
tags:
- { name: access_check, applies_to: _user_is_logged_in }
authentication.cookie:
user.authentication.cookie:
class: Drupal\user\Authentication\Provider\Cookie
arguments: ['@session_configuration', '@database']
tags:
- { name: authentication_provider, priority: 0 }
- { name: authentication_provider, provider_id: 'cookie', priority: 0, global: TRUE }
user.data:
class: Drupal\user\UserData
arguments: ['@database']
......
......@@ -27,10 +27,10 @@ class AuthenticationManagerTest extends UnitTestCase {
*
* @dataProvider providerTestDefaultFilter
*/
public function testDefaultFilter($applies, $has_route, $auth_option, $provider_id, $global_providers = ['cookie' => TRUE]) {
$authentication_manager = new AuthenticationManager($global_providers);
public function testDefaultFilter($applies, $has_route, $auth_option, $provider_id, $global) {
$authentication_manager = new AuthenticationManager();
$auth_provider = $this->getMock('Drupal\Core\Authentication\AuthenticationProviderInterface');
$authentication_manager->addProvider($auth_provider, 'authentication.' . $provider_id);
$authentication_manager->addProvider($auth_provider, $provider_id, 0, $global);
$request = new Request();
if ($has_route) {
......@@ -50,7 +50,7 @@ public function testDefaultFilter($applies, $has_route, $auth_option, $provider_
public function testApplyFilterWithFilterprovider() {
$authentication_manager = new AuthenticationManager();
$auth_provider = $this->getMock('Drupal\Tests\Core\Authentication\TestAuthenticationProviderInterface');
$authentication_manager->addProvider($auth_provider, 'authentication.filtered');
$authentication_manager->addProvider($auth_provider, 'filtered', 0);
$auth_provider->expects($this->once())
->method('appliesToRoutedRequest')
......@@ -66,17 +66,17 @@ public function testApplyFilterWithFilterprovider() {
public function providerTestDefaultFilter() {
$data = [];
// No route, cookie is global, should apply.
$data[] = [TRUE, FALSE, [], 'cookie'];
$data[] = [TRUE, FALSE, [], 'cookie', TRUE];
// No route, cookie is not global, should not apply.
$data[] = [FALSE, FALSE, [], 'cookie', ['other' => TRUE]];
$data[] = [FALSE, FALSE, [], 'cookie', FALSE];
// Route, no _auth, cookie is global, should apply.
$data[] = [TRUE, TRUE, [], 'cookie'];
$data[] = [TRUE, TRUE, [], 'cookie', TRUE];
// Route, no _auth, cookie is not global, should not apply.
$data[] = [FALSE, TRUE, [], 'cookie', ['other' => TRUE]];
$data[] = [FALSE, TRUE, [], 'cookie', FALSE];
// Route, with _auth and non-matching provider, should not apply.
$data[] = [FALSE, TRUE, ['basic_auth'], 'cookie'];
$data[] = [FALSE, TRUE, ['basic_auth'], 'cookie', TRUE];
// Route, with _auth and matching provider should not apply.
$data[] = [TRUE, TRUE, ['basic_auth'], 'basic_auth'];
$data[] = [TRUE, TRUE, ['basic_auth'], 'basic_auth', TRUE];
return $data;
}
......
......@@ -226,6 +226,128 @@ public function testProcessInterfaceMismatch() {
$handler_pass->process($container);
}
/**
* Tests consumer method with extra parameters.
*
* @covers ::process
*/
public function testProcessWithExtraArguments() {
$container = $this->buildContainer();
$container
->register('consumer_id', __NAMESPACE__ . '\ValidConsumerWithExtraArguments')
->addTag('service_collector');
$container
->register('handler1', __NAMESPACE__ . '\ValidHandler')
->addTag('consumer_id', array(
'extra1' => 'extra1',
'extra2' => 'extra2',
));
$handler_pass = new TaggedHandlersPass();
$handler_pass->process($container);
$method_calls = $container->getDefinition('consumer_id')->getMethodCalls();
$this->assertCount(4, $method_calls[0][1]);
$this->assertEquals(new Reference('handler1'), $method_calls[0][1][0]);
$this->assertEquals(0, $method_calls[0][1][1]);
$this->assertEquals('extra1', $method_calls[0][1][2]);
$this->assertEquals('extra2', $method_calls[0][1][3]);
}
/**
* Tests consumer method with extra parameters and no priority.
*
* @covers ::process
*/
public function testProcessNoPriorityAndExtraArguments() {
$container = $this->buildContainer();
$container
->register('consumer_id', __NAMESPACE__ . '\ValidConsumerWithExtraArguments')
->addTag('service_collector', array(
'call' => 'addNoPriority'
));
$container
->register('handler1', __NAMESPACE__ . '\ValidHandler')
->addTag('consumer_id', array(
'extra' => 'extra',
));
$handler_pass = new TaggedHandlersPass();
$handler_pass->process($container);
$method_calls = $container->getDefinition('consumer_id')->getMethodCalls();
$this->assertCount(2, $method_calls[0][1]);
$this->assertEquals(new Reference('handler1'), $method_calls[0][1][0]);
$this->assertEquals('extra', $method_calls[0][1][1]);
}
/**
* Tests consumer method with priority, id and extra parameters.
*
* @covers ::process
*/
public function testProcessWithIdAndExtraArguments() {
$container = $this->buildContainer();
$container
->register('consumer_id', __NAMESPACE__ . '\ValidConsumerWithExtraArguments')
->addTag('service_collector', array(
'call' => 'addWithId'
));
$container
->register('handler1', __NAMESPACE__ . '\ValidHandler')
->addTag('consumer_id', array(
'extra1' => 'extra1',
));
$handler_pass = new TaggedHandlersPass();
$handler_pass->process($container);
$method_calls = $container->getDefinition('consumer_id')->getMethodCalls();
$this->assertCount(5, $method_calls[0][1]);
$this->assertEquals(new Reference('handler1'), $method_calls[0][1][0]);
$this->assertEquals('handler1', $method_calls[0][1][1]);
$this->assertEquals(0, $method_calls[0][1][2]);
$this->assertEquals('extra1', $method_calls[0][1][3]);
$this->assertNull($method_calls[0][1][4]);
}
/**
* Tests consumer method with priority and extra parameters in different order.
*
* @covers ::process
*/
public function testProcessWithDifferentArgumentsOrderAndDefaultValue() {
$container = $this->buildContainer();
$container
->register('consumer_id', __NAMESPACE__ . '\ValidConsumerWithExtraArguments')
->addTag('service_collector', array(
'call' => 'addWithDifferentOrder'
));
$container
->register('handler1', __NAMESPACE__ . '\ValidHandler')
->addTag('consumer_id', array(
'priority' => 0,
'extra1' => 'extra1',
'extra3' => 'extra3'
));
$handler_pass = new TaggedHandlersPass();
$handler_pass->process($container);
$method_calls = $container->getDefinition('consumer_id')->getMethodCalls();
$this->assertCount(5, $method_calls[0][1]);
$expected = [new Reference('handler1'), 'extra1', 0, 'default2', 'extra3'];
$this->assertEquals($expected, array_values($method_calls[0][1]));
}
}
interface HandlerInterface {
......@@ -242,6 +364,16 @@ class InvalidConsumer {
public function addHandler($instance, $priority = 0) {
}
}
class ValidConsumerWithExtraArguments {
public function addHandler(HandlerInterface $instance, $priority = 0, $extra1 = '', $extra2 = '') {
}
public function addNoPriority(HandlerInterface $instance, $extra) {
}
public function addWithId(HandlerInterface $instance, $id, $priority = 0, $extra1 = '', $extra2 = NULL) {
}
public function addWithDifferentOrder(HandlerInterface $instance, $extra1, $priority = 0, $extra2 = 'default2', $extra3 = 'default3') {
}
}
class ValidHandler implements HandlerInterface {
}
class InvalidHandler {
......
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