Skip to content

Commit

Permalink
Merge pull request #63 from flownative/task/cleanup
Browse files Browse the repository at this point in the history
Code cleanup
  • Loading branch information
kdambekalns authored Dec 11, 2024
2 parents e4b67d3 + 2004849 commit 54b2cbe
Show file tree
Hide file tree
Showing 15 changed files with 85 additions and 109 deletions.
24 changes: 7 additions & 17 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ name: CI

on:
push:
branches: [ master ]
branches: [ main ]
pull_request:
branches: [ master ]
branches: [ main ]

jobs:
tests:
Expand All @@ -22,25 +22,18 @@ jobs:
- ubuntu-latest

php-version:
- "8.0"
- "8.1"
- "8.2"
- "8.3"

compiler:
- default
- "8.4"

dependencies:
- lowest
- highest

steps:
- name: Checkout
uses: actions/checkout@v2

- name: Override PHP ini values for JIT compiler
if: matrix.compiler == 'jit'
run: echo "PHP_INI_VALUES=assert.exception=1, zend.assertions=1, opcache.enable=1, opcache.enable_cli=1, opcache.optimization_level=-1, opcache.jit=1205, opcache.jit_buffer_size=4096M" >> $GITHUB_ENV
uses: actions/checkout@v4

- name: Install PHP with extensions
uses: shivammathur/setup-php@v2
Expand All @@ -54,17 +47,14 @@ jobs:
if: matrix.os == 'ubuntu-latest'
run: echo "COMPOSER_CACHE_DIR=$(composer config cache-dir)" >> $GITHUB_ENV

- name: Determine composer cache directory on Windows
if: matrix.os == 'windows-latest'
run: echo "COMPOSER_CACHE_DIR=~\AppData\Local\Composer" >> $GITHUB_ENV

- name: Cache dependencies installed with composer
uses: actions/cache@v1
uses: actions/cache@v4
with:
path: ${{ env.COMPOSER_CACHE_DIR }}
key: php${{ matrix.php-version }}-composer-${{ matrix.dependencies }}-${{ hashFiles('**/composer.json') }}
restore-keys: |
php${{ matrix.php-version }}-composer-${{ matrix.dependencies }}-
- name: Install lowest dependencies with composer
if: matrix.dependencies == 'lowest'
run: composer update --no-ansi --no-interaction --no-progress --prefer-lowest
Expand All @@ -80,6 +70,6 @@ jobs:
run: vendor/bin/phpunit --coverage-clover=coverage.xml

