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

Updates + compatibility table #57

Merged
merged 1 commit into from
Jun 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 6 additions & 12 deletions .github/workflows/audit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:
uses: actions/checkout@v4
if: github.event_name == 'pull_request'
with:
ref: ${{ github.event.pull_request.head.ref }}
ref: ${{ github.event.pull_request.head.sha }}
fetch-depth: 0

- name: Setup PHP
Expand All @@ -38,29 +38,23 @@ jobs:
run: echo "DIR=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT

- name: Cache dependencies
uses: actions/cache@v3
uses: actions/cache@v4
with:
path: ${{ steps.composer-cache.outputs.DIR }}
key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.json') }}
restore-keys: ${{ runner.os }}-composer-

- name: Install dependencies
run: |
echo "::group::composer install"
composer install --no-interaction
echo "::endgroup::"
echo "::group::phpunit install"
vendor/bin/simple-phpunit install
echo "::endgroup::"
run: composer install --no-interaction

- name: Auditor
uses: docker://nbgrp/auditor:0.23.1
uses: docker://nbgrp/auditor:0.26.0

- name: Tests
run: vendor/bin/simple-phpunit
run: vendor/bin/phpunit

- name: Codecov
uses: codecov/codecov-action@v3
uses: codecov/codecov-action@v4
with:
token: ${{ secrets.CODECOV_TOKEN }}
files: ./clover.xml
7 changes: 7 additions & 0 deletions .php-cs-fixer.php
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,13 @@
'function',
],
],
'phpdoc_order' => [
'order' => [
'param',
'throws',
'return',
],
],
'phpdoc_separation' => false,
'phpdoc_summary' => false,
'phpdoc_to_comment' => false,
Expand Down
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@
> use [hslavich/oneloginsaml-bundle](https://github.com/hslavich/OneloginSamlBundle)
> which this bundle based on.

### Compatibility
| Branch | Symfony |
|---------|-----------|
| 1.x | Symfony 6 |
| **2.x** | Symfony 7 |

## Installation

```
Expand Down
4 changes: 2 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
},
"require-dev": {
"doctrine/orm": "^2.3 || ^3",
"symfony/event-dispatcher": "^7",
"symfony/phpunit-bridge": "^7"
"phpunit/phpunit": "^11.2",
"symfony/event-dispatcher": "^7"
},
"autoload": {
"psr-4": {
Expand Down
2 changes: 0 additions & 2 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ parameters:
paths:
- src
- tests
bootstrapFiles:
- vendor/bin/.phpunit/phpunit/vendor/autoload.php

checkMissingIterableValueType: false
checkGenericClassInNonGenericObjectType: false
60 changes: 26 additions & 34 deletions phpunit.xml.dist
Original file line number Diff line number Diff line change
@@ -1,39 +1,31 @@
<?xml version="1.0" encoding="UTF-8"?>
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/11.2/phpunit.xsd" colors="true" bootstrap="vendor/autoload.php" cacheDirectory=".phpunit.cache">
<php>
<ini name="error_reporting" value="-1"/>
<server name="SYMFONY_PHPUNIT_REMOVE" value=""/>
<server name="SYMFONY_PHPUNIT_VERSION" value="11.2"/>
<env name="SYMFONY_DEPRECATIONS_HELPER" value="max[indirect]=1"/>
</php>

<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="vendor/bin/.phpunit/phpunit.xsd"
colors="true"
bootstrap="vendor/autoload.php"
>
<testsuites>
<testsuite name="Onelogin SAML Bundle Test Suite">
<directory>./tests/</directory>
</testsuite>
</testsuites>

<php>
<ini name="error_reporting" value="-1" />
<server name="SYMFONY_PHPUNIT_VERSION" value="9.5" />
<env name="SYMFONY_DEPRECATIONS_HELPER" value="max[indirect]=1" />
</php>
<coverage>
<report>
<clover outputFile="clover.xml"/>
</report>
</coverage>

<testsuites>
<testsuite name="Onelogin SAML Bundle Test Suite">
<directory>./tests/</directory>
</testsuite>
</testsuites>

<coverage cacheDirectory=".phpunit.cache/code-coverage"
processUncoveredFiles="true"
>
<include>
<directory suffix=".php">src</directory>
</include>
<exclude>
<directory>src/Resources</directory>
<file>src/NbgrpOneloginSamlBundle.php</file>
</exclude>
<report>
<clover outputFile="clover.xml" />
</report>
</coverage>

<listeners>
<listener class="Symfony\Bridge\PhpUnit\SymfonyTestsListener" />
</listeners>
<source>
<include>
<directory suffix=".php">src</directory>
</include>
<exclude>
<directory>src/Resources</directory>
<file>src/NbgrpOneloginSamlBundle.php</file>
</exclude>
</source>
</phpunit>
12 changes: 5 additions & 7 deletions src/Controller/Login.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@
use Symfony\Component\Security\Http\SecurityRequestAttributes;

#[AsController]
class Login
readonly class Login
{
public function __construct(
private readonly FirewallMap $firewallMap,
private FirewallMap $firewallMap,
) {}

public function __invoke(Request $request, Auth $auth): RedirectResponse
Expand Down Expand Up @@ -51,7 +51,7 @@ public function __invoke(Request $request, Auth $auth): RedirectResponse
private function getTargetPath(Request $request, SessionInterface $session): ?string
{
$firewallName = $this->firewallMap->getFirewallConfig($request)?->getName();
if (!$firewallName) {
if ($firewallName === null || $firewallName === '') {
throw new ServiceUnavailableHttpException(message: 'Unknown firewall.');
}

Expand All @@ -62,15 +62,13 @@ private function getTargetPath(Request $request, SessionInterface $session): ?st
private function processLoginAndGetRedirectUrl(Auth $auth, ?string $targetPath, ?SessionInterface $session): string
{
$redirectUrl = $auth->login(returnTo: $targetPath, stay: true);
if ($redirectUrl === null) {
throw new \RuntimeException('Login cannot be performed: Auth did not returned redirect url.');
}

$security = $auth->getSettings()->getSecurityData();
if (($security['rejectUnsolicitedResponsesWithInResponseTo'] ?? false) && $session instanceof SessionInterface) {
if (($security['rejectUnsolicitedResponsesWithInResponseTo'] ?? false) !== false && $session instanceof SessionInterface) {
$session->set(SamlAuthenticator::LAST_REQUEST_ID, $auth->getLastRequestID());
}

// @phan-suppress-next-line PhanPossiblyNullTypeReturn
return $redirectUrl;
}
}
25 changes: 14 additions & 11 deletions src/EventListener/Security/SamlLogoutListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,18 @@
/**
* Process Single Logout by current OneLogin Auth service on user logout.
*/
final class SamlLogoutListener
final readonly class SamlLogoutListener
{
public function __construct(
private readonly AuthRegistryInterface $authRegistry,
private readonly IdpResolverInterface $idpResolver,
private AuthRegistryInterface $authRegistry,
private IdpResolverInterface $idpResolver,
) {}

#[AsEventListener(LogoutEvent::class)]
public function processSingleLogout(LogoutEvent $event): void
{
$authService = $this->getAuthService($event->getRequest());
if (!$authService) {
if ($authService === null) {
return;
}

Expand All @@ -40,20 +40,23 @@
try {
$authService->processSLO();
} catch (\OneLogin\Saml2\Error) {
if (!empty($authService->getSLOurl())) {
/** @var string|null $sessionIndex */
$sessionIndex = $token->hasAttribute(SamlAuthenticator::SESSION_INDEX_ATTRIBUTE)
? $token->getAttribute(SamlAuthenticator::SESSION_INDEX_ATTRIBUTE)
: null;
$authService->logout(null, [], $token->getUserIdentifier(), $sessionIndex);
$sloUrl = $authService->getSLOurl();
if ($sloUrl === null || $sloUrl === '') {
return;

Check warning on line 45 in src/EventListener/Security/SamlLogoutListener.php

View check run for this annotation

Codecov / codecov/patch

src/EventListener/Security/SamlLogoutListener.php#L45

Added line #L45 was not covered by tests
}

/** @var string|null $sessionIndex */
$sessionIndex = $token->hasAttribute(SamlAuthenticator::SESSION_INDEX_ATTRIBUTE)
? $token->getAttribute(SamlAuthenticator::SESSION_INDEX_ATTRIBUTE)
: null;
$authService->logout(null, [], $token->getUserIdentifier(), $sessionIndex);
}
}

private function getAuthService(Request $request): ?Auth
{
$idp = $this->idpResolver->resolve($request);
if (!$idp) {
if ($idp === null || $idp === '') {
return $this->authRegistry->getDefaultService();
}

Expand Down
4 changes: 2 additions & 2 deletions src/Idp/IdpResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@

use Symfony\Component\HttpFoundation\Request;

final class IdpResolver implements IdpResolverInterface
final readonly class IdpResolver implements IdpResolverInterface
{
public function __construct(
private readonly string $idpParameterName,
private string $idpParameterName,
) {}

public function resolve(Request $request): ?string
Expand Down
10 changes: 5 additions & 5 deletions src/Onelogin/AuthArgumentResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
* Yields the OneLogin Auth instance for current request
* (default or according to an idp parameter).
*/
final class AuthArgumentResolver implements ValueResolverInterface
final readonly class AuthArgumentResolver implements ValueResolverInterface
{
public function __construct(
private readonly AuthRegistryInterface $authRegistry,
private readonly IdpResolverInterface $idpResolver,
private AuthRegistryInterface $authRegistry,
private IdpResolverInterface $idpResolver,
) {}

public function resolve(Request $request, ArgumentMetadata $argument): iterable
Expand All @@ -31,12 +31,12 @@ public function resolve(Request $request, ArgumentMetadata $argument): iterable
}

$idp = $this->idpResolver->resolve($request);
if ($idp && !$this->authRegistry->hasService($idp)) {
if (($idp !== null && $idp !== '') && !$this->authRegistry->hasService($idp)) {
throw new BadRequestHttpException('There is no OneLogin PHP toolkit settings for IdP "'.$idp.'". See nbgrp_onelogin_saml config ("onelogin_settings" section).');
}

try {
yield $idp
yield ($idp !== null && $idp !== '')
? $this->authRegistry->getService($idp)
: $this->authRegistry->getDefaultService();
} catch (\RuntimeException $exception) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@
/**
* Allows to add SAML attributes to a passport.
*/
class SamlAttributesBadge implements BadgeInterface
readonly class SamlAttributesBadge implements BadgeInterface
{
public function __construct(
private readonly array $attributes,
private array $attributes,
) {}

public function getAttributes(): array
Expand Down
8 changes: 4 additions & 4 deletions src/Security/Http/Authenticator/SamlAuthenticator.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public function start(Request $request, ?AuthenticationException $authException
{
$uri = $this->httpUtils->generateUri($request, (string) $this->options['login_path']);
$idp = $this->idpResolver->resolve($request);
if ($idp) {
if ($idp !== null && $idp !== '') {
$uri .= '?'.$this->idpParameterName.'='.$idp;
}

Expand Down Expand Up @@ -126,7 +126,7 @@ protected function processResponse(Auth $oneLoginAuth, SessionInterface $session
{
$requestId = null;
$security = $oneLoginAuth->getSettings()->getSecurityData();
if ($security['rejectUnsolicitedResponsesWithInResponseTo'] ?? false) {
if (($security['rejectUnsolicitedResponsesWithInResponseTo'] ?? false) !== false) {
/** @var string $requestId */
$requestId = $session->get(self::LAST_REQUEST_ID);
}
Expand Down Expand Up @@ -179,7 +179,7 @@ function (string $identifier) use ($deferredEventBadge, $attributes) {

protected function extractAttributes(Auth $oneLoginAuth): array
{
$attributes = $this->options['use_attribute_friendly_name'] ?? false
$attributes = ($this->options['use_attribute_friendly_name'] ?? false) !== false
? $oneLoginAuth->getAttributesWithFriendlyName()
: $oneLoginAuth->getAttributes();
$attributes[self::SESSION_INDEX_ATTRIBUTE] = $oneLoginAuth->getSessionIndex();
Expand Down Expand Up @@ -215,7 +215,7 @@ private function getOneLoginAuth(Request $request): Auth
{
try {
$idp = $this->idpResolver->resolve($request);
$authService = $idp
$authService = ($idp !== null && $idp !== '')
? $this->authRegistry->getService($idp)
: $this->authRegistry->getDefaultService();
} catch (\RuntimeException $exception) {
Expand Down
6 changes: 3 additions & 3 deletions src/Security/User/SamlUserFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@

use Symfony\Component\Security\Core\User\UserInterface;

final class SamlUserFactory implements SamlUserFactoryInterface
final readonly class SamlUserFactory implements SamlUserFactoryInterface
{
/**
* @param class-string<UserInterface> $userClass
* @param array<string, mixed> $mapping
*/
public function __construct(
private readonly string $userClass,
private readonly array $mapping,
private string $userClass,
private array $mapping,
) {}

public function createUser(string $identifier, array $attributes): UserInterface
Expand Down
Loading