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.

Sign up for GitHub

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

Remove ECS in favor of PHPCSFixer #1176

Open
wants to merge 4 commits into
base: 2.x
Choose a base branch
from
Open

Remove ECS in favor of PHPCSFixer #1176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7c088ee
Remove ECS in favor of PHPCSFixer
chalasr Dec 3, 2023
02f6df6
Fix CS
chalasr Dec 3, 2023
f4280ee
Fix code path
chalasr Dec 3, 2023
f389516
Fix build name
chalasr Dec 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions .github/workflows/coding-standards.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ jobs:
strategy:
matrix:
operating-system: [ubuntu-latest]
php-versions: ['8.1']
php-versions: ['8.2']
chalasr marked this conversation as resolved.
Show resolved Hide resolved
name: PHP ${{ matrix.php-versions }} Test on ${{ matrix.operating-system }}

steps:
- name: Checkout
uses: actions/checkout@v3
uses: actions/checkout@v4
chalasr marked this conversation as resolved.
Show resolved Hide resolved
with:
ref: ${{ github.head_ref }}

Expand All @@ -27,12 +27,14 @@ jobs:
- name: Install Composer dependencies
run: |
composer install
composer require rector/rector symplify/easy-coding-standard --dev
mkdir -p tools/php-cs-fixer tools/rector
composer require --working-dir=tools/php-cs-fixer friendsofphp/php-cs-fixer --dev
composer require rector/rector --dev
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this use case I would recommend using composer-bin-plugin with ramsey/install:

            -   name: Install PHP-CS-Fixer Composer bin dependencies
                uses: ramsey/composer-install@v2
                with:
                    working-directory: vendor-bin/php-cs-fixer

This would have three advantages:

  • You can easily install the tool locally to run it locally too.
  • The installation of the deps does not require any dependency resolution on Composer's end and the download will be cached.
  • Since those are locked dependencies, you can use dependabot to periodically upgrade them:
    -   package-ecosystem: "composer"
        directory: "vendor-bin/*/"
        schedule:
            interval: "weekly"
        groups:
            dependencies:
                patterns:
                    - "*"

I would say though for rector I put a * as a dependency to upgrade whenever possible.

Erratum as I see you committed tools/php-cs-fixer/composer.json:


- name: CONDING STANDARDS (ECS)
- name: PHP-CS-Fixer
run: |
vendor/bin/ecs check
tools/php-cs-fixer/vendor/bin/php-cs-fixer fix .

- name: CONDING STANDARDS (RECTOR)
run: |
vendor/bin/rector process --ansi --dry-run --xdebug
tools/rector/vendor/bin/rector process . --ansi --dry-run --xdebug
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is it running with Xdebgu?

14 changes: 14 additions & 0 deletions .php-cs-fixer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php

$finder = PhpCsFixer\Finder::create()->in([__DIR__]);

return (new PhpCsFixer\Config())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you feel like it: https://github.com/theofidry/php-cs-fixer-config

Or alternatively create your own, it's relatively easy.

->setRules([
'@Symfony' => true,
'ordered_imports' => true,
'array_syntax' => ['syntax' => 'short'],
'phpdoc_scalar' => false,
])
->setUsingCache(false)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no cache? Personally I like to use it but I put the output in a dist/ directory

->setFinder($finder);
;
10 changes: 1 addition & 9 deletions Command/CheckConfigCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,13 @@ public function __construct(KeyLoaderInterface $keyLoader, $signatureAlgorithm)
parent::__construct();
}

/**
* {@inheritdoc}
*/
protected function configure(): void
{
$this
->setName(static::$defaultName)
->setDescription('Checks JWT configuration');
}

