docs: establish identity product and architecture baseline - #75
docs: establish identity product and architecture baseline#75seonghobae wants to merge 29 commits into
Conversation
📝 WalkthroughWalkthroughKeyverse의 제품·기술 요구사항, 데이터 모델, 보안·운영 기준, ADR, UML 및 문서 계약 테스트를 추가했다. 문서는 protected main, active PR, Keycloak 소유 데이터와 desired-state reconciliation 경계를 정의한다. Changes제품 계약과 거버넌스
영속성·신원 수명주기
보안·운영·검증
Estimated code review effort: 3 (Moderate) | ~20 minutes 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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/adr/0006-user-operation-lock.md`:
- Around line 1-6: Include the SCIM patch_user flow in the shared cross-process
user-operation lock boundary. Protect its get_user and deactivate_user
read-modify-write sequence with user_operation_locks.hold(user_id), matching
replace_user; otherwise explicitly exclude PATCH through a separate contract and
update the ADR and merge documentation accordingly.
In `@docs/adr/README.md`:
- Line 19: The documentation contract uses the undefined term “doctoring”;
replace it with the established documentation, doctrine, or actual runbook
terminology and path. Update docs/adr/README.md lines 19-19 and
docs/TEST_STRATEGY.md lines 88-88 consistently, including the
“doctoring/operations links” reference.
In `@docs/ERD.md`:
- Around line 105-111: Update the EXTERNAL_IDENTITY_LINK definition to add a
composite unique constraint on (federation_source_id, external_subject_hash),
preventing one external subject within a federation source from linking to
multiple Keycloak users. Document that federation_source_id defines the
identity-provider scope for this uniqueness rule.
- Around line 31-37: Update the ERD definitions for IDP_CONFIG_ENTRY and the
related entities at the referenced sections so config_key, federation_alias,
directory_alias, client_id, and keycloak_user_uuid are documented as
tenant-scoped composite unique keys with tenant_deployment_id, while retaining
global uniqueness only for UUID identifiers.
- Around line 51-58: Update the three receipt entities, including
FEDERATION_APPLY_RECEIPT, to store the desired-state version or hash alongside
observed_state_hash, outcome, and timestamp. Define documentation rules for
identifying the latest desired version and handling duplicate receipts,
consistent with the versioned contract in THREAT_MODEL.md.
In `@docs/OPERABILITY.md`:
- Line 59: Update the wording around the PR `#72` claim mapper acceptance
statement in OPERABILITY.md to consistently use “Naruon” for the
product/platform and “naruon-web” only for the RP client ID. Clarify whether
authorization readiness refers to the Naruon product or the naruon-web client so
operators know which authentication flow and scope to validate.
In `@docs/TRD.md`:
- Around line 45-49: Synchronize the native loopback redirect exception between
docs/TRD.md and docs/PRD.md: either remove “except separately reviewed native
loopback profile” from the RP clients contract, or, if native loopback is
supported, document its exact exception conditions in PRD-FR-005, the threat
model, test strategy, and traceability documentation.
In `@docs/UML.md`:
- Around line 24-28: Update the UML storage relationships so the Keycloak
PostgreSQL node is explicitly Keycloak-owned and the Keyverse-owned store is
represented as a separate node. Remove the direct ADMIN --> PG relationship,
retain Keycloak’s connection to its own database, and route ADMIN through the
Keycloak Admin API to the Keycloak engine while preserving the existing
deployment relationships.
In `@tests/test_documentation_contract.py`:
- Around line 68-81: Update test_adr_index_contains_governing_identity_decisions
to verify each listed ADR both appears in docs/adr/README.md and exists as a
file at ROOT / "docs" / "adr" / adr, using is_file().
- Around line 49-57: Strengthen test_active_pr_features_are_not_promoted_to_main
so each PR’s identifier is directly associated with active-PR in both
docs/PRD.md and docs/TRACEABILITY.md. Replace the current independent substring
assertions and unrelated RP `#72` exclusion with per-PR checks that validate PR
`#72` and PR `#74` each retain active-PR status in their corresponding documentation
entries.
🪄 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: 16a1fb5b-db38-42b8-b622-4be30efcd86c
📒 Files selected for processing (18)
DOCUMENTATION.mddocs/ERD.mddocs/OPERABILITY.mddocs/PRD.mddocs/TEST_STRATEGY.mddocs/THREAT_MODEL.mddocs/TRACEABILITY.mddocs/TRD.mddocs/UML.mddocs/adr/0001-keycloak-hub.mddocs/adr/0002-passwordless-local-accounts.mddocs/adr/0003-identity-matching.mddocs/adr/0004-desired-state-reconciliation.mddocs/adr/0005-secret-ownership.mddocs/adr/0006-user-operation-lock.mddocs/adr/0007-automation-authority.mddocs/adr/README.mdtests/test_documentation_contract.py
|
@coderabbitai review Please re-review current exact head |
|
I will verify the source and documentation contract for:
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/ERD.md (1)
187-189: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winERD의
USER_OPERATION_LOCK_STATE와 구현된 잠금 스키마의 차이를 매핑하세요.
USER_OPERATION_LOCK_STATE가 물리 스키마라면lock_name TEXT PRIMARY KEY와requested_user_ids TEXT가 있는 현재SqliteUserOperationLocks._SCHEMA와 일치하지 않습니다. 논리 모델이라면 SQLite sidecar의 전역 mutex를 ERD의 per-user FK/lease 필드에 어떻게 매핑하는지 문서에 명시하세요. 어용지 구현 없이lease_expires_at동시성 테스트를 검증할 수 없습니다.🤖 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 `@docs/ERD.md` around lines 187 - 189, Update the ERD documentation to explicitly map USER_OPERATION_LOCK_STATE to the implementation in SqliteUserOperationLocks._SCHEMA: document whether it is a logical model or physical schema, reconcile lock_name TEXT PRIMARY KEY and requested_user_ids TEXT with the ERD fields, and describe how the SQLite sidecar’s global mutex maps to per-user foreign-key and lease fields, including the lease_expires_at concurrency-test implications.
🤖 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/ERD.md`:
- Line 154: Update the physical migration constraints for EXTERNAL_IDENTITY_LINK
and ACCOUNT_MERGE_AUDIT so each child record references parent records within
the same TENANT_DEPLOYMENT, using composite foreign keys or an equivalent
database constraint that includes tenant_deployment_id. Ensure cross-tenant
federation-source, Keycloak-user, and merge-audit associations are rejected
regardless of documentation labels.
---
Outside diff comments:
In `@docs/ERD.md`:
- Around line 187-189: Update the ERD documentation to explicitly map
USER_OPERATION_LOCK_STATE to the implementation in
SqliteUserOperationLocks._SCHEMA: document whether it is a logical model or
physical schema, reconcile lock_name TEXT PRIMARY KEY and requested_user_ids
TEXT with the ERD fields, and describe how the SQLite sidecar’s global mutex
maps to per-user foreign-key and lease fields, including the lease_expires_at
concurrency-test implications.
🪄 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: 93be661b-de48-455f-941e-b30b02630e44
📒 Files selected for processing (11)
docs/ERD.mddocs/OPERABILITY.mddocs/PRD.mddocs/TEST_STRATEGY.mddocs/TRACEABILITY.mddocs/TRD.mddocs/UML.mddocs/adr/0006-user-operation-lock.mddocs/adr/README.mddocs/merge-unification-flow.mdtests/test_documentation_contract.py
🚧 Files skipped from review as they are similar to previous changes (8)
- docs/PRD.md
- docs/UML.md
- docs/TRD.md
- tests/test_documentation_contract.py
- docs/TEST_STRATEGY.md
- docs/adr/README.md
- docs/OPERABILITY.md
- docs/TRACEABILITY.md
|
|
||
| `federation_source_id` defines the identity-provider scope for the external-subject uniqueness rule. Within one federation source, one normalized/hashed external subject may link to at most one Keycloak user reference. This prevents one issuer/provider subject from being attached to multiple users while still allowing unrelated providers to use the same subject string. | ||
|
|
||
| Physical migrations must enforce these constraints in the owning Keyverse store. Documentation labels such as `client_id`, `federation_alias`, or Keycloak UUID never authorize cross-tenant lookup by themselves. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== docs/ERD.md relevant sections =="
sed -n '120,175p' docs/ERD.md | cat -n -v
echo
echo "== Search related table definitions/constraints =="
rg -n "EXTERNAL_IDENTITY_LINK|ACCOUNT_MERGE_AUDIT|tenant_deployment_id|client_id|federation_alias|keycloak_user_reference_id|federation_source_id" docs -S || true
echo
echo "== Search tenant references for related entities =="
rg -n "tenant_deployment_id|tenant_id|MULTI_TENANT|tenant" . -S --glob '!node_modules' --glob '!dist' --glob '!build' | head -200Repository: ContextualWisdomLab/keyverse
Length of output: 14893
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== docs/ERD.md table definitions =="
sed -n '1,120p' docs/ERD.md | cat -n -v
echo
echo "== Full references to ACCOUNT_MERGE_AUDIT / EXTERNAL_IDENTITY_LINK =="
rg -n "ACCOUNT_MERGE_AUDIT|EXTERNAL_IDENTITY_LINK|survivor_user_reference_id|duplicate_user_reference_id|keycloak_user_reference_id|federation_source_id|tenant_deployment_id" . \
-S --glob '!node_modules' --glob '!dist' --glob '!build' \
| sed -n '1,240p'
echo
echo "== Read-only check: ERD FK constraints and tenant columns for target tables =="
python3 - <<'PY'
from pathlib import Path
p=Path('docs/ERD.md')
text=p.read_text()
targets = ['FEDERATION_SOURCE','KEYCLOAK_USER_REFERENCE','EXTERNAL_IDENTITY_LINK','ACCOUNT_MERGE_AUDIT']
in_table=False
current=None
for line in text.splitlines():
if re := any(f' {{{t}' in line for t in targets):
m = next(t for t in targets if f' {{{t}' in line)
current=m
in_table=True
print(f'\n[{current}]')
continue
if in_table:
if line.startswith(' }'):
in_table=False
current=None
continue
print(line)
PYRepository: ContextualWisdomLab/keyverse
Length of output: 7406
테넌트 간 관계를 데이터베이스 제약으로 차단하세요.
EXTERNAL_IDENTITY_LINK와 ACCOUNT_MERGE_AUDIT은 tenant_deployment_id 없이 FK만으로 부모 레코드를 참조합니다. 두 부모가 같은 TENANT_DEPLOYMENT에 속하는지 검증하는 복합 FK 또는 동등한 제약을 추가해야 합니다. 이 제약을 추가하지 않으면 테넌트 A의 federation source가 테넌트 B의 Keycloak user reference/merge audit과 연결될 수 있어 Line 154의 테넌트 격리를 구현 단계에서 보장할 수 없습니다.
🤖 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 `@docs/ERD.md` at line 154, Update the physical migration constraints for
EXTERNAL_IDENTITY_LINK and ACCOUNT_MERGE_AUDIT so each child record references
parent records within the same TENANT_DEPLOYMENT, using composite foreign keys
or an equivalent database constraint that includes tenant_deployment_id. Ensure
cross-tenant federation-source, Keycloak-user, and merge-audit associations are
rejected regardless of documentation labels.
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head4d2841071e9a8136298bb7198229d47ff406284d. -
Head SHA:
4d2841071e9a8136298bb7198229d47ff406284d -
Workflow run: 31315176332
-
Workflow attempt: 1
Coverage evidence
Coverage evidence job did not run or did not publish coverage evidence.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file: DOCUMENTATION.md"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file: DOCUMENTATION.md"]
R1 --> V1["required checks"]
Evidence --> S2["Docs (17 files)"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs (17 files)"]
R2 --> V2["docs review"]
Evidence --> S3["Test: test_documentation_contract.py"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test: test_documentation_contract.py"]
R3 --> V3["targeted test run"]
OpenCode Review Overview
Pull request overviewOpenCode cannot approve yet because required coverage evidence did not pass. Review outcome1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
Coverage evidenceCoverage evidence job did not run or did not publish coverage evidence. Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file: DOCUMENTATION.md"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file: DOCUMENTATION.md"]
R1 --> V1["required checks"]
Evidence --> S2["Docs (17 files)"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs (17 files)"]
R2 --> V2["docs review"]
Evidence --> S3["Test: test_documentation_contract.py"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test: test_documentation_contract.py"]
R3 --> V3["targeted test run"]
|
1 participant
Purpose
Make Keyverse's cross-cutting product, technical, data, security, operating, and decision memory canonical and machine-checkable instead of reconstructing it from README,
ARCHITECTURE.md, feature specs, doctoring pages, operations pages, and PR bodies.The branch was created from and revalidated against exact protected
mainc8968ec1e68fab16d0ad8216fb5c8fd0b385e95f.Added
DOCUMENTATION.mddocumentation map and maturity vocabularydocs/PRD.mddocs/TRD.mddocs/UML.mddocs/ERD.mdfor Keyverse-owned state while preserving Keycloak schema ownershipdocs/THREAT_MODEL.mddocs/TEST_STRATEGY.mddocs/OPERABILITY.mddocs/TRACEABILITY.mdtests/test_documentation_contract.pyExisting records preserved
ARCHITECTURE.md,docs/topology.md, federation/RP onboarding, merge/unification flow,docs/operations/,docs/doctoring/, anddocs/papers/remain authoritative for their existing slices. This PR consolidates discoverability and cross-cutting invariants rather than duplicating those documents.Truth boundary
The docs describe protected-main passwordless Keycloak hub, account unification, SCIM, SAML/OIDC and LDAP desired-state boundaries, secret-free RP reconciliation, operation locking, deployment-controller ownership, and current quality gates.
PR #72's closed OIDC RP mapper profile and PR #74's hourly GitHub API remediation remain explicitly active-PR, not protected-main/released behavior. Controlled external login/bind/downstream authorization acceptance is kept separate from side-effect-free preflight readiness.
Verification contract
The documentation contract runs in the existing pytest suite and prevents disappearance of canonical records, false ownership of Keycloak internal persistence, missing ADRs, or promotion of active PR work into mainline claims. Exact current-head CI, 100% production statement/branch/docstring gates, package/realm/deployment validation, CodeQL, Semgrep, Security Scan, independent review, and branch protection must pass before merge.
Summary by CodeRabbit
문서화
테스트