diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 9d32966c..bb6585c4 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -3,9 +3,23 @@ name: Pull Request Checks on: pull_request: paths: + - '.github/workflows/pr-checks.yml' - 'charts/**' + - 'scripts/test_keycloak_realm_template.py' jobs: + test-keycloak-realm-template: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: "3.x" + - name: Install test dependencies + run: python3 -m pip install pytest + - name: Test Keycloak realm template invariants + run: python3 -m pytest scripts/test_keycloak_realm_template.py + validate-chart-versions: uses: ./.github/workflows/validate-chart-versions.yml with: diff --git a/charts/openhands/Chart.yaml b/charts/openhands/Chart.yaml index e0c2d909..c2b8a94f 100644 --- a/charts/openhands/Chart.yaml +++ b/charts/openhands/Chart.yaml @@ -2,7 +2,7 @@ apiVersion: v2 description: OpenHands is an AI-driven autonomous software engineer name: openhands appVersion: cloud-1.34.0 -version: 0.7.35 +version: 0.7.36 maintainers: - name: rbren - name: xingyao diff --git a/charts/openhands/files/allhands-realm-github-provider.json.tmpl b/charts/openhands/files/allhands-realm-github-provider.json.tmpl index d5371082..b5553945 100644 --- a/charts/openhands/files/allhands-realm-github-provider.json.tmpl +++ b/charts/openhands/files/allhands-realm-github-provider.json.tmpl @@ -1846,6 +1846,7 @@ "useJwksUrl": "true", "sendClientIdOnLogout": "false", "pkceEnabled": "true", + "pkceMethod": "S256", "authorizationUrl": "https://login.microsoftonline.com/${AZURE_DEVOPS_TENANT_ID}/oauth2/v2.0/authorize", "disableUserInfo": "true", "sendIdTokenOnLogout": "true", diff --git a/charts/openhands/templates/keycloak-config-script.yaml b/charts/openhands/templates/keycloak-config-script.yaml index 3ec5d42d..13a834bc 100644 --- a/charts/openhands/templates/keycloak-config-script.yaml +++ b/charts/openhands/templates/keycloak-config-script.yaml @@ -11,7 +11,7 @@ data: keycloak_api_call() { COMMAND=$1 export RESPONSE=$(eval $COMMAND) - ERROR=$(echo "$RESPONSE" | jq -r 'try if type == "array" then .[0].error else .error end') + ERROR=$(echo "$RESPONSE" | jq -r 'try if type == "array" then (.[0].error // .[0].errorMessage) else (.error // .errorMessage) end') if [ -n "$ERROR" ] && [ "null" != "$ERROR" ]; then echo "Error from Keycloak: $RESPONSE" exit 1 diff --git a/scripts/test_keycloak_realm_template.py b/scripts/test_keycloak_realm_template.py new file mode 100644 index 00000000..8f42c0bd --- /dev/null +++ b/scripts/test_keycloak_realm_template.py @@ -0,0 +1,114 @@ +"""Tests for Keycloak realm chart invariants.""" + +from __future__ import annotations + +import json +import re +from pathlib import Path + +import pytest + + +REPO_ROOT = Path(__file__).resolve().parents[1] +REALM_TEMPLATE = ( + REPO_ROOT + / "charts" + / "openhands" + / "files" + / "allhands-realm-github-provider.json.tmpl" +) +KEYCLOAK_CONFIG_SCRIPT = ( + REPO_ROOT / "charts" / "openhands" / "templates" / "keycloak-config-script.yaml" +) + + +def pkce_enabled_providers_missing_method(realm: dict) -> list[str]: + missing_pkce_method = [] + for provider in realm.get("identityProviders", []): + config = provider.get("config") or {} + if config.get("pkceEnabled") == "true" and not config.get("pkceMethod"): + missing_pkce_method.append(provider.get("alias", "")) + return missing_pkce_method + + +def keycloak_api_call_body(script_template: str) -> str: + match = re.search( + r"(?ms)^(?P[ \t]*)keycloak_api_call\(\) \{\n" + r"(?P.*?)^(?P=indent)\}", + script_template, + ) + assert match, "Could not find keycloak_api_call() in keycloak-config-script.yaml" + return match.group("body") + + +def assert_pkce_enabled_providers_set_method(realm: dict) -> None: + missing_pkce_method = pkce_enabled_providers_missing_method(realm) + assert not missing_pkce_method, ( + "Identity providers with pkceEnabled=true must set pkceMethod: " + + ", ".join(missing_pkce_method) + ) + + +def assert_keycloak_api_call_detects_error_message(script_template: str) -> None: + body = keycloak_api_call_body(script_template) + assert "errorMessage" in body, ( + "keycloak_api_call() must treat Keycloak errorMessage responses as errors" + ) + + +def test_realm_template_is_valid_json() -> None: + json.loads(REALM_TEMPLATE.read_text(encoding="utf-8")) + + +def test_pkce_enabled_identity_providers_set_pkce_method() -> None: + realm = json.loads(REALM_TEMPLATE.read_text(encoding="utf-8")) + assert_pkce_enabled_providers_set_method(realm) + + +def test_pkce_guard_catches_missing_method() -> None: + realm = { + "identityProviders": [ + { + "alias": "azure_devops", + "config": {"pkceEnabled": "true"}, + }, + { + "alias": "github", + "config": {"pkceEnabled": "false"}, + }, + ], + } + + with pytest.raises(AssertionError, match="azure_devops"): + assert_pkce_enabled_providers_set_method(realm) + + +def test_keycloak_api_call_checks_error_message_responses() -> None: + script_template = KEYCLOAK_CONFIG_SCRIPT.read_text(encoding="utf-8") + assert_keycloak_api_call_detects_error_message(script_template) + + +def test_keycloak_api_call_extraction_is_not_tied_to_yaml_indent() -> None: + script_template = """\ + keycloak_api_call() { + ERROR=$(echo "$RESPONSE" | jq -r '.errorMessage') + } +""" + + assert "errorMessage" in keycloak_api_call_body(script_template) + + +def test_keycloak_error_guard_catches_missing_error_message() -> None: + script_template = """\ + keycloak_api_call() { + COMMAND=$1 + export RESPONSE=$(eval $COMMAND) + ERROR=$(echo "$RESPONSE" | jq -r '.error') + if [ -n "$ERROR" ] && [ "null" != "$ERROR" ]; then + exit 1 + fi + } +""" + + with pytest.raises(AssertionError, match="errorMessage"): + assert_keycloak_api_call_detects_error_message(script_template)