- name: Send code coverage report to Codecov.io
uses: codecov/codecov-action@v1
uses: codecov/codecov-action@v5
with:
token: ${{ secrets.CODECOV_TOKEN }}
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1 +1,4 @@
/.idea/
vendor
Packages
composer.lock
12 changes: 6 additions & 6 deletions Classes/Authentication/OpenIdConnectProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,10 @@ public function authenticate(TokenInterface $authenticationToken): void
throw new UnsupportedAuthenticationTokenException(sprintf('The OpenID Connect authentication provider cannot authenticate the given token of type %s.', get_class($authenticationToken)), 1559805996);
}
if (!isset($this->options['roles']) && !isset($this->options['rolesFromClaims']) && !isset($this->options['addRolesFromExistingAccount'])) {
throw new \RuntimeException(sprintf('Either "roles", "rolesFromClaims" or "addRolesFromExistingAccount" must be specified in the configuration of OpenID Connect authentication provider'), 1559806095);
throw new \RuntimeException('Either "roles", "rolesFromClaims" or "addRolesFromExistingAccount" must be specified in the configuration of OpenID Connect authentication provider', 1559806095);
}
if (!isset($this->options['serviceName'])) {
throw new \RuntimeException(sprintf('Missing "serviceName" option in the configuration of OpenID Connect authentication provider'), 1561480057);
throw new \RuntimeException('Missing "serviceName" option in the configuration of OpenID Connect authentication provider', 1561480057);
}
if (!isset($this->options['accountIdentifierTokenValueName'])) {
$this->options['accountIdentifierTokenValueName'] = 'sub';
Expand All @@ -103,7 +103,7 @@ public function authenticate(TokenInterface $authenticationToken): void
throw new SecurityException('Open ID Connect: The identity token provided by the OIDC provider had an invalid signature', 1561479176);
}
$this->logger->debug(sprintf('OpenID Connect: Successfully verified signature of identity token with %s value "%s"', $this->options['accountIdentifierTokenValueName'], $identityToken->values[$this->options['accountIdentifierTokenValueName']] ?? 'unknown'), LogEnvironment::fromMethodName(__METHOD__));
} catch (SecurityException\AuthenticationRequiredException $exception) {
} catch (SecurityException\AuthenticationRequiredException) {
$authenticationToken->setAuthenticationStatus(TokenInterface::AUTHENTICATION_NEEDED);
return;
} catch (SecurityException $exception) {
Expand Down Expand Up @@ -261,13 +261,13 @@ private function getConfiguredRoles(IdentityToken $identityToken): array
$this->logger->error(sprintf('OpenID Connect: Failed using account identifier from from identity token (%s) because the configured claim "%s" does not exist.', $identityToken->values['sub'] ?? '', $this->options['accountIdentifierTokenValueName']), LogEnvironment::fromMethodName(__METHOD__));
} else {
$existingAccount = $this->accountRepository->findActiveByAccountIdentifierAndAuthenticationProviderName($accountIdentifier, $this->name);
if (!$existingAccount instanceof Account) {
$this->logger->notice(sprintf('OpenID Connect: Could not add roles from existing account for identity token (%s) because the account "%s" (provider: %s) does not exist.', $identityToken->values['sub'] ?? '', $accountIdentifier, $this->name), LogEnvironment::fromMethodName(__METHOD__));
} else {
if ($existingAccount instanceof Account) {
foreach ($existingAccount->getRoles() as $role) {
$roleIdentifiers[] = $role->getIdentifier();
}
$this->logger->debug(sprintf('OpenID Connect: Added roles (identity token %s) from existing account "%s"', $identityToken->values['sub'] ?? '', $existingAccount->getAccountIdentifier()), LogEnvironment::fromMethodName(__METHOD__));
} else {
$this->logger->notice(sprintf('OpenID Connect: Could not add roles from existing account for identity token (%s) because the account "%s" (provider: %s) does not exist.', $identityToken->values['sub'] ?? '', $accountIdentifier, $this->name), LogEnvironment::fromMethodName(__METHOD__));
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions Classes/Authentication/OpenIdConnectToken.php
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,13 @@ public function extractIdentityTokenFromRequest(string $cookieName): IdentityTok
*/
private function extractIdentityTokenFromAuthorizationHeader(string $authorizationHeader): IdentityToken
{
if (strpos($this->authorizationHeader, 'Bearer ') !== 0) {
if (!str_starts_with($authorizationHeader, 'Bearer ')) {
$this->setAuthenticationStatus(TokenInterface::NO_CREDENTIALS_GIVEN);
throw new AuthenticationRequiredException('Could not extract access token from Authorization header: "Bearer" keyword is missing', 1589283608);
}

try {
$jwt = substr($this->authorizationHeader, strlen('Bearer '));
$jwt = substr($authorizationHeader, strlen('Bearer '));
$identityToken = IdentityToken::fromJwt($jwt);
} catch (\InvalidArgumentException $exception) {
$this->setAuthenticationStatus(TokenInterface::WRONG_CREDENTIALS);
Expand Down
24 changes: 9 additions & 15 deletions Classes/Authentication/TokenArguments.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ public static function fromSignedString(string $encodedString): TokenArguments
try {
$payloadAsString = $returnArguments->hashService->validateAndStripHmac(base64_decode($encodedString));
$returnArguments->payload = json_decode($payloadAsString, true);
} catch (InvalidHashException $exception) {
throw new \InvalidArgumentException(sprintf('OpenID Connect: The token arguments were appended by an invalid HMAC'), 1560165515);
} catch (InvalidHashException) {
throw new \InvalidArgumentException('OpenID Connect: The token arguments were appended by an invalid HMAC', 1560165515);
}

if (!is_array($returnArguments->payload)) {
throw new \InvalidArgumentException(sprintf('OpenID Connect: Failed decoding token arguments payload from given string'), 1560162452);
throw new \InvalidArgumentException('OpenID Connect: Failed decoding token arguments payload from given string', 1560162452);
}
return $returnArguments;
}
Expand Down Expand Up @@ -78,11 +78,9 @@ public function offsetExists($offset): bool
* @param mixed $offset
* @return mixed
*/
public function offsetGet($offset)
public function offsetGet($offset): mixed
{
if (isset($this->payload[$offset])) {
return $this->payload[$offset];
}
return $this->payload[$offset] ?? null;
}

/**
Expand All @@ -91,14 +89,10 @@ public function offsetGet($offset)
*/
public function offsetSet($offset, $value): void
{
switch($offset) {
case self::AUTHORIZATION_ID:
case self::SERVICE_NAME:
$this->payload[$offset] = $value;
break;
default:
throw new \InvalidArgumentException(sprintf('OpenID Connect: Invalid argument name "%s" for token arguments', $offset), 1560162220);
}
$this->payload[$offset] = match ($offset) {
self::AUTHORIZATION_ID, self::SERVICE_NAME => $value,
default => throw new \InvalidArgumentException(sprintf('OpenID Connect: Invalid argument name "%s" for token arguments', $offset), 1560162220),
};
}

/**
Expand Down
2 changes: 1 addition & 1 deletion Classes/Command/OidcCommandController.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public function discoverCommand(string $serviceName = null): void
exit(1);
}

$discoveredOptions = \GuzzleHttp\json_decode($response->getBody()->getContents(), true);
$discoveredOptions = json_decode($response->getBody()->getContents(), true);
if (!is_array($discoveredOptions)) {
$this->outputLine('<error>Discovery endpoint returned invalid response</error>');
exit(1);
Expand Down
10 changes: 0 additions & 10 deletions Classes/Http/SetJwtCookieMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
use Flownative\OpenIdConnect\Client\OAuthClient;
use GuzzleHttp\Psr7\Query;
use GuzzleHttp\Psr7\Utils;
use Neos\Flow\Http\Component\ComponentContext;
use Neos\Flow\Http\Component\TrustedProxiesComponent;
use Neos\Flow\Http\Cookie;
use Neos\Flow\Log\Utility\LogEnvironment;
use Neos\Flow\Security\Context as SecurityContext;
Expand Down Expand Up @@ -156,14 +154,6 @@ private function removeJwtCookie(ResponseInterface $response, string $cookieName
*/
private function withRedirectToRemoveOidcQueryParameters(ServerRequestInterface $request, ResponseInterface $response): ResponseInterface
{
// Provide backwards-compatibility with Flow 6.3 and earlier, where the Trusted Proxies Middleware did not exist yet:
if (class_exists(TrustedProxiesComponent::class) && class_exists(ComponentContext::class) && $this->options['disableTrustedProxiesComponentCompatibility'] === false) {
$componentContext = new ComponentContext($request, $response);
$trustedProxiesComponent = new TrustedProxiesComponent();
$trustedProxiesComponent->handle($componentContext);
$request = $componentContext->getHttpRequest();
}

$queryParameters = Query::parse($request->getUri()->getQuery());
$authorizationIdQueryParameterName = OAuthClient::generateAuthorizationIdQueryParameterName(OAuthClient::SERVICE_TYPE);
if (!isset($queryParameters[OpenIdConnectToken::OIDC_PARAMETER_NAME]) && !isset($queryParameters[$authorizationIdQueryParameterName])) {
Expand Down
5 changes: 3 additions & 2 deletions Classes/IdentityToken.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
use JsonException;
use Lcobucci\JWT\Encoding\JoseEncoder;
use Lcobucci\JWT\Token;
use Neos\Utility\Arrays;
use phpseclib3\Crypt\PublicKeyLoader;
use phpseclib3\Crypt\RSA;
use phpseclib3\Math\BigInteger;
Expand Down Expand Up @@ -157,7 +158,7 @@ public function isExpiredAt(\DateTimeInterface $now): bool
*/
public function scopeContains(string $scopeIdentifier): bool
{
$scopeIdentifiers = \Neos\Utility\Arrays::trimExplode(',', $this->values['scope'] ?? '');
$scopeIdentifiers = Arrays::trimExplode(',', $this->values['scope'] ?? '');
return in_array($scopeIdentifier, $scopeIdentifiers, true);
}

Expand Down Expand Up @@ -200,7 +201,7 @@ private function getMatchingKeyForJws(array $keys, string $algorithm, ?string $k
if ($keyIdentifier === null || !isset($key['kid']) || $key['kid'] === $keyIdentifier) {
return $key;
}
} else if (isset($key['alg']) && $key['alg'] === $algorithm && $key['kid'] === $keyIdentifier) {
} elseif (isset($key['alg']) && $key['alg'] === $algorithm && $key['kid'] === $keyIdentifier) {
return $key;
}
}
Expand Down
6 changes: 3 additions & 3 deletions Classes/OAuthClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public function getBaseUri(): string
{
$this->initializeOptionsIfNeeded();
if (!isset($this->options['issuer'])) {
throw new ConfigurationException(sprintf('Missing configuration issuer for service "%s" (%s). Either configure it explicitly via settings or make sure that auto-discovery returns "issuer".', $this->getServiceName(), $this->getServiceType()), 1559028091);
throw new ConfigurationException(sprintf('Missing configuration issuer for service "%s" (%s). Either configure it explicitly via settings or make sure that auto-discovery returns "issuer".', $this->getServiceName(), self::getServiceType()), 1559028091);
}
return $this->options['issuer'];
}
Expand All @@ -71,7 +71,7 @@ public function getAuthorizeTokenUri(): string
{
$this->initializeOptionsIfNeeded();
if (!isset($this->options['authorizationEndpoint']) || !is_string($this->options['authorizationEndpoint'])) {
throw new ConfigurationException(sprintf('Missing configuration authorizationEndpoint for service "%s" (%s). Either configure it explicitly via settings or make sure that auto-discovery returns "authorization_endpoint".', $this->getServiceName(), $this->getServiceType()), 1558612617);
throw new ConfigurationException(sprintf('Missing configuration authorizationEndpoint for service "%s" (%s). Either configure it explicitly via settings or make sure that auto-discovery returns "authorization_endpoint".', $this->getServiceName(), self::getServiceType()), 1558612617);
}
return $this->options['authorizationEndpoint'];
}
Expand All @@ -86,7 +86,7 @@ public function getAccessTokenUri(): string
{
$this->initializeOptionsIfNeeded();
if (!isset($this->options['tokenEndpoint']) || !is_string($this->options['tokenEndpoint'])) {
throw new ConfigurationException(sprintf('Missing configuration tokenEndpoint for service "%s" (%s). Either configure it explicitly via settings or make sure that auto-discovery returns "token_endpoint".', $this->getServiceName(), $this->getServiceType()), 1558615098);
throw new ConfigurationException(sprintf('Missing configuration tokenEndpoint for service "%s" (%s). Either configure it explicitly via settings or make sure that auto-discovery returns "token_endpoint".', $this->getServiceName(), self::getServiceType()), 1558615098);
}
return $this->options['tokenEndpoint'];
}
Expand Down
6 changes: 3 additions & 3 deletions Classes/OpenIdConnectClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ public function getAccessToken(string $serviceName, string $clientId, string $cl
public function startAuthorization(UriInterface $returnToUri, string $scope): UriInterface
{
$returnArguments = (string)TokenArguments::fromArray([TokenArguments::SERVICE_NAME => $this->serviceName]);
if (strpos($returnArguments, 'ERROR') === 0) {
if (str_starts_with($returnArguments, 'ERROR')) {
throw new \RuntimeException(substr($returnArguments, 6));
}
$returnToUri = $returnToUri->withQuery(trim($returnToUri->getQuery() . '&' . OpenIdConnectToken::OIDC_PARAMETER_NAME . '=' . urlencode($returnArguments), '&'));
Expand Down Expand Up @@ -288,7 +288,7 @@ public function getJwks(): array
throw new ConnectionException(sprintf('OpenID Connect Client: Failed retrieving JWKS from %s: %s', $this->options['jwksUri'], $e->getMessage()), 1559211266);
}

$response = \GuzzleHttp\json_decode($response->getBody()->getContents(), true);
$response = json_decode($response->getBody()->getContents(), true);
if (!is_array($response) || !isset($response['keys'])) {
throw new ServiceException(sprintf('OpenID Connect Client: Failed decoding response while retrieving JWKS from %s', $this->options['jwksUri']), 1559211340);
}
Expand All @@ -315,7 +315,7 @@ private function amendOptionsWithDiscovery(string $discoveryUri): void
}
$discoveredOptions = \GuzzleHttp\json_decode($response->getBody()->getContents(), true);
if (!is_array($discoveredOptions)) {
throw new ConnectionException(sprintf('OpenID Connect Client: Discovery endpoint returned invalid response.'), 1554903349);
throw new ConnectionException('OpenID Connect Client: Discovery endpoint returned invalid response.', 1554903349);
}
$this->discoveryCache->set($cacheIdentifier, $discoveredOptions);
$this->logger->info(sprintf('OpenID Connect Client: Auto-discovery via %s succeeded and stored into cache.', $discoveryUri), LogEnvironment::fromMethodName(__METHOD__));
Expand Down
Loading

0 comments on commit 54b2cbe

Please sign in to comment.