[TASK] General code cleanup in ext:rsaauth 03/37303/2
authorWouter Wolters <typo3@wouterwolters.nl>
Thu, 26 Feb 2015 22:50:46 +0000 (23:50 +0100)
committerAnja Leichsenring <aleichsenring@ab-softlab.de>
Sat, 28 Feb 2015 20:22:49 +0000 (21:22 +0100)
Resolves: #65374
Releases: master
Change-Id: I64d942327a49d6e5cb6d292ee260b4c37b459374
Reviewed-on: http://review.typo3.org/37303
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Markus Klein <klein.t3@reelworx.at>
Tested-by: Markus Klein <klein.t3@reelworx.at>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
typo3/sysext/rsaauth/Classes/Backend/AbstractBackend.php
typo3/sysext/rsaauth/Classes/Backend/BackendFactory.php
typo3/sysext/rsaauth/Classes/Backend/CommandLineBackend.php
typo3/sysext/rsaauth/Classes/Backend/PhpBackend.php
typo3/sysext/rsaauth/Classes/BackendWarnings.php
typo3/sysext/rsaauth/Classes/Hook/BackendHookForAjaxLogin.php
typo3/sysext/rsaauth/Classes/Hook/LoginFormHook.php
typo3/sysext/rsaauth/Classes/Keypair.php
typo3/sysext/rsaauth/Classes/RsaAuthService.php
typo3/sysext/rsaauth/Classes/Storage/StorageFactory.php

