[BUGFIX] Fix relogin with popup 78/54278/14
authorBenni Mack <benni@typo3.org>
Thu, 10 May 2018 13:46:20 +0000 (15:46 +0200)
committerSusanne Moog <susanne.moog@typo3.org>
Sat, 12 May 2018 10:58:12 +0000 (12:58 +0200)
The re-login popup in BE has some issues:
- An empty frameset is loaded within the popup which is not needed
and makes the code harder to understand (parent.window.opener)
- The popup always shows the logout form, although the relogin should
show the login credentials form (because the view did not get the memo)
- Login.html should not use "../../../../" for referencing
the form

The LoginFrameset controller thus is not used (will be deprecated
in a separate patch).

This patch moves the logic into the LoginController.

Resolves: #83430
Releases: master
Change-Id: If872baf26297c8b75b786c2d8881802b05b1e41a
Reviewed-on: https://review.typo3.org/54278
Reviewed-by: Mathias Schreiber <mathias.schreiber@typo3.com>
Tested-by: Mathias Schreiber <mathias.schreiber@typo3.com>
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Susanne Moog <susanne.moog@typo3.org>
Tested-by: Susanne Moog <susanne.moog@typo3.org>
typo3/sysext/backend/Classes/Controller/LoginController.php
typo3/sysext/backend/Classes/Middleware/BackendUserAuthenticator.php
typo3/sysext/backend/Configuration/Backend/Routes.php
typo3/sysext/backend/Resources/Private/Layouts/Login.html
typo3/sysext/backend/Tests/Unit/Controller/LoginControllerTest.php

index 0ee4f09..3b60c1c 100644 (file)
@@ -140,7 +140,7 @@ class LoginController implements LoggerAwareInterface
         // If "L" is "OUT", then any logged in is logged out. If redirect_url is given, we redirect to it
         if (($parsedBody['L'] ?? $queryParams['L'] ?? null) === 'OUT' && is_object($this->getBackendUserAuthentication())) {
             $this->getBackendUserAuthentication()->logoff();
-            HttpUtility::redirect($this->redirectUrl);
+            $this->redirectToUrl();
         }
 
         $this->view = $this->getFluidTemplateObject();
@@ -159,6 +159,18 @@ class LoginController implements LoggerAwareInterface
     }
 
     /**
+     * Calls the main function but with loginRefresh enabled at any time
+     *
+     * @param ServerRequestInterface $request the current request
+     * @return ResponseInterface the finished response with the content
+     */
+    public function refreshAction(ServerRequestInterface $request): ResponseInterface
+    {
+        $this->loginRefresh = true;
+        return new HtmlResponse($this->createLoginLogoutForm($request));
+    }
+
+    /**
      * Main function - creating the login/logout form
      *
      * @throws Exception
@@ -265,6 +277,7 @@ class LoginController implements LoggerAwareInterface
             ],
             'copyright' => BackendUtility::TYPO3_copyRightNotice(),
             'redirectUrl' => $this->redirectUrl,
+            'loginRefresh' => $this->loginRefresh,
             'loginNewsItems' => $this->getSystemNews(),
             'loginProviderIdentifier' => $this->loginProviderIdentifier,
             'loginProviders' => $this->loginProviders
@@ -300,8 +313,8 @@ class LoginController implements LoggerAwareInterface
     protected function checkRedirect(ServerRequestInterface $request): void
     {
         if (
-            empty($this->getBackendUserAuthentication()->user['uid'])
-            && ($this->isLoginInProgress($request) || !$this->loginRefresh)
+            empty($this->getBackendUserAuthentication()->user['uid']) ||
+            (!($this->isLoginInProgress($request) || $this->loginRefresh))
         ) {
             return;
         }
@@ -311,7 +324,7 @@ class LoginController implements LoggerAwareInterface
          * This assumes that a cookie-setting script (like this one) has been hit at
          * least once prior to this instance.
          */
