Make both Cabins fully type-safe.

paragonie · Jan 13, 2018 · abc63c3 · abc63c3
1 parent 013b86f
commit abc63c3
Show file tree

Hide file tree

Showing 49 changed files with 532 additions and 289 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -52,5 +52,5 @@ install:
 
 script:
   - vendor/bin/phpunit
-  - vendor/bin/psalm
+  - psalm.sh
 
diff --git a/psalm.sh b/psalm.sh
@@ -0,0 +1,24 @@
+#!/usr/bin/env bash
+
+echo "Checking the engine..."
+echo -e "Using \e[32mpsalm.xml\e[39m:"
+status=0
+
+vendor/bin/psalm
+status=$(($status + $?))
+
+echo "Checking each Cabin:"
+for dir in src/Cabin/*
+do
+    if [[ -d "${dir}" ]]; then
+        if [[ -f "${dir}/psalm.xml" ]]; then
+            echo -e "Using \e[32m${dir}/psalm.xml\e[39m:"
+            vendor/bin/psalm --config="${dir}/psalm.xml"
+            status=$(($status + $?))
+        else
+            echo -e "Cannot test Cabin; \e[31m${dir}/psalm.xml\e[39m not found."
+        fi
+    fi
+done
+
+exit $status
diff --git a/src/Cabin/Bridge/AirshipFilter.php b/src/Cabin/Bridge/AirshipFilter.php
@@ -78,7 +78,7 @@ public function __construct()
                 (new IntFilter())
                     ->setDefault(32)
                     ->addCallback(
-                        function ($var): int {
+                        function (int $var): int {
                             if ($var < 1) {
                                 return 1;
                             } elseif ($var > 32) {
@@ -92,7 +92,7 @@ function ($var): int {
                 (new IntFilter())
                     ->setDefault(64)
                     ->addCallback(
-                        function ($var): int {
+                        function (int $var): int {
                             if ($var < 1) {
                                 return 1;
                             } elseif ($var > 128) {
@@ -108,7 +108,7 @@ function ($var): int {
             )
             ->addFilter('universal.rate-limiting.log-public-key',
                 (new StringFilter())
-                    ->addCallback(function ($var): string {
+                    ->addCallback(function (string $var): string {
                         // Hex-encoded public keys are 64-char hex strings.
                         if (\preg_match('/^[0-9A-Fa-f]{64}$/', $var)) {
                             return \strtolower($var);

diff --git a/src/Cabin/Bridge/ConfigFilter.php b/src/Cabin/Bridge/ConfigFilter.php
@@ -47,7 +47,7 @@ public function __construct()
             ->addFilter(
                 'config_extra.two-factor.length',
                 (new IntFilter())->addCallback(
-                    function ($var): int {
+                    function (int $var): int {
                         if ($var < 6) {
                             return 6;
                         } elseif ($var > 8) {

diff --git a/src/Cabin/Bridge/Controller/Account.php b/src/Cabin/Bridge/Controller/Account.php
@@ -17,6 +17,7 @@
     AutoPilot,
     Bolt\Security,
     Gears,
+    Model,
     State
 };
 use Airship\Engine\Security\{
@@ -70,11 +71,15 @@ class Account extends ControllerGear
      * This function is called after the dependencies have been injected by
      * AutoPilot. Think of it as a user-land constructor.
      */
-    public function airshipLand()
+    public function airshipLand(): void
     {
         parent::airshipLand();
         $this->storeViewVar('showmenu', true);
-        $this->acct = $this->model('UserAccounts');
+        $acct = $this->model('UserAccounts');
+        if (!($acct instanceof UserAccounts)) {
+            throw new \TypeError(Model::TYPE_ERROR);
+        }
+        $this->acct = $acct;
         $this->includeAjaxToken();
     }
 
@@ -83,7 +88,7 @@ public function airshipLand()
      *
      * @route board
      */
