Commit 28bf41d5 authored by Kurucz István's avatar Kurucz István
Browse files

Issue #3301680 by nevergone: Better internal and valid path detection

parent d2f1171d
Loading
Loading
Loading
Loading
+1 −0
Original line number Diff line number Diff line
@@ -7,6 +7,7 @@ services:
      - '@messenger'
      - '@redirect.destination'
      - '@request_stack'
      - '@router.request_context'
      - '@token'
    tags:
      - { name: 'event_subscriber' }
+16 −3
Original line number Diff line number Diff line
@@ -7,6 +7,7 @@ use Drupal\Component\Utility\Xss;
use Drupal\Core\Config\ConfigFactory;
use Drupal\Core\Messenger\MessengerInterface;
use Drupal\Core\Routing\RedirectDestinationInterface;
use Drupal\Core\Routing\RequestContext;
use Drupal\Core\Session\AccountProxyInterface;
use Drupal\Core\Url;
use Drupal\Core\Utility\Token;
@@ -55,10 +56,17 @@ class RedirectAfterLogoutSubscriber implements EventSubscriberInterface {
  /**
   * The request stack.
   *
   * @var \Symfony\Component\HttpFoundation\RequestStack
   * @var \Drupal\Core\Http\RequestStack
   */
  protected $requestStack;

  /**
   * The request context.
   *
   * @var \Drupal\Core\Routing\RequestContext
   */
  protected $requestContext;

  /**
   * The token service.
   *
@@ -77,8 +85,10 @@ class RedirectAfterLogoutSubscriber implements EventSubscriberInterface {
   *   The messenger.
   * @param RedirectDestinationInterface $redirectDestination
   *   The redirect destination.
   * @param \Symfony\Component\HttpFoundation\RequestStack $request_stack
   * @param \Drupal\Core\Http\RequestStack $request_stack
   *   The request stack.
   * @param \Drupal\Core\Routing\RequestContext $requestContext
   *   The request context.
   * @param \Drupal\Core\Utility\Token $token
   *   The token service.
   */
@@ -87,12 +97,14 @@ class RedirectAfterLogoutSubscriber implements EventSubscriberInterface {
                              MessengerInterface $messenger,
                              RedirectDestinationInterface $redirectDestination,
                              RequestStack $request_stack,
                              RequestContext $requestContext,
                              Token $token) {
    $this->configFactory = $configFactory;
    $this->currentUser = $currentUser;
    $this->messenger = $messenger;
    $this->redirectDestination = $redirectDestination;
    $this->requestStack = $request_stack;
    $this->requestContext = $requestContext;
    $this->token = $token;
  }

@@ -121,7 +133,8 @@ class RedirectAfterLogoutSubscriber implements EventSubscriberInterface {

    $config = $this->configFactory->get('redirect_after_logout.settings');
    $logout_message = $config->get('message');
    if ($logout_message === '' || $destination->isExternal()) {
    $base_url = $this->requestContext->getCompleteBaseUrl();
    if ($logout_message === '' || ($destination->isExternal() && !UrlHelper::externalIsLocal($destination->toString(), $base_url))) {
      $destination = $destination->toString();
    }
    else {
+5 −4
Original line number Diff line number Diff line
@@ -152,6 +152,11 @@ class RedirectLogoutSettings extends ConfigFormBase {
    if ($destination === '<front>') {
      return;
    }
    if (strlen($destination) > 1 && $destination[0] === '/' && $destination[1] === '[') {
      // Token with left slash: remove left slash.
      $destination = substr($destination, 1);
      $form_state->setValue('redirect_after_logout_destination', $destination);
    }
    $tokenized_destination = UrlHelper::stripDangerousProtocols($this->tokenService->replace($destination));
    if (UrlHelper::isExternal($tokenized_destination) && !UrlHelper::isValid($tokenized_destination, TRUE)) {
      // Invalid URL.
@@ -170,10 +175,6 @@ class RedirectLogoutSettings extends ConfigFormBase {
    if ($tokenized_destination[0] !== '/') {
      $form_state->setErrorByName('redirect_after_logout_destination', $this->t("The path '%path' has to start with a slash.", ['%path' => $destination]));
    }
    else {
      // Remove unnecessary slashes from beginning tokenized destination.
      $tokenized_destination = '/' . ltrim($tokenized_destination, '/');
    }
    if (UrlHelper::isExternal($tokenized_destination)) {
      $valid = UrlHelper::isValid($tokenized_destination, TRUE);
    }
+6 −0
Original line number Diff line number Diff line
@@ -44,6 +44,12 @@ class RedirectTest extends TestBase {
    // Path with token.
    $this->setRedirectConfig('/foobar-[site:name]', $message);
    $this->logoutRedirectHelper($this->regularUser, '/foobar-example', $message);
    // Path only token.
    $this->setRedirectConfig('[current-user:url]', $message);
    $this->logoutRedirectHelper($this->regularUser, $this->regularUser->toUrl()->toString(), $message);
    // Token with left slash: /[current-user:url] transform to [current-user:url] destination.
    $this->setRedirectConfig('/[current-user:url]', $message, NULL, '[current-user:url]');
    $this->logoutRedirectHelper($this->regularUser, $this->regularUser->toUrl()->toString(), $message);
  }

}
+14 −7
Original line number Diff line number Diff line
@@ -10,6 +10,7 @@ use Drupal\Tests\Traits\Core\PathAliasTestTrait;
use Drupal\user\Entity\Role;
use Drupal\user\Entity\User;
use Drupal\user\RoleInterface;
use Drupal\user\UserInterface;

/**
 * Redirect after logout test base.
@@ -177,14 +178,22 @@ abstract class TestBase extends BrowserTestBase {
   *   Redirect path.
   * @param string $message
   *   Redirect message.
   * @param \Drupal\user\Entity\User|null $account
   *   Administrator user account.
   * @param string|null $given_destination
   *   Given destination path.
   *   Use it if you need to change destination, after submit settings form.
   */
  protected function setRedirectConfig($destination, $message = '', $account = NULL) {
    if ($account !== NULL) {
  protected function setRedirectConfig(string $destination, string $message = '', User $account = NULL, string $given_destination = NULL) {
    if ($account instanceof UserInterface) {
      $this->drupalLogin($account);
    }
    else {
      $this->drupalLogin($this->adminUser);
    }
    if ($given_destination === NULL) {
      $given_destination = $destination;
    }
    $this->drupalGet('admin/config/system/redirect_after_logout');
    $edit = [];
    if ($destination !== '') {
@@ -201,11 +210,9 @@ abstract class TestBase extends BrowserTestBase {
    $this->assertSession()
      ->addressEquals('admin/config/system/redirect_after_logout');
    $config = $this->config('redirect_after_logout.settings');
    if ($destination !== '') {
    $this->assertSession()
        ->fieldValueEquals('edit-redirect-after-logout-destination', $destination);
      $this::assertEquals($destination, $config->get('destination'));
    }
      ->fieldValueEquals('edit-redirect-after-logout-destination', $given_destination);
    $this::assertEquals($given_destination, $config->get('destination'));
    if ($message !== '') {
      $this->assertSession()
        ->fieldValueEquals('edit-redirect-after-logout-message', $message);