Skip to content

Commit 9138595

Browse files
grossmannmartingrossmannmartin
authored
[shopsys] improved administration authentication code (#3683)
2 parents 4e63ac7 + 9491960 commit 9138595

File tree

8 files changed

+103
-242
lines changed

8 files changed

+103
-242
lines changed

‎src/Controller/Admin/AdministratorController.php‎

Lines changed: 47 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@
2020
use Shopsys\FrameworkBundle\Model\Administrator\Exception\DeletingSelfException;
2121
use Shopsys\FrameworkBundle\Model\Administrator\Security\AdministratorRolesChangedFacade;
2222
use Shopsys\FrameworkBundle\Model\AdminNavigation\BreadcrumbOverrider;
23-
use Shopsys\FrameworkBundle\Model\Security\Authenticator;
2423
use Shopsys\FrameworkBundle\Model\Security\Roles;
24+
use Symfony\Bundle\SecurityBundle\Security;
2525
use Symfony\Component\Form\Extension\Core\Type\FormType;
2626
use Symfony\Component\Form\Extension\Core\Type\SubmitType;
2727
use Symfony\Component\Form\Extension\Core\Type\TextType;
@@ -37,7 +37,7 @@
3737

3838
class AdministratorController extends AdminBaseController
3939
{
40-
protected const MAX_ADMINISTRATOR_ACTIVITIES_COUNT = 10;
40+
protected const int MAX_ADMINISTRATOR_ACTIVITIES_COUNT = 10;
4141

4242
/**
4343
* @param \Shopsys\FrameworkBundle\Model\Administrator\AdministratorFacade $administratorFacade
@@ -48,7 +48,7 @@ class AdministratorController extends AdminBaseController
4848
* @param \Shopsys\FrameworkBundle\Model\Administrator\Security\AdministratorRolesChangedFacade $administratorRolesChangedFacade
4949
* @param \Shopsys\FrameworkBundle\Model\Administrator\AdministratorTwoFactorAuthenticationFacade $administratorTwoFactorAuthenticationFacade
5050
* @param \Shopsys\FrameworkBundle\Model\Administrator\AdministratorPasswordFacade $administratorPasswordFacade
51-
* @param \Shopsys\FrameworkBundle\Model\Security\Authenticator $authenticator
51+
* @param \Symfony\Bundle\SecurityBundle\Security $security
5252
*/
5353
public function __construct(
5454
protected readonly AdministratorFacade $administratorFacade,
@@ -59,12 +59,15 @@ public function __construct(
5959
protected readonly AdministratorRolesChangedFacade $administratorRolesChangedFacade,
6060
protected readonly AdministratorTwoFactorAuthenticationFacade $administratorTwoFactorAuthenticationFacade,
6161
protected readonly AdministratorPasswordFacade $administratorPasswordFacade,
62-
protected readonly Authenticator $authenticator,
62+
protected readonly Security $security,
6363
) {
6464
}
6565

66+
/**
67+
* @return \Symfony\Component\HttpFoundation\Response
68+
*/
6669
#[Route(path: '/administrator/list/')]
67-
public function listAction()
70+
public function listAction(): Response
6871
{
6972
$queryBuilder = $this->administratorFacade->getAllListableQueryBuilder();
7073
$dataSource = new QueryBuilderDataSource($queryBuilder, 'a.id');
@@ -90,10 +93,13 @@ public function listAction()
9093
/**
9194
* @param \Symfony\Component\HttpFoundation\Request $request
9295
* @param int $id
96+
* @return \Symfony\Component\HttpFoundation\Response
9397
*/
9498
#[Route(path: '/administrator/edit/{id}', requirements: ['id' => '\d+'])]
95-
public function editAction(Request $request, int $id)
99+
public function editAction(Request $request, int $id): Response
96100
{
101+
$this->denyAccessUnlessHimselfOrGranted($request, $id);
102+
97103
$administrator = $this->administratorFacade->getById($id);
98104

99105
$loggedUser = $this->getUser();
@@ -159,8 +165,31 @@ public function editAction(Request $request, int $id)
159165
]);
160166
}
161167

168+
/**
169+
* @param \Symfony\Component\HttpFoundation\Request $request
170+
* @param int $administratorId
171+
*/
172+
protected function denyAccessUnlessHimselfOrGranted(Request $request, int $administratorId): void
173+
{
174+
$currentAdministrator = $this->getCurrentAdministrator();
175+
176+
// always allow admin to edit himself
177+
if ($currentAdministrator->getId() === $administratorId) {
178+
return;
179+
}
180+
181+
if ($request->getMethod() === Request::METHOD_GET) {
182+
$this->denyAccessUnlessGranted(Roles::ROLE_ADMINISTRATOR_VIEW);
183+
} else {
184+
$this->denyAccessUnlessGranted(Roles::ROLE_ADMINISTRATOR_FULL);
185+
}
186+
}
187+
188+
/**
189+
* @return \Symfony\Component\HttpFoundation\Response
190+
*/
162191
#[Route(path: '/administrator/my-account/')]
163-
public function myAccountAction()
192+
public function myAccountAction(): Response
164193
{
165194
/** @var \Shopsys\FrameworkBundle\Model\Administrator\Administrator $loggedUser */
166195
$loggedUser = $this->getUser();
@@ -172,9 +201,10 @@ public function myAccountAction()
172201

173202
/**
174203
* @param \Symfony\Component\HttpFoundation\Request $request
204+
* @return \Symfony\Component\HttpFoundation\Response
175205
*/
176206
#[Route(path: '/administrator/new/')]
177-
public function newAction(Request $request)
207+
public function newAction(Request $request): Response
178208
{
179209
$form = $this->createForm(AdministratorFormType::class, $this->administratorDataFactory->create(), [
180210
'scenario' => AdministratorFormType::SCENARIO_CREATE,
@@ -211,9 +241,10 @@ public function newAction(Request $request)
211241
/**
212242
* @CsrfProtection
213243
* @param int $id
244+
* @return \Symfony\Component\HttpFoundation\Response
214245
*/
215246
#[Route(path: '/administrator/delete/{id}', requirements: ['id' => '\d+'])]
216-
public function deleteAction(int $id)
247+
public function deleteAction(int $id): Response
217248
{
218249
try {
219250
$realName = $this->administratorFacade->getById($id)->getRealName();
@@ -225,16 +256,16 @@ public function deleteAction(int $id)
225256
'name' => $realName,
226257
],
227258
);
228-
} catch (DeletingSelfException $ex) {
259+
} catch (DeletingSelfException) {
229260
$this->addErrorFlash(t('You can\'t delete yourself.'));
230-
} catch (DeletingLastAdministratorException $ex) {
261+
} catch (DeletingLastAdministratorException) {
231262
$this->addErrorFlashTwig(
232263
t('Administrator <strong>{{ name }}</strong> is the only one and can\'t be deleted.'),
233264
[
234265
'name' => $this->administratorFacade->getById($id)->getRealName(),
235266
],
236267
);
237-
} catch (AdministratorNotFoundException $ex) {
268+
} catch (AdministratorNotFoundException) {
238269
$this->addErrorFlash(t('Selected administrated doesn\'t exist.'));
239270
}
240271

@@ -269,7 +300,7 @@ public function enableTwoFactorAuthenticationAction(
269300
$loggedUser = $this->getUser();
270301
$this->securitySafeCheck($loggedUser);
271302

272-
if ($administrator->getUsername() !== $loggedUser->getUserIdentifier()) {
303+
if ($administrator->getUsername() !== $loggedUser?->getUserIdentifier()) {
273304
$this->addErrorFlash(t('You are allowed to set up two factor authentication only to yourself.'));
274305

275306
return $this->redirectToRoute('admin_administrator_edit', ['id' => $id]);
@@ -386,7 +417,7 @@ public function disableTwoFactorAuthenticationAction(Request $request, int $id):
386417
$loggedUser = $this->getUser();
387418
$this->securitySafeCheck($loggedUser);
388419

389-
if ($administrator->getUsername() !== $loggedUser->getUserIdentifier()) {
420+
if ($administrator->getUsername() !== $loggedUser?->getUserIdentifier()) {
390421
$this->addErrorFlash(t('You are allowed to disable two factor authentication only to yourself.'));
391422

392423
return $this->redirectToRoute('admin_administrator_edit', ['id' => $id]);
@@ -531,7 +562,8 @@ public function setNewPasswordAction(Request $request): Response
531562
);
532563

533564
if (!$this->isGranted(Roles::ROLE_ADMIN)) {
534-
$this->authenticator->loginAdministrator($administrator);
565+
$this->security->login($administrator, 'security.authenticator.form_login.administration');
566+
$request->getSession()->migrate();
535567
}
536568

537569
$this->addSuccessFlash(t('Password has been successfully set.'));

‎src/Controller/Admin/LoginController.php‎

Lines changed: 34 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -9,42 +9,44 @@
99
use Shopsys\FrameworkBundle\Form\Admin\Login\LoginFormType;
1010
use Shopsys\FrameworkBundle\Model\Administrator\Security\Exception\InvalidTokenException;
1111
use Shopsys\FrameworkBundle\Model\Security\AdministratorLoginFacade;
12-
use Shopsys\FrameworkBundle\Model\Security\Authenticator;
13-
use Shopsys\FrameworkBundle\Model\Security\Exception\LoginFailedException;
1412
use Shopsys\FrameworkBundle\Model\Security\Exception\LoginWithDefaultPasswordException;
1513
use Shopsys\FrameworkBundle\Model\Security\Roles;
1614
use Symfony\Component\HttpFoundation\Request;
15+
use Symfony\Component\HttpFoundation\Response;
1716
use Symfony\Component\Routing\Annotation\Route;
1817
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
1918
use Symfony\Component\Security\Core\Exception\TooManyLoginAttemptsAuthenticationException;
19+
use Symfony\Component\Security\Http\Authentication\AuthenticationUtils;
20+
use Symfony\Component\Security\Http\SecurityRequestAttributes;
2021

2122
class LoginController extends AdminBaseController
2223
{
23-
protected const MULTIDOMAIN_LOGIN_TOKEN_PARAMETER_NAME = 'multidomainLoginToken';
24-
public const ORIGINAL_DOMAIN_ID_PARAMETER_NAME = 'originalDomainId';
25-
public const ORIGINAL_REFERER_PARAMETER_NAME = 'originalReferer';
24+
protected const string MULTIDOMAIN_LOGIN_TOKEN_PARAMETER_NAME = 'multidomainLoginToken';
25+
public const string ORIGINAL_DOMAIN_ID_PARAMETER_NAME = 'originalDomainId';
26+
public const string ORIGINAL_REFERER_PARAMETER_NAME = 'originalReferer';
2627

2728
/**
28-
* @param \Shopsys\FrameworkBundle\Model\Security\Authenticator $authenticator
2929
* @param \Shopsys\FrameworkBundle\Component\Domain\Domain $domain
3030
* @param \Shopsys\FrameworkBundle\Component\Router\DomainRouterFactory $domainRouterFactory
3131
* @param \Shopsys\FrameworkBundle\Model\Security\AdministratorLoginFacade $administratorLoginFacade
32+
* @param \Symfony\Component\Security\Http\Authentication\AuthenticationUtils $authenticationUtils
3233
*/
3334
public function __construct(
34-
protected readonly Authenticator $authenticator,
3535
protected readonly Domain $domain,
3636
protected readonly DomainRouterFactory $domainRouterFactory,
3737
protected readonly AdministratorLoginFacade $administratorLoginFacade,
38+
protected readonly AuthenticationUtils $authenticationUtils,
3839
) {
3940
}
4041

4142
/**
4243
* @param \Symfony\Component\HttpFoundation\Request $request
44+
* @return \Symfony\Component\HttpFoundation\Response
4345
*/
4446
#[Route(path: '/', name: 'admin_login')]
4547
#[Route(path: '/login-check/', name: 'admin_login_check')]
4648
#[Route(path: '/logout/', name: 'admin_logout')]
47-
public function loginAction(Request $request)
49+
public function loginAction(Request $request): Response
4850
{
4951
$currentDomainId = $this->domain->getId();
5052

@@ -72,39 +74,44 @@ public function loginAction(Request $request)
7274
'action' => $this->generateUrl('admin_login_check'),
7375
]);
7476

75-
try {
76-
$this->authenticator->checkLoginProcess($request);
77-
} catch (LoginFailedException $e) {
78-
if ($e->getPrevious() instanceof LoginWithDefaultPasswordException) {
79-
$error = t(
77+
$lastAuthenticationError = $this->authenticationUtils->getLastAuthenticationError();
78+
79+
if ($lastAuthenticationError !== null) {
80+
$error = match (true) {
81+
$lastAuthenticationError->getPrevious() instanceof LoginWithDefaultPasswordException => t(
8082
'Oh, you just tried to log in using default credentials. We do not allow that on production'
81-
. ' environment. If you are random hacker, please go somewhere else. If you are authorized user,'
82-
. ' please use another account or contact developers and change password during deployment.',
83-
);
84-
} elseif ($e->getPrevious() instanceof TooManyLoginAttemptsAuthenticationException) {
85-
$error = t('Too many login attempts. Please try again later.');
86-
} else {
87-
$error = t('Log in failed.');
88-
}
83+
. ' environment. If you are random hacker, please go somewhere else. If you are authorized user,'
84+
. ' please use another account or contact developers and change password during deployment.',
85+
),
86+
$lastAuthenticationError->getPrevious() instanceof TooManyLoginAttemptsAuthenticationException => t(
87+
'Too many login attempts. Please try again later.',
88+
),
89+
default => t('Log in failed.'),
90+
};
8991
}
9092

93+
$lastUserName = $this->authenticationUtils->getLastUsername();
94+
$request->getSession()->remove(SecurityRequestAttributes::LAST_USERNAME);
95+
9196
return $this->render('@ShopsysFramework/Admin/Content/Login/loginForm.html.twig', [
9297
'form' => $form->createView(),
98+
'lastUsername' => $lastUserName,
9399
'error' => $error,
94100
]);
95101
}
96102

97103
/**
98104
* @param \Symfony\Component\HttpFoundation\Request $request
99105
* @param int $originalDomainId
106+
* @return \Symfony\Component\HttpFoundation\Response
100107
*/
101108
#[Route(path: '/sso/{originalDomainId}', requirements: ['originalDomainId' => '\d+'])]
102-
public function ssoAction(Request $request, $originalDomainId)
109+
public function ssoAction(Request $request, int $originalDomainId): Response
103110
{
104111
$multidomainToken = $this->administratorLoginFacade->generateMultidomainLoginTokenWithExpiration(
105112
$this->getCurrentAdministrator(),
106113
);
107-
$originalDomainRouter = $this->domainRouterFactory->getRouter((int)$originalDomainId);
114+
$originalDomainRouter = $this->domainRouterFactory->getRouter($originalDomainId);
108115
$redirectTo = $originalDomainRouter->generate(
109116
'admin_login_authorization',
110117
[
@@ -119,19 +126,20 @@ public function ssoAction(Request $request, $originalDomainId)
119126

120127
/**
121128
* @param \Symfony\Component\HttpFoundation\Request $request
129+
* @return \Symfony\Component\HttpFoundation\Response
122130
*/
123131
#[Route(path: '/authorization/')]
124-
public function authorizationAction(Request $request)
132+
public function authorizationAction(Request $request): Response
125133
{
126134
$multidomainLoginToken = $request->get(static::MULTIDOMAIN_LOGIN_TOKEN_PARAMETER_NAME);
127135
$originalReferer = $request->get(self::ORIGINAL_REFERER_PARAMETER_NAME);
128136

129137
try {
130138
$this->administratorLoginFacade->loginByMultidomainToken($request, $multidomainLoginToken);
131-
} catch (InvalidTokenException $ex) {
139+
} catch (InvalidTokenException) {
132140
return $this->render('@ShopsysFramework/Admin/Content/Login/loginFailed.html.twig');
133141
}
134-
$redirectTo = $originalReferer !== null ? $originalReferer : $this->generateUrl('admin_default_dashboard');
142+
$redirectTo = $originalReferer ?? $this->generateUrl('admin_default_dashboard');
135143

136144
return $this->redirect($redirectTo);
137145
}

‎src/Model/Security/Authenticator.php‎

Lines changed: 0 additions & 90 deletions
This file was deleted.

0 commit comments

Comments
 (0)