SSP-2035_authoauth2_module_switch_to_routes_and_controllers (#96)

* Symfony routes+Controller for callback and logout * Remove obsolete endpoints and handlers * Fix php styling.Fix error handling. * add missing optional configuration in README.md * Move from array() to [] notation. Add orcid2name.php attribute map. * improve routing service * Update phpunit.xml configuration * Update README to allow testing with v2.3.2 ssp docker image * Switch to cachedir from tempdir since tempdir is deprecated in ssp 2.3 * Update base test ssp2 config file to be ssp 2.3 default * decribe OIDC logout testing in README * Support both new and old routes --------- Co-authored-by: Patrick Radtke <patrick@cirrusidentity.com>
cirrusidentity · Oct 23, 2024 · 7904539 · 7904539
1 parent c234482
commit 7904539
Show file tree

Hide file tree

Showing 76 changed files with 2,119 additions and 1,503 deletions.
diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml
@@ -2,6 +2,10 @@ name: CI
 
 on: [push, pull_request]
 
+permissions:
+  pull-requests: write
+  contents: read
+
 jobs:
   basic-tests:
     name: Syntax and unit tests, PHP ${{ matrix.php-versions }}, ${{ matrix.operating-system }}
@@ -10,7 +14,7 @@ jobs:
       fail-fast: false
       matrix:
         operating-system: [ubuntu-latest]
-        php-versions: ['7.4', '8.0', '8.1', '8.2', '8.3']
+        php-versions: ['8.1', '8.2', '8.3']
 
     steps:
       - name: Setup PHP, with composer and extensions
@@ -20,7 +24,7 @@ jobs:
           extensions: intl, mbstring, mysql, pdo, pdo_sqlite, xml
           tools: composer:v2
           ini-values: error_reporting=E_ALL
-          coverage: xdebug
+          coverage: pcov
 
       - name: Setup problem matchers for PHP
         run: echo "::add-matcher::${{ runner.tool_cache }}/php.json"
@@ -53,7 +57,7 @@ jobs:
         run: composer install --no-progress --prefer-dist --optimize-autoloader
 
       - name: Decide whether to run code coverage or not
-        if: ${{ matrix.php-versions != '7.4' || matrix.operating-system != 'ubuntu-latest' }}
+        if: ${{ matrix.php-versions != '8.2' || matrix.operating-system != 'ubuntu-latest' }}
         run: |
           echo "NO_COVERAGE=--no-coverage" >> $GITHUB_ENV
 
@@ -63,20 +67,40 @@ jobs:
           ./vendor/bin/phpunit $NO_COVERAGE
 
       - name: Save coverage data
-        if: ${{ matrix.php-versions == '7.4' && matrix.operating-system == 'ubuntu-latest' }}
+        if: ${{ matrix.php-versions == '8.2' && matrix.operating-system == 'ubuntu-latest' }}
         uses: actions/upload-artifact@v3
         with:
           name: build-data
           path: ${{ github.workspace }}/build
 
+      - name: List files in the workspace
+        if: ${{ matrix.php-versions == '8.2' && matrix.operating-system == 'ubuntu-latest' }}
+        run: |
+          ls -la ${{ github.workspace }}/build
+          ls -la ${{ github.workspace }}/build/logs
+
+      - name: Code Coverage Report
+        if: ${{ matrix.php-versions == '8.2' && matrix.operating-system == 'ubuntu-latest' }}
+        uses: irongut/CodeCoverageSummary@v1.3.0
+        with:
+          filename: build/logs/cobertura.xml
+          format: markdown
+          badge: true
+          fail_below_min: true
+          hide_branch_rate: false
+          hide_complexity: true
+          indicators: true
+          output: both
+          thresholds: '60 80'
+
   security:
     name: Security checks
     runs-on: [ ubuntu-latest ]
     steps:
       - name: Setup PHP, with composer and extensions
         uses: shivammathur/setup-php@v2 #https://github.com/shivammathur/setup-php
         with:
-          php-version: '7.4'
+          php-version: '8.2'
           extensions: mbstring, xml
           tools: composer:v2
           coverage: none
@@ -119,7 +143,7 @@ jobs:
           id: setup-php
           uses: shivammathur/setup-php@v2 #https://github.com/shivammathur/setup-php
           with:
-            php-version: '7.4'
+            php-version: '8.2'
             tools: composer:v2
             extensions: mbstring, xml
 
@@ -156,7 +180,7 @@ jobs:
 
         - name: Psalm
           if: always()
-          run: php vendor/bin/psalm --use-baseline=psalm-baseline.xml --show-info=true --shepherd --php-version=${{ steps.setup-php.outputs.php-version }}
+          run: php vendor/bin/psalm --no-cache --show-info=true --shepherd --php-version=${{ steps.setup-php.outputs.php-version }}
 
         - name: Psalter
           if: always()

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,10 @@
 # simplesamlphp-module-authoauth2  Changelog
 
+## v5.0.0-beta.1
+* Upgrade to min SSP 2.3 and php 8.1
+* Move to controllers and routes
+* Update default callback/redirect URLS to not include `.php` extension
+
 ## v4.1.0
 _Release: 2024-10-01
 * Allow urlResourceOwnerDetails to be overridden for OIDC

diff --git a/README.md b/README.md
@@ -40,6 +40,7 @@ If you are interested in using SSP as an OIDC OP see the [OIDC module](https://g
 - [Development](#development)
   - [Docker](#docker)
     - [Facebook test user](#facebook-test-user)
+    - [Testing OIDC Logout](#testing-oidc-logout)
   - [Code style](#code-style)
 
 <!-- END doctoc generated TOC please keep comment here to allow auto update -->
@@ -51,7 +52,7 @@ The module can be installed with composer.
 
     composer require cirrusidentity/simplesamlphp-module-authoauth2
 
-
+If you are on SSP 2.3 use version 5.  See Changelog for breaking changes
 If you are on SSP 2 use version 4.
 If you are on SSP 1.X use version 3.
 
@@ -68,6 +69,11 @@ If you prefer not having dev dependencies installed, then you can use.
 
 [View the change log](CHANGELOG.md)
 
+There are breaking changes in version 5.
+* URLs are generated without `.php` extensions. To use the older style urls enable `'useLegacyRoutes' => true`
+* If you previously migrated to this module from `authfacebook` or some other module AND opted to not update your
+  client app redirect urls then you may need to do add some rewrite rules to your webserver.
+
 # Usage
 
 The generic OAuth2 client is configured with 
@@ -130,6 +136,10 @@ Generic usage provides enough configuration parameters to use with any OAuth2 or
                   // The underlying OAuth2 library also supports overriding requested scopes
                   //'scope' => ['other']
               ],
+              'useConsentErrorPage' => false|true,
+              // If set to true then /linkback.php, /logout.php, /loggedout.php, /consent/error.php legacy route is enabled
+              // if set to false or omitted the /linkback, /logout, /loggedout, /consent/error route is enable
+              'useLegacyRoutes' => false|true,
               // Default scopes to request
               'scopes' => ['email', 'profile'],
               'scopeSeparator' => ' ',
@@ -151,7 +161,6 @@ Generic usage provides enough configuration parameters to use with any OAuth2 or
               'logHttpTraffic' => true, //default is false
               'logMessageFormat' => 'A Guzzle MessageFormatter format string', // default setting is sufficient for most debugging
               'logIdTokenJson' => true, //default false. Log the json in the ID token.
-
           ),
 ```
 
@@ -374,9 +383,11 @@ Take the access token from above and call the user info endpoint
 If you are migrating away from an existing auth module, such as `authfacebook` you will need to one of the following:
  * add this module's `authoauth2` redirect URI to the facebook app, or
  * override the `authoauth2` authsource's redirect URI to match the `authfacebook` uri (`https://myserver.com/module.php/authfacebook/linkback.php)` AND do one of the following
-   * edit `/modules/authfacebook/www/linkback.php` to conditionally call `OAuth2ResponseHandler` (see below)
    * configure an Apache rewrite rule to change '/module.php/authfacebook/linkback.php' to '/module.php/authoauth2/linkback.php'
-   * symlink or edit `/modules/authfacebook/www/linkback.php` to invoke the `/modules/authoauth2/public/linkback.php`
+   * Prior to the move to version 5 and Controllers we supported two addtional ways. These will not work in version 5 since the referenced files to not exist
+     * symlink or edit `/modules/authfacebook/www/linkback.php` to invoke the `/modules/authoauth2/public/linkback.php`
+     * edit `/modules/authfacebook/www/linkback.php` to conditionally call `OAuth2ResponseHandler` (see below)
+
 
 
 Some social providers support multiple login protocols and older SSP modules may use the non-OAuth2 version for login.
@@ -402,7 +413,7 @@ if ($handler->canHandleResponse()) {
 
 ## Docker
 
-The `preprodwarning` module is include for testing authproc filters. *note:* The 1.0.2 version
+The `preprodwarning` module is included for testing authproc filters. *note:* The 1.0.2 version
 of `preprodwarning` has a bug in the redirect url. If using it you need to change `showwarning.php` to `warning`
 in your browser url.
 
@@ -415,7 +426,7 @@ docker run --name ssp-oauth2-dev \
   -e SSP_ENABLED_MODULES="authoauth2 preprodwarning" \
   --mount type=bind,source="$(pwd)/docker/config/authsources.php",target=/var/simplesamlphp/config/authsources.php,readonly \
   --mount type=bind,source="$(pwd)/docker/config/config-override.php",target=/var/simplesamlphp/config/config-override.php,readonly \
-  -p 443:443 cirrusid/simplesamlphp:v2.0.7
+  -p 443:443 cirrusid/simplesamlphp:v2.3.2
 ```
 
 and visit (which resolves to localhost, and the docker container) the [test authsource page](https://oauth2-validation.local.stack-dev.cirrusidentity.com/simplesaml/module.php/admin/test)
@@ -424,11 +435,21 @@ to test some pre-configured social integrations (yes, you can see the app passwo
 ### Facebook test user
 
 The pre-configured Facebook apps can only be accessed with a test account. You must be signed out of Facebook,
-otherwise you will get an error saying the application is not active.
+otherwise you will get an error saying
+
+```
+App not active
+
+This app is not accessible right now and the app developer is aware of the issue. You will be able to log in when the app is reactivated.
+```
 
 * email: open_nzwvghb_user@tfbnw.net
 * password: SSPisMyFavorite2022
 
+### Testing OIDC Logout
+
+The authsource `microsoftOIDCPkceSource` can be used for testing both OIDC
+login and OIDC logout.  https://oauth2-validation.local.stack-dev.cirrusidentity.com/simplesaml/module.php/admin/test/microsoftOIDCPkceSource
 
 ## Code style
 

diff --git a/attributemap/amazon2name.php b/attributemap/amazon2name.php
@@ -1,7 +1,9 @@
 <?php
 
-$attributemap = array(
+declare(strict_types=1);
+
+$attributemap = [
     'amazon.name' => ['cn', 'displayName'],
     'amazon.email' => 'mail',
     'amazon.user_id' => 'uid',
-);
+];
diff --git a/attributemap/apple2name.php b/attributemap/apple2name.php
@@ -1,9 +1,11 @@
 <?php
 
-$attributemap = array(
+declare(strict_types=1);
+
+$attributemap = [
     'apple.name.firstName' => 'givenName',
     'apple.name.lastName' => 'sn',
     'apple.email' => 'mail',
     'apple.sub' => 'uid',
     // apple.isPrivateEmail : There is no common name for this attribute
-);
+];
diff --git a/attributemap/bitbucket2name.php b/attributemap/bitbucket2name.php
@@ -1,7 +1,9 @@
 <?php
-$attributemap = array(
+
+declare(strict_types=1);
+$attributemap = [
 
     'bitbucket.display_name' => 'displayName',
     'bitbucket.account_id' => 'uid',
     'bitbucket.email' => 'mail',
-);
+];
diff --git a/attributemap/facebook2name.php b/attributemap/facebook2name.php
@@ -1,9 +1,11 @@
 <?php
 
-$attributemap = array(
+declare(strict_types=1);
+
+$attributemap = [
     'facebook.first_name'  => 'givenName',
     'facebook.last_name'   => 'sn',
     'facebook.name'        => ['cn', 'displayName'],
     'facebook.email'       => 'mail',
     'facebook.id'           => 'uid',
-);
+];
diff --git a/attributemap/linkedin2name.php b/attributemap/linkedin2name.php
@@ -1,9 +1,10 @@
 <?php
-$attributemap = array(
 
+declare(strict_types=1);
+
+$attributemap = [
     'linkedin.firstName' => 'givenName',
     'linkedin.lastName' => 'sn',
     'linkedin.id' => 'uid', // any b64 character
     'linkedin.emailAddress' => 'mail',
-
-);
+];
diff --git a/attributemap/microsoft2name.php b/attributemap/microsoft2name.php
@@ -1,10 +1,12 @@
 <?php
 
-$attributemap = array(
+declare(strict_types=1);
+
+$attributemap = [
     'microsoft.displayName' => 'displayName',
     'microsoft.id' => 'uid',
     'microsoft.mail' => 'mail',
     'microsoft.surname' => 'sn',
     'microsoft.givenName' => 'givenName',
     'microsoft.name' => 'cn',
-);
+];
diff --git a/attributemap/oidc2name.php b/attributemap/oidc2name.php
@@ -1,12 +1,13 @@
 <?php
 
-$attributemap = array(
+declare(strict_types=1);
 
+$attributemap = [
     // http://openid.net/specs/openid-connect-core-1_0.html#Claims
     'oidc.sub'  => 'uid',
     'oidc.family_name'   => 'sn',
     'oidc.given_name'   => 'givenName',
     'oidc.name'        => 'cn',
     'oidc.preferred_username' => 'displayName',
     'oidc.email'       => 'mail',
-);
+];
diff --git a/attributemap/orcid2name.php b/attributemap/orcid2name.php
@@ -0,0 +1,14 @@
+<?php
+
+declare(strict_types=1);
+
+$attributemap = [
+    // http://openid.net/specs/openid-connect-core-1_0.html#Claims
+    'orcid.id' => 'eduPersonOrcid', // URI with a 16-digit number
+    'orcid.sub'  => 'uid',
+    'orcid.family_name'   => 'sn',
+    'orcid.given_name'   => 'givenName',
+    'orcid.name'        => 'cn',
+    'orcid.preferred_username' => 'displayName',
+    'orcid.email'       => 'mail',
+];
diff --git a/composer.json b/composer.json
@@ -9,21 +9,22 @@
   ],
   "license": "LGPL-2.1-only",
   "require": {
-    "php": ">=7.4 || ^8.0",
+    "php": "^8.1",
     "simplesamlphp/composer-module-installer": "^1.1",
     "league/oauth2-client": "^2.7",
-    "simplesamlphp/simplesamlphp": "^v2.0.0",
+    "simplesamlphp/simplesamlphp": "^v2.3",
     "firebase/php-jwt": "^5.5|^6",
     "kevinrob/guzzle-cache-middleware": "^4.1.1",
     "psr/cache": "^1.0|^2.0|^3.0",
     "symfony/cache": "^6.0|^5.0|^4.3|^3.4",
     "ext-json": "*"
   },
   "require-dev": {
-    "simplesamlphp/simplesamlphp-test-framework": "^1.2",
-    "phpunit/phpunit": "^9.5",
-    "psalm/plugin-phpunit": "^0.18.3",
-    "squizlabs/php_codesniffer": "^3.7"
+    "simplesamlphp/simplesamlphp-test-framework": "^1.7",
+    "phpunit/phpunit": "^10",
+    "psalm/plugin-phpunit": "^0.19.0",
+    "squizlabs/php_codesniffer": "^3.7",
+    "dg/bypass-finals": "^1.8"
   },
   "autoload": {
     "psr-4": {
@@ -39,7 +40,8 @@
     "allow-plugins": {
       "simplesamlphp/composer-module-installer": true,
       "dealerdirect/phpcodesniffer-composer-installer": false,
-      "simplesamlphp/composer-xmlprovider-installer": false
+      "simplesamlphp/composer-xmlprovider-installer": false,
+      "phpstan/extension-installer": true
     }
   },
   "suggest": {
@@ -49,8 +51,7 @@
     "validate": [
       "vendor/bin/phpunit --no-coverage",
       "vendor/bin/phpcs -p",
-      "vendor/bin/psalm --clear-cache",
-      "vendor/bin/psalm --use-baseline=psalm-baseline.xml"
+      "vendor/bin/psalm --no-cache"
     ],
     "tests": [
       "vendor/bin/phpunit --no-coverage"

diff --git a/default-enable b/default-enable