Fix/initialize security service (#112)

* Code cleanup * Fixed security service initialization * Improved CS * Change getSessionToken() to getRequestToken() in CSRF forms * Added workaround for phalcon/cphalcon#14346 * Do not use helper container()
phalcon · Sep 2, 2019 · 3a7b403 · 3a7b403
1 parent c24a6af
commit 3a7b403
Show file tree

Hide file tree

Showing 11 changed files with 138 additions and 25 deletions.
diff --git a/app/Controllers/PermissionsController.php b/app/Controllers/PermissionsController.php
@@ -53,10 +53,10 @@ public function indexAction(): void
                 $this->acl->rebuild();
 
                 // Pass the current permissions to the view
-                $this->view->permissions = $this->acl->getPermissions($profile);
+                $this->view->setVar('permissions', $this->acl->getPermissions($profile));
             }
 
-            $this->view->profile = $profile;
+            $this->view->setVar('profile', $profile);
         }
 
         $profiles = Profiles::find([
@@ -66,7 +66,7 @@ public function indexAction(): void
             ]
         ]);
 
-        $this->view->profilesSelect = $this->tag->select([
+        $profilesSelect = $this->tag->select([
             'profileId',
             $profiles,
             'using' => [
@@ -78,5 +78,7 @@ public function indexAction(): void
             'emptyValue' => '',
             'class' => 'form-control mr-sm-2',
         ]);
+
+        $this->view->setVar('profilesSelect', $profilesSelect);
     }
 }
diff --git a/app/Controllers/ProfilesController.php b/app/Controllers/ProfilesController.php
@@ -38,7 +38,7 @@ public function initialize(): void
     public function indexAction(): void
     {
         $this->persistent->conditions = null;
-        $this->view->form = new ProfilesForm();
+        $this->view->setVar('form', new ProfilesForm(null));
     }
 
     /**
@@ -74,7 +74,7 @@ public function searchAction()
             "page" => $numberPage
         ]);
 
-        $this->view->page = $paginator->paginate();
+        $this->view->setVar('page', $paginator->paginate());
     }
 
     /**
@@ -97,7 +97,7 @@ public function createAction(): void
             }
         }
 
-        $this->view->form = new ProfilesForm(null);
+        $this->view->setVar('form', new ProfilesForm(null));
     }
 
     /**
@@ -130,11 +130,10 @@ public function editAction($id)
             }
         }
 
-        $this->view->form = new ProfilesForm(null, [
-            'edit' => true
+        $this->view->setVars([
+            'form' => new ProfilesForm(null, ['edit' => true]),
+            'profile' => $profile,
         ]);
-
-        $this->view->profile = $profile;
     }
 
     /**

diff --git a/app/Controllers/SessionController.php b/app/Controllers/SessionController.php
@@ -65,7 +65,7 @@ public function signupAction()
             }
         }
 
-        $this->view->form = $form;
+        $this->view->setVar('form', $form);
     }
 
     /**
@@ -99,7 +99,7 @@ public function loginAction()
             $this->flash->error($e->getMessage());
         }
 
-        $this->view->form = $form;
+        $this->view->setVar('form', $form);
     }
 
     /**
@@ -139,7 +139,7 @@ public function forgotPasswordAction(): void
             }
         }
 
-        $this->view->form = $form;
+        $this->view->setVar('form', $form);
     }
 
     /**

diff --git a/app/Controllers/UsersController.php b/app/Controllers/UsersController.php
@@ -36,7 +36,7 @@ public function initialize(): void
     public function indexAction(): void
     {
         $this->persistent->conditions = null;
-        $this->view->form = new UsersForm();
+        $this->view->setVar('form', new UsersForm());
     }
 
     /**
@@ -71,7 +71,7 @@ public function searchAction()
             "page" => $numberPage
         ]);
 
-        $this->view->page = $paginator->paginate();
+        $this->view->setVar('page', $paginator->paginate());
     }
 
     /**
@@ -102,7 +102,7 @@ public function createAction(): void
             }
         }
 
-        $this->view->form = $form;
+        $this->view->setVar('form', $form);
     }
 
     /**
@@ -148,9 +148,11 @@ public function editAction($id)
             }
         }
 
-        $this->view->user = $user;
-        $this->view->form = new UsersForm(null, [
-            'edit' => true
+        $this->view->setVars([
+            'user' => $user,
+            'form' => new UsersForm(null, [
+                'edit' => true
+            ]),
         ]);
     }
 
@@ -215,6 +217,6 @@ public function changePasswordAction(): void
             }
         }
 
-        $this->view->form = $form;
+        $this->view->setVar('form', $form);
     }
 }
diff --git a/app/Forms/LoginForm.php b/app/Forms/LoginForm.php
@@ -65,7 +65,7 @@ public function initialize()
         // CSRF
         $csrf = new Hidden('csrf');
         $csrf->addValidator(new Identical([
-            'value' => $this->security->getSessionToken(),
+            'value' => $this->security->getRequestToken(),
             'message' => 'CSRF validation failed'
         ]));
         $csrf->clear();

diff --git a/app/Forms/SignUpForm.php b/app/Forms/SignUpForm.php
@@ -68,7 +68,7 @@ public function initialize(string $entity = null, array $options = [])
                 'messageMinimum' => 'Password is too short. Minimum 8 characters'
             ]),
             new Confirmation([
-                'message' => 'Password doesn\'t match confirmation',
+                'message' => "Password doesn't match confirmation",
                 'with' => 'confirmPassword'
             ])
         ]);
@@ -102,7 +102,7 @@ public function initialize(string $entity = null, array $options = [])
         // CSRF
         $csrf = new Hidden('csrf');
         $csrf->addValidator(new Identical([
-            'value' => $this->security->getSessionToken(),
+            'value' => $this->security->getRequestToken(),
             'message' => 'CSRF validation failed'
         ]));
         $csrf->clear();

diff --git a/app/Helpers.php b/app/Helpers.php
@@ -4,11 +4,12 @@
 namespace Vokuro;
 
 use Phalcon\Di;
+use Phalcon\Di\DiInterface;
 
 /**
  * Call Dependency Injection container
  *
- * @return mixed
+ * @return mixed|null|DiInterface
  */
 function container()
 {

diff --git a/app/Phalcon/Beta2FixSecurity.php b/app/Phalcon/Beta2FixSecurity.php
@@ -0,0 +1,48 @@
+<?php
+declare(strict_types=1);
+
+namespace Phalcon;
+
+/**
+ * Extended class for fixing phalcon/cphalcon#14346 issue
+ *
+ * @see https://github.com/phalcon/cphalcon/pull/14347
+ */
+class Beta2FixSecurity extends Security
+{
+    /**
+     * @inheritDoc
+     */
+    public function getRequestToken(): string
+    {
+        if (empty($this->requestToken)) {
+            return $this->getSessionToken();
+        }
+
+        return (string) $this->requestToken;
+    }
+
+    /**
+     * @inheritDoc
+     *
+     * @return string
+     * @throws Exception
+     */
+    public function getSessionToken(): string
+    {
+        $di = $this->getDI();
+
+        if (!is_object($di)) {
+            throw new Exception(
+                Exception::containerServiceNotFound("the 'session' service")
+            );
+        }
+
+        if ($di->has('session')) {
+            $session = $di->getShared('session');
+            return (string) $session->get($this->tokenValueSessionId);
+        }
+
+        return '';
+    }
+}
diff --git a/app/Plugins/Auth/Auth.php b/app/Plugins/Auth/Auth.php
@@ -226,7 +226,7 @@ public function checkUserFlags(Users $user)
     /**
      * Returns the current identity
      *
-     * @return array
+     * @return array|null
      */
     public function getIdentity()
     {

diff --git a/app/Providers/SecurityProvider.php b/app/Providers/SecurityProvider.php
@@ -0,0 +1,60 @@
+<?php
+declare(strict_types=1);
+
+/**
+ * This file is part of the Vökuró.
+ *
+ * (c) Phalcon Team <team@phalcon.io>
+ *
+ * For the full copyright and license information, please view
+ * the LICENSE file that was distributed with this source code.
+ */
+
+namespace Vokuro\Providers;
+
+use Phalcon\Beta2FixSecurity;
+use Phalcon\Di\DiInterface;
+use Phalcon\Di\ServiceProviderInterface;
+use Phalcon\Security;
+use Phalcon\Version;
+
+class SecurityProvider implements ServiceProviderInterface
+{
+    /**
+     * @var string
+     */
+    protected $providerName = 'security';
+
+    /**
+     * @param DiInterface $di
+     * @return void
+     */
+    public function register(DiInterface $di): void
+    {
+        $that = $this;
+        $di->set($this->providerName, function () use ($di, $that) {
+            return $that->getSecurity($di);
+        });
+    }
+
+    /**
+     * Remove current method after after next release of Phalcon 4
+     *
+     * @see https://github.com/phalcon/cphalcon/issues/14346
+     *
+     * @param DiInterface $di
+     * @return Security
+     */
+    protected function getSecurity(DiInterface $di): Security
+    {
+        if (Version::get() !== '4.0.0-beta.2') {
+            $security = new Security();
+        } else {
+            $security = new Beta2FixSecurity();
+        }
+
+        $security->setDI($di);
+
+        return $security;
+    }
+}
diff --git a/configs/providers.php b/configs/providers.php
@@ -23,6 +23,7 @@
     \Vokuro\Providers\RouterProvider::class,
     \Vokuro\Providers\SessionBagProvider::class,
     \Vokuro\Providers\SessionProvider::class,
+    \Vokuro\Providers\SecurityProvider::class,
     \Vokuro\Providers\UrlProvider::class,
     \Vokuro\Providers\ViewProvider::class,
 ];