Skip to content

Commit 66db814

Browse files
Merge pull request #6463 from christianbeeznest/GH-6460
User: Add password rotation feature - refs #6460
2 parents 7fa20f1 + b9cef54 commit 66db814

File tree

9 files changed

+419
-151
lines changed

9 files changed

+419
-151
lines changed

assets/vue/composables/auth/login.js

Lines changed: 40 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ function isValidHttpUrl(string) {
99
try {
1010
const url = new URL(string);
1111
return url.protocol === "http:" || url.protocol === "https:";
12-
} catch {
12+
} catch (_) {
1313
return false;
1414
}
1515
}
@@ -31,23 +31,31 @@ export function useLogin() {
3131
try {
3232
const responseData = await securityService.login(payload);
3333

34-
// Step 1: Handle 2FA
34+
// Check if the backend demands 2FA and no TOTP was provided yet
3535
if (responseData.requires2FA && !payload.totp) {
3636
requires2FA.value = true;
3737
return { success: false, requires2FA: true };
3838
}
3939

40-
// Step 2: Handle explicit error message
40+
// Check rotate password flow
41+
if (responseData.rotate_password && responseData.redirect) {
42+
window.location.href = responseData.redirect;
43+
return { success: true, rotate: true };
44+
}
45+
46+
// Handle explicit backend error message
4147
if (responseData.error) {
4248
showErrorNotification(responseData.error);
4349
return { success: false, error: responseData.error };
4450
}
4551

46-
// Step 3: Set user and load platform config
47-
securityStore.setUser(responseData);
48-
await platformConfigurationStore.initialize();
52+
// Special flow for terms acceptance
53+
if (responseData.load_terms && responseData.redirect) {
54+
window.location.href = responseData.redirect;
55+
return { success: true, redirect: responseData.redirect };
56+
}
4957

50-
// Step 4: Honor a redirect query parameter
58+
// Handle external redirect param
5159
const redirectParam = route.query.redirect?.toString();
5260
if (redirectParam) {
5361
if (isValidHttpUrl(redirectParam)) {
@@ -58,39 +66,43 @@ export function useLogin() {
5866
return { success: true };
5967
}
6068

61-
// Step 5: Handle "load terms" flow
62-
if (responseData.load_terms && responseData.redirect) {
69+
if (responseData.redirect) {
6370
window.location.href = responseData.redirect;
6471
return { success: true };
6572
}
6673

67-
// Step 6: Default post-login redirect based on roles
68-
const setting = platformConfigurationStore.getSetting(
69-
"registration.redirect_after_login"
70-
);
74+
securityStore.setUser(responseData);
75+
await platformConfigurationStore.initialize();
76+
77+
// Handle redirect param again after login
78+
if (route.query.redirect) {
79+
await router.replace({ path: route.query.redirect.toString() });
80+
return { success: true };
81+
}
82+
83+
// Determine post-login route from settings
84+
const setting = platformConfigurationStore.getSetting("registration.redirect_after_login");
7185
let target = "/";
7286

7387
if (setting && typeof setting === "string") {
7488
try {
7589
const map = JSON.parse(setting);
7690
const roles = responseData.roles || [];
77-
const profile = roles.includes("ROLE_ADMIN")
78-
? "ADMIN"
79-
: roles.includes("ROLE_SESSION_MANAGER")
80-
? "SESSIONADMIN"
81-
: roles.includes("ROLE_TEACHER")
82-
? "COURSEMANAGER"
83-
: roles.includes("ROLE_STUDENT_BOSS")
84-
? "STUDENT_BOSS"
85-
: roles.includes("ROLE_DRH")
86-
? "DRH"
87-
: roles.includes("ROLE_INVITEE")
88-
? "INVITEE"
89-
: roles.includes("ROLE_STUDENT")
90-
? "STUDENT"
91-
: null;
9291

92+
const getProfile = () => {
93+
if (roles.includes("ROLE_ADMIN")) return "ADMIN";
94+
if (roles.includes("ROLE_SESSION_MANAGER")) return "SESSIONADMIN";
95+
if (roles.includes("ROLE_TEACHER")) return "COURSEMANAGER";
96+
if (roles.includes("ROLE_STUDENT_BOSS")) return "STUDENT_BOSS";
97+
if (roles.includes("ROLE_DRH")) return "DRH";
98+
if (roles.includes("ROLE_INVITEE")) return "INVITEE";
99+
if (roles.includes("ROLE_STUDENT")) return "STUDENT";
100+
return null;
101+
};
102+
103+
const profile = getProfile();
93104
const value = profile && map[profile] ? map[profile] : "";
105+
94106
switch (value) {
95107
case "user_portal.php":
96108
case "index.php":

src/CoreBundle/Controller/AccountController.php

Lines changed: 112 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,16 @@
2020
use Endroid\QrCode\Writer\PngWriter;
2121
use OTPHP\TOTP;
2222
use Security;
23+
use Symfony\Component\Form\FormError;
2324
use Symfony\Component\HttpFoundation\RedirectResponse;
2425
use Symfony\Component\HttpFoundation\Request;
2526
use Symfony\Component\HttpFoundation\Response;
2627
use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface;
2728
use Symfony\Component\Routing\Attribute\Route;
29+
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
30+
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
2831
use Symfony\Component\Security\Core\User\UserInterface;
32+
use Symfony\Component\Security\Csrf\CsrfToken;
2933
use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
3034
use Symfony\Contracts\Translation\TranslatorInterface;
3135

@@ -43,8 +47,12 @@ public function __construct(
4347
) {}
4448

4549
#[Route('/edit', name: 'chamilo_core_account_edit', methods: ['GET', 'POST'])]
46-
public function edit(Request $request, UserRepository $userRepository, IllustrationRepository $illustrationRepo, SettingsManager $settingsManager): Response
47-
{
50+
public function edit(
51+
Request $request,
52+
UserRepository $userRepository,
53+
IllustrationRepository $illustrationRepo,
54+
SettingsManager $settingsManager
55+
): Response {
4856
$user = $this->userHelper->getCurrent();
4957

5058
if (!\is_object($user) || !$user instanceof UserInterface) {
@@ -69,6 +77,7 @@ public function edit(Request $request, UserRepository $userRepository, Illustrat
6977
$password = $form['password']->getData();
7078
if ($password) {
7179
$user->setPlainPassword($password);
80+
$user->setPasswordUpdateAt(new \DateTimeImmutable());
7281
}
7382
}
7483

@@ -96,32 +105,54 @@ public function changePassword(
96105
UserRepository $userRepository,
97106
CsrfTokenManagerInterface $csrfTokenManager,
98107
SettingsManager $settingsManager,
99-
UserPasswordHasherInterface $passwordHasher
108+
UserPasswordHasherInterface $passwordHasher,
109+
TokenStorageInterface $tokenStorage,
100110
): Response {
101-
/** @var User $user */
111+
/** @var ?User $user */
102112
$user = $this->getUser();
103113

104-
// Ensure user is authenticated and has proper interface
105-
if (!\is_object($user) || !$user instanceof UserInterface) {
106-
throw $this->createAccessDeniedException('This user does not have access to this section');
114+
if (!$user || !$user instanceof UserInterface) {
115+
$userId = $request->query->get('userId');
116+
//error_log("User not logged in. Received userId from query: " . $userId);
117+
118+
if (!$userId || !ctype_digit($userId)) {
119+
//error_log("Access denied: Missing or invalid userId.");
120+
throw $this->createAccessDeniedException('This user does not have access to this section.');
121+
}
122+
123+
$user = $userRepository->find((int)$userId);
124+
125+
if (!$user || !$user instanceof UserInterface) {
126+
//error_log("Access denied: User not found with ID $userId");
127+
throw $this->createAccessDeniedException('User not found or invalid.');
128+
}
129+
130+
//error_log("Loaded user by ID: " . $user->getId());
107131
}
108132

109-
// Build the form and inject user-related options
133+
$isRotation = $request->query->getBoolean('rotate', false);
134+
110135
$form = $this->createForm(ChangePasswordType::class, [
111136
'enable2FA' => $user->getMfaEnabled(),
112137
], [
113138
'user' => $user,
114139
'portal_name' => $settingsManager->getSetting('platform.institution'),
115140
'password_hasher' => $passwordHasher,
141+
'enable_2fa_field' => !$isRotation,
116142
]);
117-
118143
$form->handleRequest($request);
144+
119145
$session = $request->getSession();
120146
$qrCodeBase64 = null;
121147
$showQRCode = false;
122148

123-
// Generate TOTP secret and QR code for 2FA activation
124-
if ($form->get('enable2FA')->getData() && !$user->getMfaSecret()) {
149+
// Build QR code preview if user opts to enable 2FA but hasn't saved yet
150+
if (
151+
$form->isSubmitted()
152+
&& $form->has('enable2FA')
153+
&& $form->get('enable2FA')->getData()
154+
&& !$user->getMfaSecret()
155+
) {
125156
if (!$session->has('temporary_mfa_secret')) {
126157
$totp = TOTP::create();
127158
$secret = $totp->getSecret();
@@ -134,7 +165,6 @@ public function changePassword(
134165
$portalName = $settingsManager->getSetting('platform.institution');
135166
$totp->setLabel($portalName . ' - ' . $user->getEmail());
136167

137-
// Build QR code image
138168
$qrCodeResult = Builder::create()
139169
->writer(new PngWriter())
140170
->data($totp->getProvisioningUri())
@@ -148,47 +178,78 @@ public function changePassword(
148178
$showQRCode = true;
149179
}
150180

151-
// Handle form submission
152-
if ($form->isSubmitted() && $form->isValid()) {
153-
$newPassword = $form->get('newPassword')->getData();
154-
$enable2FA = $form->get('enable2FA')->getData();
155-
156-
// Enable 2FA and store encrypted secret
157-
if ($enable2FA && !$user->getMfaSecret() && $session->has('temporary_mfa_secret')) {
158-
$secret = $session->get('temporary_mfa_secret');
159-
$encryptedSecret = $this->encryptTOTPSecret($secret, $_ENV['APP_SECRET']);
160-
161-
$user->setMfaSecret($encryptedSecret);
162-
$user->setMfaEnabled(true);
163-
$user->setMfaService('TOTP');
164-
165-
$userRepository->updateUser($user);
166-
$session->remove('temporary_mfa_secret');
167-
168-
$this->addFlash('success', '2FA activated successfully.');
169-
return $this->redirectToRoute('chamilo_core_account_home');
170-
}
171-
172-
// Disable 2FA if it was previously enabled
173-
if (!$enable2FA && $user->getMfaEnabled()) {
174-
$user->setMfaEnabled(false);
175-
$user->setMfaSecret(null);
176-
177-
$userRepository->updateUser($user);
178-
$this->addFlash('success', '2FA disabled successfully.');
179-
return $this->redirectToRoute('chamilo_core_account_home');
180-
}
181-
182-
// Update password if provided
183-
if (!empty($newPassword)) {
184-
$user->setPlainPassword($newPassword);
185-
$userRepository->updateUser($user);
186-
$this->addFlash('success', 'Password updated successfully.');
187-
return $this->redirectToRoute('chamilo_core_account_home');
181+
if ($form->isSubmitted()) {
182+
if ($form->isValid()) {
183+
$submittedToken = $request->request->get('_token');
184+
if (!$csrfTokenManager->isTokenValid(new CsrfToken('change_password', $submittedToken))) {
185+
$form->addError(new FormError($this->translator->trans('CSRF token is invalid. Please try again.')));
186+
} else {
187+
$currentPassword = $form->get('currentPassword')->getData();
188+
$newPassword = $form->get('newPassword')->getData();
189+
$confirmPassword = $form->get('confirmPassword')->getData();
190+
$enable2FA = !$isRotation && $form->has('enable2FA')
191+
? $form->get('enable2FA')->getData()
192+
: false;
193+
194+
if ($enable2FA && !$user->getMfaSecret()) {
195+
$secret = $session->get('temporary_mfa_secret');
196+
$encryptedSecret = $this->encryptTOTPSecret($secret, $_ENV['APP_SECRET']);
197+
$user->setMfaSecret($encryptedSecret);
198+
$user->setMfaEnabled(true);
199+
$user->setMfaService('TOTP');
200+
$userRepository->updateUser($user);
201+
202+
$session->remove('temporary_mfa_secret');
203+
204+
$this->addFlash('success', '2FA activated successfully.');
205+
206+
return $this->redirectToRoute('chamilo_core_account_home');
207+
}
208+
209+
if (!$isRotation && !$enable2FA && $user->getMfaEnabled()) {
210+
$user->setMfaEnabled(false);
211+
$user->setMfaSecret(null);
212+
$userRepository->updateUser($user);
213+
$this->addFlash('success', '2FA disabled successfully.');
214+
}
215+
216+
if ($newPassword || $confirmPassword || $currentPassword) {
217+
if (!$userRepository->isPasswordValid($user, $currentPassword)) {
218+
$form->get('currentPassword')->addError(new FormError(
219+
$this->translator->trans('The current password is incorrect')
220+
));
221+
} elseif ($newPassword !== $confirmPassword) {
222+
$form->get('confirmPassword')->addError(new FormError(
223+
$this->translator->trans('Passwords do not match')
224+
));
225+
} else {
226+
$user->setPlainPassword($newPassword);
227+
$user->setPasswordUpdateAt(new \DateTimeImmutable());
228+
$userRepository->updateUser($user);
229+
$this->addFlash('success', 'Password updated successfully.');
230+
231+
// Re-login if the user was not logged
232+
if (!$this->getUser()) {
233+
$token = new UsernamePasswordToken(
234+
$user,
235+
'main',
236+
$user->getRoles()
237+
);
238+
$tokenStorage->setToken($token);
239+
$request->getSession()->set('_security_main', serialize($token));
240+
}
241+
242+
return $this->redirectToRoute('chamilo_core_account_home');
243+
}
244+
}
245+
}
246+
} else {
247+
error_log("Form is NOT valid.");
188248
}
249+
} else {
250+
error_log("Form NOT submitted yet.");
189251
}
190252

191-
// Render form with optional QR code for 2FA
192253
return $this->render('@ChamiloCore/Account/change_password.html.twig', [
193254
'form' => $form->createView(),
194255
'qrCode' => $qrCodeBase64,
@@ -206,18 +267,7 @@ private function encryptTOTPSecret(string $secret, string $encryptionKey): strin
206267
$iv = openssl_random_pseudo_bytes(openssl_cipher_iv_length($cipherMethod));
207268
$encryptedSecret = openssl_encrypt($secret, $cipherMethod, $encryptionKey, 0, $iv);
208269

209-
return base64_encode($iv.'::'.$encryptedSecret);
210-
}
211-
212-
/**
213-
* Validates the provided TOTP code for the given user.
214-
*/
215-
private function isTOTPValid(User $user, string $totpCode): bool
216-
{
217-
$decryptedSecret = $this->decryptTOTPSecret($user->getMfaSecret(), $_ENV['APP_SECRET']);
218-
$totp = TOTP::create($decryptedSecret);
219-
220-
return $totp->verify($totpCode);
270+
return base64_encode($iv . '::' . $encryptedSecret);
221271
}
222272

223273
/**

src/CoreBundle/Controller/SecurityController.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,24 @@ public function loginJson(
145145
]);
146146
}
147147

148+
// Password rotation check
149+
$days = (int) $this->settingsManager->getSetting('security.password_rotation_days', true);
150+
if ($days > 0) {
151+
$lastUpdate = $user->getPasswordUpdateAt() ?? $user->getCreatedAt();
152+
$diffDays = (new \DateTimeImmutable())->diff($lastUpdate)->days;
153+
154+
if ($diffDays > $days) {
155+
// Clean token & session
156+
$tokenStorage->setToken(null);
157+
$request->getSession()->invalidate();
158+
159+
return $this->json([
160+
'rotate_password' => true,
161+
'redirect' => '/account/change-password?rotate=1&userId=' . $user->getId(),
162+
]);
163+
}
164+
}
165+
148166
$data = null;
149167
if ($user) {
150168
$data = $this->serializer->serialize($user, 'jsonld', ['groups' => ['user_json:read']]);

src/CoreBundle/DataFixtures/SettingsCurrentFixtures.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3139,6 +3139,11 @@ public static function getNewConfigurationSettings(): array
31393139
],
31403140
],
31413141
'security' => [
3142+
[
3143+
'name' => 'password_rotation_days',
3144+
'title' => 'Password rotation interval (days)',
3145+
'comment' => 'Number of days before users must rotate their password (0 = disabled).',
3146+
],
31423147
[
31433148
'name' => 'password_requirements',
31443149
'title' => 'Minimal password syntax requirements',

0 commit comments

Comments
 (0)