Skip to content

Commit 8ce7a26

Browse files
fix(remember-login-token): avoid unecessary insert when migrating from old table
Signed-off-by: Cristian Scheid <cristianscheid@gmail.com>
1 parent 165b993 commit 8ce7a26

3 files changed

Lines changed: 25 additions & 33 deletions

File tree

‎core/Controller/LoginController.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ public function logout(): RedirectResponse {
8787
$affectedRows = $this->rememberLoginTokenMapper->deleteByToken($loginToken);
8888
if ($affectedRows < 1) {
8989
// TODO: remove this after migration to 'remember_login_tokens' table is finished
90-
$this->config->deleteUserValue($uid, 'login_token', $loginToken);
90+
$this->userConfig->deleteUserConfig($uid, 'login_token', $loginToken);
9191
}
9292
}
9393
$this->userSession->logout();

‎lib/private/User/Session.php‎

Lines changed: 23 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -908,27 +908,32 @@ public function loginWithCookie($uid, $currentToken, $oldSessionId) {
908908
return false;
909909
}
910910

911+
$isLegacyRememberLoginToken = false;
911912
try {
912913
// get stored token
913914
$rememberLoginToken = $this->rememberLoginTokenMapper->findByToken($currentToken);
915+
if ($rememberLoginToken->uid !== $uid) {
916+
$this->logger->warning('Tried to login using remember-me token token from a different user', [
917+
'app' => 'core',
918+
'user' => $uid,
919+
]);
920+
return false;
921+
}
914922
} catch (DoesNotExistException $ex) {
915923
// TODO: remove this after migration to 'remember_login_tokens' table is finished
916-
$rememberLoginToken = $this->migrateLegacyRememberLoginToken($uid, $currentToken);
917-
if ($rememberLoginToken === null) {
924+
$legacyRememberLoginTokens = $this->config->getUserKeys($uid, 'login_token');
925+
$isLegacyRememberLoginToken = in_array($currentToken, $legacyRememberLoginTokens, true);
926+
if ($isLegacyRememberLoginToken) {
927+
// remove token from 'preferences' table
928+
$this->config->deleteUserValue($uid, 'login_token', $currentToken);
929+
} else {
918930
$this->logger->info('Tried to log in but could not verify token', [
919931
'app' => 'core',
920932
'user' => $uid,
921933
]);
922934
return false;
923935
}
924936
}
925-
if ($rememberLoginToken->uid !== $uid) {
926-
$this->logger->warning('Tried to login using remember-me token token from a different user', [
927-
'app' => 'core',
928-
'user' => $uid,
929-
]);
930-
return false;
931-
}
932937

933938
try {
934939
$oldToken = $this->tokenProvider->getToken($oldSessionId);
@@ -949,9 +954,15 @@ public function loginWithCookie($uid, $currentToken, $oldSessionId) {
949954
return false;
950955
}
951956

952-
// replace successfully used token with a new one
953-
$newToken = $this->random->generate(32);
954-
$this->rememberLoginTokenMapper->rotateToken($currentToken, $newToken);
957+
if ($isLegacyRememberLoginToken) {
958+
// legacy token was removed from 'preferences' table
959+
// create new one on 'remember_login_tokens' table
960+
$newToken = $this->createRememberLoginToken($uid);
961+
} else {
962+
// replace successfully used token with a new one
963+
$newToken = $this->random->generate(32);
964+
$this->rememberLoginTokenMapper->rotateToken($currentToken, $newToken);
965+
}
955966
$this->logger->debug('Remember-me token replaced', [
956967
'app' => 'core',
957968
'user' => $uid,
@@ -1020,24 +1031,6 @@ private function createRememberLoginToken(string $uid): string {
10201031
return $token;
10211032
}
10221033

1023-
/**
1024-
* TODO: remove this after migration to 'remember_login_tokens' table is finished
1025-
*/
1026-
private function migrateLegacyRememberLoginToken(string $uid, string $token): ?RememberLoginToken {
1027-
$legacyTokens = $this->config->getUserKeys($uid, 'login_token');
1028-
if (!in_array($token, $legacyTokens, true)) {
1029-
return null;
1030-
}
1031-
1032-
$this->config->deleteUserValue($uid, 'login_token', $token);
1033-
1034-
$rememberLoginToken = new RememberLoginToken();
1035-
$rememberLoginToken->uid = $uid;
1036-
$rememberLoginToken->token = $token;
1037-
1038-
return $this->rememberLoginTokenMapper->insert($rememberLoginToken);
1039-
}
1040-
10411034
/**
10421035
* logout the user from the session
10431036
*/

‎tests/Core/Controller/LoginControllerTest.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,7 @@ class LoginControllerTest extends TestCase {
5757
private IAppManager&MockObject $appManager;
5858
private AlternativeLoginService&MockObject $alternativeLoginService;
5959

60-
/** @var RememberLoginTokenMapper|MockObject */
61-
private $rememberLoginTokenMapper;
60+
private RememberLoginTokenMapper&MockObject $rememberLoginTokenMapper;
6261

6362
#[\Override]
6463
protected function setUp(): void {

0 commit comments

Comments
 (0)