Skip to content

Commit

Permalink
Migrate psalm to phpstan
Browse files Browse the repository at this point in the history
  • Loading branch information
tvdijen committed Aug 10, 2024
1 parent 3f3cc9c commit 16ca6f2
Show file tree
Hide file tree
Showing 21 changed files with 60 additions and 158 deletions.
25 changes: 5 additions & 20 deletions .github/workflows/php.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,7 @@ jobs:
with:
# Should be the higest supported version, so we can use the newest tools
php-version: '8.3'
tools: composer, composer-require-checker, composer-unused, phpcs, psalm
# optional performance gain for psalm: opcache
tools: composer, composer-require-checker, composer-unused, phpcs, phpstan
extensions: ctype, date, dom, fileinfo, filter, hash, intl, mbstring, opcache, openssl, pcre, spl, xml

- name: Setup problem matchers for PHP
Expand Down Expand Up @@ -84,27 +83,13 @@ jobs:
- name: PHP Code Sniffer
run: phpcs

- name: Psalm
continue-on-error: true
- name: PHPStan
run: |
psalm -c psalm.xml \
--show-info=true \
--shepherd \
--php-version=${{ steps.setup-php.outputs.php-version }}
phpstan analyze -c phpstan.neon --debug
- name: Psalm (testsuite)
- name: PHPStan (testsuite)
run: |
psalm -c psalm-dev.xml \
--show-info=true \
--shepherd \
--php-version=${{ steps.setup-php.outputs.php-version }}
- name: Psalter
run: |
psalm --alter \
--issues=UnnecessaryVarAnnotation \
--dry-run \
--php-version=${{ steps.setup-php.outputs.php-version }}
phpstan analyze -c phpstan-dev.neon --debug
security:
name: Security checks
Expand Down
11 changes: 11 additions & 0 deletions phpstan-dev-baseline.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
parameters:
ignoreErrors:
-
message: "#^Unreachable statement \\- code above always terminates\\.$#"
count: 1
path: tests/src/Controller/RegProcessTest.php

-
message: "#^Unreachable statement \\- code above always terminates\\.$#"
count: 1
path: tests/src/Controller/RegistrationTest.php
4 changes: 4 additions & 0 deletions phpstan-dev.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
parameters:
level: 5
paths:
- tests
6 changes: 6 additions & 0 deletions phpstan.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
parameters:
level: 3
paths:
- src
bootstrapFiles:
- tests/bootstrap.php
27 changes: 0 additions & 27 deletions psalm-dev.xml

This file was deleted.

37 changes: 0 additions & 37 deletions psalm.xml

This file was deleted.

