Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions docker/keycloak/desired-clients.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,18 @@
"jsonType.label": "String"
}
},
{
"name": "principal_id",
"protocolMapper": "oidc-usermodel-attribute-mapper",
"config": {
"user.attribute": "principal_id",
"claim.name": "principal_id",
"id.token.claim": "true",
"access.token.claim": "true",
"userinfo.token.claim": "true",
"jsonType.label": "String"
}
},
{
"name": "onex-api-audience",
"protocolMapper": "oidc-audience-mapper",
Expand Down
21 changes: 21 additions & 0 deletions scripts/tests/test_keycloak_desired_clients_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,24 @@ def test_omniweb_allows_the_managed_staging_callback() -> None:

assert "https://dev.app.omninode.ai/*" in omniweb["redirectUris"]
assert "https://dev.app.omninode.ai" in omniweb["webOrigins"]


def test_omniweb_user_identity_claim_contract() -> None:
config = json.loads(_CONFIG_PATH.read_text())
omniweb = _client(config, "omniweb")
mappers = {mapper["name"]: mapper for mapper in omniweb["protocolMappers"]}

principal = mappers["principal_id"]
assert principal["protocolMapper"] == "oidc-usermodel-attribute-mapper"
assert principal["config"]["user.attribute"] == "principal_id"
assert principal["config"]["claim.name"] == "principal_id"
assert principal["config"]["id.token.claim"] == "true"
assert principal["config"]["access.token.claim"] == "true"
assert principal["config"]["userinfo.token.claim"] == "true"
Comment on lines +36 to +42

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Assert the configured JSON type.

docker/keycloak/desired-clients.json Lines 54-63 set config["jsonType.label"] to "String", but this contract does not check it. Add the assertion so a non-string principal_id claim cannot pass the test.

Proposed assertion
     assert principal["config"]["userinfo.token.claim"] == "true"
+    assert principal["config"]["jsonType.label"] == "String"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
principal = mappers["principal_id"]
assert principal["protocolMapper"] == "oidc-usermodel-attribute-mapper"
assert principal["config"]["user.attribute"] == "principal_id"
assert principal["config"]["claim.name"] == "principal_id"
assert principal["config"]["id.token.claim"] == "true"
assert principal["config"]["access.token.claim"] == "true"
assert principal["config"]["userinfo.token.claim"] == "true"
principal = mappers["principal_id"]
assert principal["protocolMapper"] == "oidc-usermodel-attribute-mapper"
assert principal["config"]["user.attribute"] == "principal_id"
assert principal["config"]["claim.name"] == "principal_id"
assert principal["config"]["id.token.claim"] == "true"
assert principal["config"]["access.token.claim"] == "true"
assert principal["config"]["userinfo.token.claim"] == "true"
assert principal["config"]["jsonType.label"] == "String"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/tests/test_keycloak_desired_clients_contract.py` around lines 36 -
42, Extend the principal mapper assertions in the contract test to validate that
config["jsonType.label"] is set to "String", alongside the existing principal_id
mapper checks. Use the existing principal config object and preserve all current
assertions.


assert "gateway-attach-audience" not in mappers
assert all(
mapper.get("config", {}).get("included.custom.audience")
!= "gateway-attach"
for mapper in mappers.values()
)
Comment on lines +44 to +49

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

rg -n -C 8 \
  '"gateway-attach"|gateway-attach-audience|included.custom.audience' \
  docker/keycloak/desired-clients.json \
  scripts/tests/test_keycloak_desired_clients_contract.py

Repository: OmniNode-ai/omnibase_infra

Length of output: 1630


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf '%s\n' '--- test file ---'
cat -n scripts/tests/test_keycloak_desired_clients_contract.py

printf '%s\n' '--- relevant Keycloak declarations ---'
rg -n -C 12 \
  'gateway-attach|omniweb|clientScopes|clientScope|protocolMappers|audience' \
  docker scripts \
  -g '*.json' -g '*.py' -g '*.sh'

printf '%s\n' '--- repository-wide references ---'
rg -n -C 5 'gateway-attach|gateway-attach-audience' .

Repository: OmniNode-ai/omnibase_infra

Length of output: 50382


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

python3 - <<'PY'
import json
from pathlib import Path

path = Path("docker/keycloak/desired-clients.json")
config = json.loads(path.read_text())

print("top-level keys:", sorted(config))
print("client scopes:")
for scope in config.get("clientScopes", []):
    print(json.dumps({
        "name": scope.get("name"),
        "protocol": scope.get("protocol"),
        "mappers": scope.get("protocolMappers", []),
    }, sort_keys=True))

print("clients:")
for client in config.get("clients", []):
    if client.get("clientId") == "omniweb":
        print(json.dumps({
            "clientId": client.get("clientId"),
            "defaultClientScopes": client.get("defaultClientScopes", []),
            "optionalClientScopes": client.get("optionalClientScopes", []),
            "protocolMappers": client.get("protocolMappers", []),
        }, indent=2, sort_keys=True))
PY

printf '%s\n' '--- exact gateway references in configuration and tests ---'
rg -n 'gateway-attach|gateway-attach-audience' \
  docker/keycloak/desired-clients.json \
  scripts/tests/test_keycloak_desired_clients_contract.py \
  scripts/tests/test_seed_keycloak_clients.py || true

printf '%s\n' '--- focused diff ---'
git diff --stat -- scripts/tests/test_keycloak_desired_clients_contract.py docker/keycloak/desired-clients.json
git diff -- scripts/tests/test_keycloak_desired_clients_contract.py docker/keycloak/desired-clients.json

Repository: OmniNode-ai/omnibase_infra

Length of output: 2032


Add a positive gateway-attach audience contract.

The desired configuration defines no gateway-attach mapper or client scope, so this test can pass while the short-lived user token lacks the required audience. Add the positive source and assert it here. Retain these checks as duplicate-prevention guards.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/tests/test_keycloak_desired_clients_contract.py` around lines 44 -
49, Add a positive contract in the relevant Keycloak desired-configuration test
setup for the short-lived user token to include the gateway-attach audience,
then assert that source is present in the generated configuration. Keep the
existing gateway-attach mapper absence and included.custom.audience checks
unchanged as duplicate-prevention guards.

Loading