/**
* {@inheritdoc}
*
* @return int
*/
protected function execute(InputInterface $input, OutputInterface $output): int
{
try {
Expand All @@ -57,7 +49,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$this->keyLoader->loadKey(KeyLoaderInterface::TYPE_PUBLIC);
}
} catch (\RuntimeException $e) {
$output->writeln('<error>' . $e->getMessage() . '</error>');
$output->writeln('<error>'.$e->getMessage().'</error>');

return 1;
}
Expand Down
29 changes: 10 additions & 19 deletions Command/EnableEncryptionConfigCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,7 @@
use Jose\Component\Encryption\Algorithm\ContentEncryptionAlgorithm;
use Jose\Component\Encryption\Algorithm\KeyEncryptionAlgorithm;
use Jose\Component\Encryption\JWEBuilder;
use Jose\Component\Encryption\JWELoader;
use Jose\Component\KeyManagement\JWKFactory;
use Jose\Component\Signature\JWSBuilder;
use Jose\Component\Signature\JWSLoader;
use Lexik\Bundle\JWTAuthenticationBundle\Services\KeyLoader\KeyLoaderInterface;
use ParagonIE\ConstantTime\Base64UrlSafe;
use Symfony\Bundle\FrameworkBundle\Command\AbstractConfigCommand;
use Symfony\Component\Config\Definition\Processor;
Expand Down Expand Up @@ -47,16 +43,13 @@ final class EnableEncryptionConfigCommand extends AbstractConfigCommand
private $algorithmManagerFactory;

public function __construct(
?AlgorithmManagerFactory $algorithmManagerFactory = null
AlgorithmManagerFactory $algorithmManagerFactory = null
) {
parent::__construct();

$this->algorithmManagerFactory = $algorithmManagerFactory;
}

/**
* {@inheritdoc}
*/
protected function configure(): void
{
$this
Expand All @@ -68,14 +61,9 @@ protected function configure(): void

public function isEnabled(): bool
{
return $this->algorithmManagerFactory !== null;
return null !== $this->algorithmManagerFactory;
}

/**
* {@inheritdoc}
*
* @return int
*/
protected function execute(InputInterface $input, OutputInterface $output): int
{
$force = $input->getOption('force');
Expand All @@ -87,7 +75,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$algorithms = $this->algorithmManagerFactory->all();
$availableKeyEncryptionAlgorithms = array_map(
static fn (Algorithm $algorithm): string => $algorithm->name(),
array_filter($algorithms, static fn (Algorithm $algorithm): bool => ($algorithm instanceof KeyEncryptionAlgorithm && $algorithm->name() !== 'dir'))
array_filter($algorithms, static fn (Algorithm $algorithm): bool => ($algorithm instanceof KeyEncryptionAlgorithm && 'dir' !== $algorithm->name()))
);
$availableContentEncryptionAlgorithms = array_map(
static fn (Algorithm $algorithm): string => $algorithm->name(),
Expand All @@ -103,12 +91,14 @@ protected function execute(InputInterface $input, OutputInterface $output): int

$extension = $this->findExtension('lexik_jwt_authentication');
$config = $this->getConfiguration($extension);
if (!isset($config['encoder']['service']) || $config['encoder']['service'] !== 'lexik_jwt_authentication.encoder.web_token') {
if (!isset($config['encoder']['service']) || 'lexik_jwt_authentication.encoder.web_token' !== $config['encoder']['service']) {
$io->error('Please migrate to WebToken first.');

return self::FAILURE;
}
if (!$force && ($config['access_token_issuance']['encryption']['enabled'] || $config['access_token_verification']['encryption']['enabled'])) {
$io->error('Encryption support is already enabled.');

return self::FAILURE;
}

Expand Down Expand Up @@ -162,7 +152,7 @@ private function withOctKeys(JWKSet $keyset, string $algorithm): JWKSet
return $keyset
->with($this->createOctKey($size, $algorithm)->toPublic())
->with($this->createOctKey($size, $algorithm)->toPublic())
;
;
}

private function withRsaKeys(JWKSet $keyset, string $algorithm): JWKSet
Expand Down Expand Up @@ -214,7 +204,7 @@ private function checkRequirements(): void
ClaimCheckerManager::class => 'web-token/jwt-checker',
JWEBuilder::class => 'web-token/jwt-encryption',
];
if ($this->algorithmManagerFactory === null) {
if (null === $this->algorithmManagerFactory) {
throw new \RuntimeException(sprintf('The package "web-token/jwt-bundle" is missing. Please install it for using this migration tool.', $requirement));
}
foreach (array_keys($requirements) as $requirement) {
Expand All @@ -223,6 +213,7 @@ private function checkRequirements(): void
}
}
}

private function getConfiguration(ExtensionInterface $extension): array
{
$container = $this->compileContainer();
Expand Down Expand Up @@ -335,7 +326,7 @@ private function getOptions(string $algorithm): array
return [
'use' => 'enc',
'alg' => $algorithm,
'kid'=> Base64UrlSafe::encodeUnpadded(random_bytes(16))
'kid' => Base64UrlSafe::encodeUnpadded(random_bytes(16)),
];
}
}
10 changes: 1 addition & 9 deletions Command/GenerateTokenCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,6 @@ public function __construct(JWTTokenManagerInterface $tokenManager, \Traversable
$this->userProviders = $userProviders;
}

