More code quality improvements (#33)

* Disabled phpdoc_to_comment in php-cs
* Refactorization to fix some psalm errors and CS
* Fixed memento export interfaces in entities
* Fixed CsrfProtection form control
* Client administration UX improved
    - Use twig extensions to load assets
* Removed CsrfProtection custom control
* Fix scope in memento export interface
* Fix script paths
* Update dependencies
    - Update to ssp 1.18
    - Update to league/oauth2-server 8.1
    - Update test dependencies
    - Update jwt library
    - Change semantic-ui with fomantic-ui
    - Update from Zend to Laminas
* Update client entity to new league API
    - Removed PCKE token
    - Added is_confidential attribute
    - Added migration
* Update documentation

Author: sgomez

.php_cs.dist

@@ -29,6 +29,7 @@ return PhpCsFixer\Config::create()
         'no_useless_return' => true,
         'ordered_imports' => true,
         'phpdoc_order' => true,
+        'phpdoc_to_comment' => false,
         'semicolon_after_instruction' => true,
         'strict_comparison' => true,
         'strict_param' => true,

.travis.yml

@@ -23,6 +23,7 @@ before_script:
 
 script:
   - php vendor/bin/phpunit --no-coverage
+  - php vendor/bin/phpspec run
 
 jobs:
   fast_finish: true

CHANGELOG.md

@@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## [Unreleased]
 
+## [1.0.0-rc.2] - 2020-05-17
+### Added
+- Second release candidate
+- Updated league/oauth2-server to version 8.1
+### Changed
+- Removed pkce config option
+- New field _is_confidential_ in client (disabled for previous clients)
+- Update database schema
+
 ## [1.0.0-rc.1] - 2018-11-13
 ### Added
 - First release candidate

README.md

@@ -148,3 +148,8 @@ Once the database schema has been created, you can open the _Federation_ tab fro
 
 The module lets you create, read, update and delete all the RP you want. To see the client id and the client secret press the show button.
 
+### Create client options
+
+* Enabled: You can enable or disable a client. Disabled by default.
+* Secure client: The client is secure if it is capable of securely storing a secret. Unsecure clients
+must provide a PCKS token (code_challenge parameter during authorization phase). Disabled by default. 

composer.json

@@ -18,25 +18,27 @@
     ],
     "require": {
         "php": ">=7.2.0",
+        "ext-json": "*",
         "ext-openssl": "*",
-        "league/oauth2-server": "^7.0",
+        "league/oauth2-server": "^8.1.0",
         "nette/forms": "^2.4",
         "psr/container": "^1.0",
         "simplesamlphp/composer-module-installer": "^1.0",
         "web-token/jwt-framework": "^2.1",
         "steverhoades/oauth2-openid-connect-server": "^1.0",
-        "zendframework/zend-diactoros": "^1.3"
+        "laminas/laminas-diactoros": "^2.2.1",
+        "laminas/laminas-httphandlerrunner": "^1.1.0"
     },
     "require-dev": {
         "friendsofphp/php-cs-fixer": "^2.10",
         "friends-of-phpspec/phpspec-code-coverage": "^4.3",
         "php-coveralls/php-coveralls": "^2.0",
         "phpspec/phpspec": "^6.1",
-        "phpunit/php-code-coverage": "^6.0",
-        "phpunit/phpcov": "^5.0",
-        "phpunit/phpunit": "^7.0",
-        "simplesamlphp/simplesamlphp": "^1.18",
-        "simplesamlphp/simplesamlphp-test-framework": "^0.1.0"
+        "phpunit/php-code-coverage": "^7.0.7",
+        "phpunit/phpcov": "^6.0",
+        "phpunit/phpunit": "^8.5",
+        "simplesamlphp/simplesamlphp": "^1.18,<1.19",
+        "simplesamlphp/simplesamlphp-test-framework": "^0.1.9"
     },
     "config": {
         "preferred-install": {
@@ -58,5 +60,21 @@
         "branch-alias": {
             "dev-master": "1.0.x-dev"
         }
+    },
+    "scripts": {
+        "pre-commit": [
+            "vendor/bin/check-syntax-php.sh",
+            "vendor/bin/check-syntax-json.sh",
+            "vendor/bin/check-syntax-xml.sh",
+            "vendor/bin/check-syntax-yaml.sh",
+            "vendor/bin/security-checker security:check",
+            "vendor/bin/psalm",
+            "vendor/bin/psalter --issues=UnnecessaryVarAnnotation --dry-run",
+            "vendor/bin/phpcs"
+        ],
+        "tests": [
+            "vendor/bin/phpunit --no-coverage",
+            "vendor/bin/phpspec run "
+        ]
     }
 }

config-templates/module_oidc.php

@@ -20,8 +20,6 @@
     'authCodeDuration' => 'PT10M', // 10 minutes
     'refreshTokenDuration' => 'P1M', // 1 month
     'accessTokenDuration' => 'PT1H', // 1 hour,
-    // Enable PKCE (RFC7636)
-    'pkce' => false,
 
     // Tag to run storage cleanup script using the cron module...
     'cron_tag' => 'hourly',

hooks/hook_cron.php

@@ -12,9 +12,13 @@
  * file that was distributed with this source code.
  */
 
+use SimpleSAML\Modules\OpenIDConnect\Repositories\AccessTokenRepository;
+use SimpleSAML\Modules\OpenIDConnect\Repositories\AuthCodeRepository;
+use SimpleSAML\Modules\OpenIDConnect\Repositories\RefreshTokenRepository;
 
 /**
  * @param array &$croninfo
+ *
  * @return void
  */
 function oidc_hook_cron(&$croninfo)
