From 2d91732b4815bd1f330101ed0973685d57ec0ad4 Mon Sep 17 00:00:00 2001
From: Tim van Dijen <tvdijen@gmail.com>
Date: Tue, 31 May 2022 07:39:32 +0200
Subject: [PATCH] Fix several issues reported by scrutinizer
---
modules/core/src/Controller/ErrorReport.php | 10 ++++++++--
modules/cron/src/Controller/Cron.php | 6 ++----
.../saml/src/Controller/WebBrowserSingleSignOn.php | 1 -
src/SimpleSAML/Error/Error.php | 1 -
.../modules/core/src/Controller/ErrorReportTest.php | 12 +++++++++---
5 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/modules/core/src/Controller/ErrorReport.php b/modules/core/src/Controller/ErrorReport.php
index 1afb833c5..cd9e00e21 100644
--- a/modules/core/src/Controller/ErrorReport.php
+++ b/modules/core/src/Controller/ErrorReport.php
@@ -9,6 +9,7 @@ use SimpleSAML\Configuration;
use SimpleSAML\Error;
use SimpleSAML\HTTP\RunnableResponse;
use SimpleSAML\Logger;
+use SimpleSAML\Session;
use SimpleSAML\Utils;
use SimpleSAML\XHTML\Template;
use Symfony\Component\HttpFoundation\Request;
@@ -30,6 +31,9 @@ class ErrorReport
/** @var \SimpleSAML\Configuration */
protected Configuration $config;
+ /** @var \SimpleSAML\Session */
+ protected Configuration $session;
+
/**
* Controller constructor.
@@ -37,11 +41,13 @@ class ErrorReport
* It initializes the global configuration for the controllers implemented here.
*
* @param \SimpleSAML\Configuration $config The configuration to use by the controllers.
+ * @param \SimpleSAML\Session $config The session to use by the controllers.
*/
public function __construct(
Configuration $config
) {
$this->config = $config;
+ $this->session = $session;
}
@@ -66,10 +72,10 @@ class ErrorReport
throw new Error\Exception('Invalid reportID');
}
- $data = null;
try {
$data = $this->session->getData('core:errorreport', $reportId);
} catch (Exception $e) {
+ $data = null;
Logger::error('Error loading error report data: ' . var_export($e->getMessage(), true));
}
@@ -82,7 +88,7 @@ class ErrorReport
'referer' => 'not set',
];
- if (isset($session)) {
+ if (isset($this->session)) {
$data['trackId'] = $session->getTrackID();
}
}
diff --git a/modules/cron/src/Controller/Cron.php b/modules/cron/src/Controller/Cron.php
index 80346cd12..5772d4e7b 100644
--- a/modules/cron/src/Controller/Cron.php
+++ b/modules/cron/src/Controller/Cron.php
@@ -129,14 +129,12 @@ class Cron
{
$configKey = $this->cronconfig->getOptionalString('key', 'secret');
if ($key !== $configKey) {
- Logger::error('Cron - Wrong key provided. Cron will not run.');
- exit;
+ throw new Error\Exception('Cron - Wrong key provided. Cron will not run.');
}
$cron = new \SimpleSAML\Module\cron\Cron();
if (!$cron->isValidTag($tag)) {
- Logger::error('Cron - Illegal tag [' . $tag . '].');
- exit;
+ throw new Error\Exception(sprintf('Cron - Illegal tag [%s].', $tag));
}
$httpUtils = new Utils\HTTP();
diff --git a/modules/saml/src/Controller/WebBrowserSingleSignOn.php b/modules/saml/src/Controller/WebBrowserSingleSignOn.php
index 7a996cd4e..70c447b40 100644
--- a/modules/saml/src/Controller/WebBrowserSingleSignOn.php
+++ b/modules/saml/src/Controller/WebBrowserSingleSignOn.php
@@ -137,6 +137,5 @@ class WebBrowserSingleSignOn
} catch (UnsupportedBindingException $e) {
throw new Error\Error('SSOPARAMS', $e, 400);
}
- Assert::true(false);
}
}
diff --git a/src/SimpleSAML/Error/Error.php b/src/SimpleSAML/Error/Error.php
index 3e0791b28..968f1ebdd 100644
--- a/src/SimpleSAML/Error/Error.php
+++ b/src/SimpleSAML/Error/Error.php
@@ -247,7 +247,6 @@ class Error extends Exception
) {
// enable error reporting
$httpUtils = new Utils\HTTP();
- $baseurl = $httpUtils->getBaseURL();
$data['errorReportAddress'] = Module::getModuleURL('core/errorReport');
}
diff --git a/tests/modules/core/src/Controller/ErrorReportTest.php b/tests/modules/core/src/Controller/ErrorReportTest.php
index 9c893f0b8..66367f994 100644
--- a/tests/modules/core/src/Controller/ErrorReportTest.php
+++ b/tests/modules/core/src/Controller/ErrorReportTest.php
@@ -9,6 +9,7 @@ use SimpleSAML\Configuration;
use SimpleSAML\Error;
use SimpleSAML\HTTP\RunnableResponse;
use SimpleSAML\Module\core\Controller;
+use SimpleSAML\Session;
use SimpleSAML\XHTML\Template;
use Symfony\Component\HttpFoundation\Request;
@@ -23,6 +24,9 @@ class ErrorReportTest extends TestCase
/** @var \SimpleSAML\Configuration */
protected Configuration $config;
+ /** @var \SimpleSAML\Session */
+ protected Session $session;
+
/**
* Set up for each test.
@@ -41,6 +45,8 @@ class ErrorReportTest extends TestCase
);
Configuration::setPreLoadedConfig($this->config, 'config.php');
+
+ $this->session = Session::getSessionFromRequest();
}
@@ -54,7 +60,7 @@ class ErrorReportTest extends TestCase
'GET',
);
- $c = new Controller\ErrorReport($this->config);
+ $c = new Controller\ErrorReport($this->config, $this->session);
$response = $c->main($request);
@@ -74,7 +80,7 @@ class ErrorReportTest extends TestCase
['reportId' => 'abc123'],
);
- $c = new Controller\ErrorReport($this->config);
+ $c = new Controller\ErrorReport($this->config, $this->session);
$this->expectException(Error\Exception::class);
$this->expectExceptionMessage('Invalid reportID');
@@ -98,7 +104,7 @@ class ErrorReportTest extends TestCase
],
);
- $c = new Controller\ErrorReport($this->config);
+ $c = new Controller\ErrorReport($this->config, $this->session);
$response = $c->main($request);
--
GitLab