Skip to content

Commit

Permalink
Merge pull request #31948 from owncloud/stable10-0bdf7031e271cf1ba587…
Browse files Browse the repository at this point in the history 
…cd0125d831b603e1fafb

[stable10] If token based authentication is mandatory a login failed …
  • Loading branch information
@DeepDiver1975
DeepDiver1975 committed Jul 18, 2018
2 parents 48aac04 + 3e498e9 commit 3e0d712
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 19 deletions.
34 changes: 20 additions & 14 deletions lib/private/User/Session.php
View file
Expand Up @@ -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)) {
Expand Down Expand Up @@ -473,6 +475,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'])) {
Expand Down Expand Up @@ -504,7 +507,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
Expand Down Expand Up @@ -534,24 +537,25 @@ 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);
}

/**
* Log an user in with a given token (id)
*
* @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 {
Expand Down Expand Up @@ -683,16 +687,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);
}

/**
Expand Down Expand Up @@ -916,6 +920,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');
Expand Down Expand Up @@ -1009,6 +1014,7 @@ protected function loginUser(IUser $user = null, $password) {
* @param string $uid the username
* @param string $currentToken
* @return bool
* @throws \OCP\PreConditionNotMetException
*/
public function loginWithCookie($uid, $currentToken) {
$this->logger->debug(
Expand Down
19 changes: 14 additions & 5 deletions tests/lib/User/SessionTest.php
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -149,6 +149,7 @@ public function isLoggedInData() {

/**
* @dataProvider isLoggedInData
* @param $isLoggedIn
*/
public function testIsLoggedIn($isLoggedIn) {
$session = $this->createMock(Memory::class);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();

Expand All @@ -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);
}
Expand Down Expand Up @@ -527,7 +534,7 @@ public function testRememberLoginValidToken() {
default:
return false;
}
}, 'foo'));
}));
$session->expects($this->once())
->method('regenerateId');

Expand Down Expand Up @@ -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'])
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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 */
Expand Down

0 comments on commit 3e0d712

Please sign in to comment.