/**
* {@inheritdoc}
*/
protected function configure(): void
{
$this
Expand All @@ -51,11 +48,6 @@ protected function configure(): void
;
}

/**
* {@inheritdoc}
*
* @return int
*/
protected function execute(InputInterface $input, OutputInterface $output): int
{
if ($this->userProviders instanceof \Countable && 0 === \count($this->userProviders)) {
Expand Down Expand Up @@ -102,7 +94,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int

$output->writeln([
'',
'<info>' . $token . '</info>',
'<info>'.$token.'</info>',
'',
]);

Expand Down
25 changes: 9 additions & 16 deletions Command/MigrateConfigCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
use Jose\Component\Core\JWKSet;
use Jose\Component\KeyManagement\JWKFactory;
use Jose\Component\Signature\JWSBuilder;
use Jose\Component\Signature\JWSLoader;
use Lexik\Bundle\JWTAuthenticationBundle\Services\KeyLoader\KeyLoaderInterface;
use ParagonIE\ConstantTime\Base64UrlSafe;
use Symfony\Bundle\FrameworkBundle\Command\AbstractConfigCommand;
Expand Down Expand Up @@ -56,13 +55,10 @@ public function __construct(
) {
parent::__construct();
$this->keyLoader = $keyLoader;
$this->passphrase = $passphrase === '' ? null : $passphrase;
$this->passphrase = '' === $passphrase ? null : $passphrase;
$this->signatureAlgorithm = $signatureAlgorithm;
}

/**
* {@inheritdoc}
*/
protected function configure(): void
{
$this
Expand All @@ -71,11 +67,6 @@ protected function configure(): void
;
}

/**
* {@inheritdoc}
*
* @return int
*/
protected function execute(InputInterface $input, OutputInterface $output): int
{
$this->checkRequirements();
Expand All @@ -87,7 +78,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$key = $this->getKey();
$keyset = $this->getKeyset($key, $this->signatureAlgorithm);
} catch (\RuntimeException $e) {
$io->error('An error occurred: ' . $e->getMessage());
$io->error('An error occurred: '.$e->getMessage());

return self::FAILURE;
}
Expand All @@ -96,7 +87,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$config = $this->getConfiguration($extension);

foreach ($config['set_cookies'] as $cookieConfig) {
if ($cookieConfig['split'] !== []) {
if ([] !== $cookieConfig['split']) {
$io->error('Web-Token is not compatible with the cookie split feature. Please disable this option before using this migration tool.');

return self::FAILURE;
Expand All @@ -109,14 +100,14 @@ protected function execute(InputInterface $input, OutputInterface $output): int
'signature' => [
'signature_algorithm' => $this->signatureAlgorithm,
'key' => json_encode($key, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE),
]
],
];
$config['access_token_verification'] = [
'enabled' => true,
'signature' => [
'allowed_signature_algorithms' => [$this->signatureAlgorithm],
'keyset' => json_encode($keyset, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE),
]
],
];

$io->comment('Please replace the current configuration with the following parameters.');
Expand Down Expand Up @@ -151,7 +142,7 @@ private function withOctKeys(JWKSet $keyset, string $algorithm): JWKSet
return $keyset
->with($this->createOctKey($size, $algorithm)->toPublic())
->with($this->createOctKey($size, $algorithm)->toPublic())
;
;
}

private function withRsaKeys(JWKSet $keyset, string $algorithm): JWKSet
Expand Down Expand Up @@ -191,6 +182,7 @@ private function getKey(): JWK
$additionalValues
);
}

