Merge pull request #24748 from owncloud/login-explicitly

Log in explicitly, save login name when generating browser/device tokens

apps/dav/lib/Connector/Sabre/Auth.php

@@ -105,7 +105,7 @@ class Auth extends AbstractBasic {
 			\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)) {
-				$this->userSession->createSessionToken($this->request, $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());
 				$this->session->close();

apps/dav/tests/unit/connector/sabre/auth.php

@@ -149,7 +149,7 @@ class Auth extends TestCase {
 		$user = $this->getMockBuilder('\OCP\IUser')
 			->disableOriginalConstructor()
 			->getMock();
-		$user->expects($this->exactly(3))
+		$user->expects($this->exactly(4))
 			->method('getUID')
 			->will($this->returnValue('MyTestUser'));
 		$this->userSession
@@ -157,7 +157,7 @@ class Auth extends TestCase {
 			->method('isLoggedIn')
 			->will($this->returnValue(true));
 		$this->userSession
-			->expects($this->exactly(3))
+			->expects($this->exactly(4))
 			->method('getUser')
 			->will($this->returnValue($user));
 		$this->session
@@ -173,7 +173,7 @@ class Auth extends TestCase {
 		$this->userSession
 			->expects($this->once())
 			->method('createSessionToken')
-			->with($this->request, 'MyTestUser', 'MyTestPassword');
+			->with($this->request, 'MyTestUser', 'MyTestUser', 'MyTestPassword');
 		$this->session
 			->expects($this->once())
 			->method('set')
@@ -569,11 +569,11 @@ class Auth extends TestCase {
 		$user = $this->getMockBuilder('\OCP\IUser')
 			->disableOriginalConstructor()
 			->getMock();
-		$user->expects($this->exactly(3))
+		$user->expects($this->exactly(4))
 			->method('getUID')
 			->will($this->returnValue('MyTestUser'));
 		$this->userSession
-			->expects($this->exactly(3))
+			->expects($this->exactly(4))
 			->method('getUser')
 			->will($this->returnValue($user));
 		$response = $this->auth->check($server->httpRequest, $server->httpResponse);

core/Controller/LoginController.php

@@ -190,7 +190,10 @@ class LoginController extends Controller {
 			$args = !is_null($user) ? ['user' => $user] : [];
 			return new RedirectResponse($this->urlGenerator->linkToRoute('core.login.showLoginForm', $args));
 		}
-		$this->userSession->createSessionToken($this->request, $loginResult->getUID(), $password);
+		// TODO: remove password checks from above and let the user session handle failures
+		// requires https://github.com/owncloud/core/pull/24616
+		$this->userSession->login($user, $password);
+		$this->userSession->createSessionToken($this->request, $loginResult->getUID(), $user, $password);
 
 		if ($this->twoFactorManager->isTwoFactorAuthenticated($loginResult)) {
 			$this->twoFactorManager->prepareTwoFactorLogin($loginResult);

core/Controller/TokenController.php

@@ -75,13 +75,14 @@ class TokenController extends Controller {
 			$response->setStatus(Http::STATUS_UNPROCESSABLE_ENTITY);
 			return $response;
 		}
-		if ($this->userManager->checkPassword($user, $password) === false) {
+		$loginResult = $this->userManager->checkPassword($user, $password);
+		if ($loginResult === false) {
 			$response = new Response();
 			$response->setStatus(Http::STATUS_UNAUTHORIZED);
 			return $response;
 		}
 		$token = $this->secureRandom->generate(128);
-		$this->tokenProvider->generateToken($token, $user, $password, $name, IToken::PERMANENT_TOKEN);
+		$this->tokenProvider->generateToken($token, $loginResult->getUID(), $user, $password, $name, IToken::PERMANENT_TOKEN);
 		return [
 			'token' => $token,
 		];

core/Controller/TwoFactorChallengeController.php

@@ -63,8 +63,8 @@ class TwoFactorChallengeController extends Controller {
 	}
 
 	/**
+	 * @NoAdminRequired
 	 * @NoCSRFRequired
-	 * @PublicPage
 	 *
 	 * @return TemplateResponse
 	 */
@@ -79,8 +79,8 @@ class TwoFactorChallengeController extends Controller {
 	}
 
 	/**
+	 * @NoAdminRequired
 	 * @NoCSRFRequired
-	 * @PublicPage
 	 * @UseSession
 	 *
 	 * @param string $challengeProviderId
@@ -108,8 +108,8 @@ class TwoFactorChallengeController extends Controller {
 	}
 
 	/**
+	 * @NoAdminRequired
 	 * @NoCSRFRequired
-	 * @PublicPage
 	 * @UseSession
 	 *
 	 * @param string $challengeProviderId

db_structure.xml

@@ -1072,6 +1072,14 @@
 				<length>64</length>
 			</field>
 
+			<field>
+				<name>login_name</name>
+				<type>text</type>
+				<default></default>
+				<notnull>true</notnull>
+				<length>64</length>
+			</field>
+
 			<field>
 				<name>password</name>
 				<type>clob</type>

lib/private/Authentication/Token/DefaultToken.php

@@ -27,6 +27,8 @@ use OCP\AppFramework\Db\Entity;
 /**
  * @method void setId(int $id)
  * @method void setUid(string $uid);
+ * @method void setLoginName(string $loginName)
+ * @method string getLoginName()
  * @method void setPassword(string $password)
  * @method void setName(string $name)
  * @method string getName()
@@ -44,6 +46,11 @@ class DefaultToken extends Entity implements IToken {
 	 */
 	protected $uid;
 
+	/**
+	 * @var string login name used for generating the token
+	 */
+	protected $loginName;
+
 	/**
 	 * @var string encrypted user password
 	 */
@@ -76,7 +83,16 @@ class DefaultToken extends Entity implements IToken {
 	public function getUID() {
 		return $this->uid;
 	}
-	
+
+	/**
+	 * Get the login name used when generating the token
+	 *
+	 * @return string
+	 */
+	public function getLoginName() {
+		return parent::getLoginName();
+	}
+
 	/**
 	 * Get the (encrypted) login password
 	 *

lib/private/Authentication/Token/DefaultTokenMapper.php

@@ -71,7 +71,7 @@ class DefaultTokenMapper extends Mapper {
 	public function getToken($token) {
 		/* @var $qb IQueryBuilder */
 		$qb = $this->db->getQueryBuilder();
-		$result = $qb->select('id', 'uid', 'password', 'name', 'type', 'token', 'last_activity')
+		$result = $qb->select('id', 'uid', 'login_name', 'password', 'name', 'type', 'token', 'last_activity')
 			->from('authtoken')
 			->where($qb->expr()->eq('token', $qb->createParameter('token')))
 			->setParameter('token', $token)
@@ -96,7 +96,7 @@ class DefaultTokenMapper extends Mapper {
 	public function getTokenByUser(IUser $user) {
 		/* @var $qb IQueryBuilder */
 		$qb = $this->db->getQueryBuilder();
-		$qb->select('id', 'uid', 'password', 'name', 'type', 'token', 'last_activity')
+		$qb->select('id', 'uid', 'login_name', 'password', 'name', 'type', 'token', 'last_activity')
 			->from('authtoken')
 			->where($qb->expr()->eq('uid', $qb->createNamedParameter($user->getUID())))
 			->setMaxResults(1000);

lib/private/Authentication/Token/DefaultTokenProvider.php

@@ -68,14 +68,16 @@ class DefaultTokenProvider implements IProvider {
 	 *
 	 * @param string $token
 	 * @param string $uid
+	 * @param string $loginName
 	 * @param string $password
 	 * @param string $name
 	 * @param int $type token type
-	 * @return DefaultToken
+	 * @return IToken
 	 */
-	public function generateToken($token, $uid, $password, $name, $type = IToken::TEMPORARY_TOKEN) {
+	public function generateToken($token, $uid, $loginName, $password, $name, $type = IToken::TEMPORARY_TOKEN) {
 		$dbToken = new DefaultToken();
 		$dbToken->setUid($uid);
+		$dbToken->setLoginName($loginName);
 		$dbToken->setPassword($this->encryptPassword($password, $token));
 		$dbToken->setName($name);
 		$dbToken->setToken($this->hashToken($token));

lib/private/Authentication/Token/IProvider.php

@@ -32,12 +32,13 @@ interface IProvider {
 	 *
 	 * @param string $token
 	 * @param string $uid
+	 * @param string $loginName
 	 * @param string $password
 	 * @param string $name
 	 * @param int $type token type
 	 * @return IToken
 	 */
-	public function generateToken($token, $uid, $password, $name, $type = IToken::TEMPORARY_TOKEN);
+	public function generateToken($token, $uid, $loginName, $password, $name, $type = IToken::TEMPORARY_TOKEN);
 
 	/**
 	 * Get a token by token id

lib/private/Authentication/Token/IToken.php

@@ -43,6 +43,13 @@ interface IToken extends JsonSerializable {
 	 */
 	public function getUID();
 
+	/**
+	 * Get the login name used when generating the token
+	 *
+	 * @return string
+	 */
+	public function getLoginName();
+
 	/**
 	 * Get the (encrypted) login password
 	 *

lib/private/Setup.php

@@ -371,7 +371,8 @@ class Setup {
 			$userSession = \OC::$server->getUserSession();
 			$defaultTokenProvider = \OC::$server->query('OC\Authentication\Token\DefaultTokenProvider');
 			$userSession->setTokenProvider($defaultTokenProvider);
-			$userSession->createSessionToken($request, $username, $password);
+			$userSession->login($username, $password);
+			$userSession->createSessionToken($request, $userSession->getUser()->getUID(), $username, $password);
 
 			//guess what this does
 			Installer::installShippedApps();

lib/private/User/Session.php

@@ -219,7 +219,7 @@ class Session implements IUserSession, Emitter {
 				return;
 			}
 
-			if ($this->manager->checkPassword($user->getUID(), $pwd) === false
+			if ($this->manager->checkPassword($token->getLoginName(), $pwd) === false
 				|| !$user->isEnabled()) {
 				// Password has changed or user was disabled -> log user out
 				$this->logout();
@@ -388,25 +388,23 @@ class Session implements IUserSession, Emitter {
 	 *
 	 * @param IRequest $request
 	 * @param string $uid user UID
+	 * @param string $loginName login name
 	 * @param string $password
 	 * @return boolean
 	 */
-	public function createSessionToken(IRequest $request, $uid, $password) {
+	public function createSessionToken(IRequest $request, $uid, $loginName, $password) {
 		if (is_null($this->manager->get($uid))) {
 			// User does not exist
 			return false;
 		}
 		$name = isset($request->server['HTTP_USER_AGENT']) ? $request->server['HTTP_USER_AGENT'] : 'unknown browser';
-		$loggedIn = $this->login($uid, $password);
-		if ($loggedIn) {
-			try {
-				$sessionId = $this->session->getId();
-				$this->tokenProvider->generateToken($sessionId, $uid, $password, $name);
-			} catch (SessionNotAvailableException $ex) {
-				
-			}
+		try {
+			$sessionId = $this->session->getId();
+			$this->tokenProvider->generateToken($sessionId, $uid, $loginName, $password, $name);
+		} catch (SessionNotAvailableException $ex) {
+
 		}
-		return $loggedIn;
+		return true;
 	}
 
 	/**

settings/Controller/AuthSettingsController.php

@@ -101,6 +101,7 @@ class AuthSettingsController extends Controller {
 
 		try {
 			$sessionToken = $this->tokenProvider->getToken($sessionId);
+			$loginName = $sessionToken->getLoginName();
 			$password = $this->tokenProvider->getPassword($sessionToken, $sessionId);
 		} catch (InvalidTokenException $ex) {
 			$resp = new JSONResponse();
@@ -109,7 +110,7 @@ class AuthSettingsController extends Controller {
 		}
 
 		$token = $this->generateRandomDeviceToken();
-		$deviceToken = $this->tokenProvider->generateToken($token, $this->uid, $password, $name, IToken::PERMANENT_TOKEN);
+		$deviceToken = $this->tokenProvider->generateToken($token, $this->uid, $loginName, $password, $name, IToken::PERMANENT_TOKEN);
 
 		return [
 			'token' => $token,

tests/Core/Controller/LoginControllerTest.php

@@ -303,9 +303,12 @@ class LoginControllerTest extends TestCase {
 		$this->userManager->expects($this->once())
 			->method('checkPassword')
 			->will($this->returnValue($user));
+		$this->userSession->expects($this->once())
+			->method('login')
+			->with($user, $password);
 		$this->userSession->expects($this->once())
 			->method('createSessionToken')
-			->with($this->request, $user->getUID(), $password);
+			->with($this->request, $user->getUID(), $user, $password);
 		$this->twoFactorManager->expects($this->once())
 			->method('isTwoFactorAuthenticated')
 			->with($user)
@@ -330,11 +333,11 @@ class LoginControllerTest extends TestCase {
 
 		$this->userManager->expects($this->once())
 			->method('checkPassword')
-			->with('jane', $password)
+			->with('Jane', $password)
 			->will($this->returnValue($user));
 		$this->userSession->expects($this->once())
 			->method('createSessionToken')
-			->with($this->request, $user->getUID(), $password);
+			->with($this->request, $user->getUID(), 'Jane', $password);
 		$this->userSession->expects($this->once())
 			->method('isLoggedIn')
 			->with()
@@ -345,20 +348,26 @@ class LoginControllerTest extends TestCase {
 			->will($this->returnValue($redirectUrl));
 
 		$expected = new \OCP\AppFramework\Http\RedirectResponse(urldecode($redirectUrl));
-		$this->assertEquals($expected, $this->loginController->tryLogin($user->getUID(), $password, $originalUrl));
+		$this->assertEquals($expected, $this->loginController->tryLogin('Jane', $password, $originalUrl));
 	}
 	
 	public function testLoginWithTwoFactorEnforced() {
 		$user = $this->getMock('\OCP\IUser');
+		$user->expects($this->any())
+			->method('getUID')
+			->will($this->returnValue('john'));
 		$password = 'secret';
 		$challengeUrl = 'challenge/url';
 
 		$this->userManager->expects($this->once())
 			->method('checkPassword')
 			->will($this->returnValue($user));
+		$this->userSession->expects($this->once())
+			->method('login')
+			->with('john@doe.com', $password);
 		$this->userSession->expects($this->once())
 			->method('createSessionToken')
-			->with($this->request, $user->getUID(), $password);
+			->with($this->request, $user->getUID(), 'john@doe.com', $password);
 		$this->twoFactorManager->expects($this->once())
 			->method('isTwoFactorAuthenticated')
 			->with($user)
@@ -372,7 +381,7 @@ class LoginControllerTest extends TestCase {
 			->will($this->returnValue($challengeUrl));
 
 		$expected = new \OCP\AppFramework\Http\RedirectResponse($challengeUrl);
-		$this->assertEquals($expected, $this->loginController->tryLogin($user, $password, null));
+		$this->assertEquals($expected, $this->loginController->tryLogin('john@doe.com', $password, null));
 	}
 
 }

tests/Core/Controller/TokenControllerTest.php

@@ -75,14 +75,21 @@ class TokenControllerTest extends TestCase {
 	}
 
 	public function testWithValidCredentials() {
+		$user = $this->getMock('\OCP\IUser');
 		$this->userManager->expects($this->once())
 			->method('checkPassword')
 			->with('john', '123456')
-			->will($this->returnValue(true));
+			->will($this->returnValue($user));
+		$user->expects($this->once())
+			->method('getUID')
+			->will($this->returnValue('john'));
 		$this->secureRandom->expects($this->once())
 			->method('generate')
 			->with(128)
 			->will($this->returnValue('verysecurerandomtoken'));
+		$this->tokenProvider->expects($this->once())
+			->method('generateToken')
+			->with('verysecurerandomtoken', 'john', 'john', '123456', 'unknown client', \OC\Authentication\Token\IToken::PERMANENT_TOKEN);
 		$expected = [
 			'token' => 'verysecurerandomtoken'
 		];

tests/lib/Authentication/Token/DefaultTokenMapperTest.php

@@ -57,6 +57,7 @@ class DefaultTokenMapperTest extends TestCase {
 		$qb->delete('authtoken')->execute();
 		$qb->insert('authtoken')->values([
 			'uid' => $qb->createNamedParameter('user1'),
+			'login_name' => $qb->createNamedParameter('User1'),
 			'password' => $qb->createNamedParameter('a75c7116460c082912d8f6860a850904|3nz5qbG1nNSLLi6V|c55365a0e54cfdfac4a175bcf11a7612aea74492277bba6e5d96a24497fa9272488787cb2f3ad34d8b9b8060934fce02f008d371df3ff3848f4aa61944851ff0'),
 			'name' => $qb->createNamedParameter('Firefox on Linux'),
 			'token' => $qb->createNamedParameter('9c5a2e661482b65597408a6bb6c4a3d1af36337381872ac56e445a06cdb7fea2b1039db707545c11027a4966919918b19d875a8b774840b18c6cbb7ae56fe206'),
@@ -65,6 +66,7 @@ class DefaultTokenMapperTest extends TestCase {
 		])->execute();
 		$qb->insert('authtoken')->values([
 			'uid' => $qb->createNamedParameter('user2'),
+			'login_name' => $qb->createNamedParameter('User2'),
 			'password' => $qb->createNamedParameter('971a337057853344700bbeccf836519f|UwOQwyb34sJHtqPV|036d4890f8c21d17bbc7b88072d8ef049a5c832a38e97f3e3d5f9186e896c2593aee16883f617322fa242728d0236ff32d163caeb4bd45e14ca002c57a88665f'),
 			'name' => $qb->createNamedParameter('Firefox on Android'),
 			'token' => $qb->createNamedParameter('1504445f1524fc801035448a95681a9378ba2e83930c814546c56e5d6ebde221198792fd900c88ed5ead0555780dad1ebce3370d7e154941cd5de87eb419899b'),
@@ -73,6 +75,7 @@ class DefaultTokenMapperTest extends TestCase {
 		])->execute();
 		$qb->insert('authtoken')->values([
 			'uid' => $qb->createNamedParameter('user1'),
+			'login_name' => $qb->createNamedParameter('User1'),
 			'password' => $qb->createNamedParameter('063de945d6f6b26862d9b6f40652f2d5|DZ/z520tfdXPtd0T|395f6b89be8d9d605e409e20b9d9abe477fde1be38a3223f9e508f979bf906e50d9eaa4dca983ca4fb22a241eb696c3f98654e7775f78c4caf13108f98642b53'),
 			'name' => $qb->createNamedParameter('Iceweasel on Linux'),
 			'token' => $qb->createNamedParameter('47af8697ba590fb82579b5f1b3b6e8066773a62100abbe0db09a289a62f5d980dc300fa3d98b01d7228468d1ab05c1aa14c8d14bd5b6eee9cdf1ac14864680c3'),
@@ -118,6 +121,7 @@ class DefaultTokenMapperTest extends TestCase {
 		$token = '1504445f1524fc801035448a95681a9378ba2e83930c814546c56e5d6ebde221198792fd900c88ed5ead0555780dad1ebce3370d7e154941cd5de87eb419899b';
 		$token = new DefaultToken();
 		$token->setUid('user2');
+		$token->setLoginName('User2');
 		$token->setPassword('971a337057853344700bbeccf836519f|UwOQwyb34sJHtqPV|036d4890f8c21d17bbc7b88072d8ef049a5c832a38e97f3e3d5f9186e896c2593aee16883f617322fa242728d0236ff32d163caeb4bd45e14ca002c57a88665f');
 		$token->setName('Firefox on Android');
 		$token->setToken('1504445f1524fc801035448a95681a9378ba2e83930c814546c56e5d6ebde221198792fd900c88ed5ead0555780dad1ebce3370d7e154941cd5de87eb419899b');

tests/lib/Authentication/Token/DefaultTokenProviderTest.php

@@ -61,6 +61,7 @@ class DefaultTokenProviderTest extends TestCase {
 	public function testGenerateToken() {
 		$token = 'token';
 		$uid = 'user';
+		$user = 'User';
 		$password = 'passme';
 		$name = 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'
 			. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'
@@ -70,6 +71,7 @@ class DefaultTokenProviderTest extends TestCase {
 
 		$toInsert = new DefaultToken();
 		$toInsert->setUid($uid);
+		$toInsert->setLoginName($user);
 		$toInsert->setPassword('encryptedpassword');
 		$toInsert->setName($name);
 		$toInsert->setToken(hash('sha512', $token . '1f4h9s'));
@@ -88,7 +90,7 @@ class DefaultTokenProviderTest extends TestCase {
 			->method('insert')
 			->with($this->equalTo($toInsert));
 
-		$actual = $this->tokenProvider->generateToken($token, $uid, $password, $name, $type);
+		$actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type);
 
 		$this->assertEquals($toInsert, $actual);
 	}

tests/lib/User/SessionTest.php

@@ -38,8 +38,12 @@ class SessionTest extends \Test\TestCase {
 
 	public function testGetUser() {
 		$token = new \OC\Authentication\Token\DefaultToken();
+		$token->setLoginName('User123');
 
-		$expectedUser = new User('foo', null);
+		$expectedUser = $this->getMock('\OCP\IUser');
+		$expectedUser->expects($this->any())
+			->method('getUID')
+			->will($this->returnValue('user123'));
 		$session = $this->getMock('\OC\Session\Memory', array(), array(''));
 		$session->expects($this->at(0))
 			->method('get')
@@ -66,7 +70,10 @@ class SessionTest extends \Test\TestCase {
 			->will($this->returnValue('password123'));
 		$manager->expects($this->once())
 			->method('checkPassword')
-			->with($expectedUser->getUID(), 'password123')
+			->with('User123', 'password123')
+			->will($this->returnValue(true));
+		$expectedUser->expects($this->once())
+			->method('isEnabled')
 			->will($this->returnValue(true));
 		$session->expects($this->at(3))
 			->method('set')
@@ -540,12 +547,12 @@ class SessionTest extends \Test\TestCase {
 			->method('getPassword')
 			->with($token, 'sessionid')
 			->will($this->returnValue('123456'));
-		$user->expects($this->once())
-			->method('getUID')
-			->will($this->returnValue('user5'));
+		$token->expects($this->once())
+			->method('getLoginName')
+			->will($this->returnValue('User5'));
 		$userManager->expects($this->once())
 			->method('checkPassword')
-			->with('user5', '123456')
+			->with('User5', '123456')
 			->will($this->returnValue(true));
 		$user->expects($this->once())
 			->method('isEnabled')

tests/settings/controller/AuthSettingsControllerTest.php

@@ -89,6 +89,9 @@ class AuthSettingsControllerTest extends TestCase {
 			->method('getPassword')
 			->with($sessionToken, 'sessionid')
 			->will($this->returnValue($password));
+		$sessionToken->expects($this->once())
+			->method('getLoginName')
+			->will($this->returnValue('User13'));
 
 		$this->secureRandom->expects($this->exactly(4))
 			->method('generate')
@@ -98,7 +101,7 @@ class AuthSettingsControllerTest extends TestCase {
 
 		$this->tokenProvider->expects($this->once())
 			->method('generateToken')
-			->with($newToken, $this->uid, $password, $name, IToken::PERMANENT_TOKEN)
+			->with($newToken, $this->uid, 'User13', $password, $name, IToken::PERMANENT_TOKEN)
 			->will($this->returnValue($deviceToken));
 
 		$expected = [

version.php

@@ -26,7 +26,7 @@
 // We only can count up. The 4. digit is only for the internal patchlevel to trigger DB upgrades
 // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel
 // when updating major/minor version number.
-$OC_Version = array(9, 1, 0, 3);
+$OC_Version = array(9, 1, 0, 4);
 
 // The human readable string
 $OC_VersionString = '9.1.0 pre alpha';