Skip to content

Commit 6bf445a

Browse files
thorstenthorsten
committed
refactor: use flash message for error message if login is not successful
1 parent 4d25bd0 commit 6bf445a

File tree

6 files changed

+40
-38
lines changed

6 files changed

+40
-38
lines changed

‎phpmyfaq/assets/templates/default/login.twig‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@
88
</p>
99
{% endif %}
1010

11-
{{ loginMessage | raw }}
11+
{% if errorMessage %}
12+
<div class="alert alert-danger" role="alert">{{ errorMessage }}</div>
13+
{% endif %}
1214

1315
<div id="phpmyfaq-login" class="mb-5">
1416
<div id="phpmyfaq-login-content">

‎phpmyfaq/index.php‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
use phpMyFAQ\User\CurrentUser;
4545
use phpMyFAQ\User\TwoFactor;
4646
use phpMyFAQ\User\UserAuthentication;
47-
use phpMyFAQ\User\UserSession;
47+
use phpMyFAQ\User\UserException;use phpMyFAQ\User\UserSession;
4848
use Symfony\Component\Config\FileLocator;
4949
use Symfony\Component\DependencyInjection\ContainerBuilder;
5050
use Symfony\Component\DependencyInjection\Loader\PhpFileLoader;
@@ -235,10 +235,11 @@
235235
try {
236236
$user = $userAuth->authenticate($faqusername, $faqpassword);
237237
$userId = $user->getUserId();
238-
} catch (Exception $e) {
238+
} catch (UserException $e) {
239239
$faqConfig->getLogger()->error('Failed login: ' . $e->getMessage());
240-
$action = 'login';
241-
$error = $e->getMessage();
240+
$container->get('session')->getFlashBag()->add('error', $e->getMessage());
241+
$redirect = new RedirectResponse($faqConfig->getDefaultUrl() . 'login');
242+
$redirect->send();
242243
}
243244
} else {
244245
// Try to authenticate with cookie information

‎phpmyfaq/src/phpMyFAQ/Controller/Frontend/LoginController.php‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,25 +37,17 @@ public function index(Request $request): Response
3737
$faqSession->setCurrentUser($this->currentUser);
3838
$faqSession->userTracking('login', 0);
3939

40-
// @todo Implement login message and action handling
41-
// $loginMessage = '';
42-
// if (!is_null($error)) {
43-
// $loginMessage = '<div class="alert alert-danger" role="alert">' . $error . '</div>';
44-
// }
45-
//
46-
// $templateFile = './login.twig';
47-
// if ($action == 'twofactor') {
48-
// $templateFile = './twofactor.twig';
49-
// }
40+
$session = $this->container->get('session');
41+
$errorMessages = $session->getFlashBag()->get('error');
42+
$errorMessage = !empty($errorMessages) ? $errorMessages[0] : null;
5043

5144
return $this->render('login.twig', [
5245
...$this->getHeader($request),
5346
'title' => sprintf('%s - %s', Translation::get(key: 'msgLoginUser'), $this->configuration->getTitle()),
5447
'loginHeader' => Translation::get(key: 'msgLoginUser'),
5548
'sendPassword' => Translation::get(key: 'lostPassword'),
56-
'loginMessage' => '', //$loginMessage,
49+
'errorMessage' => $errorMessage,
5750
'writeLoginPath' => $this->configuration->getDefaultUrl(),
58-
'faqloginaction' => '', //$action,
5951
'login' => Translation::get(key: 'ad_auth_ok'),
6052
'username' => Translation::get(key: 'ad_auth_user'),
6153
'password' => Translation::get(key: 'ad_auth_passwd'),

‎phpmyfaq/src/phpMyFAQ/User.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ public function getUserByCookie(string $cookie): bool
265265

266266
$user = $this->configuration->getDb()->fetchArray($res);
267267

268-
// Don't ever log in via anonymous user
268+
// Don't ever log in via an anonymous user
269269
if (-1 === $user['user_id']) {
270270
return false;
271271
}
@@ -300,7 +300,7 @@ public function getUserId(): int
300300
}
301301

302302
/**
303-
* Checks if display name is already used. Returns true, if already in use.
303+
* Checks if the display name is already used. Returns true, if already in use.
304304
*/
305305
public function checkDisplayName(string $name): bool
306306
{
@@ -312,7 +312,7 @@ public function checkDisplayName(string $name): bool
312312
}
313313

314314
/**
315-
* Checks if email address is already used. Returns true, if already in use.
315+
* Checks if the email address is already used. Returns true, if already in use.
316316
*/
317317
public function checkMailAddress(string $name): bool
318318
{

‎phpmyfaq/src/phpMyFAQ/User/CurrentUser.php‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
namespace phpMyFAQ\User;
2626

2727
use phpMyFAQ\Auth\AuthDriverInterface;
28+
use phpMyFAQ\Auth\AuthException;
2829
use phpMyFAQ\Configuration;
2930
use phpMyFAQ\Core\Exception;
3031
use phpMyFAQ\Database;
@@ -119,7 +120,8 @@ public function __construct(Configuration $configuration)
119120
*
120121
* @param string $login Login name
121122
* @param string $password Password
122-
* @throws Exception
123+
* @throws UserException
124+
* @throws AuthException
123125
* @throws \Exception
124126
*/
125127
public function login(string $login, #[SensitiveParameter] string $password): bool
@@ -139,7 +141,7 @@ public function login(string $login, #[SensitiveParameter] string $password): bo
139141
// First check for brute force attack
140142
$this->getUserByLogin($login);
141143
if ($this->isFailedLastLoginAttempt()) {
142-
throw new Exception(parent::ERROR_USER_TOO_MANY_FAILED_LOGINS);
144+
throw new UserException(parent::ERROR_USER_TOO_MANY_FAILED_LOGINS);
143145
}
144146

145147
// Extract domain if LDAP is active and ldap_use_domain_prefix is true
@@ -212,11 +214,11 @@ public function login(string $login, #[SensitiveParameter] string $password): bo
212214
$this->configuration->get(item: 'security.loginWithEmailAddress')
213215
&& !Filter::filterVar($login, FILTER_VALIDATE_EMAIL)
214216
) {
215-
throw new Exception(parent::ERROR_USER_INCORRECT_LOGIN);
217+
throw new UserException(parent::ERROR_USER_INCORRECT_LOGIN);
216218
}
217219

218220
if (!$this->isFailedLastLoginAttempt()) {
219-
throw new Exception(parent::ERROR_USER_INCORRECT_PASSWORD);
221+
throw new UserException(parent::ERROR_USER_INCORRECT_PASSWORD);
220222
}
221223

222224
return false;

‎phpmyfaq/src/phpMyFAQ/User/UserAuthentication.php‎

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
namespace phpMyFAQ\User;
2626

27+
use phpMyFAQ\Auth\AuthException;
2728
use phpMyFAQ\Auth\AuthLdap;
2829
use phpMyFAQ\Auth\AuthSso;
2930
use phpMyFAQ\Configuration;
@@ -65,9 +66,9 @@ public function setTwoFactorAuth(bool $twoFactorAuth): void
6566

6667
/**
6768
* Authenticates a user with a given username and password against
68-
* LDAP, SSO or local database.
69+
* LDAP, SSO, or local database.
6970
*
70-
* @throws UserException|Exception
71+
* @throws UserException
7172
*/
7273
public function authenticate(string $username, #[SensitiveParameter] string $password): CurrentUser
7374
{
@@ -78,20 +79,24 @@ public function authenticate(string $username, #[SensitiveParameter] string $pas
7879
$this->authenticateLdap();
7980
$this->authenticateSso();
8081

81-
if ($this->currentUser->login($username, $password)) {
82-
if ($this->currentUser->getUserData('twofactor_enabled')) {
83-
$this->setTwoFactorAuth(true);
84-
$this->currentUser->setLoggedIn(false);
85-
} elseif ($this->currentUser->getStatus() !== 'blocked') {
86-
$this->currentUser->setLoggedIn(true);
82+
try {
83+
if ($this->currentUser->login($username, $password)) {
84+
if ($this->currentUser->getUserData('twofactor_enabled')) {
85+
$this->setTwoFactorAuth(true);
86+
$this->currentUser->setLoggedIn(false);
87+
} elseif ($this->currentUser->getStatus() !== 'blocked') {
88+
$this->currentUser->setLoggedIn(true);
89+
} else {
90+
$this->currentUser->setLoggedIn(false);
91+
throw new UserException(
92+
(Translation::get(key: 'ad_auth_fail') ?? 'Authentication failed') . ' (' . $username . ')',
93+
);
94+
}
8795
} else {
88-
$this->currentUser->setLoggedIn(false);
89-
throw new UserException(
90-
(Translation::get(key: 'ad_auth_fail') ?? 'Authentication failed') . ' (' . $username . ')',
91-
);
96+
throw new UserException(Translation::get(key: 'ad_auth_fail') ?? 'Authentication failed');
9297
}
93-
} else {
94-
throw new UserException(Translation::get(key: 'ad_auth_fail') ?? 'Authentication failed');
98+
} catch (AuthException $e) {
99+
throw new UserException($e->getMessage());
95100
}
96101

97102
return $this->currentUser;

0 commit comments

Comments
 (0)