openrat-cms

OpenRat Content Management System
git clone http://git.code.weiherhei.de/openrat-cms.git
Log | Files | Refs | README

commit dcac02bd62150e805d2847cd7e4310d63f14ab56
parent 11e79df5ffdbbc42bfad0fda3c52c89d43c517ce
Author: Jan Dankert <develop@jandankert.de>
Date:   Mon, 26 Oct 2020 22:21:42 +0100

Refactoring: Using TextMessage for creating Messages with user content.

Diffstat:
Mmodules/cms/Dispatcher.class.php | 10++++++++--
Mmodules/cms/action/LoginAction.class.php | 17++++++++++++++---
Mmodules/cms/auth/RememberAuth.class.php | 7+++++--
Mmodules/cms/generator/PageGenerator.class.php | 3++-
Mmodules/cms/generator/Publisher.class.php | 3++-
Mmodules/cms/ui/UI.class.php | 11++++++-----
Mmodules/language/Language.class.php | 4+++-
Mmodules/logger/Logger.class.php | 28+++++++---------------------
Amodules/util/text/TextMessage.class.php | 41+++++++++++++++++++++++++++++++++++++++++
9 files changed, 88 insertions(+), 36 deletions(-)

diff --git a/modules/cms/Dispatcher.class.php b/modules/cms/Dispatcher.class.php @@ -27,6 +27,7 @@ use util\exception\SecurityException; use util\json\JSON; use util\Session; use util\Text; +use util\text\TextMessage; /** @@ -165,7 +166,13 @@ class Dispatcher if ( Configuration::subset('security')->is('use_post_token',true) && $this->request->isAction && $this->request->getToken() != Session::token() ) { - Logger::error('Token mismatch: Needed ' . Session::token() . ' but got ' . Logger::sanitizeInput($this->request->getToken()) . '. Maybe an attacker?'); + Logger::error( TextMessage::create( + 'Token mismatch: Needed ${expected}), but got ${actual} Maybe an attacker?', + [ + 'expected' => Session::token(), + 'actual' => $this->request->getToken() + ]) + ); throw new SecurityException("Token mismatch"); } } @@ -175,7 +182,6 @@ class Dispatcher */ private function initializeLogger() { - $logConfig = Configuration::subset('log'); $logFile = $logConfig->get('file',''); diff --git a/modules/cms/action/LoginAction.class.php b/modules/cms/action/LoginAction.class.php @@ -22,6 +22,7 @@ use \security\Password; use util\Session; use util\Mail; use util\Text; +use util\text\TextMessage; // OpenRat Content Management System @@ -151,7 +152,12 @@ class LoginAction extends BaseAction } else { - Logger::info( "login failed for user ".Logger::sanitizeInput($user->name)." from IP $ip" ); + Logger::info( TextMessage::create('login failed for user ${name} from IP ${ip}', + [ + 'name' => $user->name, + 'ip' => $ip + ] + ) ); return false; } @@ -806,8 +812,13 @@ class LoginAction extends BaseAction { // Anmeldung nicht erfolgreich - Logger::debug("Login failed for user ".Logger::sanitizeInput($loginName)." from IP $ip"); - + Logger::debug( TextMessage::create('login failed for user ${name} from IP ${ip}', + [ + 'name' => $loginName, + 'ip' => $ip + ] + ) ); + if ( $tokenFailed ) { // Token falsch. diff --git a/modules/cms/auth/RememberAuth.class.php b/modules/cms/auth/RememberAuth.class.php @@ -3,10 +3,13 @@ namespace cms\auth; use cms\auth\Auth; +use cms\base\Configuration; +use cms\model\Text; use database\Database; use cms\model\User; use logger\Logger; use \util\exception\ObjectNotFoundException; +use util\text\TextMessage; /** * Authentifizierung mit einem Login-Token. @@ -27,11 +30,11 @@ class RememberAuth implements Auth list($selector, $token) = array_pad(explode('.', $_COOKIE['or_token']), 2, ''); $dbid = $_COOKIE['or_dbid']; - $dbConfig = \cms\base\Configuration::config()->subset('database'); + $dbConfig = Configuration::config()->subset('database'); if (!$dbConfig->has($dbid)) { - Logger::info('unknown DB-Id for token-login: ' . Logger::sanitizeInput($dbid)); + Logger::info( TextMessage::create('Unknown DB-Id for token-login: ${0}',[$dbid]) ); return null; } diff --git a/modules/cms/generator/PageGenerator.class.php b/modules/cms/generator/PageGenerator.class.php @@ -16,6 +16,7 @@ use cms\model\Value; use logger\Logger; use util\exception\GeneratorException; use util\Mustache; +use util\text\TextMessage; class PageGenerator extends BaseGenerator @@ -147,7 +148,7 @@ class PageGenerator extends BaseGenerator $templateid = array_search($name,$project->getTemplates() ); if ( ! $templateid ) - throw new \InvalidArgumentException('template '.Logger::sanitizeInput($name).' not found'); + throw new \InvalidArgumentException( TextMessage::create('template ${name} not found',['name'=>$name]) ); if ( $templateid == $template->templateid ) throw new \InvalidArgumentException('Template recursion detected on template-id '.$templateid); diff --git a/modules/cms/generator/Publisher.class.php b/modules/cms/generator/Publisher.class.php @@ -14,6 +14,7 @@ use logger\Logger; use cms\generator\target\TargetFactory; use util\exception\PublisherException; use util\Session; +use util\text\TextMessage; use util\text\variables\VariableResolver; /** @@ -136,7 +137,7 @@ class Publisher if ( $systemCommand ) { $ausgabe = array(); $rc = false; - Logger::debug('Executing system command: ' . Logger::sanitizeInput($systemCommand)); + Logger::debug( TextMessage::create('Executing system command: ${0}',[$systemCommand]) ); /** @var ModelBase $baseObjectToEnv */ foreach (['user' => Session::getUser(), diff --git a/modules/cms/ui/UI.class.php b/modules/cms/ui/UI.class.php @@ -15,6 +15,7 @@ use \util\exception\ObjectNotFoundException; use util\exception\UIException; use util\exception\SecurityException; use template_engine\engine\TemplateEngine; +use util\text\TextMessage; /** @@ -57,22 +58,22 @@ class UI } catch (BadMethodCallException $e) { // Action-Method does not exist. - Logger::debug( 'Method does not exist',$request->action,$request->method,$e->__toString() ); + Logger::debug( $e ); Http::noContent(); } catch (ObjectNotFoundException $e) { - Logger::debug("Object not found: " . $e->__toString()); // Nur Debug, da dies bei gelöschten Objekten vorkommen kann. + Logger::debug( $e ); // only debug, because this may happen on links to deleted objects. Http::noContent(); } catch (UIException $e) { - Logger::warn( $e->__toString() ); + Logger::warn( $e ); throw new LogicException(L::lang($e->key),0, $e); } catch (SecurityException $e) { - Logger::info($e->getMessage()); + Logger::info($e); Http::noContent(); // this is not good at all, because the user may have signed off. //Http::notAuthorized("You are not allowed to execute this action."); } catch (Exception $e) { - Logger::warn( $e->__toString() ); + Logger::warn( $e ); throw new LogicException("Internal CMS error",0, $e); } } diff --git a/modules/language/Language.class.php b/modules/language/Language.class.php @@ -2,7 +2,9 @@ namespace language; +use DomainException; use logger\Logger; +use util\text\TextMessage; class Language { @@ -40,7 +42,7 @@ class Language return new $languageClazz(); } - throw new \DomainException('No language file found for iso keys: '.Logger::sanitizeInput(implode(',',$isos))); + throw new DomainException( TextMessage::create('No language class found for iso keys ${0}',[implode(',',$isos)]) ); } } \ No newline at end of file diff --git a/modules/logger/Logger.class.php b/modules/logger/Logger.class.php @@ -43,7 +43,7 @@ class Logger /** * Writes a trace message to log * - * @param string message text + * @param string|Exception message text */ public static function trace($message) { @@ -62,7 +62,7 @@ class Logger /** * Writes a debug message to log * - * @param string message text + * @param string|Exception message text */ public static function debug($message) { @@ -73,7 +73,7 @@ class Logger /** * Writes a information message to log * - * @param string message text + * @param string|Exception message text */ public static function info($message) { @@ -84,7 +84,7 @@ class Logger /** * Writes a warning message to log * - * @param string message text + * @param string|Exception message text */ public static function warn($message) { @@ -95,7 +95,7 @@ class Logger /** * Writes an error message to log * - * @param string message text + * @param string|Exception message text */ public static function error($message) { @@ -107,7 +107,7 @@ class Logger * Writes a mesage into the log file * * @param string facility of log entry - * @param string message text + * @param string|Throwable message text */ private static function doLog($facility, $message) { @@ -128,7 +128,7 @@ class Logger $levelName = ''; if ($message instanceof Exception) - $message = $message->getTraceAsString(); + $message = $message->getMessage()."\n".$message->__toString(); $values = array_map( function($key) use ($message, $levelName) { switch( $key ) { @@ -198,18 +198,4 @@ class Logger error_log($text . "\n"); } - - /** - * Sanitize user input. - * Cutting out unsafe characters. - * - * @param $input string potentially dangerous user input - * @return string a safe representaton of the user input. - */ - public static function sanitizeInput( $input ) { - $length = strlen($input); - $white = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789.,_-'; - $clean = Text::clean($input,$white); - return '"'.$input.'"/'.$length.'/'.strlen($clean); - } } \ No newline at end of file diff --git a/modules/util/text/TextMessage.class.php b/modules/util/text/TextMessage.class.php @@ -0,0 +1,40 @@ +<?php + + +namespace util\text; + + +use util\Text; +use util\text\variables\VariableResolver; + +class TextMessage +{ + /** + * Creates a text message with variables. + * + * @param $text + * @param array $params + * @return string + */ + public static function create( $text, $params=[] ) { + if ( $params) { + $resolver = new VariableResolver(); + $resolver->addDefaultResolver( function($key) use ($params) { + return TextMessage::sanitizeInput( @$params[$key] ); + }); + return $resolver->resolveVariables($text); + } + else { + // no params, so no resolver needed + return $text; + } + } + + + public static function sanitizeInput( $input ) { + $white = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789.,_-'; + $clean = Text::clean($input,$white); + return "'".$input."'".(strlen($input)>strlen($clean)?'(!)':''); + } + +}+ \ No newline at end of file