From bcf027a7f438f66fd680f24667aeb960c4b091b3 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Tue, 13 Mar 2018 12:44:57 +0100 Subject: [PATCH 1/4] Update OCP lib Signed-off-by: Christoph Wurst --- composer.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/composer.lock b/composer.lock index 92c85900..ef2db346 100644 --- a/composer.lock +++ b/composer.lock @@ -49,12 +49,12 @@ "source": { "type": "git", "url": "https://github.com/ChristophWurst/nextcloud_composer.git", - "reference": "bc6390f8d87e31c6c695bfc1bce4e000cec487c3" + "reference": "cdb5f02e3eb7e0cc10b2ad221550283d6006e38a" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/ChristophWurst/nextcloud_composer/zipball/bc6390f8d87e31c6c695bfc1bce4e000cec487c3", - "reference": "bc6390f8d87e31c6c695bfc1bce4e000cec487c3", + "url": "https://api.github.com/repos/ChristophWurst/nextcloud_composer/zipball/cdb5f02e3eb7e0cc10b2ad221550283d6006e38a", + "reference": "cdb5f02e3eb7e0cc10b2ad221550283d6006e38a", "shasum": "" }, "type": "library", @@ -74,7 +74,7 @@ } ], "description": "Composer package containing Nextcloud's public API (classes, interfaces)", - "time": "2018-02-26T10:06:58+00:00" + "time": "2018-03-09T09:27:07+00:00" }, { "name": "doctrine/instantiator", From acc7a691a6799e0c954a226ba217862173b033dc Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Tue, 13 Mar 2018 12:54:43 +0100 Subject: [PATCH 2/4] Add php7 primitive type hints Signed-off-by: Christoph Wurst --- lib/Controller/SettingsController.php | 32 +++++++------------ lib/Db/Registration.php | 4 +-- lib/Db/RegistrationMapper.php | 7 ++-- lib/Provider/U2FProvider.php | 31 ++++-------------- lib/Service/U2FManager.php | 46 ++++++--------------------- lib/Settings/Personal.php | 9 ++---- 6 files changed, 35 insertions(+), 94 deletions(-) diff --git a/lib/Controller/SettingsController.php b/lib/Controller/SettingsController.php index d55dbc3f..f08ca04b 100644 --- a/lib/Controller/SettingsController.php +++ b/lib/Controller/SettingsController.php @@ -7,7 +7,7 @@ * later. See the COPYING file. * * @author Christoph Wurst - * @copyright Christoph Wurst 2016 + * @copyright Christoph Wurst 2018 */ namespace OCA\TwoFactorU2F\Controller; @@ -28,13 +28,7 @@ class SettingsController extends Controller { /** @var IUserSession */ private $userSession; - /** - * @param string $appName - * @param IRequest $request - * @param U2FManager $manager - * @param IUserSession $userSession - */ - public function __construct($appName, IRequest $request, U2FManager $manager, IUserSession $userSession) { + public function __construct(string $appName, IRequest $request, U2FManager $manager, IUserSession $userSession) { parent::__construct($appName, $request); $this->manager = $manager; $this->userSession = $userSession; @@ -42,22 +36,20 @@ public function __construct($appName, IRequest $request, U2FManager $manager, IU /** * @NoAdminRequired - * @return JSONResponse */ - public function state() { - return [ + public function state(): JSONResponse { + return new JSONResponse([ 'devices' => $this->manager->getDevices($this->userSession->getUser()) - ]; + ]); } /** * @NoAdminRequired * @PasswordConfirmationRequired * @UseSession - * @return JSONResponse */ - public function startRegister() { - return $this->manager->startRegistration($this->userSession->getUser()); + public function startRegister(): JSONResponse { + return new JSONResponse($this->manager->startRegistration($this->userSession->getUser())); } /** @@ -67,10 +59,9 @@ public function startRegister() { * @param string $registrationData * @param string $clientData * @param string|null $name device name, given by user - * @return JSONResponse */ - public function finishRegister($registrationData, $clientData, $name = null) { - return $this->manager->finishRegistration($this->userSession->getUser(), $registrationData, $clientData, $name); + public function finishRegister(string $registrationData, string $clientData, string $name = null): JSONResponse { + return new JSONResponse($this->manager->finishRegistration($this->userSession->getUser(), $registrationData, $clientData, $name)); } /** @@ -78,10 +69,9 @@ public function finishRegister($registrationData, $clientData, $name = null) { * @PasswordConfirmationRequired * * @param int $id - * @return JSONResponse */ - public function remove($id) { - return $this->manager->removeDevice($this->userSession->getUser(), $id); + public function remove(int $id): JSONResponse { + return new JSONResponse($this->manager->removeDevice($this->userSession->getUser(), $id)); } } diff --git a/lib/Db/Registration.php b/lib/Db/Registration.php index 306ebbff..6c7e7378 100644 --- a/lib/Db/Registration.php +++ b/lib/Db/Registration.php @@ -7,7 +7,7 @@ * later. See the COPYING file. * * @author Christoph Wurst - * @copyright Christoph Wurst 2016 + * @copyright Christoph Wurst 2018 */ namespace OCA\TwoFactorU2F\Db; @@ -38,7 +38,7 @@ class Registration extends Entity implements JsonSerializable { protected $counter; protected $name; - public function jsonSerialize() { + public function jsonSerialize(): array { return [ 'id' => $this->getId(), 'userId' => $this->getUserId(), diff --git a/lib/Db/RegistrationMapper.php b/lib/Db/RegistrationMapper.php index 766bc78b..191f6754 100644 --- a/lib/Db/RegistrationMapper.php +++ b/lib/Db/RegistrationMapper.php @@ -7,7 +7,7 @@ * later. See the COPYING file. * * @author Christoph Wurst - * @copyright Christoph Wurst 2016 + * @copyright Christoph Wurst 2018 */ namespace OCA\TwoFactorU2F\Db; @@ -26,9 +26,8 @@ public function __construct(IDBConnection $db) { /** * @param IUser $user * @param int $id - * @return Registration */ - public function findRegistration(IUser $user, $id) { + public function findRegistration(IUser $user, $id): Registration { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); @@ -48,7 +47,7 @@ public function findRegistration(IUser $user, $id) { * @param IUser $user * @return Registration[] */ - public function findRegistrations(IUser $user) { + public function findRegistrations(IUser $user): array { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); diff --git a/lib/Provider/U2FProvider.php b/lib/Provider/U2FProvider.php index 887f1145..93db6ee1 100644 --- a/lib/Provider/U2FProvider.php +++ b/lib/Provider/U2FProvider.php @@ -7,7 +7,7 @@ * later. See the COPYING file. * * @author Christoph Wurst - * @copyright Christoph Wurst 2016 + * @copyright Christoph Wurst 2018 */ namespace OCA\TwoFactorU2F\Provider; @@ -26,10 +26,6 @@ class U2FProvider implements IProvider { /** @var U2FManager */ private $manager; - /** - * @param IL10N $l10n - * @param U2FManager $manager - */ public function __construct(IL10N $l10n, U2FManager $manager) { $this->l10n = $l10n; $this->manager = $manager; @@ -37,38 +33,29 @@ public function __construct(IL10N $l10n, U2FManager $manager) { /** * Get unique identifier of this 2FA provider - * - * @return string */ - public function getId() { + public function getId(): string { return 'u2f'; } /** * Get the display name for selecting the 2FA provider - * - * @return string */ - public function getDisplayName() { + public function getDisplayName(): string { return 'U2F device'; } /** * Get the description for selecting the 2FA provider - * - * @return string */ - public function getDescription() { + public function getDescription(): string { return $this->l10n->t('Authenticate with an U2F device'); } /** * Get the template for rending the 2FA provider view - * - * @param IUser $user - * @return Template */ - public function getTemplate(IUser $user) { + public function getTemplate(IUser $user): Template { $reqs = $this->manager->startAuthenticate($user); $tmpl = new Template('twofactor_u2f', 'challenge'); @@ -79,20 +66,16 @@ public function getTemplate(IUser $user) { /** * Verify the given challenge * - * @param IUser $user * @param string $challenge */ - public function verifyChallenge(IUser $user, $challenge) { + public function verifyChallenge(IUser $user, $challenge): bool { return $this->manager->finishAuthenticate($user, $challenge); } /** * Decides whether 2FA is enabled for the given user - * - * @param IUser $user - * @return boolean */ - public function isTwoFactorAuthEnabledForUser(IUser $user) { + public function isTwoFactorAuthEnabledForUser(IUser $user): bool { return count($this->manager->getDevices($user)) > 0; } diff --git a/lib/Service/U2FManager.php b/lib/Service/U2FManager.php index 7b3ea384..a5f36fe6 100644 --- a/lib/Service/U2FManager.php +++ b/lib/Service/U2FManager.php @@ -42,13 +42,6 @@ class U2FManager { /** @var IManager */ private $activityManager; - /** - * @param RegistrationMapper $mapper - * @param ISession $session - * @param ILogger $logger - * @param IRequest $request - * @param IManager $activityManager - */ public function __construct(RegistrationMapper $mapper, ISession $session, ILogger $logger, IRequest $request, IManager $activityManager) { $this->mapper = $mapper; $this->session = $session; @@ -57,12 +50,12 @@ public function __construct(RegistrationMapper $mapper, ISession $session, ILogg $this->activityManager = $activityManager; } - private function getU2f() { + private function getU2f(): U2F { $url = $this->request->getServerProtocol() . '://' . $this->request->getServerHost(); return new U2F($url); } - private function getRegistrations(IUser $user) { + private function getRegistrations(IUser $user): array { $registrations = $this->mapper->findRegistrations($user); $registrationObjects = array_map(function (Registration $registration) { return (object) $registration->jsonSerialize(); @@ -70,11 +63,7 @@ private function getRegistrations(IUser $user) { return $registrationObjects; } - /** - * @param IUser $user - * @return array - */ - public function getDevices(IUser $user) { + public function getDevices(IUser $user): array { $registrations = $this->mapper->findRegistrations($user); return array_map(function(Registration $reg) { return [ @@ -84,21 +73,13 @@ public function getDevices(IUser $user) { }, $registrations); } - /** - * @param IUser $user - * @param int $id device id - */ - public function removeDevice(IUser $user, $id) { + public function removeDevice(IUser $user, int $id) { $reg = $this->mapper->findRegistration($user, $id); $this->mapper->delete($reg); $this->publishEvent($user, 'u2f_device_removed'); } - /** - * @param IUser $user - * @return array - */ - public function startRegistration(IUser $user) { + public function startRegistration(IUser $user): array { $u2f = $this->getU2f(); $data = $u2f->getRegisterData($this->getRegistrations($user)); list($req, $sigs) = $data; @@ -114,13 +95,7 @@ public function startRegistration(IUser $user) { ]; } - /** - * @param IUser $user - * @param string $registrationData - * @param string $clientData - * @param string $name - */ - public function finishRegistration(IUser $user, $registrationData, $clientData, $name = null) { + public function finishRegistration(IUser $user, string $registrationData, string $clientData, string $name = null): array { $this->logger->debug($registrationData); $this->logger->debug($clientData); @@ -152,11 +127,8 @@ public function finishRegistration(IUser $user, $registrationData, $clientData, /** * Push an U2F event the user's activity stream - * - * @param IUser $user - * @param string $event */ - private function publishEvent(IUser $user, $event) { + private function publishEvent(IUser $user, string $event) { $activity = $this->activityManager->generateEvent(); $activity->setApp('twofactor_u2f') ->setType('security') @@ -166,14 +138,14 @@ private function publishEvent(IUser $user, $event) { $this->activityManager->publish($activity); } - public function startAuthenticate(IUser $user) { + public function startAuthenticate(IUser $user): array { $u2f = $this->getU2f(); $reqs = $u2f->getAuthenticateData($this->getRegistrations($user)); $this->session->set('twofactor_u2f_authReq', json_encode($reqs)); return $reqs; } - public function finishAuthenticate(IUser $user, $challenge) { + public function finishAuthenticate(IUser $user, string $challenge): bool { $u2f = $this->getU2f(); $registrations = $this->getRegistrations($user); diff --git a/lib/Settings/Personal.php b/lib/Settings/Personal.php index a1ecd6ca..43dad1c3 100644 --- a/lib/Settings/Personal.php +++ b/lib/Settings/Personal.php @@ -29,21 +29,18 @@ class Personal implements ISettings { /** * @return TemplateResponse */ - public function getForm() { + public function getForm(): TemplateResponse { return new TemplateResponse('twofactor_u2f', 'personal'); } /** * @return string the section ID */ - public function getSection() { + public function getSection(): string { return 'security'; } - /** - * @return int - */ - public function getPriority() { + public function getPriority(): int { return 40; } From eee72c672a30152a34faacfde5bed0dd8d4e9082 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Tue, 13 Mar 2018 12:55:16 +0100 Subject: [PATCH 3/4] Use php7+ in Netbeans Signed-off-by: Christoph Wurst --- nbproject/project.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nbproject/project.properties b/nbproject/project.properties index e941663e..2b77a8da 100644 --- a/nbproject/project.properties +++ b/nbproject/project.properties @@ -12,7 +12,7 @@ auxiliary.org-netbeans-modules-php-phpunit.test_2e_run_2e_all=false auxiliary.org-netbeans-modules-php-phpunit.test_2e_run_2e_phpunit_2e_only=false file.reference.twofactor_u2f-tests=tests include.path=${php.global.include.path} -php.version=PHP_56 +php.version=PHP_70 source.encoding=UTF-8 src.dir=. tags.asp=false From 2c8b6f6ff386d303cf4b1364fcb667aa6dce40ea Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Tue, 13 Mar 2018 12:59:56 +0100 Subject: [PATCH 4/4] Add strict types and fix tests Signed-off-by: Christoph Wurst --- lib/Activity/Provider.php | 4 +++- lib/Activity/Setting.php | 4 +++- lib/Controller/SettingsController.php | 2 ++ lib/Db/Registration.php | 2 ++ lib/Db/RegistrationMapper.php | 2 ++ lib/Provider/U2FProvider.php | 2 ++ lib/Service/U2FManager.php | 2 ++ lib/Settings/Personal.php | 2 ++ tests/Unit/Controller/SettingsControllerTest.php | 16 ++++++++++------ tests/Unit/Service/U2FManagerTest.php | 15 --------------- 10 files changed, 28 insertions(+), 23 deletions(-) diff --git a/lib/Activity/Provider.php b/lib/Activity/Provider.php index b08ba993..51d3dbd1 100644 --- a/lib/Activity/Provider.php +++ b/lib/Activity/Provider.php @@ -1,8 +1,10 @@ - * @copyright Copyright (c) 2016 Christoph Wurst + * @copyright Copyright (c) 2018 Christoph Wurst * * Two-factor U2F * diff --git a/lib/Activity/Setting.php b/lib/Activity/Setting.php index 8501d2d0..a6a31475 100644 --- a/lib/Activity/Setting.php +++ b/lib/Activity/Setting.php @@ -1,8 +1,10 @@ - * @copyright Copyright (c) 2016 Christoph Wurst + * @copyright Copyright (c) 2018 Christoph Wurst * * Two-factor U2F * diff --git a/lib/Controller/SettingsController.php b/lib/Controller/SettingsController.php index f08ca04b..e7965dd7 100644 --- a/lib/Controller/SettingsController.php +++ b/lib/Controller/SettingsController.php @@ -1,5 +1,7 @@ * diff --git a/tests/Unit/Controller/SettingsControllerTest.php b/tests/Unit/Controller/SettingsControllerTest.php index 0735bf29..1233988c 100644 --- a/tests/Unit/Controller/SettingsControllerTest.php +++ b/tests/Unit/Controller/SettingsControllerTest.php @@ -14,6 +14,7 @@ use OCA\TwoFactorU2F\Controller\SettingsController; use OCA\TwoFactorU2F\Service\U2FManager; +use OCP\AppFramework\Http\JSONResponse; use OCP\IRequest; use OCP\IUser; use OCP\IUserSession; @@ -64,10 +65,10 @@ public function testState() { ->with($this->equalTo($user)) ->willReturn($devices); - $expected = [ + $expected = new JSONResponse([ 'devices' => $devices, - ]; - $this->assertSame($expected, $this->controller->state()); + ]); + $this->assertEquals($expected, $this->controller->state()); } public function testStartRegister() { @@ -81,7 +82,7 @@ public function testStartRegister() { ->with($this->equalTo($user)) ->willReturn([]); - $this->assertEquals([], $this->controller->startRegister()); + $this->assertEquals(new JSONResponse([]), $this->controller->startRegister()); } public function testFinishRegister() { @@ -94,9 +95,12 @@ public function testFinishRegister() { $this->u2fManager->expects($this->once()) ->method('finishRegistration') - ->with($this->equalTo($user), $this->equalTo($registrationData), $this->equalTo($data)); + ->with($this->equalTo($user), $this->equalTo($registrationData), $this->equalTo($data)) + ->willReturn([]); + + $resp = $this->controller->finishRegister($registrationData, $data); - $this->controller->finishRegister($registrationData, $data); + $this->assertEquals(new JSONResponse([]), $resp); } } diff --git a/tests/Unit/Service/U2FManagerTest.php b/tests/Unit/Service/U2FManagerTest.php index fdf65a2c..520f4198 100644 --- a/tests/Unit/Service/U2FManagerTest.php +++ b/tests/Unit/Service/U2FManagerTest.php @@ -141,19 +141,4 @@ public function testStartRegistrationFirstDevice() { $this->manager->startRegistration($user); } - public function testFinishRegistration() { - // TODO: get a grasp of how the u2f lib works and feed it with - // realistic data or mock it. - } - - public function testStartAuthenticate() { - // TODO: get a grasp of how the u2f lib works and feed it with - // realistic data or mock it. - } - - public function testFinishAuthenticate() { - // TODO: get a grasp of how the u2f lib works and feed it with - // realistic data or mock it. - } - }