diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 255465c0ffb2e..c78351e33bfdd 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1324,6 +1324,7 @@ 'OC\\Authentication\\Login\\LoginData' => $baseDir . '/lib/private/Authentication/Login/LoginData.php', 'OC\\Authentication\\Login\\LoginResult' => $baseDir . '/lib/private/Authentication/Login/LoginResult.php', 'OC\\Authentication\\Login\\PreLoginHookCommand' => $baseDir . '/lib/private/Authentication/Login/PreLoginHookCommand.php', + 'OC\\Authentication\\Login\\RecordInteractiveLoginCommand' => $baseDir . '/lib/private/Authentication/Login/RecordInteractiveLoginCommand.php', 'OC\\Authentication\\Login\\SetUserTimezoneCommand' => $baseDir . '/lib/private/Authentication/Login/SetUserTimezoneCommand.php', 'OC\\Authentication\\Login\\TwoFactorCommand' => $baseDir . '/lib/private/Authentication/Login/TwoFactorCommand.php', 'OC\\Authentication\\Login\\UidLoginCommand' => $baseDir . '/lib/private/Authentication/Login/UidLoginCommand.php', @@ -2439,9 +2440,11 @@ 'OC\\User\\Database' => $baseDir . '/lib/private/User/Database.php', 'OC\\User\\DisabledUserException' => $baseDir . '/lib/private/User/DisabledUserException.php', 'OC\\User\\DisplayNameCache' => $baseDir . '/lib/private/User/DisplayNameCache.php', + 'OC\\User\\LastInteractiveLogin' => $baseDir . '/lib/private/User/LastInteractiveLogin.php', 'OC\\User\\LazyUser' => $baseDir . '/lib/private/User/LazyUser.php', 'OC\\User\\Listeners\\BeforeUserDeletedListener' => $baseDir . '/lib/private/User/Listeners/BeforeUserDeletedListener.php', 'OC\\User\\Listeners\\UserChangedListener' => $baseDir . '/lib/private/User/Listeners/UserChangedListener.php', + 'OC\\User\\Listeners\\UserLoggedInWithCookieListener' => $baseDir . '/lib/private/User/Listeners/UserLoggedInWithCookieListener.php', 'OC\\User\\LoginException' => $baseDir . '/lib/private/User/LoginException.php', 'OC\\User\\Manager' => $baseDir . '/lib/private/User/Manager.php', 'OC\\User\\NoUserException' => $baseDir . '/lib/private/User/NoUserException.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 046d2cd20b693..c6f1c84603329 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1365,6 +1365,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Authentication\\Login\\LoginData' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/LoginData.php', 'OC\\Authentication\\Login\\LoginResult' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/LoginResult.php', 'OC\\Authentication\\Login\\PreLoginHookCommand' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/PreLoginHookCommand.php', + 'OC\\Authentication\\Login\\RecordInteractiveLoginCommand' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/RecordInteractiveLoginCommand.php', 'OC\\Authentication\\Login\\SetUserTimezoneCommand' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/SetUserTimezoneCommand.php', 'OC\\Authentication\\Login\\TwoFactorCommand' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/TwoFactorCommand.php', 'OC\\Authentication\\Login\\UidLoginCommand' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/UidLoginCommand.php', @@ -2480,9 +2481,11 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\User\\Database' => __DIR__ . '/../../..' . '/lib/private/User/Database.php', 'OC\\User\\DisabledUserException' => __DIR__ . '/../../..' . '/lib/private/User/DisabledUserException.php', 'OC\\User\\DisplayNameCache' => __DIR__ . '/../../..' . '/lib/private/User/DisplayNameCache.php', + 'OC\\User\\LastInteractiveLogin' => __DIR__ . '/../../..' . '/lib/private/User/LastInteractiveLogin.php', 'OC\\User\\LazyUser' => __DIR__ . '/../../..' . '/lib/private/User/LazyUser.php', 'OC\\User\\Listeners\\BeforeUserDeletedListener' => __DIR__ . '/../../..' . '/lib/private/User/Listeners/BeforeUserDeletedListener.php', 'OC\\User\\Listeners\\UserChangedListener' => __DIR__ . '/../../..' . '/lib/private/User/Listeners/UserChangedListener.php', + 'OC\\User\\Listeners\\UserLoggedInWithCookieListener' => __DIR__ . '/../../..' . '/lib/private/User/Listeners/UserLoggedInWithCookieListener.php', 'OC\\User\\LoginException' => __DIR__ . '/../../..' . '/lib/private/User/LoginException.php', 'OC\\User\\Manager' => __DIR__ . '/../../..' . '/lib/private/User/Manager.php', 'OC\\User\\NoUserException' => __DIR__ . '/../../..' . '/lib/private/User/NoUserException.php', diff --git a/lib/private/Authentication/Login/Chain.php b/lib/private/Authentication/Login/Chain.php index 50be4ecb56c1a..9256db263b978 100644 --- a/lib/private/Authentication/Login/Chain.php +++ b/lib/private/Authentication/Login/Chain.php @@ -18,6 +18,7 @@ public function __construct( private CompleteLoginCommand $completeLoginCommand, private CreateSessionTokenCommand $createSessionTokenCommand, private ClearLostPasswordTokensCommand $clearLostPasswordTokensCommand, + private RecordInteractiveLoginCommand $recordInteractiveLoginCommand, private UpdateLastPasswordConfirmCommand $updateLastPasswordConfirmCommand, private SetUserTimezoneCommand $setUserTimezoneCommand, private TwoFactorCommand $twoFactorCommand, @@ -34,6 +35,7 @@ public function process(LoginData $loginData): LoginResult { ->setNext($this->completeLoginCommand) ->setNext($this->createSessionTokenCommand) ->setNext($this->clearLostPasswordTokensCommand) + ->setNext($this->recordInteractiveLoginCommand) ->setNext($this->updateLastPasswordConfirmCommand) ->setNext($this->setUserTimezoneCommand) ->setNext($this->twoFactorCommand) diff --git a/lib/private/Authentication/Login/RecordInteractiveLoginCommand.php b/lib/private/Authentication/Login/RecordInteractiveLoginCommand.php new file mode 100644 index 0000000000000..40ceb5a03d328 --- /dev/null +++ b/lib/private/Authentication/Login/RecordInteractiveLoginCommand.php @@ -0,0 +1,26 @@ +lastInteractiveLogin->record($loginData->getUser()); + + return $this->processNextOrFinishSuccessfully($loginData); + } +} diff --git a/lib/private/Security/VerificationToken/VerificationToken.php b/lib/private/Security/VerificationToken/VerificationToken.php index 13eec2b95bfbf..d03e52996ab41 100644 --- a/lib/private/Security/VerificationToken/VerificationToken.php +++ b/lib/private/Security/VerificationToken/VerificationToken.php @@ -8,6 +8,7 @@ namespace OC\Security\VerificationToken; +use OC\User\LastInteractiveLogin; use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\IJobList; use OCP\IConfig; @@ -27,6 +28,7 @@ public function __construct( private ITimeFactory $timeFactory, private ISecureRandom $secureRandom, private IJobList $jobList, + private LastInteractiveLogin $lastInteractiveLogin, ) { } @@ -71,7 +73,7 @@ public function check( } if ($splitToken[0] < ($this->timeFactory->getTime() - self::TOKEN_LIFETIME) - || ($expiresWithLogin && $user->getLastLogin() > $splitToken[0])) { + || ($expiresWithLogin && $this->lastInteractiveLogin->get($user) > $splitToken[0])) { $this->throwInvalidTokenException(InvalidTokenException::TOKEN_EXPIRED); } diff --git a/lib/private/Server.php b/lib/private/Server.php index 92b2b4bebbb43..1115ec3aeca45 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -154,6 +154,7 @@ use OC\User\DisplayNameCache; use OC\User\Listeners\BeforeUserDeletedListener; use OC\User\Listeners\UserChangedListener; +use OC\User\Listeners\UserLoggedInWithCookieListener; use OC\User\Session; use OC\User\User; use OCA\Theming\ImageManager; @@ -1136,6 +1137,7 @@ private function connectDispatcher(): void { $eventDispatcher->addServiceListener(BeforeUserDeletedEvent::class, BeforeUserDeletedListener::class); $eventDispatcher->addServiceListener(UserDeletedEvent::class, SubAdmin::class); $eventDispatcher->addServiceListener(GroupDeletedEvent::class, SubAdmin::class); + $eventDispatcher->addServiceListener(UserLoggedInWithCookieEvent::class, UserLoggedInWithCookieListener::class); FilesMetadataManager::loadListeners($eventDispatcher); GenerateBlurhashMetadata::loadListeners($eventDispatcher); diff --git a/lib/private/User/LastInteractiveLogin.php b/lib/private/User/LastInteractiveLogin.php new file mode 100644 index 0000000000000..dc84c3d448747 --- /dev/null +++ b/lib/private/User/LastInteractiveLogin.php @@ -0,0 +1,52 @@ +userConfig->setValueInt( + $user->getUID(), + self::CONFIG_APP, + self::CONFIG_KEY, + $this->timeFactory->getTime(), + lazy: true, + ); + } + + public function get(IUser $user): int { + return $this->userConfig->getValueInt( + $user->getUID(), + self::CONFIG_APP, + self::CONFIG_KEY, + lazy: true, + ); + } +} diff --git a/lib/private/User/Listeners/UserLoggedInWithCookieListener.php b/lib/private/User/Listeners/UserLoggedInWithCookieListener.php new file mode 100644 index 0000000000000..024eabe8f0a3d --- /dev/null +++ b/lib/private/User/Listeners/UserLoggedInWithCookieListener.php @@ -0,0 +1,34 @@ + + */ +class UserLoggedInWithCookieListener implements IEventListener { + public function __construct( + private LastInteractiveLogin $lastInteractiveLogin, + ) { + } + + #[\Override] + public function handle(Event $event): void { + if (!($event instanceof UserLoggedInWithCookieEvent)) { + return; + } + + $this->lastInteractiveLogin->record($event->getUser()); + } +} diff --git a/tests/lib/Authentication/Login/RecordInteractiveLoginCommandTest.php b/tests/lib/Authentication/Login/RecordInteractiveLoginCommandTest.php new file mode 100644 index 0000000000000..7844a41eab549 --- /dev/null +++ b/tests/lib/Authentication/Login/RecordInteractiveLoginCommandTest.php @@ -0,0 +1,41 @@ +lastInteractiveLogin = $this->createMock(LastInteractiveLogin::class); + + $this->cmd = new RecordInteractiveLoginCommand( + $this->lastInteractiveLogin + ); + } + + public function testProcess(): void { + $data = $this->getLoggedInLoginData(); + $this->lastInteractiveLogin->expects($this->once()) + ->method('record') + ->with($this->user); + + $result = $this->cmd->process($data); + + $this->assertTrue($result->isSuccess()); + } +} diff --git a/tests/lib/Security/VerificationToken/VerificationTokenTest.php b/tests/lib/Security/VerificationToken/VerificationTokenTest.php index ed5890afba5a0..7976580617fe6 100644 --- a/tests/lib/Security/VerificationToken/VerificationTokenTest.php +++ b/tests/lib/Security/VerificationToken/VerificationTokenTest.php @@ -10,6 +10,7 @@ namespace Test\Security\VerificationToken; use OC\Security\VerificationToken\VerificationToken; +use OC\User\LastInteractiveLogin; use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\IJobList; use OCP\IConfig; @@ -33,6 +34,8 @@ class VerificationTokenTest extends TestCase { protected $timeFactory; /** @var IJobList|MockObject */ protected $jobList; + /** @var LastInteractiveLogin|MockObject */ + protected $lastInteractiveLogin; #[\Override] protected function setUp(): void { @@ -43,16 +46,24 @@ protected function setUp(): void { $this->timeFactory = $this->createMock(ITimeFactory::class); $this->secureRandom = $this->createMock(ISecureRandom::class); $this->jobList = $this->createMock(IJobList::class); + $this->lastInteractiveLogin = $this->createMock(LastInteractiveLogin::class); $this->token = new VerificationToken( $this->config, $this->crypto, $this->timeFactory, $this->secureRandom, - $this->jobList + $this->jobList, + $this->lastInteractiveLogin ); } + protected function mockLastInteractiveLogin(int $timestamp): void { + $this->lastInteractiveLogin->expects($this->atLeastOnce()) + ->method('get') + ->willReturn($timestamp); + } + public function testTokenUserUnknown(): void { $this->expectException(InvalidTokenException::class); $this->expectExceptionCode(InvalidTokenException::USER_UNKNOWN); @@ -148,9 +159,6 @@ public function testTokenExpired(): void { $user->expects($this->atLeastOnce()) ->method('getUID') ->willReturn('alice'); - $user->expects($this->any()) - ->method('getLastLogin') - ->willReturn(604803); $this->config->expects($this->atLeastOnce()) ->method('getUserValue') @@ -182,9 +190,7 @@ public function testTokenExpiredByLogin(): void { $user->expects($this->atLeastOnce()) ->method('getUID') ->willReturn('alice'); - $user->expects($this->any()) - ->method('getLastLogin') - ->willReturn(604803); + $this->mockLastInteractiveLogin(604803); $this->config->expects($this->atLeastOnce()) ->method('getUserValue') @@ -208,6 +214,39 @@ public function testTokenExpiredByLogin(): void { $this->token->check('encryptedToken', $user, 'fingerprintToken', 'foobar', true); } + public function testTokenNotExpiredBySessionRevalidation(): void { + $user = $this->createMock(IUser::class); + $user->expects($this->atLeastOnce()) + ->method('isEnabled') + ->willReturn(true); + $user->expects($this->atLeastOnce()) + ->method('getUID') + ->willReturn('alice'); + $user->expects($this->never()) + ->method('getLastLogin'); + // last actual authentication predates the token + $this->mockLastInteractiveLogin(604700); + + $this->config->expects($this->atLeastOnce()) + ->method('getUserValue') + ->with('alice', 'core', 'fingerprintToken', null) + ->willReturn('encryptedToken'); + $this->config->expects($this->any()) + ->method('getSystemValueString') + ->with('secret') + ->willReturn('357111317'); + + $this->crypto->method('decrypt') + ->with('encryptedToken', 'foobar' . '357111317') + ->willReturn('604800:barfoo'); + + $this->timeFactory->expects($this->any()) + ->method('getTime') + ->willReturn(604801); + + $this->token->check('barfoo', $user, 'fingerprintToken', 'foobar', true); + } + public function testTokenMismatch(): void { $user = $this->createMock(IUser::class); $user->expects($this->atLeastOnce()) @@ -216,9 +255,6 @@ public function testTokenMismatch(): void { $user->expects($this->atLeastOnce()) ->method('getUID') ->willReturn('alice'); - $user->expects($this->any()) - ->method('getLastLogin') - ->willReturn(604703); $this->config->expects($this->atLeastOnce()) ->method('getUserValue') @@ -250,9 +286,6 @@ public function testTokenSuccess(): void { $user->expects($this->atLeastOnce()) ->method('getUID') ->willReturn('alice'); - $user->expects($this->any()) - ->method('getLastLogin') - ->willReturn(604703); $this->config->expects($this->atLeastOnce()) ->method('getUserValue') diff --git a/tests/lib/User/Listeners/UserLoggedInWithCookieListenerTest.php b/tests/lib/User/Listeners/UserLoggedInWithCookieListenerTest.php new file mode 100644 index 0000000000000..ed5f0fb89a328 --- /dev/null +++ b/tests/lib/User/Listeners/UserLoggedInWithCookieListenerTest.php @@ -0,0 +1,49 @@ +lastInteractiveLogin = $this->createMock(LastInteractiveLogin::class); + $this->listener = new UserLoggedInWithCookieListener($this->lastInteractiveLogin); + } + + public function testRecordsRememberMeLogin(): void { + $user = $this->createMock(IUser::class); + $this->lastInteractiveLogin->expects($this->once()) + ->method('record') + ->with($user); + + $this->listener->handle(new UserLoggedInWithCookieEvent($user, null)); + } + + public function testIgnoresUnrelatedEvent(): void { + $user = $this->createMock(IUser::class); + $this->lastInteractiveLogin->expects($this->never()) + ->method('record'); + + $this->listener->handle(new UserLoggedInEvent($user, 'user', null, false)); + } +}