-    public function board()
+    public function board(): void
     {
         if ($this->isLoggedIn())  {
             // You're already logged in!
@@ -135,7 +140,7 @@ public function board()
      *
      * @route login
      */
-    public function login()
+    public function login(): void
     {
         if ($this->isLoggedIn())  {
             // You're already logged in!
@@ -156,7 +161,7 @@ public function login()
      * @route logout/(.*)
      * @param string $token
      */
-    public function logout(string $token)
+    public function logout(string $token): void
     {
         if (!$this->isLoggedIn())  {
             \Airship\redirect($this->airship_cabin_prefix);
@@ -172,7 +177,7 @@ public function logout(string $token)
     /**
      * @route my/account
      */
-    public function my()
+    public function my(): void
     {
         if (!$this->isLoggedIn())  {
             \Airship\redirect($this->airship_cabin_prefix);
@@ -200,7 +205,7 @@ public function my()
     /**
      * @route my
      */
-    public function myIndex()
+    public function myIndex(): void
     {
         \Airship\redirect($this->airship_cabin_prefix . '/my/account');
     }
@@ -210,7 +215,7 @@ public function myIndex()
      *
      * @route my/preferences
      */
-    public function preferences()
+    public function preferences(): void
     {
         if (!$this->isLoggedIn())  {
             \Airship\redirect($this->airship_cabin_prefix);
@@ -254,7 +259,7 @@ public function preferences()
      * @param string $page
      * @route users{_page}
      */
-    public function publicDirectory(string $page = '')
+    public function publicDirectory(string $page = ''): void
     {
         if (!$this->isLoggedIn())  {
             \Airship\redirect($this->airship_cabin_prefix);
@@ -280,7 +285,7 @@ public function publicDirectory(string $page = '')
      * @route recover-account
      * @param string $token
      */
-    public function recoverAccount(string $token = '')
+    public function recoverAccount(string $token = ''): void
     {
         if ($this->isLoggedIn())  {
             \Airship\redirect($this->airship_cabin_prefix);
@@ -315,7 +320,7 @@ public function recoverAccount(string $token = '')
      * @route my/account/2-factor/qr-code
      *
      */
-    public function twoFactorSetupQRCode()
+    public function twoFactorSetupQRCode(): void
     {
         if (!$this->isLoggedIn())  {
             \Airship\redirect($this->airship_cabin_prefix);
@@ -345,7 +350,7 @@ public function twoFactorSetupQRCode()
     /**
      * @route my/account/2-factor
      */
-    public function twoFactorSetup()
+    public function twoFactorSetup(): void
     {
         if (!$this->isLoggedIn())  {
             \Airship\redirect($this->airship_cabin_prefix);
@@ -424,7 +429,7 @@ protected function processAccountUpdate(
         array $post = [],
         array $account = [],
         string $gpg_public_key = ''
-    ) {
+    ): void {
         if (!empty($post['passphrase'])) {
             // Lazy hack
             $post['username'] = $account['username'];
@@ -516,7 +521,7 @@ protected function processAccountUpdate(
      * @throws \Airship\Alerts\Router\ControllerComplete
      * @throws \TypeError
      */
-    protected function processBoard(array $post = [])
+    protected function processBoard(array $post = []): void
     {
         if (empty($post['username']) || empty($post['passphrase'])) {
             $this->view(
@@ -586,7 +591,7 @@ protected function processBoard(array $post = [])
      * @throws \ParagonIE\Halite\Alerts\InvalidType
      * @throws \TypeError
      */
-    protected function processLogin(array $post = [])
+    protected function processLogin(array $post = []): void
     {
         $state = State::instance();
 
@@ -844,7 +849,9 @@ protected function processRecoverAccount(array $post): bool
             ??
         'no-reply@' . AutoPilot::getHttpHost();
         if (!Util::isValidEmail($from)) {
-            $from = 'no-reply@[' . \long2ip(\ip2long($_SERVER['SERVER_ADDR'])) . ']';
+            $from = 'no-reply@[' . \long2ip(
+                (int)\ip2long($_SERVER['SERVER_ADDR'])
+            ) . ']';
         }
 
         $message = (new Message())
@@ -876,7 +883,7 @@ protected function processRecoverAccount(array $post): bool
      *
      * @param string $token
      */
-    protected function processRecoveryToken(string $token)
+    protected function processRecoveryToken(string $token): void
     {
         if (Util::stringLength($token) < UserAccounts::RECOVERY_CHAR_LENGTH) {
             \Airship\redirect($this->airship_cabin_prefix . '/login');
@@ -1001,7 +1008,7 @@ protected function twoFactorPreamble(int $userID = 0): GoogleAuth
      * @param int|null $per_page
      * @return int[]
      */
-    protected function getOffsetAndLimit($page = null, ?int $per_page = null)
+    protected function getOffsetAndLimit($page = null, ?int $per_page = null): array
     {
         if (!$per_page) {
             $per_page = $this->config('user-directory.per-page') ?? 20;

diff --git a/src/Cabin/Bridge/Controller/Admin.php b/src/Cabin/Bridge/Controller/Admin.php
@@ -10,9 +10,7 @@
     SettingsFilter
 };
 use Airship\Engine\{
-    Hail,
-    Security\Util,
-    State
+    Hail, Model, Security\Util, State
 };
 use Psr\Log\LogLevel;
 
@@ -33,10 +31,15 @@ class Admin extends AdminOnly
      * This function is called after the dependencies have been injected by
      * AutoPilot. Think of it as a user-land constructor.
      */
-    public function airshipLand()
+    public function airshipLand(): void
     {
         parent::airshipLand();
-        $this->acct = $this->model('UserAccounts');
+        $acct = $this->model('UserAccounts');
+        if (!($acct instanceof UserAccounts)) {
+            throw new \TypeError(Model::TYPE_ERROR);
+        }
+        $this->acct = $acct;
+
         if (!empty($_GET['msg'])) {
             if ($_GET['msg'] === 'saved') {
                 $this->storeViewVar(
@@ -117,7 +120,7 @@ protected function deleteNotary(array $channels, array $post): bool
         if (!isset($channels[$ch])) {
             return false;
         }
-        $idx += 0;
+        $idx = (int) $idx;
 
         if ($channels[$ch]['notaries'][$idx]) {
             unset($channels[$ch]['notaries'][$idx]);
@@ -131,15 +134,15 @@ protected function deleteNotary(array $channels, array $post): bool
     /**
      * @route admin
      */
-    public function index()
+    public function index(): void
     {
         $this->view('admin');
     }
 
     /**
      * @route admin/database
      */
-    public function manageDatabase()
+    public function manageDatabase(): void
     {
         $databases = \Airship\loadJSON(
             ROOT . '/config/databases.json'
@@ -169,7 +172,7 @@ public function manageDatabase()
      *
      * @route admin/extensions
      */
-    public function manageExtensions()
+    public function manageExtensions(): void
     {
         $this->view('admin_extensions');
     }
@@ -179,7 +182,7 @@ public function manageExtensions()
      *
      * @route admin/notaries
      */
-    public function manageNotaries()
+    public function manageNotaries(): void
     {
         $this->storeViewVar('active_submenu', ['Admin', 'Extensions']);
         $channels = \Airship\loadJSON(ROOT . '/config/channels.json');
@@ -209,7 +212,7 @@ public function manageNotaries()
     /**
      * @route admin/settings
      */
-    public function manageSettings()
+    public function manageSettings(): void
     {
         $state = State::instance();
         $settings = [