-        if (!$_COOKIE[BackendUserAuthentication::getCookieName()]) {
+        if (!isset($_COOKIE[BackendUserAuthentication::getCookieName()])) {
             if ($this->submitValue === 'setCookie') {
                 /*
                  * we tried it a second time but still no cookie
@@ -355,14 +368,14 @@ class LoginController implements LoggerAwareInterface
             $formProtection->setSessionTokenFromRegistry();
             $formProtection->persistSessionToken();
             $this->getDocumentTemplate()->JScode .= GeneralUtility::wrapJS('
-                               if (parent.opener && parent.opener.TYPO3 && parent.opener.TYPO3.LoginRefresh) {
-                                       parent.opener.TYPO3.LoginRefresh.startTask();
-                                       parent.close();
+                               if (window.opener && window.opener.TYPO3 && window.opener.TYPO3.LoginRefresh) {
+                                       window.opener.TYPO3.LoginRefresh.startTask();
+                                       window.close();
                                }
                        ');
         } else {
             $formProtection->storeSessionTokenInRegistry();
-            HttpUtility::redirect($this->redirectToURL);
+            $this->redirectToUrl();
         }
     }
 
@@ -495,6 +508,14 @@ class LoginController implements LoggerAwareInterface
     }
 
     /**
+     * Wrapper method to redirect to configured redirect URL
+     */
+    protected function redirectToUrl(): void
+    {
+        HttpUtility::redirect($this->redirectToURL);
+    }
+
+    /**
      * Validates the registered login providers
      *
      * @throws \RuntimeException
index 2566d33..d4a5b89 100644 (file)
@@ -35,6 +35,7 @@ class BackendUserAuthenticator implements MiddlewareInterface
      */
     protected $publicRoutes = [
         '/login',
+        '/login/frame',
         '/ajax/login',
         '/ajax/logout',
         '/ajax/login/refresh',
index af1252c..c75b7d8 100644 (file)
@@ -35,7 +35,8 @@ return [
     // Register login frameset
     'login_frameset' => [
         'path' => '/login/frame',
-        'target' => Controller\LoginFramesetController::class . '::mainAction'
+        'access' => 'public',
+        'target' => Controller\LoginController::class . '::refreshAction'
     ],
 
     /** Wizards */
index 9567a59..8bd4807 100644 (file)
@@ -52,7 +52,7 @@
                                                                </div>
                                                        </f:then>
                                                        <f:else>
-                                                               <form action="../../../../../../index.php" method="post" name="loginform">
+                                                               <form action="index.php" method="post" name="loginform">
                                                                        <input type="hidden" name="login_status" value="logout" />
                                                                        <div class="t3-login-box-body">
                                                                                <div class="t3-login-logout-form">
index 039490a..075571d 100644 (file)
@@ -1,4 +1,5 @@
 <?php
+
 namespace TYPO3\CMS\Backend\Tests\Unit\Controller;
 
 /*
@@ -16,11 +17,18 @@ namespace TYPO3\CMS\Backend\Tests\Unit\Controller;
 
 use TYPO3\CMS\Backend\Controller\LoginController;
 use TYPO3\CMS\Backend\LoginProvider\UsernamePasswordLoginProvider;
+use TYPO3\CMS\Backend\Template\DocumentTemplate;
+use TYPO3\CMS\Core\Authentication\BackendUserAuthentication;
+use TYPO3\CMS\Core\FormProtection\BackendFormProtection;
+use TYPO3\CMS\Core\Http\ServerRequest;
+use TYPO3\CMS\Core\Localization\LanguageService;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
 
 /**
  * Class LoginControllerTest
  */
-class LoginControllerTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
+class LoginControllerTest extends UnitTestCase
 {
     /**
      * @var LoginController|\PHPUnit_Framework_MockObject_MockObject|\TYPO3\TestingFramework\Core\AccessibleObjectInterface
@@ -28,6 +36,12 @@ class LoginControllerTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
     protected $loginControllerMock;
 
     /**
+     * @var bool
+     * @see prophesizeFormProtection
+     */
+    protected static $alreadySetUp = false;
+
+    /**
      * @throws \InvalidArgumentException
      */
     protected function setUp()
@@ -76,7 +90,7 @@ class LoginControllerTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
         $this->expectException(\RuntimeException::class);
         $this->expectExceptionCode(1433416043);
         $GLOBALS['TYPO3_CONF_VARS']['EXTCONF']['backend']['loginProviders'] = [
-            1433419736 => []
+            1433419736 => [],
         ];
         $this->loginControllerMock->_call('validateAndSortLoginProviders');
     }
@@ -90,8 +104,8 @@ class LoginControllerTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
         $this->expectExceptionCode(1460977275);
         $GLOBALS['TYPO3_CONF_VARS']['EXTCONF']['backend']['loginProviders'] = [
             1433419736 => [
-                'provider' => \stdClass::class
-            ]
+                'provider' => \stdClass::class,
+            ],
         ];
         $this->loginControllerMock->_call('validateAndSortLoginProviders');
     }
@@ -107,8 +121,8 @@ class LoginControllerTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
             1433419736 => [
                 'provider' => UsernamePasswordLoginProvider::class,
                 'sorting' => 30,
-                'icon-class' => 'foo'
-            ]
+                'icon-class' => 'foo',
+            ],
         ];
         $this->loginControllerMock->_call('validateAndSortLoginProviders');
     }
