chore: use local variable for remote address

Signed-off-by: Daniel Kesselberg <[email protected]>

lib/private/User/Session.php

@@ -59,6 +59,7 @@
 use OCP\IUser;
 use OCP\IUserSession;
 use OCP\Lockdown\ILockdownManager;
+use OC\Security\Bruteforce\Throttler;
 use OCP\Security\ISecureRandom;
 use OCP\Session\Exceptions\SessionNotAvailableException;
 use OCP\User\Events\PostLoginEvent;
@@ -90,7 +91,7 @@
  */
 class Session implements IUserSession, Emitter {
 
-	/** @var Manager|PublicEmitter $manager */
+	/** @var Manager $manager */
 	private $manager;
 
 	/** @var ISession $session */
@@ -288,9 +289,9 @@ public function setLoginName($loginName) {
 	}
 
 	/**
-	 * get the login name of the current user
+	 * Get the login name of the current user
 	 *
-	 * @return string
+	 * @return ?string
 	 */
 	public function getLoginName() {
 		if ($this->activeUser) {
@@ -426,7 +427,8 @@ public function logClientIn($user,
 								$password,
 								IRequest $request,
 								OC\Security\Bruteforce\Throttler $throttler) {
-		$currentDelay = $throttler->sleepDelay($request->getRemoteAddress(), 'login');
+		$remoteAddress = $request->getRemoteAddress();
+		$currentDelay = $throttler->sleepDelay($remoteAddress, 'login');
 
 		if ($this->manager instanceof PublicEmitter) {
 			$this->manager->emit('\OC\User', 'preLogin', [$user, $password]);
@@ -451,6 +453,7 @@ public function logClientIn($user,
 
 			// Failed, maybe the user used their email address
 			if (!filter_var($user, FILTER_VALIDATE_EMAIL)) {
+				$this->handleLoginFailed($throttler, $currentDelay, $remoteAddress, $user, $password);
 				return false;
 			}
 			$users = $this->manager->getByEmail($user);
@@ -478,6 +481,17 @@ public function logClientIn($user,
 		return true;
 	}
 
+	private function handleLoginFailed(Throttler $throttler, int $currentDelay, string $remoteAddress, string $user, ?string $password) {
+		$this->logger->warning("Login failed: '" . $user . "' (Remote IP: '" . $remoteAddress . "')", ['app' => 'core']);
+
+		$throttler->registerAttempt('login', $remoteAddress, ['user' => $user]);
+		$this->dispatcher->dispatchTyped(new OC\Authentication\Events\LoginFailed($user));
+
+		if ($currentDelay === 0) {
+			$throttler->sleepDelay($remoteAddress, 'login');
+		}
+	}
+
 	protected function supportsCookies(IRequest $request) {
 		if (!is_null($request->getCookie('cookie_test'))) {
 			return true;
@@ -877,7 +891,7 @@ public function loginWithCookie($uid, $currentToken, $oldSessionId) {
 		// replace successfully used token with a new one
 		$this->config->deleteUserValue($uid, 'login_token', $currentToken);
 		$newToken = $this->random->generate(32);
-		$this->config->setUserValue($uid, 'login_token', $newToken, $this->timeFactory->getTime());
+		$this->config->setUserValue($uid, 'login_token', $newToken, (string)$this->timeFactory->getTime());
 
 		try {
 			$sessionId = $this->session->getId();
@@ -916,7 +930,7 @@ public function loginWithCookie($uid, $currentToken, $oldSessionId) {
 	 */
 	public function createRememberMeToken(IUser $user) {
 		$token = $this->random->generate(32);
-		$this->config->setUserValue($user->getUID(), 'login_token', $token, $this->timeFactory->getTime());
+		$this->config->setUserValue($user->getUID(), 'login_token', $token, (string)$this->timeFactory->getTime());
 		$this->setMagicInCookie($user->getUID(), $token);
 	}
 

tests/lib/User/SessionTest.php

@@ -9,6 +9,7 @@
 namespace Test\User;
 
 use OC\AppFramework\Http\Request;
+use OC\Authentication\Events\LoginFailed;
 use OC\Authentication\Exceptions\InvalidTokenException;
 use OC\Authentication\Exceptions\PasswordLoginForbiddenException;
 use OC\Authentication\Token\IProvider;
@@ -1055,4 +1056,100 @@ public function testUpdateTokens() {
 
 		$this->userSession->updateTokens('uid', 'pass');
 	}
+
+	public function testLogClientInThrottlerUsername() {
+		$manager = $this->createMock(Manager::class);
+		$session = $this->createMock(ISession::class);
+		$request = $this->createMock(IRequest::class);
+
+		/** @var Session $userSession */
+		$userSession = $this->getMockBuilder(Session::class)
+			->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher])
+			->setMethods(['isTokenPassword', 'login', 'supportsCookies', 'createSessionToken', 'getUser'])
+			->getMock();
+
+		$userSession->expects($this->once())
+			->method('isTokenPassword')
+			->willReturn(true);
+		$userSession->expects($this->once())
+			->method('login')
+			->with('john', 'I-AM-AN-PASSWORD')
+			->willReturn(false);
+
+		$session->expects($this->never())
+			->method('set');
+		$request
+			->method('getRemoteAddress')
+			->willReturn('192.168.0.1');
+		$this->throttler
+			->expects($this->exactly(2))
+			->method('sleepDelay')
+			->with('192.168.0.1');
+		$this->throttler
+			->expects($this->any())
+			->method('getDelay')
+			->with('192.168.0.1')
+			->willReturn(0);
+
+		$this->throttler
+			->expects($this->once())
+			->method('registerAttempt')
+			->with('login', '192.168.0.1', ['user' => 'john']);
+		$this->dispatcher
+			->expects($this->once())
+			->method('dispatchTyped')
+			->with(new LoginFailed('john', 'I-AM-AN-PASSWORD'));
+
+		$this->assertFalse($userSession->logClientIn('john', 'I-AM-AN-PASSWORD', $request, $this->throttler));
+	}
+
+	public function testLogClientInThrottlerEmail() {
+		$manager = $this->createMock(Manager::class);
+		$session = $this->createMock(ISession::class);
+		$request = $this->createMock(IRequest::class);
+
+		/** @var Session $userSession */
+		$userSession = $this->getMockBuilder(Session::class)
+			->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher])
+			->setMethods(['isTokenPassword', 'login', 'supportsCookies', 'createSessionToken', 'getUser'])
+			->getMock();
+
+		$userSession->expects($this->once())
+			->method('isTokenPassword')
+			->willReturn(true);
+		$userSession->expects($this->once())
+			->method('login')
+			->with('[email protected]', 'I-AM-AN-PASSWORD')
+			->willReturn(false);
+		$manager
+			->method('getByEmail')
+			->with('[email protected]')
+			->willReturn([]);
+
+		$session->expects($this->never())
+			->method('set');
+		$request
+			->method('getRemoteAddress')
+			->willReturn('192.168.0.1');
+		$this->throttler
+			->expects($this->exactly(2))
+			->method('sleepDelay')
+			->with('192.168.0.1');
+		$this->throttler
+			->expects($this->any())
+			->method('getDelay')
+			->with('192.168.0.1')
+			->willReturn(0);
+
+		$this->throttler
+			->expects($this->once())
+			->method('registerAttempt')
+			->with('login', '192.168.0.1', ['user' => '[email protected]']);
+		$this->dispatcher
+			->expects($this->once())
+			->method('dispatchTyped')
+			->with(new LoginFailed('[email protected]', 'I-AM-AN-PASSWORD'));
+
+		$this->assertFalse($userSession->logClientIn('[email protected]', 'I-AM-AN-PASSWORD', $request, $this->throttler));
+	}
 }