@@ -32,14 +36,16 @@ function oidc_hook_cron(&$croninfo)
         return;
     }
 
+    $container = new \SimpleSAML\Modules\OpenIDConnect\Services\Container();
+
     try {
-        $accessTokenRepository = new \SimpleSAML\Modules\OpenIDConnect\Repositories\AccessTokenRepository();
+        $accessTokenRepository = $container->get(AccessTokenRepository::class);
         $accessTokenRepository->removeExpired();
 
-        $authTokenRepository = new \SimpleSAML\Modules\OpenIDConnect\Repositories\AuthCodeRepository();
+        $authTokenRepository = $container->get(AuthCodeRepository::class);
         $authTokenRepository->removeExpired();
 
-        $refreshTokenRepository = new \SimpleSAML\Modules\OpenIDConnect\Repositories\RefreshTokenRepository();
+        $refreshTokenRepository = $container->get(RefreshTokenRepository::class);
         $refreshTokenRepository->removeExpired();
 
         $croninfo['summary'][] = 'Module `oidc` clean up. Removed expired entries from storage.';

hooks/hook_frontpage.php

@@ -14,6 +14,7 @@
 
 /**
  * @param array &$links
+ *
  * @return void
  */
 function oidc_hook_frontpage(&$links)

lib/ClaimTranslatorExtractor.php

@@ -85,12 +85,10 @@ class ClaimTranslatorExtractor extends ClaimExtractor
         ],
     ];
 
-
     /**
      * ClaimTranslatorExtractor constructor.
      *
      * @param ClaimSetEntity[] $claimSets
-     * @param array            $translationTable
      *
      * @throws \OpenIDConnectServer\Exception\InvalidArgumentException
      */
@@ -106,18 +104,16 @@ public function __construct(array $claimSets = [], array $translationTable = [])
         parent::__construct($claimSets);
     }
 
-
     /**
      * @param array $samlAttributes
-     * @return array
      */
     private function translateSamlAttributesToClaims($samlAttributes): array
     {
         $claims = [];
 
         foreach ($this->translationTable as $claim => $samlMatches) {
             foreach ($samlMatches as $samlMatch) {
-                if (array_key_exists($samlMatch, $samlAttributes)) {
+                if (\array_key_exists($samlMatch, $samlAttributes)) {
                     $claims[$claim] = current($samlAttributes[$samlMatch]);
                     break;
                 }
@@ -127,12 +123,6 @@ private function translateSamlAttributesToClaims($samlAttributes): array
         return $claims;
     }
 
-
-    /**
-     * @param array $scopes
-     * @param array $samlAttributes
-     * @return array
-     */
     public function extract(array $scopes, array $samlAttributes): array
     {
         $claims = $this->translateSamlAttributesToClaims($samlAttributes);

lib/Controller/ClientCreateController.php

@@ -14,6 +14,8 @@
 
 namespace SimpleSAML\Modules\OpenIDConnect\Controller;
 
+use Laminas\Diactoros\Response\RedirectResponse;
+use Laminas\Diactoros\ServerRequest;
 use SimpleSAML\Modules\OpenIDConnect\Controller\Traits\GetClientFromRequestTrait;
 use SimpleSAML\Modules\OpenIDConnect\Entity\ClientEntity;
 use SimpleSAML\Modules\OpenIDConnect\Factories\FormFactory;
@@ -23,8 +25,6 @@
 use SimpleSAML\Modules\OpenIDConnect\Services\SessionMessagesService;
 use SimpleSAML\Utils\HTTP;
 use SimpleSAML\Utils\Random;
-use Zend\Diactoros\Response\RedirectResponse;
-use Zend\Diactoros\ServerRequest;
 
 class ClientCreateController
 {
@@ -45,13 +45,6 @@ class ClientCreateController
      */
     private $messages;
 
-
-    /**
-     * @param \SimpleSAML\Modules\OpenIDConnect\Repositories\ClientRepository $clientRepository
-     * @param \SimpleSAML\Modules\OpenIDConnect\Factories\TemplateFactory $templateFactory
-     * @param \SimpleSAML\Modules\OpenIDConnect\Factories\FormFactory $formFactory
-     * @param \SimpleSAML\Modules\OpenIDConnect\Services\SessionMessagesService $messages
-     */
     public function __construct(
         ClientRepository $clientRepository,
         TemplateFactory $templateFactory,
@@ -64,10 +57,8 @@ public function __construct(
         $this->messages = $messages;
     }
 
-
     /**
-     * @param \Zend\Diactoros\ServerRequest $request
-     * @return \Zend\Diactoros\Response\RedirectResponse|\SimpleSAML\XHTML\Template
+     * @return \Laminas\Diactoros\Response\RedirectResponse|\SimpleSAML\XHTML\Template
      */
     public function __invoke(ServerRequest $request)
     {
@@ -87,12 +78,13 @@ public function __invoke(ServerRequest $request)
                 $client['auth_source'],
                 $client['redirect_uri'],
                 $client['scopes'],
-                $client['is_enabled']
+                $client['is_enabled'],
+                $client['is_confidential']
             ));
 
             $this->messages->addMessage('{oidc:client:added}');
 
-            return new RedirectResponse(HTTP::addURLParameters('index.php', []));
+            return new RedirectResponse(HTTP::addURLParameters('show.php', ['client_id' => $client['id']]));
         }
 
         return $this->templateFactory->render('oidc:clients/new.twig', [