Skip to content
Merged
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
14 changes: 14 additions & 0 deletions api/app/auth/core/oauth_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,13 @@ def _find_or_create_user_and_link(
status_code=status.HTTP_403_FORBIDDEN,
detail="User account is inactive",
)
# Refresh name and avatar from provider when user logs in with social
if full_name is not None:
user.full_name = full_name
if avatar_url is not None:
user.avatar_url = avatar_url
if full_name is not None or avatar_url is not None:
self.user_repository.update(user)
return user
if not email:
raise HTTPException(
Expand All @@ -229,6 +236,13 @@ def _find_or_create_user_and_link(
existing = self.user_repository.find_by_email(email)
if existing:
user = existing
# Update name and avatar from provider when linking social to existing user
if full_name is not None:
user.full_name = full_name
if avatar_url is not None:
user.avatar_url = avatar_url
if full_name is not None or avatar_url is not None:
self.user_repository.update(user)
else:
user = User(
email=email,
Expand Down
7 changes: 7 additions & 0 deletions api/app/auth/infra/user_social_account_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,10 @@ def create(self, account: UserSocialAccount) -> UserSocialAccount:
def delete(self, account: UserSocialAccount) -> None:
self.db.delete(account)
self.db.commit()

def delete_by_user_id(self, user_id: int) -> None:
"""Delete all social accounts for the given user (e.g. before deleting the user)."""
self.db.query(UserSocialAccount).filter(
UserSocialAccount.user_id == user_id
).delete(synchronize_session=False)
self.db.commit()
23 changes: 21 additions & 2 deletions api/app/organizations/infra/organization_member_repository.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
from sqlalchemy.orm import Session, joinedload
from sqlalchemy import delete
from uuid import UUID
from typing import Optional, List
from app.organizations.infra.organization_member_model import (
OrganizationMember as OrganizationMemberModel,
)
from app.organizations.infra.group_member_model import GroupMember as GroupMemberModel


class OrganizationMemberRepository:
Expand Down Expand Up @@ -53,14 +55,31 @@ def update(self, member: OrganizationMemberModel) -> OrganizationMemberModel:

def delete_by_id(self, member_id: int) -> None:
"""Delete organization member by ID."""
from sqlalchemy import delete

stmt = delete(OrganizationMemberModel).where(
OrganizationMemberModel.id == member_id
)
self.db.execute(stmt)
self.db.commit()

def delete_by_user_id(self, user_id: int) -> None:
"""Delete all organization memberships (and their group memberships) for the given user."""
members = (
self.db.query(OrganizationMemberModel)
.filter(OrganizationMemberModel.user_id == user_id)
.all()
)
member_ids = [m.id for m in members]
if not member_ids:
return
# Remove group_members that reference these organization_members first
self.db.query(GroupMemberModel).filter(
GroupMemberModel.organization_member_id.in_(member_ids)
).delete(synchronize_session=False)
self.db.query(OrganizationMemberModel).filter(
OrganizationMemberModel.user_id == user_id
).delete(synchronize_session=False)
self.db.commit()

def rollback(self) -> None:
"""Rollback current transaction."""
self.db.rollback()
19 changes: 18 additions & 1 deletion api/app/users/api/user_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,18 @@
UserNotFoundError,
UserEmailAlreadyExistsError,
CannotDeleteSelfError,
UserIsOrganizationOwnerError,
)
from app.users.infra.user_model import UserRole
from app.users.infra.user_model import User
from app.shared.dependencies.auth import require_role, get_current_user
from app.auth.core.auth_service import AuthService
from app.auth.infra.token_repository import TokenRepository
from app.auth.infra.user_social_account_repository import UserSocialAccountRepository
from app.organizations.infra.organization_repository import OrganizationRepository
from app.organizations.infra.organization_member_repository import (
OrganizationMemberRepository,
)
from app.auth.core.token_service import TokenService
from app.auth.api.token_dto import (
TokenCreate,
Expand All @@ -34,7 +40,16 @@ def get_user_service(database_session: Session = Depends(get_db)) -> UserService
"""Dependency to get UserService instance."""
user_repository = UserRepository(database_session)
auth_service = AuthService()
return UserService(user_repository, auth_service)
social_account_repository = UserSocialAccountRepository(database_session)
organization_repository = OrganizationRepository(database_session)
organization_member_repository = OrganizationMemberRepository(database_session)
return UserService(
user_repository,
auth_service,
social_account_repository,
organization_repository,
organization_member_repository,
)


def get_token_repository(
Expand Down Expand Up @@ -123,6 +138,8 @@ async def delete_user(
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=str(e))
except CannotDeleteSelfError as e:
raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e))
except UserIsOrganizationOwnerError as e:
raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e))


