From ad33992eb9eafe16a8a35b8bffe1cf72bc27569c Mon Sep 17 00:00:00 2001 From: davitol Date: Mon, 6 Jun 2016 15:28:49 +0200 Subject: Fix Decrypt message via occ --- core/Command/Encryption/DecryptAll.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'core') diff --git a/core/Command/Encryption/DecryptAll.php b/core/Command/Encryption/DecryptAll.php index 8d7d26f3d23..26dbe79b5cc 100644 --- a/core/Command/Encryption/DecryptAll.php +++ b/core/Command/Encryption/DecryptAll.php @@ -118,6 +118,8 @@ class DecryptAll extends Command { protected function execute(InputInterface $input, OutputInterface $output) { try { + + if ($this->encryptionManager->isEnabled() === true) { $output->write('Disable server side encryption... '); $this->config->setAppValue('core', 'encryption_enabled', 'no'); @@ -127,8 +129,15 @@ class DecryptAll extends Command { return; } + $uid = $input->getArgument('user'); + if (empty($uid)) { + $message = 'your ownCloud'; + } else { + $message = "$uid's account"; + } + $output->writeln("\n"); - $output->writeln('You are about to start to decrypt all files stored in your ownCloud.'); + $output->writeln("You are about to start to decrypt all files stored in $message."); $output->writeln('It will depend on the encryption module and your setup if this is possible.'); $output->writeln('Depending on the number and size of your files this can take some time'); $output->writeln('Please make sure that no user access his files during this process!'); @@ -140,6 +149,7 @@ class DecryptAll extends Command { $result = $this->decryptAll->decryptAll($input, $output, $user); if ($result === false) { $output->writeln(' aborted.'); + $output->write('Enable server side encryption... '); $this->config->setAppValue('core', 'encryption_enabled', 'yes'); } $this->resetSingleUserAndTrashbin(); -- cgit v1.2.3 From 3b5c169869839946b8cb9aee0c919c7ac0008528 Mon Sep 17 00:00:00 2001 From: davitol Date: Mon, 6 Jun 2016 16:10:49 +0200 Subject: Comments fixed --- core/Command/Encryption/DecryptAll.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'core') diff --git a/core/Command/Encryption/DecryptAll.php b/core/Command/Encryption/DecryptAll.php index 26dbe79b5cc..e9cae29e825 100644 --- a/core/Command/Encryption/DecryptAll.php +++ b/core/Command/Encryption/DecryptAll.php @@ -118,8 +118,6 @@ class DecryptAll extends Command { protected function execute(InputInterface $input, OutputInterface $output) { try { - - if ($this->encryptionManager->isEnabled() === true) { $output->write('Disable server side encryption... '); $this->config->setAppValue('core', 'encryption_enabled', 'no'); @@ -130,6 +128,7 @@ class DecryptAll extends Command { } $uid = $input->getArgument('user'); + //FIXME WHEN https://github.com/owncloud/core/issues/24994 is fixed if (empty($uid)) { $message = 'your ownCloud'; } else { @@ -149,7 +148,7 @@ class DecryptAll extends Command { $result = $this->decryptAll->decryptAll($input, $output, $user); if ($result === false) { $output->writeln(' aborted.'); - $output->write('Enable server side encryption... '); + $output->writeln('Enable server side encryption... '); $this->config->setAppValue('core', 'encryption_enabled', 'yes'); } $this->resetSingleUserAndTrashbin(); -- cgit v1.2.3 From 60e15e934c6556d34d27ff214f6a148247605bc2 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Mon, 6 Jun 2016 15:09:42 +0200 Subject: do not generate device token if 2FA is enable for user --- core/Application.php | 3 +- core/Controller/TokenController.php | 36 +++++++++++++------ .../Token/DefaultTokenCleanupJob.php | 1 + tests/Core/Controller/TokenControllerTest.php | 40 ++++++++++++++++++---- 4 files changed, 61 insertions(+), 19 deletions(-) (limited to 'core') diff --git a/core/Application.php b/core/Application.php index 25e2fa76273..a87917b626a 100644 --- a/core/Application.php +++ b/core/Application.php @@ -120,7 +120,8 @@ class Application extends App { $c->query('AppName'), $c->query('Request'), $c->query('UserManager'), - $c->query('OC\Authentication\Token\DefaultTokenProvider'), + $c->query('ServerContainer')->query('OC\Authentication\Token\IProvider'), + $c->query('TwoFactorAuthManager'), $c->query('SecureRandom') ); }); diff --git a/core/Controller/TokenController.php b/core/Controller/TokenController.php index 42cc29bad10..13b1db9044a 100644 --- a/core/Controller/TokenController.php +++ b/core/Controller/TokenController.php @@ -1,4 +1,5 @@ * @@ -23,22 +24,27 @@ namespace OC\Core\Controller; use OC\AppFramework\Http; use OC\Authentication\Token\DefaultTokenProvider; +use OC\Authentication\Token\IProvider; use OC\Authentication\Token\IToken; -use OC\User\Manager; +use OC\Authentication\TwoFactorAuth\Manager as TwoFactorAuthManager; +use OC\User\Manager as UserManager; +use OCA\User_LDAP\User\Manager; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\JSONResponse; -use OCP\AppFramework\Http\Response; use OCP\IRequest; use OCP\Security\ISecureRandom; class TokenController extends Controller { - /** @var Manager */ + /** @var UserManager */ private $userManager; - /** @var DefaultTokenProvider */ + /** @var IProvider */ private $tokenProvider; + /** @var TwoFactorAuthManager */ + private $twoFactorAuthManager; + /** @var ISecureRandom */ private $secureRandom; @@ -49,12 +55,12 @@ class TokenController extends Controller { * @param DefaultTokenProvider $tokenProvider * @param ISecureRandom $secureRandom */ - public function __construct($appName, IRequest $request, Manager $userManager, DefaultTokenProvider $tokenProvider, - ISecureRandom $secureRandom) { + public function __construct($appName, IRequest $request, UserManager $userManager, IProvider $tokenProvider, TwoFactorAuthManager $twoFactorAuthManager, ISecureRandom $secureRandom) { parent::__construct($appName, $request); $this->userManager = $userManager; $this->tokenProvider = $tokenProvider; $this->secureRandom = $secureRandom; + $this->twoFactorAuthManager = $twoFactorAuthManager; } /** @@ -70,18 +76,26 @@ class TokenController extends Controller { */ public function generateToken($user, $password, $name = 'unknown client') { if (is_null($user) || is_null($password)) { - $response = new Response(); + $response = new JSONResponse(); $response->setStatus(Http::STATUS_UNPROCESSABLE_ENTITY); return $response; } - $loginResult = $this->userManager->checkPassword($user, $password); - if ($loginResult === false) { - $response = new Response(); + $loginName = $user; + $user = $this->userManager->checkPassword($loginName, $password); + if ($user === false) { + $response = new JSONResponse(); $response->setStatus(Http::STATUS_UNAUTHORIZED); return $response; } + + if ($this->twoFactorAuthManager->isTwoFactorAuthenticated($user)) { + $resp = new JSONResponse(); + $resp->setStatus(Http::STATUS_UNAUTHORIZED); + return $resp; + } + $token = $this->secureRandom->generate(128); - $this->tokenProvider->generateToken($token, $loginResult->getUID(), $user, $password, $name, IToken::PERMANENT_TOKEN); + $this->tokenProvider->generateToken($token, $user->getUID(), $loginName, $password, $name, IToken::PERMANENT_TOKEN); return [ 'token' => $token, ]; diff --git a/lib/private/Authentication/Token/DefaultTokenCleanupJob.php b/lib/private/Authentication/Token/DefaultTokenCleanupJob.php index 04b98c6c5a0..7746d6be915 100644 --- a/lib/private/Authentication/Token/DefaultTokenCleanupJob.php +++ b/lib/private/Authentication/Token/DefaultTokenCleanupJob.php @@ -28,6 +28,7 @@ class DefaultTokenCleanupJob extends Job { protected function run($argument) { /* @var $provider DefaultTokenProvider */ + // TODO: add OC\Authentication\Token\IProvider::invalidateOldTokens and query interface $provider = OC::$server->query('OC\Authentication\Token\DefaultTokenProvider'); $provider->invalidateOldTokens(); } diff --git a/tests/Core/Controller/TokenControllerTest.php b/tests/Core/Controller/TokenControllerTest.php index 386140a8a4f..b6b54b14fad 100644 --- a/tests/Core/Controller/TokenControllerTest.php +++ b/tests/Core/Controller/TokenControllerTest.php @@ -23,8 +23,9 @@ namespace Tests\Core\Controller; use OC\AppFramework\Http; +use OC\Authentication\Token\IToken; use OC\Core\Controller\TokenController; -use OCP\AppFramework\Http\Response; +use OCP\AppFramework\Http\JSONResponse; use Test\TestCase; class TokenControllerTest extends TestCase { @@ -34,6 +35,7 @@ class TokenControllerTest extends TestCase { private $request; private $userManager; private $tokenProvider; + private $twoFactorAuthManager; private $secureRandom; protected function setUp() { @@ -43,17 +45,17 @@ class TokenControllerTest extends TestCase { $this->userManager = $this->getMockBuilder('\OC\User\Manager') ->disableOriginalConstructor() ->getMock(); - $this->tokenProvider = $this->getMockBuilder('\OC\Authentication\Token\DefaultTokenProvider') + $this->tokenProvider = $this->getMock('\OC\Authentication\Token\IProvider'); + $this->twoFactorAuthManager = $this->getMockBuilder('\OC\Authentication\TwoFactorAuth\Manager') ->disableOriginalConstructor() ->getMock(); $this->secureRandom = $this->getMock('\OCP\Security\ISecureRandom'); - $this->tokenController = new TokenController('core', $this->request, $this->userManager, $this->tokenProvider, - $this->secureRandom); + $this->tokenController = new TokenController('core', $this->request, $this->userManager, $this->tokenProvider, $this->twoFactorAuthManager, $this->secureRandom); } public function testWithoutCredentials() { - $expected = new Response(); + $expected = new JSONResponse(); $expected->setStatus(Http::STATUS_UNPROCESSABLE_ENTITY); $actual = $this->tokenController->generateToken(null, null); @@ -66,7 +68,7 @@ class TokenControllerTest extends TestCase { ->method('checkPassword') ->with('john', 'passme') ->will($this->returnValue(false)); - $expected = new Response(); + $expected = new JSONResponse(); $expected->setStatus(Http::STATUS_UNAUTHORIZED); $actual = $this->tokenController->generateToken('john', 'passme'); @@ -83,13 +85,17 @@ class TokenControllerTest extends TestCase { $user->expects($this->once()) ->method('getUID') ->will($this->returnValue('john')); + $this->twoFactorAuthManager->expects($this->once()) + ->method('isTwoFactorAuthenticated') + ->with($user) + ->will($this->returnValue(false)); $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); + ->with('verysecurerandomtoken', 'john', 'john', '123456', 'unknown client', IToken::PERMANENT_TOKEN); $expected = [ 'token' => 'verysecurerandomtoken' ]; @@ -99,4 +105,24 @@ class TokenControllerTest extends TestCase { $this->assertEquals($expected, $actual); } + public function testWithValidCredentialsBut2faEnabled() { + $user = $this->getMock('\OCP\IUser'); + $this->userManager->expects($this->once()) + ->method('checkPassword') + ->with('john', '123456') + ->will($this->returnValue($user)); + $this->twoFactorAuthManager->expects($this->once()) + ->method('isTwoFactorAuthenticated') + ->with($user) + ->will($this->returnValue(true)); + $this->secureRandom->expects($this->never()) + ->method('generate'); + $expected = new JSONResponse(); + $expected->setStatus(Http::STATUS_UNAUTHORIZED); + + $actual = $this->tokenController->generateToken('john', '123456'); + + $this->assertEquals($expected, $actual); + } + } -- cgit v1.2.3 From 4f27c2c43341e8798669a72a7da36a33e873490a Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 7 Jun 2016 09:13:11 +0200 Subject: Allow to decrypt user '0' files only --- core/Command/Encryption/DecryptAll.php | 3 ++- lib/private/Encryption/DecryptAll.php | 4 ++-- tests/lib/Encryption/DecryptAllTest.php | 35 ++++++++++++++++++++++----------- 3 files changed, 27 insertions(+), 15 deletions(-) (limited to 'core') diff --git a/core/Command/Encryption/DecryptAll.php b/core/Command/Encryption/DecryptAll.php index e9cae29e825..f9e4fba7133 100644 --- a/core/Command/Encryption/DecryptAll.php +++ b/core/Command/Encryption/DecryptAll.php @@ -111,7 +111,8 @@ class DecryptAll extends Command { $this->addArgument( 'user', InputArgument::OPTIONAL, - 'user for which you want to decrypt all files (optional)' + 'user for which you want to decrypt all files (optional)', + '' ); } diff --git a/lib/private/Encryption/DecryptAll.php b/lib/private/Encryption/DecryptAll.php index 8676bc09575..34a3e1bff91 100644 --- a/lib/private/Encryption/DecryptAll.php +++ b/lib/private/Encryption/DecryptAll.php @@ -80,7 +80,7 @@ class DecryptAll { $this->input = $input; $this->output = $output; - if (!empty($user) && $this->userManager->userExists($user) === false) { + if ($user !== '' && $this->userManager->userExists($user) === false) { $this->output->writeln('User "' . $user . '" does not exist. Please check the username and try again'); return false; } @@ -141,7 +141,7 @@ class DecryptAll { $this->output->writeln("\n"); $userList = []; - if (empty($user)) { + if ($user === '') { $fetchUsersProgress = new ProgressBar($this->output); $fetchUsersProgress->setFormat(" %message% \n [%bar%]"); diff --git a/tests/lib/Encryption/DecryptAllTest.php b/tests/lib/Encryption/DecryptAllTest.php index ffcbbc74a99..d7cf2fb7baf 100644 --- a/tests/lib/Encryption/DecryptAllTest.php +++ b/tests/lib/Encryption/DecryptAllTest.php @@ -86,13 +86,25 @@ class DecryptAllTest extends TestCase { $this->invokePrivate($this->instance, 'output', [$this->outputInterface]); } + public function dataDecryptAll() { + return [ + [true, 'user1', true], + [false, 'user1', true], + [true, '0', true], + [false, '0', true], + [true, '', false], + ]; + } + /** - * @dataProvider dataTrueFalse + * @dataProvider dataDecryptAll * @param bool $prepareResult + * @param string $user + * @param bool $userExistsChecked */ - public function testDecryptAll($prepareResult, $user) { + public function testDecryptAll($prepareResult, $user, $userExistsChecked) { - if (!empty($user)) { + if ($userExistsChecked) { $this->userManager->expects($this->once())->method('userExists')->willReturn(true); } else { $this->userManager->expects($this->never())->method('userExists'); @@ -125,15 +137,6 @@ class DecryptAllTest extends TestCase { $instance->decryptAll($this->inputInterface, $this->outputInterface, $user); } - public function dataTrueFalse() { - return [ - [true, 'user1'], - [false, 'user1'], - [true, ''], - [true, null] - ]; - } - /** * test decrypt all call with a user who doesn't exists */ @@ -147,8 +150,16 @@ class DecryptAllTest extends TestCase { ); } + public function dataTrueFalse() { + return [ + [true], + [false], + ]; + } + /** * @dataProvider dataTrueFalse + * @param bool $success */ public function testPrepareEncryptionModules($success) { -- cgit v1.2.3 From fad91e92f040fc5ab7282c1f2fd5b6a4af49328a Mon Sep 17 00:00:00 2001 From: Sergio Bertolín Date: Tue, 7 Jun 2016 07:55:28 +0000 Subject: Fixed reviews --- core/Command/Encryption/DecryptAll.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'core') diff --git a/core/Command/Encryption/DecryptAll.php b/core/Command/Encryption/DecryptAll.php index f9e4fba7133..d060918a506 100644 --- a/core/Command/Encryption/DecryptAll.php +++ b/core/Command/Encryption/DecryptAll.php @@ -130,7 +130,7 @@ class DecryptAll extends Command { $uid = $input->getArgument('user'); //FIXME WHEN https://github.com/owncloud/core/issues/24994 is fixed - if (empty($uid)) { + if ($uid === null) { $message = 'your ownCloud'; } else { $message = "$uid's account"; @@ -149,7 +149,7 @@ class DecryptAll extends Command { $result = $this->decryptAll->decryptAll($input, $output, $user); if ($result === false) { $output->writeln(' aborted.'); - $output->writeln('Enable server side encryption... '); + $output->writeln('Server side encryption remains enabled'); $this->config->setAppValue('core', 'encryption_enabled', 'yes'); } $this->resetSingleUserAndTrashbin(); -- cgit v1.2.3 From 7f88645eabd4d4345229259e0fb6e280ae8af1e8 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 7 Jun 2016 17:53:00 +0200 Subject: Allow to cancel 2FA after login --- core/Controller/TwoFactorChallengeController.php | 9 +++++++++ core/Middleware/TwoFactorMiddleware.php | 5 +++++ core/css/styles.css | 4 ++++ core/templates/twofactorselectchallenge.php | 3 ++- core/templates/twofactorshowchallenge.php | 1 + .../Controller/TwoFactorChallengeControllerTest.php | 21 +++++++++++++++++---- 6 files changed, 38 insertions(+), 5 deletions(-) (limited to 'core') diff --git a/core/Controller/TwoFactorChallengeController.php b/core/Controller/TwoFactorChallengeController.php index 499898de3bc..edaf3378cd8 100644 --- a/core/Controller/TwoFactorChallengeController.php +++ b/core/Controller/TwoFactorChallengeController.php @@ -61,6 +61,13 @@ class TwoFactorChallengeController extends Controller { $this->urlGenerator = $urlGenerator; } + /** + * @return string + */ + protected function getLogoutAttribute() { + return \OC_User::getLogoutAttribute(); + } + /** * @NoAdminRequired * @NoCSRFRequired @@ -75,6 +82,7 @@ class TwoFactorChallengeController extends Controller { $data = [ 'providers' => $providers, 'redirect_url' => $redirect_url, + 'logout_attribute' => $this->getLogoutAttribute(), ]; return new TemplateResponse($this->appName, 'twofactorselectchallenge', $data, 'guest'); } @@ -106,6 +114,7 @@ class TwoFactorChallengeController extends Controller { $data = [ 'error' => $error, 'provider' => $provider, + 'logout_attribute' => $this->getLogoutAttribute(), 'template' => $tmpl->fetchPage(), ]; return new TemplateResponse($this->appName, 'twofactorshowchallenge', $data, 'guest'); diff --git a/core/Middleware/TwoFactorMiddleware.php b/core/Middleware/TwoFactorMiddleware.php index aa82897ad46..0bad8a2c40f 100644 --- a/core/Middleware/TwoFactorMiddleware.php +++ b/core/Middleware/TwoFactorMiddleware.php @@ -82,6 +82,11 @@ class TwoFactorMiddleware extends Middleware { return; } + if ($controller instanceof \OC\Core\Controller\LoginController && $methodName === 'logout') { + // Don't block the logout page, to allow canceling the 2FA + return; + } + if ($this->userSession->isLoggedIn()) { $user = $this->userSession->getUser(); diff --git a/core/css/styles.css b/core/css/styles.css index df9509baa19..475c4fa3fb3 100644 --- a/core/css/styles.css +++ b/core/css/styles.css @@ -38,6 +38,10 @@ body { display: inline-block; } +a.two-factor-cancel { + color: #fff; +} + .float-spinner { height: 32px; display: none; diff --git a/core/templates/twofactorselectchallenge.php b/core/templates/twofactorselectchallenge.php index 14d599aab3e..4209beac4e6 100644 --- a/core/templates/twofactorselectchallenge.php +++ b/core/templates/twofactorselectchallenge.php @@ -18,4 +18,5 @@ - \ No newline at end of file + +>t('Cancel login')) ?> diff --git a/core/templates/twofactorshowchallenge.php b/core/templates/twofactorshowchallenge.php index 66f5ed312ec..c5ee9aca4b4 100644 --- a/core/templates/twofactorshowchallenge.php +++ b/core/templates/twofactorshowchallenge.php @@ -17,3 +17,4 @@ $template = $_['template']; t('An error occured while verifying the token')); ?> +>t('Cancel login')) ?> diff --git a/tests/Core/Controller/TwoFactorChallengeControllerTest.php b/tests/Core/Controller/TwoFactorChallengeControllerTest.php index 2da6dcd52ac..08d8dd1452c 100644 --- a/tests/Core/Controller/TwoFactorChallengeControllerTest.php +++ b/tests/Core/Controller/TwoFactorChallengeControllerTest.php @@ -33,7 +33,7 @@ class TwoFactorChallengeControllerTest extends TestCase { private $session; private $urlGenerator; - /** TwoFactorChallengeController */ + /** @var TwoFactorChallengeController|\PHPUnit_Framework_MockObject_MockObject */ private $controller; protected function setUp() { @@ -47,9 +47,20 @@ class TwoFactorChallengeControllerTest extends TestCase { $this->session = $this->getMock('\OCP\ISession'); $this->urlGenerator = $this->getMock('\OCP\IURLGenerator'); - $this->controller = new TwoFactorChallengeController( - 'core', $this->request, $this->twoFactorManager, $this->userSession, $this->session, $this->urlGenerator - ); + $this->controller = $this->getMockBuilder('OC\Core\Controller\TwoFactorChallengeController') + ->setConstructorArgs([ + 'core', + $this->request, + $this->twoFactorManager, + $this->userSession, + $this->session, + $this->urlGenerator, + ]) + ->setMethods(['getLogoutAttribute']) + ->getMock(); + $this->controller->expects($this->any()) + ->method('getLogoutAttribute') + ->willReturn('logoutAttribute'); } public function testSelectChallenge() { @@ -70,6 +81,7 @@ class TwoFactorChallengeControllerTest extends TestCase { $expected = new \OCP\AppFramework\Http\TemplateResponse('core', 'twofactorselectchallenge', [ 'providers' => $providers, 'redirect_url' => '/some/url', + 'logout_attribute' => 'logoutAttribute', ], 'guest'); $this->assertEquals($expected, $this->controller->selectChallenge('/some/url')); @@ -110,6 +122,7 @@ class TwoFactorChallengeControllerTest extends TestCase { $expected = new \OCP\AppFramework\Http\TemplateResponse('core', 'twofactorshowchallenge', [ 'error' => true, 'provider' => $provider, + 'logout_attribute' => 'logoutAttribute', 'template' => '', ], 'guest'); -- cgit v1.2.3