Remove callExit() from AuthenticationPlugin::showLoginForm()

Signed-off-by: Maurício Meneghini Fauth <mauricio@fauth.dev>
phpmyadmin · May 9, 2024 · d0121dc · d0121dc
1 parent aa70f48
commit d0121dc
Show file tree

Hide file tree

Showing 10 changed files with 107 additions and 98 deletions.
diff --git a/src/Plugins/Auth/AuthenticationConfig.php b/src/Plugins/Auth/AuthenticationConfig.php
@@ -36,17 +36,18 @@ class AuthenticationConfig extends AuthenticationPlugin
     /**
      * Displays authentication form
      */
-    public function showLoginForm(): void
+    public function showLoginForm(): Response|null
     {
-        $response = ResponseRenderer::getInstance();
-        if (! $response->isAjax()) {
-            return;
+        $responseRenderer = ResponseRenderer::getInstance();
+        if (! $responseRenderer->isAjax()) {
+            return null;
         }
 
-        $response->setRequestStatus(false);
+        $responseRenderer->setRequestStatus(false);
         // reload_flag removes the token parameter from the URL and reloads
-        $response->addJSON('reload_flag', '1');
-        $response->callExit();
+        $responseRenderer->addJSON('reload_flag', '1');
+
+        return $responseRenderer->response();
     }
 
     /**

diff --git a/src/Plugins/Auth/AuthenticationCookie.php b/src/Plugins/Auth/AuthenticationCookie.php
@@ -65,20 +65,20 @@ class AuthenticationCookie extends AuthenticationPlugin
      *
      * @global string $conn_error the last connection error
      */
-    public function showLoginForm(): never
+    public function showLoginForm(): Response
     {
         $GLOBALS['conn_error'] ??= null;
 
-        $response = ResponseRenderer::getInstance();
+        $responseRenderer = ResponseRenderer::getInstance();
 
         /**
          * When sending login modal after session has expired, send the
          * new token explicitly with the response to update the token
          * in all the forms having a hidden token.
          */
         $sessionExpired = isset($_REQUEST['check_timeout']) || isset($_REQUEST['session_timedout']);
-        if (! $sessionExpired && $response->loginPage()) {
-            $response->callExit();
+        if (! $sessionExpired && $responseRenderer->loginPage()) {
+            return $responseRenderer->response();
         }
 
         /**
@@ -87,16 +87,16 @@ public function showLoginForm(): never
          * in all the forms having a hidden token.
          */
         if ($sessionExpired) {
-            $response->setRequestStatus(false);
-            $response->addJSON('new_token', $_SESSION[' PMA_token ']);
+            $responseRenderer->setRequestStatus(false);
+            $responseRenderer->addJSON('new_token', $_SESSION[' PMA_token ']);
         }
 
         /**
          * logged_in response parameter is used to check if the login,
          * using the modal was successful after session expiration.
          */
         if (isset($_REQUEST['session_timedout'])) {
-            $response->addJSON('logged_in', 0);
+            $responseRenderer->addJSON('logged_in', 0);
         }
 
         $config = Config::getInstance();
@@ -161,7 +161,7 @@ public function showLoginForm(): never
 
         $configFooter = Config::renderFooter();
 
-        $response->addHTML($this->template->render('login/form', [
+        $responseRenderer->addHTML($this->template->render('login/form', [
             'login_header' => $loginHeader,
             'is_demo' => $config->config->debug->demo,
             'error_messages' => $errorMessages,
@@ -192,7 +192,7 @@ public function showLoginForm(): never
             'config_footer' => $configFooter,
         ]));
 
-        $response->callExit();
+        return $responseRenderer->response();
     }
 
     /**
@@ -559,7 +559,7 @@ public function showFailure(AuthenticationFailure $failure): Response
         $responseRenderer->addHeader('Cache-Control', 'no-store, no-cache, must-revalidate');
         $responseRenderer->addHeader('Pragma', 'no-cache');
 
-        $this->showLoginForm();
+        return $this->showLoginForm();
     }
 
     /**

diff --git a/src/Plugins/Auth/AuthenticationHttp.php b/src/Plugins/Auth/AuthenticationHttp.php
@@ -36,14 +36,15 @@ class AuthenticationHttp extends AuthenticationPlugin
     /**
      * Displays authentication form and redirect as necessary
      */
-    public function showLoginForm(): never
+    public function showLoginForm(): Response
     {
-        $response = ResponseRenderer::getInstance();
-        if ($response->isAjax()) {
-            $response->setRequestStatus(false);
+        $responseRenderer = ResponseRenderer::getInstance();
+        if ($responseRenderer->isAjax()) {
+            $responseRenderer->setRequestStatus(false);
             // reload_flag removes the token parameter from the URL and reloads
-            $response->addJSON('reload_flag', '1');
-            $response->callExit();
+            $responseRenderer->addJSON('reload_flag', '1');
+
+            return $responseRenderer->response();
         }
 
         $this->authForm();

diff --git a/src/Plugins/Auth/AuthenticationSignon.php b/src/Plugins/Auth/AuthenticationSignon.php
@@ -35,25 +35,25 @@ class AuthenticationSignon extends AuthenticationPlugin
     /**
      * Displays authentication form
      */
-    public function showLoginForm(): never
+    public function showLoginForm(): Response
     {
-        $response = ResponseRenderer::getInstance();
-        $response->disable();
+        $responseRenderer = ResponseRenderer::getInstance();
+        $responseRenderer->disable();
         unset($_SESSION['LAST_SIGNON_URL']);
         $config = Config::getInstance();
         if (empty($config->selectedServer['SignonURL'])) {
-            echo $this->template->render('error/generic', [
+            $responseRenderer->addHTML($this->template->render('error/generic', [
                 'lang' => $GLOBALS['lang'] ?? 'en',
                 'dir' => LanguageManager::$textDir,
                 'error_message' => 'You must set SignonURL!',
-            ]);
+            ]));
 
-            $response->callExit();
-        } else {
-            $response->redirect($config->selectedServer['SignonURL']);
+            return $responseRenderer->response();
         }
 
-        $response->callExit();
+        $responseRenderer->redirect($config->selectedServer['SignonURL']);
+
+        return $responseRenderer->response();
     }
 
     /**
@@ -260,7 +260,7 @@ public function showFailure(AuthenticationFailure $failure): Response
             $_SESSION['PMA_single_signon_error_message'] = $this->getErrorMessage($failure);
         }
 
-        $this->showLoginForm();
+        return $this->showLoginForm();
     }
 
     /**

diff --git a/src/Plugins/AuthenticationPlugin.php b/src/Plugins/AuthenticationPlugin.php
@@ -65,7 +65,7 @@ public function __construct()
     /**
      * Displays authentication form
      */
-    abstract public function showLoginForm(): void;
+    abstract public function showLoginForm(): Response|null;
 
     /**
      * Gets authentication credentials
@@ -249,7 +249,10 @@ public function authenticate(): Response|null
                 return $responseRenderer->response();
             }
 
-            $this->showLoginForm();
+            $response = $this->showLoginForm();
+            if ($response !== null) {
+                return $response;
+            }
         }
 
         /* Store credentials (eg. in cookies) */

diff --git a/tests/unit/Plugins/Auth/AuthenticationConfigTest.php b/tests/unit/Plugins/Auth/AuthenticationConfigTest.php
@@ -8,14 +8,15 @@
 use PhpMyAdmin\Current;
 use PhpMyAdmin\DatabaseInterface;
 use PhpMyAdmin\Exceptions\AuthenticationFailure;
-use PhpMyAdmin\Exceptions\ExitException;
 use PhpMyAdmin\Plugins\Auth\AuthenticationConfig;
 use PhpMyAdmin\ResponseRenderer;
 use PhpMyAdmin\Tests\AbstractTestCase;
 use PHPUnit\Framework\Attributes\CoversClass;
 use PHPUnit\Framework\Attributes\Medium;
 use ReflectionProperty;
 
+use function json_decode;
+
 #[CoversClass(AuthenticationConfig::class)]
 #[Medium]
 class AuthenticationConfigTest extends AbstractTestCase
@@ -52,12 +53,25 @@ protected function tearDown(): void
         unset($this->object);
     }
 
-    public function testAuth(): void
+    public function testShowLoginFormWithoutAjax(): void
+    {
+        (new ReflectionProperty(ResponseRenderer::class, 'instance'))->setValue(null, null);
+        ResponseRenderer::getInstance()->setAjax(false);
+        self::assertNull($this->object->showLoginForm());
+    }
+
+    public function testShowLoginFormWithAjax(): void
     {
         (new ReflectionProperty(ResponseRenderer::class, 'instance'))->setValue(null, null);
         ResponseRenderer::getInstance()->setAjax(true);
-        $this->expectException(ExitException::class);
-        $this->object->showLoginForm();
+        $response = $this->object->showLoginForm();
+        self::assertNotNull($response);
+        $body = (string) $response->getBody();
+        self::assertJson($body);
+        $json = json_decode($body, true);
+        self::assertIsArray($json);
+        self::assertArrayHasKey('reload_flag', $json);
+        self::assertSame('1', $json['reload_flag']);
     }
 
     public function testAuthCheck(): void

diff --git a/tests/unit/Plugins/Auth/AuthenticationCookieTest.php b/tests/unit/Plugins/Auth/AuthenticationCookieTest.php
@@ -4,6 +4,7 @@
 
 namespace PhpMyAdmin\Tests\Plugins\Auth;
 
+use Fig\Http\Message\StatusCodeInterface;
 use PhpMyAdmin\Config;
 use PhpMyAdmin\Current;
 use PhpMyAdmin\DatabaseInterface;
@@ -24,6 +25,7 @@
 use function base64_decode;
 use function base64_encode;
 use function is_readable;
+use function json_decode;
 use function json_encode;
 use function mb_strlen;
 use function ob_get_clean;
@@ -76,20 +78,21 @@ public function testAuthErrorAJAX(): void
     {
         $GLOBALS['conn_error'] = true;
 
-        $responseStub = new ResponseRendererStub();
-        $responseStub->setAjax(true);
-        (new ReflectionProperty(ResponseRenderer::class, 'instance'))->setValue(null, $responseStub);
-
-        try {
-            $this->object->showLoginForm();
-        } catch (Throwable $throwable) {
-        }
-
-        self::assertInstanceOf(ExitException::class, $throwable);
-        $response = $responseStub->getResponse();
-        self::assertSame(200, $response->getStatusCode());
-        self::assertFalse($responseStub->hasSuccessState());
-        self::assertSame(['redirect_flag' => '1'], $responseStub->getJSONResult());
+        (new ReflectionProperty(ResponseRenderer::class, 'instance'))->setValue(null, null);
+        $responseRenderer = ResponseRenderer::getInstance();
+        $responseRenderer->setAjax(true);
+
+        $response = $this->object->showLoginForm();
+
+        self::assertSame(StatusCodeInterface::STATUS_OK, $response->getStatusCode());
+        $body = (string) $response->getBody();
+        self::assertJson($body);
+        $json = json_decode($body, true);
+        self::assertIsArray($json);
+        self::assertArrayHasKey('success', $json);
+        self::assertFalse($json['success']);
+        self::assertArrayHasKey('redirect_flag', $json);
+        self::assertSame('1', $json['redirect_flag']);
     }
 
     public function testAuthError(): void
@@ -115,17 +118,9 @@ public function testAuthError(): void
         Current::$table = 'testTable';
         $config->settings['Servers'] = [1, 2];
 
-        $responseStub = new ResponseRendererStub();
-        (new ReflectionProperty(ResponseRenderer::class, 'instance'))->setValue(null, $responseStub);
-
-        try {
-            $this->object->showLoginForm();
-        } catch (Throwable $throwable) {
-        }
+        $response = $this->object->showLoginForm();
 
-        $result = $responseStub->getHTMLResult();
-
-        self::assertInstanceOf(ExitException::class, $throwable);
+        $result = (string) $response->getBody();
 
         self::assertStringContainsString(' id="imLogo"', $result);
 
@@ -185,14 +180,9 @@ public function testAuthCaptcha(): void
         $responseStub = new ResponseRendererStub();
         (new ReflectionProperty(ResponseRenderer::class, 'instance'))->setValue(null, $responseStub);
 
-        try {
-            $this->object->showLoginForm();
-        } catch (Throwable $throwable) {
-        }
-
-        $result = $responseStub->getHTMLResult();
+        $response = $this->object->showLoginForm();
 
-        self::assertInstanceOf(ExitException::class, $throwable);
+        $result = (string) $response->getBody();
 
         self::assertStringContainsString('id="imLogo"', $result);
 
@@ -246,14 +236,9 @@ public function testAuthCaptchaCheckbox(): void
         $responseStub = new ResponseRendererStub();
         (new ReflectionProperty(ResponseRenderer::class, 'instance'))->setValue(null, $responseStub);
 
-        try {
-            $this->object->showLoginForm();
-        } catch (Throwable $throwable) {
-        }
-
-        $result = $responseStub->getHTMLResult();
+        $response = $this->object->showLoginForm();
 
-        self::assertInstanceOf(ExitException::class, $throwable);
+        $result = (string) $response->getBody();
 
         self::assertStringContainsString('id="imLogo"', $result);