Skip to content
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
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ through a SimpleSAMLphp module installable through Composer. It is based on
Currently supported flows are:
* Authorization Code flow, with PKCE support (response_type 'code')
* Implicit flow (response_type 'id_token token' or 'id_token')
* Plain OAuth2 Implicit flow (response_type 'token')
* Refresh Token flow

[![Build Status](https://github.com/simplesamlphp/simplesamlphp-module-oidc/actions/workflows/test.yaml/badge.svg)](https://github.com/simplesamlphp/simplesamlphp-module-oidc/actions/workflows/test.yaml)
Expand Down
4 changes: 3 additions & 1 deletion UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ known 'issue': https://github.com/symfony/symfony/issues/19693). If you don't se
about this situation in your logs.
- The new authproc filter processing will look in an additional location for filters, in the main `config.php` under
key `authproc.oidc`
- Removed support for plain OAuth2 Implicit flow (response_type `token`), because of very low usage. Note that the OIDC
Implicit flow is still supported (response_type `id_token token` or `id_token`).

## Low impact changes

Expand All @@ -95,7 +97,7 @@ has been refactored:
- Upgraded to v3 of laminas/laminas-diactoros https://github.com/laminas/laminas-diactoros
- 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.
now, 'openid' value was dynamically added if not present. In Implicit Code Flow this validation was already present.
- Removed importer from legacy OAuth2 module, as it is very unlikely that someone will upgrade from legacy OAuth2
module to v6 of oidc module. If needed, one can upgrade to earlier versions of oidc module, and then to v6.

Expand Down
2 changes: 0 additions & 2 deletions routing/services/services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,6 @@ services:
# Grants
SimpleSAML\Module\oidc\Server\Grants\AuthCodeGrant:
factory: ['@SimpleSAML\Module\oidc\Factories\Grant\AuthCodeGrantFactory', 'build']
SimpleSAML\Module\oidc\Server\Grants\OAuth2ImplicitGrant:
factory: ['@SimpleSAML\Module\oidc\Factories\Grant\OAuth2ImplicitGrantFactory', 'build']
SimpleSAML\Module\oidc\Server\Grants\ImplicitGrant:
factory: ['@SimpleSAML\Module\oidc\Factories\Grant\ImplicitGrantFactory', 'build']
SimpleSAML\Module\oidc\Server\Grants\RefreshTokenGrant:
Expand Down
7 changes: 0 additions & 7 deletions src/Factories/AuthorizationServerFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
use SimpleSAML\Module\oidc\Server\AuthorizationServer;
use SimpleSAML\Module\oidc\Server\Grants\AuthCodeGrant;
use SimpleSAML\Module\oidc\Server\Grants\ImplicitGrant;
use SimpleSAML\Module\oidc\Server\Grants\OAuth2ImplicitGrant;
use SimpleSAML\Module\oidc\Server\Grants\RefreshTokenGrant;
use SimpleSAML\Module\oidc\Server\RequestRules\RequestRulesManager;
use SimpleSAML\Module\oidc\Server\ResponseTypes\IdTokenResponse;
Expand All @@ -37,7 +36,6 @@ public function __construct(
private readonly AccessTokenRepository $accessTokenRepository,
private readonly ScopeRepository $scopeRepository,
private readonly AuthCodeGrant $authCodeGrant,
private readonly OAuth2ImplicitGrant $oAuth2ImplicitGrant,
private readonly ImplicitGrant $implicitGrant,
private readonly RefreshTokenGrant $refreshTokenGrant,
private readonly IdTokenResponse $idTokenResponse,
Expand All @@ -63,11 +61,6 @@ public function build(): AuthorizationServer
$this->moduleConfig->getAccessTokenDuration(),
);

$authorizationServer->enableGrantType(
$this->oAuth2ImplicitGrant,
$this->moduleConfig->getAccessTokenDuration(),
);

$authorizationServer->enableGrantType(
$this->implicitGrant,
$this->moduleConfig->getAccessTokenDuration(),
Expand Down
34 changes: 0 additions & 34 deletions src/Factories/Grant/OAuth2ImplicitGrantFactory.php

This file was deleted.

6 changes: 3 additions & 3 deletions src/Server/AuthorizationServer.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
use Psr\Http\Message\ServerRequestInterface;
use SimpleSAML\Error\BadRequest;
use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException;
use SimpleSAML\Module\oidc\Server\Grants\Interfaces\AuthorizationValidatableWithCheckerResultBagInterface;
use SimpleSAML\Module\oidc\Server\Grants\Interfaces\AuthorizationValidatableWithRequestRules;
use SimpleSAML\Module\oidc\Server\RequestRules\RequestRulesManager;
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\ClientIdRule;
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\IdTokenHintRule;
Expand Down Expand Up @@ -103,12 +103,12 @@

foreach ($this->enabledGrantTypes as $grantType) {
if ($grantType->canRespondToAuthorizationRequest($request)) {
if (! $grantType instanceof AuthorizationValidatableWithCheckerResultBagInterface) {
if (! $grantType instanceof AuthorizationValidatableWithRequestRules) {

Check warning on line 106 in src/Server/AuthorizationServer.php

View check run for this annotation

Codecov / codecov/patch

src/Server/AuthorizationServer.php#L106

Added line #L106 was not covered by tests
throw OidcServerException::serverError('grant type must be validatable with already validated ' .
'result bag');
}

return $grantType->validateAuthorizationRequestWithCheckerResultBag($request, $resultBag);
return $grantType->validateAuthorizationRequestWithRequestRules($request, $resultBag);

Check warning on line 111 in src/Server/AuthorizationServer.php

View check run for this annotation

Codecov / codecov/patch

src/Server/AuthorizationServer.php#L111

Added line #L111 was not covered by tests
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/Server/Grants/AuthCodeGrant.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
use SimpleSAML\Module\oidc\Repositories\Interfaces\AuthCodeRepositoryInterface;
use SimpleSAML\Module\oidc\Repositories\Interfaces\RefreshTokenRepositoryInterface;
use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException;
use SimpleSAML\Module\oidc\Server\Grants\Interfaces\AuthorizationValidatableWithCheckerResultBagInterface;
use SimpleSAML\Module\oidc\Server\Grants\Interfaces\AuthorizationValidatableWithRequestRules;
use SimpleSAML\Module\oidc\Server\Grants\Interfaces\OidcCapableGrantTypeInterface;
use SimpleSAML\Module\oidc\Server\Grants\Interfaces\PkceEnabledGrantTypeInterface;
use SimpleSAML\Module\oidc\Server\Grants\Traits\IssueAccessTokenTrait;
Expand Down Expand Up @@ -72,7 +72,7 @@
// phpcs:ignore
OidcCapableGrantTypeInterface,
// phpcs:ignore
AuthorizationValidatableWithCheckerResultBagInterface
AuthorizationValidatableWithRequestRules
{
use IssueAccessTokenTrait;

Expand Down Expand Up @@ -641,7 +641,7 @@
* @inheritDoc
* @throws \Throwable
*/
public function validateAuthorizationRequestWithCheckerResultBag(
public function validateAuthorizationRequestWithRequestRules(

Check warning on line 644 in src/Server/Grants/AuthCodeGrant.php

View check run for this annotation

Codecov / codecov/patch

src/Server/Grants/AuthCodeGrant.php#L644

Added line #L644 was not covered by tests
ServerRequestInterface $request,
ResultBagInterface $resultBag,
): OAuth2AuthorizationRequest {
Expand Down
49 changes: 39 additions & 10 deletions src/Server/Grants/ImplicitGrant.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace SimpleSAML\Module\oidc\Server\Grants;

use DateInterval;
use League\OAuth2\Server\Grant\ImplicitGrant as OAuth2ImplicitGrant;
use League\OAuth2\Server\RequestTypes\AuthorizationRequest as OAuth2AuthorizationRequest;
use League\OAuth2\Server\ResponseTypes\RedirectResponse;
use League\OAuth2\Server\ResponseTypes\ResponseTypeInterface;
Expand All @@ -17,24 +18,32 @@
use SimpleSAML\Module\oidc\Factories\Entities\AccessTokenEntityFactory;
use SimpleSAML\Module\oidc\Repositories\Interfaces\AccessTokenRepositoryInterface;
use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException;
use SimpleSAML\Module\oidc\Server\Grants\Interfaces\AuthorizationValidatableWithRequestRules;
use SimpleSAML\Module\oidc\Server\Grants\Traits\IssueAccessTokenTrait;
use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultBagInterface;
use SimpleSAML\Module\oidc\Server\RequestRules\RequestRulesManager;
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\AcrValuesRule;
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\AddClaimsToIdTokenRule;
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\ClientIdRule;
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\MaxAgeRule;
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\PromptRule;
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\RedirectUriRule;
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\RequestedClaimsRule;
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\RequestObjectRule;
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\RequiredNonceRule;
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\RequiredOpenIdScopeRule;
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\ResponseTypeRule;
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\ScopeRule;
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\StateRule;
use SimpleSAML\Module\oidc\Server\RequestTypes\AuthorizationRequest;
use SimpleSAML\Module\oidc\Services\IdTokenBuilder;
use SimpleSAML\Module\oidc\Utils\RequestParamsResolver;
use SimpleSAML\OpenID\Codebooks\HttpMethodsEnum;

class ImplicitGrant extends OAuth2ImplicitGrant
/**
* @psalm-suppress PropertyNotSetInConstructor
*/
class ImplicitGrant extends OAuth2ImplicitGrant implements AuthorizationValidatableWithRequestRules
{
use IssueAccessTokenTrait;

Expand All @@ -49,14 +58,15 @@

public function __construct(
protected IdTokenBuilder $idTokenBuilder,
DateInterval $accessTokenTTL,
protected DateInterval $accessTokenTTL,
AccessTokenRepositoryInterface $accessTokenRepository,
RequestRulesManager $requestRulesManager,
protected RequestRulesManager $requestRulesManager,
protected RequestParamsResolver $requestParamsResolver,
string $queryDelimiter,
protected string $queryDelimiter,
AccessTokenEntityFactory $accessTokenEntityFactory,
) {
parent::__construct($accessTokenTTL, $queryDelimiter, $requestRulesManager);
parent::__construct($accessTokenTTL, $queryDelimiter);

$this->accessTokenRepository = $accessTokenRepository;
$this->accessTokenEntityFactory = $accessTokenEntityFactory;
}
Expand Down Expand Up @@ -108,14 +118,12 @@
* @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException
* @throws \Throwable
*/
public function validateAuthorizationRequestWithCheckerResultBag(
public function validateAuthorizationRequestWithRequestRules(

Check warning on line 121 in src/Server/Grants/ImplicitGrant.php

View check run for this annotation

Codecov / codecov/patch

src/Server/Grants/ImplicitGrant.php#L121

Added line #L121 was not covered by tests
ServerRequestInterface $request,
ResultBagInterface $resultBag,
): OAuth2AuthorizationRequest {
$oAuth2AuthorizationRequest =
parent::validateAuthorizationRequestWithCheckerResultBag($request, $resultBag);

$rulesToExecute = [
ScopeRule::class,

Check warning on line 126 in src/Server/Grants/ImplicitGrant.php

View check run for this annotation

Codecov / codecov/patch

src/Server/Grants/ImplicitGrant.php#L126

Added line #L126 was not covered by tests
RequestObjectRule::class,
PromptRule::class,
MaxAgeRule::class,
Expand All @@ -129,14 +137,35 @@

$this->requestRulesManager->predefineResultBag($resultBag);

/** @var string $redirectUri */
$redirectUri = $resultBag->getOrFail(RedirectUriRule::class)->getValue();

Check warning on line 141 in src/Server/Grants/ImplicitGrant.php

View check run for this annotation

Codecov / codecov/patch

src/Server/Grants/ImplicitGrant.php#L141

Added line #L141 was not covered by tests
/** @var string|null $state */
$state = $resultBag->getOrFail(StateRule::class)->getValue();

Check warning on line 143 in src/Server/Grants/ImplicitGrant.php

View check run for this annotation

Codecov / codecov/patch

src/Server/Grants/ImplicitGrant.php#L143

Added line #L143 was not covered by tests
/** @var \SimpleSAML\Module\oidc\Entities\Interfaces\ClientEntityInterface $client */
$client = $resultBag->getOrFail(ClientIdRule::class)->getValue();

Check warning on line 145 in src/Server/Grants/ImplicitGrant.php

View check run for this annotation

Codecov / codecov/patch

src/Server/Grants/ImplicitGrant.php#L145

Added line #L145 was not covered by tests

// Some rules need certain things available in order to work properly...
$this->requestRulesManager->setData('default_scope', $this->defaultScope);
$this->requestRulesManager->setData('scope_delimiter_string', self::SCOPE_DELIMITER_STRING);

Check warning on line 149 in src/Server/Grants/ImplicitGrant.php

View check run for this annotation

Codecov / codecov/patch

src/Server/Grants/ImplicitGrant.php#L148-L149

Added lines #L148 - L149 were not covered by tests

$resultBag = $this->requestRulesManager->check(
$request,
$rulesToExecute,
$this->shouldUseFragment(),
$this->allowedAuthorizationHttpMethods,
);

$authorizationRequest = AuthorizationRequest::fromOAuth2AuthorizationRequest($oAuth2AuthorizationRequest);
/** @var \League\OAuth2\Server\Entities\ScopeEntityInterface[] $scopes */
$scopes = $resultBag->getOrFail(ScopeRule::class)->getValue();

Check warning on line 159 in src/Server/Grants/ImplicitGrant.php

View check run for this annotation

Codecov / codecov/patch

src/Server/Grants/ImplicitGrant.php#L159

Added line #L159 was not covered by tests

$authorizationRequest = new AuthorizationRequest();
$authorizationRequest->setClient($client);
$authorizationRequest->setRedirectUri($redirectUri);
$authorizationRequest->setScopes($scopes);
$authorizationRequest->setGrantTypeId($this->getIdentifier());
if ($state !== null) {
$authorizationRequest->setState($state);

Check warning on line 167 in src/Server/Grants/ImplicitGrant.php

View check run for this annotation

Codecov / codecov/patch

src/Server/Grants/ImplicitGrant.php#L161-L167

Added lines #L161 - L167 were not covered by tests
}

// nonce existence is validated using a rule, so we can get it from there.
$authorizationRequest->setNonce((string)$resultBag->getOrFail(RequiredNonceRule::class)->getValue());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@
use Psr\Http\Message\ServerRequestInterface;
use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultBagInterface;

interface AuthorizationValidatableWithCheckerResultBagInterface
interface AuthorizationValidatableWithRequestRules
{
/**
* Validate authorization request using an existing ResultBag instance (with already validated checkers).
* This is to evade usage of original validateAuthorizationRequest() method in which it is expected to
* validate client and redirect_uri (which was already validated).
*/
public function validateAuthorizationRequestWithCheckerResultBag(
public function validateAuthorizationRequestWithRequestRules(
ServerRequestInterface $request,
ResultBagInterface $resultBag,
): OAuth2AuthorizationRequest;
Expand Down
Loading