Skip to content

Commit

Permalink
Merge pull request #19154 from MauricioFauth/auth-plugin-response-han…
Browse files Browse the repository at this point in the history
…dling

Remove ResponseRenderer::callExit() calls from auth plugins
  • Loading branch information
MauricioFauth committed May 11, 2024
2 parents c6c9aeb + 6a425a2 commit b93d1b9
Show file tree
Hide file tree
Showing 15 changed files with 475 additions and 377 deletions.
5 changes: 3 additions & 2 deletions docs/faq.rst
Expand Up @@ -2235,9 +2235,10 @@ logs. Currently there are two variables available:
User name of currently active user (they do not have to be logged in).
``userStatus``
Status of currently active user, one of ``ok`` (user is logged in),
``mysql-denied`` (MySQL denied user login), ``allow-denied`` (user denied
``server-denied`` (database server denied user login), ``allow-denied`` (user denied
by allow/deny rules), ``root-denied`` (root is denied in configuration),
``empty-denied`` (empty password is denied).
``empty-denied`` (empty password is denied),
``no-activity`` (automatically logged out due to inactivity).

``LogFormat`` directive for Apache can look like following:

Expand Down
5 changes: 5 additions & 0 deletions psalm-baseline.xml
Expand Up @@ -7672,6 +7672,7 @@
<code><![CDATA[$password]]></code>
</PossiblyInvalidPropertyAssignmentValue>
<PossiblyUnusedReturnValue>
<code><![CDATA[Response]]></code>
<code><![CDATA[bool]]></code>
</PossiblyUnusedReturnValue>
<RedundantCast>
Expand Down Expand Up @@ -7761,6 +7762,9 @@
<code><![CDATA[$config->selectedServer]]></code>
<code><![CDATA[array_merge($config->selectedServer, $singleSignonCfgUpdate)]]></code>
</MixedPropertyTypeCoercion>
<PossiblyUnusedReturnValue>
<code><![CDATA[Response]]></code>
</PossiblyUnusedReturnValue>
<RiskyTruthyFalsyComparison>
<code><![CDATA[empty($config->selectedServer['SignonURL'])]]></code>
</RiskyTruthyFalsyComparison>
Expand Down Expand Up @@ -14310,6 +14314,7 @@
<code><![CDATA[$config->settings]]></code>
<code><![CDATA[$config->settings]]></code>
<code><![CDATA[$config->settings]]></code>
<code><![CDATA[$config->settings]]></code>
</PropertyTypeCoercion>
<TypeDoesNotContainType>
<code><![CDATA[assertSame]]></code>
Expand Down
78 changes: 78 additions & 0 deletions src/Exceptions/AuthenticationFailure.php
@@ -0,0 +1,78 @@
<?php

declare(strict_types=1);

namespace PhpMyAdmin\Exceptions;

use RuntimeException;
use Throwable;

use function __;

final class AuthenticationFailure extends RuntimeException
{
public const SERVER_DENIED = 'server-denied';
public const ALLOW_DENIED = 'allow-denied';
public const ROOT_DENIED = 'root-denied';
public const EMPTY_DENIED = 'empty-denied';
public const NO_ACTIVITY = 'no-activity';

/** @psalm-param self::* $failureType */
public function __construct(
public readonly string $failureType,
string $message = '',
int $code = 0,

Check warning on line 24 in src/Exceptions/AuthenticationFailure.php

View workflow job for this annotation

GitHub Actions / Infection (8.2, ubuntu-latest)

Escaped Mutant for Mutator "IncrementInteger": --- Original +++ New @@ @@ public const EMPTY_DENIED = 'empty-denied'; public const NO_ACTIVITY = 'no-activity'; /** @psalm-param self::* $failureType */ - public function __construct(public readonly string $failureType, string $message = '', int $code = 0, Throwable|null $previous = null) + public function __construct(public readonly string $failureType, string $message = '', int $code = 1, Throwable|null $previous = null) { parent::__construct($message, $code, $previous); }

Check warning on line 24 in src/Exceptions/AuthenticationFailure.php

View workflow job for this annotation

GitHub Actions / Infection (8.2, ubuntu-latest)

Escaped Mutant for Mutator "DecrementInteger": --- Original +++ New @@ @@ public const EMPTY_DENIED = 'empty-denied'; public const NO_ACTIVITY = 'no-activity'; /** @psalm-param self::* $failureType */ - public function __construct(public readonly string $failureType, string $message = '', int $code = 0, Throwable|null $previous = null) + public function __construct(public readonly string $failureType, string $message = '', int $code = -1, Throwable|null $previous = null) { parent::__construct($message, $code, $previous); }
Throwable|null $previous = null,
) {
parent::__construct($message, $code, $previous);
}

/**
* Database server denied user login
*/
public static function deniedByDatabaseServer(): self
{
return new self(self::SERVER_DENIED, __('Cannot log in to the database server.'));
}

/**
* User denied by allow/deny rules
*/
public static function deniedByAllowDenyRules(): self
{
return new self(self::ALLOW_DENIED, __('Access denied!'));
}

/**
* User 'root' is denied in configuration
*/
public static function rootDeniedByConfiguration(): self
{
return new self(self::ROOT_DENIED, __('Access denied!'));
}

/**
* Empty password is denied
*/
public static function emptyPasswordDeniedByConfiguration(): self
{
return new self(
self::EMPTY_DENIED,
__('Login without a password is forbidden by configuration (see AllowNoPassword).'),
);
}

/**
* Automatically logged out due to inactivity
*/
public static function loggedOutDueToInactivity(): self
{
return new self(
self::NO_ACTIVITY,
__(
'You have been automatically logged out due to inactivity of %s seconds.'
. ' Once you log in again, you should be able to resume the work where you left off.',
),
);
}
}
48 changes: 37 additions & 11 deletions src/Http/Middleware/Authentication.php
Expand Up @@ -11,13 +11,13 @@
use PhpMyAdmin\Container\ContainerBuilder;
use PhpMyAdmin\DatabaseInterface;
use PhpMyAdmin\Dbal\ConnectionType;
use PhpMyAdmin\Exceptions\AuthenticationFailure;
use PhpMyAdmin\Exceptions\AuthenticationPluginException;
use PhpMyAdmin\Exceptions\ExitException;
use PhpMyAdmin\Http\Factory\ResponseFactory;
use PhpMyAdmin\Http\ServerRequest;
use PhpMyAdmin\LanguageManager;
use PhpMyAdmin\Logging;
use PhpMyAdmin\Plugins\AuthenticationPlugin;
use PhpMyAdmin\Plugins\AuthenticationPluginFactory;
use PhpMyAdmin\ResponseRenderer;
use PhpMyAdmin\Template;
Expand All @@ -26,6 +26,7 @@
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\MiddlewareInterface;
use Psr\Http\Server\RequestHandlerInterface;
use Throwable;

use function assert;
use function define;
Expand Down Expand Up @@ -60,7 +61,23 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
}

