Merge pull request #37235 from nextcloud/backport/37227/stable26

[stable26] chore: use local variable for remote address
This commit is contained in:
Arthur Schiwon 2023-03-16 10:38:24 +01:00 committed by GitHub
commit 45ecf37248
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 113 additions and 10 deletions

View file

@ -59,6 +59,7 @@ use OCP\ISession;
use OCP\IUser;
use OCP\IUserSession;
use OCP\Lockdown\ILockdownManager;
use OCP\Security\Bruteforce\IThrottler;
use OCP\Security\ISecureRandom;
use OCP\Session\Exceptions\SessionNotAvailableException;
use OCP\User\Events\PostLoginEvent;
@ -426,7 +427,8 @@ class Session implements IUserSession, Emitter {
$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]);
@ -450,19 +452,12 @@ class Session implements IUserSession, Emitter {
if (!$this->login($user, $password)) {
// 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);
if (!(\count($users) === 1 && $this->login($users[0]->getUID(), $password))) {
$this->logger->warning('Login failed: \'' . $user . '\' (Remote IP: \'' . \OC::$server->getRequest()->getRemoteAddress() . '\')', ['app' => 'core']);
$throttler->registerAttempt('login', $request->getRemoteAddress(), ['user' => $user]);
$this->dispatcher->dispatchTyped(new OC\Authentication\Events\LoginFailed($user, $password));
if ($currentDelay === 0) {
$throttler->sleepDelay($request->getRemoteAddress(), 'login');
}
$this->handleLoginFailed($throttler, $currentDelay, $remoteAddress, $user, $password);
return false;
}
}
@ -477,6 +472,17 @@ class Session implements IUserSession, Emitter {
return true;
}
private function handleLoginFailed(IThrottler $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, $password));
if ($currentDelay === 0) {
$throttler->sleepDelay($remoteAddress, 'login');
}
}
protected function supportsCookies(IRequest $request) {
if (!is_null($request->getCookie('cookie_test'))) {
return true;

View file

@ -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;
@ -1057,4 +1058,100 @@ class SessionTest extends \Test\TestCase {
$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('john@foo.bar', 'I-AM-AN-PASSWORD')
->willReturn(false);
$manager
->method('getByEmail')
->with('john@foo.bar')
->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' => 'john@foo.bar']);
$this->dispatcher
->expects($this->once())
->method('dispatchTyped')
->with(new LoginFailed('john@foo.bar', 'I-AM-AN-PASSWORD'));
$this->assertFalse($userSession->logClientIn('john@foo.bar', 'I-AM-AN-PASSWORD', $request, $this->throttler));
}
}