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

Start testing with SSP 2.3 #268

Merged
merged 5 commits into from
Dec 6, 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
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ PHP version requirement changes in minor releases for SimpleSAMLphp.

| OIDC module | Tested SimpleSAMLphp | PHP | Note |
|:------------|:---------------------|:------:|-----------------------------|
| v6.\* | v2.2.\* | \>=8.2 | Recommended |
| v6.\* | v2.3.\* | \>=8.2 | Recommended |
| v5.\* | v2.1.\* | \>=8.1 | |
| v4.\* | v2.0.\* | \>=8.0 | |
| v3.\* | v2.0.\* | \>=7.4 | Abandoned from August 2023. |
Expand Down Expand Up @@ -329,7 +329,7 @@ docker run --name ssp-oidc-dev \
--mount type=bind,source="$(pwd)/docker/ssp/oidc_module.crt",target=/var/simplesamlphp/cert/oidc_module.crt,readonly \
--mount type=bind,source="$(pwd)/docker/ssp/oidc_module.key",target=/var/simplesamlphp/cert/oidc_module.key,readonly \
--mount type=bind,source="$(pwd)/docker/apache-override.cf",target=/etc/apache2/sites-enabled/ssp-override.cf,readonly \
-p 443:443 cirrusid/simplesamlphp:v2.2.2
-p 443:443 cirrusid/simplesamlphp:v2.3.5
```

Visit https://localhost/simplesaml/ and confirm you get the default page.
Expand Down
2 changes: 1 addition & 1 deletion UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ has been refactored:

- upgraded to v5 of lcobucci/jwt https://github.com/lcobucci/jwt
- upgraded to v3 of laminas/laminas-diactoros https://github.com/laminas/laminas-diactoros
- SimpleSAMLphp version used during development was bumped to v2.2
- SimpleSAMLphp version used during development was bumped to v2.3
- In Authorization Code Flow, a new validation was added which checks for 'openid' value in 'scope' parameter. Up to
now, 'openid' value was dynamically added if not present. In Implicit Code Flow this validation was already present.

Expand Down
7 changes: 4 additions & 3 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
"friendsofphp/php-cs-fixer": "^3",
"phpunit/phpunit": "^10",
"rector/rector": "^0.18.3",
"simplesamlphp/simplesamlphp": "2.2.*",
"simplesamlphp/simplesamlphp": "2.3.*",
"simplesamlphp/simplesamlphp-test-framework": "^1.5",
"squizlabs/php_codesniffer": "^3",
"vimeo/psalm": "^5",
Expand All @@ -56,9 +56,10 @@
},
"sort-packages": true,
"allow-plugins": {
"simplesamlphp/composer-module-installer": true,
"dealerdirect/phpcodesniffer-composer-installer": true,
"phpstan/extension-installer": true
"phpstan/extension-installer": true,
"simplesamlphp/composer-module-installer": true,
"simplesamlphp/composer-xmlprovider-installer": true
},
"cache-dir": "build/composer"
},
Expand Down
4 changes: 2 additions & 2 deletions docker/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#FROM cirrusid/simplesamlphp:v2.2.2
FROM cicnavi/simplesamlphp:dev
FROM cirrusid/simplesamlphp:v2.3.5
#FROM cicnavi/simplesamlphp:dev

RUN apt-get update && apt-get install -y sqlite3
# Prepopulate the DB with items needed for testing
Expand Down
2 changes: 1 addition & 1 deletion src/Admin/Menu.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public function __construct(Item ...$items)
array_push($this->items, ...$items);
}

public function addItem(Item $menuItem, int $offset = null): void
public function addItem(Item $menuItem, ?int $offset = null): void
{
$offset ??= count($this->items);

Expand Down
10 changes: 5 additions & 5 deletions src/Entities/AccessTokenEntity.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,11 @@ public function __construct(
DateTimeImmutable $expiryDateTime,
CryptKey $privateKey,
protected JsonWebTokenBuilderService $jsonWebTokenBuilderService,
int|string $userIdentifier = null,
string $authCodeId = null,
array $requestedClaims = null,
bool $isRevoked = false,
Configuration $jwtConfiguration = null,
int|string|null $userIdentifier = null,
?string $authCodeId = null,
?array $requestedClaims = null,
?bool $isRevoked = false,
?Configuration $jwtConfiguration = null,
) {
$this->setIdentifier($id);
$this->setClient($clientEntity);
Expand Down
6 changes: 3 additions & 3 deletions src/Entities/AuthCodeEntity.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ public function __construct(
OAuth2ClientEntityInterface $client,
array $scopes,
DateTimeImmutable $expiryDateTime,
string $userIdentifier = null,
string $redirectUri = null,
string $nonce = null,
?string $userIdentifier = null,
?string $redirectUri = null,
?string $nonce = null,
bool $isRevoked = false,
) {
$this->identifier = $id;
Expand Down
8 changes: 4 additions & 4 deletions src/Factories/Entities/AccessTokenEntityFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ public function fromData(
OAuth2ClientEntityInterface $clientEntity,
array $scopes,
DateTimeImmutable $expiryDateTime,
int|string $userIdentifier = null,
string $authCodeId = null,
array $requestedClaims = null,
bool $isRevoked = false,
int|string|null $userIdentifier = null,
?string $authCodeId = null,
?array $requestedClaims = null,
?bool $isRevoked = false,
): AccessTokenEntity {
return new AccessTokenEntity(
$id,
Expand Down
6 changes: 3 additions & 3 deletions src/Factories/Entities/AuthCodeEntityFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ public function fromData(
OAuth2ClientEntityInterface $client,
array $scopes,
DateTimeImmutable $expiryDateTime,
string $userIdentifier = null,
string $redirectUri = null,
string $nonce = null,
?string $userIdentifier = null,
?string $redirectUri = null,
?string $nonce = null,
bool $isRevoked = false,
): AuthCodeEntity {
return new AuthCodeEntity(
Expand Down
4 changes: 2 additions & 2 deletions src/Factories/Entities/ScopeEntityFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ class ScopeEntityFactory
*/
public function fromData(
string $identifier,
string $description = null,
string $icon = null,
?string $description = null,
?string $icon = null,
array $claims = [],
): ScopeEntity {
return new ScopeEntity(
Expand Down
2 changes: 1 addition & 1 deletion src/Factories/TemplateFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public function __construct(
public function build(
string $templateName,
array $data = [],
string $activeHrefPath = null,
?string $activeHrefPath = null,
?bool $includeDefaultMenuItems = null,
?bool $showMenu = null,
?bool $showModuleName = null,
Expand Down
4 changes: 2 additions & 2 deletions src/ModuleConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ class ModuleConfig
public function __construct(
string $fileName = self::DEFAULT_FILE_NAME, // Primarily used for easy (unit) testing overrides.
array $overrides = [], // Primarily used for easy (unit) testing overrides.
Configuration $sspConfig = null,
?Configuration $sspConfig = null,
private readonly SspBridge $sspBridge = new SspBridge(),
) {
$this->moduleConfig = Configuration::loadFromArray(
Expand Down Expand Up @@ -225,7 +225,7 @@ public function config(): Configuration
}

// TODO mivanci Move to dedicated \SimpleSAML\Module\oidc\Utils\Routes::getModuleUrl
public function getModuleUrl(string $path = null): string
public function getModuleUrl(?string $path = null): string
{
$base = $this->sspBridge->module()->getModuleURL(self::MODULE_NAME);

Expand Down
8 changes: 4 additions & 4 deletions src/Repositories/AccessTokenRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,10 @@ public function getNewToken(
OAuth2ClientEntityInterface $clientEntity,
array $scopes,
$userIdentifier = null,
string $authCodeId = null,
array $requestedClaims = null,
string $id = null,
DateTimeImmutable $expiryDateTime = null,
?string $authCodeId = null,
?array $requestedClaims = null,
?string $id = null,
?DateTimeImmutable $expiryDateTime = null,
): AccessTokenEntityInterface {
if (!is_null($userIdentifier)) {
$userIdentifier = (string)$userIdentifier;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public function getNewToken(
OAuth2ClientEntityInterface $clientEntity,
array $scopes,
$userIdentifier = null,
string $authCodeId = null,
array $requestedClaims = null,
?string $authCodeId = null,
?array $requestedClaims = null,
): AccessTokenEntityInterface;
}
4 changes: 2 additions & 2 deletions src/Server/AuthorizationServer.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ public function __construct(
ScopeRepositoryInterface $scopeRepository,
CryptKey|string $privateKey,
Key|string $encryptionKey,
ResponseTypeInterface $responseType = null,
RequestRulesManager $requestRulesManager = null,
?ResponseTypeInterface $responseType = null,
?RequestRulesManager $requestRulesManager = null,
) {
parent::__construct(
$clientRepository,
Expand Down
68 changes: 39 additions & 29 deletions src/Server/Exceptions/OidcServerException.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use League\OAuth2\Server\Exception\OAuthServerException;
use Psr\Http\Message\ResponseInterface;
use SimpleSAML\OpenID\Codebooks\ErrorsEnum;
use Throwable;

use function http_build_query;
Expand Down Expand Up @@ -57,10 +58,10 @@ public function __construct(
int $code,
string $errorType,
int $httpStatusCode = 400,
string $hint = null,
string $redirectUri = null,
Throwable $previous = null,
string $state = null,
?string $hint = null,
?string $redirectUri = null,
?Throwable $previous = null,
?string $state = null,
) {
parent::__construct($message, $code, $errorType, $httpStatusCode, $hint, $redirectUri, $previous);

Expand Down Expand Up @@ -93,8 +94,8 @@ public function __construct(
* @return self
*/
public static function unsupportedResponseType(
string $redirectUri = null,
string $state = null,
?string $redirectUri = null,
?string $state = null,
bool $useFragment = false,
): OidcServerException {
$errorMessage = 'The response type is not supported by the authorization server.';
Expand All @@ -117,7 +118,7 @@ public static function unsupportedResponseType(
public static function invalidScope(
$scope,
$redirectUri = null,
string $state = null,
?string $state = null,
bool $useFragment = false,
): OidcServerException {
// OAuthServerException correctly implements this error, however, it misses state parameter.
Expand All @@ -142,9 +143,9 @@ public static function invalidScope(
public static function invalidRequest(
$parameter,
$hint = null,
Throwable $previous = null,
string $redirectUri = null,
string $state = null,
?Throwable $previous = null,
?string $redirectUri = null,
?string $state = null,
bool $useFragment = false,
): OidcServerException {
$e = parent::invalidRequest($parameter, $hint, $previous);
Expand All @@ -167,8 +168,8 @@ public static function invalidRequest(
public static function accessDenied(
$hint = null,
$redirectUri = null,
Throwable $previous = null,
string $state = null,
?Throwable $previous = null,
?string $state = null,
bool $useFragment = false,
): OidcServerException {
$e = parent::accessDenied($hint, $redirectUri, $previous);
Expand All @@ -190,10 +191,10 @@ public static function accessDenied(
* @return self
*/
public static function loginRequired(
string $hint = null,
string $redirectUri = null,
Throwable $previous = null,
string $state = null,
?string $hint = null,
?string $redirectUri = null,
?Throwable $previous = null,
?string $state = null,
bool $useFragment = false,
): OidcServerException {
$errorMessage = "End-User is not already authenticated.";
Expand All @@ -216,10 +217,10 @@ public static function loginRequired(
* @return self
*/
public static function requestNotSupported(
string $hint = null,
string $redirectUri = null,
Throwable $previous = null,
string $state = null,
?string $hint = null,
?string $redirectUri = null,
?Throwable $previous = null,
?string $state = null,
bool $useFragment = false,
): OidcServerException {
$errorMessage = "Request object not supported.";
Expand All @@ -239,21 +240,30 @@ public static function requestNotSupported(
* @return self
* @psalm-suppress LessSpecificImplementedReturnType
*/
public static function invalidRefreshToken($hint = null, Throwable $previous = null): OidcServerException
public static function invalidRefreshToken($hint = null, ?Throwable $previous = null): OidcServerException
{
return new self('The refresh token is invalid.', 8, 'invalid_grant', 400, $hint, null, $previous);
}

public static function invalidTrustChain(
string $hint = null,
string $redirectUri = null,
Throwable $previous = null,
string $state = null,
?string $hint = null,
?string $redirectUri = null,
?Throwable $previous = null,
?string $state = null,
bool $useFragment = false,
): OidcServerException {
$errorMessage = 'Trust chain validation failed.';

$e = new self($errorMessage, 12, 'trust_chain_validation_failed', 400, $hint, $redirectUri, $previous, $state);
$e = new self(
$errorMessage,
12,
ErrorsEnum::InvalidTrustChain->value,
400,
$hint,
$redirectUri,
$previous,
$state,
);
$e->useFragmentInHttpResponses($useFragment);

return $e;
Expand All @@ -268,7 +278,7 @@ public static function invalidTrustChain(
* @return self
* @psalm-suppress LessSpecificImplementedReturnType
*/
public static function forbidden(string $hint = null, Throwable $previous = null): OidcServerException
public static function forbidden(?string $hint = null, ?Throwable $previous = null): OidcServerException
{
return new self(
'Request understood, but refused to process it.',
Expand Down Expand Up @@ -304,7 +314,7 @@ public function setPayload(array $payload): void
/**
* @param string|null $redirectUri Set to string, or unset it with null
*/
public function setRedirectUri(string $redirectUri = null): void
public function setRedirectUri(?string $redirectUri = null): void
{
$this->redirectUri = $redirectUri;
}
Expand Down Expand Up @@ -337,7 +347,7 @@ public function getRedirectUri(): ?string
/**
* @param string|null $state Set to string, or unset it with null
*/
public function setState(string $state = null): void
public function setState(?string $state = null): void
{
if ($state === null) {
unset($this->payload['state']);
Expand Down
4 changes: 2 additions & 2 deletions src/Server/Grants/AuthCodeGrant.php
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ protected function issueOidcAuthCode(
string $userIdentifier,
string $redirectUri,
array $scopes = [],
string $nonce = null,
?string $nonce = null,
): AuthCodeEntityInterface {
$maxGenerationAttempts = self::MAX_RANDOM_TOKEN_GENERATION_ATTEMPTS;

Expand Down Expand Up @@ -748,7 +748,7 @@ public function validateAuthorizationRequestWithCheckerResultBag(
*/
protected function issueRefreshToken(
OAuth2AccessTokenEntityInterface $accessToken,
string $authCodeId = null,
?string $authCodeId = null,
): ?RefreshTokenEntityInterface {
if (! is_a($accessToken, AccessTokenEntityInterface::class)) {
throw OidcServerException::serverError('Unexpected access token entity type.');
Expand Down
4 changes: 2 additions & 2 deletions src/Server/Grants/Traits/IssueAccessTokenTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ protected function issueAccessToken(
ClientEntityInterface $client,
$userIdentifier = null,
array $scopes = [],
string $authCodeId = null,
array $requestedClaims = null,
?string $authCodeId = null,
?array $requestedClaims = null,
): AccessTokenEntityInterface {
$maxGenerationAttempts = AbstractGrant::MAX_RANDOM_TOKEN_GENERATION_ATTEMPTS;

Expand Down
2 changes: 1 addition & 1 deletion src/Server/LogoutHandlers/BackChannelLogoutHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public function __construct(
* @param \GuzzleHttp\HandlerStack|null $handlerStack For easier testing
* @throws \League\OAuth2\Server\Exception\OAuthServerException
*/
public function handle(array $relyingPartyAssociations, HandlerStack $handlerStack = null): void
public function handle(array $relyingPartyAssociations, ?HandlerStack $handlerStack = null): void
{
$clientConfig = ['timeout' => 3, 'verify' => false, 'handler' => $handlerStack];

Expand Down
Loading
Loading