fix(realm): reconcile application RPs at runtime - #83
Conversation
|
Warning Review limit reached
Next review available in: 44 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough포터블 realm에서 Changes포터블 Realm 정책과 검증
런타임 RP 복구와 reconciliation 검증
온보딩과 권한 경계 문서
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant Keycloak
participant AccountUnification
participant DesiredState
participant NaruonRP
Operator->>Keycloak: 포터블 realm import
Keycloak->>AccountUnification: account-unification-svc 부트스트랩
DesiredState->>AccountUnification: 런타임 RP desired state 제공
AccountUnification->>NaruonRP: Naruon RP reconciliation 실행
NaruonRP-->>Keycloak: relying-party 클라이언트 생성 또는 동기화
Operator->>Keycloak: 통제된 로그인 검증
Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review @OpenCode review Please review exact head |
|
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
services/account_unification/tests/test_realm_policy.py (1)
87-97: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win알 수 없는 애플리케이션 클라이언트의 거부 경로를 테스트하십시오.
현재 테스트는
ecosystem-rp-template및naruon-web만 확인합니다.elif client_id != "account-unification-svc"경로는 검증하지 않습니다. 이 경로가 회귀하면 알려진 두 클라이언트의 전용 오류는 계속 발생하므로 테스트가 통과할 수 있습니다. 알 수 없는clientId와 일반 거부 오류를 추가로 확인하십시오.수정 예시
[ {"clientId": "ecosystem-rp-template"}, {"clientId": "naruon-web", "publicClient": True}, + {"clientId": "unmanaged-web", "publicClient": True}, ] ) errors = validator.validate(realm) assert any("runtime application client 'ecosystem-rp-template'" in error for error in errors) assert any("runtime application client 'naruon-web'" in error for error in errors) + assert any( + "portable realm may contain only the account-unification-svc" in error + for error in errors + )🤖 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 `@services/account_unification/tests/test_realm_policy.py` around lines 87 - 97, Extend the test around validator.validate(realm) to include an unknown clientId and assert that validation returns the general application-client rejection error for it. Keep the existing dedicated assertions for ecosystem-rp-template and naruon-web, ensuring the unknown client exercises the elif client_id != "account-unification-svc" branch.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@docs/doctoring/oidc-rp-claim-mapper-profile.md`:
- Around line 118-120: Update the portable-realm statement in the OIDC RP
claim-mapper profile to explicitly preserve the account-unification-svc
control-plane client while stating that only runtime application RP clients are
removed; retain the existing deterministic clean-realm and Keyverse
synchronization claims.
---
Nitpick comments:
In `@services/account_unification/tests/test_realm_policy.py`:
- Around line 87-97: Extend the test around validator.validate(realm) to include
an unknown clientId and assert that validation returns the general
application-client rejection error for it. Keep the existing dedicated
assertions for ecosystem-rp-template and naruon-web, ensuring the unknown client
exercises the elif client_id != "account-unification-svc" branch.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6093b08a-425f-4fe1-a41c-d648c97fe548
📒 Files selected for processing (13)
CHANGELOG.mdREADME.mddeploy/keycloak/README.mddeploy/keycloak/realm-cwl.jsondeploy/templates/README.mddocs/OPERABILITY.mddocs/adr/0008-keyverse-rp-authorization-boundary.mddocs/doctoring/oidc-rp-claim-mapper-profile.mddocs/passwordless-policy.mddocs/rp-onboarding.mdscripts/validate_realm.pyservices/account_unification/tests/test_realm_policy.pyservices/account_unification/tests/test_relying_party_mapper_reconciliation.py
💤 Files with no reviewable changes (1)
- deploy/keycloak/realm-cwl.json
|
Addressed the exact-head review on
Focused realm and relying-party mapper verification: 46 passed; Ruff clean; diff check clean. Please re-review this current head only. @coderabbitai review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Checkout =='
git rev-parse HEAD
git status --short
git show -s --format='%H%n%P%n%s' HEAD
echo '== PR metadata =='
gh pr view 83 --repo ContextualWisdomLab/keyverse --json baseRefOid,headRefOid,files \
--jq '{baseRefOid, headRefOid, files: [.files[].path]}'
echo '== Diff summary from PR base =='
base="$(gh pr view 83 --repo ContextualWisdomLab/keyverse --json baseRefOid --jq .baseRefOid)"
git diff --stat "$base...HEAD"
git diff --name-only "$base...HEAD"
echo '== Validator outline and relevant content =='
ast-grep outline scripts/validate_realm.py --items all || true
rg -n -C 5 'account-unification-svc|application|client|forbidden|reject|allowed' scripts/validate_realm.py
echo '== Realm client and scope declarations =='
rg -n -C 4 '"clientId"|"clientScopes"|"basic"|"profile"|"email"|naruon|ecosystem' deploy/keycloak/realm-cwl.json
echo '== Changed test outlines and relevant assertions =='
ast-grep outline services/account_unification/tests/test_realm_policy.py --items all || true
ast-grep outline services/account_unification/tests/test_relying_party_mapper_reconciliation.py --items all || true
rg -n -C 6 'portable|unmanaged|application.client|application client|naruon|IN_SYNC|realm-cwl' \
services/account_unification/tests/test_realm_policy.py \
services/account_unification/tests/test_relying_party_mapper_reconciliation.py
echo '== Changed documentation references =='
git diff --unified=30 "$base...HEAD" -- \
CHANGELOG.md README.md deploy/keycloak/README.md deploy/templates/README.md \
docs/OPERABILITY.md docs/adr/0008-keyverse-rp-authorization-boundary.md \
docs/doctoring/oidc-rp-claim-mapper-profile.md docs/passwordless-policy.md \
docs/rp-onboarding.mdLength of output: 72925 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Full validator policy section =='
sed -n '60,175p' scripts/validate_realm.py
echo '== Exact updated tests =='
sed -n '1,115p' services/account_unification/tests/test_realm_policy.py
sed -n '1,95p' services/account_unification/tests/test_relying_party_mapper_reconciliation.py
echo '== Test fixture definitions and in-memory client initialization =='
rg -n -C 8 --type py \
'def api\b|`@pytest`\.fixture|class .*Api|relying_party_clients|InMemory.*Api|Fake.*Api' \
services/account_unification/tests services/account_unification/app
echo '== Reconciliation entry points =='
rg -n -C 8 --type py \
'def put_registration\b|def reconcile_all\b|def get_registration\b|class RelyingPartyService' \
services/account_unification
echo '== Rollback and startup instructions =='
rg -n -i -C 5 \
'clean realm|realm import|bootstrap|reconcile|rollback|roll back|remote-first|route traffic' \
docs deploy/keycloak README.md CHANGELOG.md
echo '== Potential stale portable-realm ownership statements =='
rg -n -i -C 3 \
'portable realm.*(naruon|application|RP|client)|realm.*(naruon-web|ecosystem-rp-template)|embedded.*(RP|client|naruon)|concrete Naruon' \
--glob '!CHANGELOG.md' \
.
echo '== Portable realm JSON structural checks without executing repository code =='
python3 - <<'PY'
import json
from pathlib import Path
realm = json.loads(Path("deploy/keycloak/realm-cwl.json").read_text(encoding="utf-8"))
clients = realm.get("clients")
scopes = realm.get("clientScopes")
print("clients_type:", type(clients).__name__)
print("client_ids:", [item.get("clientId") if isinstance(item, dict) else type(item).__name__ for item in clients])
print("default_scopes:", realm.get("defaultDefaultClientScopes"))
print("declared_scopes:", [item.get("name") if isinstance(item, dict) else type(item).__name__ for item in scopes])
PYLength of output: 228560
I found one documentation issue.
The validator regression now covers named legacy clients and an arbitrary unmanaged client. The clean-realm regression starts with no runtime application client and recreates
|
1 participant
Buyer-visible defect
The portable Keycloak realm still embedded
ecosystem-rp-templateandnaruon-webafter #72 made application relying parties runtime desired state. A clean import could therefore recreate application clients outside Keyverse reconciliation and split ownership between realm source and durable desired state.Closes #71
Tracks #2
Test-first repair
The focused RED contract failed because both application clients remained in
realm-cwl.jsonand the validator did not reject them. This change now:account-unification-svcin the portable realm;basic,profile, andemailscopes;oidc-rp-naruon.jsonprofile recreates one exact in-sync client from an empty application-client state through Keyverse desired-state reconciliation;Exact-head evidence
Current head:
30eeadecad5c453c032a08fe503591712817a765on protected base214afb444e64038906a03556e411583b2f58ee71.git diff --check: passed.The first full local run inherited an ambient SOCKS proxy and failed 13 HTTPX transport tests because
socksiois intentionally not a project dependency. Re-running the same full boundary in the CI-equivalent proxy-free environment passed; no dependency or product gate was weakened. Docker Compose is not installed in this execution environment, so hosted CI remains authoritative for Compose/template validation.Safety and completion boundary
No secret, provider credential, password authenticator, dependency, database schema, admin bypass, release, or review credential changes. This remains Draft until exact-head CI, CodeQL, Semgrep, Security Scan, independent review, zero unresolved findings, and protected merge requirements are satisfied.
Summary by CodeRabbit
변경 사항
문서
테스트