From 6e5f0651f807620d65e8207582698d6fb9792a74 Mon Sep 17 00:00:00 2001 From: Wouter Wolters <typo3@wouterwolters.nl> Date: Thu, 26 Feb 2015 23:50:46 +0100 Subject: [PATCH] [TASK] General code cleanup in ext:rsaauth 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> --- .../Classes/Backend/AbstractBackend.php | 2 +- .../Classes/Backend/BackendFactory.php | 16 ++++++------- .../Classes/Backend/CommandLineBackend.php | 24 ++++++++++++------- .../rsaauth/Classes/Backend/PhpBackend.php | 7 +++--- .../rsaauth/Classes/BackendWarnings.php | 22 ++++++++++++----- .../Classes/Hook/BackendHookForAjaxLogin.php | 1 + .../rsaauth/Classes/Hook/LoginFormHook.php | 3 +-- typo3/sysext/rsaauth/Classes/Keypair.php | 6 ----- .../sysext/rsaauth/Classes/RsaAuthService.php | 4 ++-- .../Classes/Storage/StorageFactory.php | 2 +- 10 files changed, 48 insertions(+), 39 deletions(-) diff --git a/typo3/sysext/rsaauth/Classes/Backend/AbstractBackend.php b/typo3/sysext/rsaauth/Classes/Backend/AbstractBackend.php index 780221899c9f..e5ad9c8471aa 100644 --- a/typo3/sysext/rsaauth/Classes/Backend/AbstractBackend.php +++ b/typo3/sysext/rsaauth/Classes/Backend/AbstractBackend.php @@ -65,7 +65,7 @@ abstract class AbstractBackend { /** * Checks if this backend is available for calling. * - * @return void + * @return bool */ abstract public function isAvailable(); diff --git a/typo3/sysext/rsaauth/Classes/Backend/BackendFactory.php b/typo3/sysext/rsaauth/Classes/Backend/BackendFactory.php index 262fb70bbd15..af382077e6f1 100644 --- a/typo3/sysext/rsaauth/Classes/Backend/BackendFactory.php +++ b/typo3/sysext/rsaauth/Classes/Backend/BackendFactory.php @@ -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; diff --git a/typo3/sysext/rsaauth/Classes/Backend/CommandLineBackend.php b/typo3/sysext/rsaauth/Classes/Backend/CommandLineBackend.php index ddfd5f65754a..d2a942de13a6 100644 --- a/typo3/sysext/rsaauth/Classes/Backend/CommandLineBackend.php +++ b/typo3/sysext/rsaauth/Classes/Backend/CommandLineBackend.php @@ -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; } diff --git a/typo3/sysext/rsaauth/Classes/Backend/PhpBackend.php b/typo3/sysext/rsaauth/Classes/Backend/PhpBackend.php index 9503eef5e80e..c388e0bcdbad 100644 --- a/typo3/sysext/rsaauth/Classes/Backend/PhpBackend.php +++ b/typo3/sysext/rsaauth/Classes/Backend/PhpBackend.php @@ -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))); } } diff --git a/typo3/sysext/rsaauth/Classes/BackendWarnings.php b/typo3/sysext/rsaauth/Classes/BackendWarnings.php index 4f21e22ac311..6185cc9f5f94 100644 --- a/typo3/sysext/rsaauth/Classes/BackendWarnings.php +++ b/typo3/sysext/rsaauth/Classes/BackendWarnings.php @@ -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']; + } + } diff --git a/typo3/sysext/rsaauth/Classes/Hook/BackendHookForAjaxLogin.php b/typo3/sysext/rsaauth/Classes/Hook/BackendHookForAjaxLogin.php index 636beb11a4fd..60f4b46a5ca2 100644 --- a/typo3/sysext/rsaauth/Classes/Hook/BackendHookForAjaxLogin.php +++ b/typo3/sysext/rsaauth/Classes/Hook/BackendHookForAjaxLogin.php @@ -18,6 +18,7 @@ namespace TYPO3\CMS\Rsaauth\Hook; * This class adds RSA JavaScript to the backend */ class BackendHookForAjaxLogin { + /** * Adds RSA-specific JavaScript * diff --git a/typo3/sysext/rsaauth/Classes/Hook/LoginFormHook.php b/typo3/sysext/rsaauth/Classes/Hook/LoginFormHook.php index e54c5e5e2251..b062bf3637e7 100644 --- a/typo3/sysext/rsaauth/Classes/Hook/LoginFormHook.php +++ b/typo3/sysext/rsaauth/Classes/Hook/LoginFormHook.php @@ -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; diff --git a/typo3/sysext/rsaauth/Classes/Keypair.php b/typo3/sysext/rsaauth/Classes/Keypair.php index d76465702427..026a36e8de8f 100644 --- a/typo3/sysext/rsaauth/Classes/Keypair.php +++ b/typo3/sysext/rsaauth/Classes/Keypair.php @@ -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) { diff --git a/typo3/sysext/rsaauth/Classes/RsaAuthService.php b/typo3/sysext/rsaauth/Classes/RsaAuthService.php index 955a6f5cc323..491430849959 100644 --- a/typo3/sysext/rsaauth/Classes/RsaAuthService.php +++ b/typo3/sysext/rsaauth/Classes/RsaAuthService.php @@ -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; } } diff --git a/typo3/sysext/rsaauth/Classes/Storage/StorageFactory.php b/typo3/sysext/rsaauth/Classes/Storage/StorageFactory.php index 4ce1a8bf4bfb..db1190b9eb27 100644 --- a/typo3/sysext/rsaauth/Classes/Storage/StorageFactory.php +++ b/typo3/sysext/rsaauth/Classes/Storage/StorageFactory.php @@ -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; -- GitLab