Skip to content
Open
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: 11 additions & 1 deletion app/api/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@
from app.core.auth_cache import invalidate_user
from app.core.auth_middleware import get_current_user
from app.core.database import get_db
from app.core.org_scope import get_managed_org_ids
from app.core.org_scope import get_managed_org_ids, get_managed_project_ids
from app.db.models.auth import User
from app.models.auth import (
AuthResponse,
ForgotPasswordRequest,
MyManagedOrgsResponse,
MyManagedProjectsResponse,
MyProjectRolesResponse,
PasswordResetResponse,
ProfileUpdate,
Expand Down Expand Up @@ -135,6 +136,15 @@ async def my_managed_orgs(
return MyManagedOrgsResponse(managed_org_ids=org_ids)


@router.get("/my-managed-projects", response_model=MyManagedProjectsResponse)
async def my_managed_projects(
user: User = Depends(get_current_user),
db: AsyncSession = Depends(get_db),
) -> MyManagedProjectsResponse:
project_ids = await get_managed_project_ids(db, user.id)
return MyManagedProjectsResponse(managed_project_ids=project_ids)


@router.post("/forgot-password", response_model=PasswordResetResponse)
async def forgot_password(
payload: ForgotPasswordRequest, db: AsyncSession = Depends(get_db)
Expand Down
9 changes: 7 additions & 2 deletions app/api/languages.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from app.core.auth_middleware import get_current_user
from app.core.database import get_db
from app.core.exceptions import NotFoundError
from app.core.org_scope import get_managed_project_ids
from app.db.models.auth import User
from app.models.language import LanguageCreate, LanguageResponse
from app.services import language_service
Expand All @@ -14,9 +15,13 @@
@router.get("", response_model=list[LanguageResponse])
async def list_languages(
db: AsyncSession = Depends(get_db),
_: User = Depends(get_current_user),
user: User = Depends(get_current_user),
) -> list[LanguageResponse]:
languages = await language_service.list_languages(db)
if user.is_platform_admin:
languages = await language_service.list_languages(db)
else:
managed_project_ids = await get_managed_project_ids(db, user.id)
languages = await language_service.list_languages_by_projects(db, managed_project_ids)
return [LanguageResponse.model_validate(lang) for lang in languages]


Expand Down
10 changes: 7 additions & 3 deletions app/api/organizations.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from app.core.auth_middleware import get_current_user, require_platform_admin
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 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 -S

Repository: 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.

from app.db.models.auth import User
from app.models.org import (
OrganizationCreate,
Expand All @@ -24,9 +24,13 @@
@router.get("", response_model=list[OrganizationResponse])
async def list_organizations(
db: AsyncSession = Depends(get_db),
_: User = Depends(get_current_user),
user: User = Depends(get_current_user),
) -> list[OrganizationResponse]:
orgs = await organization_service.list_organizations(db)
if user.is_platform_admin:
orgs = await organization_service.list_organizations(db)
else:
managed_project_ids = await get_managed_project_ids(db, user.id)
orgs = await organization_service.list_organizations_by_projects(db, managed_project_ids)
return [OrganizationResponse.model_validate(o) for o in orgs]


Expand Down
20 changes: 13 additions & 7 deletions app/api/phases.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

from app.core.auth_middleware import get_current_user
from app.core.database import get_db
from app.core.org_scope import get_managed_project_ids
from app.db.models.auth import User
from app.models.phase import (
DependencyCreate,
Expand Down Expand Up @@ -33,20 +34,25 @@ async def list_phases(
db: AsyncSession = Depends(get_db),
user: User = Depends(get_current_user),
) -> list[PhaseResponse]:
phases = await phase_service.list_phases(db, project_id=project_id)
result = []
for phase in phases:
data = PhaseResponse.model_validate(phase)
result.append(data)
return result
if user.is_platform_admin:
phases = await phase_service.list_phases(db, project_id=project_id)
else:
managed_project_ids = await get_managed_project_ids(db, user.id)
phases = await phase_service.list_phases_by_projects(
db, managed_project_ids, project_id=project_id
)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
return [PhaseResponse.model_validate(phase) for phase in phases]


@router.get("/with-dependencies", response_model=PhasesWithDepsResponse)
async def list_phases_with_dependencies(
db: AsyncSession = Depends(get_db),
user: User = Depends(get_current_user),
) -> PhasesWithDepsResponse:
return await phase_service.list_all_phases_with_deps(db)
if user.is_platform_admin:
return await phase_service.list_all_phases_with_deps(db)
managed_project_ids = await get_managed_project_ids(db, user.id)
return await phase_service.list_phases_with_deps_by_projects(db, managed_project_ids)


@router.get("/{phase_id}", response_model=PhaseResponse)
Expand Down
4 changes: 2 additions & 2 deletions app/api/projects/_deps.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@
async def assert_project_access(db: AsyncSession, user: User, project_id: str) -> None:
if user.is_platform_admin:
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)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

if not is_manager:
raise AuthorizationError("You do not have access to this project")
5 changes: 2 additions & 3 deletions app/api/projects/access.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from sqlalchemy.ext.asyncio import AsyncSession

from app.api.projects._deps import assert_project_access
from app.core.auth_middleware import get_current_user
from app.core.auth_middleware import get_current_user, require_platform_admin
from app.core.database import get_db
from app.db.models.auth import User
from app.models.project import (
Expand Down Expand Up @@ -115,10 +115,9 @@ async def update_user_access_role(
project_id: str,
user_id: str,
payload: ProjectUserAccessRoleUpdate,
actor: User = Depends(get_current_user),
_: User = Depends(require_platform_admin),
db: AsyncSession = Depends(get_db),
) -> ProjectUserAccessResponse:
await assert_project_access(db, actor, project_id)
access = await project_service.update_user_access_role(db, project_id, user_id, payload.role)
return ProjectUserAccessResponse.model_validate(access)

Expand Down
15 changes: 15 additions & 0 deletions app/core/auth_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,18 @@ async def require_platform_admin(
if not user.is_platform_admin:
raise AuthorizationError("Forbidden")
return user


async def require_admin_or_manager(
db: AsyncSession = Depends(get_db),
user: User = Depends(get_current_user),
) -> User:
if user.is_platform_admin:
return user

from app.core.org_scope import get_managed_org_ids, get_managed_project_ids

if await get_managed_org_ids(db, user.id) or await get_managed_project_ids(db, user.id):
return user

raise AuthorizationError("Forbidden")
64 changes: 36 additions & 28 deletions app/core/org_scope.py
Original file line number Diff line number Diff line change
@@ -1,28 +1,36 @@
from fastapi import Depends
from sqlalchemy import select, union
from sqlalchemy.ext.asyncio import AsyncSession

from app.core.auth_middleware import get_current_user
from app.core.database import get_db
from app.db.models.auth import User
from app.db.models.org import MemberRole, Organization, OrganizationMember


async def get_managed_org_ids(db: AsyncSession, user_id: str) -> list[str]:
from_members = select(OrganizationMember.organization_id.label("org_id")).where(
OrganizationMember.user_id == user_id,
OrganizationMember.role == MemberRole.MANAGER,
)

from_orgs = select(Organization.id.label("org_id")).where(Organization.manager_id == user_id)

combined = union(from_members, from_orgs).subquery()
result = await db.execute(select(combined.c.org_id))
return sorted(result.scalars().all())


async def get_current_user_managed_org_ids(
db: AsyncSession = Depends(get_db),
user: User = Depends(get_current_user),
) -> list[str]:
return await get_managed_org_ids(db, user.id)
from fastapi import Depends
from sqlalchemy import select, union
from sqlalchemy.ext.asyncio import AsyncSession

from app.core.auth_middleware import get_current_user
from app.core.database import get_db
from app.db.models.auth import User
from app.db.models.org import MemberRole, Organization, OrganizationMember


async def get_managed_project_ids(db: AsyncSession, user_id: str) -> list[str]:
from app.services.project.get_managed_project_ids import (
get_managed_project_ids as _get_managed_project_ids,
)

return await _get_managed_project_ids(db, user_id)


async def get_managed_org_ids(db: AsyncSession, user_id: str) -> list[str]:
from_members = select(OrganizationMember.organization_id.label("org_id")).where(
OrganizationMember.user_id == user_id,
OrganizationMember.role == MemberRole.MANAGER,
)

from_orgs = select(Organization.id.label("org_id")).where(Organization.manager_id == user_id)

combined = union(from_members, from_orgs).subquery()
result = await db.execute(select(combined.c.org_id))
return sorted(result.scalars().all())


async def get_current_user_managed_org_ids(
db: AsyncSession = Depends(get_db),
user: User = Depends(get_current_user),
) -> list[str]:
return await get_managed_org_ids(db, user.id)
32 changes: 27 additions & 5 deletions app/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from collections.abc import AsyncIterator
from contextlib import asynccontextmanager

from fastapi import FastAPI
from fastapi import Depends, FastAPI, status
from fastapi.middleware.cors import CORSMiddleware

from app.api.access_requests import router as access_requests_router
Expand Down Expand Up @@ -41,6 +41,7 @@
from app.api.translation_helper import router as translation_helper_router
from app.api.uploads import router as uploads_router
from app.api.users import router as users_router
from app.core.auth_middleware import require_admin_or_manager
from app.core.config import get_settings
from app.core.database import AsyncSessionLocal, close_db, init_db
from app.core.exceptions import register_exception_handlers
Expand Down Expand Up @@ -129,11 +130,32 @@ def create_app() -> FastAPI:
app.include_router(platform_router, prefix="/api/platform", tags=["platform"])
app.include_router(uploads_router, prefix="/api/uploads", tags=["uploads"])
app.include_router(users_router, prefix="/api/users", tags=["users"])
app.include_router(languages_router, prefix="/api/languages", tags=["languages"])
app.include_router(organizations_router, prefix="/api/organizations", tags=["organizations"])
console_guard = [Depends(require_admin_or_manager)]
app.include_router(
languages_router,
prefix="/api/languages",
tags=["languages"],
dependencies=console_guard,
)
app.include_router(
organizations_router,
prefix="/api/organizations",
tags=["organizations"],
dependencies=console_guard,
)
app.include_router(places_router, prefix="/api/places", tags=["places"])
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,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/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.

prefix="/api/projects",
tags=["projects"],
dependencies=console_guard,
)
app.include_router(
phases_router,
prefix="/api/phases",
tags=["phases"],
dependencies=console_guard,
)
app.include_router(books_router, prefix="/api/books", tags=["books"])
app.include_router(pericopes_router, prefix="/api/pericopes", tags=["pericopes"])
app.include_router(meaning_maps_router, prefix="/api/meaning-maps", tags=["meaning-maps"])
Expand Down
4 changes: 4 additions & 0 deletions app/models/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ class MyManagedOrgsResponse(BaseModel):
managed_org_ids: list[str]


class MyManagedProjectsResponse(BaseModel):
managed_project_ids: list[str]


class ForgotPasswordRequest(BaseModel):
email: EmailStr
app_key: str = Field(max_length=100)
Expand Down
2 changes: 2 additions & 0 deletions app/services/language/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@
from app.services.language.get_language_by_id import get_language_by_id
from app.services.language.get_language_or_404 import get_language_or_404
from app.services.language.list_languages import list_languages
from app.services.language.list_languages_by_projects import list_languages_by_projects

__all__ = [
"create_language",
"get_language_by_code",
"get_language_by_id",
"get_language_or_404",
"list_languages",
"list_languages_by_projects",
]
14 changes: 14 additions & 0 deletions app/services/language/list_languages_by_projects.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
from sqlalchemy import select
from sqlalchemy.ext.asyncio import AsyncSession

from app.db.models.language import Language
from app.db.models.project import Project


async def list_languages_by_projects(db: AsyncSession, project_ids: list[str]) -> list[Language]:
if not project_ids:
return []
language_ids_subq = select(Project.language_id).where(Project.id.in_(project_ids)).distinct()
stmt = select(Language).where(Language.id.in_(language_ids_subq)).order_by(Language.code)
result = await db.execute(stmt)
return list(result.scalars().all())
2 changes: 2 additions & 0 deletions app/services/org/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from app.services.org.is_member import is_member
from app.services.org.list_members import list_members
from app.services.org.list_organizations import list_organizations
from app.services.org.list_organizations_by_projects import list_organizations_by_projects
from app.services.org.remove_member import remove_member
from app.services.org.update_member_role import update_member_role
from app.services.org.update_organization import update_organization
Expand All @@ -21,6 +22,7 @@
"is_member",
"list_members",
"list_organizations",
"list_organizations_by_projects",
"remove_member",
"update_member_role",
"update_organization",
Expand Down
20 changes: 20 additions & 0 deletions app/services/org/list_organizations_by_projects.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
from sqlalchemy import select
from sqlalchemy.ext.asyncio import AsyncSession

from app.db.models.org import Organization
from app.db.models.project import ProjectOrganizationAccess


async def list_organizations_by_projects(
db: AsyncSession, project_ids: list[str]
) -> list[Organization]:
if not project_ids:
return []
org_ids_subq = (
select(ProjectOrganizationAccess.organization_id)
.where(ProjectOrganizationAccess.project_id.in_(project_ids))
.distinct()
)
stmt = select(Organization).where(Organization.id.in_(org_ids_subq)).order_by(Organization.name)
result = await db.execute(stmt)
return list(result.scalars().unique().all())
6 changes: 6 additions & 0 deletions app/services/phase/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@
from app.services.phase.list_all_phases_with_deps import list_all_phases_with_deps
from app.services.phase.list_dependencies import list_dependencies
from app.services.phase.list_phases import list_phases
from app.services.phase.list_phases_by_projects import (
list_phases_by_projects,
list_phases_with_deps_by_projects,
)
from app.services.phase.list_project_phases_with_deps import list_project_phases_with_deps
from app.services.phase.list_project_phases_with_details import list_project_phases_with_details
from app.services.phase.list_projects_for_phase import list_projects_for_phase
Expand All @@ -26,6 +30,8 @@
"list_all_phases_with_deps",
"list_dependencies",
"list_phases",
"list_phases_by_projects",
"list_phases_with_deps_by_projects",
"list_project_phases_with_deps",
"list_project_phases_with_details",
"list_projects_for_phase",
Expand Down
Loading
Loading