try {
$authPlugin->authenticate();
try {
$response = $authPlugin->authenticate();
if ($response !== null) {
return $response;
}
} catch (AuthenticationFailure $exception) {
return $authPlugin->showFailure($exception);
} catch (Throwable $exception) {
$response = $this->responseFactory->createResponse(StatusCodeInterface::STATUS_INTERNAL_SERVER_ERROR);

return $response->write($this->template->render('error/generic', [
'lang' => $GLOBALS['lang'] ?? 'en',
'dir' => LanguageManager::$textDir,
'error_message' => $exception->getMessage(),
]));
}

$currentServer = new Server(Config::getInstance()->selectedServer);

/* Enable LOAD DATA LOCAL INFILE for LDI plugin */
Expand All @@ -71,7 +88,11 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
// phpcs:enable
}

$this->connectToDatabaseServer(DatabaseInterface::getInstance(), $authPlugin, $currentServer);
try {
$this->connectToDatabaseServer(DatabaseInterface::getInstance(), $currentServer);
} catch (AuthenticationFailure $exception) {
return $authPlugin->showFailure($exception);
}

// Relation should only be initialized after the connection is successful
/** @var Relation $relation */
Expand All @@ -81,9 +102,16 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
// Tracker can only be activated after the relation has been initialized
Tracker::enable();

$authPlugin->rememberCredentials();
$response = $authPlugin->rememberCredentials();
if ($response !== null) {
return $response;
}

assert($request instanceof ServerRequest);
$authPlugin->checkTwoFactor($request);
$response = $authPlugin->checkTwoFactor($request);
if ($response !== null) {
return $response;
}
} catch (ExitException) {
return ResponseRenderer::getInstance()->response();
}
Expand All @@ -94,11 +122,9 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
return $handler->handle($request);
}

