New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

do not allow client password logins if token auth is enforced or 2FA … #24811

Merged
merged 2 commits into from May 25, 2016
Jump to file or symbol
Failed to load files and symbols.
+152 −25
Diff settings

Always

Just for now

@@ -103,8 +103,7 @@ protected function validateUserPass($username, $password) {
return true;
} else {
\OC_Util::setUpFS(); //login hooks may need early access to the filesystem
// TODO: do not allow basic auth if the user is 2FA enforced
if($this->userSession->login($username, $password)) {
if($this->userSession->logClientIn($username, $password)) {
$this->userSession->createSessionToken($this->request, $this->userSession->getUser()->getUID(), $username, $password);
\OC_Util::setUpFS($this->userSession->getUser()->getUID());
$this->session->set(self::DAV_AUTHENTICATED, $this->userSession->getUser()->getUID());
@@ -167,7 +167,7 @@ public function testValidateUserPassOfInvalidDAVAuthenticatedUserWithValidPasswo
->will($this->returnValue('AnotherUser'));
$this->userSession
->expects($this->once())
->method('login')
->method('logClientIn')
->with('MyTestUser', 'MyTestPassword')
->will($this->returnValue(true));
$this->userSession
@@ -192,7 +192,7 @@ public function testValidateUserPassWithInvalidPassword() {
->will($this->returnValue(false));
$this->userSession
->expects($this->once())
->method('login')
->method('logClientIn')
->with('MyTestUser', 'MyTestPassword')
->will($this->returnValue(false));
$this->session
@@ -560,7 +560,7 @@ public function testAuthenticateValidCredentials() {
->getMock();
$this->userSession
->expects($this->once())
->method('login')
->method('logClientIn')
->with('username', 'password')
->will($this->returnValue(true));
$this->userSession
@@ -602,7 +602,7 @@ public function testAuthenticateInvalidCredentials() {
->getMock();
$this->userSession
->expects($this->once())
->method('login')
->method('logClientIn')
->with('username', 'password')
->will($this->returnValue(false));
$response = $this->auth->check($server->httpRequest, $server->httpResponse);
View
@@ -194,6 +194,13 @@
*/
'session_keepalive' => true,
/**
* Enforce token authentication for clients, which blocks requests using the user
* password for enhanced security. Users need to generate tokens in personal settings
* which can be used as passwords on their clients.
*/
'token_auth_enforced' => false,
/**
* The directory where the skeleton files are located. These files will be
* copied to the data directory of new users. Leave empty to not copy any
View
@@ -236,7 +236,7 @@ public function __construct($webRoot, \OC\Config $config) {
$defaultTokenProvider = null;
}
$userSession = new \OC\User\Session($manager, $session, $timeFactory, $defaultTokenProvider);
$userSession = new \OC\User\Session($manager, $session, $timeFactory, $defaultTokenProvider, $c->getConfig());
$userSession->listen('\OC\User', 'preCreateUser', function ($uid, $password) {
\OC_Hook::emit('OC_User', 'pre_createUser', array('run' => true, 'uid' => $uid, 'password' => $password));
});
@@ -42,6 +42,7 @@
use OC_Util;
use OCA\DAV\Connector\Sabre\Auth;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\IConfig;
use OCP\IRequest;
use OCP\ISession;
use OCP\IUser;
@@ -68,8 +69,8 @@
* @package OC\User
*/
class Session implements IUserSession, Emitter {
/** @var Manager $manager */
/** @var IUserManager $manager */
private $manager;
/** @var ISession $session */
@@ -81,6 +82,9 @@ class Session implements IUserSession, Emitter {
/** @var IProvider */
private $tokenProvider;
/** @var IConfig */
private $config;
/** @var User $activeUser */
protected $activeUser;
@@ -89,12 +93,14 @@ class Session implements IUserSession, Emitter {
* @param ISession $session
* @param ITimeFactory $timeFacory
* @param IProvider $tokenProvider
* @param IConfig $config
*/
public function __construct(IUserManager $manager, ISession $session, ITimeFactory $timeFacory, $tokenProvider) {
public function __construct(IUserManager $manager, ISession $session, ITimeFactory $timeFacory, $tokenProvider, IConfig $config) {
$this->manager = $manager;
$this->session = $session;
$this->timeFacory = $timeFacory;
$this->tokenProvider = $tokenProvider;
$this->config = $config;
}
/**
@@ -279,7 +285,7 @@ public function getLoginName() {
}
/**
* try to login with the provided credentials
* try to log in with the provided credentials
*
* @param string $uid
* @param string $password
@@ -327,6 +333,63 @@ public function login($uid, $password) {
return false;
}
/**
* Tries to log in a client
*
* Checks token auth enforced
* Checks 2FA enabled
*
* @param string $user
* @param string $password
* @throws LoginException

This comment has been minimized.

@nickvergessen

nickvergessen May 24, 2016

Contributor

Are you sure?

@nickvergessen

nickvergessen May 24, 2016

Contributor

Are you sure?

This comment has been minimized.

@ChristophWurst

ChristophWurst May 24, 2016

Contributor

yes, login() throws exceptions

@ChristophWurst

ChristophWurst May 24, 2016

Contributor

yes, login() throws exceptions

This comment has been minimized.

@nickvergessen

nickvergessen May 24, 2016

Contributor

ah missed the return line

@nickvergessen

nickvergessen May 24, 2016

Contributor

ah missed the return line

* @return boolean
*/
public function logClientIn($user, $password) {
$isTokenPassword = $this->isTokenPassword($password);
if (!$isTokenPassword && $this->isTokenAuthEnforced()) {
// TODO: throw LoginException instead (https://github.com/owncloud/core/pull/24616)
return false;
}
if (!$isTokenPassword && $this->isTwoFactorEnforced($user)) {
// TODO: throw LoginException instead (https://github.com/owncloud/core/pull/24616)
return false;
}
return $this->login($user, $password);
}
private function isTokenAuthEnforced() {
return $this->config->getSystemValue('token_auth_enforced', false);
}
protected function isTwoFactorEnforced($username) {
\OCP\Util::emitHook(
'\OCA\Files_Sharing\API\Server2Server',
'preLoginNameUsedAsUserName',
array('uid' => &$username)
);
$user = $this->manager->get($username);
if (is_null($user)) {
return true;
}
// DI not possible due to cyclic dependencies :'-/
return OC::$server->getTwoFactorAuthManager()->isTwoFactorAuthenticated($user);
}
/**
* Check if the given 'password' is actually a device token
*
* @param type $password
* @return boolean
*/
public function isTokenPassword($password) {
try {
$this->tokenProvider->getToken($password);
return true;
} catch (InvalidTokenException $ex) {
return false;
}
}
protected function prepareUserLogin() {
// TODO: mock/inject/use non-static
// Refresh the token
@@ -347,7 +410,7 @@ protected function prepareUserLogin() {
*/
public function tryBasicAuthLogin(IRequest $request) {
if (!empty($request->server['PHP_AUTH_USER']) && !empty($request->server['PHP_AUTH_PW'])) {
$result = $this->login($request->server['PHP_AUTH_USER'], $request->server['PHP_AUTH_PW']);
$result = $this->logClientIn($request->server['PHP_AUTH_USER'], $request->server['PHP_AUTH_PW']);
if ($result === true) {
/**
* Add DAV authenticated. This should in an ideal world not be
@@ -24,6 +24,9 @@ class SessionTest extends \Test\TestCase {
/** @var \OC\Authentication\Token\DefaultTokenProvider */
protected $defaultProvider;
/** @var \OCP\IConfig */
private $config;
protected function setUp() {
parent::setUp();
@@ -34,6 +37,7 @@ protected function setUp() {
$this->defaultProvider = $this->getMockBuilder('\OC\Authentication\Token\DefaultTokenProvider')
->disableOriginalConstructor()
->getMock();
$this->config = $this->getMock('\OCP\IConfig');
}
public function testGetUser() {
@@ -95,7 +99,7 @@ public function testGetUser() {
->with($expectedUser->getUID())
->will($this->returnValue($expectedUser));
$userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider);
$userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, $this->config);
$user = $userSession->getUser();
$this->assertSame($expectedUser, $user);
}
@@ -118,7 +122,7 @@ public function testIsLoggedIn($isLoggedIn) {
->getMock();
$userSession = $this->getMockBuilder('\OC\User\Session')
->setConstructorArgs([$manager, $session, $this->timeFactory, $this->defaultProvider])
->setConstructorArgs([$manager, $session, $this->timeFactory, $this->defaultProvider, $this->config])
->setMethods([
'getUser'
])
@@ -145,7 +149,7 @@ public function testSetUser() {
->method('getUID')
->will($this->returnValue('foo'));
$userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider);
$userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, $this->config);
$userSession->setUser($user);
}
@@ -197,7 +201,7 @@ public function testLoginValidPasswordEnabled() {
->will($this->returnValue($user));
$userSession = $this->getMockBuilder('\OC\User\Session')
->setConstructorArgs([$manager, $session, $this->timeFactory, $this->defaultProvider])
->setConstructorArgs([$manager, $session, $this->timeFactory, $this->defaultProvider, $this->config])
->setMethods([
'prepareUserLogin'
])
@@ -244,7 +248,7 @@ public function testLoginValidPasswordDisabled() {
->with('foo', 'bar')
->will($this->returnValue($user));
$userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider);
$userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, $this->config);
$userSession->login('foo', 'bar');
}
@@ -280,7 +284,7 @@ public function testLoginInvalidPassword() {
->with('foo', 'bar')
->will($this->returnValue(false));
$userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider);
$userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, $this->config);
$userSession->login('foo', 'bar');
}
@@ -300,10 +304,64 @@ public function testLoginNonExisting() {
->with('foo', 'bar')
->will($this->returnValue(false));
$userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider);
$userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, $this->config);
$userSession->login('foo', 'bar');
}
public function testLogClientInNoTokenPasswordWith2fa() {
$manager = $this->getMockBuilder('\OC\User\Manager')
->disableOriginalConstructor()
->getMock();
$session = $this->getMock('\OCP\ISession');
/** @var \OC\User\Session $userSession */
$userSession = $this->getMockBuilder('\OC\User\Session')
->setConstructorArgs([$manager, $session, $this->timeFactory, $this->defaultProvider, $this->config])
->setMethods(['login'])
->getMock();
$this->defaultProvider->expects($this->once())
->method('getToken')
->with('doe')
->will($this->throwException(new \OC\Authentication\Exceptions\InvalidTokenException()));
$this->config->expects($this->once())
->method('getSystemValue')
->with('token_auth_enforced', false)
->will($this->returnValue(true));
$this->assertFalse($userSession->logClientIn('john', 'doe'));
}
public function testLogClientInNoTokenPasswordNo2fa() {
$manager = $this->getMockBuilder('\OC\User\Manager')
->disableOriginalConstructor()
->getMock();
$session = $this->getMock('\OCP\ISession');
$user = $this->getMock('\OCP\IUser');
/** @var \OC\User\Session $userSession */
$userSession = $this->getMockBuilder('\OC\User\Session')
->setConstructorArgs([$manager, $session, $this->timeFactory, $this->defaultProvider, $this->config])
->setMethods(['login', 'isTwoFactorEnforced'])
->getMock();
$this->defaultProvider->expects($this->once())
->method('getToken')
->with('doe')
->will($this->throwException(new \OC\Authentication\Exceptions\InvalidTokenException()));
$this->config->expects($this->once())
->method('getSystemValue')
->with('token_auth_enforced', false)
->will($this->returnValue(false));
$userSession->expects($this->once())
->method('isTwoFactorEnforced')
->with('john')
->will($this->returnValue(true));
$this->assertFalse($userSession->logClientIn('john', 'doe'));
}
public function testRememberLoginValidToken() {
$session = $this->getMock('\OC\Session\Memory', array(), array(''));
$session->expects($this->exactly(1))
@@ -355,7 +413,7 @@ public function testRememberLoginValidToken() {
//override, otherwise tests will fail because of setcookie()
array('setMagicInCookie'),
//there are passed as parameters to the constructor
array($manager, $session, $this->timeFactory, $this->defaultProvider));
array($manager, $session, $this->timeFactory, $this->defaultProvider, $this->config));
$granted = $userSession->loginWithCookie('foo', $token);
@@ -400,7 +458,7 @@ public function testRememberLoginInvalidToken() {
$token = 'goodToken';
\OC::$server->getConfig()->setUserValue('foo', 'login_token', $token, time());
$userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider);
$userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, $this->config);
$granted = $userSession->loginWithCookie('foo', 'badToken');
$this->assertSame($granted, false);
@@ -443,7 +501,7 @@ public function testRememberLoginInvalidUser() {
$token = 'goodToken';
\OC::$server->getConfig()->setUserValue('foo', 'login_token', $token, time());
$userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider);
$userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, $this->config);
$granted = $userSession->loginWithCookie('foo', $token);
$this->assertSame($granted, false);
@@ -468,7 +526,7 @@ public function testActiveUserAfterSetSession() {
$session = new Memory('');
$session->set('user_id', 'foo');
$userSession = $this->getMockBuilder('\OC\User\Session')
->setConstructorArgs([$manager, $session, $this->timeFactory, $this->defaultProvider])
->setConstructorArgs([$manager, $session, $this->timeFactory, $this->defaultProvider, $this->config])
->setMethods([
'validateSession'
])
@@ -491,7 +549,7 @@ public function testTryTokenLoginWithDisabledUser() {
$session = new Memory('');
$token = $this->getMock('\OC\Authentication\Token\IToken');
$user = $this->getMock('\OCP\IUser');
$userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider);
$userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, $this->config);
$request = $this->getMock('\OCP\IRequest');
$request->expects($this->once())
@@ -522,7 +580,7 @@ public function testValidateSessionDisabledUser() {
$timeFactory = $this->getMock('\OCP\AppFramework\Utility\ITimeFactory');
$tokenProvider = $this->getMock('\OC\Authentication\Token\IProvider');
$userSession = $this->getMockBuilder('\OC\User\Session')
->setConstructorArgs([$userManager, $session, $timeFactory, $tokenProvider])
->setConstructorArgs([$userManager, $session, $timeFactory, $tokenProvider, $this->config])
->setMethods(['logout'])
->getMock();