From 3e498e9c01491dfece4c0ebae03f454a53cded6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Tue, 26 Jun 2018 09:24:14 +0200 Subject: [PATCH] [stable10] If token based authentication is mandatory a login failed entry is written to the log so that fail2ban can handle this case. --- lib/private/User/Session.php | 34 ++++++++++++++++++++-------------- tests/lib/User/SessionTest.php | 19 ++++++++++++++----- 2 files changed, 34 insertions(+), 19 deletions(-) diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index c59aa6f0f098..aa06b2dab09a 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -354,6 +354,8 @@ public function logClientIn($user, $password, IRequest $request) { throw new \InvalidArgumentException('$user cannot be empty'); } if (!$isTokenPassword && $this->isTokenAuthEnforced()) { + $this->logger->warning("Login failed: '$user' (Remote IP: '{$request->getRemoteAddress()}')", ['app' => 'core']); + $this->emitFailedLogin($user); throw new PasswordLoginForbiddenException(); } if (!$isTokenPassword && $this->isTwoFactorEnforced($user)) { @@ -472,6 +474,7 @@ public function prepareUserLogin($firstTimeLogin = false) { * @todo do not allow basic auth if the user is 2FA enforced * @param IRequest $request * @return boolean if the login was successful + * @throws LoginException */ public function tryBasicAuthLogin(IRequest $request) { if (!empty($request->server['PHP_AUTH_USER']) && !empty($request->server['PHP_AUTH_PW'])) { @@ -503,7 +506,7 @@ public function tryBasicAuthLogin(IRequest $request) { * @param string $login * @param string $password * @return boolean - * @throws LoginException if an app canceld the login process or the user is not enabled + * @throws LoginException if an app canceled the login process or the user is not enabled * * Two new keys 'login' in the before event and 'user' in the after event * are introduced. We should use this keys in future when trying to listen @@ -534,16 +537,16 @@ private function loginWithPassword($login, $password) { $this->eventDispatcher->dispatch('user.afterlogin', $afterEvent); return true; - } else { - // injecting l10n does not work - there is a circular dependency between session and \OCP\L10N\IFactory - $message = \OC::$server->getL10N('lib')->t('Login canceled by app'); - throw new LoginException($message); } - } else { + // injecting l10n does not work - there is a circular dependency between session and \OCP\L10N\IFactory - $message = \OC::$server->getL10N('lib')->t('User disabled'); + $message = \OC::$server->getL10N('lib')->t('Login canceled by app'); throw new LoginException($message); } + + // injecting l10n does not work - there is a circular dependency between session and \OCP\L10N\IFactory + $message = \OC::$server->getL10N('lib')->t('User disabled'); + throw new LoginException($message); } /** @@ -551,7 +554,8 @@ private function loginWithPassword($login, $password) { * * @param string $token * @return boolean - * @throws LoginException if an app canceld the login process or the user is not enabled + * @throws LoginException if an app canceled the login process or the user is not enabled + * @throws InvalidTokenException */ private function loginWithToken($token) { try { @@ -684,16 +688,16 @@ public function loginWithApache(IApacheBackend $apacheBackend) { if ($this->isLoggedIn()) { $this->prepareUserLogin($firstTimeLogin); return true; - } else { - // injecting l10n does not work - there is a circular dependency between session and \OCP\L10N\IFactory - $message = \OC::$server->getL10N('lib')->t('Login canceled by app'); - throw new LoginException($message); } - } else { + // injecting l10n does not work - there is a circular dependency between session and \OCP\L10N\IFactory - $message = \OC::$server->getL10N('lib')->t('User disabled'); + $message = \OC::$server->getL10N('lib')->t('Login canceled by app'); throw new LoginException($message); } + + // injecting l10n does not work - there is a circular dependency between session and \OCP\L10N\IFactory + $message = \OC::$server->getL10N('lib')->t('User disabled'); + throw new LoginException($message); } /** @@ -904,6 +908,7 @@ private function validateToken($token, $user = null) { * @param IRequest $request * @todo check remember me cookie * @return boolean + * @throws LoginException */ public function tryTokenLogin(IRequest $request) { $authHeader = $request->getHeader('Authorization'); @@ -994,6 +999,7 @@ protected function loginUser($user, $password) { * @param string $uid the username * @param string $currentToken * @return bool + * @throws \OCP\PreConditionNotMetException */ public function loginWithCookie($uid, $currentToken) { $this->logger->debug( diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php index 7edf192f6396..7b9b14e46743 100644 --- a/tests/lib/User/SessionTest.php +++ b/tests/lib/User/SessionTest.php @@ -18,7 +18,7 @@ use OC\Authentication\Token\IToken; use OC\Security\CSRF\CsrfTokenManager; use OC\Session\Memory; -use OC\User\LoginException; + use OC\User\Manager; use OC\User\Session; use OCP\App\IServiceLoader; @@ -149,6 +149,7 @@ public function isLoggedInData() { /** * @dataProvider isLoggedInData + * @param $isLoggedIn */ public function testIsLoggedIn($isLoggedIn) { $session = $this->createMock(Memory::class); @@ -214,7 +215,7 @@ public function testLoginValidPasswordEnabled() { return false; break; } - }, 'foo')); + })); $managerMethods = get_class_methods(Manager::class); //keep following methods intact in order to ensure hooks are @@ -405,10 +406,14 @@ public function testLogClientInNoTokenPasswordWith2fa() { $session = $this->createMock(ISession::class); /** @var IRequest | \PHPUnit_Framework_MockObject_MockObject $request */ $request = $this->createMock(IRequest::class); + $request->method('getRemoteAddress')->willReturn('12.34.56.78'); + + /** @var EventDispatcher | \PHPUnit_Framework_MockObject_MockObject $eventDispatcher */ + $eventDispatcher = $this->createMock(EventDispatcher::class); /** @var Session $userSession */ $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $eventDispatcher]) ->setMethods(['login', 'supportsCookies', 'createSessionToken', 'getUser']) ->getMock(); @@ -420,6 +425,8 @@ public function testLogClientInNoTokenPasswordWith2fa() { ->method('getSystemValue') ->with('token_auth_enforced', false) ->will($this->returnValue(true)); + $this->logger->expects($this->once())->method('warning')->with("Login failed: 'john' (Remote IP: '12.34.56.78')"); + $eventDispatcher->expects($this->once())->method('dispatch')->with('user.loginfailed', new GenericEvent(null, ['user' => 'john'])); $userSession->logClientIn('john', 'doe', $request); } @@ -527,7 +534,7 @@ public function testRememberLoginValidToken() { default: return false; } - }, 'foo')); + })); $session->expects($this->once()) ->method('regenerateId'); @@ -1078,7 +1085,7 @@ public function testLogout() { ->method('getUID') ->will($this->returnValue('foo')); - /** @var Session | \PHPUnit_Framework_MockObject_MockObject $session */ + /** @var Session | \PHPUnit_Framework_MockObject_MockObject $userSession */ $userSession = $this->getMockBuilder(Session::class) ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $this->eventDispatcher]) ->setMethods(['getAuthModules', 'unsetMagicInCookie']) @@ -1367,6 +1374,7 @@ public function providesModules() { * @dataProvider providesModules * @param $expectedReturn * @param array $modules + * @param null $loggedInUser */ public function testVerifyAuthHeaders($expectedReturn, array $modules, $loggedInUser = null) { /** @var IRequest | \PHPUnit_Framework_MockObject_MockObject $request */ @@ -1415,6 +1423,7 @@ public function providesModulesForLogin() { * @dataProvider providesModulesForLogin * @param mixed $expectedReturn * @param array $modules + * @throws \Exception */ public function testTryAuthModuleLogin($expectedReturn, array $modules) { /** @var IRequest | \PHPUnit_Framework_MockObject_MockObject $request */