diff --git a/core/lib/Drupal/Core/Access/AuthenticationProviderAccessCheck.php b/core/lib/Drupal/Core/Access/AuthenticationProviderAccessCheck.php index 090ac00..dc475aa 100644 --- a/core/lib/Drupal/Core/Access/AuthenticationProviderAccessCheck.php +++ b/core/lib/Drupal/Core/Access/AuthenticationProviderAccessCheck.php @@ -10,26 +10,25 @@ use Symfony\Component\Routing\Route; use Symfony\Component\HttpFoundation\Request; +/** + * Access check based on authentication mechanisms. + */ class AuthenticationProviderAccessCheck implements AccessCheckInterface { /** - * This access check applies to every route. + * {@inheritdoc} * - * @param Route $route - * - * @return bool + * This access check applies to every route. */ public function applies(Route $route) { return TRUE; } /** - * Check if route allows authentication provider. - * - * @param Route $route - * @param Request $request + * {@inheritdoc} * - * @return mixed|void + * A route can have a list of allowed authentication mechanism. Check if the + * triggered provider is part of this list. */ public function access(Route $route, Request $request) { $auth_provider_triggered = $request->attributes->get('_authentication_provider'); diff --git a/core/lib/Drupal/Core/Authentication/AuthenticationManager.php b/core/lib/Drupal/Core/Authentication/AuthenticationManager.php index 9f76338..3279824 100644 --- a/core/lib/Drupal/Core/Authentication/AuthenticationManager.php +++ b/core/lib/Drupal/Core/Authentication/AuthenticationManager.php @@ -8,8 +8,18 @@ namespace Drupal\Core\Authentication; use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; +/** + * Manager for authentication. + * + * On each request, let all authentication provider to authenticate the user. + * The provider are iterated according to their priority and the first provider + * detecting credentials for his method will become the triggered provider. No + * further provider will get triggered. + * If no provider felt responsible the manager assumes that the least + * prioritized should have and therefore is the triggered provider and the user + * is set to anonymous. + */ class AuthenticationManager implements AuthenticationProviderInterface { /** @@ -20,7 +30,7 @@ class AuthenticationManager implements AuthenticationProviderInterface { protected $providers; /** - * Priority array all registered authentication providers. + * Priority array off all registered authentication providers. * * This is used to sort the providers based on their priority. * @@ -58,14 +68,13 @@ class AuthenticationManager implements AuthenticationProviderInterface { public function addProvider($provider_id, AuthenticationProviderInterface $provider, $priority = 0) { $this->providers[$provider_id] = $provider; $this->providerPriorities[$provider_id] = $priority; - uksort($this->providers, array($this, 'compareProviderPriority')); } /** * Compares providers by their priority. * - * This is a uksort() callback to sort the providers by their keys based on - * the priorities. + * This is a uksort() callback allowing to sort the providers by their keys + * based on their priorities. * * @param string $a * A provider key. @@ -87,25 +96,29 @@ public function compareProviderPriority($a, $b) { } /** - * Authenticate user by running authenticate() method on each of providers. + * Authenticate user. * - * @throws \Symfony\Component\HttpKernel\Exception\BadRequestHttpException - * Throws exception in case two authentication providers had credentials. + * Iterate the available providers according th their priority. + * + * @return Drupal\Core\Session\AccountInterface + * The account interface of the authenticated user. Defaults to a anonymous. */ public function authenticate(Request $request) { global $user; + // Ensure providers get triggered according to their priority. + uksort($this->providers, array($this, 'compareProviderPriority')); + + // Iterate the availlable providers. foreach ($this->providers as $provider_id => $provider) { + // Trigger authentication. $account = $provider->authenticate($request); + // Provider felt responsible for this request. if ($account !== NULL) { - // We do not allow request to have information to authenticate - // with two methods at the same time. - if (!empty($this->triggeredProvider)) { - throw new BadRequestHttpException(t('Multiple authentication methods are not allowed.')); - } $this->account = $account; $this->triggeredProvider = $provider_id; + break; } } @@ -125,21 +138,27 @@ public function authenticate(Request $request) { $user = $this->account; // Save the ID of the triggered provider to the request so that it can be - // accessed in AuthenticationProviderAccessCheck. + // accessed in Drupal\Core\Access\AuthenticationProviderAccessCheck. $request->attributes->set('_authentication_provider', substr($this->triggeredProvider, strlen('authentication.'))); + + return $this->account; } /** - * Does clean up by running cleanup() of provider that authenticated the user. + * Do clean up. + * + * Allow the triggered provider to clean up before the response is sent, e.g. + * trigger a session commit. * * @param Request $request * The request object. + * + * @see Drupal\Core\Authentication\Provider\Cookie::cleanup() */ public function cleanup(Request $request) { - if (empty($this->triggeredProvider)) { + if (empty($this->providers[$this->triggeredProvider])) { return; } - $provider = $this->providers[$this->triggeredProvider]; - $provider->cleanup($request); + $this->providers[$this->triggeredProvider]->cleanup($request); } } diff --git a/core/lib/Drupal/Core/Authentication/AuthenticationProviderInterface.php b/core/lib/Drupal/Core/Authentication/AuthenticationProviderInterface.php index 3511429..f489d5c 100644 --- a/core/lib/Drupal/Core/Authentication/AuthenticationProviderInterface.php +++ b/core/lib/Drupal/Core/Authentication/AuthenticationProviderInterface.php @@ -23,15 +23,19 @@ * The request object. * * @return Drupal\Core\Session\AccountInterface|FALSE|NULL - * AccountInterface - in case we authenticated user - * FALSE - in case we had credentials for authentication but user failed - * NULL - no authentication credentials were found in request + * AccountInterface - in case of a successful authentication. + * FALSE - in case we had credentials for authentication but failed. + * NULL - no authentication credentials were found. */ public function authenticate(Request $request); /** * Do cleanup. * + * Allow the authentication provider to clean up before the response is sent. + * This is uses for instance in Drupal\Core\Authentication\Provider\Cookie to + * ensure the session gets committed. + * * @param Request $request * The request object. */ diff --git a/core/lib/Drupal/Core/Authentication/Provider/HttpBasic.php b/core/lib/Drupal/Core/Authentication/Provider/HttpBasic.php index 61284c4..b425f12 100644 --- a/core/lib/Drupal/Core/Authentication/Provider/HttpBasic.php +++ b/core/lib/Drupal/Core/Authentication/Provider/HttpBasic.php @@ -26,8 +26,9 @@ public function authenticate(Request $request) { if ($uid) { return user_load($uid); } - return NULL; + return FALSE; } + return NULL; } /** diff --git a/core/lib/Drupal/Core/CoreBundle.php b/core/lib/Drupal/Core/CoreBundle.php index 013b4a0..b5aaa93 100644 --- a/core/lib/Drupal/Core/CoreBundle.php +++ b/core/lib/Drupal/Core/CoreBundle.php @@ -68,7 +68,7 @@ public function build(ContainerBuilder $container) { // Add the compiler pass that will process the tagged breadcrumb builder // services. $container->addCompilerPass(new RegisterBreadcrumbBuilderPass()); - // Addd the compiler pass that will process tagged authentication services. + // Add the compiler pass that will process tagged authentication services. $container->addCompilerPass(new RegisterAuthenticationPass()); } diff --git a/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterAuthenticationPass.php b/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterAuthenticationPass.php index cda687a..c66a03a 100644 --- a/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterAuthenticationPass.php +++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterAuthenticationPass.php @@ -17,13 +17,21 @@ class RegisterAuthenticationPass implements CompilerPassInterface { /** - * {@inheritdoc} + * Adds authentication providers to the authentication manager. + * + * Check for services tagged with 'authentication_provider' and add them to + * the authentication manager. + * + * @see Drupal\Core\Authentication\AuthenticationManager + * @see Drupal\Core\Authentication\AuthenticationProviderInterface */ public function process(ContainerBuilder $container) { if (!$container->hasDefinition('authentication')) { return; } + // Get the authentication manager. $matcher = $container->getDefinition('authentication'); + // Iterate all autentication providers and add them to the manager. foreach ($container->findTaggedServiceIds('authentication_provider') as $id => $attributes) { $priority = isset($attributes[0]['priority']) ? $attributes[0]['priority'] : 0; $matcher->addMethodCall('addProvider', array( diff --git a/core/lib/Drupal/Core/EventSubscriber/AuthenticationSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/AuthenticationSubscriber.php index 073a6dd..8feb2ee 100644 --- a/core/lib/Drupal/Core/EventSubscriber/AuthenticationSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/AuthenticationSubscriber.php @@ -16,6 +16,8 @@ /** * Authentication subscriber. + * + * Trigger authentication and cleanup during the request. */ class AuthenticationSubscriber implements EventSubscriberInterface { @@ -30,13 +32,16 @@ class AuthenticationSubscriber implements EventSubscriberInterface { * Keep authentication manager as private variable. * * @param AuthenticationProviderInterface $authentication_manager + * The authentication manager. */ public function __construct(AuthenticationProviderInterface $authentication_manager) { $this->authenticationManager = $authentication_manager; } /** - * Authenticate user. + * Authenticates user on request. + * + * @see Drupal\Core\Authentication\AuthenticationProviderInterface::authenticate() */ public function onKernelRequestAuthenticate(GetResponseEvent $event) { if ($event->getRequestType() == HttpKernelInterface::MASTER_REQUEST) { @@ -46,7 +51,9 @@ public function onKernelRequestAuthenticate(GetResponseEvent $event) { } /** - * Trigger clean up. + * Triggers authentication clean up on response. + * + * @see Drupal\Core\Authentication\AuthenticationProviderInterface::cleanup() */ public function onRespond(FilterResponseEvent $event) { if ($event->getRequestType() == HttpKernelInterface::MASTER_REQUEST) { @@ -57,10 +64,12 @@ public function onRespond(FilterResponseEvent $event) { } /** - * Registers the methods in this class that should be listeners. + * {@inheritdoc} * - * @return array - * An array of event listener definitions. + * The priority for request must be higher than the highest event subscriber + * accessing the global $user. + * The priority for the response must be as low as possible allowing e.g the + * Cookie provider to send all relevant session data to the user. */ public static function getSubscribedEvents() { // Priority must be higher than LanguageRequestSubscriber as LanguageManager diff --git a/core/modules/system/lib/Drupal/system/Tests/Authentication/HttpBasicTest.php b/core/modules/system/lib/Drupal/system/Tests/Authentication/HttpBasicTest.php index 1e9a652..4c2e875 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Authentication/HttpBasicTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Authentication/HttpBasicTest.php @@ -11,8 +11,16 @@ use Drupal\simpletest\WebTestBase; use Symfony\Component\HttpFoundation\Request; +/** + * Test for http basic authentication. + */ class HttpBasicTest extends WebTestBase { + /** + * Modules enabled for all tests. + * + * @var array + */ public static $modules = array('router_test'); public static function getInfo() { @@ -31,18 +39,12 @@ public function testHttpBasic() { $this->basicAuthGet('router_test/test11', $account->name, $account->pass_raw); $this->assertText($account->name, 'Account name is displayed.'); + $this->assertResponse('200', 'HTTP response is OK'); $this->curlClose(); $this->basicAuthGet('router_test/test11', $account->name, $this->randomName()); $this->assertNoText($account->name, 'Bad basic auth credentials do not authenticate the user.'); - $this->curlClose(); - - // Login to have a valid session. - $this->drupalLogin($account); - // This call should have both session cookies and basic authentication. So - // it should fail. - $this->basicAuthGet('router_test/test11', $account->name, $account->pass_raw); - $this->assertResponse('400', 'Multiple authentication methods in request lead to 400.'); + $this->assertResponse('200', 'HTTP response is OK'); $this->curlClose(); $this->drupalGet('router_test/test11'); @@ -50,7 +52,7 @@ public function testHttpBasic() { } /** - * Do HTTP basic auth request. + * Dos HTTP basic auth request. * * We do not use drupalGet because we need to set curl settings for basic * authentication. diff --git a/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/TestContent.php b/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/TestContent.php index f7deee8..22a07cf 100644 --- a/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/TestContent.php +++ b/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/TestContent.php @@ -21,10 +21,13 @@ public function test1() { /** * Provides example content for route specific authentication. + * + * @returns string + * The user name of the current logged in user. */ public function test11() { global $user; - return $user->name; + return isset($user->name) ? $user->name : ''; } }