[US-11.10 / OBT-239] Expose the authenticated user's managed project IDs - #93
Conversation
Non-admin users receive only the data tied to the projects they manage (ProjectUserAccess.role == "manager"), instead of the full dataset: organizations linked to those projects, the languages of those projects, and the phases attached to them. Platform admins are unaffected and continue to see everything. Adds get_managed_project_ids and per-domain "by projects" services (list_organizations_by_projects, list_languages_by_projects, list_phases_by_projects and list_phases_with_deps_by_projects), replacing the earlier organization-based scoping to align with the 2026-07 per-project manager model. Wires them into the organizations, languages and phases list endpoints.
…project manager rule Adds require_admin_or_manager and applies it to the console content routers (organizations, projects, languages, phases), so users who are neither platform admins nor managers of any organization or project now receive 403 instead of an empty 200 list. Enforces the 2026-07 per-project manager rule on the projects surface: a non-admin may only view and manage the projects on which they hold the manager role. list_projects_for_user returns only the caller's managed projects and assert_project_access denies project detail, access and phase sub-routes for projects outside that set. Reuses get_managed_project_ids (US-11.6) and adds the is_project_manager and list_projects_by_ids services. Updates the affected tests.
The project Access-tab role update endpoint (PATCH /api/projects/{project_id}/access/users/
{user_id}) now requires platform admin. Organization/project managers and other users who
merely have access to the project receive 403; unauthenticated requests still receive 401.
The backend becomes the source of truth for the admin-only role rule (the frontend gate lives
in US-12.8). The update_user_access_role service is unchanged; authorization is enforced in the
router per backend guidelines.
Adds GET /api/auth/my-managed-projects returning { "managed_project_ids": [...] } for the current
user, resolved via get_managed_project_ids (projects where the user's access role is manager).
The endpoint mirrors my-managed-orgs and is protected by get_current_user, letting the console
derive manager status consistently with the backend's per-project scoping (US-12.9).
48b4335 to
17d9664
Compare
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: QUIET Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughChangesProject manager scope is derived from project access roles and applied to console routers, resource listings, project access checks, and project listing endpoints. Platform admins retain unrestricted listing access, while access-role updates require platform-admin authorization. A managed-projects endpoint and scope tests were added. Console scoping and authorization
Sequence Diagram(s)sequenceDiagram
participant Client
participant ConsoleRouter
participant AuthDependency
participant ScopeHelper
participant ListingService
Client->>ConsoleRouter: request console resource list
ConsoleRouter->>AuthDependency: validate admin or manager
AuthDependency->>ScopeHelper: resolve managed project IDs
ScopeHelper-->>AuthDependency: project IDs
AuthDependency-->>ConsoleRouter: authorized user
ConsoleRouter->>ListingService: list resources for project IDs
ListingService-->>Client: scoped response
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 3
🧹 Nitpick comments (3)
app/services/phase/list_phases_by_projects.py (1)
8-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd concise docstrings to the new public service functions. All four newly added public service functions omit the docstrings required by guidelines for
app/services/**/*.py.
app/services/phase/list_phases_by_projects.py#L8-L27: add a one-line docstring tolist_phases_by_projectsdescribing the project-scoped phase listing contract (including theproject_idnarrowing behavior).app/services/phase/list_phases_by_projects.py#L30-L55: add a one-line docstring tolist_phases_with_deps_by_projectsdescribing the scoped dependency-map contract.app/services/org/list_organizations_by_projects.py#L8-L20: add a one-line docstring tolist_organizations_by_projects.app/services/project/list_projects_by_ids.py#L7-L12: add a one-line docstring tolist_projects_by_ids.As per coding guidelines, "Keep service functions function-oriented and composable, with concise docstrings on public service functions."
🤖 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 `@app/services/phase/list_phases_by_projects.py` around lines 8 - 27, Add concise one-line docstrings to all four public service functions: list_phases_by_projects in app/services/phase/list_phases_by_projects.py (lines 8-27), including its project_id narrowing behavior; list_phases_with_deps_by_projects in app/services/phase/list_phases_by_projects.py (lines 30-55), describing its scoped dependency-map contract; list_organizations_by_projects in app/services/org/list_organizations_by_projects.py (lines 8-20); and list_projects_by_ids in app/services/project/list_projects_by_ids.py (lines 7-12). Preserve each function’s existing behavior.Source: Coding guidelines
app/services/project/is_project_manager.py (1)
8-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the new public service functions.
Both functions are correct but omit the concise docstrings required for public service functions.
app/services/project/is_project_manager.py#L8-L8: add a concise docstring describing the manager-role check.app/services/language/list_languages_by_projects.py#L8-L8: add a concise docstring describing the project-scoped language listing.🤖 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 `@app/services/project/is_project_manager.py` at line 8, Add concise docstrings to the public functions is_project_manager in app/services/project/is_project_manager.py at lines 8-8 and list_languages_by_projects in app/services/language/list_languages_by_projects.py at lines 8-8, describing the manager-role check and project-scoped language listing respectively.Source: Coding guidelines
app/core/auth_middleware.py (1)
26-40: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winScope lookups are recomputed per-handler instead of shared via FastAPI's dependency cache.
require_admin_or_managercallsget_managed_org_ids/get_managed_project_idsas plain awaited functions rather than throughDepends(...), so FastAPI's per-request dependency cache never applies; each guarded listing handler then reissues the sameget_managed_project_idsquery, and theis_platform_adminbranch is duplicated across handlers.
app/core/auth_middleware.py#L26-L40: extract the org/project scope lookup into aDepends-based helper so its result can be cached and reused by downstream handlers instead of being called as a bare function.app/api/languages.py#L18-L24: consume the shared scope dependency instead of re-invokingget_managed_project_idsdirectly.app/api/organizations.py#L27-L33: consume the shared scope dependency instead of re-invokingget_managed_project_idsdirectly.🤖 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 `@app/core/auth_middleware.py` around lines 26 - 40, Scope lookups are duplicated because require_admin_or_manager and guarded handlers call them directly instead of using FastAPI dependency caching. In app/core/auth_middleware.py lines 26-40, extract the shared org/project scope lookup into a Depends-based helper and use it from require_admin_or_manager while preserving the platform-admin bypass. In app/api/languages.py lines 18-24 and app/api/organizations.py lines 27-33, consume that shared dependency rather than invoking get_managed_project_ids directly.
🤖 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 `@app/api/organizations.py`:
- Line 7: Update list_organizations() to include organization IDs returned by
get_managed_org_ids() alongside the existing get_managed_project_ids() scope
before applying require_admin_or_manager, ensuring users who directly manage an
organization receive it even when they manage no projects.
In `@app/api/phases.py`:
- Around line 40-43: Move the database query behind get_managed_project_ids into
an appropriate function under app/services/, then update all callers to use the
service function: app/api/phases.py lines 40-43 and 54-55, and app/api/auth.py
lines 144-145. Remove direct router usage of the core helper while preserving
the existing managed-project filtering behavior.
In `@app/core/org_scope.py`:
- Around line 12-19: Move the database query out of core function
get_managed_project_ids into a service module under app/services/project,
preserving its filtering and sorted-ID behavior. In app/core/org_scope.py,
re-export or delegate to the service function so existing consumers such as
require_admin_or_manager and routers keep their current import surface
unchanged.
---
Nitpick comments:
In `@app/core/auth_middleware.py`:
- Around line 26-40: Scope lookups are duplicated because
require_admin_or_manager and guarded handlers call them directly instead of
using FastAPI dependency caching. In app/core/auth_middleware.py lines 26-40,
extract the shared org/project scope lookup into a Depends-based helper and use
it from require_admin_or_manager while preserving the platform-admin bypass. In
app/api/languages.py lines 18-24 and app/api/organizations.py lines 27-33,
consume that shared dependency rather than invoking get_managed_project_ids
directly.
In `@app/services/phase/list_phases_by_projects.py`:
- Around line 8-27: Add concise one-line docstrings to all four public service
functions: list_phases_by_projects in
app/services/phase/list_phases_by_projects.py (lines 8-27), including its
project_id narrowing behavior; list_phases_with_deps_by_projects in
app/services/phase/list_phases_by_projects.py (lines 30-55), describing its
scoped dependency-map contract; list_organizations_by_projects in
app/services/org/list_organizations_by_projects.py (lines 8-20); and
list_projects_by_ids in app/services/project/list_projects_by_ids.py (lines
7-12). Preserve each function’s existing behavior.
In `@app/services/project/is_project_manager.py`:
- Line 8: Add concise docstrings to the public functions is_project_manager in
app/services/project/is_project_manager.py at lines 8-8 and
list_languages_by_projects in
app/services/language/list_languages_by_projects.py at lines 8-8, describing the
manager-role check and project-scoped language listing respectively.
🪄 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: Organization UI
Review profile: QUIET
Plan: Pro Plus
Run ID: a8d60b5e-35d8-4eac-b60d-83ae8b05ec09
📒 Files selected for processing (23)
app/api/auth.pyapp/api/languages.pyapp/api/organizations.pyapp/api/phases.pyapp/api/projects/_deps.pyapp/api/projects/access.pyapp/core/auth_middleware.pyapp/core/org_scope.pyapp/main.pyapp/models/auth.pyapp/services/language/__init__.pyapp/services/language/list_languages_by_projects.pyapp/services/org/__init__.pyapp/services/org/list_organizations_by_projects.pyapp/services/phase/__init__.pyapp/services/phase/list_phases_by_projects.pyapp/services/project/__init__.pyapp/services/project/is_project_manager.pyapp/services/project/list_projects_by_ids.pyapp/services/project/list_projects_for_user.pytests/baker.pytests/test_console_scoping.pytests/test_project_org_filter.py
| from app.core.database import get_db | ||
| from app.core.exceptions import AuthorizationError, NotFoundError | ||
| from app.core.org_scope import get_managed_org_ids | ||
| from app.core.org_scope import get_managed_org_ids, get_managed_project_ids |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== app/api/organizations.py ==\n'
cat -n app/api/organizations.py | sed -n '1,220p'
printf '\n== app/core/auth_middleware.py ==\n'
cat -n app/core/auth_middleware.py | sed -n '1,240p'
printf '\n== app/core/org_scope.py ==\n'
cat -n app/core/org_scope.py | sed -n '1,260p'
printf '\n== search references ==\n'
rg -n "get_managed_org_ids|get_managed_project_ids|list_organizations_by_projects|require_admin_or_manager|list_organizations" app -SRepository: shemaobt/tripod-api
Length of output: 13753
Include organizations managed directly by the user. list_organizations() only scopes non-admins through get_managed_project_ids(), so users who manage an org directly but no projects pass require_admin_or_manager and still get an empty list. Merge in get_managed_org_ids() here, or let the service accept both scopes.
🤖 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 `@app/api/organizations.py` at line 7, Update list_organizations() to include
organization IDs returned by get_managed_org_ids() alongside the existing
get_managed_project_ids() scope before applying require_admin_or_manager,
ensuring users who directly manage an organization receive it even when they
manage no projects.
|
@coderabbitai — decisions on this review:
|
|
Tip For best results, initiate chat on the files or code changes.
Thanks for taking the time to go through these one by one. ✏️ Learnings added
|
…uters) Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/main.py (1)
128-153: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winScope
console_guardto console-only project routes
app/main.py:128-153appliesrequire_admin_or_managerto the wholeprojects_router, butapp/api/projects/core.pystill exposes project list/read/update endpoints to any authenticated user with project access. This blanket guard will 403 regular project members on/api/projects. Split the console-only routes or move the dependency to those routers only.🤖 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 `@app/main.py` around lines 128 - 153, Restrict console_guard in the router registration around projects_router so it applies only to console-only project endpoints, not the member-accessible project list/read/update routes. Split the project routes into separate routers or otherwise attach require_admin_or_manager only to the console-specific router, while preserving project access for authenticated members.
🤖 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.
Outside diff comments:
In `@app/main.py`:
- Around line 128-153: Restrict console_guard in the router registration around
projects_router so it applies only to console-only project endpoints, not the
member-accessible project list/read/update routes. Split the project routes into
separate routers or otherwise attach require_admin_or_manager only to the
console-specific router, while preserving project access for authenticated
members.
… layer; core delegates Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
|
Follow-up on decision 2: done. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@app/services/project/get_managed_project_ids.py`:
- Around line 8-15: Add a concise public-service docstring to
get_managed_project_ids describing that it returns the sorted project IDs
managed by the specified user. Keep the query and return behavior unchanged.
🪄 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: Organization UI
Review profile: QUIET
Plan: Pro Plus
Run ID: 10465f91-a96b-4cfa-b73d-ce8cdd44cfc0
📒 Files selected for processing (2)
app/core/org_scope.pyapp/services/project/get_managed_project_ids.py
| async def get_managed_project_ids(db: AsyncSession, user_id: str) -> list[str]: | ||
| result = await db.execute( | ||
| select(ProjectUserAccess.project_id).where( | ||
| ProjectUserAccess.user_id == user_id, | ||
| ProjectUserAccess.role == MemberRole.MANAGER, | ||
| ) | ||
| ) | ||
| return sorted(result.scalars().all()) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add the required public service docstring.
This public service function should document that it returns the sorted project IDs managed by the specified user.
Suggested change
async def get_managed_project_ids(db: AsyncSession, user_id: str) -> list[str]:
+ """Return sorted project IDs managed by the user."""
result = await db.execute(As per coding guidelines, public service functions must have concise docstrings.
📝 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.
| async def get_managed_project_ids(db: AsyncSession, user_id: str) -> list[str]: | |
| result = await db.execute( | |
| select(ProjectUserAccess.project_id).where( | |
| ProjectUserAccess.user_id == user_id, | |
| ProjectUserAccess.role == MemberRole.MANAGER, | |
| ) | |
| ) | |
| return sorted(result.scalars().all()) | |
| async def get_managed_project_ids(db: AsyncSession, user_id: str) -> list[str]: | |
| """Return sorted project IDs managed by the user.""" | |
| result = await db.execute( | |
| select(ProjectUserAccess.project_id).where( | |
| ProjectUserAccess.user_id == user_id, | |
| ProjectUserAccess.role == MemberRole.MANAGER, | |
| ) | |
| ) | |
| return sorted(result.scalars().all()) |
🤖 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 `@app/services/project/get_managed_project_ids.py` around lines 8 - 15, Add a
concise public-service docstring to get_managed_project_ids describing that it
returns the sorted project IDs managed by the specified user. Keep the query and
return behavior unchanged.
Source: Coding guidelines
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
| return | ||
| allowed = await project_service.can_access_project(db, user.id, project_id) | ||
| if not allowed: | ||
| is_manager = await project_service.is_project_manager(db, user.id, project_id) |
There was a problem hiding this comment.
can_access_project was true for any access row or via the org membership, is_project_manager only for role == manager. So every plain member and everyone that had access through the org now takes 403 on all the endpoints using this dep, not only the console ones. Is that correct? The PR is only for expose the managed project ids — if it is really intentional the name and the message "You do not have access to this project" are lying now. @levigtri
| app.include_router(projects_router, prefix="/api/projects", tags=["projects"]) | ||
| app.include_router(phases_router, prefix="/api/phases", tags=["phases"]) | ||
| app.include_router( | ||
| projects_router, |
There was a problem hiding this comment.
/api/projects and /api/phases are not consumed only by the console I think, and with this guard anyone that is not manager/admin somewhere takes 403 on the whole surface. Nothing of this is on the PR description ("Type of Change: Feature (new read-only endpoint)"). Just make sure here that this is really the ticket. ps: same access question of _deps.py.
…ed-users-managed-project-ids # Conflicts: # app/main.py
2 participants
[US-11.10 / OBT-239] Expose the authenticated user's managed project IDs
Summary
Adds
GET /api/auth/my-managed-projectsreturning the projects the current user manages, so the console can derive manager status consistently with the backend's per-project scoping (US-12.9).Changes
1. Response schema
app/models/auth.py— newMyManagedProjectsResponse(managed_project_ids: list[str]).2. Endpoint
app/api/auth.py—GET /api/auth/my-managed-projectsprotected byget_current_user, returningget_managed_project_ids(db, user.id). Mirrorsmy-managed-orgs.Type of Change
Testing
{ managed_project_ids: [...] }; unauth → 401. Covered bytests/test_console_scoping.py.Summary by CodeRabbit