Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: PSR-3 compatible Logger #1438

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
"composer/composer": "^2.3",
"gettext/gettext": "^5.6.1",
"gettext/translator": "^1.0.1",
"monolog/monolog": "^2.8",
"phpmailer/phpmailer": "^6.5",
"simplesamlphp/assert": "^0.8.0",
"simplesamlphp/saml2": "^4.6",
Expand Down
281 changes: 192 additions & 89 deletions composer.lock

Large diffs are not rendered by default.

12 changes: 6 additions & 6 deletions config/config.php.dist
Original file line number Diff line number Diff line change
Expand Up @@ -334,11 +334,11 @@ $config = [

/*
* Define the minimum log level to log. Available levels:
* - SimpleSAML\Logger::ERR No statistics, only errors
* - SimpleSAML\Logger::WARNING No statistics, only warnings/errors
* - SimpleSAML\Logger::NOTICE Statistics and errors
* - SimpleSAML\Logger::INFO Verbose logs
* - SimpleSAML\Logger::DEBUG Full debug logs - not recommended for production
* - \Psr\Log\LogLevel::ERROR No statistics, only errors
* - \Psr\Log\LogLevel::WARNING No statistics, only warnings/errors
* - \Psr\Log\LogLevel::NOTICE Statistics and errors
* - \Psr\Log\LogLevel::INFO Verbose logs
* - \Psr\Log\LogLevel::DEBUG Full debug logs - not recommended for production
*
* Choose logging handler.
*
Expand All @@ -348,7 +348,7 @@ $config = [
* must exist and be writable for SimpleSAMLphp. If set to something else, set
* loggingdir above to 'null'.
*/
'logging.level' => SimpleSAML\Logger::NOTICE,
'logging.level' => \Psr\Log\LogLevel::NOTICE,
'logging.handler' => 'syslog',

/*
Expand Down
13 changes: 9 additions & 4 deletions modules/admin/src/Controller/Federation.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@
namespace SimpleSAML\Module\admin\Controller;

use Exception;
use Psr\Log\LoggerAwareInterface;
use SAML2\Constants as C;
use SimpleSAML\Assert\Assert;
use SimpleSAML\Auth;
use SimpleSAML\Configuration;
use SimpleSAML\Locale\Translate;
use SimpleSAML\Logger;
use SimpleSAML\Logger\LoggerAwareTrait;
use SimpleSAML\Metadata\MetaDataStorageHandler;
use SimpleSAML\Metadata\SAMLBuilder;
use SimpleSAML\Metadata\SAMLParser;
Expand All @@ -32,11 +33,14 @@
*
* @package SimpleSAML\Module\admin
*/
class Federation
class Federation implements LoggerAwareInterface
{
use LoggerAwareTrait;

/** @var \SimpleSAML\Configuration */
protected Configuration $config;


/**
* @var \SimpleSAML\Auth\Source|string
* @psalm-var \SimpleSAML\Auth\Source|class-string
Expand Down Expand Up @@ -64,6 +68,7 @@ class Federation
public function __construct(Configuration $config)
{
$this->config = $config;
$this->logger = $this->getLogger();
$this->menu = new Menu();
$this->mdHandler = MetaDataStorageHandler::getMetadataHandler();
$this->authUtils = new Utils\Auth();
Expand Down Expand Up @@ -233,7 +238,7 @@ private function getHostedIdP(): array
$entities[$index] = $entity;
}
} catch (\Exception $e) {
Logger::error('Federation: Error loading saml20-idp: ' . $e->getMessage());
$this->logger->error('Federation: Error loading saml20-idp: ' . $e->getMessage());
}
}

Expand Down Expand Up @@ -284,7 +289,7 @@ private function getHostedIdP(): array
$entities[$index] = $entity;
}
} catch (\Exception $e) {
Logger::error('Federation: Error loading adfs-idp: ' . $e->getMessage());
$this->logger->error('Federation: Error loading adfs-idp: ' . $e->getMessage());
}
}

Expand Down
3 changes: 1 addition & 2 deletions modules/core/src/Auth/Process/AttributeLimit.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
use SimpleSAML\Assert\Assert;
use SimpleSAML\Auth;
use SimpleSAML\Error;
use SimpleSAML\Logger;

/**
* A filter for limiting which attributes are passed on.
Expand Down Expand Up @@ -151,7 +150,7 @@ private function filterAttributeValues(array $values, array $allowedConfigValues
*/
$regexResult = @preg_match($pattern, $attributeValue);
if ($regexResult === false) {
Logger::warning("Error processing regex '$pattern' on value '$attributeValue'");
$this->logger->warning("Error processing regex '$pattern' on value '$attributeValue'");
break;
} elseif ($regexResult === 1) {
$matchedValues[] = $attributeValue;
Expand Down
7 changes: 3 additions & 4 deletions modules/core/src/Auth/Process/AttributeValueMap.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
use SimpleSAML\Assert\Assert;
use SimpleSAML\Auth;
use SimpleSAML\Error;
use SimpleSAML\Logger;

/**
* Filter to create target attribute based on value(s) in source attribute
Expand Down Expand Up @@ -68,7 +67,7 @@ public function __construct(array &$config, $reserved)
$this->keep = true;
} else {
// unknown configuration option, log it and ignore the error
Logger::warning(
$this->logger->warning(
"AttributeValueMap: unknown configuration flag '" . var_export($value, true) . "'"
);
}
Expand Down Expand Up @@ -111,7 +110,7 @@ public function __construct(array &$config, $reserved)
*/
public function process(array &$state): void
{
Logger::debug('Processing the AttributeValueMap filter.');
$this->logger->debug('Processing the AttributeValueMap filter.');

Assert::keyExists($state, 'Attributes');
$attributes = &$state['Attributes'];
Expand All @@ -130,7 +129,7 @@ public function process(array &$state): void
$values = [$values];
}
if (count(array_intersect($values, $sourceattribute)) > 0) {
Logger::debug("AttributeValueMap: intersect match for '$value'");
$this->logger->debug("AttributeValueMap: intersect match for '$value'");
$targetvalues[] = $value;
}
}
Expand Down
10 changes: 4 additions & 6 deletions modules/core/src/Auth/Process/Cardinality.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
use SimpleSAML\Assert\Assert;
use SimpleSAML\Auth;
use SimpleSAML\Error;
use SimpleSAML\Logger;
use SimpleSAML\Module;
use SimpleSAML\Utils;

Expand Down Expand Up @@ -41,7 +40,6 @@ public function __construct(array &$config, $reserved, Utils\HTTP $httpUtils = n
parent::__construct($config, $reserved);

$this->httpUtils = $httpUtils ?: new Utils\HTTP();

foreach ($config as $attribute => $rules) {
if ($attribute === '%ignoreEntities') {
$this->ignoreEntities = $config['%ignoreEntities'];
Expand Down Expand Up @@ -117,7 +115,7 @@ public function process(array &$state): void
$entityid = $state['Source']['entityid'];
}
if (in_array($entityid, $this->ignoreEntities, true)) {
Logger::debug('Cardinality: Ignoring assertions from ' . $entityid);
$this->logger->debug('Cardinality: Ignoring assertions from ' . $entityid);
return;
}

Expand All @@ -132,7 +130,7 @@ public function process(array &$state): void
/* minimum cardinality */
if (count($v) < $this->cardinality[$k]['min']) {
if ($this->cardinality[$k]['warn']) {
Logger::warning(
$this->logger->warning(
sprintf(
'Cardinality: attribute %s from %s does not meet minimum cardinality of %d (%d)',
$k,
Expand All @@ -153,7 +151,7 @@ public function process(array &$state): void
/* maximum cardinality */
if (array_key_exists('max', $this->cardinality[$k]) && count($v) > $this->cardinality[$k]['max']) {
if ($this->cardinality[$k]['warn']) {
Logger::warning(
$this->logger->warning(
sprintf(
'Cardinality: attribute %s from %s does not meet maximum cardinality of %d (%d)',
$k,
Expand All @@ -178,7 +176,7 @@ public function process(array &$state): void
continue;
}
if ($this->cardinality[$k]['warn']) {
Logger::warning(sprintf(
$this->logger->warning(sprintf(
'Cardinality: attribute %s from %s is missing',
$k,
$entityid
Expand Down
4 changes: 1 addition & 3 deletions modules/core/src/Auth/Process/CardinalitySingle.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

use SimpleSAML\Assert\Assert;
use SimpleSAML\Auth;
use SimpleSAML\Logger;
use SimpleSAML\Module;
use SimpleSAML\Utils;

Expand Down Expand Up @@ -51,7 +50,6 @@ public function __construct(array &$config, $reserved, Utils\HTTP $httpUtils = n
parent::__construct($config, $reserved);

$this->httpUtils = $httpUtils ?: new Utils\HTTP();

if (array_key_exists('singleValued', $config)) {
$this->singleValued = $config['singleValued'];
}
Expand Down Expand Up @@ -92,7 +90,7 @@ public function process(array &$state): void
&& array_key_exists('entityid', $state['Source'])
&& in_array($state['Source']['entityid'], $this->ignoreEntities, true)
) {
Logger::debug('CardinalitySingle: Ignoring assertions from ' . $state['Source']['entityid']);
$this->logger->debug('CardinalitySingle: Ignoring assertions from ' . $state['Source']['entityid']);
return;
}

Expand Down
3 changes: 1 addition & 2 deletions modules/core/src/Auth/Process/GenerateGroups.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
use Exception;
use SimpleSAML\Assert\Assert;
use SimpleSAML\Auth;
use SimpleSAML\Logger;

/**
* Filter to generate a groups attribute based on many of the attributes of the user.
Expand Down Expand Up @@ -72,7 +71,7 @@ public function process(array &$state): void

foreach ($this->generateGroupsFrom as $name) {
if (!array_key_exists($name, $attributes)) {
Logger::debug('GenerateGroups - attribute \'' . $name . '\' not found.');
$this->logger->debug('GenerateGroups - attribute \'' . $name . '\' not found.');
// Attribute not present
continue;
}
Expand Down
5 changes: 2 additions & 3 deletions modules/core/src/Auth/Process/LanguageAdaptor.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
use SimpleSAML\Assert\Assert;
use SimpleSAML\Auth;
use SimpleSAML\Locale\Language;
use SimpleSAML\Logger;

/**
* Filter to set and get language settings from attributes.
Expand Down Expand Up @@ -57,10 +56,10 @@ public function process(array &$state): void
$lang = Language::getLanguageCookie();

if (isset($attrlang)) {
Logger::debug('LanguageAdaptor: Language in attribute was set [' . $attrlang . ']');
$this->logger->debug('LanguageAdaptor: Language in attribute was set [' . $attrlang . ']');
}
if (isset($lang)) {
Logger::debug('LanguageAdaptor: Language in session was set [' . $lang . ']');
$this->logger->debug('LanguageAdaptor: Language in session was set [' . $lang . ']');
}

if (isset($attrlang) && !isset($lang)) {
Expand Down
5 changes: 2 additions & 3 deletions modules/core/src/Auth/Process/ScopeFromAttribute.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
use SimpleSAML\Assert\Assert;
use SimpleSAML\Auth;
use SimpleSAML\Configuration;
use SimpleSAML\Logger;

/**
* Retrieve a scope from a source attribute and add it as a virtual target
Expand Down Expand Up @@ -88,11 +87,11 @@ public function process(array &$state): void
$attributes[$this->targetAttribute] = [];
$scope = substr($sourceAttrVal, $scopeIndex + 1);
$attributes[$this->targetAttribute][] = $scope;
Logger::debug(
$this->logger->debug(
'ScopeFromAttribute: Inserted new attribute ' . $this->targetAttribute . ', with scope ' . $scope
);
} else {
Logger::warning('ScopeFromAttribute: The configured source attribute ' . $this->sourceAttribute
$this->logger->warning('ScopeFromAttribute: The configured source attribute ' . $this->sourceAttribute
. ' does not have a scope. Did not add attribute ' . $this->targetAttribute . '.');
}
}
Expand Down
5 changes: 2 additions & 3 deletions modules/core/src/Auth/Process/StatisticsWithAttribute.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
use Exception;
use SimpleSAML\Assert\Assert;
use SimpleSAML\Auth;
use SimpleSAML\Logger;

/**
* Log a line in the STAT log with one attribute.
Expand Down Expand Up @@ -92,10 +91,10 @@ public function process(array &$state): void

if (!array_key_exists('PreviousSSOTimestamp', $state)) {
// The user hasn't authenticated with this SP earlier in this session
Logger::stats($isPassive . $this->typeTag . '-first ' . $dest . ' ' . $source . ' ' . $logAttribute);
$this->logger->stats($isPassive . $this->typeTag . '-first ' . $dest . ' ' . $source . ' ' . $logAttribute);
}

Logger::stats($isPassive . $this->typeTag . ' ' . $dest . ' ' . $source . ' ' . $logAttribute);
$this->logger->stats($isPassive . $this->typeTag . ' ' . $dest . ' ' . $source . ' ' . $logAttribute);
}


Expand Down
3 changes: 1 addition & 2 deletions modules/core/src/Auth/Process/TargetedID.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
use SAML2\XML\saml\NameID;
use SimpleSAML\Assert\Assert;
use SimpleSAML\Auth;
use SimpleSAML\Logger;
use SimpleSAML\Utils;

/**
Expand Down Expand Up @@ -102,7 +101,7 @@ public function process(array &$state): void
{
Assert::keyExists($state, 'Attributes');
if (!array_key_exists($this->identifyingAttribute, $state['Attributes'])) {
Logger::warning(
$this->logger->warning(
sprintf(
"core:TargetedID: Missing attribute '%s', which is needed to generate the TargetedID.",
$this->identifyingAttribute
Expand Down
8 changes: 5 additions & 3 deletions modules/core/src/Auth/Process/WarnShortSSOInterval.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

use SimpleSAML\Assert\Assert;
use SimpleSAML\Auth;
use SimpleSAML\Logger;
use SimpleSAML\Module;
use SimpleSAML\Utils;

Expand Down Expand Up @@ -47,8 +46,11 @@ public function process(array &$state): void
$entityId = 'UNKNOWN';
}

Logger::warning('WarnShortSSOInterval: Only ' . $timeDelta .
' seconds since last SSO for this user from the SP ' . var_export($entityId, true));
$this->logger->warning(sprintf(
'WarnShortSSOInterval: Only %d seconds since last SSO for this user from the SP %s',
$timeDelta,
var_export($entityId, true)
));

// Save state and redirect
$id = Auth\State::saveState($state, 'core:short_sso_interval');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
use SAML2\Exception\Protocol\NoAuthnContextException;
use SimpleSAML\Assert\Assert;
use SimpleSAML\Error\Exception;
use SimpleSAML\Logger;
use SimpleSAML\Module\core\Auth\Source\AbstractSourceSelector;

use function array_key_exists;
use function sprintf;
Expand Down Expand Up @@ -101,7 +101,7 @@ protected function selectAuthSource(array &$state): string
{
$requestedContexts = $state['saml:RequestedAuthnContext'];
if ($requestedContexts['AuthnContextClassRef'] === null) {
Logger::info(
$this->logger->info(
"core:RequestedAuthnContextSelector: no RequestedAuthnContext provided; selecting default authsource"
);
return $this->defaultSource;
Expand Down
6 changes: 3 additions & 3 deletions modules/core/src/Auth/Source/SourceIPSelector.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

use SimpleSAML\Assert\Assert;
use SimpleSAML\Error\Exception;
use SimpleSAML\Logger;
use SimpleSAML\Module\core\Auth\Source\AbstractSourceSelector;
use SimpleSAML\Utils;

use function array_key_exists;
Expand Down Expand Up @@ -95,7 +95,7 @@ protected function selectAuthSource(/** @scrutinizer ignore-unused */ array &$st
foreach ($zone['subnet'] as $subnet) {
if ($netUtils->ipCIDRcheck($subnet, $ip)) {
// Client's IP is in one of the ranges for the secondary auth source
Logger::info(sprintf(
$this->logger->info(sprintf(
"core:SourceIPSelector: Selecting zone `%s` based on client IP %s",
$name,
$ip
Expand All @@ -107,7 +107,7 @@ protected function selectAuthSource(/** @scrutinizer ignore-unused */ array &$st
}

if ($source === $this->defaultSource) {
Logger::info("core:SourceIPSelector: no match on client IP; selecting default zone");
$this->logger->info("core:SourceIPSelector: no match on client IP; selecting default zone");
}

return $source;
Expand Down