4 changes: 0 additions & 4 deletions src/Auth/Process/WebAuthn.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,6 @@ class WebAuthn extends Auth\ProcessingFilter
*/
public function __construct(array $config, $reserved)
{
/**
* Remove annotation + assert as soon as this method can be typehinted (SSP 2.0)
* @psalm-suppress RedundantConditionGivenDocblockType
*/
parent::__construct($config, $reserved);

$moduleConfig = Configuration::getOptionalConfig('module_webauthn.php')->toArray();
Expand Down
11 changes: 2 additions & 9 deletions src/Controller/AuthProcess.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,10 @@
*/
class AuthProcess
{
/**
* @var \SimpleSAML\Auth\State|string
* @psalm-var \SimpleSAML\Auth\State|class-string
*/
/** @var \SimpleSAML\Auth\State|string */
protected $authState = Auth\State::class;

/**
* @var \SimpleSAML\Logger|string
* @psalm-var \SimpleSAML\Logger|class-string
*/
/** @var \SimpleSAML\Logger|string */
protected $logger = Logger::class;

/**
Expand Down Expand Up @@ -151,7 +145,6 @@ public function main(Request $request): Response
$oneToken[1] = stream_get_contents($oneToken[1]);
}

/** @psalm-var array $oneToken */
$authObject = new WebAuthnAuthenticationEvent(
$request->request->get('type'),
($state['FIDO2Scope'] === null ? $state['FIDO2DerivedScope'] : $state['FIDO2Scope']),
Expand Down
10 changes: 2 additions & 8 deletions src/Controller/ManageToken.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,10 @@
*/
class ManageToken
{
/**
* @var \SimpleSAML\Auth\State|string
* @psalm-var \SimpleSAML\Auth\State|class-string
*/
/** @var \SimpleSAML\Auth\State|string */
protected $authState = Auth\State::class;

/**
* @var \SimpleSAML\Logger|string
* @psalm-var \SimpleSAML\Logger|class-string
*/
/** @var \SimpleSAML\Logger|string */
protected $logger = Logger::class;


Expand Down
19 changes: 6 additions & 13 deletions src/Controller/PushbackUserPass.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,13 @@

use Exception;
use SimpleSAML\Auth;
use SimpleSAML\Auth\Source;
use SimpleSAML\Configuration;
use SimpleSAML\Error;
use SimpleSAML\HTTP\RunnableResponse;
use SimpleSAML\Logger;
use SimpleSAML\Module\webauthn\Auth\Source\AuthSourceOverloader;
use SimpleSAML\Session;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpFoundation\{Request, Response};

/**
* Controller class for the webauthn module.
Expand All @@ -24,16 +23,10 @@
*/
class PushbackUserPass
{
/**
* @var \SimpleSAML\Auth\State|string
* @psalm-var \SimpleSAML\Auth\State|class-string
*/
/** @var \SimpleSAML\Auth\State|string */
protected $authState = Auth\State::class;

/**
* @var \SimpleSAML\Logger|string
* @psalm-var \SimpleSAML\Logger|class-string
*/
/** @var \SimpleSAML\Logger|string */
protected $logger = Logger::class;

/**
Expand Down Expand Up @@ -92,7 +85,7 @@ public function main(Request $request): Response

$authsources = Configuration::getConfig('authsources.php')->toArray();
$authsourceString = $state['pushbackAuthsource'];
$classname = get_class(Source::getById($authsourceString));
$classname = get_class(Auth\Source::getById($authsourceString));
class_alias($classname, '\SimpleSAML\Module\webauthn\Auth\Source\AuthSourceOverloader');
$overrideSource = new class (
['AuthId' => $authsourceString],
Expand All @@ -117,7 +110,7 @@ public function loginOverload(string $username, string $password): array
unset($attribs);

// now properly return our final state to the framework
Source::completeAuth($state);
return new RunnableResponse([Auth\Source::class, 'completeAuth'], [&$state]);
}

public function login(string $username, string $password): array
Expand Down
10 changes: 2 additions & 8 deletions src/Controller/RegProcess.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,10 @@
*/
class RegProcess
{
/**
* @var \SimpleSAML\Auth\State|string
* @psalm-var \SimpleSAML\Auth\State|class-string
*/
/** @var \SimpleSAML\Auth\State|string */
protected $authState = Auth\State::class;

/**
* @var \SimpleSAML\Logger|string
* @psalm-var \SimpleSAML\Logger|class-string
*/
/** @var \SimpleSAML\Logger|string */
protected $logger = Logger::class;


Expand Down
17 changes: 4 additions & 13 deletions src/Controller/Registration.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,13 @@
*/
class Registration
{
/**
* @var \SimpleSAML\Auth\State|string
* @psalm-var \SimpleSAML\Auth\State|class-string
*/
/** @var \SimpleSAML\Auth\State|string */
protected $authState = Auth\State::class;

/**
* @var \SimpleSAML\Auth\Simple|string
* @psalm-var \SimpleSAML\Auth\Simple|class-string
*/
/** @var \SimpleSAML\Auth\Simple|string */
protected $authSimple = Auth\Simple::class;

/**
* @var \SimpleSAML\Logger|string
* @psalm-var \SimpleSAML\Logger|class-string
*/
/** @var \SimpleSAML\Logger|string */
protected $logger = Logger::class;


Expand Down Expand Up @@ -108,7 +99,7 @@ public function main(/** @scrutinizer ignore-unused */ Request $request): Runnab

$state = [];
$state['SPMetadata']['entityid'] = "WEBAUTHN-SP-REGISTRATION";
/** @psalm-var class-string $authSimple */

$authSimple = $this->authSimple;
$as = new $authSimple($registrationAuthSource);
$as->requireAuth();
Expand Down
10 changes: 2 additions & 8 deletions src/Controller/WebAuthn.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,10 @@ class WebAuthn
public const STATE_AUTH_ALLOWMGMT = 2; // allow to switch to mgmt page
public const STATE_MGMT = 4; // show token management page

/**
* @var \SimpleSAML\Auth\State|string
* @psalm-var \SimpleSAML\Auth\State|class-string
*/
/** @var \SimpleSAML\Auth\State|string */
protected $authState = Auth\State::class;

/**
* @var \SimpleSAML\Logger|string
* @psalm-var \SimpleSAML\Logger|class-string
*/
/** @var \SimpleSAML\Logger|string */
protected $logger = Logger::class;


Expand Down
6 changes: 2 additions & 4 deletions src/Store.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ abstract class Store
* This constructor should always be called first in any class which implements this class.
*
* @param array &$config The configuration for this storage handler.
* @phpstan-ignore constructor.unusedParameter
*/
protected function __construct(array &$config)
{
Expand Down Expand Up @@ -168,10 +169,7 @@ public static function parseStoreConfig(string|array $config): Store
'\SimpleSAML\Module\webauthn\Store',
);

/**
* @psalm-suppress InvalidStringClass
* @var \SimpleSAML\Module\webauthn\Store
*/
/** @var \SimpleSAML\Module\webauthn\Store */
return new $className($config);
}
}
6 changes: 3 additions & 3 deletions src/WebAuthn/AAGUID.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class AAGUID
/**
* The singleton instance.
*
* @var static
* @var \SimpleSAML\Module\webauthn\WebAuthn\AAGUID
*/
protected static AAGUID $instance;

Expand All @@ -45,7 +45,7 @@ protected function __construct()
$path = $config->getConfigDir() . '/' . self::AAGUID_CONFIG_FILE;
if (!file_exists($path)) {
Logger::warning("Missing AAGUID configuration file ($path). No device will be recognized.");
return null;
return;
}

$data = file_get_contents($path);
Expand All @@ -62,7 +62,7 @@ protected function __construct()
/**
* Get the singleton instance of the AAGUID dictionary.
*
* @return static
* @return self
*/
public static function getInstance(): self
{
Expand Down
3 changes: 3 additions & 0 deletions src/WebAuthn/WebAuthnAbstractEvent.php
Original file line number Diff line number Diff line change
Expand Up @@ -430,8 +430,11 @@ protected function cborDecode(string $rawData): array

$decoder = new Decoder($tagManager, $otherObjectManager);
$stream = new StringStream($rawData);

/** @var \CBOR\OtherObject $object */
$object = $decoder->decode($stream);
$finalData = $object->normalize();

Check failure on line 436 in src/WebAuthn/WebAuthnAbstractEvent.php

View workflow job for this annotation

GitHub Actions / Quality control

Call to an undefined method CBOR\OtherObject::normalize().

if ($finalData === null) {
$this->fail("CBOR data decoding failed.");
}
Expand Down
2 changes: 0 additions & 2 deletions src/WebAuthn/WebAuthnRegistrationEvent.php
Original file line number Diff line number Diff line change
Expand Up @@ -628,8 +628,6 @@ private function validateAttestationFormatFidoU2F(array $attestationData): void
}
/**
* §8.6 Verification Step 5: create verificationData
*
* @psalm-var string $publicKeyU2F
*/
$verificationData = chr(0) . $this->rpIdHash . $this->clientDataHash . $this->credentialId . $publicKeyU2F;
/**
Expand Down
6 changes: 6 additions & 0 deletions tests/bootstrap.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,9 @@
print "Linking '$linkPath' to '$projectRoot'\n";
symlink($projectRoot, $linkPath);
}

// Little dirty hack to satisfy PHPstan
class_alias(
\SimpleSAML\Module\core\Auth\Source\AdminPassword::class,
'\SimpleSAML\Module\webauthn\Auth\Source\AuthSourceOverloader',
);
Loading

0 comments on commit 16ca6f2

Please sign in to comment.