Remove OAuth2 Implicit flow (#290)

* Remove OAuth2 Implicit flow

* Update docs

* Add some coverage

Author: cicnavi

README.md

@@ -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) 

UPGRADE.md

@@ -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
 
@@ -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.
 

routing/services/services.yml

@@ -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:

src/Factories/AuthorizationServerFactory.php

@@ -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;
@@ -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,
@@ -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(),

src/Factories/Grant/OAuth2ImplicitGrantFactory.php

@@ -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;
@@ -103,12 +103,12 @@ public function validateAuthorizationRequest(ServerRequestInterface $request): O
 
         foreach ($this->enabledGrantTypes as $grantType) {
             if ($grantType->canRespondToAuthorizationRequest($request)) {
-                if (! $grantType instanceof AuthorizationValidatableWithCheckerResultBagInterface) {
+                if (! $grantType instanceof AuthorizationValidatableWithRequestRules) {
                     throw OidcServerException::serverError('grant type must be validatable with already validated ' .
                                                            'result bag');
                 }
 
-                return $grantType->validateAuthorizationRequestWithCheckerResultBag($request, $resultBag);
+                return $grantType->validateAuthorizationRequestWithRequestRules($request, $resultBag);
             }
         }
 

src/Server/AuthorizationServer.php

@@ -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;
@@ -72,7 +72,7 @@ class AuthCodeGrant extends OAuth2AuthCodeGrant implements
     // phpcs:ignore
     OidcCapableGrantTypeInterface,
     // phpcs:ignore
-    AuthorizationValidatableWithCheckerResultBagInterface
+    AuthorizationValidatableWithRequestRules
 {
     use IssueAccessTokenTrait;
 
@@ -641,7 +641,7 @@ protected function validateAuthorizationCode(
      * @inheritDoc
      * @throws \Throwable
      */
-    public function validateAuthorizationRequestWithCheckerResultBag(
+    public function validateAuthorizationRequestWithRequestRules(
         ServerRequestInterface $request,
         ResultBagInterface $resultBag,
     ): OAuth2AuthorizationRequest {

src/Server/Grants/AuthCodeGrant.php

@@ -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;
@@ -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;
 
@@ -49,14 +58,15 @@ class ImplicitGrant extends OAuth2ImplicitGrant
 
     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;
     }
@@ -108,14 +118,12 @@ public function completeAuthorizationRequest(
      * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException
      * @throws \Throwable
      */
-    public function validateAuthorizationRequestWithCheckerResultBag(
+    public function validateAuthorizationRequestWithRequestRules(
         ServerRequestInterface $request,
         ResultBagInterface $resultBag,
     ): OAuth2AuthorizationRequest {
-        $oAuth2AuthorizationRequest =
-        parent::validateAuthorizationRequestWithCheckerResultBag($request, $resultBag);
-
         $rulesToExecute = [
+            ScopeRule::class,
             RequestObjectRule::class,
             PromptRule::class,
             MaxAgeRule::class,
@@ -129,14 +137,35 @@ public function validateAuthorizationRequestWithCheckerResultBag(
 
         $this->requestRulesManager->predefineResultBag($resultBag);
 
+        /** @var string $redirectUri */
+        $redirectUri = $resultBag->getOrFail(RedirectUriRule::class)->getValue();
+        /** @var string|null $state */
+        $state = $resultBag->getOrFail(StateRule::class)->getValue();
+        /** @var \SimpleSAML\Module\oidc\Entities\Interfaces\ClientEntityInterface $client */
+        $client = $resultBag->getOrFail(ClientIdRule::class)->getValue();
+
+        // 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);
+
         $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();
+
+        $authorizationRequest = new AuthorizationRequest();
+        $authorizationRequest->setClient($client);
+        $authorizationRequest->setRedirectUri($redirectUri);
+        $authorizationRequest->setScopes($scopes);
+        $authorizationRequest->setGrantTypeId($this->getIdentifier());
+        if ($state !== null) {
+            $authorizationRequest->setState($state);
+        }
 
         // nonce existence is validated using a rule, so we can get it from there.
         $authorizationRequest->setNonce((string)$resultBag->getOrFail(RequiredNonceRule::class)->getValue());

src/Server/Grants/ImplicitGrant.php

@@ -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;