private function connectToDatabaseServer(
DatabaseInterface $dbi,
AuthenticationPlugin $auth,
Server $currentServer,
): void {
/** @throws AuthenticationFailure */
private function connectToDatabaseServer(DatabaseInterface $dbi, Server $currentServer): void
{
/**
* Try to connect MySQL with the control user profile (will be used to get the privileges list for the current
* user but the true user link must be open after this one, so it would be default one for all the scripts).
Expand All @@ -111,7 +137,7 @@ private function connectToDatabaseServer(
// Connects to the server (validates user's login)
$userConnection = $dbi->connect($currentServer, ConnectionType::User);
if ($userConnection === null) {
$auth->showFailure('mysql-denied');
throw AuthenticationFailure::deniedByDatabaseServer();
}

if ($controlConnection !== null) {
Expand Down
42 changes: 25 additions & 17 deletions src/Plugins/Auth/AuthenticationConfig.php
Expand Up @@ -10,14 +10,18 @@
use PhpMyAdmin\Config;
use PhpMyAdmin\DatabaseInterface;
use PhpMyAdmin\Error\ErrorHandler;
use PhpMyAdmin\Exceptions\AuthenticationFailure;
use PhpMyAdmin\Html\Generator;
use PhpMyAdmin\Http\Response;
use PhpMyAdmin\Plugins\AuthenticationPlugin;
use PhpMyAdmin\ResponseRenderer;
use PhpMyAdmin\Server\Select;
use PhpMyAdmin\Util;

use function __;
use function count;
use function ob_get_clean;
use function ob_start;
use function sprintf;
use function trigger_error;

Expand All @@ -32,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();
}

/**
Expand All @@ -65,25 +70,25 @@ public function readCredentials(): bool

/**
* User is not allowed to login to MySQL -> authentication failed
*
* @param string $failure String describing why authentication has failed
*/
public function showFailure(string $failure): never
public function showFailure(AuthenticationFailure $failure): Response
{
parent::showFailure($failure);
$this->logFailure($failure);

Check warning on line 76 in src/Plugins/Auth/AuthenticationConfig.php

View workflow job for this annotation

GitHub Actions / Infection (8.2, ubuntu-latest)

Escaped Mutant for Mutator "MethodCallRemoval": --- Original +++ New @@ @@ */ public function showFailure(AuthenticationFailure $failure): Response { - $this->logFailure($failure); + $connError = DatabaseInterface::getInstance()->getError(); if ($connError === '' || $connError === '0') { $connError = __('Cannot connect: invalid settings.');

$connError = DatabaseInterface::getInstance()->getError();
if ($connError === '' || $connError === '0') {
$connError = __('Cannot connect: invalid settings.');
}

/* HTML header */
$response = ResponseRenderer::getInstance();
$response->setMinimalFooter();
$header = $response->getHeader();
$responseRenderer = ResponseRenderer::getInstance();
$responseRenderer->setMinimalFooter();

Check warning on line 85 in src/Plugins/Auth/AuthenticationConfig.php

View workflow job for this annotation

GitHub Actions / Infection (8.2, ubuntu-latest)

Escaped Mutant for Mutator "MethodCallRemoval": --- Original +++ New @@ @@ } /* HTML header */ $responseRenderer = ResponseRenderer::getInstance(); - $responseRenderer->setMinimalFooter(); + $header = $responseRenderer->getHeader(); $header->setBodyId('loginform'); $header->setTitle(__('Access denied!'));
$header = $responseRenderer->getHeader();
$header->setBodyId('loginform');
$header->setTitle(__('Access denied!'));
$header->disableMenuAndConsole();

ob_start();
echo '<br><br>
<div class="text-center">
<h1>';
Expand All @@ -95,8 +100,8 @@ public function showFailure(string $failure): never
<tr>
<td>';
$config = Config::getInstance();
if ($failure === 'allow-denied') {
trigger_error(__('Access denied!'), E_USER_NOTICE);
if ($failure->failureType === AuthenticationFailure::ALLOW_DENIED) {
trigger_error($failure->getMessage(), E_USER_NOTICE);
} else {
// Check whether user has configured something
if ($config->sourceMtime == 0) {
Expand Down Expand Up @@ -158,6 +163,9 @@ public function showFailure(string $failure): never
}

echo '</table>' , "\n";
$response->callExit();

$responseRenderer->addHTML((string) ob_get_clean());

Check warning on line 167 in src/Plugins/Auth/AuthenticationConfig.php

View workflow job for this annotation

GitHub Actions / Infection (8.2, ubuntu-latest)

Escaped Mutant for Mutator "CastString": --- Original +++ New @@ @@ echo '</tr>', "\n"; } echo '</table>', "\n"; - $responseRenderer->addHTML((string) ob_get_clean()); + $responseRenderer->addHTML(ob_get_clean()); return $responseRenderer->response(); } }

return $responseRenderer->response();
}
}

0 comments on commit b93d1b9

Please sign in to comment.