index 7802218..e5ad9c8 100644 (file)
@@ -65,7 +65,7 @@ abstract class AbstractBackend {
        /**
         * Checks if this backend is available for calling.
         *
-        * @return void
+        * @return bool
         */
        abstract public function isAvailable();
 
index 262fb70..af38207 100644 (file)
@@ -29,8 +29,8 @@ class BackendFactory {
         * @var array
         */
        static protected $availableBackends = array(
-               \TYPO3\CMS\Rsaauth\Backend\PhpBackend::class,
-               \TYPO3\CMS\Rsaauth\Backend\CommandLineBackend::class
+               PhpBackend::class,
+               CommandLineBackend::class
        );
 
        /**
@@ -44,18 +44,18 @@ class BackendFactory {
        /**
         * A selected backend. This member is set in the getBackend() function. It
         * will not be an abstract backend as shown below but a real class, which is
-        * derived from the \TYPO3\CMS\Rsaauth\Backend\AbstractBackend.
+        * derived from the AbstractBackend.
         *
-        * @var \TYPO3\CMS\Rsaauth\Backend\AbstractBackend
+        * @var AbstractBackend
         */
        static protected $selectedBackend = NULL;
 
        /**
         * Obtains a backend. This function will return a non-abstract class, which
-        * is derived from the \TYPO3\CMS\Rsaauth\Backend\AbstractBackend. Applications should
-        * not use any methods that are not declared in the \TYPO3\CMS\Rsaauth\Backend\AbstractBackend.
+        * is derived from the AbstractBackend. Applications should
+        * not use any methods that are not declared in the AbstractBackend.
         *
-        * @return \TYPO3\CMS\Rsaauth\Backend\AbstractBackend A backend
+        * @return AbstractBackend A backend
         */
        static public function getBackend() {
                if (!self::$initialized) {
@@ -64,7 +64,7 @@ class BackendFactory {
                                $backendObject = \TYPO3\CMS\Core\Utility\GeneralUtility::getUserObj($backend);
                                // Check that it is derived from the proper base class
                                if ($backendObject instanceof AbstractBackend) {
-                                       /** @var $backendObject \TYPO3\CMS\Rsaauth\Backend\AbstractBackend */
+                                       /** @var $backendObject AbstractBackend */
                                        if ($backendObject->isAvailable()) {
                                                // The backend is available, save it and stop the loop
                                                self::$selectedBackend = $backendObject;
index ddfd5f6..d2a942d 100644 (file)
@@ -15,6 +15,7 @@ namespace TYPO3\CMS\Rsaauth\Backend;
  */
 
 use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Core\Utility\CommandUtility;
 
 /**
  * This class contains a OpenSSL backend for the TYPO3 RSA authentication
@@ -33,7 +34,7 @@ class CommandLineBackend extends AbstractBackend {
        /**
         * A path to the openssl binary or FALSE if the binary does not exist
         *
-        * @var mixed
+        * @var string|bool
         */
        protected $opensslPath;
 
@@ -51,11 +52,16 @@ class CommandLineBackend extends AbstractBackend {
         * binary.
         */
        public function __construct() {
-               $this->opensslPath = \TYPO3\CMS\Core\Utility\CommandUtility::getCommand('openssl');
+               $this->opensslPath = CommandUtility::getCommand('openssl');
                $this->temporaryDirectory = PATH_site . 'typo3temp';
                // Get temporary directory from the configuration
                $extconf = unserialize($GLOBALS['TYPO3_CONF_VARS']['EXT']['extConf']['rsaauth']);
-               if ($extconf['temporaryDirectory'] != '' && $extconf['temporaryDirectory'][0] == '/' && @is_dir($extconf['temporaryDirectory']) && is_writable($extconf['temporaryDirectory'])) {
+               if (
+                       $extconf['temporaryDirectory'] !== ''
+                       && $extconf['temporaryDirectory'][0] === '/'
+                       && @is_dir($extconf['temporaryDirectory'])
+                       && is_writable($extconf['temporaryDirectory'])
+               ) {
                        $this->temporaryDirectory = $extconf['temporaryDirectory'];
                }
        }
@@ -93,13 +99,13 @@ class CommandLineBackend extends AbstractBackend {
                } else {
                        $command .= ' 2>/dev/null';
                }
-               \TYPO3\CMS\Core\Utility\CommandUtility::exec($command);
+               CommandUtility::exec($command);
                // Test that we got a private key
                $privateKey = file_get_contents($privateKeyFile);
                if (FALSE !== strpos($privateKey, 'BEGIN RSA PRIVATE KEY')) {
                        // Ok, we got the private key. Get the modulus.
                        $command = $this->opensslPath . ' rsa -noout -modulus -in ' . escapeshellarg($privateKeyFile);
-                       $value = \TYPO3\CMS\Core\Utility\CommandUtility::exec($command);
+                       $value = CommandUtility::exec($command);
                        if (substr($value, 0, 8) === 'Modulus=') {
                                $publicKey = substr($value, 8);
 
@@ -131,7 +137,7 @@ class CommandLineBackend extends AbstractBackend {
                $command = $this->opensslPath . ' rsautl -inkey ' . escapeshellarg($privateKeyFile) . ' -in ' . escapeshellarg($dataFile) . ' -decrypt';
                // Execute the command and capture the result
                $output = array();
-               \TYPO3\CMS\Core\Utility\CommandUtility::exec($command, $output);
+               CommandUtility::exec($command, $output);
                // Remove the file
                @unlink($privateKeyFile);
                @unlink($dataFile);
@@ -142,15 +148,15 @@ class CommandLineBackend extends AbstractBackend {
         * Checks if command line version of the OpenSSL is available and can be
         * executed successfully.
         *
-        * @return void
+        * @return bool
         * @see \TYPO3\CMS\Rsaauth\Backend\AbstractBackend::isAvailable()
         */
        public function isAvailable() {
                $result = FALSE;
                if ($this->opensslPath) {
                        // If path exists, test that command runs and can produce output
-                       $test = \TYPO3\CMS\Core\Utility\CommandUtility::exec($this->opensslPath . ' version');
-                       $result = substr($test, 0, 8) == 'OpenSSL ';
+                       $test = CommandUtility::exec($this->opensslPath . ' version');
+                       $result = substr($test, 0, 8) === 'OpenSSL ';
                }
                return $result;
        }
index 9503eef..c388e0b 100644 (file)
@@ -68,7 +68,7 @@ class PhpBackend extends AbstractBackend {
         *
         * @param string $privateKey The private key (obtained from a call to createNewKeyPair())
         * @param string $data Data to decrypt (base64-encoded)
-        * @return string Decrypted data or NULL in case of a error
+        * @return string|NULL Decrypted data or NULL in case of a error
         * @see \TYPO3\CMS\Rsaauth\Backend\AbstractBackend::decrypt()
         */
        public function decrypt($privateKey, $data) {
@@ -83,7 +83,7 @@ class PhpBackend extends AbstractBackend {
         * Checks if this backend is available for calling. In particular checks if
         * PHP OpenSSl extension is installed and functional.
         *
-        * @return void
+        * @return bool
         * @see \TYPO3\CMS\Rsaauth\Backend\AbstractBackend::isAvailable()
         */
        public function isAvailable() {
@@ -123,8 +123,7 @@ class PhpBackend extends AbstractBackend {
        protected function extractPublicKeyModulus($data) {
                $fragment = preg_replace('/.*Modulus.*?\\n(.*)Exponent:.*/ms', '\\1', $data);
                $fragment = preg_replace('/[\\s\\n\\r:]/', '', $fragment);
-               $result = trim(strtoupper(substr($fragment, 2)));
-               return $result;
+               return trim(strtoupper(substr($fragment, 2)));
        }
 
 }
index 4f21e22..6185cc9 100644 (file)
@@ -32,27 +32,37 @@ class BackendWarnings {
                $backend = \TYPO3\CMS\Rsaauth\Backend\BackendFactory::getBackend();
                if ($backend instanceof \TYPO3\CMS\Rsaauth\Backend\CommandLineBackend) {
                        // Not using the PHP extension!
-                       $warnings['rsaauth_cmdline'] = $GLOBALS['LANG']->sL('LLL:EXT:rsaauth/hooks/locallang.xlf:hook_using_cmdline');
+                       $lang = $this->getLanguageService();
+                       $warnings['rsaauth_cmdline'] = $lang->sL('LLL:EXT:rsaauth/hooks/locallang.xlf:hook_using_cmdline');
                        // Check the path
                        $extconf = unserialize($GLOBALS['TYPO3_CONF_VARS']['EXT']['extConf']['rsaauth']);
                        $path = trim($extconf['temporaryDirectory']);
                        if ($path == '') {
                                // Path is empty
-                               $warnings['rsaauth'] = $GLOBALS['LANG']->sL('LLL:EXT:rsaauth/hooks/locallang.xlf:hook_empty_directory');
+                               $warnings['rsaauth'] = $lang->sL('LLL:EXT:rsaauth/hooks/locallang.xlf:hook_empty_directory');
                        } elseif (!\TYPO3\CMS\Core\Utility\GeneralUtility::isAbsPath($path)) {
                                // Path is not absolute
-                               $warnings['rsaauth'] = $GLOBALS['LANG']->sL('LLL:EXT:rsaauth/hooks/locallang.xlf:hook_directory_not_absolute');
+                               $warnings['rsaauth'] = $lang->sL('LLL:EXT:rsaauth/hooks/locallang.xlf:hook_directory_not_absolute');
                        } elseif (!@is_dir($path)) {
                                // Path does not represent a directory
-                               $warnings['rsaauth'] = $GLOBALS['LANG']->sL('LLL:EXT:rsaauth/hooks/locallang.xlf:hook_directory_not_exist');
+                               $warnings['rsaauth'] = $lang->sL('LLL:EXT:rsaauth/hooks/locallang.xlf:hook_directory_not_exist');
                        } elseif (!@is_writable($path)) {
                                // Directory is not writable
-                               $warnings['rsaauth'] = $GLOBALS['LANG']->sL('LLL:EXT:rsaauth/hooks/locallang.xlf:hook_directory_not_writable');
+                               $warnings['rsaauth'] = $lang->sL('LLL:EXT:rsaauth/hooks/locallang.xlf:hook_directory_not_writable');
                        } elseif (substr($path, 0, strlen(PATH_site)) == PATH_site) {
                                // Directory is inside the site root
-                               $warnings['rsaauth'] = $GLOBALS['LANG']->sL('LLL:EXT:rsaauth/hooks/locallang.xlf:hook_directory_inside_siteroot');
+                               $warnings['rsaauth'] = $lang->sL('LLL:EXT:rsaauth/hooks/locallang.xlf:hook_directory_inside_siteroot');
                        }
                }
        }
 
+       /**
+        * Returns LanguageService
+        *
+        * @return \TYPO3\CMS\Lang\LanguageService
+        */
+       protected function getLanguageService() {
+               return $GLOBALS['LANG'];
+       }
+
 }
index 636beb1..60f4b46 100644 (file)
@@ -18,6 +18,7 @@ namespace TYPO3\CMS\Rsaauth\Hook;
  * This class adds RSA JavaScript to the backend
  */
 class BackendHookForAjaxLogin {
+
        /**
         * Adds RSA-specific JavaScript
         *
index e54c5e5..b062bf3 100644 (file)
@@ -28,8 +28,7 @@ class LoginFormHook {
         *
         * @param array $params
         * @param \TYPO3\CMS\Backend\Controller\LoginController $pObj
-        * @return string Form tag
-        * @throws \TYPO3\CMS\Core\Error\Exception
+        * @return string|NULL Form tag or NULL if security level is not rsa
         */
        public function getLoginFormTag(array $params, \TYPO3\CMS\Backend\Controller\LoginController &$pObj) {
                $form = NULL;
index d764657..026a36e 100644 (file)
@@ -67,9 +67,7 @@ class Keypair implements \TYPO3\CMS\Core\SingletonInterface {
         * Note: This method must not be called more than one time.
         *
         * @param int $exponent the new exponent
-        *
         * @return void
-        *
         * @throws \BadMethodCallException if the method was called more than one time
         */
        public function setExponent($exponent) {
@@ -104,9 +102,7 @@ class Keypair implements \TYPO3\CMS\Core\SingletonInterface {
         * Note: This method must not be called more than one time.
         *
         * @param string $privateKey The new private key
-        *
         * @return void
-        *
         * @throws \BadMethodCallException if the method was called more than one time
         */
        public function setPrivateKey($privateKey) {
@@ -141,9 +137,7 @@ class Keypair implements \TYPO3\CMS\Core\SingletonInterface {
         * Note: This method must not be called more than one time.
         *
         * @param int $publicKeyModulus the new public key modulus
-        *
         * @return void
-        *
         * @throws \BadMethodCallException if the method was called more than one time
         */
        public function setPublicKey($publicKeyModulus) {
index 955a6f5..4914308 100644 (file)
@@ -72,7 +72,7 @@ class RsaAuthService extends \TYPO3\CMS\Sv\AuthenticationService {
                        // Decrypt the password
                        $password = $loginData['uident'];
                        $key = $storage->get();
-                       if ($key != NULL && substr($password, 0, 4) === 'rsa:') {
+                       if ($key !== NULL && substr($password, 0, 4) === 'rsa:') {
                                // Decode password and store it in loginData
                                $decryptedPassword = $this->backend->decrypt($key, substr($password, 4));
                                if ($decryptedPassword !== NULL) {
@@ -104,7 +104,7 @@ class RsaAuthService extends \TYPO3\CMS\Sv\AuthenticationService {
                if ($available) {
                        // Get the backend
                        $this->backend = \TYPO3\CMS\Rsaauth\Backend\BackendFactory::getBackend();
-                       if (is_null($this->backend)) {
+                       if ($this->backend === NULL) {
                                $available = FALSE;
                        }
                }
index 4ce1a8b..db1190b 100644 (file)
@@ -49,7 +49,7 @@ class StorageFactory {
         * @return \TYPO3\CMS\Rsaauth\Storage\AbstractStorage A storage
         */
        static public function getStorage() {
-               if (is_null(self::$storageInstance)) {
+               if (self::$storageInstance === NULL) {
                        self::$storageInstance = \TYPO3\CMS\Core\Utility\GeneralUtility::getUserObj(self::$preferredStorage);
                }
                return self::$storageInstance;