feat(auth): cut over project identity reads - #214
Conversation
|
Warning Review limit reached
Next review available in: 47 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis change activates project identity and actor authorization-context reads, adds grant-based authorization and response projections, introduces a self-context endpoint, updates project routing and repositories, and expands tests, API contracts, documentation, and AUTH-11B review evidence. ChangesProject identity and actor context authorization
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthRoutes
participant AuthorizationService
participant AdminAuthorizationRepository
participant ProjectService
Client->>AuthRoutes: Request project identity or actor context
AuthRoutes->>AuthorizationService: Authorize project-scoped action
AuthorizationService->>AdminAuthorizationRepository: Evaluate current grants
AuthRoutes->>ProjectService: Resolve project and response projection
ProjectService-->>AuthRoutes: Return full or contributor identity
AuthRoutes-->>Client: Return authorized response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/app/api/deps/authorization.py (1)
126-139: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winLifecycle denials on
/me/authorization-contextare now concealed as a project 404.For
ACTOR_AUTHORIZATION_CONTEXT_READ,_authorization_context_denialreturnsactor_suspended/actor_deactivated/identity_link_revokedbefore any project fact is consulted, but this branch rewrites all of them intoproject_authorization_resource_not_found. A suspended caller therefore cannot distinguish their own lifecycle state from a badproject_id. Concealment is only needed for the project-existence and grant-scope outcomes; consider letting lifecycle codes through for this self-owned action.🤖 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 `@backend/app/api/deps/authorization.py` around lines 126 - 139, Update the concealment logic in the action handling around concealed_project_reads so ACTOR_AUTHORIZATION_CONTEXT_READ preserves lifecycle denial codes such as actor_suspended, actor_deactivated, and identity_link_revoked. Only rewrite project-existence and grant-scope denials to project_authorization_resource_not_found, while keeping concealment unchanged for the other project actions.
🧹 Nitpick comments (2)
backend/app/modules/authorization/kernel.py (1)
474-498: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDenial-ordering differs between the two new paths; confirm that is intentional.
_project_read_denialevaluates type guard → availability → revalidation → lifecycle, while_authorization_context_denialevaluates lifecycle → availability → guards. Both actions are concealed to 404 inauthorization_http_error, so no information leaks today, but the recordeddenial_codeevidence for an identical suspended-actor request differs by action. Worth aligning if the evidence stream is compared across actions.🤖 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 `@backend/app/modules/authorization/kernel.py` around lines 474 - 498, The PROJECT_READ and ACTOR_AUTHORIZATION_CONTEXT_READ paths use different denial evaluation ordering, producing inconsistent denial_code evidence for equivalent suspended-actor requests. Align the ordering between _project_read_denial and _authorization_context_denial, while preserving the existing 404 concealment behavior and each method’s established guards, availability, revalidation, and lifecycle checks.backend/app/modules/projects/router.py (1)
94-133: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winUnion
response_modelrelies on member-ordering to avoid over-disclosure.
ProjectResponse | ContributorProjectResponseis resolved left-to-right; contributor payloads only fall through to the minimal shape becauseProjectResponsefails on the missingslug/description/timestamps. The projection itself is already decided byproject_identity_response, so the union is only a serialization contract — but ifProjectResponseever gains defaults for those fields, a contributor response would silently validate against the admin shape. Considerresponse_model=Nonewith the return annotation carrying the contract, or keeping the regression test inbackend/tests/test_projects.py(Line 991) extended to the route level.🤖 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 `@backend/app/modules/projects/router.py` around lines 94 - 133, The get_project route’s union response_model depends on ProjectResponse field validation failure to prevent contributor data from matching the full admin schema. Remove the order-sensitive response_model serialization contract, preferably by setting response_model=None while retaining the return annotation, and preserve the projection selected by ProjectService.project_identity_response; extend the existing route-level regression coverage in test_projects.py to verify contributor responses remain minimal.
🤖 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
@.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-11B-pr-trust-bundle.md:
- Around line 92-94: Update the “External review” status text to remove the
obsolete condition that checks are pending until the PR opens, and state that
GitHub Actions and the remaining external review are pending completion.
In `@backend/app/api/routes/auth.py`:
- Around line 96-120: Update read_current_actor_authorization_context to mirror
the rollback and error-mapping pattern used by read_current_actor_profile and
update_current_actor_profile: wrap the project lookup, service read, and commit
in the same handling structure, roll back the session on failure, and map
ActorRegistryError and SQLAlchemyError to the established structured responses.
In `@backend/app/modules/authorization/kernel.py`:
- Around line 526-668: Remove the unnecessary row locking from the read-only
grant lookups in _project_read_denial and _authorization_context_denial by using
the non-locking option for find_effective_grant and
find_active_project_role_any. Preserve the existing authorization decisions and
lookup scope while ensuring these GET authorization paths do not serialize
readers.
In `@backend/app/modules/authorization/repository.py`:
- Around line 198-218: Update effective_admin_roles_for_project to apply the
same eligibility predicate as find_effective_grant and
has_effective_permission_any_scope: join ActorProfile and require a human actor
with active status and an active identity link. Preserve the existing
active-grant, project/system-scope, distinct, and ordering behavior so projected
roles match enforcement decisions.
In `@docs/operations_project_operating_manual.md`:
- Around line 73-79: Update the lead-in before the endpoint bullets to
distinguish project identity and self authorization-context grant rules from the
remaining setup reads, noting their broader access conditions. Remove the blank
line between the two endpoint bullets so they remain one continuous Markdown
list, and retain the documented minimal identity response and self-only
authorization-context behavior.
In `@docs/spec_authorization_service.md`:
- Around line 240-246: Correct the AUTH-11B narrative in
docs/spec_authorization_service.md to state that only project.read and
actor.authorization_context.read are active under 11B; keep the other nine
project identity, setup, policy, and active-guide reads planned under
AUTH-11C1/11C2, and ensure the 27-active/51-planned totals remain consistent.
---
Outside diff comments:
In `@backend/app/api/deps/authorization.py`:
- Around line 126-139: Update the concealment logic in the action handling
around concealed_project_reads so ACTOR_AUTHORIZATION_CONTEXT_READ preserves
lifecycle denial codes such as actor_suspended, actor_deactivated, and
identity_link_revoked. Only rewrite project-existence and grant-scope denials to
project_authorization_resource_not_found, while keeping concealment unchanged
for the other project actions.
---
Nitpick comments:
In `@backend/app/modules/authorization/kernel.py`:
- Around line 474-498: The PROJECT_READ and ACTOR_AUTHORIZATION_CONTEXT_READ
paths use different denial evaluation ordering, producing inconsistent
denial_code evidence for equivalent suspended-actor requests. Align the ordering
between _project_read_denial and _authorization_context_denial, while preserving
the existing 404 concealment behavior and each method’s established guards,
availability, revalidation, and lifecycle checks.
In `@backend/app/modules/projects/router.py`:
- Around line 94-133: The get_project route’s union response_model depends on
ProjectResponse field validation failure to prevent contributor data from
matching the full admin schema. Remove the order-sensitive response_model
serialization contract, preferably by setting response_model=None while
retaining the return annotation, and preserve the projection selected by
ProjectService.project_identity_response; extend the existing route-level
regression coverage in test_projects.py to verify contributor responses remain
minimal.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3b197400-7799-45f7-932c-325f8b377e3b
📒 Files selected for processing (25)
.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-11B-project-identity-actor-context.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-11B-internal-review-evidence.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-11B-pr-trust-bundle.md.github/workflows/backend.ymlbackend/app/api/deps/authorization.pybackend/app/api/routes/auth.pybackend/app/modules/actors/schemas.pybackend/app/modules/authorization/catalogue.pybackend/app/modules/authorization/kernel.pybackend/app/modules/authorization/read_service.pybackend/app/modules/authorization/repository.pybackend/app/modules/authorization/runtime.pybackend/app/modules/projects/router.pybackend/app/modules/projects/schemas.pybackend/app/modules/projects/service.pybackend/scripts/api_contract_e2e.pybackend/tests/test_api_controls.pybackend/tests/test_authorization.pybackend/tests/test_projects.pydocs/operations_authorization_service.mddocs/operations_project_operating_manual.mddocs/operations_roles_permissions.mddocs/spec_authorization_service.md
|
CodeRabbit follow-up on the outside-diff and nitpick observations:\n\n- Lifecycle denial concealment is intentionally retained. The signed 11B contract requires unknown, cross-project, unauthorized, inactive-grant, and stale actor/link outcomes to use the identical concealed not-found envelope; the live route test covers suspended actors and revoked links on both endpoints.\n- Denial evaluation ordering was aligned in 751e235 so both new paths perform type/resource guard and availability checks before lifecycle evidence.\n- The project response union is intentionally retained because 11B requires two distinct strict response schemas in OpenAPI. The selected projection is server-owned, and the live HTTP regression asserts the contributor response is exactly id/name/status. Setting response_model=None would discard the API schema guarantee; making full-schema fields optional would weaken the strict boundary. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/tests/test_projects.py (1)
1057-1063: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAssert the concealed error code, not only 404.
The revoked-grant, suspended-actor, and revoked-link loops only assert HTTP 404. A different 404 response could pass, so these tests do not enforce the required
project_authorization_resource_not_foundconcealment contract. Add the same error-code assertion used by the cross-project case.Also applies to: 1075-1080, 1099-1104
🤖 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 `@backend/tests/test_projects.py` around lines 1057 - 1063, The revoked-grant, suspended-actor, and revoked-link test loops currently assert only HTTP 404. Update each loop’s response assertions to also verify the concealed error code is project_authorization_resource_not_found, matching the existing cross-project case while preserving the 404 status assertion.
🤖 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
@.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-11C1-project-setup-diagnostic-reads.md:
- Around line 72-82: Restore explicit 90% project coverage enforcement in the
verification commands for both
.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-11C1-project-setup-diagnostic-reads.md:72-82
and
.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-11C2-effective-policy-active-guide-reads.md:70-80
by including app.modules.projects coverage alongside authorization and applying
the project-specific 90% floor; preserve the existing API E2E, repository-wide
78% floor, and focused project-read coverage requirements.
In `@docs/operations_project_operating_manual.md`:
- Around line 69-73: Update the setup-state description to refer to “API
endpoints” rather than “API reads” or “setup reads,” reflecting that the list
includes POST approval and correction actions; also hyphenate
“self-authorization context” while preserving the existing permission and
authorization rules.
---
Outside diff comments:
In `@backend/tests/test_projects.py`:
- Around line 1057-1063: The revoked-grant, suspended-actor, and revoked-link
test loops currently assert only HTTP 404. Update each loop’s response
assertions to also verify the concealed error code is
project_authorization_resource_not_found, matching the existing cross-project
case while preserving the 404 status assertion.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3e19e408-4b87-4576-8396-5eb30d1ea467
📒 Files selected for processing (13)
.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-11B-project-identity-actor-context.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-11C1-project-setup-diagnostic-reads.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-11C2-effective-policy-active-guide-reads.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-11B-internal-review-evidence.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-11B-pr-trust-bundle.mdbackend/app/api/routes/auth.pybackend/app/modules/authorization/kernel.pybackend/app/modules/authorization/read_service.pybackend/app/modules/authorization/repository.pybackend/tests/test_audit.pybackend/tests/test_projects.pydocs/operations_project_operating_manual.mddocs/spec_authorization_service.md
🚧 Files skipped from review as they are similar to previous changes (8)
- .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-11B-internal-review-evidence.md
- .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-11B-pr-trust-bundle.md
- docs/spec_authorization_service.md
- backend/app/api/routes/auth.py
- backend/app/modules/authorization/repository.py
- backend/app/modules/authorization/read_service.py
- backend/app/modules/authorization/kernel.py
- .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-11B-project-identity-actor-context.md
2 participants
WS-AUTH-001-11B PR Trust Bundle
Chunk
WS-AUTH-001-11B- Project Identity And Actor Context Cutover (L1/P1).Goal
Make project identity and the caller's project authorization context depend
only on canonical Workstream grants, with no token-role fallback.
Human-approved intent
The user approved resuming AUTH-11 while ART proceeds independently and required
each critical authorization chunk to be re-reviewed before implementation.
What changed and why
project.readandactor.authorization_context.read.GET /api/v1/projects/{project_id}to local admin/project grants.GET /api/v1/actors/me/authorization-context?project_id=....authority evidence, scope-aware denials, and missing-project evidence.
actor/link revalidation to both reads.
with shared project-lifecycle guards.
app/modules/projects/*90% coverage gate.These changes remove issuer token roles from project-read authority and prevent
the context route from becoming a project-existence or capability oracle.
Design chosen
Routers resolve the canonical project, the kernel owns authorization and audit
evidence, project service owns response projection, and AUTH read service owns
the context read model. Missing selectors use a bounded AUTH selector solely for
denial evidence. Matched grants are locked through context projection.
Alternatives rejected
require()calls that would create synthetic audit events.Scope control
No project mutations, grant mutations, setup/policy/guide read cutovers,
collection endpoint, new migration, or compatibility alias was added. AUTH-11C
actions remain planned.
Product behavior
Operator, covered Project Manager, Finance Authority, and Audit Authority grants
receive the registered full project identity. Exact-project Submitter, Reviewer,
or Adjudicator grants receive id, name, and status only. Access Administrator
alone and token roles are denied. The context route returns only relevant role
names and active project actions for the exact project.
Acceptance criteria proof and test delta
Tests cover catalogue activation, admin/project-role evidence, admin precedence,
minimal projection, cross-project and revoked-grant denial, suspended actor,
revoked exact link, missing-project evidence, rate-before-lookup, nonhuman
concealment, planned-action exclusion, archived lifecycle filtering, OpenAPI,
and real API E2E expectations. Existing token-role project-read success was
intentionally changed to concealed denial.
Tests and checks run
Local focused tests, Ruff, OpenAPI controls, compile/docstring checks, stale
wording and authorization scans, Markdown links, lightweight agent gates, and
git diff --checkpassed. Full database-backed semantic lanes, API E2E, andcoverage run in GitHub Actions because the local machine is intentionally not
used for the multi-hour full suite.
CI integrity
The existing four semantic lanes, API E2E, repository-wide 78% floor, actor 90%
floor, and authorization 90% floor are unchanged. The project 90% floor is
additive. No skipped failure, threshold reduction, or bypass was introduced.
Reviewer results
Architecture, security, QA, product/ops, senior engineering, and CI integrity
all passed after their findings were fixed. Detailed evidence is in
WS-AUTH-001-11B-internal-review-evidence.md.External review
GitHub Actions and CodeRabbit are pending until the PR is opened.
Remaining risks and follow-up
Hosted Backend must prove the database-backed route test, API E2E, full suite,
and all coverage floors. Future project-action activation must update the
explicit context-action inventory. AUTH-11C1 and 11C2 remain separate chunks.
Human review focus
Review exact grant precedence, minimal contributor fields, missing/cross-project
concealment with evidence, matched-grant locking, and context action filtering.
Human merge ownership
Only the user may approve and merge this PR.
Summary by CodeRabbit
New Features
Security Improvements
Documentation