From 64543f0ad4de4bc7e019f5b3f38b62f6e1f1cc8e Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Sun, 11 Jun 2023 20:48:40 +0200 Subject: [PATCH] Fix several psalm-issues --- src/SAML2/Assertion/Processor.php | 21 +++++++++++-------- .../NameIdDecryptionTransformer.php | 4 +++- src/SAML2/Certificate/Key.php | 15 ++++--------- src/SAML2/Compat/MockContainer.php | 5 +++-- .../Exception/ProtocolViolationException.php | 12 ++++++++--- src/SAML2/HTTPArtifact.php | 4 ++-- src/SAML2/XML/md/AffiliationDescriptor.php | 2 +- .../XML/md/AttributeAuthorityDescriptor.php | 6 +++--- src/SAML2/XML/md/ContactPerson.php | 2 +- src/SAML2/XML/md/PDPDescriptor.php | 4 ++-- src/SAML2/XML/mdrpi/RegistrationInfo.php | 2 +- src/SAML2/XML/mdui/DiscoHints.php | 1 + src/SAML2/XML/mdui/UIInfo.php | 3 ++- src/SAML2/XML/saml/Assertion.php | 2 +- src/SAML2/XML/saml/AuthzDecisionStatement.php | 2 +- src/SAML2/XML/saml/Subject.php | 2 +- src/SAML2/XML/samlp/IDPList.php | 2 +- src/SAML2/XML/samlp/NameIDPolicy.php | 1 + 18 files changed, 49 insertions(+), 41 deletions(-) diff --git a/src/SAML2/Assertion/Processor.php b/src/SAML2/Assertion/Processor.php index 4adf4064a..231c97113 100644 --- a/src/SAML2/Assertion/Processor.php +++ b/src/SAML2/Assertion/Processor.php @@ -132,15 +132,18 @@ public function validateAssertion(Assertion $assertion): void )); } - foreach ($assertion->getSubject()->getSubjectConfirmation() as $subjectConfirmation) { - $subjectConfirmationValidationResult = $this->subjectConfirmationValidator->validate( - $subjectConfirmation, - ); - if (!$subjectConfirmationValidationResult->isValid()) { - throw new InvalidSubjectConfirmationException(sprintf( - 'Invalid SubjectConfirmation in Assertion, errors: "%s"', - implode('", "', $subjectConfirmationValidationResult->getErrors()), - )); + $subject = $assertion->getSubject(); + if ($subject !== null) { + foreach ($subject->getSubjectConfirmation() as $subjectConfirmation) { + $subjectConfirmationValidationResult = $this->subjectConfirmationValidator->validate( + $subjectConfirmation, + ); + if (!$subjectConfirmationValidationResult->isValid()) { + throw new InvalidSubjectConfirmationException(sprintf( + 'Invalid SubjectConfirmation in Assertion, errors: "%s"', + implode('", "', $subjectConfirmationValidationResult->getErrors()), + )); + } } } } diff --git a/src/SAML2/Assertion/Transformer/NameIdDecryptionTransformer.php b/src/SAML2/Assertion/Transformer/NameIdDecryptionTransformer.php index 5e2951e84..f41f29c22 100644 --- a/src/SAML2/Assertion/Transformer/NameIdDecryptionTransformer.php +++ b/src/SAML2/Assertion/Transformer/NameIdDecryptionTransformer.php @@ -15,6 +15,7 @@ use SimpleSAML\SAML2\Configuration\ServiceProviderAware; use SimpleSAML\SAML2\XML\saml\Assertion; use SimpleSAML\SAML2\XML\saml\EncryptedID; +use SimpleSAML\SAML2\XML\saml\IdentifierInterface; use SimpleSAML\SAML2\XML\saml\Subject; use function get_class; @@ -92,11 +93,12 @@ public function transform(Assertion $assertion): Assertion 'Could not decrypt the assertion NameId with the configured keys, see the debug log for information', ); } + Assert::implementsInterface($decrypted, IdentifierInterface::class); return new Assertion( $assertion->getIssuer(), - $assertion->getId(), $assertion->getIssueInstant(), + $assertion->getId(), new Subject($decrypted, $subject->getSubjectConfirmation()), $assertion->getConditions(), $assertion->getStatements(), diff --git a/src/SAML2/Certificate/Key.php b/src/SAML2/Certificate/Key.php index c176342bc..4fe7b1fa9 100644 --- a/src/SAML2/Certificate/Key.php +++ b/src/SAML2/Certificate/Key.php @@ -64,10 +64,8 @@ public function canBeUsedFor(string $usage): bool * @param mixed $offset * @throws \SimpleSAML\SAML2\Exception\InvalidArgumentException * @return bool - * - * Type hint not possible due to upstream method signature */ - public function offsetExists($offset): bool + public function offsetExists(mixed $offset): bool { if (!is_string($offset)) { throw InvalidArgumentException::invalidType('string', $offset); @@ -80,11 +78,8 @@ public function offsetExists($offset): bool * @param mixed $offset * @throws \SimpleSAML\SAML2\Exception\InvalidArgumentException * @return mixed - * - * Type hint not possible due to upstream method signature */ - #[\ReturnTypeWillChange] - public function offsetGet($offset) + public function offsetGet($offset): mixed { if (!is_string($offset)) { throw InvalidArgumentException::invalidType('string', $offset); @@ -98,7 +93,7 @@ public function offsetGet($offset) * @param mixed $value * @throws \SimpleSAML\SAML2\Exception\InvalidArgumentException */ - public function offsetSet($offset, $value): void + public function offsetSet(mixed $offset, mixed $value): void { if (!is_string($offset)) { throw InvalidArgumentException::invalidType('string', $offset); @@ -110,10 +105,8 @@ public function offsetSet($offset, $value): void /** * @param mixed $offset * @throws \SimpleSAML\SAML2\Exception\InvalidArgumentException - * - * Type hint not possible due to upstream method signature */ - public function offsetUnset($offset): void + public function offsetUnset(mixed $offset): void { if (!is_string($offset)) { throw InvalidArgumentException::invalidType('string', $offset); diff --git a/src/SAML2/Compat/MockContainer.php b/src/SAML2/Compat/MockContainer.php index 8597c7f29..f03cad57c 100644 --- a/src/SAML2/Compat/MockContainer.php +++ b/src/SAML2/Compat/MockContainer.php @@ -10,6 +10,7 @@ use function chmod; use function file_put_contents; +use function strval; use function sys_get_temp_dir; /** @@ -63,7 +64,7 @@ public function getPOSTRedirectURL( /** @scrutinizer ignore-unused */string $url = null, /** @scrutinizer ignore-unused */array $data = [] ): string { - return $url; + return strval($url); } @@ -103,7 +104,7 @@ public function setBlacklistedAlgorithms(?array $algos): void /** * Set the system clock * - * @param \Psr\Clock\ClockInterface + * @param \Psr\Clock\ClockInterface $clock * @return void */ public function setClock(ClockInterface $clock): void diff --git a/src/SAML2/Exception/ProtocolViolationException.php b/src/SAML2/Exception/ProtocolViolationException.php index df7d2440e..99eb73d37 100644 --- a/src/SAML2/Exception/ProtocolViolationException.php +++ b/src/SAML2/Exception/ProtocolViolationException.php @@ -11,13 +11,19 @@ */ class ProtocolViolationException extends RuntimeException { - public const DEFAULT_MESSAGE = 'A violation of the SAML2 protocol occurred.'; - /** * @param string $message */ public function __construct(string $message = null) { - parent::__construct($message ?? static::DEFAULT_MESSAGE); + if ($message === null) { + if (defined('static::DEFAULT_MESSAGE')) { + $message = static::DEFAULT_MESSAGE; + } else { + $message = 'A violation of the SAML2 protocol occurred.'; + } + } + + parent::__construct($message); } } diff --git a/src/SAML2/HTTPArtifact.php b/src/SAML2/HTTPArtifact.php index f19e0e86c..5dffb11aa 100644 --- a/src/SAML2/HTTPArtifact.php +++ b/src/SAML2/HTTPArtifact.php @@ -76,7 +76,7 @@ public function getRedirectURL(AbstractMessage $message): string $artifactDataString = $artifactData->ownerDocument?->saveXML($artifactData); $clock = Utils::getContainer()->getClock(); - $store->set('artifact', $artifact, $artifactDataString, $clock->now()->add(new DateDInterval('PT15M'))); + $store->set('artifact', $artifact, $artifactDataString, $clock->now()->add(new DateInterval('PT15M'))); $destination = $message->getDestination(); if ($destination === null) { @@ -168,7 +168,7 @@ public function receive(ServerRequestInterface $request): AbstractMessage $issuer = new Issuer($this->spMetadata->getString('entityid')); // Construct the ArtifactResolve Request - $ar = new ArtifactResolve($query['SAMLart'], $issuer, null, null, null, $endpoint['Location']); + $ar = new ArtifactResolve($query['SAMLart'], null, $issuer, null, '2.0', $endpoint['Location']); // sign the request /** @psalm-suppress UndefinedClass */ diff --git a/src/SAML2/XML/md/AffiliationDescriptor.php b/src/SAML2/XML/md/AffiliationDescriptor.php index 3c9b33157..136a97e46 100644 --- a/src/SAML2/XML/md/AffiliationDescriptor.php +++ b/src/SAML2/XML/md/AffiliationDescriptor.php @@ -39,7 +39,7 @@ final class AffiliationDescriptor extends AbstractMetadataDocument * @param \DateTimeImmutable|null $validUntil Unix time of validity for this document. Defaults to null. * @param string|null $cacheDuration Maximum time this document can be cached. Defaults to null. * @param \SimpleSAML\SAML2\XML\md\Extensions|null $extensions An array of extensions. Defaults to an empty array. - * @param \SimpleSAML\SAML2\XML\md\KeyDescriptor[] $KeyDescriptor + * @param \SimpleSAML\SAML2\XML\md\KeyDescriptor[] $keyDescriptor * An optional array of KeyDescriptors. Defaults to an empty array. * @param list<\SimpleSAML\XML\Attribute> $namespacedAttribute */ diff --git a/src/SAML2/XML/md/AttributeAuthorityDescriptor.php b/src/SAML2/XML/md/AttributeAuthorityDescriptor.php index 9561f5e72..2c6712daa 100644 --- a/src/SAML2/XML/md/AttributeAuthorityDescriptor.php +++ b/src/SAML2/XML/md/AttributeAuthorityDescriptor.php @@ -28,10 +28,10 @@ final class AttributeAuthorityDescriptor extends AbstractRoleDescriptorType * * @param \SimpleSAML\SAML2\XML\md\AttributeService[] $attributeService * @param string[] $protocolSupportEnumeration - * @param \SimpleSAML\SAML2\XML\md\AssertionIDRequestService[] $sssertionIDRequestService + * @param \SimpleSAML\SAML2\XML\md\AssertionIDRequestService[] $asssertionIDRequestService * @param \SimpleSAML\SAML2\XML\md\NameIDFormat[] $nameIDFormat - * @param \SimpleSAML\SAML2\XML\md\AttributeProfile[] $sttributeProfile - * @param \SimpleSAML\SAML2\XML\saml\Attribute[] $sttribute + * @param \SimpleSAML\SAML2\XML\md\AttributeProfile[] $attributeProfile + * @param \SimpleSAML\SAML2\XML\saml\Attribute[] $attribute * @param string|null $ID * @param \DateTimeImmutable|null $validUntil * @param string|null $cacheDuration diff --git a/src/SAML2/XML/md/ContactPerson.php b/src/SAML2/XML/md/ContactPerson.php index dd5d8becf..53f331a47 100644 --- a/src/SAML2/XML/md/ContactPerson.php +++ b/src/SAML2/XML/md/ContactPerson.php @@ -356,7 +356,7 @@ public function toArray(): array 'SurName' => $this->getSurName()?->getContent(), 'EmailAddress' => [], 'TelephoneNumber' => [], - 'Extensions' => $this?->Extensions->getList(), + 'Extensions' => $this->Extensions?->getList(), 'attributes' => [], ]; diff --git a/src/SAML2/XML/md/PDPDescriptor.php b/src/SAML2/XML/md/PDPDescriptor.php index 660511d68..d777633c9 100644 --- a/src/SAML2/XML/md/PDPDescriptor.php +++ b/src/SAML2/XML/md/PDPDescriptor.php @@ -24,10 +24,10 @@ final class PDPDescriptor extends AbstractRoleDescriptorType /** * PDPDescriptor constructor. * - * @param \SimpleSAML\SAML2\XML\md\AuthzService[] $authzServiceEndpoints + * @param \SimpleSAML\SAML2\XML\md\AuthzService[] $authzService * @param string[] $protocolSupportEnumeration * @param \SimpleSAML\SAML2\XML\md\AssertionIDRequestService[] $assertionIDRequestService - * @param \SimpleSAML\SAML2\XML\md\NameIDFormat[] $nameIDFormats + * @param \SimpleSAML\SAML2\XML\md\NameIDFormat[] $nameIDFormat * @param string|null $ID * @param \DateTimeImmutable|null $validUntil * @param string|null $cacheDuration diff --git a/src/SAML2/XML/mdrpi/RegistrationInfo.php b/src/SAML2/XML/mdrpi/RegistrationInfo.php index 280d7e33b..0f4168a63 100644 --- a/src/SAML2/XML/mdrpi/RegistrationInfo.php +++ b/src/SAML2/XML/mdrpi/RegistrationInfo.php @@ -29,7 +29,7 @@ final class RegistrationInfo extends AbstractMdrpiElement implements Arrayizable * * @param string $registrationAuthority * @param \DateTimeImmutable|null $registrationInstant - * @param \SimpleSAML\SAML2\XML\mdrpi\RegistrationPolicy[] $RegistrationPolicy + * @param \SimpleSAML\SAML2\XML\mdrpi\RegistrationPolicy[] $registrationPolicy */ public function __construct( protected string $registrationAuthority, diff --git a/src/SAML2/XML/mdui/DiscoHints.php b/src/SAML2/XML/mdui/DiscoHints.php index 271caf873..896d8bef1 100644 --- a/src/SAML2/XML/mdui/DiscoHints.php +++ b/src/SAML2/XML/mdui/DiscoHints.php @@ -13,6 +13,7 @@ use SimpleSAML\XML\Constants as C; use SimpleSAML\XML\Exception\InvalidDOMElementException; use SimpleSAML\XML\ExtendableElementTrait; +use SimpleSAML\XML\SerializableElementInterface; use function array_filter; use function array_key_exists; diff --git a/src/SAML2/XML/mdui/UIInfo.php b/src/SAML2/XML/mdui/UIInfo.php index 4d0fb3653..35f3a576b 100644 --- a/src/SAML2/XML/mdui/UIInfo.php +++ b/src/SAML2/XML/mdui/UIInfo.php @@ -14,6 +14,7 @@ use SimpleSAML\XML\Constants as C; use SimpleSAML\XML\Exception\InvalidDOMElementException; use SimpleSAML\XML\ExtendableElementTrait; +use SimpleSAML\XML\SerializableElementInterface; use function array_filter; use function array_key_exists; @@ -228,7 +229,7 @@ public function isEmptyElement(): bool * * @param (\SimpleSAML\SAML2\XML\md\AbstractLocalizedURL| * \SimpleSAML\SAML2\XML\md\AbstractLocalizedName| - * \SimpleSAML\XML\SAML2\mdui\Keywords)[] $items + * \SimpleSAML\SAML2\XML\mdui\Keywords)[] $items * @return void */ private function testLocalizedElements(array $elements) diff --git a/src/SAML2/XML/saml/Assertion.php b/src/SAML2/XML/saml/Assertion.php index 1de3efc9e..61948ce86 100644 --- a/src/SAML2/XML/saml/Assertion.php +++ b/src/SAML2/XML/saml/Assertion.php @@ -83,7 +83,7 @@ public function __construct( ) { $this->dataType = C::XMLENC_ELEMENT; - Assert::same($issueInstant?->getTimeZone()->getName(), 'Z', ProtocolViolationException::class); + Assert::same($issueInstant->getTimeZone()->getName(), 'Z', ProtocolViolationException::class); Assert::nullOrValidNCName($id); // Covers the empty string Assert::true( $subject || !empty($statements), diff --git a/src/SAML2/XML/saml/AuthzDecisionStatement.php b/src/SAML2/XML/saml/AuthzDecisionStatement.php index d3ff3b400..dde5bc4ae 100644 --- a/src/SAML2/XML/saml/AuthzDecisionStatement.php +++ b/src/SAML2/XML/saml/AuthzDecisionStatement.php @@ -30,7 +30,7 @@ final class AuthzDecisionStatement extends AbstractStatementType * @param string $resource * @param string $decision * @param \SimpleSAML\SAML2\XML\saml\Action[] $action - * @param \SimpleSAML\SAML2\XML\saml\Evidence|null + * @param \SimpleSAML\SAML2\XML\saml\Evidence|null $evidence */ public function __construct( protected string $resource, diff --git a/src/SAML2/XML/saml/Subject.php b/src/SAML2/XML/saml/Subject.php index 8ff0e5c24..64190e8a0 100644 --- a/src/SAML2/XML/saml/Subject.php +++ b/src/SAML2/XML/saml/Subject.php @@ -26,7 +26,7 @@ final class Subject extends AbstractSamlElement * Initialize a Subject element. * * @param \SimpleSAML\SAML2\XML\saml\IdentifierInterface|null $identifier - * @param \SimpleSAML\SAML2\XML\saml\SubjectConfirmation[] $SubjectConfirmation + * @param \SimpleSAML\SAML2\XML\saml\SubjectConfirmation[] $subjectConfirmation */ public function __construct( ?IdentifierInterface $identifier, diff --git a/src/SAML2/XML/samlp/IDPList.php b/src/SAML2/XML/samlp/IDPList.php index 369299774..4e8850844 100644 --- a/src/SAML2/XML/samlp/IDPList.php +++ b/src/SAML2/XML/samlp/IDPList.php @@ -25,7 +25,7 @@ final class IDPList extends AbstractSamlpElement /** * Initialize an IDPList element. * - * @param \SimpleSAML\SAML2\XML\samlp\IDPEntry[] $idpEntry + * @param \SimpleSAML\SAML2\XML\samlp\IDPEntry[] $IDPEntry * @param \SimpleSAML\SAML2\XML\samlp\GetComplete|null $getComplete */ public function __construct( diff --git a/src/SAML2/XML/samlp/NameIDPolicy.php b/src/SAML2/XML/samlp/NameIDPolicy.php index 4393c91df..e119937dc 100644 --- a/src/SAML2/XML/samlp/NameIDPolicy.php +++ b/src/SAML2/XML/samlp/NameIDPolicy.php @@ -6,6 +6,7 @@ use DOMElement; use SimpleSAML\Assert\Assert; +use SimpleSAML\SAML2\Exception\ArrayValidationException; use SimpleSAML\XML\ArrayizableElementInterface; use SimpleSAML\XML\Exception\InvalidDOMElementException;