return JWKFactory::createFromKey(
$this->keyLoader->loadKey(KeyLoaderInterface::TYPE_PRIVATE),
$this->passphrase,
Expand All @@ -213,6 +205,7 @@ private function checkRequirements(): void
}
}
}

private function getConfiguration(ExtensionInterface $extension): array
{
$container = $this->compileContainer();
Expand Down Expand Up @@ -317,7 +310,7 @@ private function getOptions(string $algorithm): array
return [
'use' => 'sig',
'alg' => $algorithm,
'kid'=> Base64UrlSafe::encodeUnpadded(random_bytes(16))
'kid' => Base64UrlSafe::encodeUnpadded(random_bytes(16)),
];
}
}
4 changes: 2 additions & 2 deletions DependencyInjection/Compiler/ApiPlatformOpenApiPass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ public function process(ContainerBuilder $container): void
$passwordPath = null;
$firewalls = $container->getParameter('security.firewalls');
foreach ($firewalls as $firewallName) {
if ($container->hasDefinition('security.authenticator.json_login.' . $firewallName)) {
$firewallOptions = $container->getDefinition('security.authenticator.json_login.' . $firewallName)->getArgument(4);
if ($container->hasDefinition('security.authenticator.json_login.'.$firewallName)) {
$firewallOptions = $container->getDefinition('security.authenticator.json_login.'.$firewallName)->getArgument(4);
$checkPath = $firewallOptions['check_path'];
$usernamePath = $firewallOptions['username_path'];
$passwordPath = $firewallOptions['password_path'];
Expand Down
3 changes: 0 additions & 3 deletions DependencyInjection/Compiler/DeprecateLegacyGuardAuthenticatorPass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,9 @@

namespace Lexik\Bundle\JWTAuthenticationBundle\DependencyInjection\Compiler;

use Lexik\Bundle\JWTAuthenticationBundle\Security\Guard\JWTTokenAuthenticator;
use Symfony\Component\Config\Definition\BaseNode;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;

/**
* @internal
Expand Down
6 changes: 1 addition & 5 deletions DependencyInjection/Configuration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

namespace Lexik\Bundle\JWTAuthenticationBundle\DependencyInjection;

use ApiPlatform\Symfony\Bundle\ApiPlatformBundle;
use Symfony\Component\Config\Definition\BaseNode;
use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
Expand All @@ -17,9 +16,6 @@ class Configuration implements ConfigurationInterface
{
public const INVALID_KEY_PATH = "The file %s doesn't exist or is not readable.\nIf the configured encoder doesn't need this to be configured, please don't set this option or leave it null.";

/**
* {@inheritdoc}
*/
public function getConfigTreeBuilder(): TreeBuilder
{
$treeBuilder = new TreeBuilder('lexik_jwt_authentication');
Expand Down Expand Up @@ -80,7 +76,7 @@ public function getConfigTreeBuilder(): TreeBuilder
->end()
->end()
->scalarNode('user_identity_field')
->setDeprecated(...$this->getDeprecationParameters('The "%path%.%node%" configuration key is deprecated since version 2.16, use "%path%.user_id_claim" or implement "' . UserInterface::class . '::getUserIdentifier()" instead.', '2.16'))
->setDeprecated(...$this->getDeprecationParameters('The "%path%.%node%" configuration key is deprecated since version 2.16, use "%path%.user_id_claim" or implement "'.UserInterface::class.'::getUserIdentifier()" instead.', '2.16'))
->defaultValue('username')
->cannotBeEmpty()
->end()
Expand Down
Loading