@@ -124,8 +138,8 @@ class LoginControllerTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
             1433419736 => [
                 'provider' => UsernamePasswordLoginProvider::class,
                 'sorting' => 30,
-                'label' => 'foo'
-            ]
+                'label' => 'foo',
+            ],
         ];
         $this->loginControllerMock->_call('validateAndSortLoginProviders');
     }
@@ -141,9 +155,154 @@ class LoginControllerTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
             1433419736 => [
                 'provider' => UsernamePasswordLoginProvider::class,
                 'label' => 'foo',
-                'icon-class' => 'foo'
-            ]
+                'icon-class' => 'foo',
+            ],
         ];
         $this->loginControllerMock->_call('validateAndSortLoginProviders');
     }
+
+    /**
+     * @test
+     */
+    public function checkRedirectRedirectsIfLoginIsInProgressAndUserWasFound(): void
+    {
+        $GLOBALS['LANG'] = ($this->prophesize(LanguageService::class))->reveal();
+        $authenticationProphecy = $this->prophesize(BackendUserAuthentication::class);
+        $authenticationProphecy->getTSConfigVal('auth.BE.redirectToURL')->willReturn('http://example.com');
+        $authenticationProphecy->writeUC()->willReturn();
+        $authenticationProphecy->getSessionData('formProtectionSessionToken')->willReturn('foo');
+        $GLOBALS['BE_USER'] = $authenticationProphecy->reveal();
+        $this->prophesizeFormProtection();
+
+        $this->loginControllerMock = $this->getAccessibleMock(
+            LoginController::class,
+            ['isLoginInProgress', 'redirectToUrl'],
+            [],
+            '',
+            false
+        );
+
+        $GLOBALS['BE_USER']->user['uid'] = 1;
+        $this->loginControllerMock->method('isLoginInProgress')->willReturn(true);
+        $this->loginControllerMock->_set('loginRefresh', false);
+
+        $this->loginControllerMock->expects($this->once())->method('redirectToUrl');
+        $this->loginControllerMock->_call('checkRedirect', $this->prophesize(ServerRequest::class)->reveal());
+    }
+
+    /**
+     * @test
+     */
+    public function checkRedirectAddsJavaScriptForCaseLoginRefresh(): void
+    {
+        $GLOBALS['LANG'] = $this->prophesize(LanguageService::class)->reveal();
+        $authenticationProphecy = $this->prophesize(BackendUserAuthentication::class);
+        $authenticationProphecy->getTSConfigVal('auth.BE.redirectToURL')->willReturn('http://example.com');
+        $authenticationProphecy->writeUC()->willReturn();
+        $this->prophesizeFormProtection();
+        $GLOBALS['BE_USER'] = $authenticationProphecy->reveal();
+        $documentTemplateProphecy = $this->prophesize(DocumentTemplate::class);
+        $GLOBALS['TBE_TEMPLATE'] = $documentTemplateProphecy->reveal();
+
+        $this->loginControllerMock = $this->getAccessibleMock(
+            LoginController::class,
+            ['isLoginInProgress', 'redirectToUrl'],
+            [],
+            '',
+            false
+        );
+
+        $GLOBALS['BE_USER']->user['uid'] = 1;
+        $this->loginControllerMock->method('isLoginInProgress')->willReturn(false);
+        $this->loginControllerMock->_set('loginRefresh', true);
+
+        $this->loginControllerMock->_call('checkRedirect', $this->prophesize(ServerRequest::class)->reveal());
+
+        self::assertContains('window.opener.TYPO3.LoginRefresh.startTask();', $documentTemplateProphecy->JScode);
+    }
+
+    /**
+     * @test
+     */
+    public function checkRedirectAddsJavaScriptForCaseLoginRefreshWhileLoginIsInProgress(): void
+    {
+        $GLOBALS['LANG'] = $this->prophesize(LanguageService::class)->reveal();
+        $authenticationProphecy = $this->prophesize(BackendUserAuthentication::class);
+        $authenticationProphecy->getTSConfigVal('auth.BE.redirectToURL')->willReturn('http://example.com');
+        $authenticationProphecy->writeUC()->willReturn();
+        $GLOBALS['BE_USER'] = $authenticationProphecy->reveal();
+        $this->prophesizeFormProtection();
+        $documentTemplateProphecy = $this->prophesize(DocumentTemplate::class);
+        $GLOBALS['TBE_TEMPLATE'] = $documentTemplateProphecy->reveal();
+
+        $this->loginControllerMock = $this->getAccessibleMock(
+            LoginController::class,
+            ['isLoginInProgress', 'redirectToUrl'],
+            [],
+            '',
+            false
+        );
+
+        $GLOBALS['BE_USER']->user['uid'] = 1;
+        $this->loginControllerMock->method('isLoginInProgress')->willReturn(true);
+        $this->loginControllerMock->_set('loginRefresh', true);
+
+        $this->loginControllerMock->_call('checkRedirect', $this->prophesize(ServerRequest::class)->reveal());
+
+        self::assertContains('window.opener.TYPO3.LoginRefresh.startTask();', $documentTemplateProphecy->JScode);
+    }
+
+    /**
+     * @test
+     */
+    public function checkRedirectDoesNotRedirectIfNoUserIsFound(): void
+    {
+        $GLOBALS['BE_USER'] = $this->prophesize(BackendUserAuthentication::class)->reveal();
+        $this->loginControllerMock = $this->getAccessibleMock(
+            LoginController::class,
+            ['redirectToUrl'],
+            [],
+            '',
+            false
+        );
+
+        $GLOBALS['BE_USER']->user['uid'] = null;
+
+        $this->loginControllerMock->expects($this->never())->method('redirectToUrl');
+        $this->loginControllerMock->_call('checkRedirect', $this->prophesize(ServerRequest::class)->reveal());
+    }
+
+    /**
+     * @test
+     */
+    public function checkRedirectDoesNotRedirectIfLoginIsNotInProgressAndLoginRefreshIsFalse(): void
+    {
+        $GLOBALS['BE_USER'] = $this->prophesize(BackendUserAuthentication::class)->reveal();
+        $this->loginControllerMock = $this->getAccessibleMock(
+            LoginController::class,
+            ['redirectToUrl', 'isLoginInProgress'],
+            [],
+            '',
+            false
+        );
+
+        $GLOBALS['BE_USER']->user['uid'] = 1;
+        $this->loginControllerMock->method('isLoginInProgress')->willReturn(false);
+        $this->loginControllerMock->_set('loginRefresh', false);
+
+        $this->loginControllerMock->expects($this->never())->method('redirectToUrl');
+        $this->loginControllerMock->_call('checkRedirect', $this->prophesize(ServerRequest::class)->reveal());
+    }
+
+    /**
+     * FormProtectionFactory has an internal static instance cache we need to work around here
+     */
+    protected function prophesizeFormProtection(): void
+    {
+        if (!self::$alreadySetUp) {
+            $formProtectionProphecy = $this->prophesize(BackendFormProtection::class);
+            GeneralUtility::addInstance(BackendFormProtection::class, $formProtectionProphecy->reveal());
+            self::$alreadySetUp = true;
+        }
+    }
 }