From 9db8fdf7525b3b4d9c838d2a79f95c1a111355e1 Mon Sep 17 00:00:00 2001 From: H2CK Date: Fri, 5 Jun 2026 11:44:36 +0200 Subject: [PATCH 01/28] Add OIDC conformance workflow --- .../docker-compose.github-actions.yml | 15 ++ .github/conformance/oidc-basic-config.json | 61 ++++++ .github/conformance/write-config.py | 25 +++ .github/workflows/build-test.yaml | 203 +++++++++++++++++- 4 files changed, 302 insertions(+), 2 deletions(-) create mode 100644 .github/conformance/docker-compose.github-actions.yml create mode 100644 .github/conformance/oidc-basic-config.json create mode 100644 .github/conformance/write-config.py diff --git a/.github/conformance/docker-compose.github-actions.yml b/.github/conformance/docker-compose.github-actions.yml new file mode 100644 index 00000000..fa265143 --- /dev/null +++ b/.github/conformance/docker-compose.github-actions.yml @@ -0,0 +1,15 @@ +services: + nginx: + extra_hosts: + - "host.docker.internal:host-gateway" + - "localhost.emobix.co.uk:host-gateway" + + server: + extra_hosts: + - "host.docker.internal:host-gateway" + - "localhost.emobix.co.uk:host-gateway" + + test: + extra_hosts: + - "host.docker.internal:host-gateway" + - "localhost.emobix.co.uk:host-gateway" diff --git a/.github/conformance/oidc-basic-config.json b/.github/conformance/oidc-basic-config.json new file mode 100644 index 00000000..0c071c48 --- /dev/null +++ b/.github/conformance/oidc-basic-config.json @@ -0,0 +1,61 @@ +{ + "alias": "${CONFORMANCE_ALIAS}", + "description": "Nextcloud OIDC GitHub Actions basic conformance", + "publish": "no", + "server": { + "discoveryUrl": "${NEXTCLOUD_BASE_URL}/index.php/apps/oidc/openid-configuration" + }, + "client": { + "client_id": "${OIDC_CLIENT_ID_1}", + "client_secret": "${OIDC_CLIENT_SECRET_1}" + }, + "client_secret_post": { + "client_id": "${OIDC_CLIENT_ID_1}", + "client_secret": "${OIDC_CLIENT_SECRET_1}" + }, + "client2": { + "client_id": "${OIDC_CLIENT_ID_2}", + "client_secret": "${OIDC_CLIENT_SECRET_2}" + }, + "browser": [ + { + "match": "${NEXTCLOUD_BASE_URL}/*", + "tasks": [ + { + "task": "Login", + "match": "${NEXTCLOUD_BASE_URL}/login*", + "optional": true, + "commands": [ + [ + "text", + "id", + "user", + "${OIDC_TEST_USER}" + ], + [ + "text", + "id", + "password", + "${OIDC_TEST_PASSWORD}" + ], + [ + "click", + "id", + "submit-form" + ], + [ + "wait", + "contains", + "/test/a/", + 30 + ] + ] + }, + { + "task": "Verify", + "match": "https://localhost.emobix.co.uk:8443/test/a/${CONFORMANCE_ALIAS}/callback*" + } + ] + } + ] +} diff --git a/.github/conformance/write-config.py b/.github/conformance/write-config.py new file mode 100644 index 00000000..4af43772 --- /dev/null +++ b/.github/conformance/write-config.py @@ -0,0 +1,25 @@ +#!/usr/bin/env python3 + +import os +import string +import sys + + +def main() -> int: + if len(sys.argv) != 3: + print("Usage: write-config.py TEMPLATE OUTPUT", file=sys.stderr) + return 2 + + with open(sys.argv[1], encoding="utf-8") as template_file: + template = string.Template(template_file.read()) + + rendered = template.safe_substitute(os.environ) + + with open(sys.argv[2], "w", encoding="utf-8") as output_file: + output_file.write(rendered) + + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/.github/workflows/build-test.yaml b/.github/workflows/build-test.yaml index 5c8bd6f3..ce5c97bb 100644 --- a/.github/workflows/build-test.yaml +++ b/.github/workflows/build-test.yaml @@ -13,8 +13,12 @@ on: branches: - "*" workflow_dispatch: - branches: - - "*" + inputs: + run_conformance: + description: "Run the OpenID Foundation OIDC conformance suite" + required: false + default: false + type: boolean env: PHP_VERSION: 8.5 @@ -149,3 +153,198 @@ jobs: openssl dgst -sha512 -sign ${{ env.APP_NAME }}.key nextcloud/apps/${{ env.APP_NAME }}/build/artifacts/${{ env.APP_NAME }}.tar.gz | openssl base64 -A > signature.sha rm -f ${{ env.APP_NAME }}.key rm -f ${{ env.APP_NAME }}.crt + + oidc_conformance: + runs-on: ubuntu-latest + needs: build_test + if: github.event_name == 'workflow_dispatch' && inputs.run_conformance + timeout-minutes: 90 + + env: + CONFORMANCE_ALIAS: nextcloud-${{ github.run_id }} + NEXTCLOUD_BASE_URL: http://host.docker.internal:8080 + OIDC_TEST_USER: oidc-test-user + OIDC_TEST_PASSWORD: oidc-test-password + OIDC_CLIENT_ID_1: nextcloud-conformance-client-00001 + OIDC_CLIENT_SECRET_1: nextcloud-conformance-secret-00001 + OIDC_CLIENT_ID_2: nextcloud-conformance-client-00002 + OIDC_CLIENT_SECRET_2: nextcloud-conformance-secret-00002 + + steps: + - name: Set app env + run: | + echo "APP_NAME=${GITHUB_REPOSITORY##*/}" >> $GITHUB_ENV + + - name: Checkout + uses: actions/checkout@v3 + with: + path: ${{ env.APP_NAME }} + + - name: Get appinfo data + id: appinfo + uses: mavrosxristoforos/get-xml-info@1.1.1 + with: + xml-file: ${{ env.APP_NAME }}/appinfo/info.xml + xpath: "//info//dependencies//nextcloud/@max-version" + + - name: Derive Nextcloud checkout ref + shell: bash + run: | + max_version="${{ steps.appinfo.outputs.info }}" + if [ -z "$max_version" ]; then + max_version="${{ steps.appinfo.outputs.value }}" + fi + if [ -z "$max_version" ]; then + echo "Error: could not read nextcloud max-version from appinfo" >&2 + exit 1 + fi + major="${max_version%%.*}" + echo "NEXTCLOUD_REF=stable$major" >> $GITHUB_ENV + + - name: Read package.json node and npm engines version + uses: skjnldsv/read-package-engines-version-actions@v2 + id: versions + continue-on-error: true + with: + path: ${{ env.APP_NAME }} + fallbackNode: "^22" + fallbackNpm: "^10" + + - name: Set up node ${{ steps.versions.outputs.nodeVersion }} + if: ${{ steps.versions.outputs.nodeVersion }} + uses: actions/setup-node@v3 + with: + node-version: ${{ steps.versions.outputs.nodeVersion }} + + - name: Set up npm ${{ steps.versions.outputs.npmVersion }} + if: ${{ steps.versions.outputs.npmVersion }} + run: npm i -g npm@"${{ steps.versions.outputs.npmVersion }}" + + - name: Set up php ${{ env.PHP_VERSION }} + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ env.PHP_VERSION }} + coverage: none + + - name: Set up Java + uses: actions/setup-java@v4 + with: + distribution: temurin + java-version: "21" + + - name: Install app dependencies and build assets + run: | + cd ${{ env.APP_NAME }} + composer install + npm ci + npm run build + + - name: Checkout Nextcloud server + uses: actions/checkout@v3 + with: + repository: nextcloud/server + path: nextcloud + submodules: recursive + ref: ${{ env.NEXTCLOUD_REF }} + + - name: Install Nextcloud dependencies + run: | + cd nextcloud + composer install + + - name: Install and configure Nextcloud OIDC provider + run: | + mv -v ${{ env.APP_NAME }} nextcloud/apps/ + cd nextcloud + php occ maintenance:install --admin-user admin --admin-pass admin + php occ app:enable ${{ env.APP_NAME }} + php occ config:system:set trusted_domains 1 --value=host.docker.internal + php occ config:system:set trusted_domains 2 --value=localhost.emobix.co.uk + php occ config:system:set trusted_domains 3 --value=127.0.0.1 + php occ config:system:set overwrite.cli.url --value="${NEXTCLOUD_BASE_URL}" + php occ config:system:set overwritehost --value=host.docker.internal:8080 + php occ config:system:set overwriteprotocol --value=http + php occ config:app:set ${{ env.APP_NAME }} allow_user_settings --value no + OC_PASS="${OIDC_TEST_PASSWORD}" php occ user:add --password-from-env "${OIDC_TEST_USER}" + php occ user:setting "${OIDC_TEST_USER}" settings email oidc-test@example.test + callback="https://localhost.emobix.co.uk:8443/test/a/${CONFORMANCE_ALIAS}/callback" + php occ oidc:create "OIDC Conformance Basic 1" "${callback}" \ + --client_id "${OIDC_CLIENT_ID_1}" \ + --client_secret "${OIDC_CLIENT_SECRET_1}" \ + --type confidential \ + --flow code \ + --allowed_scopes "openid profile email offline_access" + php occ oidc:create "OIDC Conformance Basic 2" "${callback}" \ + --client_id "${OIDC_CLIENT_ID_2}" \ + --client_secret "${OIDC_CLIENT_SECRET_2}" \ + --type confidential \ + --flow code \ + --allowed_scopes "openid profile email offline_access" + + - name: Start Nextcloud web server + run: | + cd nextcloud + php -S 0.0.0.0:8080 -t . > ../nextcloud-server.log 2>&1 & + echo $! > ../nextcloud-server.pid + for i in {1..30}; do + if curl --fail --silent http://127.0.0.1:8080/index.php/apps/oidc/openid-configuration > /dev/null; then + exit 0 + fi + sleep 2 + done + cat ../nextcloud-server.log + exit 1 + + - name: Checkout OpenID conformance suite + run: git clone --depth 1 https://gitlab.com/openid/conformance-suite.git conformance-suite + + - name: Build and start OpenID conformance suite + run: | + sudo sh -c 'echo "127.0.0.1 localhost.emobix.co.uk" >> /etc/hosts' + cd conformance-suite + mvn -B -DskipTests package + docker compose \ + -f docker-compose-localtest.yml \ + -f ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/docker-compose.github-actions.yml \ + up -d mongodb nginx server + for i in {1..60}; do + if curl --insecure --fail --silent https://localhost.emobix.co.uk:8443/api/runner/available > /dev/null; then + exit 0 + fi + sleep 5 + done + docker compose -f docker-compose-localtest.yml logs server nginx + exit 1 + + - name: Install conformance runner dependencies + run: python3 -m pip install --user httpx pyparsing + + - name: Write conformance plan config + run: | + mkdir -p conformance-results conformance-config + python3 nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/write-config.py \ + nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/oidc-basic-config.json \ + conformance-config/oidc-basic-config.json + python3 -m json.tool conformance-config/oidc-basic-config.json > /dev/null + + - name: Run OIDC basic conformance plan + run: | + cd conformance-suite + python3 scripts/run-test-plan.py \ + --export-dir ../conformance-results \ + --verbose \ + "oidcc-basic-certification-test-plan[server_metadata=discovery][client_registration=static_client]" \ + ../conformance-config/oidc-basic-config.json + env: + CONFORMANCE_DEV_MODE: 1 + CONFORMANCE_SERVER: https://localhost.emobix.co.uk:8443/ + CONFORMANCE_SERVER_MTLS: https://localhost.emobix.co.uk:8444/ + + - name: Upload conformance results + if: always() + uses: actions/upload-artifact@v4 + with: + name: oidc-conformance-results + path: | + conformance-results + nextcloud-server.log From ccfd803c0cc07d685bf309ad57deec177a388082 Mon Sep 17 00:00:00 2001 From: H2CK Date: Mon, 8 Jun 2026 10:52:32 +0200 Subject: [PATCH 02/28] Fix conformance test Signed-off-by: H2CK --- .github/conformance/docker-compose.github-actions.yml | 3 --- .github/conformance/oidc-basic-config.json | 4 ++-- .github/workflows/build-test.yaml | 8 ++++---- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/.github/conformance/docker-compose.github-actions.yml b/.github/conformance/docker-compose.github-actions.yml index fa265143..c0c48733 100644 --- a/.github/conformance/docker-compose.github-actions.yml +++ b/.github/conformance/docker-compose.github-actions.yml @@ -2,14 +2,11 @@ services: nginx: extra_hosts: - "host.docker.internal:host-gateway" - - "localhost.emobix.co.uk:host-gateway" server: extra_hosts: - "host.docker.internal:host-gateway" - - "localhost.emobix.co.uk:host-gateway" test: extra_hosts: - "host.docker.internal:host-gateway" - - "localhost.emobix.co.uk:host-gateway" diff --git a/.github/conformance/oidc-basic-config.json b/.github/conformance/oidc-basic-config.json index 0c071c48..e0f67132 100644 --- a/.github/conformance/oidc-basic-config.json +++ b/.github/conformance/oidc-basic-config.json @@ -23,7 +23,7 @@ "tasks": [ { "task": "Login", - "match": "${NEXTCLOUD_BASE_URL}/login*", + "match": "${NEXTCLOUD_BASE_URL}/index.php/login*", "optional": true, "commands": [ [ @@ -53,7 +53,7 @@ }, { "task": "Verify", - "match": "https://localhost.emobix.co.uk:8443/test/a/${CONFORMANCE_ALIAS}/callback*" + "match": "https://nginx:8443/test/a/${CONFORMANCE_ALIAS}/callback*" } ] } diff --git a/.github/workflows/build-test.yaml b/.github/workflows/build-test.yaml index ce5c97bb..4643eecc 100644 --- a/.github/workflows/build-test.yaml +++ b/.github/workflows/build-test.yaml @@ -267,7 +267,7 @@ jobs: php occ config:app:set ${{ env.APP_NAME }} allow_user_settings --value no OC_PASS="${OIDC_TEST_PASSWORD}" php occ user:add --password-from-env "${OIDC_TEST_USER}" php occ user:setting "${OIDC_TEST_USER}" settings email oidc-test@example.test - callback="https://localhost.emobix.co.uk:8443/test/a/${CONFORMANCE_ALIAS}/callback" + callback="https://nginx:8443/test/a/${CONFORMANCE_ALIAS}/callback" php occ oidc:create "OIDC Conformance Basic 1" "${callback}" \ --client_id "${OIDC_CLIENT_ID_1}" \ --client_secret "${OIDC_CLIENT_SECRET_1}" \ @@ -308,7 +308,7 @@ jobs: -f ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/docker-compose.github-actions.yml \ up -d mongodb nginx server for i in {1..60}; do - if curl --insecure --fail --silent https://localhost.emobix.co.uk:8443/api/runner/available > /dev/null; then + if curl --insecure --fail --silent https://nginx:8443/api/runner/available > /dev/null; then exit 0 fi sleep 5 @@ -337,8 +337,8 @@ jobs: ../conformance-config/oidc-basic-config.json env: CONFORMANCE_DEV_MODE: 1 - CONFORMANCE_SERVER: https://localhost.emobix.co.uk:8443/ - CONFORMANCE_SERVER_MTLS: https://localhost.emobix.co.uk:8444/ + CONFORMANCE_SERVER: https://nginx:8443/ + CONFORMANCE_SERVER_MTLS: https://nginx:8444/ - name: Upload conformance results if: always() From 3d16d6d4b804061f37b730490a806e88889ea0b0 Mon Sep 17 00:00:00 2001 From: H2CK Date: Mon, 8 Jun 2026 11:28:29 +0200 Subject: [PATCH 03/28] Update pipeline concerning reachability Signed-off-by: H2CK --- .github/workflows/build-test.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build-test.yaml b/.github/workflows/build-test.yaml index 4643eecc..28348007 100644 --- a/.github/workflows/build-test.yaml +++ b/.github/workflows/build-test.yaml @@ -300,7 +300,8 @@ jobs: - name: Build and start OpenID conformance suite run: | - sudo sh -c 'echo "127.0.0.1 localhost.emobix.co.uk" >> /etc/hosts' + sudo sh -c 'echo "127.0.0.1 nginx" >> /etc/hosts' + sudo sh -c 'echo "127.0.0.1 oidcc-provider" >> /etc/hosts' cd conformance-suite mvn -B -DskipTests package docker compose \ From 5d3b12f3541ae410b5d73e1fa12f74a428c97ea6 Mon Sep 17 00:00:00 2001 From: H2CK Date: Mon, 8 Jun 2026 13:34:33 +0200 Subject: [PATCH 04/28] Switch to chrome headless Signed-off-by: H2CK --- .github/conformance/HEADLESS_CHROME_GUIDE.md | 159 ++++++++++++++++++ .github/conformance/README.md | 115 +++++++++++++ .github/conformance/docker-compose.chrome.yml | 23 +++ .github/conformance/run-conformance-chrome.sh | 151 +++++++++++++++++ .github/workflows/build-test.yaml | 17 +- 5 files changed, 463 insertions(+), 2 deletions(-) create mode 100644 .github/conformance/HEADLESS_CHROME_GUIDE.md create mode 100644 .github/conformance/README.md create mode 100644 .github/conformance/docker-compose.chrome.yml create mode 100644 .github/conformance/run-conformance-chrome.sh diff --git a/.github/conformance/HEADLESS_CHROME_GUIDE.md b/.github/conformance/HEADLESS_CHROME_GUIDE.md new file mode 100644 index 00000000..86abce9e --- /dev/null +++ b/.github/conformance/HEADLESS_CHROME_GUIDE.md @@ -0,0 +1,159 @@ +# Guide: Running OIDC Conformance Tests with Headless Chrome + +## Problem +The default OIDC conformance test runner uses HtmlUnit (a Java-based headless browser) which doesn't support modern ES6 JavaScript syntax (e.g., `class` keyword) used in Nextcloud's core bundles. This causes JavaScript execution errors like: +``` +org.htmlunit.ScriptException: identifier is a reserved word: class +``` + +## Solution: Use Selenium + Headless Chrome + +### Option 1: GitHub Actions Workflow (CI/CD) + +**Changes to `.github/workflows/build-test.yaml`:** + +In the `oidc_conformance` job, modify the "Build and start OpenID conformance suite" step: + +```yaml + - name: Build and start OpenID conformance suite + run: | + sudo sh -c 'echo "127.0.0.1 nginx" >> /etc/hosts' + sudo sh -c 'echo "127.0.0.1 oidcc-provider" >> /etc/hosts' + sudo sh -c 'echo "127.0.0.1 chrome" >> /etc/hosts' + cd conformance-suite + mvn -B -DskipTests package + docker compose \ + -f docker-compose-localtest.yml \ + -f ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/docker-compose.github-actions.yml \ + -f ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/docker-compose.chrome.yml \ + up -d mongodb nginx server chrome + + # Wait for Chrome/Selenium service to be ready + for i in {1..60}; do + if curl --insecure --fail --silent http://chrome:4444/status > /dev/null; then + echo "Chrome/Selenium service is ready" + break + fi + sleep 2 + done + + # Wait for API runner + for i in {1..60}; do + if curl --insecure --fail --silent https://nginx:8443/api/runner/available > /dev/null; then + exit 0 + fi + sleep 5 + done + docker compose -f docker-compose-localtest.yml logs server nginx chrome + exit 1 +``` + +**Add environment variables before running test plan:** + +In the "Run OIDC basic conformance plan" step, add `SELENIUM_HUB_HOST` and related environment variables: + +```yaml + - name: Run OIDC basic conformance plan + run: | + cd conformance-suite + python3 scripts/run-test-plan.py \ + --export-dir ../conformance-results \ + --verbose \ + "oidcc-basic-certification-test-plan[server_metadata=discovery][client_registration=static_client]" \ + ../conformance-config/oidc-basic-config.json + env: + CONFORMANCE_DEV_MODE: 1 + CONFORMANCE_SERVER: https://nginx:8443/ + CONFORMANCE_SERVER_MTLS: https://nginx:8444/ + SELENIUM_HUB_HOST: http://chrome:4444 + BROWSER_DRIVER: chrome +``` + +### Option 2: Local Development + +**Prerequisites:** +```bash +# Install Chrome +brew install --cask google-chrome + +# Install Chromedriver (must match Chrome version) +brew install chromedriver +# or +brew install --cask chromedriver + +# Verify versions match +google-chrome --version +chromedriver --version +``` + +**Start Chromedriver:** +```bash +chromedriver --port=9515 & +echo $! > chromedriver.pid +``` + +**Run tests locally:** +```bash +cd conformance-suite +mvn -B -DskipTests package + +# Start containers (without Chrome, we'll use local Chromedriver) +docker compose \ + -f docker-compose-localtest.yml \ + -f ../nextcloud/apps/oidc/.github/conformance/docker-compose.github-actions.yml \ + up -d mongodb nginx server + +# Wait for services +sleep 30 + +# Run test with local Chromedriver +python3 scripts/run-test-plan.py \ + --export-dir ../conformance-results \ + --verbose \ + "oidcc-basic-certification-test-plan[server_metadata=discovery][client_registration=static_client]" \ + ../conformance-config/oidc-basic-config.json +``` + +Set environment variables: +```bash +export CONFORMANCE_DEV_MODE=1 +export CONFORMANCE_SERVER=https://nginx:8443/ +export CONFORMANCE_SERVER_MTLS=https://nginx:8444/ +export SELENIUM_HUB_HOST=http://localhost:9515 +export BROWSER_DRIVER=chrome +``` + +### Option 3: Alternative - ES5 Transpilation (Without Browser Engine Change) + +If you want to keep HtmlUnit but fix the ES6 compatibility issue, ensure Nextcloud's JavaScript is built as ES5: + +Check `/nextcloud/webpack.config.js` or build configuration for: +```javascript +target: 'es5' // or targets: ['es2015'] as minimum +``` + +Then rebuild: +```bash +cd nextcloud +npm ci +npm run build // Ensure target is ES5 compatible +``` + +## Validation + +After running with Chrome: +1. Check test logs for WebDriver calls instead of HtmlUnit errors +2. Expected log entries should show Chrome/Chromium processes executing JavaScript +3. No `org.htmlunit.ScriptException` errors +4. JavaScript should execute properly on login pages + +## Files Modified + +- `.github/workflows/build-test.yaml` - Added Chrome service and environment variables +- `.github/conformance/docker-compose.chrome.yml` - New Selenium/Chrome service definition + +## References + +- [OpenID Conformance Suite](https://gitlab.com/openid/conformance-suite) +- [Selenium Docker Images](https://github.com/SeleniumHQ/docker-selenium) +- [Chromium WebDriver Protocol](https://chromedriver.chromium.org/) diff --git a/.github/conformance/README.md b/.github/conformance/README.md new file mode 100644 index 00000000..3b82ad24 --- /dev/null +++ b/.github/conformance/README.md @@ -0,0 +1,115 @@ +# OIDC Conformance Test Configuration + +This directory contains configuration files and scripts for running OpenID Connect (OIDC) conformance tests against the Nextcloud OIDC Identity Provider app. + +## Files + +- **`oidc-basic-config.json`** - Conformance test plan configuration with test user credentials and client credentials +- **`write-config.py`** - Python script to generate final conformance config from template (replaces environment variables) +- **`docker-compose.github-actions.yml`** - Docker Compose overrides for GitHub Actions CI/CD environment +- **`docker-compose.chrome.yml`** - Docker Compose service definition for Selenium + Headless Chrome (alternative to HtmlUnit) +- **`run-conformance-chrome.sh`** - Local development script to run conformance tests with Headless Chrome +- **`HEADLESS_CHROME_GUIDE.md`** - Detailed guide for setting up and running tests with Chrome instead of HtmlUnit + +## Quick Start + +### Local Development with Headless Chrome + +```bash +# Make script executable +chmod +x run-conformance-chrome.sh + +# Run tests (handles all setup) +./run-conformance-chrome.sh +``` + +The script will: +1. Verify Chrome and Chromedriver are installed +2. Start Chromedriver on port 9515 +3. Start Nextcloud Docker containers (if not running) +4. Run the conformance test suite +5. Cleanup resources + +### GitHub Actions CI/CD + +The workflow defined in `.github/workflows/build-test.yaml` runs conformance tests when manually triggered with `run_conformance=true`: + +```bash +# Via GitHub CLI +gh workflow run build-test.yaml -f run_conformance=true + +# Or manually in GitHub Actions UI: +# Actions > Build app > Run workflow > run_conformance checkbox +``` + +## Browser Engine Selection + +### HtmlUnit (Default - Not Recommended) +- ❌ Does not support modern ES6 JavaScript +- ❌ Causes `org.htmlunit.ScriptException: identifier is a reserved word: class` errors +- ✓ No additional setup needed + +### Headless Chrome + Selenium (Recommended) +- ✓ Full modern JavaScript support (ES6+) +- ✓ More accurate simulation of real browser behavior +- ✓ Better debugging with VNC access (port 7900) +- ⚠ Requires Chrome/Chromedriver installation + +## Environment Variables + +When running conformance tests, these variables control browser engine and server URLs: + +- `SELENIUM_HUB_HOST` - WebDriver endpoint (e.g., `http://localhost:9515`) +- `BROWSER_DRIVER` - Browser engine (e.g., `chrome`, defaults to `htmlunit`) +- `CONFORMANCE_SERVER` - Nextcloud OIDC provider URL (e.g., `https://nginx:8443/`) +- `CONFORMANCE_SERVER_MTLS` - Mutual TLS endpoint (e.g., `https://nginx:8444/`) +- `CONFORMANCE_DEV_MODE` - Developer mode (`1` = enabled) + +## Troubleshooting + +### Chrome/Chromedriver Version Mismatch +```bash +# Check versions +google-chrome --version +chromedriver --version + +# Install matching versions +brew install --cask google-chrome chromedriver +# or +brew upgrade google-chrome chromedriver +``` + +### Chromedriver Port Already in Use +```bash +# Find and kill process on port 9515 +lsof -i :9515 +kill -9 +``` + +### Docker Compose Networking Issues +Ensure all container names resolve correctly: +```bash +# Add to /etc/hosts (macOS/Linux) +sudo sh -c 'echo "127.0.0.1 nginx" >> /etc/hosts' +sudo sh -c 'echo "127.0.0.1 chrome" >> /etc/hosts' +``` + +### VNC Debugging (to watch Chrome execution) +```bash +# On local machine, access VNC viewer +# Address: localhost:7900 (or docker_host:7900 if remote) +# Password: secret (default) +``` + +## References + +- [OpenID Connect Conformance Suite Documentation](https://gitlab.com/openid/conformance-suite) +- [OIDC Specification](https://openid.net/specs/openid-connect-core-1_0.html) +- [Selenium Docker Images](https://github.com/SeleniumHQ/docker-selenium) +- [Chromium WebDriver](https://chromedriver.chromium.org/) + +## Related Configuration + +- Test plan: `oidcc-basic-certification-test-plan[server_metadata=discovery][client_registration=static_client]` +- Test user credentials: `OIDC_TEST_USER` / `OIDC_TEST_PASSWORD` (set in workflow) +- OIDC clients: Two clients created dynamically in Nextcloud (`OIDC_CLIENT_ID_1/2`, `OIDC_CLIENT_SECRET_1/2`) diff --git a/.github/conformance/docker-compose.chrome.yml b/.github/conformance/docker-compose.chrome.yml new file mode 100644 index 00000000..7648e3be --- /dev/null +++ b/.github/conformance/docker-compose.chrome.yml @@ -0,0 +1,23 @@ +services: + # Selenium Chrome service for conformance testing + # Provides WebDriver endpoint at http://chrome:4444/wd/hub + chrome: + image: seleniarm/standalone-chromium:latest + container_name: conformance-chrome + ports: + - "4444:4444" + - "7900:7900" # VNC port for debugging + environment: + - SE_NODE_MAX_SESSIONS=3 + - SE_SESSION_REQUEST_TIMEOUT=60 + - SE_SESSION_RETRY_INTERVAL=2 + volumes: + - /dev/shm:/dev/shm # Shared memory for Chrome stability + networks: + - conformance_default + extra_hosts: + - "host.docker.internal:host-gateway" + +networks: + conformance_default: + driver: bridge diff --git a/.github/conformance/run-conformance-chrome.sh b/.github/conformance/run-conformance-chrome.sh new file mode 100644 index 00000000..c9c4958c --- /dev/null +++ b/.github/conformance/run-conformance-chrome.sh @@ -0,0 +1,151 @@ +#!/bin/bash +# Local conformance test runner with Headless Chrome +# Usage: ./run-conformance-chrome.sh + +set -e + +# Colors for output +RED='\033[0;31m' +GREEN='\033[0;32m' +YELLOW='\033[1;33m' +NC='\033[0m' # No Color + +echo -e "${YELLOW}OIDC Conformance Test Runner with Headless Chrome${NC}" +echo "======================================================" + +# Check prerequisites +echo -e "\n${YELLOW}Checking prerequisites...${NC}" + +if ! command -v google-chrome &> /dev/null && ! command -v chromium &> /dev/null; then + echo -e "${RED}✗ Chrome/Chromium not found${NC}" + echo "Install with: brew install --cask google-chrome" + exit 1 +fi +echo -e "${GREEN}✓ Chrome found${NC}" + +if ! command -v chromedriver &> /dev/null; then + echo -e "${RED}✗ Chromedriver not found${NC}" + echo "Install with: brew install chromedriver" + exit 1 +fi + +# Verify Chrome and Chromedriver versions match (major version) +CHROME_VERSION=$(google-chrome --version | awk '{print $3}' | cut -d. -f1) +DRIVER_VERSION=$(chromedriver --version | awk '{print $2}' | cut -d. -f1) + +echo -e "${GREEN}✓ Chromedriver found (Chrome v${CHROME_VERSION}, Driver v${DRIVER_VERSION})${NC}" + +if [ "$CHROME_VERSION" != "$DRIVER_VERSION" ]; then + echo -e "${YELLOW}⚠ Warning: Chrome major version ($CHROME_VERSION) differs from Chromedriver major version ($DRIVER_VERSION)${NC}" + echo "This may cause compatibility issues. Consider updating one of them." + read -p "Continue anyway? (y/N) " -n 1 -r + echo + if [[ ! $REPLY =~ ^[Yy]$ ]]; then + exit 1 + fi +fi + +# Start chromedriver +echo -e "\n${YELLOW}Starting Chromedriver on port 9515...${NC}" +chromedriver --port=9515 > /tmp/chromedriver.log 2>&1 & +CHROMEDRIVER_PID=$! +echo $CHROMEDRIVER_PID > /tmp/chromedriver.pid +echo -e "${GREEN}✓ Chromedriver started (PID: $CHROMEDRIVER_PID)${NC}" + +# Wait for chromedriver to be ready +echo -e "\n${YELLOW}Waiting for Chromedriver to be ready...${NC}" +for i in {1..30}; do + if curl -s http://localhost:9515/status | grep -q "ready"; then + echo -e "${GREEN}✓ Chromedriver is ready${NC}" + break + fi + if [ $i -eq 30 ]; then + echo -e "${RED}✗ Chromedriver did not become ready in time${NC}" + kill $CHROMEDRIVER_PID + exit 1 + fi + sleep 1 +done + +# Set environment variables +export CONFORMANCE_DEV_MODE=1 +export CONFORMANCE_SERVER=https://nginx:8443/ +export CONFORMANCE_SERVER_MTLS=https://nginx:8444/ +export SELENIUM_HUB_HOST=http://localhost:9515 +export BROWSER_DRIVER=chrome +export HEADLESS_BROWSER=true + +echo -e "\n${YELLOW}Environment variables set:${NC}" +echo " SELENIUM_HUB_HOST=$SELENIUM_HUB_HOST" +echo " BROWSER_DRIVER=$BROWSER_DRIVER" +echo " CONFORMANCE_SERVER=$CONFORMANCE_SERVER" + +# Check if conformance-suite exists +if [ ! -d "./conformance-suite" ]; then + echo -e "\n${YELLOW}Conformance suite not found. Cloning...${NC}" + git clone --depth 1 https://gitlab.com/openid/conformance-suite.git conformance-suite +fi + +# Build conformance suite +if [ ! -f "./conformance-suite/target/fintechlabs-conformance-suite-5.1.0-SNAPSHOT-jar-with-dependencies.jar" ]; then + echo -e "\n${YELLOW}Building conformance suite...${NC}" + cd conformance-suite + mvn -B -DskipTests package -q + cd .. +fi + +# Check if Nextcloud containers are running +echo -e "\n${YELLOW}Checking Nextcloud Docker containers...${NC}" +if ! docker compose -f conformance-suite/docker-compose-localtest.yml ps | grep -q "nginx"; then + echo -e "${YELLOW}Starting Nextcloud containers...${NC}" + cd conformance-suite + docker compose \ + -f docker-compose-localtest.yml \ + -f ../../.github/conformance/docker-compose.github-actions.yml \ + up -d mongodb nginx server + + # Wait for services + echo -e "\n${YELLOW}Waiting for services to be ready...${NC}" + for i in {1..60}; do + if curl --insecure --fail --silent https://nginx:8443/api/runner/available > /dev/null 2>&1; then + echo -e "${GREEN}✓ Services are ready${NC}" + break + fi + if [ $i -eq 60 ]; then + echo -e "${RED}✗ Services did not become ready in time${NC}" + kill $(cat /tmp/chromedriver.pid) + exit 1 + fi + echo -n "." + sleep 2 + done + cd .. +fi + +# Run conformance tests +echo -e "\n${YELLOW}Running conformance tests...${NC}" +echo "==================================================" +cd conformance-suite +python3 scripts/run-test-plan.py \ + --export-dir ../conformance-results \ + --verbose \ + "oidcc-basic-certification-test-plan[server_metadata=discovery][client_registration=static_client]" \ + ../conformance-config/oidc-basic-config.json + +TEST_RESULT=$? +cd .. + +# Cleanup +echo -e "\n${YELLOW}Cleaning up...${NC}" +if [ -f /tmp/chromedriver.pid ]; then + kill $(cat /tmp/chromedriver.pid) 2>/dev/null || true + rm /tmp/chromedriver.pid +fi + +if [ $TEST_RESULT -eq 0 ]; then + echo -e "\n${GREEN}✓ Conformance tests completed successfully${NC}" + echo "Results available in: conformance-results/" +else + echo -e "\n${RED}✗ Conformance tests failed${NC}" + exit 1 +fi diff --git a/.github/workflows/build-test.yaml b/.github/workflows/build-test.yaml index 28348007..772d20d9 100644 --- a/.github/workflows/build-test.yaml +++ b/.github/workflows/build-test.yaml @@ -302,19 +302,30 @@ jobs: run: | sudo sh -c 'echo "127.0.0.1 nginx" >> /etc/hosts' sudo sh -c 'echo "127.0.0.1 oidcc-provider" >> /etc/hosts' + sudo sh -c 'echo "127.0.0.1 chrome" >> /etc/hosts' cd conformance-suite mvn -B -DskipTests package docker compose \ -f docker-compose-localtest.yml \ -f ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/docker-compose.github-actions.yml \ - up -d mongodb nginx server + -f ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/docker-compose.chrome.yml \ + up -d mongodb nginx server chrome + # Wait for Chrome/Selenium service to be ready + for i in {1..60}; do + if curl --fail --silent http://chrome:4444/status > /dev/null 2>&1; then + echo "Chrome/Selenium service is ready" + break + fi + sleep 2 + done + # Wait for API runner for i in {1..60}; do if curl --insecure --fail --silent https://nginx:8443/api/runner/available > /dev/null; then exit 0 fi sleep 5 done - docker compose -f docker-compose-localtest.yml logs server nginx + docker compose -f docker-compose-localtest.yml logs server nginx chrome exit 1 - name: Install conformance runner dependencies @@ -340,6 +351,8 @@ jobs: CONFORMANCE_DEV_MODE: 1 CONFORMANCE_SERVER: https://nginx:8443/ CONFORMANCE_SERVER_MTLS: https://nginx:8444/ + SELENIUM_HUB_HOST: http://chrome:4444 + BROWSER_DRIVER: chrome - name: Upload conformance results if: always() From 28577d11e394790dcd4eeba753ceb82d8d79b335 Mon Sep 17 00:00:00 2001 From: H2CK Date: Mon, 8 Jun 2026 14:37:45 +0200 Subject: [PATCH 05/28] Fix configuration Signed-off-by: H2CK --- .github/conformance/docker-compose.chrome.yml | 6 ------ .github/workflows/build-test.yaml | 21 +++++++++++-------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/.github/conformance/docker-compose.chrome.yml b/.github/conformance/docker-compose.chrome.yml index 7648e3be..6960397b 100644 --- a/.github/conformance/docker-compose.chrome.yml +++ b/.github/conformance/docker-compose.chrome.yml @@ -13,11 +13,5 @@ services: - SE_SESSION_RETRY_INTERVAL=2 volumes: - /dev/shm:/dev/shm # Shared memory for Chrome stability - networks: - - conformance_default extra_hosts: - "host.docker.internal:host-gateway" - -networks: - conformance_default: - driver: bridge diff --git a/.github/workflows/build-test.yaml b/.github/workflows/build-test.yaml index 772d20d9..2726055a 100644 --- a/.github/workflows/build-test.yaml +++ b/.github/workflows/build-test.yaml @@ -259,8 +259,7 @@ jobs: php occ maintenance:install --admin-user admin --admin-pass admin php occ app:enable ${{ env.APP_NAME }} php occ config:system:set trusted_domains 1 --value=host.docker.internal - php occ config:system:set trusted_domains 2 --value=localhost.emobix.co.uk - php occ config:system:set trusted_domains 3 --value=127.0.0.1 + php occ config:system:set trusted_domains 2 --value=127.0.0.1 php occ config:system:set overwrite.cli.url --value="${NEXTCLOUD_BASE_URL}" php occ config:system:set overwritehost --value=host.docker.internal:8080 php occ config:system:set overwriteprotocol --value=http @@ -325,7 +324,11 @@ jobs: fi sleep 5 done - docker compose -f docker-compose-localtest.yml logs server nginx chrome + docker compose \ + -f docker-compose-localtest.yml \ + -f ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/docker-compose.github-actions.yml \ + -f ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/docker-compose.chrome.yml \ + logs server nginx chrome exit 1 - name: Install conformance runner dependencies @@ -340,6 +343,12 @@ jobs: python3 -m json.tool conformance-config/oidc-basic-config.json > /dev/null - name: Run OIDC basic conformance plan + env: + CONFORMANCE_DEV_MODE: 1 + CONFORMANCE_SERVER: https://nginx:8443/ + CONFORMANCE_SERVER_MTLS: https://nginx:8444/ + SELENIUM_HUB_HOST: http://chrome:4444 + BROWSER_DRIVER: chrome run: | cd conformance-suite python3 scripts/run-test-plan.py \ @@ -347,12 +356,6 @@ jobs: --verbose \ "oidcc-basic-certification-test-plan[server_metadata=discovery][client_registration=static_client]" \ ../conformance-config/oidc-basic-config.json - env: - CONFORMANCE_DEV_MODE: 1 - CONFORMANCE_SERVER: https://nginx:8443/ - CONFORMANCE_SERVER_MTLS: https://nginx:8444/ - SELENIUM_HUB_HOST: http://chrome:4444 - BROWSER_DRIVER: chrome - name: Upload conformance results if: always() From 8e3e5d73fef1f3938320a199a88691a7ebeaa994 Mon Sep 17 00:00:00 2001 From: H2CK Date: Mon, 8 Jun 2026 15:12:49 +0200 Subject: [PATCH 06/28] Enable Chrome headless Signed-off-by: H2CK --- .github/conformance/docker-compose.chrome.yml | 1 + .github/workflows/build-test.yaml | 2 ++ 2 files changed, 3 insertions(+) diff --git a/.github/conformance/docker-compose.chrome.yml b/.github/conformance/docker-compose.chrome.yml index 6960397b..e2bbbccc 100644 --- a/.github/conformance/docker-compose.chrome.yml +++ b/.github/conformance/docker-compose.chrome.yml @@ -11,6 +11,7 @@ services: - SE_NODE_MAX_SESSIONS=3 - SE_SESSION_REQUEST_TIMEOUT=60 - SE_SESSION_RETRY_INTERVAL=2 + - CHROME_OPTIONS=--headless=new,--no-sandbox,--disable-dev-shm-usage volumes: - /dev/shm:/dev/shm # Shared memory for Chrome stability extra_hosts: diff --git a/.github/workflows/build-test.yaml b/.github/workflows/build-test.yaml index 2726055a..092bb678 100644 --- a/.github/workflows/build-test.yaml +++ b/.github/workflows/build-test.yaml @@ -349,6 +349,8 @@ jobs: CONFORMANCE_SERVER_MTLS: https://nginx:8444/ SELENIUM_HUB_HOST: http://chrome:4444 BROWSER_DRIVER: chrome + HEADLESS_BROWSER: true + WEBDRIVER_CHROME_OPTIONS: "--headless=new,--no-sandbox,--disable-dev-shm-usage" run: | cd conformance-suite python3 scripts/run-test-plan.py \ From 93ba44270b7d12299a99b1581016e23c27b3c4ce Mon Sep 17 00:00:00 2001 From: Thorsten Jagel Date: Tue, 9 Jun 2026 15:19:41 +0200 Subject: [PATCH 07/28] Changes approach to switch to headless chrome --- .github/conformance/HEADLESS_CHROME_GUIDE.md | 41 ++- .github/conformance/README.md | 14 +- .github/conformance/browser-runner.py | 258 ++++++++++++++++++ .github/conformance/docker-compose.chrome.yml | 9 +- .github/conformance/oidc-basic-config.json | 43 +-- .github/conformance/run-conformance-chrome.sh | 22 +- .github/workflows/build-test.yaml | 13 +- appinfo/routes.php | 5 + 8 files changed, 331 insertions(+), 74 deletions(-) create mode 100644 .github/conformance/browser-runner.py diff --git a/.github/conformance/HEADLESS_CHROME_GUIDE.md b/.github/conformance/HEADLESS_CHROME_GUIDE.md index 86abce9e..99618fcd 100644 --- a/.github/conformance/HEADLESS_CHROME_GUIDE.md +++ b/.github/conformance/HEADLESS_CHROME_GUIDE.md @@ -6,7 +6,13 @@ The default OIDC conformance test runner uses HtmlUnit (a Java-based headless br org.htmlunit.ScriptException: identifier is a reserved word: class ``` -## Solution: Use Selenium + Headless Chrome +## Solution: Use an External Selenium + Headless Chrome Worker + +The conformance suite's built-in browser automation is implemented in Java with +HtmlUnit. The workflow does not try to switch that internal implementation. +Instead, `oidc-basic-config.json` omits `browser` commands, which makes the +suite expose front-channel URLs through its runner API. `browser-runner.py` +polls that API and drives the URLs with Selenium/Chromium. ### Option 1: GitHub Actions Workflow (CI/CD) @@ -48,25 +54,27 @@ In the `oidc_conformance` job, modify the "Build and start OpenID conformance su exit 1 ``` -**Add environment variables before running test plan:** - -In the "Run OIDC basic conformance plan" step, add `SELENIUM_HUB_HOST` and related environment variables: +**Run the external browser worker with the test plan:** ```yaml - name: Run OIDC basic conformance plan + env: + CONFORMANCE_DEV_MODE: 1 + CONFORMANCE_SERVER: https://nginx:8443/ + CONFORMANCE_SERVER_MTLS: https://nginx:8444/ + SELENIUM_REMOTE_URL: http://chrome:4444/wd/hub + CONFORMANCE_BROWSER_VISIT_TIMEOUT: 90 run: | cd conformance-suite + python3 ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/browser-runner.py \ + > ../conformance-browser.log 2>&1 & + browser_runner_pid=$! + trap 'kill "${browser_runner_pid}" 2>/dev/null || true' EXIT python3 scripts/run-test-plan.py \ --export-dir ../conformance-results \ --verbose \ "oidcc-basic-certification-test-plan[server_metadata=discovery][client_registration=static_client]" \ ../conformance-config/oidc-basic-config.json - env: - CONFORMANCE_DEV_MODE: 1 - CONFORMANCE_SERVER: https://nginx:8443/ - CONFORMANCE_SERVER_MTLS: https://nginx:8444/ - SELENIUM_HUB_HOST: http://chrome:4444 - BROWSER_DRIVER: chrome ``` ### Option 2: Local Development @@ -106,7 +114,11 @@ docker compose \ # Wait for services sleep 30 -# Run test with local Chromedriver +# Run the browser worker with local Chromedriver +python3 ../.github/conformance/browser-runner.py > ../conformance-browser.log 2>&1 & +browser_runner_pid=$! +trap 'kill "${browser_runner_pid}" 2>/dev/null || true' EXIT + python3 scripts/run-test-plan.py \ --export-dir ../conformance-results \ --verbose \ @@ -119,8 +131,7 @@ Set environment variables: export CONFORMANCE_DEV_MODE=1 export CONFORMANCE_SERVER=https://nginx:8443/ export CONFORMANCE_SERVER_MTLS=https://nginx:8444/ -export SELENIUM_HUB_HOST=http://localhost:9515 -export BROWSER_DRIVER=chrome +export SELENIUM_REMOTE_URL=http://localhost:9515 ``` ### Option 3: Alternative - ES5 Transpilation (Without Browser Engine Change) @@ -149,7 +160,9 @@ After running with Chrome: ## Files Modified -- `.github/workflows/build-test.yaml` - Added Chrome service and environment variables +- `.github/workflows/build-test.yaml` - Starts the external browser worker with the test plan +- `.github/conformance/browser-runner.py` - Polls conformance front-channel URLs and drives Chromium +- `.github/conformance/oidc-basic-config.json` - Omits HtmlUnit browser commands - `.github/conformance/docker-compose.chrome.yml` - New Selenium/Chrome service definition ## References diff --git a/.github/conformance/README.md b/.github/conformance/README.md index 3b82ad24..4505a4d5 100644 --- a/.github/conformance/README.md +++ b/.github/conformance/README.md @@ -8,6 +8,7 @@ This directory contains configuration files and scripts for running OpenID Conne - **`write-config.py`** - Python script to generate final conformance config from template (replaces environment variables) - **`docker-compose.github-actions.yml`** - Docker Compose overrides for GitHub Actions CI/CD environment - **`docker-compose.chrome.yml`** - Docker Compose service definition for Selenium + Headless Chrome (alternative to HtmlUnit) +- **`browser-runner.py`** - External Selenium/Chromium browser worker for conformance front-channel URLs - **`run-conformance-chrome.sh`** - Local development script to run conformance tests with Headless Chrome - **`HEADLESS_CHROME_GUIDE.md`** - Detailed guide for setting up and running tests with Chrome instead of HtmlUnit @@ -49,18 +50,23 @@ gh workflow run build-test.yaml -f run_conformance=true - ❌ Causes `org.htmlunit.ScriptException: identifier is a reserved word: class` errors - ✓ No additional setup needed -### Headless Chrome + Selenium (Recommended) +### External Headless Chrome + Selenium (Recommended) - ✓ Full modern JavaScript support (ES6+) - ✓ More accurate simulation of real browser behavior - ✓ Better debugging with VNC access (port 7900) - ⚠ Requires Chrome/Chromedriver installation +The conformance suite's built-in browser automation is HtmlUnit-based. To avoid +modifying the suite source, keep the test config free of `browser` commands and +run `browser-runner.py` alongside `scripts/run-test-plan.py`. The runner polls +the suite API for exposed front-channel URLs and drives them through Chromium. + ## Environment Variables -When running conformance tests, these variables control browser engine and server URLs: +When running conformance tests, these variables configure the external browser +runner and server URLs: -- `SELENIUM_HUB_HOST` - WebDriver endpoint (e.g., `http://localhost:9515`) -- `BROWSER_DRIVER` - Browser engine (e.g., `chrome`, defaults to `htmlunit`) +- `SELENIUM_REMOTE_URL` - WebDriver endpoint (e.g., `http://chrome:4444/wd/hub`) - `CONFORMANCE_SERVER` - Nextcloud OIDC provider URL (e.g., `https://nginx:8443/`) - `CONFORMANCE_SERVER_MTLS` - Mutual TLS endpoint (e.g., `https://nginx:8444/`) - `CONFORMANCE_DEV_MODE` - Developer mode (`1` = enabled) diff --git a/.github/conformance/browser-runner.py b/.github/conformance/browser-runner.py new file mode 100644 index 00000000..94d7b1c0 --- /dev/null +++ b/.github/conformance/browser-runner.py @@ -0,0 +1,258 @@ +#!/usr/bin/env python3 +""" +Drive OIDC conformance front-channel URLs with Selenium/Chromium. + +The OpenID conformance suite's built-in BrowserControl uses HtmlUnit. For +Nextcloud, the login form is rendered by modern JavaScript, so the suite is +configured without browser commands and this worker handles exposed browser URLs +through the suite's existing runner API. +""" + +import os +import sys +import time +import urllib.parse + +import httpx +from selenium import webdriver +from selenium.common.exceptions import NoSuchElementException +from selenium.common.exceptions import StaleElementReferenceException +from selenium.webdriver import ChromeOptions +from selenium.webdriver.common.by import By + + +CONFORMANCE_SERVER = os.environ.get("CONFORMANCE_SERVER", "https://nginx:8443/").rstrip("/") +SELENIUM_REMOTE_URL = os.environ.get("SELENIUM_REMOTE_URL") or os.environ.get("SELENIUM_HUB_HOST") or "http://chrome:4444/wd/hub" +OIDC_TEST_USER = os.environ["OIDC_TEST_USER"] +OIDC_TEST_PASSWORD = os.environ["OIDC_TEST_PASSWORD"] +POLL_SECONDS = float(os.environ.get("CONFORMANCE_BROWSER_POLL_SECONDS", "1")) +VISIT_TIMEOUT_SECONDS = int(os.environ.get("CONFORMANCE_BROWSER_VISIT_TIMEOUT", "90")) + + +def log(message): + print(f"[conformance-browser] {message}", flush=True) + + +def new_driver(): + options = ChromeOptions() + options.set_capability("acceptInsecureCerts", True) + for argument in ( + "--headless=new", + "--no-sandbox", + "--disable-dev-shm-usage", + "--ignore-certificate-errors", + "--window-size=1280,1000", + ): + options.add_argument(argument) + return webdriver.Remote(command_executor=SELENIUM_REMOTE_URL, options=options) + + +def first_present(driver, selectors, timeout=5): + end = time.monotonic() + timeout + while time.monotonic() < end: + for by, value in selectors: + try: + element = driver.find_element(by, value) + if element.is_displayed(): + return element + except (NoSuchElementException, StaleElementReferenceException): + pass + time.sleep(0.2) + raise NoSuchElementException(f"None of the selectors were found: {selectors}") + + +def first_clickable(driver, selectors, timeout=5): + end = time.monotonic() + timeout + while time.monotonic() < end: + for by, value in selectors: + try: + element = driver.find_element(by, value) + if element.is_displayed() and element.is_enabled(): + return element + except (NoSuchElementException, StaleElementReferenceException): + pass + time.sleep(0.2) + raise NoSuchElementException(f"None of the selectors were clickable: {selectors}") + + +def submit_post(driver, url): + parsed = urllib.parse.urlsplit(url) + action = urllib.parse.urlunsplit((parsed.scheme, parsed.netloc, parsed.path, "", "")) + params = urllib.parse.parse_qsl(parsed.query, keep_blank_values=True) + + inputs = "\n".join( + f'' + for name, value in params + ) + page = f""" + + + +
+ {inputs} +
+ + + +""" + driver.get("data:text/html;charset=utf-8," + urllib.parse.quote(page)) + + +def html_escape(value): + return ( + value.replace("&", "&") + .replace('"', """) + .replace("<", "<") + .replace(">", ">") + ) + + +def is_login_page(driver): + current_url = driver.current_url + if "/index.php/login" in current_url: + return True + try: + return bool(driver.find_elements(By.ID, "login")) + except StaleElementReferenceException: + return False + + +def login(driver): + log(f"Logging in at {driver.current_url}") + user = first_present(driver, ((By.ID, "user"), (By.NAME, "user")), timeout=30) + password = first_present(driver, ((By.ID, "password"), (By.NAME, "password")), timeout=30) + user.clear() + user.send_keys(OIDC_TEST_USER) + password.clear() + password.send_keys(OIDC_TEST_PASSWORD) + submit = first_clickable( + driver, + ( + (By.ID, "submit-form"), + (By.CSS_SELECTOR, "button[type='submit']"), + (By.CSS_SELECTOR, "input[type='submit']"), + ), + timeout=10, + ) + submit.click() + + +def grant_consent_if_present(driver): + if "/apps/oidc/consent" not in driver.current_url and not driver.find_elements(By.ID, "oidc-consent"): + return False + + try: + allow = first_clickable( + driver, + ( + (By.XPATH, "//button[normalize-space()='Allow']"), + (By.XPATH, "//button[contains(normalize-space(.), 'Allow')]"), + ), + timeout=15, + ) + log("Granting consent") + allow.click() + return True + except NoSuchElementException: + return False + + +def drive_url(driver, method, url): + log(f"Visiting {method} {url}") + if method.upper() == "POST": + submit_post(driver, url) + else: + driver.get(url) + + deadline = time.monotonic() + VISIT_TIMEOUT_SECONDS + while time.monotonic() < deadline: + current_url = driver.current_url + if "/test/a/" in current_url or "/test-mtls/a/" in current_url: + log(f"Reached conformance callback {current_url}") + return current_url + + if is_login_page(driver): + login(driver) + continue + + if grant_consent_if_present(driver): + continue + + time.sleep(0.5) + + log(f"Timed out waiting for callback; current URL is {driver.current_url}") + return driver.current_url + + +def get_browser_items(status): + browser = status.get("browser") or {} + items = browser.get("urlsWithMethod") or [] + if items: + return [ + {"url": item["url"], "method": item.get("method") or "GET"} + for item in items + if item.get("url") + ] + return [{"url": url, "method": "GET"} for url in browser.get("urls", [])] + + +def main(): + drivers = {} + processed = set() + active_test_id = None + + with httpx.Client(verify=False, timeout=15.0) as client: + while True: + try: + running = client.get(f"{CONFORMANCE_SERVER}/api/runner/running").json() + except Exception as exc: + log(f"Unable to read running tests: {exc}") + time.sleep(POLL_SECONDS) + continue + + for test_id in running: + try: + status = client.get(f"{CONFORMANCE_SERVER}/api/runner/{test_id}").json() + except Exception as exc: + log(f"Unable to read status for {test_id}: {exc}") + continue + + for item in get_browser_items(status): + url = item["url"] + method = item["method"].upper() + key = (test_id, method, url) + if key in processed: + continue + + if active_test_id is not None and active_test_id != test_id: + for old_test_id, old_driver in list(drivers.items()): + if old_test_id != test_id: + log(f"Closing browser session for {old_test_id}") + old_driver.quit() + drivers.pop(old_test_id, None) + + active_test_id = test_id + driver = drivers.setdefault(test_id, new_driver()) + processed.add(key) + + try: + drive_url(driver, method, url) + except Exception as exc: + log(f"Browser visit failed for {test_id}: {exc}") + finally: + try: + client.post( + f"{CONFORMANCE_SERVER}/api/runner/browser/{test_id}/visit", + params={"url": url}, + ) + except Exception as exc: + log(f"Unable to mark URL visited for {test_id}: {exc}") + + time.sleep(POLL_SECONDS) + + +if __name__ == "__main__": + try: + main() + except KeyboardInterrupt: + sys.exit(0) diff --git a/.github/conformance/docker-compose.chrome.yml b/.github/conformance/docker-compose.chrome.yml index e2bbbccc..3bd4ca49 100644 --- a/.github/conformance/docker-compose.chrome.yml +++ b/.github/conformance/docker-compose.chrome.yml @@ -2,8 +2,9 @@ services: # Selenium Chrome service for conformance testing # Provides WebDriver endpoint at http://chrome:4444/wd/hub chrome: - image: seleniarm/standalone-chromium:latest + image: selenium/standalone-chromium:latest container_name: conformance-chrome + shm_size: 2gb ports: - "4444:4444" - "7900:7900" # VNC port for debugging @@ -11,8 +12,8 @@ services: - SE_NODE_MAX_SESSIONS=3 - SE_SESSION_REQUEST_TIMEOUT=60 - SE_SESSION_RETRY_INTERVAL=2 - - CHROME_OPTIONS=--headless=new,--no-sandbox,--disable-dev-shm-usage - volumes: - - /dev/shm:/dev/shm # Shared memory for Chrome stability + - SE_BROWSER_ARGS_HEADLESS=--headless=new + - SE_BROWSER_ARGS_NO_SANDBOX=--no-sandbox + - SE_BROWSER_ARGS_DISABLE_DEV_SHM_USAGE=--disable-dev-shm-usage extra_hosts: - "host.docker.internal:host-gateway" diff --git a/.github/conformance/oidc-basic-config.json b/.github/conformance/oidc-basic-config.json index e0f67132..cfc11cd3 100644 --- a/.github/conformance/oidc-basic-config.json +++ b/.github/conformance/oidc-basic-config.json @@ -16,46 +16,5 @@ "client2": { "client_id": "${OIDC_CLIENT_ID_2}", "client_secret": "${OIDC_CLIENT_SECRET_2}" - }, - "browser": [ - { - "match": "${NEXTCLOUD_BASE_URL}/*", - "tasks": [ - { - "task": "Login", - "match": "${NEXTCLOUD_BASE_URL}/index.php/login*", - "optional": true, - "commands": [ - [ - "text", - "id", - "user", - "${OIDC_TEST_USER}" - ], - [ - "text", - "id", - "password", - "${OIDC_TEST_PASSWORD}" - ], - [ - "click", - "id", - "submit-form" - ], - [ - "wait", - "contains", - "/test/a/", - 30 - ] - ] - }, - { - "task": "Verify", - "match": "https://nginx:8443/test/a/${CONFORMANCE_ALIAS}/callback*" - } - ] - } - ] + } } diff --git a/.github/conformance/run-conformance-chrome.sh b/.github/conformance/run-conformance-chrome.sh index c9c4958c..06235140 100644 --- a/.github/conformance/run-conformance-chrome.sh +++ b/.github/conformance/run-conformance-chrome.sh @@ -29,6 +29,12 @@ if ! command -v chromedriver &> /dev/null; then exit 1 fi +if ! python3 -c "import httpx, pyparsing, selenium" &> /dev/null; then + echo -e "${RED}Python conformance dependencies not found${NC}" + echo "Install with: python3 -m pip install --user httpx pyparsing selenium" + exit 1 +fi + # Verify Chrome and Chromedriver versions match (major version) CHROME_VERSION=$(google-chrome --version | awk '{print $3}' | cut -d. -f1) DRIVER_VERSION=$(chromedriver --version | awk '{print $2}' | cut -d. -f1) @@ -71,13 +77,12 @@ done export CONFORMANCE_DEV_MODE=1 export CONFORMANCE_SERVER=https://nginx:8443/ export CONFORMANCE_SERVER_MTLS=https://nginx:8444/ -export SELENIUM_HUB_HOST=http://localhost:9515 -export BROWSER_DRIVER=chrome -export HEADLESS_BROWSER=true +export SELENIUM_REMOTE_URL=http://localhost:9515 +export OIDC_TEST_USER=${OIDC_TEST_USER:-oidc-test-user} +export OIDC_TEST_PASSWORD=${OIDC_TEST_PASSWORD:-oidc-test-password} echo -e "\n${YELLOW}Environment variables set:${NC}" -echo " SELENIUM_HUB_HOST=$SELENIUM_HUB_HOST" -echo " BROWSER_DRIVER=$BROWSER_DRIVER" +echo " SELENIUM_REMOTE_URL=$SELENIUM_REMOTE_URL" echo " CONFORMANCE_SERVER=$CONFORMANCE_SERVER" # Check if conformance-suite exists @@ -126,6 +131,9 @@ fi echo -e "\n${YELLOW}Running conformance tests...${NC}" echo "==================================================" cd conformance-suite +python3 ../.github/conformance/browser-runner.py > ../conformance-browser.log 2>&1 & +BROWSER_RUNNER_PID=$! +set +e python3 scripts/run-test-plan.py \ --export-dir ../conformance-results \ --verbose \ @@ -133,6 +141,7 @@ python3 scripts/run-test-plan.py \ ../conformance-config/oidc-basic-config.json TEST_RESULT=$? +set -e cd .. # Cleanup @@ -141,6 +150,9 @@ if [ -f /tmp/chromedriver.pid ]; then kill $(cat /tmp/chromedriver.pid) 2>/dev/null || true rm /tmp/chromedriver.pid fi +if [ -n "${BROWSER_RUNNER_PID:-}" ]; then + kill "$BROWSER_RUNNER_PID" 2>/dev/null || true +fi if [ $TEST_RESULT -eq 0 ]; then echo -e "\n${GREEN}✓ Conformance tests completed successfully${NC}" diff --git a/.github/workflows/build-test.yaml b/.github/workflows/build-test.yaml index 092bb678..de7c4fd6 100644 --- a/.github/workflows/build-test.yaml +++ b/.github/workflows/build-test.yaml @@ -332,7 +332,7 @@ jobs: exit 1 - name: Install conformance runner dependencies - run: python3 -m pip install --user httpx pyparsing + run: python3 -m pip install --user httpx pyparsing selenium - name: Write conformance plan config run: | @@ -347,12 +347,14 @@ jobs: CONFORMANCE_DEV_MODE: 1 CONFORMANCE_SERVER: https://nginx:8443/ CONFORMANCE_SERVER_MTLS: https://nginx:8444/ - SELENIUM_HUB_HOST: http://chrome:4444 - BROWSER_DRIVER: chrome - HEADLESS_BROWSER: true - WEBDRIVER_CHROME_OPTIONS: "--headless=new,--no-sandbox,--disable-dev-shm-usage" + SELENIUM_REMOTE_URL: http://chrome:4444/wd/hub + CONFORMANCE_BROWSER_VISIT_TIMEOUT: 90 run: | cd conformance-suite + python3 ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/browser-runner.py \ + > ../conformance-browser.log 2>&1 & + browser_runner_pid=$! + trap 'kill "${browser_runner_pid}" 2>/dev/null || true' EXIT python3 scripts/run-test-plan.py \ --export-dir ../conformance-results \ --verbose \ @@ -366,4 +368,5 @@ jobs: name: oidc-conformance-results path: | conformance-results + conformance-browser.log nextcloud-server.log diff --git a/appinfo/routes.php b/appinfo/routes.php index 52a48fc0..de236caf 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -184,6 +184,11 @@ 'url' => '/authorize', 'verb' => 'GET', ], + [ + 'name' => 'LoginRedirector#authorize', + 'url' => '/authorize', + 'verb' => 'POST', + ], [ 'name' => 'Cors#authorizeCorsResponse', 'url' => '/authorize', From 9fb6736916c4c69e7319b044048316ccae400edb Mon Sep 17 00:00:00 2001 From: Thorsten Jagel Date: Tue, 9 Jun 2026 15:44:02 +0200 Subject: [PATCH 08/28] Add debug output --- .github/workflows/build-test.yaml | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build-test.yaml b/.github/workflows/build-test.yaml index de7c4fd6..655af8ed 100644 --- a/.github/workflows/build-test.yaml +++ b/.github/workflows/build-test.yaml @@ -351,15 +351,33 @@ jobs: CONFORMANCE_BROWSER_VISIT_TIMEOUT: 90 run: | cd conformance-suite - python3 ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/browser-runner.py \ - > ../conformance-browser.log 2>&1 & + : > ../conformance-browser.log + PYTHONUNBUFFERED=1 python3 ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/browser-runner.py \ + >> ../conformance-browser.log 2>&1 & browser_runner_pid=$! - trap 'kill "${browser_runner_pid}" 2>/dev/null || true' EXIT + tail -n +1 -f ../conformance-browser.log & + browser_log_tail_pid=$! + cleanup_browser_runner() { + kill "${browser_runner_pid}" 2>/dev/null || true + kill "${browser_log_tail_pid}" 2>/dev/null || true + wait "${browser_runner_pid}" 2>/dev/null || true + wait "${browser_log_tail_pid}" 2>/dev/null || true + } + trap cleanup_browser_runner EXIT + set +e python3 scripts/run-test-plan.py \ --export-dir ../conformance-results \ --verbose \ "oidcc-basic-certification-test-plan[server_metadata=discovery][client_registration=static_client]" \ ../conformance-config/oidc-basic-config.json + test_plan_status=$? + set -e + cleanup_browser_runner + trap - EXIT + echo "::group::conformance-browser.log" + cat ../conformance-browser.log || true + echo "::endgroup::" + exit "${test_plan_status}" - name: Upload conformance results if: always() From 26f93acf78e39113bcb073d4bb97475fe97ae541 Mon Sep 17 00:00:00 2001 From: Thorsten Jagel Date: Tue, 9 Jun 2026 16:03:13 +0200 Subject: [PATCH 09/28] Fixed browser runner --- .github/conformance/browser-runner.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.github/conformance/browser-runner.py b/.github/conformance/browser-runner.py index 94d7b1c0..7ed65138 100644 --- a/.github/conformance/browser-runner.py +++ b/.github/conformance/browser-runner.py @@ -157,6 +157,14 @@ def grant_consent_if_present(driver): return False +def is_conformance_callback(url): + parsed = urllib.parse.urlsplit(url) + return parsed.netloc == "nginx:8443" and ( + parsed.path.startswith("/test/a/") + or parsed.path.startswith("/test-mtls/a/") + ) + + def drive_url(driver, method, url): log(f"Visiting {method} {url}") if method.upper() == "POST": @@ -167,7 +175,7 @@ def drive_url(driver, method, url): deadline = time.monotonic() + VISIT_TIMEOUT_SECONDS while time.monotonic() < deadline: current_url = driver.current_url - if "/test/a/" in current_url or "/test-mtls/a/" in current_url: + if is_conformance_callback(current_url): log(f"Reached conformance callback {current_url}") return current_url From 745f5c87bfce2cd8121e3c9ecd3d482ede0bbf0c Mon Sep 17 00:00:00 2001 From: Thorsten Jagel Date: Tue, 9 Jun 2026 16:18:11 +0200 Subject: [PATCH 10/28] Another fix --- .github/conformance/browser-runner.py | 29 +++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/.github/conformance/browser-runner.py b/.github/conformance/browser-runner.py index 7ed65138..3bfbac44 100644 --- a/.github/conformance/browser-runner.py +++ b/.github/conformance/browser-runner.py @@ -165,6 +165,26 @@ def is_conformance_callback(url): ) +def page_diagnostics(driver): + try: + ready_state = driver.execute_script("return document.readyState") + except Exception: + ready_state = "unavailable" + try: + body = driver.execute_script("return document.body ? document.body.innerText : ''") + except Exception: + body = "" + body = " ".join((body or "").split()) + if len(body) > 1000: + body = body[:1000] + "..." + return { + "url": driver.current_url, + "title": driver.title, + "ready_state": ready_state, + "body": body, + } + + def drive_url(driver, method, url): log(f"Visiting {method} {url}") if method.upper() == "POST": @@ -173,8 +193,13 @@ def drive_url(driver, method, url): driver.get(url) deadline = time.monotonic() + VISIT_TIMEOUT_SECONDS + last_seen_url = None while time.monotonic() < deadline: current_url = driver.current_url + if current_url != last_seen_url: + last_seen_url = current_url + log(f"Browser at {current_url}") + if is_conformance_callback(current_url): log(f"Reached conformance callback {current_url}") return current_url @@ -189,6 +214,10 @@ def drive_url(driver, method, url): time.sleep(0.5) log(f"Timed out waiting for callback; current URL is {driver.current_url}") + diag = page_diagnostics(driver) + log(f"Timeout page title: {diag['title']}") + log(f"Timeout page readyState: {diag['ready_state']}") + log(f"Timeout page body: {diag['body']}") return driver.current_url From 3dacc372288815d5b2aa51cccf4fb89d6200a8c8 Mon Sep 17 00:00:00 2001 From: Thorsten Jagel Date: Tue, 9 Jun 2026 16:38:59 +0200 Subject: [PATCH 11/28] Update route --- appinfo/routes.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/appinfo/routes.php b/appinfo/routes.php index de236caf..76233b51 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -183,11 +183,13 @@ 'name' => 'LoginRedirector#authorize', 'url' => '/authorize', 'verb' => 'GET', + 'postfix' => 'get', ], [ 'name' => 'LoginRedirector#authorize', 'url' => '/authorize', 'verb' => 'POST', + 'postfix' => 'post', ], [ 'name' => 'Cors#authorizeCorsResponse', From 84fc0d0cbb37966f2cd8e532162d7fcb3a726e0a Mon Sep 17 00:00:00 2001 From: Thorsten Jagel Date: Wed, 10 Jun 2026 08:55:32 +0200 Subject: [PATCH 12/28] Update routes --- appinfo/routes.php | 4 +- lib/Controller/LoginRedirectorController.php | 45 ++++++++++++++++++++ 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/appinfo/routes.php b/appinfo/routes.php index 76233b51..fd337ced 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -183,13 +183,11 @@ 'name' => 'LoginRedirector#authorize', 'url' => '/authorize', 'verb' => 'GET', - 'postfix' => 'get', ], [ - 'name' => 'LoginRedirector#authorize', + 'name' => 'LoginRedirector#authorizePost', 'url' => '/authorize', 'verb' => 'POST', - 'postfix' => 'post', ], [ 'name' => 'Cors#authorizeCorsResponse', diff --git a/lib/Controller/LoginRedirectorController.php b/lib/Controller/LoginRedirectorController.php index b186691c..bc264cc8 100644 --- a/lib/Controller/LoginRedirectorController.php +++ b/lib/Controller/LoginRedirectorController.php @@ -144,6 +144,51 @@ public function __construct( $this->logger = $logger; } + /** + * @PublicPage + * @NoCSRFRequired + * @UseSession + * @BruteForceProtection(action=oidc_login) + * + * @param string $client_id + * @param string $state + * @param string $response_type + * @param string $redirect_uri + * @param string $scope + * @param string $nonce + * @param string $resource + * @param string $code_challenge + * @param string $code_challenge_method + * @return Response + */ + #[BruteForceProtection(action: 'oidc_login')] + #[NoCSRFRequired] + #[UseSession] + #[PublicPage] + public function authorizePost( + $client_id, + $state, + $response_type, + $redirect_uri, + $scope, + $nonce, + $resource = null, + $code_challenge = null, + $code_challenge_method = null + ): Response + { + return authorize( + $client_id, + $state, + $response_type, + $redirect_uri, + $scope, + $nonce, + $resource, + $code_challenge, + $code_challenge_method); + } + /** * @PublicPage * @NoCSRFRequired From 50301175049b2640dd3f846fab1c310e3ff5b18d Mon Sep 17 00:00:00 2001 From: Thorsten Jagel Date: Wed, 10 Jun 2026 09:27:03 +0200 Subject: [PATCH 13/28] Fix browser-runner --- .github/conformance/browser-runner.py | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/.github/conformance/browser-runner.py b/.github/conformance/browser-runner.py index 3bfbac44..9945f4af 100644 --- a/.github/conformance/browser-runner.py +++ b/.github/conformance/browser-runner.py @@ -233,6 +233,25 @@ def get_browser_items(status): return [{"url": url, "method": "GET"} for url in browser.get("urls", [])] +def get_driver(drivers, test_id): + driver = drivers.get(test_id) + if driver is None: + driver = new_driver() + drivers[test_id] = driver + return driver + + +def close_driver(drivers, test_id): + driver = drivers.pop(test_id, None) + if driver is None: + return + log(f"Closing browser session for {test_id}") + try: + driver.quit() + except Exception as exc: + log(f"Unable to close browser session for {test_id}: {exc}") + + def main(): drivers = {} processed = set() @@ -262,14 +281,12 @@ def main(): continue if active_test_id is not None and active_test_id != test_id: - for old_test_id, old_driver in list(drivers.items()): + for old_test_id in list(drivers): if old_test_id != test_id: - log(f"Closing browser session for {old_test_id}") - old_driver.quit() - drivers.pop(old_test_id, None) + close_driver(drivers, old_test_id) active_test_id = test_id - driver = drivers.setdefault(test_id, new_driver()) + driver = get_driver(drivers, test_id) processed.add(key) try: From baeb7fcb0089682de15769dbda17b714f0a24fde Mon Sep 17 00:00:00 2001 From: Thorsten Jagel Date: Wed, 10 Jun 2026 10:36:55 +0200 Subject: [PATCH 14/28] Fixes for workflow --- .github/conformance/browser-runner.py | 14 ++ .github/conformance/generate-report.py | 193 +++++++++++++++++++ .github/workflows/build-test.yaml | 9 + lib/Controller/LoginRedirectorController.php | 2 +- lib/Controller/OIDCApiController.php | 10 +- 5 files changed, 226 insertions(+), 2 deletions(-) create mode 100644 .github/conformance/generate-report.py diff --git a/.github/conformance/browser-runner.py b/.github/conformance/browser-runner.py index 9945f4af..2cef57d8 100644 --- a/.github/conformance/browser-runner.py +++ b/.github/conformance/browser-runner.py @@ -27,6 +27,7 @@ OIDC_TEST_PASSWORD = os.environ["OIDC_TEST_PASSWORD"] POLL_SECONDS = float(os.environ.get("CONFORMANCE_BROWSER_POLL_SECONDS", "1")) VISIT_TIMEOUT_SECONDS = int(os.environ.get("CONFORMANCE_BROWSER_VISIT_TIMEOUT", "90")) +LOGIN_REDIRECT_TIMEOUT_SECONDS = int(os.environ.get("CONFORMANCE_BROWSER_LOGIN_REDIRECT_TIMEOUT", "15")) def log(message): @@ -118,6 +119,7 @@ def is_login_page(driver): def login(driver): + login_url = driver.current_url log(f"Logging in at {driver.current_url}") user = first_present(driver, ((By.ID, "user"), (By.NAME, "user")), timeout=30) password = first_present(driver, ((By.ID, "password"), (By.NAME, "password")), timeout=30) @@ -135,6 +137,18 @@ def login(driver): timeout=10, ) submit.click() + wait_for_login_redirect(driver, login_url) + + +def wait_for_login_redirect(driver, login_url): + deadline = time.monotonic() + LOGIN_REDIRECT_TIMEOUT_SECONDS + while time.monotonic() < deadline: + current_url = driver.current_url + if current_url != login_url: + return + if not is_login_page(driver): + return + time.sleep(0.2) def grant_consent_if_present(driver): diff --git a/.github/conformance/generate-report.py b/.github/conformance/generate-report.py new file mode 100644 index 00000000..c04dc19f --- /dev/null +++ b/.github/conformance/generate-report.py @@ -0,0 +1,193 @@ +#!/usr/bin/env python3 +"""Create a Markdown summary from OpenID conformance-suite exports.""" + +from __future__ import annotations + +import argparse +import datetime as dt +import json +import pathlib +import re +import zipfile +from collections import Counter + + +ISSUE_RESULTS = {"FAILURE", "ERROR", "WARNING"} +RESULT_ORDER = { + "FAILED": 0, + "FAILURE": 0, + "ERROR": 1, + "INTERRUPTED": 2, + "WARNING": 3, + "PASSED": 4, + "SUCCESS": 4, + "SKIPPED": 5, + "UNKNOWN": 6, +} + + +def compact(value: object) -> str: + return re.sub(r"\s+", " ", str(value or "")).strip() + + +def table_cell(value: object) -> str: + text = compact(value) + return text.replace("|", "\\|") + + +def truncate(value: str, max_length: int) -> str: + if len(value) <= max_length: + return value + return value[: max_length - 3].rstrip() + "..." + + +def iter_export_logs(results_dir: pathlib.Path): + for zip_path in sorted(results_dir.glob("*.zip")): + with zipfile.ZipFile(zip_path) as export: + for name in sorted(export.namelist()): + if not name.endswith(".json"): + continue + with export.open(name) as handle: + yield zip_path.name, name, json.load(handle) + + for json_path in sorted(results_dir.glob("test-log-*.json")): + with json_path.open(encoding="utf-8") as handle: + yield json_path.name, json_path.name, json.load(handle) + + +def normalized_result(test_info: dict) -> str: + result = test_info.get("result") + if result: + return str(result) + status = test_info.get("status") + if status and status != "FINISHED": + return str(status) + return "UNKNOWN" + + +def issue_summary(results: list[dict], max_issues: int) -> str: + issues = [ + f"{item.get('result')}: {compact(item.get('msg'))}" + for item in results + if item.get("result") in ISSUE_RESULTS and item.get("msg") + ] + if not issues: + return "" + extra = len(issues) - max_issues + selected = issues[:max_issues] + if extra > 0: + selected.append(f"+ {extra} more") + return "; ".join(selected) + + +def collect_tests(results_dir: pathlib.Path, max_issues: int) -> list[dict]: + tests = [] + for export_name, log_name, data in iter_export_logs(results_dir): + test_info = data.get("testInfo", {}) + test_results = data.get("results", []) + result = normalized_result(test_info) + tests.append( + { + "name": test_info.get("testName", pathlib.Path(log_name).stem), + "id": test_info.get("testId", test_info.get("_id", "")), + "status": test_info.get("status", ""), + "result": result, + "summary": compact(test_info.get("summary")), + "issues": issue_summary(test_results, max_issues), + "started": test_info.get("started", ""), + "export": export_name, + } + ) + return sorted( + tests, + key=lambda item: ( + RESULT_ORDER.get(item["result"], RESULT_ORDER["UNKNOWN"]), + item["name"], + item["id"], + ), + ) + + +def write_report(tests: list[dict], output: pathlib.Path, results_dir: pathlib.Path) -> None: + now = dt.datetime.now(dt.UTC).replace(microsecond=0).isoformat() + counts = Counter(test["result"] for test in tests) + + lines = [ + "# OIDC Conformance Report", + "", + f"Generated: {now}", + f"Results source: `{results_dir}`", + f"Executed tests: {len(tests)}", + "", + "## Result Summary", + "", + "| Result | Count |", + "| --- | ---: |", + ] + + if counts: + for result, count in sorted(counts.items(), key=lambda item: RESULT_ORDER.get(item[0], RESULT_ORDER["UNKNOWN"])): + lines.append(f"| {table_cell(result)} | {count} |") + else: + lines.append("| UNKNOWN | 0 |") + + lines.extend( + [ + "", + "## Executed Tests", + "", + "| Result | Status | Test | Description | Issues |", + "| --- | --- | --- | --- | --- |", + ] + ) + + if tests: + for test in tests: + test_name = test["name"] + if test["id"]: + test_name = f"{test_name} ({test['id']})" + lines.append( + "| " + + " | ".join( + [ + table_cell(test["result"]), + table_cell(test["status"]), + table_cell(test_name), + table_cell(truncate(test["summary"], 260)), + table_cell(truncate(test["issues"], 320)), + ] + ) + + " |" + ) + else: + lines.append("| UNKNOWN | UNKNOWN | No conformance test logs found | | |") + + lines.extend( + [ + "", + "## Notes", + "", + "- `WARNING` is reported by the conformance suite for behavior that may still complete the test module.", + "- `INTERRUPTED` means the test module did not reach a final pass/fail result in the exported run.", + "- Full logs and signed test exports are available in the workflow artifact.", + "", + ] + ) + + output.parent.mkdir(parents=True, exist_ok=True) + output.write_text("\n".join(lines), encoding="utf-8") + + +def main() -> None: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--results-dir", default="conformance-results", type=pathlib.Path) + parser.add_argument("--output", default="CONFORMANCE.md", type=pathlib.Path) + parser.add_argument("--max-issues", default=3, type=int) + args = parser.parse_args() + + tests = collect_tests(args.results_dir, args.max_issues) + write_report(tests, args.output, args.results_dir) + + +if __name__ == "__main__": + main() diff --git a/.github/workflows/build-test.yaml b/.github/workflows/build-test.yaml index 655af8ed..2c4565f9 100644 --- a/.github/workflows/build-test.yaml +++ b/.github/workflows/build-test.yaml @@ -379,12 +379,21 @@ jobs: echo "::endgroup::" exit "${test_plan_status}" + - name: Generate conformance report + if: always() + run: | + python3 nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/generate-report.py \ + --results-dir conformance-results \ + --output CONFORMANCE.md + cat CONFORMANCE.md + - name: Upload conformance results if: always() uses: actions/upload-artifact@v4 with: name: oidc-conformance-results path: | + CONFORMANCE.md conformance-results conformance-browser.log nextcloud-server.log diff --git a/lib/Controller/LoginRedirectorController.php b/lib/Controller/LoginRedirectorController.php index bc264cc8..ddb91aa0 100644 --- a/lib/Controller/LoginRedirectorController.php +++ b/lib/Controller/LoginRedirectorController.php @@ -177,7 +177,7 @@ public function authorizePost( $code_challenge_method = null ): Response { - return authorize( + return $this->authorize( $client_id, $state, $response_type, diff --git a/lib/Controller/OIDCApiController.php b/lib/Controller/OIDCApiController.php index a8189e05..4b3d6c45 100644 --- a/lib/Controller/OIDCApiController.php +++ b/lib/Controller/OIDCApiController.php @@ -190,12 +190,20 @@ public function getToken( $code = $refresh_token; } + if ($code === null || trim($code) === '') { + $this->logger->info('Missing code or refresh_token for client id ' . $client_id . '.'); + return new JSONResponse([ + 'error' => 'invalid_request', + 'error_description' => 'Missing code or refresh_token.', + ], Http::STATUS_BAD_REQUEST); + } + try { $accessToken = $this->accessTokenMapper->getByCode($code); } catch (AccessTokenNotFoundException $e) { $this->logger->info('Could not find access token for code or refresh_token for client id ' . $client_id . '.'); return new JSONResponse([ - 'error' => 'invalid_request', + 'error' => 'invalid_grant', 'error_description' => 'Could not find access token for code or refresh_token.', ], Http::STATUS_BAD_REQUEST); } From 8465b1d2a669ad604b48c1c744e0f4cad47e081a Mon Sep 17 00:00:00 2001 From: Thorsten Jagel Date: Wed, 10 Jun 2026 11:51:41 +0200 Subject: [PATCH 15/28] Fix missing response type --- lib/Controller/LoginRedirectorController.php | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/Controller/LoginRedirectorController.php b/lib/Controller/LoginRedirectorController.php index ddb91aa0..51938640 100644 --- a/lib/Controller/LoginRedirectorController.php +++ b/lib/Controller/LoginRedirectorController.php @@ -285,8 +285,8 @@ public function authorize( // Guard: if critical OAuth params are still missing after session fallback, // return a meaningful error instead of letting downstream code crash with a 500 // (e.g. matchRedirectUri(null, ...) or trim(null) in PHP 8.4). - // Note: state is not critical for processing the request and might also not be passed by the client, e.g Guacamole, so we only check response_type and redirect_uri here. - if (empty($response_type) || empty($redirect_uri)) { + // Note: state is not critical for processing the request and might also not be passed by the client, e.g Guacamole. + if (empty($redirect_uri)) { $this->logger->error('Missing critical OAuth params after session fallback: ' . 'response_type=' . var_export($response_type, true) . ', ' . 'redirect_uri=' . var_export($redirect_uri, true)); @@ -370,6 +370,16 @@ public function authorize( return new TemplateResponse('core', '403', $params, 'error'); } + if (empty($response_type)) { + $this->logger->notice('Missing response_type in request for client ' . $client_id . '.'); + $separator = str_contains($redirect_uri, '?') ? '&' : '?'; + $url = $redirect_uri . $separator + . 'error=unsupported_response_type' + . '&error_description=Missing%20response_type' + . '&state=' . urlencode((string)$state); + return new RedirectResponse($url); + } + // Check response type $responseTypeEntries = explode(' ', strtolower(trim($response_type)), 3); $codeFlow = false; From 8931b3f75437442e5f4d9c23df350ff30b1d1c53 Mon Sep 17 00:00:00 2001 From: Thorsten Jagel Date: Wed, 10 Jun 2026 13:38:03 +0200 Subject: [PATCH 16/28] Fix oidcc-refresh-token compliance --- lib/Controller/OIDCApiController.php | 57 +++++++++++------- lib/Util/DiscoveryGenerator.php | 1 + tests/Integration/OIDCCodeFlowTest.php | 67 ++++++++++++++++++---- tests/Unit/Util/DiscoveryGeneratorTest.php | 2 +- 4 files changed, 95 insertions(+), 32 deletions(-) diff --git a/lib/Controller/OIDCApiController.php b/lib/Controller/OIDCApiController.php index 4b3d6c45..9326ef15 100644 --- a/lib/Controller/OIDCApiController.php +++ b/lib/Controller/OIDCApiController.php @@ -208,21 +208,11 @@ public function getToken( ], Http::STATUS_BAD_REQUEST); } - try { - $client = $this->clientMapper->getByUid($accessToken->getClientId()); - } catch (ClientNotFoundException $e) { - $this->logger->error('Could not find client for access token. Client id was ' . $client_id . '.'); - return new JSONResponse([ - 'error' => 'invalid_request', - 'error_description' => 'Could not find client for access token.', - ], Http::STATUS_BAD_REQUEST); - } - if (!isset($client_id)) { $this->logger->debug('No client_id in request. Trying to fetch from Authorization Header.'); if (isset($this->request->server['PHP_AUTH_USER'])) { $client_id = $this->request->server['PHP_AUTH_USER']; - $client_secret = $this->request->server['PHP_AUTH_PW']; + $client_secret = $this->request->server['PHP_AUTH_PW'] ?? null; } if (!isset($client_id)) { $this->logger->debug('No client_id in PHP_AUTH_USER superglobal. Trying to fetch from Authorization Header directly.'); @@ -239,18 +229,37 @@ public function getToken( } } + if ($client_id === null || trim($client_id) === '') { + $this->logger->info('Missing client_id in token request.'); + return new JSONResponse([ + 'error' => 'invalid_client', + 'error_description' => 'Missing client_id.', + ], Http::STATUS_BAD_REQUEST); + } + + try { + $client = $this->clientMapper->getByIdentifier($client_id); + } catch (ClientNotFoundException $e) { + $this->logger->info('Client not found. Client id was ' . $client_id . '.'); + return new JSONResponse([ + 'error' => 'invalid_client', + 'error_description' => 'Client not found.', + ], Http::STATUS_BAD_REQUEST); + } + if ($client === null) { + $this->logger->info('Client not found. Client id was ' . $client_id . '.'); + return new JSONResponse([ + 'error' => 'invalid_client', + 'error_description' => 'Client not found.', + ], Http::STATUS_BAD_REQUEST); + } + if ($client->getType() === 'public') { - // Only the client id must match for a public client. Else we don't provide an access token! - if ($client->getClientIdentifier() !== $client_id) { - $this->logger->info('Client not found. Client id was ' . $client_id . '.'); - return new JSONResponse([ - 'error' => 'invalid_client', - 'error_description' => 'Client not found.', - ], Http::STATUS_BAD_REQUEST); - } + // Only the client id must be present for a public client. + $this->logger->debug('Authenticated public client. Client id was ' . $client_id . '.'); } else { // The client id and secret must match. Else we don't provide an access token! - if ($client->getClientIdentifier() !== $client_id || $client->getSecret() !== $client_secret) { + if ($client->getSecret() !== $client_secret) { $this->logger->error('Client authentication failed. Client id was ' . $client_id . '.'); return new JSONResponse([ 'error' => 'invalid_client', @@ -259,6 +268,14 @@ public function getToken( } } + if ($accessToken->getClientId() !== $client->getId()) { + $this->logger->info('Grant is not valid for client id ' . $client_id . '.'); + return new JSONResponse([ + 'error' => 'invalid_grant', + 'error_description' => 'Grant is not valid for this client.', + ], Http::STATUS_BAD_REQUEST); + } + // The client must not be expired if ($client->isDcr() && $this->time->getTime() > ($client->getIssuedAt() + (int)$this->appConfig->getAppValueString(Application::APP_CONFIG_DEFAULT_CLIENT_EXPIRE_TIME, Application::DEFAULT_CLIENT_EXPIRE_TIME))) { $this->logger->warning('Client expired. Client id was ' . $client_id . '.'); diff --git a/lib/Util/DiscoveryGenerator.php b/lib/Util/DiscoveryGenerator.php index c7cc9270..fb322ab0 100644 --- a/lib/Util/DiscoveryGenerator.php +++ b/lib/Util/DiscoveryGenerator.php @@ -125,6 +125,7 @@ public function generateDiscovery(IRequest $request): JSONResponse $grantTypesSupported = [ 'authorization_code', 'implicit', + 'refresh_token', ]; $acrValuesSupported = [ '0', diff --git a/tests/Integration/OIDCCodeFlowTest.php b/tests/Integration/OIDCCodeFlowTest.php index b9e4d7f8..ef5505ea 100644 --- a/tests/Integration/OIDCCodeFlowTest.php +++ b/tests/Integration/OIDCCodeFlowTest.php @@ -97,6 +97,12 @@ class OIDCCodeFlowTest extends \Test\TestCase /** @var string */ private $testClientSecret = 'test-secret'; + /** @var string */ + private $secondTestClientId = 'test-client-2'; + + /** @var string */ + private $secondTestClientSecret = 'test-secret-2'; + /** @var string */ private $testRedirectUri = 'https://client.example.com/callback'; @@ -244,14 +250,16 @@ protected function tearDown(): void private function cleanupTestData(): void { - try { - // Delete the test client (this will cascade and delete related redirect URIs and access tokens) - $client = $this->clientMapper->getByIdentifier($this->testClientId); - if ($client !== null) { - $this->clientMapper->delete($client); + foreach ([$this->testClientId, $this->secondTestClientId] as $clientId) { + try { + // Delete the test client (this will cascade and delete related redirect URIs and access tokens) + $client = $this->clientMapper->getByIdentifier($clientId); + if ($client !== null) { + $this->clientMapper->delete($client); + } + } catch (\Exception $e) { + // Ignore errors during cleanup } - } catch (\Exception $e) { - // Ignore errors during cleanup } try { @@ -265,11 +273,15 @@ private function cleanupTestData(): void } } - private function createTestClient(): Client + private function createTestClient( + string|null $clientId = null, + string|null $clientSecret = null, + string $name = 'Test Client' + ): Client { // Create client - the redirect URIs will be automatically created by ClientMapper $client = new Client( - 'Test Client', + $name, [$this->testRedirectUri], 'RS256', 'confidential', @@ -279,8 +291,8 @@ private function createTestClient(): Client '', false ); - $client->setClientIdentifier($this->testClientId); - $client->setSecret($this->testClientSecret); + $client->setClientIdentifier($clientId ?? $this->testClientId); + $client->setSecret($clientSecret ?? $this->testClientSecret); $insertedClient = $this->clientMapper->insert($client); @@ -437,6 +449,39 @@ public function testTokenExchangeWithInvalidClient(): void $this->assertEquals('invalid_client', $responseData['error'], 'Error should be invalid_client'); } + /** + * Test refresh token use with a different authenticated client + */ + public function testRefreshTokenForDifferentClientReturnsInvalidGrant(): void + { + $this->createTestClient(); + $secondClient = $this->createTestClient( + $this->secondTestClientId, + $this->secondTestClientSecret, + 'Second Test Client' + ); + $user = $this->createTestUser(); + + $tokenResult = $this->createAccessToken($secondClient, $user, 'openid offline_access'); + $refreshToken = $tokenResult['rawCode']; + + $response = $this->oidcApiController->getToken( + 'refresh_token', + null, + $refreshToken, + $this->testClientId, + $this->testClientSecret, + null + ); + + $this->assertInstanceOf(JSONResponse::class, $response, 'Response is not a JSONResponse'); + $this->assertEquals(400, $response->getStatus(), 'Token endpoint should reject refresh tokens from another client'); + + $responseData = $response->getData(); + $this->assertArrayHasKey('error', $responseData, 'Response missing error field'); + $this->assertEquals('invalid_grant', $responseData['error'], 'Error should be invalid_grant'); + } + /** * Test user info with invalid access token */ diff --git a/tests/Unit/Util/DiscoveryGeneratorTest.php b/tests/Unit/Util/DiscoveryGeneratorTest.php index 02224731..f3374111 100644 --- a/tests/Unit/Util/DiscoveryGeneratorTest.php +++ b/tests/Unit/Util/DiscoveryGeneratorTest.php @@ -155,7 +155,7 @@ public function testGenerateDiscoveryHasGrantTypesSupported() { $this->assertArrayHasKey('grant_types_supported', $data); $grantTypes = $data['grant_types_supported']; - $expectedTypes = ['authorization_code', 'implicit']; + $expectedTypes = ['authorization_code', 'implicit', 'refresh_token']; foreach ($expectedTypes as $type) { $this->assertContains($type, $grantTypes, "Missing grant type: $type"); } From 82e2cf64e68447613ff5429007b262b537b27157 Mon Sep 17 00:00:00 2001 From: Thorsten Jagel Date: Wed, 10 Jun 2026 13:48:51 +0200 Subject: [PATCH 17/28] Fix oidcc-scope-profile conformance error --- lib/Controller/UserInfoController.php | 8 +++++--- lib/Util/JwtGenerator.php | 16 ++++++++++------ .../Unit/Controller/DiscoveryControllerTest.php | 1 + tests/Unit/Controller/UserInfoControllerTest.php | 2 ++ tests/Unit/Util/JwtGeneratorTest.php | 10 +++++++++- 5 files changed, 27 insertions(+), 10 deletions(-) diff --git a/lib/Controller/UserInfoController.php b/lib/Controller/UserInfoController.php index 1d9b1555..6c601d1f 100644 --- a/lib/Controller/UserInfoController.php +++ b/lib/Controller/UserInfoController.php @@ -265,10 +265,12 @@ public function getInfo(): JSONResponse $names = $this->converter->splitFullName($displayName); $profile = array_merge($profile, [ 'name' => $displayName, - 'family_name' => $names[0], - 'given_name' => $names[1], - 'middle_name' => $names[2] ]); + foreach (['family_name', 'given_name', 'middle_name'] as $index => $claimName) { + if (isset($names[$index]) && trim($names[$index]) !== '') { + $profile[$claimName] = $names[$index]; + } + } } else { $profile = array_merge($profile, ['name' => $user->getDisplayName()]); } diff --git a/lib/Util/JwtGenerator.php b/lib/Util/JwtGenerator.php index e2a709f5..048523c6 100644 --- a/lib/Util/JwtGenerator.php +++ b/lib/Util/JwtGenerator.php @@ -228,10 +228,12 @@ public function generateIdToken(AccessToken $accessToken, Client $client, string $names = $this->converter->splitFullName($displayName); $profile = array_merge($profile, [ 'name' => $displayName, - 'family_name' => $names[0], - 'given_name' => $names[1], - 'middle_name' => $names[2] ]); + foreach (['family_name', 'given_name', 'middle_name'] as $index => $claimName) { + if (isset($names[$index]) && trim($names[$index]) !== '') { + $profile[$claimName] = $names[$index]; + } + } } else { $profile = array_merge($profile, ['name' => $user->getDisplayName()]); } @@ -427,10 +429,12 @@ public function generateAccessToken(AccessToken $accessToken, Client $client, st $names = $this->converter->splitFullName($displayName); $profile = array_merge($profile, [ 'name' => $displayName, - 'family_name' => $names[0], - 'given_name' => $names[1], - 'middle_name' => $names[2] ]); + foreach (['family_name', 'given_name', 'middle_name'] as $index => $claimName) { + if (isset($names[$index]) && trim($names[$index]) !== '') { + $profile[$claimName] = $names[$index]; + } + } } else { $profile = array_merge($profile, ['name' => $user->getDisplayName()]); } diff --git a/tests/Unit/Controller/DiscoveryControllerTest.php b/tests/Unit/Controller/DiscoveryControllerTest.php index f2adb004..310a5b1e 100644 --- a/tests/Unit/Controller/DiscoveryControllerTest.php +++ b/tests/Unit/Controller/DiscoveryControllerTest.php @@ -112,6 +112,7 @@ public function testDiscoveryResponse() { $grantTypesSupported = [ 'authorization_code', 'implicit', + 'refresh_token', ]; $acrValuesSupported = [ '0', diff --git a/tests/Unit/Controller/UserInfoControllerTest.php b/tests/Unit/Controller/UserInfoControllerTest.php index 05b494fe..405c3f3a 100644 --- a/tests/Unit/Controller/UserInfoControllerTest.php +++ b/tests/Unit/Controller/UserInfoControllerTest.php @@ -390,6 +390,8 @@ public function testGetInfoSuccess() { $this->assertEquals('user1', $data['preferred_username']); $this->assertArrayHasKey('scope', $data); $this->assertEquals('openid profile email', $data['scope']); + $this->assertEquals('Test User', $data['name']); + $this->assertArrayNotHasKey('middle_name', $data); } public function testGetInfoPostSuccess() { diff --git a/tests/Unit/Util/JwtGeneratorTest.php b/tests/Unit/Util/JwtGeneratorTest.php index 1288bbf6..405777b7 100644 --- a/tests/Unit/Util/JwtGeneratorTest.php +++ b/tests/Unit/Util/JwtGeneratorTest.php @@ -217,13 +217,19 @@ public function testGenerateIdToken() { $mockAccountProperty = $this->createMock(IAccountProperty::class); $mockAccountProperty->method('getValue')->willReturn(''); + $mockDisplayNameProperty = $this->createMock(IAccountProperty::class); + $mockDisplayNameProperty->method('getValue')->willReturn('Test User'); + // Special handling for email property $mockEmailProperty = $this->createMock(IAccountProperty::class); $mockEmailProperty->method('getValue')->willReturn($testEmail); $mockAccount ->method('getProperty') - ->willReturnCallback(function($prop) use ($mockEmailProperty, $mockAccountProperty) { + ->willReturnCallback(function($prop) use ($mockDisplayNameProperty, $mockEmailProperty, $mockAccountProperty) { + if ($prop === IAccountManager::PROPERTY_DISPLAYNAME) { + return $mockDisplayNameProperty; + } if ($prop === IAccountManager::PROPERTY_EMAIL) { return $mockEmailProperty; } @@ -287,6 +293,8 @@ public function testGenerateIdToken() { $this->assertEquals($client->getClientIdentifier(), $decodedJwt['aud']); $this->assertEquals($scope, $decodedJwt['scope']); $this->assertEquals($client->getClientIdentifier(), $decodedJwt['azp']); + $this->assertEquals('Test User', $decodedJwt['name']); + $this->assertArrayNotHasKey('middle_name', $decodedJwt); $this->assertArrayHasKey('email', $decodedJwt); $this->assertEquals('testuser@example.com', $decodedJwt['email']); $this->assertArrayHasKey('nonce', $decodedJwt); From c633157fb9a6e7e257871310a27a9aeeadc1fcee Mon Sep 17 00:00:00 2001 From: Thorsten Jagel Date: Wed, 10 Jun 2026 14:13:56 +0200 Subject: [PATCH 18/28] Fix oidcc-unsigned-request-object-supported-correctly-or-rejected-as-unsupported conformance error --- lib/Controller/LoginRedirectorController.php | 49 ++++++++++++++++ lib/Util/DiscoveryGenerator.php | 4 +- .../Controller/DiscoveryControllerTest.php | 2 + .../LoginRedirectorControllerTest.php | 56 +++++++++++++++++++ tests/Unit/Util/DiscoveryGeneratorTest.php | 10 ++++ 5 files changed, 119 insertions(+), 2 deletions(-) diff --git a/lib/Controller/LoginRedirectorController.php b/lib/Controller/LoginRedirectorController.php index 51938640..573ebe51 100644 --- a/lib/Controller/LoginRedirectorController.php +++ b/lib/Controller/LoginRedirectorController.php @@ -370,6 +370,28 @@ public function authorize( return new TemplateResponse('core', '403', $params, 'error'); } + $requestObject = $this->request->getParam('request'); + if ($this->hasNonEmptyRequestParameter($requestObject)) { + $this->logger->notice('Request object parameter is not supported for client ' . $client_id . '.'); + return $this->createAuthorizationErrorRedirect( + $redirect_uri, + 'request_not_supported', + 'Request object parameter is not supported.', + $state + ); + } + + $requestUri = $this->request->getParam('request_uri'); + if ($this->hasNonEmptyRequestParameter($requestUri)) { + $this->logger->notice('Request URI parameter is not supported for client ' . $client_id . '.'); + return $this->createAuthorizationErrorRedirect( + $redirect_uri, + 'request_uri_not_supported', + 'Request URI parameter is not supported.', + $state + ); + } + if (empty($response_type)) { $this->logger->notice('Missing response_type in request for client ' . $client_id . '.'); $separator = str_contains($redirect_uri, '?') ? '&' : '?'; @@ -614,4 +636,31 @@ public function authorize( return new RedirectResponse($url); } + + private function hasNonEmptyRequestParameter(mixed $value): bool + { + if (is_string($value)) { + return trim($value) !== ''; + } + + return !empty($value); + } + + private function createAuthorizationErrorRedirect( + string $redirectUri, + string $error, + string $errorDescription, + mixed $state + ): RedirectResponse { + $params = [ + 'error' => $error, + 'error_description' => $errorDescription, + ]; + if ($state !== null && trim((string)$state) !== '') { + $params['state'] = (string)$state; + } + + $separator = str_contains($redirectUri, '?') ? '&' : '?'; + return new RedirectResponse($redirectUri . $separator . http_build_query($params, '', '&', PHP_QUERY_RFC3986)); + } } diff --git a/lib/Util/DiscoveryGenerator.php b/lib/Util/DiscoveryGenerator.php index fb322ab0..5d2b5a74 100644 --- a/lib/Util/DiscoveryGenerator.php +++ b/lib/Util/DiscoveryGenerator.php @@ -218,8 +218,8 @@ public function generateDiscovery(IRequest $request): JSONResponse // 'claims_locales_supported' => , // 'ui_locales_supported' => , // 'claims_parameter_supported' => true, - // 'request_parameter_supported' => true, - // 'request_uri_parameter_supported' => false, + 'request_parameter_supported' => false, + 'request_uri_parameter_supported' => false, // 'require_request_uri_registration' => true, // 'op_policy_uri' => , // 'op_tos_uri' => , diff --git a/tests/Unit/Controller/DiscoveryControllerTest.php b/tests/Unit/Controller/DiscoveryControllerTest.php index 310a5b1e..110daafd 100644 --- a/tests/Unit/Controller/DiscoveryControllerTest.php +++ b/tests/Unit/Controller/DiscoveryControllerTest.php @@ -191,6 +191,8 @@ public function testDiscoveryResponse() { $this->assertEquals($displayValuesSupported, $result->getData()['display_values_supported']); $this->assertEquals($claimTypesSupported, $result->getData()['claim_types_supported']); $this->assertEquals($claimsSupported, $result->getData()['claims_supported']); + $this->assertFalse($result->getData()['request_parameter_supported']); + $this->assertFalse($result->getData()['request_uri_parameter_supported']); } diff --git a/tests/Unit/Controller/LoginRedirectorControllerTest.php b/tests/Unit/Controller/LoginRedirectorControllerTest.php index 0ceaed9f..2cd790ba 100644 --- a/tests/Unit/Controller/LoginRedirectorControllerTest.php +++ b/tests/Unit/Controller/LoginRedirectorControllerTest.php @@ -256,4 +256,60 @@ function ($arg1, $arg2) { $this->assertEquals('http://oidc.local/login-form', $result->getRedirectURL()); } + public function testAuthorizeRejectsUnsupportedRequestObject() { + $clientId = 'client1'; + $state = 'state-1'; + $redirectUri = 'https://client.example.com/callback'; + + $client = new Client( + 'Test Client', + [$redirectUri], + 'RS256', + 'confidential', + 'code', + 'opaque', + 'openid', + '', + false + ); + $client->id = 1; + $client->setClientIdentifier($clientId); + + $registeredRedirectUri = new RedirectUri(); + $registeredRedirectUri->setClientId(1); + $registeredRedirectUri->setRedirectUri($redirectUri); + + $this->userSession + ->method('isLoggedIn') + ->willReturn(true); + $this->clientMapper + ->method('getByIdentifier') + ->with($clientId) + ->willReturn($client); + $this->redirectUriMapper + ->method('getByClientId') + ->with(1) + ->willReturn([$registeredRedirectUri]); + $this->request + ->method('getParam') + ->willReturnCallback(function ($key) { + return $key === 'request' ? 'eyJhbGciOiJub25lIn0.e30.' : null; + }); + + $result = $this->controller->authorize( + $clientId, + $state, + 'code', + $redirectUri, + 'openid', + 'nonce-1' + ); + + $this->assertEquals(Http::STATUS_SEE_OTHER, $result->getStatus(), 'Status Code does not match!'); + $this->assertEquals( + $redirectUri . '?error=request_not_supported&error_description=Request%20object%20parameter%20is%20not%20supported.&state=state-1', + $result->getRedirectURL() + ); + } + } diff --git a/tests/Unit/Util/DiscoveryGeneratorTest.php b/tests/Unit/Util/DiscoveryGeneratorTest.php index f3374111..3436afc1 100644 --- a/tests/Unit/Util/DiscoveryGeneratorTest.php +++ b/tests/Unit/Util/DiscoveryGeneratorTest.php @@ -208,6 +208,16 @@ public function testGenerateDiscoveryHasPkceSupport() { $this->assertContains('plain', $methods); } + public function testGenerateDiscoveryDoesNotAdvertiseRequestObjectSupport() { + $result = $this->generator->generateDiscovery($this->request); + $data = $result->getData(); + + $this->assertArrayHasKey('request_parameter_supported', $data); + $this->assertFalse($data['request_parameter_supported']); + $this->assertArrayHasKey('request_uri_parameter_supported', $data); + $this->assertFalse($data['request_uri_parameter_supported']); + } + public function testGenerateDiscoveryHasIntrospectionEndpoint() { $result = $this->generator->generateDiscovery($this->request); $data = $result->getData(); From e64b648fd11ad88083495638d38d88d7d3de54e3 Mon Sep 17 00:00:00 2001 From: Thorsten Jagel Date: Wed, 10 Jun 2026 15:50:22 +0200 Subject: [PATCH 19/28] Fix oidcc-unsigned-request-object-supported-correctly-or-rejected-as-unsupported --- .github/conformance/HEADLESS_CHROME_GUIDE.md | 172 --------------- .github/conformance/README.md | 121 ----------- .github/conformance/run-conformance-chrome.sh | 163 --------------- lib/Controller/LoginRedirectorController.php | 197 +++++++++++++----- .../LoginRedirectorControllerTest.php | 61 ++++++ 5 files changed, 202 insertions(+), 512 deletions(-) delete mode 100644 .github/conformance/HEADLESS_CHROME_GUIDE.md delete mode 100644 .github/conformance/README.md delete mode 100644 .github/conformance/run-conformance-chrome.sh diff --git a/.github/conformance/HEADLESS_CHROME_GUIDE.md b/.github/conformance/HEADLESS_CHROME_GUIDE.md deleted file mode 100644 index 99618fcd..00000000 --- a/.github/conformance/HEADLESS_CHROME_GUIDE.md +++ /dev/null @@ -1,172 +0,0 @@ -# Guide: Running OIDC Conformance Tests with Headless Chrome - -## Problem -The default OIDC conformance test runner uses HtmlUnit (a Java-based headless browser) which doesn't support modern ES6 JavaScript syntax (e.g., `class` keyword) used in Nextcloud's core bundles. This causes JavaScript execution errors like: -``` -org.htmlunit.ScriptException: identifier is a reserved word: class -``` - -## Solution: Use an External Selenium + Headless Chrome Worker - -The conformance suite's built-in browser automation is implemented in Java with -HtmlUnit. The workflow does not try to switch that internal implementation. -Instead, `oidc-basic-config.json` omits `browser` commands, which makes the -suite expose front-channel URLs through its runner API. `browser-runner.py` -polls that API and drives the URLs with Selenium/Chromium. - -### Option 1: GitHub Actions Workflow (CI/CD) - -**Changes to `.github/workflows/build-test.yaml`:** - -In the `oidc_conformance` job, modify the "Build and start OpenID conformance suite" step: - -```yaml - - name: Build and start OpenID conformance suite - run: | - sudo sh -c 'echo "127.0.0.1 nginx" >> /etc/hosts' - sudo sh -c 'echo "127.0.0.1 oidcc-provider" >> /etc/hosts' - sudo sh -c 'echo "127.0.0.1 chrome" >> /etc/hosts' - cd conformance-suite - mvn -B -DskipTests package - docker compose \ - -f docker-compose-localtest.yml \ - -f ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/docker-compose.github-actions.yml \ - -f ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/docker-compose.chrome.yml \ - up -d mongodb nginx server chrome - - # Wait for Chrome/Selenium service to be ready - for i in {1..60}; do - if curl --insecure --fail --silent http://chrome:4444/status > /dev/null; then - echo "Chrome/Selenium service is ready" - break - fi - sleep 2 - done - - # Wait for API runner - for i in {1..60}; do - if curl --insecure --fail --silent https://nginx:8443/api/runner/available > /dev/null; then - exit 0 - fi - sleep 5 - done - docker compose -f docker-compose-localtest.yml logs server nginx chrome - exit 1 -``` - -**Run the external browser worker with the test plan:** - -```yaml - - name: Run OIDC basic conformance plan - env: - CONFORMANCE_DEV_MODE: 1 - CONFORMANCE_SERVER: https://nginx:8443/ - CONFORMANCE_SERVER_MTLS: https://nginx:8444/ - SELENIUM_REMOTE_URL: http://chrome:4444/wd/hub - CONFORMANCE_BROWSER_VISIT_TIMEOUT: 90 - run: | - cd conformance-suite - python3 ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/browser-runner.py \ - > ../conformance-browser.log 2>&1 & - browser_runner_pid=$! - trap 'kill "${browser_runner_pid}" 2>/dev/null || true' EXIT - python3 scripts/run-test-plan.py \ - --export-dir ../conformance-results \ - --verbose \ - "oidcc-basic-certification-test-plan[server_metadata=discovery][client_registration=static_client]" \ - ../conformance-config/oidc-basic-config.json -``` - -### Option 2: Local Development - -**Prerequisites:** -```bash -# Install Chrome -brew install --cask google-chrome - -# Install Chromedriver (must match Chrome version) -brew install chromedriver -# or -brew install --cask chromedriver - -# Verify versions match -google-chrome --version -chromedriver --version -``` - -**Start Chromedriver:** -```bash -chromedriver --port=9515 & -echo $! > chromedriver.pid -``` - -**Run tests locally:** -```bash -cd conformance-suite -mvn -B -DskipTests package - -# Start containers (without Chrome, we'll use local Chromedriver) -docker compose \ - -f docker-compose-localtest.yml \ - -f ../nextcloud/apps/oidc/.github/conformance/docker-compose.github-actions.yml \ - up -d mongodb nginx server - -# Wait for services -sleep 30 - -# Run the browser worker with local Chromedriver -python3 ../.github/conformance/browser-runner.py > ../conformance-browser.log 2>&1 & -browser_runner_pid=$! -trap 'kill "${browser_runner_pid}" 2>/dev/null || true' EXIT - -python3 scripts/run-test-plan.py \ - --export-dir ../conformance-results \ - --verbose \ - "oidcc-basic-certification-test-plan[server_metadata=discovery][client_registration=static_client]" \ - ../conformance-config/oidc-basic-config.json -``` - -Set environment variables: -```bash -export CONFORMANCE_DEV_MODE=1 -export CONFORMANCE_SERVER=https://nginx:8443/ -export CONFORMANCE_SERVER_MTLS=https://nginx:8444/ -export SELENIUM_REMOTE_URL=http://localhost:9515 -``` - -### Option 3: Alternative - ES5 Transpilation (Without Browser Engine Change) - -If you want to keep HtmlUnit but fix the ES6 compatibility issue, ensure Nextcloud's JavaScript is built as ES5: - -Check `/nextcloud/webpack.config.js` or build configuration for: -```javascript -target: 'es5' // or targets: ['es2015'] as minimum -``` - -Then rebuild: -```bash -cd nextcloud -npm ci -npm run build // Ensure target is ES5 compatible -``` - -## Validation - -After running with Chrome: -1. Check test logs for WebDriver calls instead of HtmlUnit errors -2. Expected log entries should show Chrome/Chromium processes executing JavaScript -3. No `org.htmlunit.ScriptException` errors -4. JavaScript should execute properly on login pages - -## Files Modified - -- `.github/workflows/build-test.yaml` - Starts the external browser worker with the test plan -- `.github/conformance/browser-runner.py` - Polls conformance front-channel URLs and drives Chromium -- `.github/conformance/oidc-basic-config.json` - Omits HtmlUnit browser commands -- `.github/conformance/docker-compose.chrome.yml` - New Selenium/Chrome service definition - -## References - -- [OpenID Conformance Suite](https://gitlab.com/openid/conformance-suite) -- [Selenium Docker Images](https://github.com/SeleniumHQ/docker-selenium) -- [Chromium WebDriver Protocol](https://chromedriver.chromium.org/) diff --git a/.github/conformance/README.md b/.github/conformance/README.md deleted file mode 100644 index 4505a4d5..00000000 --- a/.github/conformance/README.md +++ /dev/null @@ -1,121 +0,0 @@ -# OIDC Conformance Test Configuration - -This directory contains configuration files and scripts for running OpenID Connect (OIDC) conformance tests against the Nextcloud OIDC Identity Provider app. - -## Files - -- **`oidc-basic-config.json`** - Conformance test plan configuration with test user credentials and client credentials -- **`write-config.py`** - Python script to generate final conformance config from template (replaces environment variables) -- **`docker-compose.github-actions.yml`** - Docker Compose overrides for GitHub Actions CI/CD environment -- **`docker-compose.chrome.yml`** - Docker Compose service definition for Selenium + Headless Chrome (alternative to HtmlUnit) -- **`browser-runner.py`** - External Selenium/Chromium browser worker for conformance front-channel URLs -- **`run-conformance-chrome.sh`** - Local development script to run conformance tests with Headless Chrome -- **`HEADLESS_CHROME_GUIDE.md`** - Detailed guide for setting up and running tests with Chrome instead of HtmlUnit - -## Quick Start - -### Local Development with Headless Chrome - -```bash -# Make script executable -chmod +x run-conformance-chrome.sh - -# Run tests (handles all setup) -./run-conformance-chrome.sh -``` - -The script will: -1. Verify Chrome and Chromedriver are installed -2. Start Chromedriver on port 9515 -3. Start Nextcloud Docker containers (if not running) -4. Run the conformance test suite -5. Cleanup resources - -### GitHub Actions CI/CD - -The workflow defined in `.github/workflows/build-test.yaml` runs conformance tests when manually triggered with `run_conformance=true`: - -```bash -# Via GitHub CLI -gh workflow run build-test.yaml -f run_conformance=true - -# Or manually in GitHub Actions UI: -# Actions > Build app > Run workflow > run_conformance checkbox -``` - -## Browser Engine Selection - -### HtmlUnit (Default - Not Recommended) -- ❌ Does not support modern ES6 JavaScript -- ❌ Causes `org.htmlunit.ScriptException: identifier is a reserved word: class` errors -- ✓ No additional setup needed - -### External Headless Chrome + Selenium (Recommended) -- ✓ Full modern JavaScript support (ES6+) -- ✓ More accurate simulation of real browser behavior -- ✓ Better debugging with VNC access (port 7900) -- ⚠ Requires Chrome/Chromedriver installation - -The conformance suite's built-in browser automation is HtmlUnit-based. To avoid -modifying the suite source, keep the test config free of `browser` commands and -run `browser-runner.py` alongside `scripts/run-test-plan.py`. The runner polls -the suite API for exposed front-channel URLs and drives them through Chromium. - -## Environment Variables - -When running conformance tests, these variables configure the external browser -runner and server URLs: - -- `SELENIUM_REMOTE_URL` - WebDriver endpoint (e.g., `http://chrome:4444/wd/hub`) -- `CONFORMANCE_SERVER` - Nextcloud OIDC provider URL (e.g., `https://nginx:8443/`) -- `CONFORMANCE_SERVER_MTLS` - Mutual TLS endpoint (e.g., `https://nginx:8444/`) -- `CONFORMANCE_DEV_MODE` - Developer mode (`1` = enabled) - -## Troubleshooting - -### Chrome/Chromedriver Version Mismatch -```bash -# Check versions -google-chrome --version -chromedriver --version - -# Install matching versions -brew install --cask google-chrome chromedriver -# or -brew upgrade google-chrome chromedriver -``` - -### Chromedriver Port Already in Use -```bash -# Find and kill process on port 9515 -lsof -i :9515 -kill -9 -``` - -### Docker Compose Networking Issues -Ensure all container names resolve correctly: -```bash -# Add to /etc/hosts (macOS/Linux) -sudo sh -c 'echo "127.0.0.1 nginx" >> /etc/hosts' -sudo sh -c 'echo "127.0.0.1 chrome" >> /etc/hosts' -``` - -### VNC Debugging (to watch Chrome execution) -```bash -# On local machine, access VNC viewer -# Address: localhost:7900 (or docker_host:7900 if remote) -# Password: secret (default) -``` - -## References - -- [OpenID Connect Conformance Suite Documentation](https://gitlab.com/openid/conformance-suite) -- [OIDC Specification](https://openid.net/specs/openid-connect-core-1_0.html) -- [Selenium Docker Images](https://github.com/SeleniumHQ/docker-selenium) -- [Chromium WebDriver](https://chromedriver.chromium.org/) - -## Related Configuration - -- Test plan: `oidcc-basic-certification-test-plan[server_metadata=discovery][client_registration=static_client]` -- Test user credentials: `OIDC_TEST_USER` / `OIDC_TEST_PASSWORD` (set in workflow) -- OIDC clients: Two clients created dynamically in Nextcloud (`OIDC_CLIENT_ID_1/2`, `OIDC_CLIENT_SECRET_1/2`) diff --git a/.github/conformance/run-conformance-chrome.sh b/.github/conformance/run-conformance-chrome.sh deleted file mode 100644 index 06235140..00000000 --- a/.github/conformance/run-conformance-chrome.sh +++ /dev/null @@ -1,163 +0,0 @@ -#!/bin/bash -# Local conformance test runner with Headless Chrome -# Usage: ./run-conformance-chrome.sh - -set -e - -# Colors for output -RED='\033[0;31m' -GREEN='\033[0;32m' -YELLOW='\033[1;33m' -NC='\033[0m' # No Color - -echo -e "${YELLOW}OIDC Conformance Test Runner with Headless Chrome${NC}" -echo "======================================================" - -# Check prerequisites -echo -e "\n${YELLOW}Checking prerequisites...${NC}" - -if ! command -v google-chrome &> /dev/null && ! command -v chromium &> /dev/null; then - echo -e "${RED}✗ Chrome/Chromium not found${NC}" - echo "Install with: brew install --cask google-chrome" - exit 1 -fi -echo -e "${GREEN}✓ Chrome found${NC}" - -if ! command -v chromedriver &> /dev/null; then - echo -e "${RED}✗ Chromedriver not found${NC}" - echo "Install with: brew install chromedriver" - exit 1 -fi - -if ! python3 -c "import httpx, pyparsing, selenium" &> /dev/null; then - echo -e "${RED}Python conformance dependencies not found${NC}" - echo "Install with: python3 -m pip install --user httpx pyparsing selenium" - exit 1 -fi - -# Verify Chrome and Chromedriver versions match (major version) -CHROME_VERSION=$(google-chrome --version | awk '{print $3}' | cut -d. -f1) -DRIVER_VERSION=$(chromedriver --version | awk '{print $2}' | cut -d. -f1) - -echo -e "${GREEN}✓ Chromedriver found (Chrome v${CHROME_VERSION}, Driver v${DRIVER_VERSION})${NC}" - -if [ "$CHROME_VERSION" != "$DRIVER_VERSION" ]; then - echo -e "${YELLOW}⚠ Warning: Chrome major version ($CHROME_VERSION) differs from Chromedriver major version ($DRIVER_VERSION)${NC}" - echo "This may cause compatibility issues. Consider updating one of them." - read -p "Continue anyway? (y/N) " -n 1 -r - echo - if [[ ! $REPLY =~ ^[Yy]$ ]]; then - exit 1 - fi -fi - -# Start chromedriver -echo -e "\n${YELLOW}Starting Chromedriver on port 9515...${NC}" -chromedriver --port=9515 > /tmp/chromedriver.log 2>&1 & -CHROMEDRIVER_PID=$! -echo $CHROMEDRIVER_PID > /tmp/chromedriver.pid -echo -e "${GREEN}✓ Chromedriver started (PID: $CHROMEDRIVER_PID)${NC}" - -# Wait for chromedriver to be ready -echo -e "\n${YELLOW}Waiting for Chromedriver to be ready...${NC}" -for i in {1..30}; do - if curl -s http://localhost:9515/status | grep -q "ready"; then - echo -e "${GREEN}✓ Chromedriver is ready${NC}" - break - fi - if [ $i -eq 30 ]; then - echo -e "${RED}✗ Chromedriver did not become ready in time${NC}" - kill $CHROMEDRIVER_PID - exit 1 - fi - sleep 1 -done - -# Set environment variables -export CONFORMANCE_DEV_MODE=1 -export CONFORMANCE_SERVER=https://nginx:8443/ -export CONFORMANCE_SERVER_MTLS=https://nginx:8444/ -export SELENIUM_REMOTE_URL=http://localhost:9515 -export OIDC_TEST_USER=${OIDC_TEST_USER:-oidc-test-user} -export OIDC_TEST_PASSWORD=${OIDC_TEST_PASSWORD:-oidc-test-password} - -echo -e "\n${YELLOW}Environment variables set:${NC}" -echo " SELENIUM_REMOTE_URL=$SELENIUM_REMOTE_URL" -echo " CONFORMANCE_SERVER=$CONFORMANCE_SERVER" - -# Check if conformance-suite exists -if [ ! -d "./conformance-suite" ]; then - echo -e "\n${YELLOW}Conformance suite not found. Cloning...${NC}" - git clone --depth 1 https://gitlab.com/openid/conformance-suite.git conformance-suite -fi - -# Build conformance suite -if [ ! -f "./conformance-suite/target/fintechlabs-conformance-suite-5.1.0-SNAPSHOT-jar-with-dependencies.jar" ]; then - echo -e "\n${YELLOW}Building conformance suite...${NC}" - cd conformance-suite - mvn -B -DskipTests package -q - cd .. -fi - -# Check if Nextcloud containers are running -echo -e "\n${YELLOW}Checking Nextcloud Docker containers...${NC}" -if ! docker compose -f conformance-suite/docker-compose-localtest.yml ps | grep -q "nginx"; then - echo -e "${YELLOW}Starting Nextcloud containers...${NC}" - cd conformance-suite - docker compose \ - -f docker-compose-localtest.yml \ - -f ../../.github/conformance/docker-compose.github-actions.yml \ - up -d mongodb nginx server - - # Wait for services - echo -e "\n${YELLOW}Waiting for services to be ready...${NC}" - for i in {1..60}; do - if curl --insecure --fail --silent https://nginx:8443/api/runner/available > /dev/null 2>&1; then - echo -e "${GREEN}✓ Services are ready${NC}" - break - fi - if [ $i -eq 60 ]; then - echo -e "${RED}✗ Services did not become ready in time${NC}" - kill $(cat /tmp/chromedriver.pid) - exit 1 - fi - echo -n "." - sleep 2 - done - cd .. -fi - -# Run conformance tests -echo -e "\n${YELLOW}Running conformance tests...${NC}" -echo "==================================================" -cd conformance-suite -python3 ../.github/conformance/browser-runner.py > ../conformance-browser.log 2>&1 & -BROWSER_RUNNER_PID=$! -set +e -python3 scripts/run-test-plan.py \ - --export-dir ../conformance-results \ - --verbose \ - "oidcc-basic-certification-test-plan[server_metadata=discovery][client_registration=static_client]" \ - ../conformance-config/oidc-basic-config.json - -TEST_RESULT=$? -set -e -cd .. - -# Cleanup -echo -e "\n${YELLOW}Cleaning up...${NC}" -if [ -f /tmp/chromedriver.pid ]; then - kill $(cat /tmp/chromedriver.pid) 2>/dev/null || true - rm /tmp/chromedriver.pid -fi -if [ -n "${BROWSER_RUNNER_PID:-}" ]; then - kill "$BROWSER_RUNNER_PID" 2>/dev/null || true -fi - -if [ $TEST_RESULT -eq 0 ]; then - echo -e "\n${GREEN}✓ Conformance tests completed successfully${NC}" - echo "Results available in: conformance-results/" -else - echo -e "\n${RED}✗ Conformance tests failed${NC}" - exit 1 -fi diff --git a/lib/Controller/LoginRedirectorController.php b/lib/Controller/LoginRedirectorController.php index 573ebe51..4d562f40 100644 --- a/lib/Controller/LoginRedirectorController.php +++ b/lib/Controller/LoginRedirectorController.php @@ -222,6 +222,15 @@ public function authorize( $code_challenge_method = null ): Response { + $unsupportedRequestParameterResponse = $this->rejectUnsupportedRequestParameters( + $client_id, + $redirect_uri, + $state + ); + if ($unsupportedRequestParameterResponse !== null) { + return $unsupportedRequestParameterResponse; + } + if (!$this->userSession->isLoggedIn()) { // Not authenticated yet // Store oidc attributes in user session to be available after login @@ -300,26 +309,11 @@ public function authorize( $scope = Application::DEFAULT_SCOPE; } - $this->clientMapper->cleanUp(); - - try { - $client = $this->clientMapper->getByIdentifier($client_id); - } catch (ClientNotFoundException $e) { - $params = [ - 'message' => $this->l->t('Your client is not authorized to connect. Please inform the administrator of your client.'), - ]; - $this->logger->notice('Client ' . $client_id . ' is not authorized to connect.'); - return new TemplateResponse('core', '403', $params, 'error'); - } - - // The client must not be expired - if ($client->isDcr() && $this->time->getTime() > ($client->getIssuedAt() + (int)$this->appConfig->getAppValueString(Application::APP_CONFIG_DEFAULT_CLIENT_EXPIRE_TIME, Application::DEFAULT_CLIENT_EXPIRE_TIME))) { - $this->logger->warning('Client expired. Client id was ' . $client_id . '.'); - $params = [ - 'message' => $this->l->t('Your client is expired. Please inform the administrator of your client.'), - ]; - return new TemplateResponse('core', '400', $params, 'error'); + $clientOrResponse = $this->loadAuthorizationClient($client_id); + if ($clientOrResponse instanceof Response) { + return $clientOrResponse; } + $client = $clientOrResponse; // Set default resource if resource is not set at all if (!isset($resource) || trim($resource)==='') { @@ -353,43 +347,9 @@ public function authorize( $scope = $newScope; $this->logger->debug('[SCOPE DEBUG] Scope after filtering: ' . $scope); - // Check if redirect URI is configured for client - $redirectUris = $this->redirectUriMapper->getByClientId($client->getId()); - $redirectUriFound = false; - foreach ($redirectUris as $redirectUri) { - if ($this->redirectUriService->matchRedirectUri($redirect_uri, $redirectUri->getRedirectUri())) { - $redirectUriFound = true; - break; - } - } - if (!$redirectUriFound) { - $params = [ - 'message' => $this->l->t('The received redirect URI is not accepted to connect. Please inform the administrator of your client.'), - ]; - $this->logger->notice('Redirect URI ' . $redirect_uri . ' is not accepted for client ' . $client_id . '.'); - return new TemplateResponse('core', '403', $params, 'error'); - } - - $requestObject = $this->request->getParam('request'); - if ($this->hasNonEmptyRequestParameter($requestObject)) { - $this->logger->notice('Request object parameter is not supported for client ' . $client_id . '.'); - return $this->createAuthorizationErrorRedirect( - $redirect_uri, - 'request_not_supported', - 'Request object parameter is not supported.', - $state - ); - } - - $requestUri = $this->request->getParam('request_uri'); - if ($this->hasNonEmptyRequestParameter($requestUri)) { - $this->logger->notice('Request URI parameter is not supported for client ' . $client_id . '.'); - return $this->createAuthorizationErrorRedirect( - $redirect_uri, - 'request_uri_not_supported', - 'Request URI parameter is not supported.', - $state - ); + $redirectUriErrorResponse = $this->validateAuthorizationRedirectUri($client, $client_id, $redirect_uri); + if ($redirectUriErrorResponse !== null) { + return $redirectUriErrorResponse; } if (empty($response_type)) { @@ -637,6 +597,131 @@ public function authorize( return new RedirectResponse($url); } + private function rejectUnsupportedRequestParameters( + mixed $clientId, + mixed $redirectUri, + mixed $state + ): ?Response { + $requestObject = $this->request->getParam('request'); + $requestUri = $this->request->getParam('request_uri'); + + if ( + !$this->hasNonEmptyRequestParameter($requestObject) + && !$this->hasNonEmptyRequestParameter($requestUri) + ) { + return null; + } + + if (!$this->hasNonEmptyRequestParameter($redirectUri)) { + $this->logger->notice('Unsupported request parameter received without a redirect URI.'); + return new TemplateResponse('core', '400', [ + 'message' => $this->l->t('Authorization request is missing a redirect URI.'), + ], 'error'); + } + + $clientOrResponse = $this->loadAuthorizationClient($clientId); + if ($clientOrResponse instanceof Response) { + return $clientOrResponse; + } + + $redirectUriErrorResponse = $this->validateAuthorizationRedirectUri( + $clientOrResponse, + $clientId, + $redirectUri + ); + if ($redirectUriErrorResponse !== null) { + return $redirectUriErrorResponse; + } + + if ($this->hasNonEmptyRequestParameter($requestObject)) { + $this->logger->notice('Request object parameter is not supported for client ' . $clientId . '.'); + return $this->createAuthorizationErrorRedirect( + (string)$redirectUri, + 'request_not_supported', + 'Request object parameter is not supported.', + $state + ); + } + + $this->logger->notice('Request URI parameter is not supported for client ' . $clientId . '.'); + return $this->createAuthorizationErrorRedirect( + (string)$redirectUri, + 'request_uri_not_supported', + 'Request URI parameter is not supported.', + $state + ); + } + + private function loadAuthorizationClient(mixed $clientId): Client|Response + { + if (!is_string($clientId) || trim($clientId) === '') { + $params = [ + 'message' => $this->l->t('Your client is not authorized to connect. Please inform the administrator of your client.'), + ]; + $this->logger->notice('Client ' . var_export($clientId, true) . ' is not authorized to connect.'); + return new TemplateResponse('core', '403', $params, 'error'); + } + + $this->clientMapper->cleanUp(); + + try { + $client = $this->clientMapper->getByIdentifier($clientId); + } catch (ClientNotFoundException $e) { + $params = [ + 'message' => $this->l->t('Your client is not authorized to connect. Please inform the administrator of your client.'), + ]; + $this->logger->notice('Client ' . $clientId . ' is not authorized to connect.'); + return new TemplateResponse('core', '403', $params, 'error'); + } + + if ($client === null) { + $params = [ + 'message' => $this->l->t('Your client is not authorized to connect. Please inform the administrator of your client.'), + ]; + $this->logger->notice('Client ' . $clientId . ' is not authorized to connect.'); + return new TemplateResponse('core', '403', $params, 'error'); + } + + // The client must not be expired + if ($client->isDcr() && $this->time->getTime() > ($client->getIssuedAt() + (int)$this->appConfig->getAppValueString(Application::APP_CONFIG_DEFAULT_CLIENT_EXPIRE_TIME, Application::DEFAULT_CLIENT_EXPIRE_TIME))) { + $this->logger->warning('Client expired. Client id was ' . $clientId . '.'); + $params = [ + 'message' => $this->l->t('Your client is expired. Please inform the administrator of your client.'), + ]; + return new TemplateResponse('core', '400', $params, 'error'); + } + + return $client; + } + + private function validateAuthorizationRedirectUri( + Client $client, + mixed $clientId, + mixed $redirectUri + ): ?TemplateResponse { + if (!is_string($redirectUri) || trim($redirectUri) === '') { + $params = [ + 'message' => $this->l->t('The received redirect URI is not accepted to connect. Please inform the administrator of your client.'), + ]; + $this->logger->notice('Redirect URI ' . var_export($redirectUri, true) . ' is not accepted for client ' . $clientId . '.'); + return new TemplateResponse('core', '403', $params, 'error'); + } + + // Check if redirect URI is configured for client + $redirectUris = $this->redirectUriMapper->getByClientId($client->getId()); + foreach ($redirectUris as $registeredRedirectUri) { + if ($this->redirectUriService->matchRedirectUri($redirectUri, $registeredRedirectUri->getRedirectUri())) { + return null; + } + } + + $params = [ + 'message' => $this->l->t('The received redirect URI is not accepted to connect. Please inform the administrator of your client.'), + ]; + $this->logger->notice('Redirect URI ' . $redirectUri . ' is not accepted for client ' . $clientId . '.'); + return new TemplateResponse('core', '403', $params, 'error'); + } + private function hasNonEmptyRequestParameter(mixed $value): bool { if (is_string($value)) { diff --git a/tests/Unit/Controller/LoginRedirectorControllerTest.php b/tests/Unit/Controller/LoginRedirectorControllerTest.php index 2cd790ba..9deb621d 100644 --- a/tests/Unit/Controller/LoginRedirectorControllerTest.php +++ b/tests/Unit/Controller/LoginRedirectorControllerTest.php @@ -312,4 +312,65 @@ public function testAuthorizeRejectsUnsupportedRequestObject() { ); } + public function testAuthorizeRejectsUnsupportedRequestObjectBeforeLogin() { + $clientId = 'client1'; + $redirectUri = 'https://client.example.com/callback'; + + $client = new Client( + 'Test Client', + [$redirectUri], + 'RS256', + 'confidential', + 'code', + 'opaque', + 'openid', + '', + false + ); + $client->id = 1; + $client->setClientIdentifier($clientId); + + $registeredRedirectUri = new RedirectUri(); + $registeredRedirectUri->setClientId(1); + $registeredRedirectUri->setRedirectUri($redirectUri); + + $this->userSession + ->expects($this->never()) + ->method('isLoggedIn'); + $this->session + ->expects($this->never()) + ->method('set'); + $this->urlGenerator + ->expects($this->never()) + ->method('linkToRoute'); + $this->clientMapper + ->method('getByIdentifier') + ->with($clientId) + ->willReturn($client); + $this->redirectUriMapper + ->method('getByClientId') + ->with(1) + ->willReturn([$registeredRedirectUri]); + $this->request + ->method('getParam') + ->willReturnCallback(function ($key) { + return $key === 'request' ? 'eyJhbGciOiJub25lIn0.e30.' : null; + }); + + $result = $this->controller->authorize( + $clientId, + null, + 'code', + $redirectUri, + 'openid', + null + ); + + $this->assertEquals(Http::STATUS_SEE_OTHER, $result->getStatus(), 'Status Code does not match!'); + $this->assertEquals( + $redirectUri . '?error=request_not_supported&error_description=Request%20object%20parameter%20is%20not%20supported.', + $result->getRedirectURL() + ); + } + } From 084831703cc35519b70c858cd714d9e4285d90b1 Mon Sep 17 00:00:00 2001 From: Thorsten Jagel Date: Thu, 11 Jun 2026 08:51:22 +0200 Subject: [PATCH 20/28] Fix for oidcc-prompt-none-not-logged-in --- lib/Controller/ConsentController.php | 2 + lib/Controller/LoginRedirectorController.php | 47 +++++++++++++- lib/Controller/PageController.php | 5 +- src/Redirect.vue | 1 + templates/main.php | 1 + .../LoginRedirectorControllerTest.php | 64 +++++++++++++++++++ 6 files changed, 116 insertions(+), 4 deletions(-) diff --git a/lib/Controller/ConsentController.php b/lib/Controller/ConsentController.php index 67c2c98d..0d197b04 100644 --- a/lib/Controller/ConsentController.php +++ b/lib/Controller/ConsentController.php @@ -216,6 +216,7 @@ public function grant(): RedirectResponse { 'resource' => $this->session->get('oidc_resource'), 'code_challenge' => $this->session->get('oidc_code_challenge'), 'code_challenge_method' => $this->session->get('oidc_code_challenge_method'), + 'prompt' => $this->session->get('oidc_prompt'), ])); // IMPORTANT: Close the session to commit changes before redirecting @@ -434,6 +435,7 @@ public function deny(): RedirectResponse { $this->session->remove('oidc_resource'); $this->session->remove('oidc_code_challenge'); $this->session->remove('oidc_code_challenge_method'); + $this->session->remove('oidc_prompt'); $this->session->remove('oidc_requested_scopes'); // Return error to client diff --git a/lib/Controller/LoginRedirectorController.php b/lib/Controller/LoginRedirectorController.php index 4d562f40..a8b74465 100644 --- a/lib/Controller/LoginRedirectorController.php +++ b/lib/Controller/LoginRedirectorController.php @@ -159,6 +159,7 @@ public function __construct( * @param string $resource * @param string $code_challenge * @param string $code_challenge_method + * @param string $prompt * @return Response */ #[BruteForceProtection(action: 'oidc_login')] @@ -174,7 +175,8 @@ public function authorizePost( $nonce, $resource = null, $code_challenge = null, - $code_challenge_method = null + $code_challenge_method = null, + $prompt = null ): Response { return $this->authorize( @@ -186,7 +188,8 @@ public function authorizePost( $nonce, $resource, $code_challenge, - $code_challenge_method); + $code_challenge_method, + $prompt); } /** @@ -204,6 +207,7 @@ public function authorizePost( * @param string $resource * @param string $code_challenge * @param string $code_challenge_method + * @param string $prompt * @return Response */ #[BruteForceProtection(action: 'oidc_login')] @@ -219,9 +223,12 @@ public function authorize( $nonce, $resource = null, $code_challenge = null, - $code_challenge_method = null + $code_challenge_method = null, + $prompt = null ): Response { + $prompt = $prompt ?? $this->request->getParam('prompt'); + $unsupportedRequestParameterResponse = $this->rejectUnsupportedRequestParameters( $client_id, $redirect_uri, @@ -232,6 +239,26 @@ public function authorize( } if (!$this->userSession->isLoggedIn()) { + if ($this->promptContains($prompt, 'none')) { + $clientOrResponse = $this->loadAuthorizationClient($client_id); + if ($clientOrResponse instanceof Response) { + return $clientOrResponse; + } + + $redirectUriErrorResponse = $this->validateAuthorizationRedirectUri($clientOrResponse, $client_id, $redirect_uri); + if ($redirectUriErrorResponse !== null) { + return $redirectUriErrorResponse; + } + + $this->logger->debug('prompt=none requested without authenticated user for client ' . $client_id . '. Returning login_required.'); + return $this->createAuthorizationErrorRedirect( + (string)$redirect_uri, + 'login_required', + 'User is not logged in.', + $state + ); + } + // Not authenticated yet // Store oidc attributes in user session to be available after login $this->session->set('oidc_client_id', $client_id); @@ -243,6 +270,7 @@ public function authorize( $this->session->set('oidc_resource', $resource); $this->session->set('oidc_code_challenge', $code_challenge); $this->session->set('oidc_code_challenge_method', $code_challenge_method); + $this->session->set('oidc_prompt', $prompt); $afterLoginRedirectUrl = $this->urlGenerator->linkToRoute('oidc.Page.index', array_filter([ 'client_id' => $client_id, @@ -254,6 +282,7 @@ public function authorize( 'resource' => $resource, 'code_challenge' => $code_challenge, 'code_challenge_method' => $code_challenge_method, + 'prompt' => $prompt, ])); $loginUrl = $this->urlGenerator->linkToRoute( @@ -289,6 +318,7 @@ public function authorize( $resource = $this->session->get('oidc_resource'); $code_challenge = $this->session->get('oidc_code_challenge'); $code_challenge_method = $this->session->get('oidc_code_challenge_method'); + $prompt = $this->session->get('oidc_prompt'); } // Guard: if critical OAuth params are still missing after session fallback, @@ -486,6 +516,7 @@ public function authorize( $this->session->set('oidc_resource', $resource); $this->session->set('oidc_code_challenge', $code_challenge); $this->session->set('oidc_code_challenge_method', $code_challenge_method); + $this->session->set('oidc_prompt', $prompt); $this->session->close(); // Close session to prevent session locking issues during redirect @@ -731,6 +762,16 @@ private function hasNonEmptyRequestParameter(mixed $value): bool return !empty($value); } + private function promptContains(mixed $prompt, string $expectedPrompt): bool + { + if (!is_string($prompt) || trim($prompt) === '') { + return false; + } + + $promptEntries = array_filter(array_map('trim', explode(' ', strtolower($prompt)))); + return in_array(strtolower($expectedPrompt), $promptEntries, true); + } + private function createAuthorizationErrorRedirect( string $redirectUri, string $error, diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index 7e5349d5..6840cd31 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -73,6 +73,7 @@ public function index() $resource = $this->request->getParam('resource'); $code_challenge = $this->request->getParam('code_challenge'); $code_challenge_method = $this->request->getParam('code_challenge_method'); + $prompt = $this->request->getParam('prompt'); } else { $client_id = $this->session->get('oidc_client_id'); $state = $this->session->get('oidc_state'); @@ -83,6 +84,7 @@ public function index() $resource = $this->session->get('oidc_resource'); $code_challenge = $this->session->get('oidc_code_challenge'); $code_challenge_method = $this->session->get('oidc_code_challenge_method'); + $prompt = $this->session->get('oidc_prompt'); } $parameters = [ @@ -94,7 +96,8 @@ public function index() 'code_challenge' => $code_challenge, 'code_challenge_method' => $code_challenge_method, 'scope' => $scope, - 'resource' => $resource + 'resource' => $resource, + 'prompt' => $prompt ]; $response = new TemplateResponse('oidc', 'main', $parameters); diff --git a/src/Redirect.vue b/src/Redirect.vue index 846d815c..6e69d57d 100644 --- a/src/Redirect.vue +++ b/src/Redirect.vue @@ -25,6 +25,7 @@ export default { const keys = [ 'client_id', 'state', 'response_type', 'redirect_uri', 'scope', 'nonce', 'resource', 'code_challenge', 'code_challenge_method', + 'prompt', ] keys.forEach(key => { const value = el?.getAttribute('data-' + key.replace(/_/g, '-')) diff --git a/templates/main.php b/templates/main.php index f5e5f763..f8da7e16 100644 --- a/templates/main.php +++ b/templates/main.php @@ -13,6 +13,7 @@ data-="" diff --git a/tests/Unit/Controller/LoginRedirectorControllerTest.php b/tests/Unit/Controller/LoginRedirectorControllerTest.php index 9deb621d..305a3b75 100644 --- a/tests/Unit/Controller/LoginRedirectorControllerTest.php +++ b/tests/Unit/Controller/LoginRedirectorControllerTest.php @@ -256,6 +256,70 @@ function ($arg1, $arg2) { $this->assertEquals('http://oidc.local/login-form', $result->getRedirectURL()); } + public function testAuthorizePromptNoneNotLoggedInReturnsLoginRequired() { + $clientId = 'client1'; + $state = 'state-1'; + $redirectUri = 'https://client.example.com/callback'; + + $client = new Client( + 'Test Client', + [$redirectUri], + 'RS256', + 'confidential', + 'code', + 'opaque', + 'openid', + '', + false + ); + $client->id = 1; + $client->setClientIdentifier($clientId); + + $registeredRedirectUri = new RedirectUri(); + $registeredRedirectUri->setClientId(1); + $registeredRedirectUri->setRedirectUri($redirectUri); + + $this->userSession + ->method('isLoggedIn') + ->willReturn(false); + $this->session + ->expects($this->never()) + ->method('set'); + $this->urlGenerator + ->expects($this->never()) + ->method('linkToRoute'); + $this->clientMapper + ->method('getByIdentifier') + ->with($clientId) + ->willReturn($client); + $this->redirectUriMapper + ->method('getByClientId') + ->with(1) + ->willReturn([$registeredRedirectUri]); + $this->accessTokenMapper + ->expects($this->never()) + ->method('insert'); + + $result = $this->controller->authorize( + $clientId, + $state, + 'code', + $redirectUri, + 'openid', + 'nonce-1', + null, + null, + null, + 'none' + ); + + $this->assertEquals(Http::STATUS_SEE_OTHER, $result->getStatus(), 'Status Code does not match!'); + $this->assertEquals( + $redirectUri . '?error=login_required&error_description=User%20is%20not%20logged%20in.&state=state-1', + $result->getRedirectURL() + ); + } + public function testAuthorizeRejectsUnsupportedRequestObject() { $clientId = 'client1'; $state = 'state-1'; From 04939959f1c48c4ef8b17b1514caf768035e15af Mon Sep 17 00:00:00 2001 From: Thorsten Jagel Date: Thu, 11 Jun 2026 09:20:26 +0200 Subject: [PATCH 21/28] Fix conformance test oidcc-max-age-10000 --- lib/Controller/ConsentController.php | 2 + lib/Controller/LoginRedirectorController.php | 76 +++++++++- lib/Controller/PageController.php | 5 +- src/Redirect.vue | 2 +- templates/main.php | 2 +- .../LoginRedirectorControllerTest.php | 131 ++++++++++++++++++ 6 files changed, 211 insertions(+), 7 deletions(-) diff --git a/lib/Controller/ConsentController.php b/lib/Controller/ConsentController.php index 0d197b04..534e9ebb 100644 --- a/lib/Controller/ConsentController.php +++ b/lib/Controller/ConsentController.php @@ -217,6 +217,7 @@ public function grant(): RedirectResponse { 'code_challenge' => $this->session->get('oidc_code_challenge'), 'code_challenge_method' => $this->session->get('oidc_code_challenge_method'), 'prompt' => $this->session->get('oidc_prompt'), + 'max_age' => $this->session->get('oidc_max_age'), ])); // IMPORTANT: Close the session to commit changes before redirecting @@ -436,6 +437,7 @@ public function deny(): RedirectResponse { $this->session->remove('oidc_code_challenge'); $this->session->remove('oidc_code_challenge_method'); $this->session->remove('oidc_prompt'); + $this->session->remove('oidc_max_age'); $this->session->remove('oidc_requested_scopes'); // Return error to client diff --git a/lib/Controller/LoginRedirectorController.php b/lib/Controller/LoginRedirectorController.php index a8b74465..feb87c58 100644 --- a/lib/Controller/LoginRedirectorController.php +++ b/lib/Controller/LoginRedirectorController.php @@ -160,6 +160,7 @@ public function __construct( * @param string $code_challenge * @param string $code_challenge_method * @param string $prompt + * @param string $max_age * @return Response */ #[BruteForceProtection(action: 'oidc_login')] @@ -176,7 +177,8 @@ public function authorizePost( $resource = null, $code_challenge = null, $code_challenge_method = null, - $prompt = null + $prompt = null, + $max_age = null ): Response { return $this->authorize( @@ -189,7 +191,8 @@ public function authorizePost( $resource, $code_challenge, $code_challenge_method, - $prompt); + $prompt, + $max_age); } /** @@ -208,6 +211,7 @@ public function authorizePost( * @param string $code_challenge * @param string $code_challenge_method * @param string $prompt + * @param string $max_age * @return Response */ #[BruteForceProtection(action: 'oidc_login')] @@ -224,10 +228,12 @@ public function authorize( $resource = null, $code_challenge = null, $code_challenge_method = null, - $prompt = null + $prompt = null, + $max_age = null ): Response { $prompt = $prompt ?? $this->request->getParam('prompt'); + $max_age = $max_age ?? $this->request->getParam('max_age'); $unsupportedRequestParameterResponse = $this->rejectUnsupportedRequestParameters( $client_id, @@ -271,6 +277,8 @@ public function authorize( $this->session->set('oidc_code_challenge', $code_challenge); $this->session->set('oidc_code_challenge_method', $code_challenge_method); $this->session->set('oidc_prompt', $prompt); + $this->session->set('oidc_max_age', $max_age); + $this->session->set('oidc_login_pending', true); $afterLoginRedirectUrl = $this->urlGenerator->linkToRoute('oidc.Page.index', array_filter([ 'client_id' => $client_id, @@ -283,6 +291,7 @@ public function authorize( 'code_challenge' => $code_challenge, 'code_challenge_method' => $code_challenge_method, 'prompt' => $prompt, + 'max_age' => $max_age, ])); $loginUrl = $this->urlGenerator->linkToRoute( @@ -319,8 +328,11 @@ public function authorize( $code_challenge = $this->session->get('oidc_code_challenge'); $code_challenge_method = $this->session->get('oidc_code_challenge_method'); $prompt = $this->session->get('oidc_prompt'); + $max_age = $this->session->get('oidc_max_age'); } + $authTime = $this->getOidcAuthenticationTime(); + // Guard: if critical OAuth params are still missing after session fallback, // return a meaningful error instead of letting downstream code crash with a 500 // (e.g. matchRedirectUri(null, ...) or trim(null) in PHP 8.4). @@ -382,6 +394,16 @@ public function authorize( return $redirectUriErrorResponse; } + if ($this->promptContains($prompt, 'none') && $this->maxAgeExceeded($max_age, $authTime)) { + $this->logger->debug('prompt=none requested but max_age is exceeded for client ' . $client_id . '. Returning login_required.'); + return $this->createAuthorizationErrorRedirect( + (string)$redirect_uri, + 'login_required', + 'User authentication is too old.', + $state + ); + } + if (empty($response_type)) { $this->logger->notice('Missing response_type in request for client ' . $client_id . '.'); $separator = str_contains($redirect_uri, '?') ? '&' : '?'; @@ -517,6 +539,7 @@ public function authorize( $this->session->set('oidc_code_challenge', $code_challenge); $this->session->set('oidc_code_challenge_method', $code_challenge_method); $this->session->set('oidc_prompt', $prompt); + $this->session->set('oidc_max_age', $max_age); $this->session->close(); // Close session to prevent session locking issues during redirect @@ -569,7 +592,7 @@ public function authorize( } else { $accessToken->setResource(substr($resource, 0, 2000)); } - $accessToken->setCreated($this->time->getTime()); + $accessToken->setCreated($authTime); $accessToken->setRefreshed($this->time->getTime()); if (empty($nonce) || !isset($nonce)) { $nonce = ''; @@ -772,6 +795,51 @@ private function promptContains(mixed $prompt, string $expectedPrompt): bool return in_array(strtolower($expectedPrompt), $promptEntries, true); } + private function getOidcAuthenticationTime(): int + { + $storedAuthTime = $this->session->get('oidc_auth_time'); + $loginPending = $this->session->get('oidc_login_pending'); + + if ($loginPending || !$this->isPositiveIntegerLike($storedAuthTime)) { + $storedAuthTime = $this->time->getTime(); + $this->session->set('oidc_auth_time', $storedAuthTime); + } + + if ($loginPending) { + $this->session->remove('oidc_login_pending'); + } + + return (int)$storedAuthTime; + } + + private function maxAgeExceeded(mixed $maxAge, int $authTime): bool + { + if (!$this->isNonNegativeIntegerLike($maxAge)) { + return false; + } + + return $this->time->getTime() > $authTime + (int)$maxAge; + } + + private function isPositiveIntegerLike(mixed $value): bool + { + return $this->isNonNegativeIntegerLike($value) && (int)$value > 0; + } + + private function isNonNegativeIntegerLike(mixed $value): bool + { + if (is_int($value)) { + return $value >= 0; + } + + if (!is_string($value)) { + return false; + } + + $value = trim($value); + return $value !== '' && ctype_digit($value); + } + private function createAuthorizationErrorRedirect( string $redirectUri, string $error, diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index 6840cd31..43fa0a2a 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -74,6 +74,7 @@ public function index() $code_challenge = $this->request->getParam('code_challenge'); $code_challenge_method = $this->request->getParam('code_challenge_method'); $prompt = $this->request->getParam('prompt'); + $max_age = $this->request->getParam('max_age'); } else { $client_id = $this->session->get('oidc_client_id'); $state = $this->session->get('oidc_state'); @@ -85,6 +86,7 @@ public function index() $code_challenge = $this->session->get('oidc_code_challenge'); $code_challenge_method = $this->session->get('oidc_code_challenge_method'); $prompt = $this->session->get('oidc_prompt'); + $max_age = $this->session->get('oidc_max_age'); } $parameters = [ @@ -97,7 +99,8 @@ public function index() 'code_challenge_method' => $code_challenge_method, 'scope' => $scope, 'resource' => $resource, - 'prompt' => $prompt + 'prompt' => $prompt, + 'max_age' => $max_age ]; $response = new TemplateResponse('oidc', 'main', $parameters); diff --git a/src/Redirect.vue b/src/Redirect.vue index 6e69d57d..b1074632 100644 --- a/src/Redirect.vue +++ b/src/Redirect.vue @@ -25,7 +25,7 @@ export default { const keys = [ 'client_id', 'state', 'response_type', 'redirect_uri', 'scope', 'nonce', 'resource', 'code_challenge', 'code_challenge_method', - 'prompt', + 'prompt', 'max_age', ] keys.forEach(key => { const value = el?.getAttribute('data-' + key.replace(/_/g, '-')) diff --git a/templates/main.php b/templates/main.php index f8da7e16..8967b5df 100644 --- a/templates/main.php +++ b/templates/main.php @@ -13,7 +13,7 @@ data-="" diff --git a/tests/Unit/Controller/LoginRedirectorControllerTest.php b/tests/Unit/Controller/LoginRedirectorControllerTest.php index 305a3b75..6386b3c3 100644 --- a/tests/Unit/Controller/LoginRedirectorControllerTest.php +++ b/tests/Unit/Controller/LoginRedirectorControllerTest.php @@ -28,9 +28,11 @@ use OC\Authentication\Token\IProvider; use OC\Security\SecureRandom; +use OCA\OIDCIdentityProvider\AppInfo\Application; use OCA\OIDCIdentityProvider\Db\ClientMapper; use OCA\OIDCIdentityProvider\Db\AccessTokenMapper; use OCA\OIDCIdentityProvider\Controller\LoginRedirectorController; +use OCA\OIDCIdentityProvider\Db\AccessToken; use OCA\OIDCIdentityProvider\Db\Client; use OCA\OIDCIdentityProvider\Db\GroupMapper; use OCA\OIDCIdentityProvider\Db\RedirectUri; @@ -320,6 +322,135 @@ public function testAuthorizePromptNoneNotLoggedInReturnsLoginRequired() { ); } + public function testAuthorizeUsesStoredOidcAuthenticationTimeForAccessToken() { + $clientId = 'client1'; + $state = 'state-1'; + $redirectUri = 'https://client.example.com/callback'; + $authTime = 1234567890; + + $client = new Client( + 'Test Client', + [$redirectUri], + 'RS256', + 'confidential', + 'code', + 'opaque', + 'openid', + '', + false + ); + $client->id = 1; + $client->setClientIdentifier($clientId); + + $registeredRedirectUri = new RedirectUri(); + $registeredRedirectUri->setClientId(1); + $registeredRedirectUri->setRedirectUri($redirectUri); + + $user = $this->createMock(\OCP\IUser::class); + $user + ->method('getUID') + ->willReturn('testuser'); + + $jwtGenerator = $this->createMock(JwtGenerator::class); + $jwtGenerator + ->method('generateAccessToken') + ->willReturn('access-token'); + + $controller = new LoginRedirectorController( + 'oidc', + $this->request, + $this->urlGenerator, + $this->clientMapper, + $this->groupMapper, + $this->secureRandom, + $this->session, + $this->l, + $this->time, + $this->userSession, + $this->groupManager, + $this->accessTokenMapper, + $this->redirectUriMapper, + $this->userConsentMapper, + $this->appConfig, + $jwtGenerator, + $this->redirectUriService, + $this->logger + ); + + $this->userSession + ->method('isLoggedIn') + ->willReturn(true); + $this->userSession + ->method('getUser') + ->willReturn($user); + $this->session + ->method('get') + ->willReturnCallback(function ($key) use ($authTime) { + $values = [ + 'oidc_auth_time' => $authTime, + 'oidc_login_pending' => false, + ]; + return $values[$key] ?? null; + }); + $this->request + ->method('getServerProtocol') + ->willReturn('https'); + $this->request + ->method('getServerHost') + ->willReturn('server.example.com'); + $this->clientMapper + ->method('getByIdentifier') + ->with($clientId) + ->willReturn($client); + $this->redirectUriMapper + ->method('getByClientId') + ->with(1) + ->willReturn([$registeredRedirectUri]); + $this->groupMapper + ->method('getGroupsByClientId') + ->with(1) + ->willReturn([]); + $this->groupManager + ->method('getUserGroups') + ->with($user) + ->willReturn([]); + $this->userConsentMapper + ->method('findByUserAndClient') + ->with('testuser', 1) + ->willReturn(null); + $this->appConfig + ->method('getAppValueString') + ->willReturnCallback(function ($key, $default = '') { + if ($key === Application::APP_CONFIG_ALLOW_USER_SETTINGS) { + return 'no'; + } + return $default; + }); + $this->accessTokenMapper + ->expects($this->once()) + ->method('insert') + ->with($this->callback(function (AccessToken $accessToken) use ($authTime) { + return $accessToken->getCreated() === $authTime; + })); + + $result = $controller->authorize( + $clientId, + $state, + 'code', + $redirectUri, + 'openid', + 'nonce-1', + null, + null, + null, + null, + '10000' + ); + + $this->assertEquals(Http::STATUS_SEE_OTHER, $result->getStatus(), 'Status Code does not match!'); + $this->assertStringStartsWith($redirectUri . '?state=state-1&code=', $result->getRedirectURL()); + } + public function testAuthorizeRejectsUnsupportedRequestObject() { $clientId = 'client1'; $state = 'state-1'; From a332c7800e695a7191d982174e7d102c24a93444 Mon Sep 17 00:00:00 2001 From: Thorsten Jagel Date: Thu, 11 Jun 2026 10:27:09 +0200 Subject: [PATCH 22/28] Implemented oidcc-max-age-1 fix --- lib/Controller/ConsentController.php | 4 +- lib/Controller/LoginRedirectorController.php | 150 ++++++++++++------ .../LoginRedirectorControllerTest.php | 113 ++++++++++++- 3 files changed, 215 insertions(+), 52 deletions(-) diff --git a/lib/Controller/ConsentController.php b/lib/Controller/ConsentController.php index 534e9ebb..808079fa 100644 --- a/lib/Controller/ConsentController.php +++ b/lib/Controller/ConsentController.php @@ -218,7 +218,9 @@ public function grant(): RedirectResponse { 'code_challenge_method' => $this->session->get('oidc_code_challenge_method'), 'prompt' => $this->session->get('oidc_prompt'), 'max_age' => $this->session->get('oidc_max_age'), - ])); + ], static function ($value): bool { + return $value !== null && $value !== ''; + })); // IMPORTANT: Close the session to commit changes before redirecting // Without this, the authorize endpoint won't see the updated session values diff --git a/lib/Controller/LoginRedirectorController.php b/lib/Controller/LoginRedirectorController.php index feb87c58..922a2b9e 100644 --- a/lib/Controller/LoginRedirectorController.php +++ b/lib/Controller/LoginRedirectorController.php @@ -265,47 +265,20 @@ public function authorize( ); } - // Not authenticated yet - // Store oidc attributes in user session to be available after login - $this->session->set('oidc_client_id', $client_id); - $this->session->set('oidc_state', $state); - $this->session->set('oidc_response_type', $response_type); - $this->session->set('oidc_redirect_uri', $redirect_uri); - $this->session->set('oidc_scope', $scope); - $this->session->set('oidc_nonce', $nonce); - $this->session->set('oidc_resource', $resource); - $this->session->set('oidc_code_challenge', $code_challenge); - $this->session->set('oidc_code_challenge_method', $code_challenge_method); - $this->session->set('oidc_prompt', $prompt); - $this->session->set('oidc_max_age', $max_age); - $this->session->set('oidc_login_pending', true); - - $afterLoginRedirectUrl = $this->urlGenerator->linkToRoute('oidc.Page.index', array_filter([ - 'client_id' => $client_id, - 'state' => $state, - 'response_type' => $response_type, - 'redirect_uri' => $redirect_uri, - 'scope' => $scope, - 'nonce' => $nonce, - 'resource' => $resource, - 'code_challenge' => $code_challenge, - 'code_challenge_method' => $code_challenge_method, - 'prompt' => $prompt, - 'max_age' => $max_age, - ])); - - $loginUrl = $this->urlGenerator->linkToRoute( - 'core.login.showLoginForm', - [ - 'redirect_url' => $afterLoginRedirectUrl - ] + return $this->redirectToLoginAfterOidcAuthentication( + $client_id, + $state, + $response_type, + $redirect_uri, + $scope, + $nonce, + $resource, + $code_challenge, + $code_challenge_method, + $prompt, + $max_age, + 'Not authenticated yet for client ' . $client_id . '. Redirect to login.' ); - - $this->session->close(); // Close session to prevent session locking issues during redirect - - $this->logger->debug('Not authenticated yet for client ' . $client_id . '. Redirect to login.'); - - return new RedirectResponse($loginUrl); } // Debug: Log client id before and after fallback @@ -394,16 +367,6 @@ public function authorize( return $redirectUriErrorResponse; } - if ($this->promptContains($prompt, 'none') && $this->maxAgeExceeded($max_age, $authTime)) { - $this->logger->debug('prompt=none requested but max_age is exceeded for client ' . $client_id . '. Returning login_required.'); - return $this->createAuthorizationErrorRedirect( - (string)$redirect_uri, - 'login_required', - 'User authentication is too old.', - $state - ); - } - if (empty($response_type)) { $this->logger->notice('Missing response_type in request for client ' . $client_id . '.'); $separator = str_contains($redirect_uri, '?') ? '&' : '?'; @@ -446,6 +409,35 @@ public function authorize( return new RedirectResponse($url); } + if ($this->maxAgeExceeded($max_age, $authTime)) { + if ($this->promptContains($prompt, 'none')) { + $this->logger->debug('prompt=none requested but max_age is exceeded for client ' . $client_id . '. Returning login_required.'); + return $this->createAuthorizationErrorRedirect( + (string)$redirect_uri, + 'login_required', + 'User authentication is too old.', + $state + ); + } + + $this->logger->debug('max_age is exceeded for client ' . $client_id . '. Forcing reauthentication.'); + $this->userSession->logout(); + return $this->redirectToLoginAfterOidcAuthentication( + $client_id, + $state, + $response_type, + $redirect_uri, + $scope, + $nonce, + $resource, + $code_challenge, + $code_challenge_method, + $prompt, + $max_age, + 'Redirect to login for max_age reauthentication for client ' . $client_id . '.' + ); + } + // Check if user is in allowed groups for client $clientGroups = $this->groupMapper->getGroupsByClientId($client->getId()); $userGroups = $this->groupManager->getUserGroups($this->userSession->getUser()); @@ -785,6 +777,64 @@ private function hasNonEmptyRequestParameter(mixed $value): bool return !empty($value); } + private function redirectToLoginAfterOidcAuthentication( + mixed $clientId, + mixed $state, + mixed $responseType, + mixed $redirectUri, + mixed $scope, + mixed $nonce, + mixed $resource, + mixed $codeChallenge, + mixed $codeChallengeMethod, + mixed $prompt, + mixed $maxAge, + string $logMessage + ): RedirectResponse { + // Store OIDC attributes in the user session to be available after login. + $this->session->set('oidc_client_id', $clientId); + $this->session->set('oidc_state', $state); + $this->session->set('oidc_response_type', $responseType); + $this->session->set('oidc_redirect_uri', $redirectUri); + $this->session->set('oidc_scope', $scope); + $this->session->set('oidc_nonce', $nonce); + $this->session->set('oidc_resource', $resource); + $this->session->set('oidc_code_challenge', $codeChallenge); + $this->session->set('oidc_code_challenge_method', $codeChallengeMethod); + $this->session->set('oidc_prompt', $prompt); + $this->session->set('oidc_max_age', $maxAge); + $this->session->set('oidc_login_pending', true); + + $afterLoginRedirectUrl = $this->urlGenerator->linkToRoute('oidc.Page.index', array_filter([ + 'client_id' => $clientId, + 'state' => $state, + 'response_type' => $responseType, + 'redirect_uri' => $redirectUri, + 'scope' => $scope, + 'nonce' => $nonce, + 'resource' => $resource, + 'code_challenge' => $codeChallenge, + 'code_challenge_method' => $codeChallengeMethod, + 'prompt' => $prompt, + 'max_age' => $maxAge, + ], static function ($value): bool { + return $value !== null && $value !== ''; + })); + + $loginUrl = $this->urlGenerator->linkToRoute( + 'core.login.showLoginForm', + [ + 'redirect_url' => $afterLoginRedirectUrl + ] + ); + + $this->session->close(); // Close session to prevent session locking issues during redirect + + $this->logger->debug($logMessage); + + return new RedirectResponse($loginUrl); + } + private function promptContains(mixed $prompt, string $expectedPrompt): bool { if (!is_string($prompt) || trim($prompt) === '') { diff --git a/tests/Unit/Controller/LoginRedirectorControllerTest.php b/tests/Unit/Controller/LoginRedirectorControllerTest.php index 6386b3c3..78185d27 100644 --- a/tests/Unit/Controller/LoginRedirectorControllerTest.php +++ b/tests/Unit/Controller/LoginRedirectorControllerTest.php @@ -444,13 +444,124 @@ public function testAuthorizeUsesStoredOidcAuthenticationTimeForAccessToken() { null, null, null, - '10000' + null ); $this->assertEquals(Http::STATUS_SEE_OTHER, $result->getStatus(), 'Status Code does not match!'); $this->assertStringStartsWith($redirectUri . '?state=state-1&code=', $result->getRedirectURL()); } + public function testAuthorizeMaxAgeExceededForcesReauthentication() { + $clientId = 'client1'; + $state = 'state-1'; + $redirectUri = 'https://client.example.com/callback'; + + $client = new Client( + 'Test Client', + [$redirectUri], + 'RS256', + 'confidential', + 'code', + 'opaque', + 'openid', + '', + false + ); + $client->id = 1; + $client->setClientIdentifier($clientId); + + $registeredRedirectUri = new RedirectUri(); + $registeredRedirectUri->setClientId(1); + $registeredRedirectUri->setRedirectUri($redirectUri); + + $time = $this->createMock(ITimeFactory::class); + $time + ->method('getTime') + ->willReturn(2000); + + $controller = new LoginRedirectorController( + 'oidc', + $this->request, + $this->urlGenerator, + $this->clientMapper, + $this->groupMapper, + $this->secureRandom, + $this->session, + $this->l, + $time, + $this->userSession, + $this->groupManager, + $this->accessTokenMapper, + $this->redirectUriMapper, + $this->userConsentMapper, + $this->appConfig, + $this->jwtGenerator, + $this->redirectUriService, + $this->logger + ); + + $this->userSession + ->method('isLoggedIn') + ->willReturn(true); + $this->userSession + ->expects($this->once()) + ->method('logout'); + $this->session + ->method('get') + ->willReturnCallback(function ($key) { + $values = [ + 'oidc_auth_time' => 1000, + 'oidc_login_pending' => false, + ]; + return $values[$key] ?? null; + }); + $this->session + ->expects($this->atLeastOnce()) + ->method('set'); + $this->clientMapper + ->method('getByIdentifier') + ->with($clientId) + ->willReturn($client); + $this->redirectUriMapper + ->method('getByClientId') + ->with(1) + ->willReturn([$registeredRedirectUri]); + $this->urlGenerator + ->method('linkToRoute') + ->willReturnCallback(function ($route) { + if ($route === 'oidc.Page.index') { + return '/index.php/apps/oidc/redirect?client_id=client1'; + } + if ($route === 'core.login.showLoginForm') { + return '/index.php/login?redirect_url=/index.php/apps/oidc/redirect?client_id=client1'; + } + return '/unexpected'; + }); + $this->accessTokenMapper + ->expects($this->never()) + ->method('insert'); + + $result = $controller->authorize( + $clientId, + $state, + 'code', + $redirectUri, + 'openid', + 'nonce-1', + null, + null, + null, + null, + '1' + ); + + $this->assertEquals(Http::STATUS_SEE_OTHER, $result->getStatus(), 'Status Code does not match!'); + $this->assertEquals( + '/index.php/login?redirect_url=/index.php/apps/oidc/redirect?client_id=client1', + $result->getRedirectURL() + ); + } + public function testAuthorizeRejectsUnsupportedRequestObject() { $clientId = 'client1'; $state = 'state-1'; From 088252241f61dabcd25d90a16ac49b6a87cea71a Mon Sep 17 00:00:00 2001 From: Thorsten Jagel Date: Thu, 11 Jun 2026 10:39:14 +0200 Subject: [PATCH 23/28] oidcc-prompt-login compliance fix --- lib/Controller/LoginRedirectorController.php | 20 +++++ .../LoginRedirectorControllerTest.php | 84 +++++++++++++++++++ 2 files changed, 104 insertions(+) diff --git a/lib/Controller/LoginRedirectorController.php b/lib/Controller/LoginRedirectorController.php index 922a2b9e..f3defe26 100644 --- a/lib/Controller/LoginRedirectorController.php +++ b/lib/Controller/LoginRedirectorController.php @@ -304,6 +304,7 @@ public function authorize( $max_age = $this->session->get('oidc_max_age'); } + $oidcLoginPending = $this->session->get('oidc_login_pending') === true; $authTime = $this->getOidcAuthenticationTime(); // Guard: if critical OAuth params are still missing after session fallback, @@ -409,6 +410,25 @@ public function authorize( return new RedirectResponse($url); } + if ($this->promptContains($prompt, 'login') && !$oidcLoginPending) { + $this->logger->debug('prompt=login requested for client ' . $client_id . '. Forcing reauthentication.'); + $this->userSession->logout(); + return $this->redirectToLoginAfterOidcAuthentication( + $client_id, + $state, + $response_type, + $redirect_uri, + $scope, + $nonce, + $resource, + $code_challenge, + $code_challenge_method, + $prompt, + $max_age, + 'Redirect to login for prompt=login reauthentication for client ' . $client_id . '.' + ); + } + if ($this->maxAgeExceeded($max_age, $authTime)) { if ($this->promptContains($prompt, 'none')) { $this->logger->debug('prompt=none requested but max_age is exceeded for client ' . $client_id . '. Returning login_required.'); diff --git a/tests/Unit/Controller/LoginRedirectorControllerTest.php b/tests/Unit/Controller/LoginRedirectorControllerTest.php index 78185d27..3f175a12 100644 --- a/tests/Unit/Controller/LoginRedirectorControllerTest.php +++ b/tests/Unit/Controller/LoginRedirectorControllerTest.php @@ -562,6 +562,90 @@ public function testAuthorizeMaxAgeExceededForcesReauthentication() { ); } + public function testAuthorizePromptLoginForcesReauthentication() { + $clientId = 'client1'; + $state = 'state-1'; + $redirectUri = 'https://client.example.com/callback'; + + $client = new Client( + 'Test Client', + [$redirectUri], + 'RS256', + 'confidential', + 'code', + 'opaque', + 'openid', + '', + false + ); + $client->id = 1; + $client->setClientIdentifier($clientId); + + $registeredRedirectUri = new RedirectUri(); + $registeredRedirectUri->setClientId(1); + $registeredRedirectUri->setRedirectUri($redirectUri); + + $this->userSession + ->method('isLoggedIn') + ->willReturn(true); + $this->userSession + ->expects($this->once()) + ->method('logout'); + $this->session + ->method('get') + ->willReturnCallback(function ($key) { + $values = [ + 'oidc_auth_time' => 2000, + 'oidc_login_pending' => false, + ]; + return $values[$key] ?? null; + }); + $this->session + ->expects($this->atLeastOnce()) + ->method('set'); + $this->clientMapper + ->method('getByIdentifier') + ->with($clientId) + ->willReturn($client); + $this->redirectUriMapper + ->method('getByClientId') + ->with(1) + ->willReturn([$registeredRedirectUri]); + $this->urlGenerator + ->method('linkToRoute') + ->willReturnCallback(function ($route) { + if ($route === 'oidc.Page.index') { + return '/index.php/apps/oidc/redirect?client_id=client1&prompt=login'; + } + if ($route === 'core.login.showLoginForm') { + return '/index.php/login?redirect_url=/index.php/apps/oidc/redirect?client_id=client1&prompt=login'; + } + return '/unexpected'; + }); + $this->accessTokenMapper + ->expects($this->never()) + ->method('insert'); + + $result = $this->controller->authorize( + $clientId, + $state, + 'code', + $redirectUri, + 'openid', + 'nonce-1', + null, + null, + null, + 'login' + ); + + $this->assertEquals(Http::STATUS_SEE_OTHER, $result->getStatus(), 'Status Code does not match!'); + $this->assertEquals( + '/index.php/login?redirect_url=/index.php/apps/oidc/redirect?client_id=client1&prompt=login', + $result->getRedirectURL() + ); + } + public function testAuthorizeRejectsUnsupportedRequestObject() { $clientId = 'client1'; $state = 'state-1'; From 31d114bbbcd3e3d231f5e6e61d266b017e587898 Mon Sep 17 00:00:00 2001 From: Thorsten Jagel Date: Thu, 11 Jun 2026 12:14:08 +0200 Subject: [PATCH 24/28] Fixed oidcc-ensure-registered-redirect-uri and oidcc-ensure-request-object-with-redirect-uri --- .github/conformance/browser-runner.py | 116 +++++++++++++++++++++++++- 1 file changed, 114 insertions(+), 2 deletions(-) diff --git a/.github/conformance/browser-runner.py b/.github/conformance/browser-runner.py index 2cef57d8..05df3d28 100644 --- a/.github/conformance/browser-runner.py +++ b/.github/conformance/browser-runner.py @@ -8,6 +8,7 @@ through the suite's existing runner API. """ +import base64 import os import sys import time @@ -28,6 +29,8 @@ POLL_SECONDS = float(os.environ.get("CONFORMANCE_BROWSER_POLL_SECONDS", "1")) VISIT_TIMEOUT_SECONDS = int(os.environ.get("CONFORMANCE_BROWSER_VISIT_TIMEOUT", "90")) LOGIN_REDIRECT_TIMEOUT_SECONDS = int(os.environ.get("CONFORMANCE_BROWSER_LOGIN_REDIRECT_TIMEOUT", "15")) +PLACEHOLDER_CHECK_SECONDS = float(os.environ.get("CONFORMANCE_BROWSER_PLACEHOLDER_CHECK_SECONDS", "2")) +SCREENSHOT_STABILITY_SECONDS = float(os.environ.get("CONFORMANCE_BROWSER_SCREENSHOT_STABILITY_SECONDS", "2")) def log(message): @@ -199,7 +202,98 @@ def page_diagnostics(driver): } -def drive_url(driver, method, url): +def page_ready_for_screenshot(driver): + diag = page_diagnostics(driver) + if diag["ready_state"] != "complete": + return False + return bool(diag["title"] or diag["body"]) + + +def get_pending_upload_placeholder(client, test_id, uploaded_placeholders): + try: + response = client.get(f"{CONFORMANCE_SERVER}/api/log/{test_id}") + response.raise_for_status() + log_entries = response.json() + except Exception as exc: + log(f"Unable to read log placeholders for {test_id}: {exc}") + return None + + for entry in reversed(log_entries): + placeholder = entry.get("upload") + if ( + placeholder + and entry.get("result") == "REVIEW" + and (test_id, placeholder) not in uploaded_placeholders + ): + return placeholder + + return None + + +def screenshot_data_urls(driver): + try: + for quality in (80, 60, 40): + result = driver.execute_cdp_cmd( + "Page.captureScreenshot", + { + "format": "jpeg", + "quality": quality, + "captureBeyondViewport": False, + }, + ) + encoded = result.get("data") + if encoded: + yield f"jpeg q{quality}", f"data:image/jpeg;base64,{encoded}" + except Exception as exc: + log(f"Unable to capture JPEG screenshot through Chrome DevTools: {exc}") + + encoded = base64.b64encode(driver.get_screenshot_as_png()).decode("ascii") + yield "png", f"data:image/png;base64,{encoded}" + + +def encoded_data_size(data_url): + marker = "base64," + marker_index = data_url.find(marker) + if marker_index == -1: + return 0 + try: + return len(base64.b64decode(data_url[marker_index + len(marker):])) + except Exception: + return 0 + + +def upload_review_screenshot(client, test_id, placeholder, driver): + url = f"{CONFORMANCE_SERVER}/api/log/{test_id}/images/{placeholder}" + + for label, data_url in screenshot_data_urls(driver): + size = encoded_data_size(data_url) + log(f"Uploading {label} screenshot for placeholder {placeholder} ({size} bytes)") + try: + response = client.post( + url, + content=data_url, + headers={"Content-Type": "text/plain; charset=utf-8"}, + ) + except Exception as exc: + log(f"Unable to upload screenshot for {test_id}: {exc}") + return False + + if response.status_code == 200: + log(f"Uploaded screenshot for review placeholder {placeholder}") + return True + + body = " ".join(response.text.split()) + if len(body) > 300: + body = body[:300] + "..." + log(f"Screenshot upload failed with HTTP {response.status_code}: {body}") + + if response.status_code != 400: + return False + + return False + + +def drive_url(driver, client, test_id, uploaded_placeholders, method, url): log(f"Visiting {method} {url}") if method.upper() == "POST": submit_post(driver, url) @@ -208,10 +302,14 @@ def drive_url(driver, method, url): deadline = time.monotonic() + VISIT_TIMEOUT_SECONDS last_seen_url = None + last_url_changed_at = time.monotonic() + next_placeholder_check = 0 while time.monotonic() < deadline: + now = time.monotonic() current_url = driver.current_url if current_url != last_seen_url: last_seen_url = current_url + last_url_changed_at = now log(f"Browser at {current_url}") if is_conformance_callback(current_url): @@ -225,6 +323,19 @@ def drive_url(driver, method, url): if grant_consent_if_present(driver): continue + if ( + now >= next_placeholder_check + and now - last_url_changed_at >= SCREENSHOT_STABILITY_SECONDS + and page_ready_for_screenshot(driver) + ): + next_placeholder_check = now + PLACEHOLDER_CHECK_SECONDS + placeholder = get_pending_upload_placeholder(client, test_id, uploaded_placeholders) + if placeholder: + log(f"Review placeholder {placeholder} is pending at {current_url}") + if upload_review_screenshot(client, test_id, placeholder, driver): + uploaded_placeholders.add((test_id, placeholder)) + return current_url + time.sleep(0.5) log(f"Timed out waiting for callback; current URL is {driver.current_url}") @@ -269,6 +380,7 @@ def close_driver(drivers, test_id): def main(): drivers = {} processed = set() + uploaded_placeholders = set() active_test_id = None with httpx.Client(verify=False, timeout=15.0) as client: @@ -304,7 +416,7 @@ def main(): processed.add(key) try: - drive_url(driver, method, url) + drive_url(driver, client, test_id, uploaded_placeholders, method, url) except Exception as exc: log(f"Browser visit failed for {test_id}: {exc}") finally: From 31641cdf590f2c2ef9477033b8b26f7a34a129cb Mon Sep 17 00:00:00 2001 From: Thorsten Jagel Date: Thu, 11 Jun 2026 12:58:37 +0200 Subject: [PATCH 25/28] Fix oidcc-max-age-1 / oidcc-prompt-login interruption --- .github/conformance/browser-runner.py | 34 ++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/.github/conformance/browser-runner.py b/.github/conformance/browser-runner.py index 05df3d28..cc6c8498 100644 --- a/.github/conformance/browser-runner.py +++ b/.github/conformance/browser-runner.py @@ -293,6 +293,22 @@ def upload_review_screenshot(client, test_id, placeholder, driver): return False +def maybe_upload_pending_review_screenshot(client, test_id, uploaded_placeholders, driver): + placeholder = get_pending_upload_placeholder(client, test_id, uploaded_placeholders) + if not placeholder: + return False + + if not page_ready_for_screenshot(driver): + return None + + log(f"Review placeholder {placeholder} is pending at {driver.current_url}") + if upload_review_screenshot(client, test_id, placeholder, driver): + uploaded_placeholders.add((test_id, placeholder)) + return True + + return False + + def drive_url(driver, client, test_id, uploaded_placeholders, method, url): log(f"Visiting {method} {url}") if method.upper() == "POST": @@ -317,6 +333,16 @@ def drive_url(driver, client, test_id, uploaded_placeholders, method, url): return current_url if is_login_page(driver): + upload_result = maybe_upload_pending_review_screenshot( + client, + test_id, + uploaded_placeholders, + driver, + ) + if upload_result is None: + time.sleep(0.2) + continue + login(driver) continue @@ -329,12 +355,8 @@ def drive_url(driver, client, test_id, uploaded_placeholders, method, url): and page_ready_for_screenshot(driver) ): next_placeholder_check = now + PLACEHOLDER_CHECK_SECONDS - placeholder = get_pending_upload_placeholder(client, test_id, uploaded_placeholders) - if placeholder: - log(f"Review placeholder {placeholder} is pending at {current_url}") - if upload_review_screenshot(client, test_id, placeholder, driver): - uploaded_placeholders.add((test_id, placeholder)) - return current_url + if maybe_upload_pending_review_screenshot(client, test_id, uploaded_placeholders, driver): + return current_url time.sleep(0.5) From 3ac4029c2ceed38c7ce84c797ccc880efac255dc Mon Sep 17 00:00:00 2001 From: Thorsten Jagel Date: Thu, 11 Jun 2026 14:24:38 +0200 Subject: [PATCH 26/28] Fixed browser runner --- .github/conformance/browser-runner.py | 39 +++++++++++++++++---------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/.github/conformance/browser-runner.py b/.github/conformance/browser-runner.py index cc6c8498..571051a9 100644 --- a/.github/conformance/browser-runner.py +++ b/.github/conformance/browser-runner.py @@ -209,7 +209,7 @@ def page_ready_for_screenshot(driver): return bool(diag["title"] or diag["body"]) -def get_pending_upload_placeholder(client, test_id, uploaded_placeholders): +def get_pending_upload_entry(client, test_id, uploaded_placeholders): try: response = client.get(f"{CONFORMANCE_SERVER}/api/log/{test_id}") response.raise_for_status() @@ -225,7 +225,7 @@ def get_pending_upload_placeholder(client, test_id, uploaded_placeholders): and entry.get("result") == "REVIEW" and (test_id, placeholder) not in uploaded_placeholders ): - return placeholder + return entry return None @@ -293,14 +293,22 @@ def upload_review_screenshot(client, test_id, placeholder, driver): return False -def maybe_upload_pending_review_screenshot(client, test_id, uploaded_placeholders, driver): - placeholder = get_pending_upload_placeholder(client, test_id, uploaded_placeholders) - if not placeholder: +def is_second_login_placeholder(entry): + return ( + entry.get("src") == "ExpectSecondLoginPage" + or "login for a second time" in (entry.get("msg") or "").lower() + ) + + +def maybe_upload_pending_review_screenshot(client, test_id, uploaded_placeholders, driver, entry=None): + entry = entry or get_pending_upload_entry(client, test_id, uploaded_placeholders) + if not entry: return False if not page_ready_for_screenshot(driver): return None + placeholder = entry.get("upload") log(f"Review placeholder {placeholder} is pending at {driver.current_url}") if upload_review_screenshot(client, test_id, placeholder, driver): uploaded_placeholders.add((test_id, placeholder)) @@ -333,15 +341,18 @@ def drive_url(driver, client, test_id, uploaded_placeholders, method, url): return current_url if is_login_page(driver): - upload_result = maybe_upload_pending_review_screenshot( - client, - test_id, - uploaded_placeholders, - driver, - ) - if upload_result is None: - time.sleep(0.2) - continue + placeholder_entry = get_pending_upload_entry(client, test_id, uploaded_placeholders) + if placeholder_entry and is_second_login_placeholder(placeholder_entry): + upload_result = maybe_upload_pending_review_screenshot( + client, + test_id, + uploaded_placeholders, + driver, + placeholder_entry, + ) + if upload_result is None: + time.sleep(0.2) + continue login(driver) continue From 0e31d556a0c57388e76461907c47c797cfd9ebca Mon Sep 17 00:00:00 2001 From: Thorsten Jagel Date: Thu, 11 Jun 2026 15:03:31 +0200 Subject: [PATCH 27/28] Change failure behaviour --- .github/conformance/check-results.py | 110 +++++++++++++++++++++++++++ .github/workflows/build-test.yaml | 15 +++- 2 files changed, 124 insertions(+), 1 deletion(-) create mode 100644 .github/conformance/check-results.py diff --git a/.github/conformance/check-results.py b/.github/conformance/check-results.py new file mode 100644 index 00000000..356aa413 --- /dev/null +++ b/.github/conformance/check-results.py @@ -0,0 +1,110 @@ +#!/usr/bin/env python3 +"""Exit non-zero only when exported conformance results contain blocking failures.""" + +from __future__ import annotations + +import argparse +import json +import pathlib +import sys +import zipfile +from collections import Counter + + +BLOCKING_RESULTS = {"FAILED", "FAILURE", "ERROR", "INTERRUPTED", "UNKNOWN"} +NON_BLOCKING_RESULTS = {"PASSED", "SUCCESS", "SKIPPED", "WARNING", "REVIEW"} +FINISHED_STATUSES = {"FINISHED"} + + +def iter_export_logs(results_dir: pathlib.Path): + for zip_path in sorted(results_dir.glob("*.zip")): + with zipfile.ZipFile(zip_path) as export: + for name in sorted(export.namelist()): + if not name.endswith(".json"): + continue + with export.open(name) as handle: + yield zip_path.name, name, json.load(handle) + + for json_path in sorted(results_dir.glob("test-log-*.json")): + with json_path.open(encoding="utf-8") as handle: + yield json_path.name, json_path.name, json.load(handle) + + +def normalized_result(test_info: dict) -> str: + result = test_info.get("result") + if result: + return str(result).upper() + + status = test_info.get("status") + if status and status != "FINISHED": + return str(status).upper() + + return "UNKNOWN" + + +def is_blocking(test_info: dict) -> bool: + result = normalized_result(test_info) + status = str(test_info.get("status") or "").upper() + + if result in BLOCKING_RESULTS: + return True + + if result not in NON_BLOCKING_RESULTS: + return True + + return bool(status and status not in FINISHED_STATUSES and result != "SKIPPED") + + +def test_label(test_info: dict, log_name: str) -> str: + name = test_info.get("testName") or pathlib.Path(log_name).stem + test_id = test_info.get("testId") or test_info.get("_id") + if test_id: + return f"{name} ({test_id})" + return str(name) + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--results-dir", default="conformance-results", type=pathlib.Path) + args = parser.parse_args() + + tests = [] + blocking = [] + counts = Counter() + + for export_name, log_name, data in iter_export_logs(args.results_dir): + test_info = data.get("testInfo", {}) + result = normalized_result(test_info) + counts[result] += 1 + item = { + "export": export_name, + "label": test_label(test_info, log_name), + "result": result, + "status": test_info.get("status") or "", + } + tests.append(item) + if is_blocking(test_info): + blocking.append(item) + + if not tests: + print(f"No conformance test logs found in {args.results_dir}", file=sys.stderr) + return 1 + + summary = ", ".join(f"{result}={count}" for result, count in sorted(counts.items())) + print(f"Conformance result summary: {summary}") + + if not blocking: + print("No blocking conformance failures found.") + return 0 + + print("Blocking conformance failures found:", file=sys.stderr) + for item in blocking: + print( + f"- {item['label']}: result={item['result']} status={item['status']} export={item['export']}", + file=sys.stderr, + ) + return 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.github/workflows/build-test.yaml b/.github/workflows/build-test.yaml index 2c4565f9..a73661d0 100644 --- a/.github/workflows/build-test.yaml +++ b/.github/workflows/build-test.yaml @@ -377,7 +377,20 @@ jobs: echo "::group::conformance-browser.log" cat ../conformance-browser.log || true echo "::endgroup::" - exit "${test_plan_status}" + set +e + python3 ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/check-results.py \ + --results-dir ../conformance-results + blocking_status=$? + set -e + if [ "${blocking_status}" -ne 0 ]; then + if [ "${test_plan_status}" -ne 0 ]; then + exit "${test_plan_status}" + fi + exit "${blocking_status}" + fi + if [ "${test_plan_status}" -ne 0 ]; then + echo "Conformance suite exited with ${test_plan_status}, but exported results contain only non-blocking WARNING/REVIEW outcomes." + fi - name: Generate conformance report if: always() From 090092238ba38fbd77b50d50b4af7e475d7fca7d Mon Sep 17 00:00:00 2001 From: Thorsten Jagel Date: Thu, 11 Jun 2026 15:25:12 +0200 Subject: [PATCH 28/28] Moved oidc conformance to separate workflow --- .github/workflows/build-test.yaml | 263 ----------------------- .github/workflows/oidc-conformance.yaml | 266 ++++++++++++++++++++++++ 2 files changed, 266 insertions(+), 263 deletions(-) create mode 100644 .github/workflows/oidc-conformance.yaml diff --git a/.github/workflows/build-test.yaml b/.github/workflows/build-test.yaml index a73661d0..bda2f700 100644 --- a/.github/workflows/build-test.yaml +++ b/.github/workflows/build-test.yaml @@ -13,12 +13,6 @@ on: branches: - "*" workflow_dispatch: - inputs: - run_conformance: - description: "Run the OpenID Foundation OIDC conformance suite" - required: false - default: false - type: boolean env: PHP_VERSION: 8.5 @@ -153,260 +147,3 @@ jobs: openssl dgst -sha512 -sign ${{ env.APP_NAME }}.key nextcloud/apps/${{ env.APP_NAME }}/build/artifacts/${{ env.APP_NAME }}.tar.gz | openssl base64 -A > signature.sha rm -f ${{ env.APP_NAME }}.key rm -f ${{ env.APP_NAME }}.crt - - oidc_conformance: - runs-on: ubuntu-latest - needs: build_test - if: github.event_name == 'workflow_dispatch' && inputs.run_conformance - timeout-minutes: 90 - - env: - CONFORMANCE_ALIAS: nextcloud-${{ github.run_id }} - NEXTCLOUD_BASE_URL: http://host.docker.internal:8080 - OIDC_TEST_USER: oidc-test-user - OIDC_TEST_PASSWORD: oidc-test-password - OIDC_CLIENT_ID_1: nextcloud-conformance-client-00001 - OIDC_CLIENT_SECRET_1: nextcloud-conformance-secret-00001 - OIDC_CLIENT_ID_2: nextcloud-conformance-client-00002 - OIDC_CLIENT_SECRET_2: nextcloud-conformance-secret-00002 - - steps: - - name: Set app env - run: | - echo "APP_NAME=${GITHUB_REPOSITORY##*/}" >> $GITHUB_ENV - - - name: Checkout - uses: actions/checkout@v3 - with: - path: ${{ env.APP_NAME }} - - - name: Get appinfo data - id: appinfo - uses: mavrosxristoforos/get-xml-info@1.1.1 - with: - xml-file: ${{ env.APP_NAME }}/appinfo/info.xml - xpath: "//info//dependencies//nextcloud/@max-version" - - - name: Derive Nextcloud checkout ref - shell: bash - run: | - max_version="${{ steps.appinfo.outputs.info }}" - if [ -z "$max_version" ]; then - max_version="${{ steps.appinfo.outputs.value }}" - fi - if [ -z "$max_version" ]; then - echo "Error: could not read nextcloud max-version from appinfo" >&2 - exit 1 - fi - major="${max_version%%.*}" - echo "NEXTCLOUD_REF=stable$major" >> $GITHUB_ENV - - - name: Read package.json node and npm engines version - uses: skjnldsv/read-package-engines-version-actions@v2 - id: versions - continue-on-error: true - with: - path: ${{ env.APP_NAME }} - fallbackNode: "^22" - fallbackNpm: "^10" - - - name: Set up node ${{ steps.versions.outputs.nodeVersion }} - if: ${{ steps.versions.outputs.nodeVersion }} - uses: actions/setup-node@v3 - with: - node-version: ${{ steps.versions.outputs.nodeVersion }} - - - name: Set up npm ${{ steps.versions.outputs.npmVersion }} - if: ${{ steps.versions.outputs.npmVersion }} - run: npm i -g npm@"${{ steps.versions.outputs.npmVersion }}" - - - name: Set up php ${{ env.PHP_VERSION }} - uses: shivammathur/setup-php@v2 - with: - php-version: ${{ env.PHP_VERSION }} - coverage: none - - - name: Set up Java - uses: actions/setup-java@v4 - with: - distribution: temurin - java-version: "21" - - - name: Install app dependencies and build assets - run: | - cd ${{ env.APP_NAME }} - composer install - npm ci - npm run build - - - name: Checkout Nextcloud server - uses: actions/checkout@v3 - with: - repository: nextcloud/server - path: nextcloud - submodules: recursive - ref: ${{ env.NEXTCLOUD_REF }} - - - name: Install Nextcloud dependencies - run: | - cd nextcloud - composer install - - - name: Install and configure Nextcloud OIDC provider - run: | - mv -v ${{ env.APP_NAME }} nextcloud/apps/ - cd nextcloud - php occ maintenance:install --admin-user admin --admin-pass admin - php occ app:enable ${{ env.APP_NAME }} - php occ config:system:set trusted_domains 1 --value=host.docker.internal - php occ config:system:set trusted_domains 2 --value=127.0.0.1 - php occ config:system:set overwrite.cli.url --value="${NEXTCLOUD_BASE_URL}" - php occ config:system:set overwritehost --value=host.docker.internal:8080 - php occ config:system:set overwriteprotocol --value=http - php occ config:app:set ${{ env.APP_NAME }} allow_user_settings --value no - OC_PASS="${OIDC_TEST_PASSWORD}" php occ user:add --password-from-env "${OIDC_TEST_USER}" - php occ user:setting "${OIDC_TEST_USER}" settings email oidc-test@example.test - callback="https://nginx:8443/test/a/${CONFORMANCE_ALIAS}/callback" - php occ oidc:create "OIDC Conformance Basic 1" "${callback}" \ - --client_id "${OIDC_CLIENT_ID_1}" \ - --client_secret "${OIDC_CLIENT_SECRET_1}" \ - --type confidential \ - --flow code \ - --allowed_scopes "openid profile email offline_access" - php occ oidc:create "OIDC Conformance Basic 2" "${callback}" \ - --client_id "${OIDC_CLIENT_ID_2}" \ - --client_secret "${OIDC_CLIENT_SECRET_2}" \ - --type confidential \ - --flow code \ - --allowed_scopes "openid profile email offline_access" - - - name: Start Nextcloud web server - run: | - cd nextcloud - php -S 0.0.0.0:8080 -t . > ../nextcloud-server.log 2>&1 & - echo $! > ../nextcloud-server.pid - for i in {1..30}; do - if curl --fail --silent http://127.0.0.1:8080/index.php/apps/oidc/openid-configuration > /dev/null; then - exit 0 - fi - sleep 2 - done - cat ../nextcloud-server.log - exit 1 - - - name: Checkout OpenID conformance suite - run: git clone --depth 1 https://gitlab.com/openid/conformance-suite.git conformance-suite - - - name: Build and start OpenID conformance suite - run: | - sudo sh -c 'echo "127.0.0.1 nginx" >> /etc/hosts' - sudo sh -c 'echo "127.0.0.1 oidcc-provider" >> /etc/hosts' - sudo sh -c 'echo "127.0.0.1 chrome" >> /etc/hosts' - cd conformance-suite - mvn -B -DskipTests package - docker compose \ - -f docker-compose-localtest.yml \ - -f ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/docker-compose.github-actions.yml \ - -f ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/docker-compose.chrome.yml \ - up -d mongodb nginx server chrome - # Wait for Chrome/Selenium service to be ready - for i in {1..60}; do - if curl --fail --silent http://chrome:4444/status > /dev/null 2>&1; then - echo "Chrome/Selenium service is ready" - break - fi - sleep 2 - done - # Wait for API runner - for i in {1..60}; do - if curl --insecure --fail --silent https://nginx:8443/api/runner/available > /dev/null; then - exit 0 - fi - sleep 5 - done - docker compose \ - -f docker-compose-localtest.yml \ - -f ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/docker-compose.github-actions.yml \ - -f ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/docker-compose.chrome.yml \ - logs server nginx chrome - exit 1 - - - name: Install conformance runner dependencies - run: python3 -m pip install --user httpx pyparsing selenium - - - name: Write conformance plan config - run: | - mkdir -p conformance-results conformance-config - python3 nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/write-config.py \ - nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/oidc-basic-config.json \ - conformance-config/oidc-basic-config.json - python3 -m json.tool conformance-config/oidc-basic-config.json > /dev/null - - - name: Run OIDC basic conformance plan - env: - CONFORMANCE_DEV_MODE: 1 - CONFORMANCE_SERVER: https://nginx:8443/ - CONFORMANCE_SERVER_MTLS: https://nginx:8444/ - SELENIUM_REMOTE_URL: http://chrome:4444/wd/hub - CONFORMANCE_BROWSER_VISIT_TIMEOUT: 90 - run: | - cd conformance-suite - : > ../conformance-browser.log - PYTHONUNBUFFERED=1 python3 ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/browser-runner.py \ - >> ../conformance-browser.log 2>&1 & - browser_runner_pid=$! - tail -n +1 -f ../conformance-browser.log & - browser_log_tail_pid=$! - cleanup_browser_runner() { - kill "${browser_runner_pid}" 2>/dev/null || true - kill "${browser_log_tail_pid}" 2>/dev/null || true - wait "${browser_runner_pid}" 2>/dev/null || true - wait "${browser_log_tail_pid}" 2>/dev/null || true - } - trap cleanup_browser_runner EXIT - set +e - python3 scripts/run-test-plan.py \ - --export-dir ../conformance-results \ - --verbose \ - "oidcc-basic-certification-test-plan[server_metadata=discovery][client_registration=static_client]" \ - ../conformance-config/oidc-basic-config.json - test_plan_status=$? - set -e - cleanup_browser_runner - trap - EXIT - echo "::group::conformance-browser.log" - cat ../conformance-browser.log || true - echo "::endgroup::" - set +e - python3 ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/check-results.py \ - --results-dir ../conformance-results - blocking_status=$? - set -e - if [ "${blocking_status}" -ne 0 ]; then - if [ "${test_plan_status}" -ne 0 ]; then - exit "${test_plan_status}" - fi - exit "${blocking_status}" - fi - if [ "${test_plan_status}" -ne 0 ]; then - echo "Conformance suite exited with ${test_plan_status}, but exported results contain only non-blocking WARNING/REVIEW outcomes." - fi - - - name: Generate conformance report - if: always() - run: | - python3 nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/generate-report.py \ - --results-dir conformance-results \ - --output CONFORMANCE.md - cat CONFORMANCE.md - - - name: Upload conformance results - if: always() - uses: actions/upload-artifact@v4 - with: - name: oidc-conformance-results - path: | - CONFORMANCE.md - conformance-results - conformance-browser.log - nextcloud-server.log diff --git a/.github/workflows/oidc-conformance.yaml b/.github/workflows/oidc-conformance.yaml new file mode 100644 index 00000000..70715f6c --- /dev/null +++ b/.github/workflows/oidc-conformance.yaml @@ -0,0 +1,266 @@ +name: OIDC conformance + +on: + schedule: + - cron: "17 2 * * *" + workflow_dispatch: + +env: + PHP_VERSION: 8.5 + +jobs: + oidc_conformance: + runs-on: ubuntu-latest + if: github.event_name != 'schedule' || github.ref == 'refs/heads/master' + timeout-minutes: 90 + + env: + CONFORMANCE_ALIAS: nextcloud-${{ github.run_id }} + NEXTCLOUD_BASE_URL: http://host.docker.internal:8080 + OIDC_TEST_USER: oidc-test-user + OIDC_TEST_PASSWORD: oidc-test-password + OIDC_CLIENT_ID_1: nextcloud-conformance-client-00001 + OIDC_CLIENT_SECRET_1: nextcloud-conformance-secret-00001 + OIDC_CLIENT_ID_2: nextcloud-conformance-client-00002 + OIDC_CLIENT_SECRET_2: nextcloud-conformance-secret-00002 + + steps: + - name: Set app env + run: | + echo "APP_NAME=${GITHUB_REPOSITORY##*/}" >> $GITHUB_ENV + + - name: Checkout + uses: actions/checkout@v3 + with: + path: ${{ env.APP_NAME }} + + - name: Get appinfo data + id: appinfo + uses: mavrosxristoforos/get-xml-info@1.1.1 + with: + xml-file: ${{ env.APP_NAME }}/appinfo/info.xml + xpath: "//info//dependencies//nextcloud/@max-version" + + - name: Derive Nextcloud checkout ref + shell: bash + run: | + max_version="${{ steps.appinfo.outputs.info }}" + if [ -z "$max_version" ]; then + max_version="${{ steps.appinfo.outputs.value }}" + fi + if [ -z "$max_version" ]; then + echo "Error: could not read nextcloud max-version from appinfo" >&2 + exit 1 + fi + major="${max_version%%.*}" + echo "NEXTCLOUD_REF=stable$major" >> $GITHUB_ENV + + - name: Read package.json node and npm engines version + uses: skjnldsv/read-package-engines-version-actions@v2 + id: versions + continue-on-error: true + with: + path: ${{ env.APP_NAME }} + fallbackNode: "^22" + fallbackNpm: "^10" + + - name: Set up node ${{ steps.versions.outputs.nodeVersion }} + if: ${{ steps.versions.outputs.nodeVersion }} + uses: actions/setup-node@v3 + with: + node-version: ${{ steps.versions.outputs.nodeVersion }} + + - name: Set up npm ${{ steps.versions.outputs.npmVersion }} + if: ${{ steps.versions.outputs.npmVersion }} + run: npm i -g npm@"${{ steps.versions.outputs.npmVersion }}" + + - name: Set up php ${{ env.PHP_VERSION }} + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ env.PHP_VERSION }} + coverage: none + + - name: Set up Java + uses: actions/setup-java@v4 + with: + distribution: temurin + java-version: "21" + + - name: Install app dependencies and build assets + run: | + cd ${{ env.APP_NAME }} + composer install + npm ci + npm run build + + - name: Checkout Nextcloud server + uses: actions/checkout@v3 + with: + repository: nextcloud/server + path: nextcloud + submodules: recursive + ref: ${{ env.NEXTCLOUD_REF }} + + - name: Install Nextcloud dependencies + run: | + cd nextcloud + composer install + + - name: Install and configure Nextcloud OIDC provider + run: | + mv -v ${{ env.APP_NAME }} nextcloud/apps/ + cd nextcloud + php occ maintenance:install --admin-user admin --admin-pass admin + php occ app:enable ${{ env.APP_NAME }} + php occ config:system:set trusted_domains 1 --value=host.docker.internal + php occ config:system:set trusted_domains 2 --value=127.0.0.1 + php occ config:system:set overwrite.cli.url --value="${NEXTCLOUD_BASE_URL}" + php occ config:system:set overwritehost --value=host.docker.internal:8080 + php occ config:system:set overwriteprotocol --value=http + php occ config:app:set ${{ env.APP_NAME }} allow_user_settings --value no + OC_PASS="${OIDC_TEST_PASSWORD}" php occ user:add --password-from-env "${OIDC_TEST_USER}" + php occ user:setting "${OIDC_TEST_USER}" settings email oidc-test@example.test + callback="https://nginx:8443/test/a/${CONFORMANCE_ALIAS}/callback" + php occ oidc:create "OIDC Conformance Basic 1" "${callback}" \ + --client_id "${OIDC_CLIENT_ID_1}" \ + --client_secret "${OIDC_CLIENT_SECRET_1}" \ + --type confidential \ + --flow code \ + --allowed_scopes "openid profile email offline_access" + php occ oidc:create "OIDC Conformance Basic 2" "${callback}" \ + --client_id "${OIDC_CLIENT_ID_2}" \ + --client_secret "${OIDC_CLIENT_SECRET_2}" \ + --type confidential \ + --flow code \ + --allowed_scopes "openid profile email offline_access" + + - name: Start Nextcloud web server + run: | + cd nextcloud + php -S 0.0.0.0:8080 -t . > ../nextcloud-server.log 2>&1 & + echo $! > ../nextcloud-server.pid + for i in {1..30}; do + if curl --fail --silent http://127.0.0.1:8080/index.php/apps/oidc/openid-configuration > /dev/null; then + exit 0 + fi + sleep 2 + done + cat ../nextcloud-server.log + exit 1 + + - name: Checkout OpenID conformance suite + run: git clone --depth 1 https://gitlab.com/openid/conformance-suite.git conformance-suite + + - name: Build and start OpenID conformance suite + run: | + sudo sh -c 'echo "127.0.0.1 nginx" >> /etc/hosts' + sudo sh -c 'echo "127.0.0.1 oidcc-provider" >> /etc/hosts' + sudo sh -c 'echo "127.0.0.1 chrome" >> /etc/hosts' + cd conformance-suite + mvn -B -DskipTests package + docker compose \ + -f docker-compose-localtest.yml \ + -f ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/docker-compose.github-actions.yml \ + -f ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/docker-compose.chrome.yml \ + up -d mongodb nginx server chrome + # Wait for Chrome/Selenium service to be ready + for i in {1..60}; do + if curl --fail --silent http://chrome:4444/status > /dev/null 2>&1; then + echo "Chrome/Selenium service is ready" + break + fi + sleep 2 + done + # Wait for API runner + for i in {1..60}; do + if curl --insecure --fail --silent https://nginx:8443/api/runner/available > /dev/null; then + exit 0 + fi + sleep 5 + done + docker compose \ + -f docker-compose-localtest.yml \ + -f ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/docker-compose.github-actions.yml \ + -f ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/docker-compose.chrome.yml \ + logs server nginx chrome + exit 1 + + - name: Install conformance runner dependencies + run: python3 -m pip install --user httpx pyparsing selenium + + - name: Write conformance plan config + run: | + mkdir -p conformance-results conformance-config + python3 nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/write-config.py \ + nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/oidc-basic-config.json \ + conformance-config/oidc-basic-config.json + python3 -m json.tool conformance-config/oidc-basic-config.json > /dev/null + + - name: Run OIDC basic conformance plan + env: + CONFORMANCE_DEV_MODE: 1 + CONFORMANCE_SERVER: https://nginx:8443/ + CONFORMANCE_SERVER_MTLS: https://nginx:8444/ + SELENIUM_REMOTE_URL: http://chrome:4444/wd/hub + CONFORMANCE_BROWSER_VISIT_TIMEOUT: 90 + run: | + cd conformance-suite + : > ../conformance-browser.log + PYTHONUNBUFFERED=1 python3 ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/browser-runner.py \ + >> ../conformance-browser.log 2>&1 & + browser_runner_pid=$! + tail -n +1 -f ../conformance-browser.log & + browser_log_tail_pid=$! + cleanup_browser_runner() { + kill "${browser_runner_pid}" 2>/dev/null || true + kill "${browser_log_tail_pid}" 2>/dev/null || true + wait "${browser_runner_pid}" 2>/dev/null || true + wait "${browser_log_tail_pid}" 2>/dev/null || true + } + trap cleanup_browser_runner EXIT + set +e + python3 scripts/run-test-plan.py \ + --export-dir ../conformance-results \ + --verbose \ + "oidcc-basic-certification-test-plan[server_metadata=discovery][client_registration=static_client]" \ + ../conformance-config/oidc-basic-config.json + test_plan_status=$? + set -e + cleanup_browser_runner + trap - EXIT + echo "::group::conformance-browser.log" + cat ../conformance-browser.log || true + echo "::endgroup::" + set +e + python3 ../nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/check-results.py \ + --results-dir ../conformance-results + blocking_status=$? + set -e + if [ "${blocking_status}" -ne 0 ]; then + if [ "${test_plan_status}" -ne 0 ]; then + exit "${test_plan_status}" + fi + exit "${blocking_status}" + fi + if [ "${test_plan_status}" -ne 0 ]; then + echo "Conformance suite exited with ${test_plan_status}, but exported results contain only non-blocking WARNING/REVIEW outcomes." + fi + + - name: Generate conformance report + if: always() + run: | + python3 nextcloud/apps/${{ env.APP_NAME }}/.github/conformance/generate-report.py \ + --results-dir conformance-results \ + --output CONFORMANCE.md + cat CONFORMANCE.md + + - name: Upload conformance results + if: always() + uses: actions/upload-artifact@v4 + with: + name: oidc-conformance-results + path: | + CONFORMANCE.md + conformance-results + conformance-browser.log + nextcloud-server.log