Merge pull request #239 from nextcloud/enhancement/php7

Make app strict
nextcloud · Mar 13, 2018 · 920fa2a · 920fa2a
2 parents d63050e + da6100e
commit 920fa2a
Show file tree

Hide file tree

Showing 12 changed files with 124 additions and 86 deletions.
diff --git a/composer.json b/composer.json
@@ -1,7 +1,8 @@
 {
 	"require": {
 		"christian-riesen/otp": "1.*",
-		"endroid/qr-code": "^2.5"
+		"endroid/qr-code": "^2.5",
+		"christophwurst/nextcloud": "14.0.0.x-dev"
 	},
 	"platform": {
 		"php": ">=7.0.0"

diff --git a/composer.lock b/composer.lock
diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php
@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types = 1);
+
 /**
  * @author Christoph Wurst <christoph@winzerhof-wurst.at>
  *
@@ -27,7 +29,7 @@
 
 class Application extends App {
 
-	public function __construct($urlParams = []) {
+	public function __construct(array $urlParams = []) {
 		parent::__construct('twofactor_totp', $urlParams);
 
 		$container = $this->getContainer();

diff --git a/lib/Controller/SettingsController.php b/lib/Controller/SettingsController.php
@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types = 1);
+
 /**
  * @author Christoph Wurst <christoph@winzerhof-wurst.at>
  *
@@ -29,6 +31,7 @@
 use OCP\Defaults;
 use OCP\IRequest;
 use OCP\IUserSession;
+use function is_null;
 
 class SettingsController extends Controller {
 
@@ -41,14 +44,7 @@ class SettingsController extends Controller {
 	/** @var Defaults */
 	private $defaults;
 
-	/**
-	 * @param string $appName
-	 * @param IRequest $request
-	 * @param IUserSession $userSession
-	 * @param ITotp $totp
-	 * @param Defaults $defaults
-	 */
-	public function __construct($appName, IRequest $request, IUserSession $userSession, ITotp $totp, Defaults $defaults) {
+	public function __construct(string $appName, IRequest $request, IUserSession $userSession, ITotp $totp, Defaults $defaults) {
 		parent::__construct($appName, $request);
 		$this->userSession = $userSession;
 		$this->totp = $totp;
@@ -59,28 +55,33 @@ public function __construct($appName, IRequest $request, IUserSession $userSessi
 	 * @NoAdminRequired
 	 * @return JSONResponse
 	 */
-	public function state() {
+	public function state(): JSONResponse {
 		$user = $this->userSession->getUser();
-		return [
+		if (is_null($user)) {
+			throw new Exception('user not available');
+		}
+		return new JSONResponse([
 			'state' => $this->totp->hasSecret($user),
-		];
+		]);
 	}
 
 	/**
 	 * @NoAdminRequired
 	 * @PasswordConfirmationRequired
 	 * @param int $state
 	 * @param string|null $key for verification
-	 * @return JSONResponse
 	 */
-	public function enable($state, $key = null) {
+	public function enable(int $state, string $key = null): JSONResponse {
 		$user = $this->userSession->getUser();
+		if (is_null($user)) {
+			throw new Exception('user not available');
+		}
 		switch ($state) {
 			case ITotp::STATE_DISABLED:
 				$this->totp->deleteSecret($user);
-				return [
+				return new JSONResponse([
 					'state' => ITotp::STATE_DISABLED,
-				];
+				]);
 			case ITotp::STATE_CREATED:
 				$secret = $this->totp->createSecret($user);
 
@@ -90,16 +91,16 @@ public function enable($state, $key = null) {
 				$qr = $qrCode->setText("otpauth://totp/$secretName?secret=$secret&issuer=$issuer")
 					->setSize(150)
 					->writeDataUri();
-				return [
+				return new JSONResponse([
 					'state' => ITotp::STATE_CREATED,
 					'secret' => $secret,
 					'qr' => $qr,
-				];
+				]);
 			case ITotp::STATE_ENABLED:
 				$success = $this->totp->enable($user, $key);
-				return [
+				return new JSONResponse([
 					'state' => $success ? ITotp::STATE_ENABLED : ITotp::STATE_CREATED,
-				];
+				]);
 			default:
 				throw new InvalidArgumentException('Invalid TOTP state');
 		}
@@ -110,18 +111,18 @@ public function enable($state, $key = null) {
 	 *
 	 * @return string
 	 */
-	private function getSecretName() {
+	private function getSecretName(): string {
 		$productName = $this->defaults->getName();
 		$userName = $this->userSession->getUser()->getCloudId();
 		return rawurlencode("$productName:$userName");
 	}
 
 	/**
-	 * The issuer, e.g. "Nextcloud" or "ownCloud"
+	 * The issuer, e.g. "Nextcloud"
 	 *
 	 * @return string
 	 */
-	private function getSecretIssuer() {
+	private function getSecretIssuer(): string {
 		$productName = $this->defaults->getName();
 		return rawurlencode($productName);
 	}

diff --git a/lib/Db/TotpSecret.php b/lib/Db/TotpSecret.php
@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types = 1);
+
 /**
  * @author Christoph Wurst <christoph@winzerhof-wurst.at>
  *

diff --git a/lib/Db/TotpSecretMapper.php b/lib/Db/TotpSecretMapper.php
@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types = 1);
+
 /**
  * @author Christoph Wurst <christoph@winzerhof-wurst.at>
  *
@@ -29,30 +31,30 @@
 
 class TotpSecretMapper extends Mapper {
 
-    public function __construct(IDBConnection $db) {
-        parent::__construct($db, 'twofactor_totp_secrets');
-    }
-
-    /**
-     * @param IUser $user
-     * @throws DoesNotExistException
-     * @return TotpSecret
-     */
-    public function getSecret(IUser $user) {
-        /* @var $qb IQueryBuilder */
-        $qb = $this->db->getQueryBuilder();
-
-        $qb->select('id', 'user_id', 'secret', 'state')
-                ->from('twofactor_totp_secrets')
-                ->where($qb->expr()->eq('user_id', $qb->createNamedParameter($user->getUID())));
-        $result = $qb->execute();
-
-        $row = $result->fetch();
-	$result->closeCursor();
-        if ($row === false) {
-            throw new DoesNotExistException('Secret does not exist');
-        }
-        return TotpSecret::fromRow($row);
-    }
+	public function __construct(IDBConnection $db) {
+		parent::__construct($db, 'twofactor_totp_secrets');
+	}
+
+	/**
+	 * @param IUser $user
+	 * @throws DoesNotExistException
+	 * @return TotpSecret
+	 */
+	public function getSecret(IUser $user): TotpSecret {
+		/* @var $qb IQueryBuilder */
+		$qb = $this->db->getQueryBuilder();
+
+		$qb->select('id', 'user_id', 'secret', 'state')
+			->from('twofactor_totp_secrets')
+			->where($qb->expr()->eq('user_id', $qb->createNamedParameter($user->getUID())));
+		$result = $qb->execute();
+
+		$row = $result->fetch();
+		$result->closeCursor();
+		if ($row === false) {
+			throw new DoesNotExistException('Secret does not exist');
+		}
+		return TotpSecret::fromRow($row);
+	}
 
 }
diff --git a/lib/Exception/NoTotpSecretFoundException.php b/lib/Exception/NoTotpSecretFoundException.php
@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types = 1);
+
 /**
  * @author Christoph Wurst <christoph@winzerhof-wurst.at>
  *

diff --git a/lib/Exception/TotpSecretAlreadySet.php b/lib/Exception/TotpSecretAlreadySet.php
@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types = 1);
+
 /**
  * @author Christoph Wurst <christoph@winzerhof-wurst.at>
  *

diff --git a/lib/Provider/TotpProvider.php b/lib/Provider/TotpProvider.php
@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types = 1);
+
 /**
  * @author Christoph Wurst <christoph@winzerhof-wurst.at>
  *
@@ -35,49 +37,36 @@ class TotpProvider implements IProvider {
 	/** @var IL10N */
 	private $l10n;
 
-	/**
-	 * @param ITotp $totp
-	 * @param IL10N $l10n
-	 */
 	public function __construct(ITotp $totp, IL10N $l10n) {
 		$this->totp = $totp;
 		$this->l10n = $l10n;
 	}
 
 	/**
 	 * Get unique identifier of this 2FA provider
-	 *
-	 * @return string
 	 */
-	public function getId() {
+	public function getId(): string {
 		return 'totp';
 	}
 
 	/**
 	 * Get the display name for selecting the 2FA provider
-	 *
-	 * @return string
 	 */
-	public function getDisplayName() {
+	public function getDisplayName(): string {
 		return 'TOTP (Authenticator app)';
 	}
 
 	/**
 	 * Get the description for selecting the 2FA provider
-	 *
-	 * @return string
 	 */
-	public function getDescription() {
+	public function getDescription(): string {
 		return $this->l10n->t('Authenticate with a TOTP app');
 	}
 
 	/**
 	 * 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 {
 		$tmpl = new Template('twofactor_totp', 'challenge');
 		return $tmpl;
 	}
@@ -95,11 +84,8 @@ public function verifyChallenge(IUser $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 $this->totp->hasSecret($user);
 	}
 

diff --git a/lib/Service/ITotp.php b/lib/Service/ITotp.php
@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types = 1);
+
 /**
  * @author Christoph Wurst <christoph@winzerhof-wurst.at>
  *
@@ -45,7 +47,7 @@ public function hasSecret(IUser $user);
 	 * @return string the newly created secret
 	 * @throws TotpSecretAlreadySet
 	 */
-	public function createSecret(IUser $user);
+	public function createSecret(IUser $user): string;
 
 	/**
 	 * Enable OTP for the given user. The secret has to be generated
@@ -56,7 +58,7 @@ public function createSecret(IUser $user);
 	 * @return bool whether the key is valid and the secret has been enabled
 	 * @throws DoesNotExistException
 	 */
-	public function enable(IUser $user, $key);
+	public function enable(IUser $user, $key): bool;
 
 	/**
 	 * @param IUser $user
@@ -67,5 +69,5 @@ public function deleteSecret(IUser $user);
 	 * @param IUser $user
 	 * @param string $key
 	 */
-	public function validateSecret(IUser $user, $key);
+	public function validateSecret(IUser $user, $key): bool;
 }