Commit abcfdc17 authored by catch's avatar catch

Issue #2294571 by znerol, effulgentsia: Redirect anonymous users to login page...

Issue #2294571 by znerol, effulgentsia: Redirect anonymous users to login page from an exception listener instead of in MaintenanceModeSubscriber and restrict access to the my-account link to authenticated users.
parent 2acc93b9
......@@ -38,9 +38,6 @@ public function __construct(ConfigFactoryInterface $config) {
public function processInbound($path, Request $request) {
if (empty($path)) {
$path = $this->config->get('system.site')->get('page.front');
if (empty($path)) {
$path = 'user';
}
}
return $path;
}
......
......@@ -273,7 +273,7 @@ function testSchemaData() {
$property = $meta->get('page')->get('front');
$this->assertTrue($property instanceof StringInterface, 'Got the right wrapper fo the page.front property.');
$this->assertEqual($property->getValue(), 'user', 'Got the right value for page.front data.');
$this->assertEqual($property->getValue(), 'user/login', 'Got the right value for page.front data.');
$definition = $property->getDataDefinition();
$this->assertTrue(empty($definition['translatable']), 'Got the right translatability setting for page.front data.');
......@@ -281,13 +281,13 @@ function testSchemaData() {
$list = $meta->get('page');
$this->assertEqual(count($list), 3, 'Got a list with the right number of properties for site page data');
$this->assertTrue(isset($list['front']) && isset($list['403']) && isset($list['404']), 'Got a list with the right properties for site page data.');
$this->assertEqual($list['front']->getValue(), 'user', 'Got the right value for page.front data from the list.');
$this->assertEqual($list['front']->getValue(), 'user/login', 'Got the right value for page.front data from the list.');
// And test some ComplexDataInterface methods.
$properties = $list->getProperties();
$this->assertTrue(count($properties) == 3 && $properties['front'] == $list['front'], 'Got the right properties for site page.');
$values = $list->toArray();
$this->assertTrue(count($values) == 3 && $values['front'] == 'user', 'Got the right property values for site page.');
$this->assertTrue(count($values) == 3 && $values['front'] == 'user/login', 'Got the right property values for site page.');
// Now let's try something more complex, with nested objects.
$wrapper = \Drupal::service('config.typed')->get('image.style.large');
......
......@@ -64,7 +64,7 @@ public function testInternalBrowser() {
'name' => $user->getUsername(),
'pass' => $user->pass_raw
);
$this->drupalPostForm('user', $edit, t('Log in'), array(
$this->drupalPostForm('user/login', $edit, t('Log in'), array(
'query' => array('destination' => 'user/logout'),
));
$headers = $this->drupalGetHeaders(TRUE);
......@@ -86,9 +86,13 @@ public function testInternalBrowser() {
*/
public function testUserAgentValidation() {
global $base_url;
// Logout the user which was logged in during test-setup.
$this->drupalLogout();
$system_path = $base_url . '/' . drupal_get_path('module', 'system');
$HTTP_path = $system_path .'/tests/http.php?q=node';
$https_path = $system_path .'/tests/https.php?q=node';
$HTTP_path = $system_path .'/tests/http.php/user/login';
$https_path = $system_path .'/tests/https.php/user/login';
// Generate a valid simpletest User-Agent to pass validation.
$this->assertTrue(preg_match('/simpletest\d+/', $this->databasePrefix, $matches), 'Database prefix contains simpletest prefix.');
$test_ua = drupal_generate_test_ua($matches[0]);
......
......@@ -673,7 +673,7 @@ protected function drupalLogin(AccountInterface $account) {
'name' => $account->getUsername(),
'pass' => $account->pass_raw
);
$this->drupalPostForm('user', $edit, t('Log in'));
$this->drupalPostForm('user/login', $edit, t('Log in'));
// @see WebTestBase::drupalUserIsLoggedIn()
if (isset($this->session_id)) {
......@@ -710,7 +710,7 @@ protected function drupalLogout() {
// Make a request to the logout page, and redirect to the user page, the
// idea being if you were properly logged out you should be seeing a login
// screen.
$this->drupalGet('user/logout', array('query' => array('destination' => 'user')));
$this->drupalGet('user/logout', array('query' => array('destination' => 'user/login')));
$this->assertResponse(200, 'User was logged out.');
$pass = $this->assertField('name', 'Username field found.', 'Logout');
$pass = $pass && $this->assertField('pass', 'Password field found.', 'Logout');
......
......@@ -5,7 +5,7 @@ slogan: ''
page:
403: ''
404: ''
front: user
front: user/login
admin_compact_mode: false
weight_select_max: 100
langcode: en
......@@ -107,7 +107,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
'#title' => t('Front page'),
'#open' => TRUE,
);
$front_page = $site_config->get('page.front') != 'user' ? $this->aliasManager->getAliasByPath($site_config->get('page.front')) : '';
$front_page = $site_config->get('page.front') != 'user/login' ? $this->aliasManager->getAliasByPath($site_config->get('page.front')) : '';
$form['front_page']['site_frontpage'] = array(
'#type' => 'textfield',
'#title' => t('Default front page'),
......@@ -147,8 +147,8 @@ public function buildForm(array $form, FormStateInterface $form_state) {
public function validateForm(array &$form, FormStateInterface $form_state) {
// Check for empty front page path.
if ($form_state->isValueEmpty('site_frontpage')) {
// Set to default "user".
form_set_value($form['front_page']['site_frontpage'], 'user', $form_state);
// Set to default "user/login".
form_set_value($form['front_page']['site_frontpage'], 'user/login', $form_state);
}
else {
// Get the normal path of the front page.
......
......@@ -46,17 +46,17 @@ protected function testHttpsSession() {
// Test HTTPS session handling by altering the form action to submit the
// login form through https.php, which creates a mock HTTPS request.
$this->drupalGet('user');
$this->drupalGet('user/login');
$form = $this->xpath('//form[@id="user-login-form"]');
$form[0]['action'] = $this->httpsUrl('user');
$form[0]['action'] = $this->httpsUrl('user/login');
$edit = array('name' => $user->getUsername(), 'pass' => $user->pass_raw);
$this->drupalPostForm(NULL, $edit, t('Log in'));
// Test a second concurrent session.
$this->curlClose();
$this->drupalGet('user');
$this->drupalGet('user/login');
$form = $this->xpath('//form[@id="user-login-form"]');
$form[0]['action'] = $this->httpsUrl('user');
$form[0]['action'] = $this->httpsUrl('user/login');
$this->drupalPostForm(NULL, $edit, t('Log in'));
// Check secure cookie on secure page.
......@@ -89,9 +89,9 @@ protected function testHttpsSession() {
// login form through http.php, which creates a mock HTTP request on HTTPS
// test environments.
$this->curlClose();
$this->drupalGet('user');
$this->drupalGet('user/login');
$form = $this->xpath('//form[@id="user-login-form"]');
$form[0]['action'] = $this->httpUrl('user');
$form[0]['action'] = $this->httpUrl('user/login');
$edit = array('name' => $user->getUsername(), 'pass' => $user->pass_raw);
$this->drupalPostForm(NULL, $edit, t('Log in'));
$this->drupalGet($this->httpUrl('admin/config'));
......@@ -150,13 +150,13 @@ protected function testMixedModeSslSession() {
$this->drupalGet('user/password');
$form = $this->xpath('//form[@id="user-pass"]');
$this->assertNotEqual(substr($form[0]['action'], 0, 6), 'https:', 'Password request form action is not secure');
$form[0]['action'] = $this->httpsUrl('user');
$form[0]['action'] = $this->httpsUrl('user/login');
// Check that user login form action is secure.
$this->drupalGet('user');
$this->drupalGet('user/login');
$form = $this->xpath('//form[@id="user-login-form"]');
$this->assertEqual(substr($form[0]['action'], 0, 6), 'https:', 'Login form action is secure');
$form[0]['action'] = $this->httpsUrl('user');
$form[0]['action'] = $this->httpsUrl('user/login');
$edit = array(
'name' => $user->getUsername(),
......@@ -213,9 +213,9 @@ protected function testMixedModeSslSession() {
$this->drupalGet($this->httpsUrl('session-test/set/1'));
// Mock a login to the secure site using the secure session cookie.
$this->drupalGet('user');
$this->drupalGet('user/login');
$form = $this->xpath('//form[@id="user-login-form"]');
$form[0]['action'] = $this->httpsUrl('user');
$form[0]['action'] = $this->httpsUrl('user/login');
$this->drupalPostForm(NULL, $edit, t('Log in'));
// Test that the user is also authenticated on the insecure site.
......@@ -248,9 +248,9 @@ protected function testCsrfTokenWithMixedModeSsl() {
$user = $this->drupalCreateUser(array('access administration pages'));
// Login using the HTTPS user-login form.
$this->drupalGet('user');
$this->drupalGet('user/login');
$form = $this->xpath('//form[@id="user-login-form"]');
$form[0]['action'] = $this->httpsUrl('user');
$form[0]['action'] = $this->httpsUrl('user/login');
$edit = array('name' => $user->getUsername(), 'pass' => $user->pass_raw);
$this->drupalPostForm(NULL, $edit, t('Log in'));
......@@ -318,7 +318,7 @@ protected function assertSessionIds($sid, $ssid, $assertion_text) {
* Builds a URL for submitting a mock HTTPS request to HTTP test environments.
*
* @param $url
* A Drupal path such as 'user'.
* A Drupal path such as 'user/login'.
*
* @return
* An absolute URL.
......@@ -333,7 +333,7 @@ protected function httpsUrl($url) {
* Builds a URL for submitting a mock HTTP request to HTTPS test environments.
*
* @param $url
* A Drupal path such as 'user'.
* A Drupal path such as 'user/login'.
*
* @return
* An absolute URL.
......
......@@ -61,7 +61,7 @@ function testSessionSaveRegenerate() {
'name' => $user->getUsername(),
'pass' => $user->pass_raw
);
$this->drupalPostForm('user', $edit, t('Log in'));
$this->drupalPostForm('user/login', $edit, t('Log in'));
$this->drupalGet('user');
$pass = $this->assertText($user->getUsername(), format_string('Found name: %name', array('%name' => $user->getUsername())), 'User login');
$this->_logged_in = $pass;
......
......@@ -70,9 +70,9 @@ function testAccessDenied() {
$this->assertResponse(403);
$this->assertText(t('Username'), 'Blocks are shown on the default 403 page');
// Log back in, set the custom 403 page to /user and remove the block
// Log back in, set the custom 403 page to /user/login and remove the block
$this->drupalLogin($this->admin_user);
\Drupal::config('system.site')->set('page.403', 'user')->save();
\Drupal::config('system.site')->set('page.403', 'user/login')->save();
$edit = array(
'region' => -1,
);
......
......@@ -149,8 +149,9 @@ protected function getTopLevelMenuLinks() {
* Test compact mode.
*/
function testCompactMode() {
// The front page defaults to 'user', which redirects to 'user/{user}'. We
// cannot use '<front>', since this does not match the redirected url.
// The front page defaults to 'user/login', which redirects to 'user/{user}'
// for authenticated users. We cannot use '<front>', since this does not
// match the redirected url.
$frontpage_url = 'user/' . $this->admin_user->id();
$this->drupalGet('admin/compact/on');
......
......@@ -126,25 +126,17 @@ public function resetPass($uid, $timestamp, $hash) {
}
/**
* Returns the user page.
* Redirects users to their profile page.
*
* Displays user profile if user is logged in, or login form for anonymous
* users.
* This controller assumes that it is only invoked for authenticated users.
* This is enforced for the 'user.page' route with the '_user_is_logged_in'
* requirement.
*
* @return \Symfony\Component\HttpFoundation\RedirectResponse|array
* Returns either a redirect to the user page or the render
* array of the login form.
* @return \Symfony\Component\HttpFoundation\RedirectResponse
* Returns a redirect to the profile of the currently logged in user.
*/
public function userPage() {
$user = $this->currentUser();
if ($user->id()) {
$response = $this->redirect('entity.user.canonical', array('user' => $user->id()));
}
else {
$form_builder = $this->formBuilder();
$response = $form_builder->getForm('Drupal\user\Form\UserLoginForm');
}
return $response;
return $this->redirect('entity.user.canonical', array('user' => $this->currentUser()->id()));
}
/**
......
<?php
/**
* @file
* Contains \Drupal\user\EventSubscriber\AccessDeniedSubscriber.
*/
namespace Drupal\user\EventSubscriber;
use Drupal\Core\Session\AccountInterface;
use Drupal\Core\Routing\RouteMatch;
use Drupal\Core\Routing\UrlGeneratorTrait;
use Drupal\Core\Routing\UrlGeneratorInterface;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
use Symfony\Component\HttpKernel\KernelEvents;
/**
* Redirects anonymous users from user.page to user.login.
*/
class AccessDeniedSubscriber implements EventSubscriberInterface {
use UrlGeneratorTrait;
/**
* The current user.
*
* @var \Drupal\Core\Session\AccountInterface
*/
protected $account;
/**
* Constructs a new redirect subscriber.
*
* @param \Drupal\Core\Session\AccountInterface $account
* The current user.
* @param \Drupal\Core\Routing\UrlGeneratorInterface $url_generator
* The URL generator.
*/
public function __construct(AccountInterface $account, URLGeneratorInterface $url_generator) {
$this->account = $account;
$this->setUrlGenerator($url_generator);
}
/**
* Redirects anonymous users from user.page to user.login.
*
* @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event
* The event to process.
*/
public function onException(GetResponseForExceptionEvent $event) {
$exception = $event->getException();
if ($exception instanceof AccessDeniedHttpException) {
$route_name = RouteMatch::createFromRequest($event->getRequest())->getRouteName();
if ($route_name == 'user.page' && !$this->account->isAuthenticated()) {
$event->setResponse($this->redirect('user.login'));
}
}
}
/**
* {@inheritdoc}
*/
public static function getSubscribedEvents() {
// Use a higher priority than
// \Drupal\Core\EventSubscriber\ExceptionLoggingSubscriber, because there's
// no need to log the exception if we can redirect.
$events[KernelEvents::EXCEPTION][] = ['onException', 75];
return $events;
}
}
......@@ -68,21 +68,15 @@ public function onKernelRequestMaintenance(GetResponseEvent $event) {
$event->setResponse(new RedirectResponse($this->url('<front>', [], ['absolute' => TRUE])));
return;
}
if ($this->account->isAnonymous() && $path == 'user') {
// 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])));
// If the user is already logged in, redirect to their profile page.
$event->setResponse($this->redirect('entity.user.canonical', ['user' => $this->account->id()]));
return;
}
if ($path == 'user/register') {
// Authenticated user should be redirected to user edit page.
// If the user is already registered, redirect to their edit page.
$event->setResponse(new RedirectResponse($this->url('entity.user.edit_form', ['user' => $this->account->id()], ['absolute' => TRUE])));
return;
}
......
<?php
/**
* @file
* Contains \Drupal\user\Plugin\Menu\MyAccountMenuLink.
*/
namespace Drupal\user\Plugin\Menu;
use Drupal\Core\Menu\MenuLinkDefault;
/**
* Provides custom logic for the user.page menu link.
*/
class MyAccountMenuLink extends MenuLinkDefault {
/**
* {@inheritdoc}
*/
public function isEnabled() {
// The path 'user' must be accessible for anonymous users, but only visible
// for authenticated users. Authenticated users should see "My account", but
// anonymous users should not see it at all.
// @todo Re-write this as a link to entity.user.canonical with dynamic route parameters
// to affect access since hidden should not be dynamic.
// https://www.drupal.org/node/2306991
return $this->pluginDefinition['enabled'] && !\Drupal::currentUser()->isAnonymous();
}
/**
* {@inheritdoc}
*/
public function isCacheable() {
return FALSE;
}
}
......@@ -72,9 +72,7 @@ function testSecondaryMenu() {
array('callable' => 'menu.default_tree_manipulators:checkAccess'),
);
$tree = $menu_tree->transform($tree, $manipulators);
$this->assertEqual(count($tree), 1, 'The secondary links menu contains only one menu link.');
$element = reset($tree);
$this->assertFalse($element->link->isEnabled(), 'The menu link is disabled.');
$this->assertEqual(count($tree), 0, 'The secondary links menu contains no menu link.');
}
/**
......
......@@ -21,7 +21,7 @@ class UserLoginTest extends WebTestBase {
*/
function testLoginDestination() {
$user = $this->drupalCreateUser(array());
$this->drupalGet('user', array('query' => array('destination' => 'foo')));
$this->drupalGet('user/login', array('query' => array('destination' => 'foo')));
$edit = array('name' => $user->getUserName(), 'pass' => $user->pass_raw);
$this->drupalPostForm(NULL, $edit, t('Log in'));
$this->assertUrl('foo', [], 'Redirected to the correct URL');
......@@ -151,7 +151,7 @@ function assertFailedLogin($account, $flood_trigger = NULL) {
'name' => $account->getUsername(),
'pass' => $account->pass_raw,
);
$this->drupalPostForm('user', $edit, t('Log in'));
$this->drupalPostForm('user/login', $edit, t('Log in'));
$this->assertNoFieldByXPath("//input[@name='pass' and @value!='']", NULL, 'Password value attribute is blank.');
if (isset($flood_trigger)) {
if ($flood_trigger == 'user') {
......
......@@ -149,7 +149,7 @@ public function testUserResetPasswordTextboxFilled() {
'name' => $this->randomMachineName(),
'pass' => $this->randomMachineName(),
);
$this->drupalPostForm('user', $edit, t('Log in'));
$this->drupalPostForm('user/login', $edit, t('Log in'));
$this->assertRaw(t('Sorry, unrecognized username or password. <a href="@password">Have you forgotten your password?</a>',
array('@password' => \Drupal::url('user.pass', [], array('query' => array('name' => $edit['name']))))));
unset($edit['pass']);
......
......@@ -3,7 +3,6 @@ user.page:
weight: -10
route_name: user.page
menu_name: account
class: Drupal\user\Plugin\Menu\MyAccountMenuLink
user.logout:
title: 'Log out'
route_name: user.logout
......
......@@ -129,9 +129,9 @@ user.page:
path: '/user'
defaults:
_content: '\Drupal\user\Controller\UserController::userPage'
_title: 'Log in'
_title: 'My account'
requirements:
_access: 'TRUE'
_user_is_logged_in: 'TRUE'
entity.user.canonical:
path: '/user/{user}'
......
......@@ -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']
......
......@@ -108,7 +108,7 @@ function testProcessInbound() {
// Passing in anything else should return the same string.
array('fr/foo', NULL, 'fr/foo'),
array('fr', NULL, 'fr'),
array('user', NULL, 'user'),
array('user/login', NULL, 'user/login'),
);
$alias_manager->expects($this->any())
......@@ -120,7 +120,7 @@ function testProcessInbound() {
$config_factory_stub = $this->getConfigFactoryStub(
array(
'system.site' => array(
'page.front' => 'user'
'page.front' => 'user/login'
),
'language.negotiation' => array(
'url' => array(
......@@ -198,7 +198,7 @@ function testProcessInbound() {
$test_path = 'fr';
$request = Request::create($test_path);
$processed = $processor_manager->processInbound($test_path, $request);
$this->assertEquals('user', $processed, 'Processing in the correct order resolves the system path from the empty path.');
$this->assertEquals('user/login', $processed, 'Processing in the correct order resolves the system path from the empty path.');
// Test resolving an existing alias using the correct processor order.
$test_path = 'fr/foo';
......
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