diff --git a/core/core.services.yml b/core/core.services.yml index 7abab67..0d9907d 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -182,7 +182,7 @@ services: arguments: ['@request_stack', '@url_generator'] form_cache: class: Drupal\Core\Form\FormCache - arguments: ['@keyvalue.expirable', '@module_handler', '@current_user', '@csrf_token'] + arguments: ['@keyvalue.expirable', '@module_handler', '@current_user', '@csrf_token', '@session_manager'] public: false # Private to form_builder keyvalue: class: Drupal\Core\KeyValueStore\KeyValueFactory diff --git a/core/lib/Drupal/Core/Authentication/AuthenticationManager.php b/core/lib/Drupal/Core/Authentication/AuthenticationManager.php index 0553d09..8047eba 100644 --- a/core/lib/Drupal/Core/Authentication/AuthenticationManager.php +++ b/core/lib/Drupal/Core/Authentication/AuthenticationManager.php @@ -7,9 +7,9 @@ namespace Drupal\Core\Authentication; +use Drupal\Core\Routing\RouteMatch; use Drupal\Core\Session\AnonymousUserSession; use Symfony\Component\HttpFoundation\Request; -use Symfony\Cmf\Component\Routing\RouteObjectInterface; use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; /** @@ -89,8 +89,8 @@ public function authenticate(Request $request) { $account = NULL; - // Iterate the available providers. - foreach ($this->getSortedProviders() as $provider_id => $provider) { + // Iterate the allowed providers. + foreach ($this->filterProviders($this->getSortedProviders(), $request) as $provider_id => $provider) { if ($provider->applies($request)) { // Try to authenticate with this provider, skipping all others. $account = $provider->authenticate($request); @@ -156,6 +156,36 @@ public function getSortedProviders() { } /** + * Filters a list of providers and only return those allowed on the request. + * + * @param \Drupal\Core\Authentication\AuthenticationProviderInterface[] $providers + * An array of authentication provider objects. + * @param Request $request + * The request object. + * + * @return \Drupal\Core\Authentication\AuthenticationProviderInterface[] + * The filtered array authentication provider objects. + */ + protected function filterProviders(array $providers, Request $request) { + $route = RouteMatch::createFromRequest($request)->getRouteObject(); + $allowed_providers = array(); + if ($route && $route->hasOption('_auth')) { + $allowed_providers = $route->getOption('_auth'); + } + elseif ($default_provider = $this->defaultProviderId()) { + // @todo Mirrors the defective behavior of AuthenticationEnhancer and + // restricts the list of allowed providers to the default provider if no + // _auth was specified on the current route. + // + // This restriction will be removed by https://www.drupal.org/node/2286971 + // See also https://www.drupal.org/node/2283637 + $allowed_providers = array($default_provider); + } + + return array_intersect_key($providers, array_flip($allowed_providers)); + } + + /** * Cleans up the authentication. * * Allow the triggered provider to clean up before the response is sent, e.g. @@ -177,18 +207,11 @@ public function cleanup(Request $request) { * {@inheritdoc} */ public function handleException(GetResponseForExceptionEvent $event) { - $request = $event->getRequest(); - - $route = $request->attributes->get(RouteObjectInterface::ROUTE_OBJECT); - $active_providers = ($route && $route->getOption('_auth')) ? $route->getOption('_auth') : array($this->defaultProviderId()); - - // Get the sorted list of active providers for the given route. - $providers = array_intersect($active_providers, array_keys($this->getSortedProviders())); - - foreach ($providers as $provider_id) { - if ($this->providers[$provider_id]->handleException($event) == TRUE) { + foreach ($this->filterProviders($this->getSortedProviders(), $event->getRequest()) as $provider) { + if ($provider->handleException($event) == TRUE) { break; } } } + } diff --git a/core/lib/Drupal/Core/Form/FormCache.php b/core/lib/Drupal/Core/Form/FormCache.php index 93da48c..669771b 100644 --- a/core/lib/Drupal/Core/Form/FormCache.php +++ b/core/lib/Drupal/Core/Form/FormCache.php @@ -12,6 +12,7 @@ use Drupal\Core\Extension\ModuleHandlerInterface; use Drupal\Core\KeyValueStore\KeyValueExpirableFactoryInterface; use Drupal\Core\Session\AccountInterface; +use Drupal\Core\Session\SessionManagerInterface; /** * Encapsulates the caching of a form and its form state. @@ -35,6 +36,13 @@ class FormCache implements FormCacheInterface { protected $csrfToken; /** + * The session manager. + * + * @var \Drupal\Core\Session\SessionManagerInterface + */ + protected $sessionManager; + + /** * The current user. * * @var \Drupal\Core\Session\AccountInterface @@ -60,12 +68,15 @@ class FormCache implements FormCacheInterface { * The current user. * @param \Drupal\Core\Access\CsrfTokenGenerator $csrf_token * The CSRF token generator. + * @param \Drupal\Core\Session\SessionManagerInterface $session_manager + * The session manager */ - public function __construct(KeyValueExpirableFactoryInterface $key_value_expirable_factory, ModuleHandlerInterface $module_handler, AccountInterface $current_user, CsrfTokenGenerator $csrf_token = NULL) { + public function __construct(KeyValueExpirableFactoryInterface $key_value_expirable_factory, ModuleHandlerInterface $module_handler, AccountInterface $current_user, CsrfTokenGenerator $csrf_token, SessionManagerInterface $session_manager) { $this->keyValueExpirableFactory = $key_value_expirable_factory; $this->moduleHandler = $module_handler; $this->currentUser = $current_user; $this->csrfToken = $csrf_token; + $this->sessionManager = $session_manager; } /** @@ -73,6 +84,9 @@ public function __construct(KeyValueExpirableFactoryInterface $key_value_expirab */ public function getCache($form_build_id, FormStateInterface $form_state) { if ($form = $this->keyValueExpirableFactory->get('form')->get($form_build_id)) { + // The CSRF token generator requires a session to be started in order to + // optain the token seed from the session. + $this->sessionManager->start(); if ((isset($form['#cache_token']) && $this->csrfToken->validate($form['#cache_token'])) || (!isset($form['#cache_token']) && $this->currentUser->isAnonymous())) { $this->loadCachedFormState($form_build_id, $form_state); return $form; diff --git a/core/lib/Drupal/Core/Form/NullFormCache.php b/core/lib/Drupal/Core/Form/NullFormCache.php new file mode 100644 index 0000000..99a843e --- /dev/null +++ b/core/lib/Drupal/Core/Form/NullFormCache.php @@ -0,0 +1,29 @@ +addArgument(new Reference('request_stack')); $container ->register('router.dumper', 'Drupal\Core\Routing\NullMatcherDumper'); + $container + ->register('form_cache', 'Drupal\Core\Form\NullFormCache'); // Replace the route builder with an empty implementation. // @todo Convert installer steps into routes; add an installer.routing.yml. diff --git a/core/lib/Drupal/Core/Routing/AccessAwareRouter.php b/core/lib/Drupal/Core/Routing/AccessAwareRouter.php index 1cf937c..b3033ff 100644 --- a/core/lib/Drupal/Core/Routing/AccessAwareRouter.php +++ b/core/lib/Drupal/Core/Routing/AccessAwareRouter.php @@ -88,6 +88,10 @@ public function getContext() { public function matchRequest(Request $request) { $parameters = $this->chainRouter->matchRequest($request); $request->attributes->add($parameters); + // Trigger a session start and authentication by accessing any property of + // the current user. + // @todo: This can will be removed in #2229145. + $this->account->id(); $this->checkAccess($request); // We can not return $parameters because the access check can change the // request attributes. diff --git a/core/modules/user/src/Access/LoginStatusCheck.php b/core/modules/user/src/Access/LoginStatusCheck.php index 56790cb..f31e056 100644 --- a/core/modules/user/src/Access/LoginStatusCheck.php +++ b/core/modules/user/src/Access/LoginStatusCheck.php @@ -10,6 +10,7 @@ use Drupal\Core\Access\AccessResult; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; +use Symfony\Component\Routing\Route; /** * Determines access to routes based on login status of current user. @@ -21,12 +22,16 @@ class LoginStatusCheck implements AccessInterface { * * @param \Drupal\Core\Session\AccountInterface $account * The currently logged in account. + * @param \Symfony\Component\Routing\Route $route + * The route to check against. * * @return \Drupal\Core\Access\AccessResultInterface * The access result. */ - public function access(AccountInterface $account) { - return AccessResult::allowedIf($account->isAuthenticated())->cachePerRole(); + public function access(AccountInterface $account, Route $route) { + $required_status = filter_var($route->getRequirement('_user_is_logged_in'), FILTER_VALIDATE_BOOLEAN); + $actual_status = $account->isAuthenticated(); + return AccessResult::allowedIf($required_status === $actual_status)->cachePerRole(); } } diff --git a/core/modules/user/src/EventSubscriber/AccessDeniedSubscriber.php b/core/modules/user/src/EventSubscriber/AccessDeniedSubscriber.php new file mode 100644 index 0000000..61d6bf1 --- /dev/null +++ b/core/modules/user/src/EventSubscriber/AccessDeniedSubscriber.php @@ -0,0 +1,98 @@ +account = $account; + $this->urlGenerator = $url_generator; + } + + /** + * Redirects authenticated users from login/register form to the profile. + * + * @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event + * The event to process. + */ + public function onException(GetResponseForExceptionEvent $event) { + $exception = $event->getException(); + if ($exception instanceof AccessDeniedHttpException && $this->account->isAuthenticated()) { + $route_name = RouteMatch::createFromRequest($event->getRequest())->getRouteName(); + switch ($route_name) { + case 'user.login'; + // Redirect an authenticated user to the profile page. + $event->setResponse(new RedirectResponse($this->url('user.view', array('user' => $this->account->id()), array('absolute' => TRUE)))); + break; + + case 'user.register'; + // Redirect an authenticated user to the profile form. + $event->setResponse(new RedirectResponse($this->url('user.edit', array('user' => $this->account->id()), array('absolute' => TRUE)))); + break; + } + } + } + + /** + * Generates a URL or path for a specific route based on the given parameters. + * + * @see \Drupal\Core\Routing\UrlGeneratorInterface::generateFromRoute() for + * details on the arguments, usage, and possible exceptions. + * + * @return string + * The generated URL for the given route. + * + * @todo: Use UrlGeneratorTrait (see https://www.drupal.org/node/2282161) + */ + protected function url($route_name, $route_parameters = array(), $options = array()) { + return $this->urlGenerator->generateFromRoute($route_name, $route_parameters, $options); + } + + /** + * {@inheritdoc} + */ + public static function getSubscribedEvents() { + $events[KernelEvents::EXCEPTION][] = array('onException'); + return $events; + } + +} diff --git a/core/modules/user/src/EventSubscriber/MaintenanceModeSubscriber.php b/core/modules/user/src/EventSubscriber/MaintenanceModeSubscriber.php index ee5fb4f..51ee0a0 100644 --- a/core/modules/user/src/EventSubscriber/MaintenanceModeSubscriber.php +++ b/core/modules/user/src/EventSubscriber/MaintenanceModeSubscriber.php @@ -51,7 +51,7 @@ public function __construct(MaintenanceModeInterface $maintenance_mode, AccountI } /** - * Determine whether the page is configured to be offline. + * Logout users if site is in maintenance mode. * * @param \Symfony\Component\HttpKernel\Event\GetResponseEvent $event * The event to process. @@ -59,7 +59,7 @@ public function __construct(MaintenanceModeInterface $maintenance_mode, AccountI public function onKernelRequestMaintenance(GetResponseEvent $event) { $request = $event->getRequest(); $route_match = RouteMatch::createFromRequest($request); - $path = $request->attributes->get('_system_path'); + $route_name = $route_match->getRouteName(); if ($this->maintenanceMode->applies($route_match)) { // If the site is offline, log out unprivileged users. if ($this->account->isAuthenticated() && !$this->maintenanceMode->exempt($this->account)) { @@ -69,31 +69,19 @@ public function onKernelRequestMaintenance(GetResponseEvent $event) { return; } - if ($this->account->isAnonymous() && $path == 'user') { + if ($this->account->isAnonymous() && $route_name == 'user.page') { // Forward anonymous user to login page. $event->setResponse(new RedirectResponse($this->url('user.login', [], ['absolute' => TRUE]))); return; } } - if ($this->account->isAuthenticated()) { - if ($path == 'user/login') { - // If user is logged in, redirect to 'user' instead of giving 403. - $event->setResponse(new RedirectResponse($this->url('user.page', [], ['absolute' => TRUE]))); - return; - } - if ($path == 'user/register') { - // Authenticated user should be redirected to user edit page. - $event->setResponse(new RedirectResponse($this->url('entity.user.edit_form', ['user' => $this->account->id()], ['absolute' => TRUE]))); - return; - } - } } /** * {@inheritdoc} */ public static function getSubscribedEvents() { - $events[KernelEvents::REQUEST][] = array('onKernelRequestMaintenance', 35); + $events[KernelEvents::REQUEST][] = array('onKernelRequestMaintenance', 31); return $events; } diff --git a/core/modules/user/src/RegisterForm.php b/core/modules/user/src/RegisterForm.php index be73f39..326adcf 100644 --- a/core/modules/user/src/RegisterForm.php +++ b/core/modules/user/src/RegisterForm.php @@ -42,11 +42,6 @@ public function form(array $form, FormStateInterface $form_state) { '#value' => $admin, ); - // If we aren't admin but already logged on, go to the user page instead. - if (!$admin && $user->isAuthenticated()) { - return new RedirectResponse($this->url('entity.user.canonical', ['user' => \Drupal::currentUser()->id()], array('absolute' => TRUE))); - } - $form['#attached']['library'][] = 'core/drupal.form'; // For non-admin users, populate the form fields using data from the diff --git a/core/modules/user/user.routing.yml b/core/modules/user/user.routing.yml index a036d4b..7ae76f8 100644 --- a/core/modules/user/user.routing.yml +++ b/core/modules/user/user.routing.yml @@ -147,7 +147,7 @@ user.login: _form: '\Drupal\user\Form\UserLoginForm' _title: 'Log in' requirements: - _access: 'TRUE' + _user_is_logged_in: 'FALSE' options: _maintenance_access: TRUE diff --git a/core/modules/user/user.services.yml b/core/modules/user/user.services.yml index 7ffcc65..e89aa73 100644 --- a/core/modules/user/user.services.yml +++ b/core/modules/user/user.services.yml @@ -43,6 +43,11 @@ services: arguments: ['@maintenance_mode', '@current_user'] tags: - { name: event_subscriber } + user_access_denied_subscriber: + class: Drupal\user\EventSubscriber\AccessDeniedSubscriber + arguments: ['@current_user', '@url_generator'] + tags: + - { name: event_subscriber } theme.negotiator.admin_theme: class: Drupal\user\Theme\AdminNegotiator arguments: ['@current_user', '@config.factory', '@entity.manager', '@router.admin_context'] diff --git a/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php b/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php index 9e36fcc..e89812b 100644 --- a/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php +++ b/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php @@ -12,6 +12,7 @@ use Drupal\Core\Form\FormState; use Drupal\Core\Form\FormStateInterface; use Symfony\Component\DependencyInjection\ContainerInterface; +use Symfony\Component\DependencyInjection\IntrospectableContainerInterface; /** * @coversDefaultClass \Drupal\Core\Form\FormBuilder @@ -53,7 +54,7 @@ public function testGetFormIdWithClassName() { * Tests the getFormId() method with an injected class name form ID. */ public function testGetFormIdWithInjectedClassName() { - $container = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface'); + $container = $this->getMock('Symfony\Component\DependencyInjection\IntrospectableContainerInterface'); \Drupal::setContainer($container); $form_arg = 'Drupal\Tests\Core\Form\TestFormInjected'; diff --git a/core/tests/Drupal/Tests/Core/Form/FormCacheTest.php b/core/tests/Drupal/Tests/Core/Form/FormCacheTest.php index d08557a..0d7d6a0 100644 --- a/core/tests/Drupal/Tests/Core/Form/FormCacheTest.php +++ b/core/tests/Drupal/Tests/Core/Form/FormCacheTest.php @@ -48,6 +48,13 @@ class FormCacheTest extends UnitTestCase { protected $csrfToken; /** + * The session manager. + * + * @var \Drupal\Core\Session\SessionManagerInterface|\PHPUnit_Framework_MockObject_MockObject + */ + protected $sessionManager; + + /** * The mocked module handler. * * @var \Drupal\Core\Extension\ModuleHandlerInterface|\PHPUnit_Framework_MockObject_MockObject @@ -91,8 +98,9 @@ protected function setUp() { $this->csrfToken = $this->getMockBuilder('Drupal\Core\Access\CsrfTokenGenerator') ->disableOriginalConstructor() ->getMock(); + $this->sessionManager = $this->getMock('Drupal\Core\Session\SessionManagerInterface'); $this->account = $this->getMock('Drupal\Core\Session\AccountInterface'); - $this->formCache = new FormCache($this->keyValueExpirableFactory, $this->moduleHandler, $this->account, $this->csrfToken); + $this->formCache = new FormCache($this->keyValueExpirableFactory, $this->moduleHandler, $this->account, $this->csrfToken, $this->sessionManager); } /** @@ -116,6 +124,8 @@ public function testGetCacheValidToken() { ->method('get') ->with($form_build_id) ->willReturn($cached_form); + $this->sessionManager->expects($this->once()) + ->method('start'); $this->csrfToken->expects($this->once()) ->method('validate') ->with($cache_token) @@ -140,6 +150,8 @@ public function testGetCacheInvalidToken() { ->method('get') ->with($form_build_id) ->willReturn($cached_form); + $this->sessionManager->expects($this->once()) + ->method('start'); $this->csrfToken->expects($this->once()) ->method('validate') ->with($cache_token) @@ -166,6 +178,8 @@ public function testGetCacheAnonUser() { $this->account->expects($this->once()) ->method('isAnonymous') ->willReturn(TRUE); + $this->sessionManager->expects($this->once()) + ->method('start'); $this->csrfToken->expects($this->never()) ->method('validate');