# Token endpoints under /users/{user_uuid}/tokens
Expand Down
29 changes: 27 additions & 2 deletions api/app/users/core/user_service.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from uuid import UUID
from typing import List, Optional
from typing import List, Optional, TYPE_CHECKING
from app.users.infra.user_repository import UserRepository
from app.users.infra.user_model import User as UserModel, UserRole
from app.users.api.user_dto import UserCreate, UserUpdate, UserResponse
Expand All @@ -9,16 +9,36 @@
validate_user_exists,
validate_user_email_uniqueness,
validate_can_delete_user,
validate_user_is_not_org_owner,
)
from app.auth.core.auth_service import AuthService

if TYPE_CHECKING:
from app.auth.infra.user_social_account_repository import (
UserSocialAccountRepository,
)
from app.organizations.infra.organization_repository import OrganizationRepository
from app.organizations.infra.organization_member_repository import (
OrganizationMemberRepository,
)


class UserService:
"""Business logic for users. No direct database access."""

def __init__(self, repository: UserRepository, auth_service: AuthService):
def __init__(
self,
repository: UserRepository,
auth_service: AuthService,
social_account_repository: "Optional[UserSocialAccountRepository]" = None,
organization_repository: "Optional[OrganizationRepository]" = None,
organization_member_repository: "Optional[OrganizationMemberRepository]" = None,
):
self.repository = repository
self.auth_service = auth_service
self.social_account_repository = social_account_repository
self.organization_repository = organization_repository
self.organization_member_repository = organization_member_repository

def create_user(self, dto: UserCreate) -> UserResponse:
"""Create a new user."""
Expand Down Expand Up @@ -74,6 +94,11 @@ def delete_user(self, uuid: UUID, current_user_uuid: UUID) -> None:
validate_can_delete_user(self.repository, uuid, current_user_uuid)

user = self.repository.find_by_uuid(uuid)
validate_user_is_not_org_owner(self.organization_repository, user.id)
if self.organization_member_repository:
self.organization_member_repository.delete_by_user_id(user.id)
if self.social_account_repository:
self.social_account_repository.delete_by_user_id(user.id)
self.repository.delete(user)

def _build_user_entity(self, dto: UserCreate, hashed_password: str) -> UserModel:
Expand Down
23 changes: 23 additions & 0 deletions api/app/users/core/user_validators.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from uuid import UUID
from typing import List
from app.users.infra.user_repository import UserRepository
from app.users.api.user_dto import UserCreate, UserUpdate

Expand All @@ -21,6 +22,18 @@ class CannotDeleteSelfError(Exception):
pass


class UserIsOrganizationOwnerError(Exception):
"""Raised when trying to delete a user who is owner of one or more organizations."""

def __init__(self, organization_names: List[str]):
self.organization_names = organization_names
names = ", ".join(organization_names)
super().__init__(
f"User is owner of organization(s): {names}. "
"Transfer ownership to another user before deleting."
)


def validate_user_create_dto(dto: UserCreate) -> None:
"""Validate user create DTO. Raises ValueError if validation fails."""
if not dto.email or not dto.email.strip():
Expand Down Expand Up @@ -63,3 +76,13 @@ def validate_can_delete_user(
"""Validate that user can be deleted (not self)."""
if user_uuid == current_user_uuid:
raise CannotDeleteSelfError("Cannot delete your own user")


def validate_user_is_not_org_owner(organization_repository, user_id: int) -> None:
"""Validate that user is not owner of any organization. Raises UserIsOrganizationOwnerError if owner."""
if organization_repository is None:
return
orgs = organization_repository.find_by_owner_user_id(user_id, skip=0, limit=1000)
if orgs:
names = [o.name for o in orgs]
raise UserIsOrganizationOwnerError(names)
6 changes: 6 additions & 0 deletions api/tests/integration/conftest.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Configuration for integration tests."""
import pytest
from uuid import UUID
from fastapi.testclient import TestClient
from sqlalchemy import create_engine, event
from sqlalchemy.orm import sessionmaker
Expand Down Expand Up @@ -83,6 +84,11 @@ def receive_before_flush(session, flush_context, instances):
# Fallback: set to now
instance.updated_at = datetime.now()

# SQLite may return UUID columns as string; ensure uuid.UUID for flush
if hasattr(instance, "uuid") and instance.uuid is not None:
if isinstance(instance.uuid, str):
instance.uuid = UUID(instance.uuid)

TestingSessionLocal = sessionmaker(autocommit=False, autoflush=False, bind=engine)


Expand Down
Loading