From 0adeadd3f469680a8bcb4803ff9b4ab278e3378e Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Thu, 10 Mar 2022 23:32:50 +0100 Subject: [PATCH 1/5] PoC Config objects --- config-templates/module_logpeek.yml | 4 + lib/Config/Logpeek.php | 116 ++++++++++++++++++++++++++++ lib/Controller/Logpeek.php | 16 ++-- 3 files changed, 128 insertions(+), 8 deletions(-) create mode 100644 config-templates/module_logpeek.yml create mode 100644 lib/Config/Logpeek.php diff --git a/config-templates/module_logpeek.yml b/config-templates/module_logpeek.yml new file mode 100644 index 0000000..c987e70 --- /dev/null +++ b/config-templates/module_logpeek.yml @@ -0,0 +1,4 @@ +--- +logFile: /var/log/simplesamlphp/idp.moo-archive.nl.log +lines: 1500 +blockSize: 8192 diff --git a/lib/Config/Logpeek.php b/lib/Config/Logpeek.php new file mode 100644 index 0000000..ef03b52 --- /dev/null +++ b/lib/Config/Logpeek.php @@ -0,0 +1,116 @@ +getConfigDir(); + $yamlConfig = Yaml::parse(file_get_contents($configDir . '/' . $defaultConfigFile)) ?? []; + if (isset($yamlConfig['logFile'])) { + $this->setLogfile($yamlConfig['logFile']); + } else { + $config = Configuration::getInstance(); + $loggingDir = $config->getPathValue('loggingdir', 'log/'); + $this->setLogfile($loggingDir . $config->getString('logging.logfile', 'simplesamlphp.log')); + } + + if (isset($yamlConfig['lines'])) { + $this->setLines($yamlConfig['lines']); + } + + if (isset($yamlConfig['blockSize'])) { + $this->setBlockSize($yamlConfig['blockSize']); + } + } + + + /** + * @return string + */ + public function getLogFile(): string { + return $this->logFile; + } + + + /** + * @param string $logFile + * @return void + */ + protected function setLogFile(string $logFile): void { + $this->logFile = $logFile; + } + + + /** + * @return int + */ + public function getLines(): int { + return $this->lines; + } + + + /** + * @param int $lines + * @return void + */ + protected function setLines(int $lines): void { + Assert::positiveInteger($lines); + $this->lines = $lines; + } + + + /** + * @return int + */ + public function getBlockSize(): int { + return $this->blockSize; + } + + + /** + * @param int $blockSize + * @return void + */ + protected function setBlockSize(int $blockSize): void { + Assert::range($blockSize, 0, self::MAX_BLOCKSIZE); + $this->blockSize = $blockSize; + } +}; diff --git a/lib/Controller/Logpeek.php b/lib/Controller/Logpeek.php index 1310c0c..1e82182 100644 --- a/lib/Controller/Logpeek.php +++ b/lib/Controller/Logpeek.php @@ -7,6 +7,7 @@ use Exception; use SimpleSAML\Assert\Assert; use SimpleSAML\Configuration; +use SimpleSAML\Module\logpeek\Config; use SimpleSAML\Module\logpeek\File; use SimpleSAML\Module\logpeek\Syslog; use SimpleSAML\Session; @@ -33,8 +34,8 @@ class Logpeek /** @var \SimpleSAML\Configuration */ protected Configuration $config; - /** @var \SimpleSAML\Configuration */ - protected Configuration $moduleConfig; + /** @var \SimpleSAML\Module\logpeek\Config\Logpeek */ + protected Config\Logpeek $moduleConfig; /** @var \SimpleSAML\Session */ protected Session $session; @@ -59,7 +60,7 @@ public function __construct( ) { $this->authUtils = new Utils\Auth(); $this->config = $config; - $this->moduleConfig = Configuration::getConfig('module_logpeek.php'); + $this->moduleConfig = new Config\Logpeek('module_logpeek.yml'); $this->session = $session; } @@ -86,10 +87,9 @@ public function main(Request $request): Template { $this->authUtils->requireAdmin(); - $logfile = $this->moduleConfig->getValue('logfile', '/var/log/simplesamlphp.log'); - $blockSize = $this->moduleConfig->getValue('blocksz', 8192); - - $myLog = new File\ReverseRead($logfile, $blockSize); + $logFile = $this->moduleConfig->getLogFile(); + $blockSize = $this->moduleConfig->getBlockSize(); + $myLog = new File\ReverseRead($logFile, $blockSize); $results = []; if ($request->query->has('tag') === true) { @@ -97,7 +97,7 @@ public function main(Request $request): Template $tag = $request->query->get('tag'); Assert::notNull($tag); - $results = $this->logFilter($myLog, $tag, $this->moduleConfig->getValue('lines', 500)); + $results = $this->logFilter($myLog, $tag, $this->moduleConfig->getLines()); } $fileModYear = intval(date("Y", $myLog->getFileMtime())); From 1eac7e63b5b671735f949993ac4524638673d3ae Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Tue, 15 Mar 2022 13:00:29 +0100 Subject: [PATCH 2/5] Some improvements --- config-templates/module_logpeek.yml | 2 +- lib/Config/Logpeek.php | 90 ++++++++++++++++++++--------- 2 files changed, 65 insertions(+), 27 deletions(-) diff --git a/config-templates/module_logpeek.yml b/config-templates/module_logpeek.yml index c987e70..32c8985 100644 --- a/config-templates/module_logpeek.yml +++ b/config-templates/module_logpeek.yml @@ -1,4 +1,4 @@ --- -logFile: /var/log/simplesamlphp/idp.moo-archive.nl.log +logFile: /var/log/simplesamlphp.log lines: 1500 blockSize: 8192 diff --git a/lib/Config/Logpeek.php b/lib/Config/Logpeek.php index ef03b52..ddd6abc 100644 --- a/lib/Config/Logpeek.php +++ b/lib/Config/Logpeek.php @@ -4,7 +4,6 @@ namespace SimpleSAML\Module\logpeek\Config; -use E_USER_NOTICE; use SimpleSAML\Assert\Assert; use SimpleSAML\Configuration; use SimpleSAML\Utils; @@ -16,6 +15,9 @@ */ class Logpeek { + /** @var string */ + public const DEFAULT_CONFIGFILE = 'module_logpeek.yml'; + /** @var int */ public const MAX_BLOCKSIZE = 8192; @@ -29,43 +31,36 @@ class Logpeek { private string $logFile; /** @var int */ - private int $lines = self::DEFAULT_LINES; + private int $lines; /** @var int */ - private int $blockSize = self::DEFAULT_BLOCKSIZE; + private int $blockSize; /** - * @param string $defaultConfigFile - * @throws \Symfony\Component\Yaml\Exception\ParseException + * @param string|null $logFile + * @param int $blockSize + * @param int $lines */ - public function __construct(string $defaultConfigFile = 'module_logpeek.yml') + public function __construct(?string $logFile, ?int $blockSize = null, ?int $lines = null) { - $configUtils = new Utils\Config(); - $configDir = $configUtils->getConfigDir(); - $yamlConfig = Yaml::parse(file_get_contents($configDir . '/' . $defaultConfigFile)) ?? []; - if (isset($yamlConfig['logFile'])) { - $this->setLogfile($yamlConfig['logFile']); - } else { + if ($logFile === null) { $config = Configuration::getInstance(); $loggingDir = $config->getPathValue('loggingdir', 'log/'); - $this->setLogfile($loggingDir . $config->getString('logging.logfile', 'simplesamlphp.log')); - } - - if (isset($yamlConfig['lines'])) { - $this->setLines($yamlConfig['lines']); + $logFile = $loggingDir . $config->getString('logging.logfile', 'simplesamlphp.log'); } - if (isset($yamlConfig['blockSize'])) { - $this->setBlockSize($yamlConfig['blockSize']); - } + $this->setLogFile($logFile); + $this->setBlockSize($blockSize ?? self::DEFAULT_BLOCKSIZE); + $this->setLines($lines ?? self::DEFAULT_LINES); } /** * @return string */ - public function getLogFile(): string { + public function getLogFile(): string + { return $this->logFile; } @@ -74,7 +69,8 @@ public function getLogFile(): string { * @param string $logFile * @return void */ - protected function setLogFile(string $logFile): void { + protected function setLogFile(string $logFile): void + { $this->logFile = $logFile; } @@ -82,7 +78,8 @@ protected function setLogFile(string $logFile): void { /** * @return int */ - public function getLines(): int { + public function getLines(): int + { return $this->lines; } @@ -91,7 +88,8 @@ public function getLines(): int { * @param int $lines * @return void */ - protected function setLines(int $lines): void { + protected function setLines(int $lines): void + { Assert::positiveInteger($lines); $this->lines = $lines; } @@ -100,7 +98,8 @@ protected function setLines(int $lines): void { /** * @return int */ - public function getBlockSize(): int { + public function getBlockSize(): int + { return $this->blockSize; } @@ -109,8 +108,47 @@ public function getBlockSize(): int { * @param int $blockSize * @return void */ - protected function setBlockSize(int $blockSize): void { + protected function setBlockSize(int $blockSize): void + { Assert::range($blockSize, 0, self::MAX_BLOCKSIZE); $this->blockSize = $blockSize; } + + + /** + * @param string $file + * @return self + */ + public static function fromPhp(string $configFile = 'module_logpeek.php'): self + { + $configUtils = new Utils\Config(); + $configDir = $configUtils->getConfigDir(); + include($configDir . '/' . $configFile); + + return static::fromArray($config ?? []); + } + + + /** + * @param string $file + * @return self + */ + public static function fromYaml(string $configFile = 'module_logpeek.yml'): self + { + $configUtils = new Utils\Config(); + $configDir = $configUtils->getConfigDir(); + $yamlConfig = Yaml::parse(file_get_contents($configDir . '/' . $configFile)) ?? []; + + return static::fromArray($yamlConfig); + } + + + /** + * @param array $config + * @return self + */ + public static function fromArray(array $config): self + { + return new self($config['logFile'], $config['blockSize'], $config['lines']); + } }; From acc468c5efb1ed304dbc188259c59315b6c527c4 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Tue, 15 Mar 2022 17:54:57 +0100 Subject: [PATCH 3/5] Fix unit tests --- lib/Controller/Logpeek.php | 6 ++++-- tests/lib/Controller/LogpeekTest.php | 32 +++++++++++++--------------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/lib/Controller/Logpeek.php b/lib/Controller/Logpeek.php index 1e82182..8c218b5 100644 --- a/lib/Controller/Logpeek.php +++ b/lib/Controller/Logpeek.php @@ -51,16 +51,18 @@ class Logpeek * * @param \SimpleSAML\Configuration $config The configuration to use by the controllers. * @param \SimpleSAML\Session $session The session to use by the controllers. + * @param \SimpleSAML\Module\logpeek\Config\Logpeek $moduleConfig The configuration for this module * * @throws \Exception */ public function __construct( Configuration $config, - Session $session + Session $session, + Config\Logpeek $moduleConfig ) { $this->authUtils = new Utils\Auth(); $this->config = $config; - $this->moduleConfig = new Config\Logpeek('module_logpeek.yml'); + $this->moduleConfig = $moduleConfig; $this->session = $session; } diff --git a/tests/lib/Controller/LogpeekTest.php b/tests/lib/Controller/LogpeekTest.php index b52c4d1..f082d82 100644 --- a/tests/lib/Controller/LogpeekTest.php +++ b/tests/lib/Controller/LogpeekTest.php @@ -8,6 +8,7 @@ use PHPUnit\Framework\TestCase; use SimpleSAML\Configuration; use SimpleSAML\Logger; +use SimpleSAML\Module\logpeek\Config; use SimpleSAML\Module\logpeek\Controller; use SimpleSAML\Session; use SimpleSAML\Utils; @@ -29,6 +30,9 @@ class LogpeekTest extends TestCase /** @var \SimpleSAML\Configuration */ protected Configuration $config; + /** @var \SimpleSAML\Module\logpeek\Config\Logpeek */ + protected Config\Logpeek $moduleConfig; + /** @var \SimpleSAML\Session */ protected Session $session; @@ -77,20 +81,14 @@ public function requireAdmin(): void } }; - Configuration::setPreLoadedConfig( - Configuration::loadFromArray( - [ - 'logfile' => $this->tmpfile, - 'lines' => 1500, - - // Read block size. 8192 is max, limited by fread. - 'blocksz' => 8192, - ], - '[ARRAY]', - 'simplesaml' - ), - 'module_logpeek.php', - 'simplesaml' + $this->moduleConfig = Config\Logpeek::fromArray( + [ + 'logFile' => $this->tmpfile, + 'lines' => 1500, + + // Read block size. 8192 is max, limited by fread. + 'blockSize' => 8192, + ] ); } @@ -115,7 +113,7 @@ public function testMain(): void 'GET' ); - $c = new Controller\Logpeek($this->config, $this->session); + $c = new Controller\Logpeek($this->config, $this->session, $this->moduleConfig); $c->setAuthUtils($this->authUtils); $response = $c->main($request); @@ -133,7 +131,7 @@ public function testMainWithTag(): void ['tag' => $this->session->getTrackID()] ); - $c = new Controller\Logpeek($this->config, $this->session); + $c = new Controller\Logpeek($this->config, $this->session, $this->moduleConfig); $c->setAuthUtils($this->authUtils); $response = $c->main($request); @@ -151,7 +149,7 @@ public function testMainWithInvalidTag(): void ['tag' => 'WRONG'] ); - $c = new Controller\Logpeek($this->config, $this->session); + $c = new Controller\Logpeek($this->config, $this->session, $this->moduleConfig); $c->setAuthUtils($this->authUtils); $this->expectException(Exception::class); From c75c8d04b82639ab31e5432e597c15caf899926b Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Tue, 15 Mar 2022 17:56:58 +0100 Subject: [PATCH 4/5] Consistency fix --- lib/Config/Logpeek.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Config/Logpeek.php b/lib/Config/Logpeek.php index ddd6abc..e917d60 100644 --- a/lib/Config/Logpeek.php +++ b/lib/Config/Logpeek.php @@ -137,9 +137,9 @@ public static function fromYaml(string $configFile = 'module_logpeek.yml'): self { $configUtils = new Utils\Config(); $configDir = $configUtils->getConfigDir(); - $yamlConfig = Yaml::parse(file_get_contents($configDir . '/' . $configFile)) ?? []; + $yamlConfig = Yaml::parse(file_get_contents($configDir . '/' . $configFile)); - return static::fromArray($yamlConfig); + return static::fromArray($yamlConfig ?? []); } From 925df5738ab4b808dabdc95bd2a75f0c0dab856b Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Tue, 15 Mar 2022 18:10:01 +0100 Subject: [PATCH 5/5] Fix --- lib/Config/Logpeek.php | 2 +- lib/Controller/Logpeek.php | 17 +++++++++++++---- tests/lib/Controller/LogpeekTest.php | 3 +++ 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/lib/Config/Logpeek.php b/lib/Config/Logpeek.php index e917d60..d6cd323 100644 --- a/lib/Config/Logpeek.php +++ b/lib/Config/Logpeek.php @@ -133,7 +133,7 @@ public static function fromPhp(string $configFile = 'module_logpeek.php'): self * @param string $file * @return self */ - public static function fromYaml(string $configFile = 'module_logpeek.yml'): self + public static function fromYaml(string $configFile = self::DEFAULT_CONFIGFILE): self { $configUtils = new Utils\Config(); $configDir = $configUtils->getConfigDir(); diff --git a/lib/Controller/Logpeek.php b/lib/Controller/Logpeek.php index 8c218b5..7a1c77e 100644 --- a/lib/Controller/Logpeek.php +++ b/lib/Controller/Logpeek.php @@ -51,18 +51,16 @@ class Logpeek * * @param \SimpleSAML\Configuration $config The configuration to use by the controllers. * @param \SimpleSAML\Session $session The session to use by the controllers. - * @param \SimpleSAML\Module\logpeek\Config\Logpeek $moduleConfig The configuration for this module * * @throws \Exception */ public function __construct( Configuration $config, - Session $session, - Config\Logpeek $moduleConfig + Session $session ) { $this->authUtils = new Utils\Auth(); $this->config = $config; - $this->moduleConfig = $moduleConfig; + $this->moduleConfig = Config\Logpeek::fromYaml(); $this->session = $session; } @@ -78,6 +76,17 @@ public function setAuthUtils(Utils\Auth $authUtils): void } + /** + * Inject the \SimpleSAML\Module\logpeek\Config\Logpeek dependency. + * + * @param \SimpleSAML\Module\logpeek\Config\Logpeek $moduleConfig + */ + public function setModuleConfig(Config\Logpeek $moduleConfig): void + { + $this->moduleConfig = $moduleConfig; + } + + /** * Main index controller. * diff --git a/tests/lib/Controller/LogpeekTest.php b/tests/lib/Controller/LogpeekTest.php index f082d82..12013b0 100644 --- a/tests/lib/Controller/LogpeekTest.php +++ b/tests/lib/Controller/LogpeekTest.php @@ -115,6 +115,7 @@ public function testMain(): void $c = new Controller\Logpeek($this->config, $this->session, $this->moduleConfig); $c->setAuthUtils($this->authUtils); + $c->setModuleConfig($this->moduleConfig); $response = $c->main($request); $this->assertTrue($response->isSuccessful()); @@ -133,6 +134,7 @@ public function testMainWithTag(): void $c = new Controller\Logpeek($this->config, $this->session, $this->moduleConfig); $c->setAuthUtils($this->authUtils); + $c->setModuleConfig($this->moduleConfig); $response = $c->main($request); $this->assertTrue($response->isSuccessful()); @@ -151,6 +153,7 @@ public function testMainWithInvalidTag(): void $c = new Controller\Logpeek($this->config, $this->session, $this->moduleConfig); $c->setAuthUtils($this->authUtils); + $c->setModuleConfig($this->moduleConfig); $this->expectException(Exception::class); $c->main($request);