From e6469822cc5c30ad52cff60d59204be82ca1ae72 Mon Sep 17 00:00:00 2001 From: Yannick Mueller Date: Wed, 26 Mar 2025 09:41:53 +0100 Subject: [PATCH 01/23] Add Schema to reflect Users and Groups --- ...b248c_implemented_user_and_groups_model.py | 87 +++++++++++++++++++ src/foxops/database/schema.py | 52 +++++++++++ 2 files changed, 139 insertions(+) create mode 100644 alembic/versions/2025_03_26-5ae180bb248c_implemented_user_and_groups_model.py diff --git a/alembic/versions/2025_03_26-5ae180bb248c_implemented_user_and_groups_model.py b/alembic/versions/2025_03_26-5ae180bb248c_implemented_user_and_groups_model.py new file mode 100644 index 00000000..8aaeab03 --- /dev/null +++ b/alembic/versions/2025_03_26-5ae180bb248c_implemented_user_and_groups_model.py @@ -0,0 +1,87 @@ +"""Add User and Groups Model + +Revision ID: 5ae180bb248c +Revises: 00ee97d0b7a3 +Create Date: 2025-03-26 07:30:50.314010+00:00 + +""" + +import sqlalchemy as sa + +from alembic import op + +# revision identifiers, used by Alembic. +revision = "5ae180bb248c" +down_revision = "00ee97d0b7a3" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.create_table( + "group", + sa.Column("id", sa.Integer(), nullable=False), + sa.Column("system_name", sa.String(), nullable=False), + sa.Column("display_name", sa.String(), nullable=False), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint("system_name"), + ) + op.create_table( + "user", + sa.Column("id", sa.Integer(), nullable=False), + sa.Column("username", sa.String(), nullable=False), + sa.Column("is_admin", sa.Boolean(), nullable=False), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint("username"), + ) + op.create_table( + "group_user", + sa.Column("group_id", sa.Integer(), nullable=False), + sa.Column("user_id", sa.Integer(), nullable=False), + sa.ForeignKeyConstraint(["group_id"], ["group.id"], ondelete="CASCADE"), + sa.ForeignKeyConstraint(["user_id"], ["user.id"], ondelete="CASCADE"), + sa.PrimaryKeyConstraint("group_id", "user_id"), + ) + op.create_table( + "group_incarnation_permission", + sa.Column("group_id", sa.Integer(), nullable=False), + sa.Column("incarnation_id", sa.Integer(), nullable=False), + sa.Column("type", sa.Enum("READ", "WRITE", name="permission"), nullable=False), + sa.ForeignKeyConstraint(["group_id"], ["group.id"], ondelete="CASCADE"), + sa.ForeignKeyConstraint(["incarnation_id"], ["incarnation.id"], ondelete="CASCADE"), + sa.PrimaryKeyConstraint("group_id", "incarnation_id"), + ) + op.create_table( + "user_incarnation_permission", + sa.Column("user_id", sa.Integer(), nullable=False), + sa.Column("incarnation_id", sa.Integer(), nullable=False), + sa.Column("type", sa.Enum("READ", "WRITE", name="permission"), nullable=False), + sa.ForeignKeyConstraint(["incarnation_id"], ["incarnation.id"], ondelete="CASCADE"), + sa.ForeignKeyConstraint(["user_id"], ["user.id"], ondelete="CASCADE"), + sa.PrimaryKeyConstraint("user_id", "incarnation_id"), + ) + with op.batch_alter_table("change") as batch_op: + batch_op.add_column(sa.Column("initialized_by", sa.Integer(), nullable=True)) + batch_op.create_foreign_key("change_user_fk", "user", ["initialized_by"], ["id"], ondelete="SET NULL") + + with op.batch_alter_table("incarnation") as batch_op: + batch_op.add_column(sa.Column("owner", sa.Integer(), nullable=True)) + batch_op.create_foreign_key("incarnation_owner_fk", "user", ["owner"], ["id"], ondelete="NO ACTION") + + op.execute("INSERT INTO USER (id, username, is_admin) VALUES (1, 'root', TRUE)") + op.execute("UPDATE incarnation SET owner = 1") + + with op.batch_alter_table("incarnation") as batch_op: + batch_op.alter_column("owner", nullable=False) + + +def downgrade() -> None: + op.drop_constraint("change_user_fk", "incarnation", type_="foreignkey") + op.drop_column("incarnation", "owner") + op.drop_constraint("incarnation_owner_fk", "change", type_="foreignkey") + op.drop_column("change", "initialized_by") + op.drop_table("user_incarnation_permission") + op.drop_table("group_incarnation_permission") + op.drop_table("group_user") + op.drop_table("user") + op.drop_table("group") diff --git a/src/foxops/database/schema.py b/src/foxops/database/schema.py index ddcffe61..f8a82d85 100644 --- a/src/foxops/database/schema.py +++ b/src/foxops/database/schema.py @@ -1,7 +1,10 @@ +import enum + from sqlalchemy import ( Boolean, Column, DateTime, + Enum, ForeignKey, Integer, MetaData, @@ -10,6 +13,12 @@ UniqueConstraint, ) + +class Permission(enum.Enum): + READ = "read" + WRITE = "write" + + meta = MetaData() # Database schemas @@ -20,6 +29,7 @@ Column("incarnation_repository", String, nullable=False), Column("target_directory", String, nullable=False), Column("template_repository", String, nullable=False), + Column("owner", Integer, ForeignKey("user.id", ondelete="noaction"), nullable=False), UniqueConstraint("incarnation_repository", "target_directory", name="incarnation_identity"), ) @@ -37,8 +47,50 @@ Column("template_data_full", String, nullable=False), Column("commit_sha", String, nullable=False), Column("commit_pushed", Boolean, nullable=False), + Column("initialized_by", Integer, ForeignKey("user.id", ondelete="setnull"), nullable=True), # fields for merge request changes Column("merge_request_id", String), Column("merge_request_branch_name", String), UniqueConstraint("incarnation_id", "revision", name="change_incarnation_revision"), ) + + +group = Table( + "group", + meta, + Column("id", Integer, primary_key=True), + Column("system_name", String, nullable=False, unique=True), + Column("display_name", String, nullable=False), +) + +user = Table( + "user", + meta, + Column("id", Integer, primary_key=True), + Column("username", String, nullable=False, unique=True), + Column("is_admin", Boolean, nullable=False, default=False), +) + +group_user = Table( + "group_user", + meta, + Column("group_id", Integer, ForeignKey("group.id", ondelete="CASCADE"), primary_key=True), + Column("user_id", Integer, ForeignKey("user.id", ondelete="CASCADE"), primary_key=True), +) + +user_incarnation_permission = Table( + "user_incarnation_permission", + meta, + Column("user_id", Integer, ForeignKey("user.id", ondelete="CASCADE"), primary_key=True), + Column("incarnation_id", Integer, ForeignKey("incarnation.id", ondelete="CASCADE"), primary_key=True), + Column("type", Enum(Permission), nullable=False), +) + + +group_incarnation_permission = Table( + "group_incarnation_permission", + meta, + Column("group_id", Integer, ForeignKey("group.id", ondelete="CASCADE"), primary_key=True), + Column("incarnation_id", Integer, ForeignKey("incarnation.id", ondelete="CASCADE"), primary_key=True), + Column("type", Enum(Permission), nullable=False), +) From 1160c7b7c1a08a98a5bf81dd19ae095be9ec9c15 Mon Sep 17 00:00:00 2001 From: Yannick Mueller Date: Wed, 26 Mar 2025 11:11:11 +0100 Subject: [PATCH 02/23] Removed reserved keywords from database schema --- ...b248c_implemented_user_and_groups_model.py | 22 +++++++++---------- src/foxops/database/schema.py | 16 +++++++------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/alembic/versions/2025_03_26-5ae180bb248c_implemented_user_and_groups_model.py b/alembic/versions/2025_03_26-5ae180bb248c_implemented_user_and_groups_model.py index 8aaeab03..329abf26 100644 --- a/alembic/versions/2025_03_26-5ae180bb248c_implemented_user_and_groups_model.py +++ b/alembic/versions/2025_03_26-5ae180bb248c_implemented_user_and_groups_model.py @@ -19,7 +19,7 @@ def upgrade() -> None: op.create_table( - "group", + "foxops_group", sa.Column("id", sa.Integer(), nullable=False), sa.Column("system_name", sa.String(), nullable=False), sa.Column("display_name", sa.String(), nullable=False), @@ -27,7 +27,7 @@ def upgrade() -> None: sa.UniqueConstraint("system_name"), ) op.create_table( - "user", + "foxops_user", sa.Column("id", sa.Integer(), nullable=False), sa.Column("username", sa.String(), nullable=False), sa.Column("is_admin", sa.Boolean(), nullable=False), @@ -38,8 +38,8 @@ def upgrade() -> None: "group_user", sa.Column("group_id", sa.Integer(), nullable=False), sa.Column("user_id", sa.Integer(), nullable=False), - sa.ForeignKeyConstraint(["group_id"], ["group.id"], ondelete="CASCADE"), - sa.ForeignKeyConstraint(["user_id"], ["user.id"], ondelete="CASCADE"), + sa.ForeignKeyConstraint(["group_id"], ["foxops_group.id"], ondelete="CASCADE"), + sa.ForeignKeyConstraint(["user_id"], ["foxops_user.id"], ondelete="CASCADE"), sa.PrimaryKeyConstraint("group_id", "user_id"), ) op.create_table( @@ -47,7 +47,7 @@ def upgrade() -> None: sa.Column("group_id", sa.Integer(), nullable=False), sa.Column("incarnation_id", sa.Integer(), nullable=False), sa.Column("type", sa.Enum("READ", "WRITE", name="permission"), nullable=False), - sa.ForeignKeyConstraint(["group_id"], ["group.id"], ondelete="CASCADE"), + sa.ForeignKeyConstraint(["group_id"], ["foxops_group.id"], ondelete="CASCADE"), sa.ForeignKeyConstraint(["incarnation_id"], ["incarnation.id"], ondelete="CASCADE"), sa.PrimaryKeyConstraint("group_id", "incarnation_id"), ) @@ -57,18 +57,18 @@ def upgrade() -> None: sa.Column("incarnation_id", sa.Integer(), nullable=False), sa.Column("type", sa.Enum("READ", "WRITE", name="permission"), nullable=False), sa.ForeignKeyConstraint(["incarnation_id"], ["incarnation.id"], ondelete="CASCADE"), - sa.ForeignKeyConstraint(["user_id"], ["user.id"], ondelete="CASCADE"), + sa.ForeignKeyConstraint(["user_id"], ["foxops_user.id"], ondelete="CASCADE"), sa.PrimaryKeyConstraint("user_id", "incarnation_id"), ) with op.batch_alter_table("change") as batch_op: batch_op.add_column(sa.Column("initialized_by", sa.Integer(), nullable=True)) - batch_op.create_foreign_key("change_user_fk", "user", ["initialized_by"], ["id"], ondelete="SET NULL") + batch_op.create_foreign_key("change_user_fk", "foxops_user", ["initialized_by"], ["id"], ondelete="SET NULL") with op.batch_alter_table("incarnation") as batch_op: batch_op.add_column(sa.Column("owner", sa.Integer(), nullable=True)) - batch_op.create_foreign_key("incarnation_owner_fk", "user", ["owner"], ["id"], ondelete="NO ACTION") + batch_op.create_foreign_key("incarnation_owner_fk", "foxops_user", ["owner"], ["id"], ondelete="NO ACTION") - op.execute("INSERT INTO USER (id, username, is_admin) VALUES (1, 'root', TRUE)") + op.execute("INSERT INTO foxops_user (id, username, is_admin) VALUES (1, 'root', TRUE)") op.execute("UPDATE incarnation SET owner = 1") with op.batch_alter_table("incarnation") as batch_op: @@ -83,5 +83,5 @@ def downgrade() -> None: op.drop_table("user_incarnation_permission") op.drop_table("group_incarnation_permission") op.drop_table("group_user") - op.drop_table("user") - op.drop_table("group") + op.drop_table("foxops_user") + op.drop_table("foxops_group") diff --git a/src/foxops/database/schema.py b/src/foxops/database/schema.py index f8a82d85..ba6a713d 100644 --- a/src/foxops/database/schema.py +++ b/src/foxops/database/schema.py @@ -29,7 +29,7 @@ class Permission(enum.Enum): Column("incarnation_repository", String, nullable=False), Column("target_directory", String, nullable=False), Column("template_repository", String, nullable=False), - Column("owner", Integer, ForeignKey("user.id", ondelete="noaction"), nullable=False), + Column("owner", Integer, ForeignKey("foxops_user.id", ondelete="noaction"), nullable=False), UniqueConstraint("incarnation_repository", "target_directory", name="incarnation_identity"), ) @@ -47,7 +47,7 @@ class Permission(enum.Enum): Column("template_data_full", String, nullable=False), Column("commit_sha", String, nullable=False), Column("commit_pushed", Boolean, nullable=False), - Column("initialized_by", Integer, ForeignKey("user.id", ondelete="setnull"), nullable=True), + Column("initialized_by", Integer, ForeignKey("foxops_user.id", ondelete="setnull"), nullable=True), # fields for merge request changes Column("merge_request_id", String), Column("merge_request_branch_name", String), @@ -56,7 +56,7 @@ class Permission(enum.Enum): group = Table( - "group", + "foxops_group", meta, Column("id", Integer, primary_key=True), Column("system_name", String, nullable=False, unique=True), @@ -64,7 +64,7 @@ class Permission(enum.Enum): ) user = Table( - "user", + "foxops_user", meta, Column("id", Integer, primary_key=True), Column("username", String, nullable=False, unique=True), @@ -74,14 +74,14 @@ class Permission(enum.Enum): group_user = Table( "group_user", meta, - Column("group_id", Integer, ForeignKey("group.id", ondelete="CASCADE"), primary_key=True), - Column("user_id", Integer, ForeignKey("user.id", ondelete="CASCADE"), primary_key=True), + Column("group_id", Integer, ForeignKey("foxops_group.id", ondelete="CASCADE"), primary_key=True), + Column("user_id", Integer, ForeignKey("foxops_user.id", ondelete="CASCADE"), primary_key=True), ) user_incarnation_permission = Table( "user_incarnation_permission", meta, - Column("user_id", Integer, ForeignKey("user.id", ondelete="CASCADE"), primary_key=True), + Column("user_id", Integer, ForeignKey("foxops_user.id", ondelete="CASCADE"), primary_key=True), Column("incarnation_id", Integer, ForeignKey("incarnation.id", ondelete="CASCADE"), primary_key=True), Column("type", Enum(Permission), nullable=False), ) @@ -90,7 +90,7 @@ class Permission(enum.Enum): group_incarnation_permission = Table( "group_incarnation_permission", meta, - Column("group_id", Integer, ForeignKey("group.id", ondelete="CASCADE"), primary_key=True), + Column("group_id", Integer, ForeignKey("foxops_group.id", ondelete="CASCADE"), primary_key=True), Column("incarnation_id", Integer, ForeignKey("incarnation.id", ondelete="CASCADE"), primary_key=True), Column("type", Enum(Permission), nullable=False), ) From 8cdfe4e2a5eb6e6f244e72a115c0325e26e52319 Mon Sep 17 00:00:00 2001 From: Yannick Mueller Date: Wed, 26 Mar 2025 11:25:13 +0100 Subject: [PATCH 03/23] Automatically create user/group on request * Add User/Group header * Automatically create user if it doesn't exist * Automatically create group if it doesn't exist * Automatically update relationship between user and group --- src/foxops/__main__.py | 11 ++- .../database/repositories/group/__init__.py | 0 .../database/repositories/group/errors.py | 6 ++ .../database/repositories/group/model.py | 8 ++ .../database/repositories/group/repository.py | 52 ++++++++++ .../database/repositories/user/__init__.py | 0 .../database/repositories/user/errors.py | 6 ++ .../database/repositories/user/model.py | 8 ++ .../database/repositories/user/repository.py | 56 +++++++++++ src/foxops/database/schema.py | 4 +- src/foxops/dependencies.py | 99 +++++++++++++++++++ src/foxops/routers/auth.py | 12 ++- src/foxops/services/group.py | 38 +++++++ src/foxops/services/user.py | 50 ++++++++++ 14 files changed, 344 insertions(+), 6 deletions(-) create mode 100644 src/foxops/database/repositories/group/__init__.py create mode 100644 src/foxops/database/repositories/group/errors.py create mode 100644 src/foxops/database/repositories/group/model.py create mode 100644 src/foxops/database/repositories/group/repository.py create mode 100644 src/foxops/database/repositories/user/__init__.py create mode 100644 src/foxops/database/repositories/user/errors.py create mode 100644 src/foxops/database/repositories/user/model.py create mode 100644 src/foxops/database/repositories/user/repository.py create mode 100644 src/foxops/services/group.py create mode 100644 src/foxops/services/user.py diff --git a/src/foxops/__main__.py b/src/foxops/__main__.py index 00eb6805..d0fae03e 100644 --- a/src/foxops/__main__.py +++ b/src/foxops/__main__.py @@ -3,7 +3,12 @@ from fastapi.staticfiles import StaticFiles from starlette.responses import FileResponse -from foxops.dependencies import get_settings, static_token_auth_scheme +from foxops.dependencies import ( + get_settings, + group_auth_scheme, + static_token_auth_scheme, + user_auth_scheme, +) from foxops.error_handlers import __error_handlers__ from foxops.logger import get_logger, setup_logging from foxops.middlewares import request_id_middleware, request_time_middleware @@ -46,7 +51,9 @@ def create_app(): public_router.include_router(auth.router) # Add routes to the protected router (authentication required) - protected_router = APIRouter(dependencies=[Depends(static_token_auth_scheme)]) + protected_router = APIRouter( + dependencies=[Depends(static_token_auth_scheme), Depends(user_auth_scheme), Depends(group_auth_scheme)] + ) protected_router.include_router(incarnations.router) app.include_router(public_router) diff --git a/src/foxops/database/repositories/group/__init__.py b/src/foxops/database/repositories/group/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/src/foxops/database/repositories/group/errors.py b/src/foxops/database/repositories/group/errors.py new file mode 100644 index 00000000..13c5a8c1 --- /dev/null +++ b/src/foxops/database/repositories/group/errors.py @@ -0,0 +1,6 @@ +class GroupNotFoundError(Exception): + pass + + +class GroupAlreadyExistsError(Exception): + pass diff --git a/src/foxops/database/repositories/group/model.py b/src/foxops/database/repositories/group/model.py new file mode 100644 index 00000000..18245b8b --- /dev/null +++ b/src/foxops/database/repositories/group/model.py @@ -0,0 +1,8 @@ +from pydantic import BaseModel, ConfigDict + + +class GroupInDB(BaseModel): + id: int + system_name: str + display_name: str + model_config = ConfigDict(from_attributes=True) diff --git a/src/foxops/database/repositories/group/repository.py b/src/foxops/database/repositories/group/repository.py new file mode 100644 index 00000000..8a22837f --- /dev/null +++ b/src/foxops/database/repositories/group/repository.py @@ -0,0 +1,52 @@ +from sqlalchemy import insert, select +from sqlalchemy.exc import IntegrityError, NoResultFound +from sqlalchemy.ext.asyncio import AsyncEngine + +from foxops.database.repositories.group.errors import ( + GroupAlreadyExistsError, + GroupNotFoundError, +) +from foxops.database.repositories.group.model import GroupInDB +from foxops.database.schema import group, group_user + + +class GroupRepository: + def __init__(self, engine: AsyncEngine) -> None: + self.engine = engine + + async def create(self, system_name: str, display_name: str) -> GroupInDB: + async with self.engine.begin() as conn: + query = insert(group).values(system_name=system_name, display_name=display_name).returning(group) + + try: + result = await conn.execute(query) + except IntegrityError as e: + raise GroupAlreadyExistsError(f"system_name={system_name}") from e + + row = result.one() + return GroupInDB.model_validate(row) + + async def get_by_system_name(self, system_name: str) -> GroupInDB: + query = select(group).where(group.c.system_name == system_name) + + async with self.engine.begin() as conn: + result = await conn.execute(query) + + try: + row = result.one() + except NoResultFound as e: + raise GroupNotFoundError(f"system_name={system_name}") from e + + return GroupInDB.model_validate(row) + + async def get_by_userid(self, user_id: int) -> list[GroupInDB]: + query = select(group).join(group_user).where(group_user.c.user_id == user_id) + groups = [] + + async with self.engine.begin() as conn: + result = await conn.execute(query) + + for row in result: + groups.append(GroupInDB.model_validate(row)) + + return groups diff --git a/src/foxops/database/repositories/user/__init__.py b/src/foxops/database/repositories/user/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/src/foxops/database/repositories/user/errors.py b/src/foxops/database/repositories/user/errors.py new file mode 100644 index 00000000..c9fd9b91 --- /dev/null +++ b/src/foxops/database/repositories/user/errors.py @@ -0,0 +1,6 @@ +class UserNotFoundError(Exception): + pass + + +class UserAlreadyExistsError(Exception): + pass diff --git a/src/foxops/database/repositories/user/model.py b/src/foxops/database/repositories/user/model.py new file mode 100644 index 00000000..575a82fc --- /dev/null +++ b/src/foxops/database/repositories/user/model.py @@ -0,0 +1,8 @@ +from pydantic import BaseModel, ConfigDict + + +class UserInDB(BaseModel): + id: int + username: str + is_admin: bool + model_config = ConfigDict(from_attributes=True) diff --git a/src/foxops/database/repositories/user/repository.py b/src/foxops/database/repositories/user/repository.py new file mode 100644 index 00000000..110da27a --- /dev/null +++ b/src/foxops/database/repositories/user/repository.py @@ -0,0 +1,56 @@ +from typing import AsyncIterator + +from sqlalchemy import insert, select +from sqlalchemy.exc import IntegrityError, NoResultFound +from sqlalchemy.ext.asyncio import AsyncEngine + +from foxops.database.repositories.user.errors import ( + UserAlreadyExistsError, + UserNotFoundError, +) +from foxops.database.repositories.user.model import UserInDB +from foxops.database.schema import group_user, user + + +class UserRepository: + def __init__(self, engine: AsyncEngine) -> None: + self.engine = engine + + async def create(self, username: str, is_admin: bool) -> UserInDB: + async with self.engine.begin() as conn: + query = insert(user).values(username=username, is_admin=is_admin).returning(user) + + try: + result = await conn.execute(query) + except IntegrityError as e: + raise UserAlreadyExistsError(f"username={username}") from e + + row = result.one() + return UserInDB.model_validate(row) + + async def get_by_username(self, username: str) -> UserInDB: + query = select(user).where(user.c.username == username) + + async with self.engine.begin() as conn: + result = await conn.execute(query) + + try: + row = result.one() + except NoResultFound as e: + raise UserNotFoundError(f"username={username}") from e + + return UserInDB.model_validate(row) + + async def remove_old_groups_from_user(self, user_id: int, group_ids_to_keep: list[int]): + async with self.engine.begin() as conn: + query = ( + group_user.delete() + .where(group_user.c.user_id == user_id) + .where(group_user.c.group_id.notin_(group_ids_to_keep)) + ) + await conn.execute(query) + + async def join_groups(self, user_id: int, group_ids: list[int]): + async with self.engine.begin() as conn: + query = insert(group_user).values([{"user_id": user_id, "group_id": group_id} for group_id in group_ids]) + await conn.execute(query) diff --git a/src/foxops/database/schema.py b/src/foxops/database/schema.py index ba6a713d..5b21075f 100644 --- a/src/foxops/database/schema.py +++ b/src/foxops/database/schema.py @@ -29,7 +29,7 @@ class Permission(enum.Enum): Column("incarnation_repository", String, nullable=False), Column("target_directory", String, nullable=False), Column("template_repository", String, nullable=False), - Column("owner", Integer, ForeignKey("foxops_user.id", ondelete="noaction"), nullable=False), + Column("owner", Integer, ForeignKey("foxops_user.id", ondelete="NO ACTION"), nullable=False), UniqueConstraint("incarnation_repository", "target_directory", name="incarnation_identity"), ) @@ -47,7 +47,7 @@ class Permission(enum.Enum): Column("template_data_full", String, nullable=False), Column("commit_sha", String, nullable=False), Column("commit_pushed", Boolean, nullable=False), - Column("initialized_by", Integer, ForeignKey("foxops_user.id", ondelete="setnull"), nullable=True), + Column("initialized_by", Integer, ForeignKey("foxops_user.id", ondelete="SET NULL"), nullable=True), # fields for merge request changes Column("merge_request_id", String), Column("merge_request_branch_name", String), diff --git a/src/foxops/dependencies.py b/src/foxops/dependencies.py index a1d0b646..ae0720ba 100644 --- a/src/foxops/dependencies.py +++ b/src/foxops/dependencies.py @@ -1,3 +1,4 @@ +import re from typing import Annotated from fastapi import Depends, HTTPException, Request, status @@ -7,13 +8,19 @@ from foxops.database.engine import create_engine from foxops.database.repositories.change.repository import ChangeRepository +from foxops.database.repositories.group.errors import GroupNotFoundError +from foxops.database.repositories.group.repository import GroupRepository from foxops.database.repositories.incarnation.repository import IncarnationRepository +from foxops.database.repositories.user.errors import UserNotFoundError +from foxops.database.repositories.user.repository import UserRepository from foxops.hosters import Hoster from foxops.hosters.gitlab import GitlabHoster from foxops.hosters.local import LocalHoster from foxops.logger import get_logger from foxops.services.change import ChangeService +from foxops.services.group import Group, GroupService from foxops.services.incarnation import IncarnationService +from foxops.services.user import User, UserService from foxops.settings import ( DatabaseSettings, GitlabHosterSettings, @@ -87,6 +94,27 @@ def get_change_repository(database_engine: AsyncEngine = Depends(get_database_en return ChangeRepository(database_engine) +def get_user_repository(database_engine: AsyncEngine = Depends(get_database_engine)) -> UserRepository: + return UserRepository(database_engine) + + +def get_group_repository(database_engine: AsyncEngine = Depends(get_database_engine)) -> GroupRepository: + return GroupRepository(database_engine) + + +def get_group_service( + group_repository: GroupRepository = Depends(get_group_repository), +) -> GroupService: + return GroupService(group_repository=group_repository) + + +def get_user_service( + user_repository: UserRepository = Depends(get_user_repository), + group_repository: GroupRepository = Depends(get_group_repository), +) -> UserService: + return UserService(user_repository=user_repository, group_repository=group_repository) + + def get_incarnation_service( incarnation_repository: IncarnationRepository = Depends(get_incarnation_repository), hoster: Hoster = Depends(get_hoster), @@ -126,4 +154,75 @@ async def __call__(self, request: Request, settings: Settings = Depends(get_sett raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Token is invalid") +class UserHeaderAuth(SecurityBase): + def __init__(self): + self.model = APIKey(**{"in": APIKeyIn.header}, name="User") + self.scheme_name = self.__class__.__name__ + + async def __call__(self) -> None: + # This class only defines the scheme, the actual validation is done in the GroupHeaderAuth class. + pass + + +class GroupHeaderAuth(SecurityBase): + def __init__(self): + self.model = APIKey(**{"in": APIKeyIn.header}, name="Groups") + self.scheme_name = self.__class__.__name__ + + async def get_user(self, user_header: str | None, user_service: UserService) -> User: + if not user_header: + raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="Missing User header") + + try: + return await user_service.get_user_by_username(user_header) + + except UserNotFoundError: + return await user_service.create_user(username=user_header) + + async def get_groups(self, group_header: str | None, group_service: GroupService) -> list[Group]: + if not group_header or group_header.strip() == "": + # Group header is optional. So it could also be empty/non existing + return [] + + groups = [] + + for group in group_header.split(","): + group = group.strip() + + if not re.match(r"^[a-zA-Z0-9_\-]+$", group): + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Group names must be alphanumeric and may contain underscores and dashes", + ) + + try: + groups.append(await group_service.get_group_by_system_name(group)) + except GroupNotFoundError: + groups.append(await group_service.create_group(system_name=group, display_name=group)) + + return groups + + async def user_join_groups(self, user: User, groups: list[Group], user_service: UserService) -> None: + await user_service.join_groups(user.username, [group.id for group in groups], remove_old_ref=True) + + async def __call__( + self, + request: Request, + group_service: GroupService = Depends(get_group_service), + user_service: UserService = Depends(get_user_service), + ) -> None: + user_header: str | None = request.headers.get("User") + + user = await self.get_user(user_header, user_service) + + group_header: str | None = request.headers.get("Groups") + + groups = await self.get_groups(group_header, group_service) + + if len(groups) > 0: + await self.user_join_groups(user, groups, user_service) + + static_token_auth_scheme = StaticTokenHeaderAuth() +user_auth_scheme = UserHeaderAuth() +group_auth_scheme = GroupHeaderAuth() diff --git a/src/foxops/routers/auth.py b/src/foxops/routers/auth.py index bc400807..a83fecab 100644 --- a/src/foxops/routers/auth.py +++ b/src/foxops/routers/auth.py @@ -1,12 +1,20 @@ from fastapi import APIRouter, Depends from fastapi.responses import PlainTextResponse -from foxops.dependencies import static_token_auth_scheme +from foxops.dependencies import ( + group_auth_scheme, + static_token_auth_scheme, + user_auth_scheme, +) #: Holds the router for the version endpoint router = APIRouter(prefix="/auth", tags=["authentication"]) -@router.get("/test", response_class=PlainTextResponse, dependencies=[Depends(static_token_auth_scheme)]) +@router.get( + "/test", + response_class=PlainTextResponse, + dependencies=[Depends(static_token_auth_scheme), Depends(user_auth_scheme), Depends(group_auth_scheme)], +) def test_authentication_route(): return "OK" diff --git a/src/foxops/services/group.py b/src/foxops/services/group.py new file mode 100644 index 00000000..1b97910b --- /dev/null +++ b/src/foxops/services/group.py @@ -0,0 +1,38 @@ +from typing import Optional + +from pydantic import BaseModel, ConfigDict + +from foxops.database.repositories.group.repository import GroupRepository + + +class Group(BaseModel): + id: int + system_name: str + display_name: str + + model_config = ConfigDict(from_attributes=True) + + +class GroupService: + def __init__(self, group_repository: GroupRepository) -> None: + self.group_repository = group_repository + + async def create_group( + self, + system_name: str, + display_name: Optional[str] = None, + ) -> Group: + if display_name is None: + display_name = system_name + + group = await self.group_repository.create( + system_name=system_name, + display_name=display_name, + ) + + return Group.model_validate(group) + + async def get_group_by_system_name(self, system_name: str) -> Group: + group = await self.group_repository.get_by_system_name(system_name) + + return Group.model_validate(group) diff --git a/src/foxops/services/user.py b/src/foxops/services/user.py new file mode 100644 index 00000000..1e746451 --- /dev/null +++ b/src/foxops/services/user.py @@ -0,0 +1,50 @@ +from pydantic import BaseModel, ConfigDict + +from foxops.database.repositories.group.repository import GroupRepository +from foxops.database.repositories.user.repository import UserRepository + + +class User(BaseModel): + id: int + username: str + is_admin: bool + + model_config = ConfigDict(from_attributes=True) + + +class UserService: + def __init__(self, user_repository: UserRepository, group_repository: GroupRepository) -> None: + self.user_repository = user_repository + self.group_repository = group_repository + + async def create_user( + self, + username: str, + is_admin: bool = False, + ) -> User: + user = await self.user_repository.create( + username=username, + is_admin=is_admin, + ) + + return User.model_validate(user) + + async def get_user_by_username(self, username: str) -> User: + user = await self.user_repository.get_by_username(username) + + return User.model_validate(user) + + async def join_groups(self, username: str, group_ids: list[int], remove_old_ref: bool = False) -> User: + user = await self.user_repository.get_by_username(username) + + if remove_old_ref: + await self.user_repository.remove_old_groups_from_user(user.id, group_ids) + + existing_groups = [group.id for group in await self.group_repository.get_by_userid(user.id)] + + not_joined_groups = [group_id for group_id in group_ids if group_id not in existing_groups] + + if len(not_joined_groups) > 0: + await self.user_repository.join_groups(user.id, not_joined_groups) + + return User.model_validate(user) From 39291300e951a7e37c5589ac1be991328b0e1462 Mon Sep 17 00:00:00 2001 From: Yannick Mueller Date: Wed, 26 Mar 2025 13:07:04 +0100 Subject: [PATCH 04/23] Implement Authorization service --- src/foxops/__main__.py | 6 ++---- .../database/repositories/user/repository.py | 5 +++++ src/foxops/dependencies.py | 19 +++++++++++++++++-- src/foxops/routers/auth.py | 6 ++---- src/foxops/services/authorization.py | 18 ++++++++++++++++++ src/foxops/services/user.py | 14 +++++++++++++- 6 files changed, 57 insertions(+), 11 deletions(-) create mode 100644 src/foxops/services/authorization.py diff --git a/src/foxops/__main__.py b/src/foxops/__main__.py index d0fae03e..4cda1f05 100644 --- a/src/foxops/__main__.py +++ b/src/foxops/__main__.py @@ -5,9 +5,7 @@ from foxops.dependencies import ( get_settings, - group_auth_scheme, - static_token_auth_scheme, - user_auth_scheme, + authorization ) from foxops.error_handlers import __error_handlers__ from foxops.logger import get_logger, setup_logging @@ -52,7 +50,7 @@ def create_app(): # Add routes to the protected router (authentication required) protected_router = APIRouter( - dependencies=[Depends(static_token_auth_scheme), Depends(user_auth_scheme), Depends(group_auth_scheme)] + dependencies=[Depends(authorization)] ) protected_router.include_router(incarnations.router) diff --git a/src/foxops/database/repositories/user/repository.py b/src/foxops/database/repositories/user/repository.py index 110da27a..745a9d39 100644 --- a/src/foxops/database/repositories/user/repository.py +++ b/src/foxops/database/repositories/user/repository.py @@ -54,3 +54,8 @@ async def join_groups(self, user_id: int, group_ids: list[int]): async with self.engine.begin() as conn: query = insert(group_user).values([{"user_id": user_id, "group_id": group_id} for group_id in group_ids]) await conn.execute(query) + + async def remove_all_groups(self, user_id: int): + async with self.engine.begin() as conn: + query = group_user.delete().where(group_user.c.user_id == user_id) + await conn.execute(query) \ No newline at end of file diff --git a/src/foxops/dependencies.py b/src/foxops/dependencies.py index ae0720ba..fbb807e5 100644 --- a/src/foxops/dependencies.py +++ b/src/foxops/dependencies.py @@ -20,7 +20,8 @@ from foxops.services.change import ChangeService from foxops.services.group import Group, GroupService from foxops.services.incarnation import IncarnationService -from foxops.services.user import User, UserService +from foxops.services.user import User, UserService, UserWithGroups +from foxops.services.authorization import AuthorizationService from foxops.settings import ( DatabaseSettings, GitlabHosterSettings, @@ -210,7 +211,7 @@ async def __call__( request: Request, group_service: GroupService = Depends(get_group_service), user_service: UserService = Depends(get_user_service), - ) -> None: + ) -> UserWithGroups: user_header: str | None = request.headers.get("User") user = await self.get_user(user_header, user_service) @@ -221,8 +222,22 @@ async def __call__( if len(groups) > 0: await self.user_join_groups(user, groups, user_service) + else: + await user_service.remove_all_groups_from_user(user.id) + + return UserWithGroups(groups=groups, **user.model_dump()) static_token_auth_scheme = StaticTokenHeaderAuth() user_auth_scheme = UserHeaderAuth() group_auth_scheme = GroupHeaderAuth() + + +def authorization( + _static_token: None = Depends(static_token_auth_scheme), + _user_schema: None = Depends(user_auth_scheme), + current_user: UserWithGroups = Depends(group_auth_scheme), + user_service: UserService = Depends(get_user_service), + group_service: GroupService = Depends(get_group_service), +) -> AuthorizationService: + return AuthorizationService(current_user=current_user, user_service=user_service, group_service=group_service) \ No newline at end of file diff --git a/src/foxops/routers/auth.py b/src/foxops/routers/auth.py index a83fecab..2d187ea4 100644 --- a/src/foxops/routers/auth.py +++ b/src/foxops/routers/auth.py @@ -2,9 +2,7 @@ from fastapi.responses import PlainTextResponse from foxops.dependencies import ( - group_auth_scheme, - static_token_auth_scheme, - user_auth_scheme, + authorization ) #: Holds the router for the version endpoint @@ -14,7 +12,7 @@ @router.get( "/test", response_class=PlainTextResponse, - dependencies=[Depends(static_token_auth_scheme), Depends(user_auth_scheme), Depends(group_auth_scheme)], + dependencies=[Depends(authorization)], ) def test_authentication_route(): return "OK" diff --git a/src/foxops/services/authorization.py b/src/foxops/services/authorization.py new file mode 100644 index 00000000..237cf37a --- /dev/null +++ b/src/foxops/services/authorization.py @@ -0,0 +1,18 @@ +from foxops.services.group import GroupService +from foxops.services.user import UserService, UserWithGroups + + +class AuthorizationService: + def __init__(self, current_user: UserWithGroups, user_service: UserService, group_service: GroupService) -> None: + self.user_service = user_service + self.group_service = group_service + self.current_user = current_user + + def is_admin(self) -> bool: + return self.current_user.is_admin + + def is_in_group(self, group_system_name_or_id: str | int) -> bool: + if isinstance(group_system_name_or_id, str): + return any(group.system_name == group_system_name_or_id for group in self.current_user.groups) + else: + return any(group.id == group_system_name_or_id for group in self.current_user.groups) \ No newline at end of file diff --git a/src/foxops/services/user.py b/src/foxops/services/user.py index 1e746451..10135602 100644 --- a/src/foxops/services/user.py +++ b/src/foxops/services/user.py @@ -2,7 +2,7 @@ from foxops.database.repositories.group.repository import GroupRepository from foxops.database.repositories.user.repository import UserRepository - +from foxops.services.group import Group class User(BaseModel): id: int @@ -11,6 +11,8 @@ class User(BaseModel): model_config = ConfigDict(from_attributes=True) +class UserWithGroups(User): + groups: list[Group] class UserService: def __init__(self, user_repository: UserRepository, group_repository: GroupRepository) -> None: @@ -48,3 +50,13 @@ async def join_groups(self, username: str, group_ids: list[int], remove_old_ref: await self.user_repository.join_groups(user.id, not_joined_groups) return User.model_validate(user) + + async def get_user_by_username_with_groups(self, username: str) -> UserWithGroups: + user = await self.user_repository.get_by_username(username) + + groups = await self.group_repository.get_by_userid(user.id) + + return UserWithGroups(groups=groups, **user.model_dump()) + + async def remove_all_groups_from_user(self, user_id: int) -> None: + await self.user_repository.remove_all_groups(user_id) \ No newline at end of file From 635213b7656426e0ca6c9ad262788c4743740410 Mon Sep 17 00:00:00 2001 From: Yannick Mueller Date: Wed, 26 Mar 2025 17:06:48 +0100 Subject: [PATCH 05/23] Adapt incarnation endpoints to new schema --- .github/workflows/ci.yml | 2 +- ...b248c_implemented_user_and_groups_model.py | 12 ++ src/foxops/__main__.py | 9 +- .../database/repositories/change/model.py | 1 + .../repositories/change/repository.py | 3 + .../database/repositories/group/errors.py | 25 +++- .../database/repositories/group/repository.py | 15 +- .../repositories/incarnation/errors.py | 4 +- .../repositories/incarnation/model.py | 21 +++ .../repositories/incarnation/repository.py | 110 +++++++++++++- .../database/repositories/user/errors.py | 20 ++- .../database/repositories/user/repository.py | 21 ++- src/foxops/database/schema.py | 5 + src/foxops/dependencies.py | 35 +++-- src/foxops/models/group.py | 9 ++ src/foxops/models/incarnation.py | 59 +++++++- src/foxops/models/user.py | 15 ++ src/foxops/routers/auth.py | 4 +- src/foxops/routers/incarnations.py | 137 +++++++++++++++--- src/foxops/services/authorization.py | 9 +- src/foxops/services/change.py | 66 ++++++--- src/foxops/services/group.py | 11 +- src/foxops/services/incarnation.py | 78 +++++++++- src/foxops/services/user.py | 17 +-- tests/conftest.py | 32 ++++ .../database/migrations/test_00ee97d0b7a3.py | 13 -- tests/database/test_change_repository.py | 26 +++- tests/database/test_incarnation_repository.py | 23 ++- tests/e2e/conftest.py | 1 + tests/e2e/test_incarnations_api.py | 19 +++ tests/routers/test_auth.py | 5 + tests/routers/test_incarnations.py | 26 +++- tests/services/test_change.py | 40 ++++- 33 files changed, 721 insertions(+), 152 deletions(-) create mode 100644 src/foxops/models/group.py create mode 100644 src/foxops/models/user.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8b8eb941..c475c3b2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -73,7 +73,7 @@ jobs: fail-fast: false matrix: python-version: ["3.12"] - gitlab-version: ["17.2.8-ce.0"] + gitlab-version: ["17.2.8-ce.0", "17.9.3-ce.0"] env: GITLAB_CE_VERSION: ${{ matrix.gitlab-version }} diff --git a/alembic/versions/2025_03_26-5ae180bb248c_implemented_user_and_groups_model.py b/alembic/versions/2025_03_26-5ae180bb248c_implemented_user_and_groups_model.py index 329abf26..5fe120e0 100644 --- a/alembic/versions/2025_03_26-5ae180bb248c_implemented_user_and_groups_model.py +++ b/alembic/versions/2025_03_26-5ae180bb248c_implemented_user_and_groups_model.py @@ -74,6 +74,16 @@ def upgrade() -> None: with op.batch_alter_table("incarnation") as batch_op: batch_op.alter_column("owner", nullable=False) + op.create_index( + "ix_group_permission_incarnation_id", "group_incarnation_permission", ["incarnation_id"], unique=False + ) + op.create_index( + "ix_user_permission_incarnation_id", "user_incarnation_permission", ["incarnation_id"], unique=False + ) + + op.create_index("ix_user_username", "foxops_user", ["username"]) + op.create_index("ix_group_system_name", "foxops_group", ["system_name"]) + def downgrade() -> None: op.drop_constraint("change_user_fk", "incarnation", type_="foreignkey") @@ -85,3 +95,5 @@ def downgrade() -> None: op.drop_table("group_user") op.drop_table("foxops_user") op.drop_table("foxops_group") + op.drop_index("ix_group_permission_incarnation_id", table_name="group_incarnation_permission") + op.drop_index("ix_user_permission_incarnation_id", table_name="user_incarnation_permission") diff --git a/src/foxops/__main__.py b/src/foxops/__main__.py index 4cda1f05..25bb441e 100644 --- a/src/foxops/__main__.py +++ b/src/foxops/__main__.py @@ -3,10 +3,7 @@ from fastapi.staticfiles import StaticFiles from starlette.responses import FileResponse -from foxops.dependencies import ( - get_settings, - authorization -) +from foxops.dependencies import authorization, get_settings from foxops.error_handlers import __error_handlers__ from foxops.logger import get_logger, setup_logging from foxops.middlewares import request_id_middleware, request_time_middleware @@ -49,9 +46,7 @@ def create_app(): public_router.include_router(auth.router) # Add routes to the protected router (authentication required) - protected_router = APIRouter( - dependencies=[Depends(authorization)] - ) + protected_router = APIRouter(dependencies=[Depends(authorization)]) protected_router.include_router(incarnations.router) app.include_router(public_router) diff --git a/src/foxops/database/repositories/change/model.py b/src/foxops/database/repositories/change/model.py index 527ebf2e..b788e69f 100644 --- a/src/foxops/database/repositories/change/model.py +++ b/src/foxops/database/repositories/change/model.py @@ -55,4 +55,5 @@ class IncarnationWithChangesSummary(BaseModel): requested_version: str merge_request_id: str | None created_at: datetime + owner: int model_config = ConfigDict(from_attributes=True) diff --git a/src/foxops/database/repositories/change/repository.py b/src/foxops/database/repositories/change/repository.py index fda39b1f..f8452a6a 100644 --- a/src/foxops/database/repositories/change/repository.py +++ b/src/foxops/database/repositories/change/repository.py @@ -90,6 +90,7 @@ async def create_incarnation_with_first_change( requested_version: str, requested_data: str, template_data_full: str, + owner_id: int, ) -> ChangeInDB: logger = self.log.bind(function="create_incarnation_with_first_change") logger.debug( @@ -106,6 +107,7 @@ async def create_incarnation_with_first_change( incarnation_repository=incarnation_repository, target_directory=target_directory, template_repository=template_repository, + owner=owner_id, ) .returning(incarnations.c.id) ) @@ -193,6 +195,7 @@ def _incarnations_with_changes_summary_query(self): incarnations.c.incarnation_repository, incarnations.c.target_directory, incarnations.c.template_repository, + incarnations.c.owner, alias_change.c.revision, alias_change.c.type, alias_change.c.requested_version, diff --git a/src/foxops/database/repositories/group/errors.py b/src/foxops/database/repositories/group/errors.py index 13c5a8c1..40289616 100644 --- a/src/foxops/database/repositories/group/errors.py +++ b/src/foxops/database/repositories/group/errors.py @@ -1,6 +1,23 @@ -class GroupNotFoundError(Exception): - pass +from typing import Optional +from foxops.errors import FoxopsError -class GroupAlreadyExistsError(Exception): - pass + +class GroupNotFoundError(FoxopsError): + def __init__(self, system_name: Optional[str] = None, id: Optional[int] = None): + if system_name is None and id is None: + raise ValueError("system_name or id must be provided") + if system_name is not None and id is not None: + raise ValueError("system_name and id cannot both be provided") + if system_name is not None: + self.system_name = system_name + super().__init__(f"Group with system name '{system_name}' not found.") + else: + self.id = id + super().__init__(f"Group with id '{id}' not found.") + + +class GroupAlreadyExistsError(FoxopsError): + def __init__(self, system_name: str): + self.system_name = system_name + super().__init__(f"Group with System name ${system_name} already exists") diff --git a/src/foxops/database/repositories/group/repository.py b/src/foxops/database/repositories/group/repository.py index 8a22837f..ee8f7139 100644 --- a/src/foxops/database/repositories/group/repository.py +++ b/src/foxops/database/repositories/group/repository.py @@ -21,7 +21,7 @@ async def create(self, system_name: str, display_name: str) -> GroupInDB: try: result = await conn.execute(query) except IntegrityError as e: - raise GroupAlreadyExistsError(f"system_name={system_name}") from e + raise GroupAlreadyExistsError(system_name) from e row = result.one() return GroupInDB.model_validate(row) @@ -35,7 +35,7 @@ async def get_by_system_name(self, system_name: str) -> GroupInDB: try: row = result.one() except NoResultFound as e: - raise GroupNotFoundError(f"system_name={system_name}") from e + raise GroupNotFoundError(system_name=system_name) from e return GroupInDB.model_validate(row) @@ -50,3 +50,14 @@ async def get_by_userid(self, user_id: int) -> list[GroupInDB]: groups.append(GroupInDB.model_validate(row)) return groups + + async def get_by_id(self, group_id: int) -> GroupInDB: + query = select(group).where(group.c.id == group_id) + + async with self.engine.begin() as conn: + result = await conn.execute(query) + try: + row = result.one() + except NoResultFound as e: + raise GroupNotFoundError(id=group_id) from e + return GroupInDB.model_validate(row) diff --git a/src/foxops/database/repositories/incarnation/errors.py b/src/foxops/database/repositories/incarnation/errors.py index ffb1914c..4798023d 100644 --- a/src/foxops/database/repositories/incarnation/errors.py +++ b/src/foxops/database/repositories/incarnation/errors.py @@ -1,5 +1,7 @@ class IncarnationNotFoundError(Exception): - pass + def __init__(self, id: int): + self.id = id + super().__init__(f"Incarnation with id '{id}' not found.") class IncarnationAlreadyExistsError(Exception): diff --git a/src/foxops/database/repositories/incarnation/model.py b/src/foxops/database/repositories/incarnation/model.py index 476dead0..48cdb25a 100644 --- a/src/foxops/database/repositories/incarnation/model.py +++ b/src/foxops/database/repositories/incarnation/model.py @@ -1,9 +1,30 @@ from pydantic import BaseModel, ConfigDict +from foxops.database.schema import Permission + class IncarnationInDB(BaseModel): id: int incarnation_repository: str target_directory: str template_repository: str + owner: int + model_config = ConfigDict(from_attributes=True) + + +class GroupPermissionInDB(BaseModel): + group_id: int + group_system_name: str + group_display_name: str + incarnation_id: int + type: Permission + model_config = ConfigDict(from_attributes=True) + + +class UserPermissionInDB(BaseModel): + user_id: int + user_username: str + user_is_admin: bool + incarnation_id: int + type: Permission model_config = ConfigDict(from_attributes=True) diff --git a/src/foxops/database/repositories/incarnation/repository.py b/src/foxops/database/repositories/incarnation/repository.py index 37e438ce..bbfba764 100644 --- a/src/foxops/database/repositories/incarnation/repository.py +++ b/src/foxops/database/repositories/incarnation/repository.py @@ -1,6 +1,6 @@ -from typing import AsyncIterator +from typing import AsyncIterator, List -from sqlalchemy import delete, insert, select +from sqlalchemy import delete, insert, select, update from sqlalchemy.exc import IntegrityError, NoResultFound from sqlalchemy.ext.asyncio import AsyncEngine @@ -8,8 +8,20 @@ IncarnationAlreadyExistsError, IncarnationNotFoundError, ) -from foxops.database.repositories.incarnation.model import IncarnationInDB -from foxops.database.schema import incarnations +from foxops.database.repositories.incarnation.model import ( + GroupPermissionInDB, + IncarnationInDB, + UserPermissionInDB, +) +from foxops.database.repositories.user.errors import UserNotFoundError +from foxops.database.schema import ( + group, + group_incarnation_permission, + incarnations, + user, + user_incarnation_permission, +) +from foxops.models.incarnation import GroupPermission, UserPermission class IncarnationRepository: @@ -21,6 +33,7 @@ async def create( incarnation_repository: str, target_directory: str, template_repository: str, + owner_id: int, ) -> IncarnationInDB: async with self.engine.begin() as conn: query = ( @@ -29,6 +42,7 @@ async def create( incarnation_repository=incarnation_repository, target_directory=target_directory, template_repository=template_repository, + owner=owner_id, ) .returning(incarnations) ) @@ -59,7 +73,7 @@ async def get_by_id(self, id_: int) -> IncarnationInDB: try: row = result.one() except NoResultFound: - raise IncarnationNotFoundError(f"could not find incarnation in DB with id: {id_}") + raise IncarnationNotFoundError(id_) else: return IncarnationInDB.model_validate(row) @@ -70,4 +84,88 @@ async def delete_by_id(self, id_: int) -> None: result = await conn.execute(query) if result.rowcount == 0: - raise IncarnationNotFoundError(f"could not find incarnation in DB with id: {id_}") + raise IncarnationNotFoundError(id_) + + async def get_group_permissions(self, incarnation_id: int) -> List[GroupPermissionInDB]: + query = ( + select( + group_incarnation_permission, + group.c.system_name.label("group_system_name"), + group.c.display_name.label("group_display_name"), + ) + .where(group_incarnation_permission.c.incarnation_id == incarnation_id) + .join(group) + ) + + async with self.engine.begin() as conn: + rows = await conn.execute(query) + + return [GroupPermissionInDB.model_validate(row) for row in rows] + + async def get_user_permissions(self, incarnation_id: int) -> List[UserPermissionInDB]: + query = ( + select( + user_incarnation_permission, + user.c.username.label("user_username"), + user.c.is_admin.label("user_is_admin"), + ) + .where(user_incarnation_permission.c.incarnation_id == incarnation_id) + .join(user) + ) + + async with self.engine.begin() as conn: + rows = await conn.execute(query) + return [UserPermissionInDB.model_validate(row) for row in rows] + + async def set_user_permissions(self, incarnation_id: int, user_permissions: List[UserPermission]): + query = insert(user_incarnation_permission).values( + [ + {"user_id": user_permission.user.id, "incarnation_id": incarnation_id, "type": user_permission.type} + for user_permission in user_permissions + ] + ) + async with self.engine.begin() as conn: + await conn.execute(query) + + async def set_group_permissions(self, incarnation_id: int, group_permissions: List[GroupPermission]): + query = insert(group_incarnation_permission).values( + [ + {"group_id": group_permission.group.id, "incarnation_id": incarnation_id, "type": group_permission.type} + for group_permission in group_permissions + ] + ) + async with self.engine.begin() as conn: + await conn.execute(query) + + async def remove_all_user_permissions(self, incarnation_id: int): + async with self.engine.begin() as conn: + query = user_incarnation_permission.delete().where( + user_incarnation_permission.c.incarnation_id == incarnation_id + ) + await conn.execute(query) + + async def remove_all_group_permissions(self, incarnation_id: int): + async with self.engine.begin() as conn: + query = group_incarnation_permission.delete().where( + group_incarnation_permission.c.incarnation_id == incarnation_id + ) + await conn.execute(query) + + async def set_owner(self, incarnation_id: int, user_id: int): + async with self.engine.begin() as conn: + query = ( + update(incarnations) + .where(incarnations.c.id == incarnation_id) + .values(owner=user_id) + .returning(incarnations) + ) + try: + result = await conn.execute(query) + except IntegrityError as e: + raise UserNotFoundError(id=user_id) from e + try: + row = result.one() + except NoResultFound as e: + raise IncarnationNotFoundError(incarnation_id) from e + + return IncarnationInDB.model_validate(row) diff --git a/src/foxops/database/repositories/user/errors.py b/src/foxops/database/repositories/user/errors.py index c9fd9b91..68af1ab3 100644 --- a/src/foxops/database/repositories/user/errors.py +++ b/src/foxops/database/repositories/user/errors.py @@ -1,6 +1,20 @@ -class UserNotFoundError(Exception): - pass +from typing import Optional + +from foxops.errors import FoxopsError + + +class UserNotFoundError(FoxopsError): + def __init__(self, id: Optional[int] = None, username: Optional[str] = None): + if id is None and username is None: + raise ValueError("id or username must be provided") + if id is not None and username is not None: + raise ValueError("id and username cannot both be provided") + if id is not None: + super().__init__(f"User with id '{id}' not found.") + else: + super().__init__(f"User with username '{username}' not found.") class UserAlreadyExistsError(Exception): - pass + def __init__(self, username: str): + super().__init__(f"User with username '{username}' already exists.") diff --git a/src/foxops/database/repositories/user/repository.py b/src/foxops/database/repositories/user/repository.py index 745a9d39..7b402a3a 100644 --- a/src/foxops/database/repositories/user/repository.py +++ b/src/foxops/database/repositories/user/repository.py @@ -1,5 +1,3 @@ -from typing import AsyncIterator - from sqlalchemy import insert, select from sqlalchemy.exc import IntegrityError, NoResultFound from sqlalchemy.ext.asyncio import AsyncEngine @@ -23,7 +21,7 @@ async def create(self, username: str, is_admin: bool) -> UserInDB: try: result = await conn.execute(query) except IntegrityError as e: - raise UserAlreadyExistsError(f"username={username}") from e + raise UserAlreadyExistsError(username=username) from e row = result.one() return UserInDB.model_validate(row) @@ -37,7 +35,7 @@ async def get_by_username(self, username: str) -> UserInDB: try: row = result.one() except NoResultFound as e: - raise UserNotFoundError(f"username={username}") from e + raise UserNotFoundError(username=username) from e return UserInDB.model_validate(row) @@ -58,4 +56,17 @@ async def join_groups(self, user_id: int, group_ids: list[int]): async def remove_all_groups(self, user_id: int): async with self.engine.begin() as conn: query = group_user.delete().where(group_user.c.user_id == user_id) - await conn.execute(query) \ No newline at end of file + await conn.execute(query) + + async def get_by_id(self, user_id: int) -> UserInDB: + query = select(user).where(user.c.id == user_id) + + async with self.engine.begin() as conn: + result = await conn.execute(query) + + try: + row = result.one() + except NoResultFound as e: + raise UserNotFoundError(id=user_id) from e + + return UserInDB.model_validate(row) diff --git a/src/foxops/database/schema.py b/src/foxops/database/schema.py index 5b21075f..96fedb09 100644 --- a/src/foxops/database/schema.py +++ b/src/foxops/database/schema.py @@ -6,6 +6,7 @@ DateTime, Enum, ForeignKey, + Index, Integer, MetaData, String, @@ -61,6 +62,7 @@ class Permission(enum.Enum): Column("id", Integer, primary_key=True), Column("system_name", String, nullable=False, unique=True), Column("display_name", String, nullable=False), + Index("ix_group_system_name", "system_name"), ) user = Table( @@ -69,6 +71,7 @@ class Permission(enum.Enum): Column("id", Integer, primary_key=True), Column("username", String, nullable=False, unique=True), Column("is_admin", Boolean, nullable=False, default=False), + Index("ix_user_username", "username"), ) group_user = Table( @@ -84,6 +87,7 @@ class Permission(enum.Enum): Column("user_id", Integer, ForeignKey("foxops_user.id", ondelete="CASCADE"), primary_key=True), Column("incarnation_id", Integer, ForeignKey("incarnation.id", ondelete="CASCADE"), primary_key=True), Column("type", Enum(Permission), nullable=False), + Index("ix_user_permission_incarnation_id", "incarnation_id"), ) @@ -93,4 +97,5 @@ class Permission(enum.Enum): Column("group_id", Integer, ForeignKey("foxops_group.id", ondelete="CASCADE"), primary_key=True), Column("incarnation_id", Integer, ForeignKey("incarnation.id", ondelete="CASCADE"), primary_key=True), Column("type", Enum(Permission), nullable=False), + Index("ix_group_permission_incarnation_id", "incarnation_id"), ) diff --git a/src/foxops/dependencies.py b/src/foxops/dependencies.py index fbb807e5..ed251cab 100644 --- a/src/foxops/dependencies.py +++ b/src/foxops/dependencies.py @@ -17,11 +17,13 @@ from foxops.hosters.gitlab import GitlabHoster from foxops.hosters.local import LocalHoster from foxops.logger import get_logger +from foxops.models.group import Group +from foxops.models.user import User, UserWithGroups +from foxops.services.authorization import AuthorizationService from foxops.services.change import ChangeService -from foxops.services.group import Group, GroupService +from foxops.services.group import GroupService from foxops.services.incarnation import IncarnationService -from foxops.services.user import User, UserService, UserWithGroups -from foxops.services.authorization import AuthorizationService +from foxops.services.user import UserService from foxops.settings import ( DatabaseSettings, GitlabHosterSettings, @@ -119,17 +121,28 @@ def get_user_service( def get_incarnation_service( incarnation_repository: IncarnationRepository = Depends(get_incarnation_repository), hoster: Hoster = Depends(get_hoster), + user_repository: UserRepository = Depends(get_user_repository), + group_repository: GroupRepository = Depends(get_group_repository), ) -> IncarnationService: - return IncarnationService(incarnation_repository=incarnation_repository, hoster=hoster) + return IncarnationService( + incarnation_repository=incarnation_repository, + hoster=hoster, + user_repository=user_repository, + group_repository=group_repository, + ) def get_change_service( hoster: Hoster = Depends(get_hoster), change_repository: ChangeRepository = Depends(get_change_repository), incarnation_repository: IncarnationRepository = Depends(get_incarnation_repository), + user_repository: UserRepository = Depends(get_user_repository), ) -> ChangeService: return ChangeService( - hoster=hoster, incarnation_repository=incarnation_repository, change_repository=change_repository + hoster=hoster, + incarnation_repository=incarnation_repository, + change_repository=change_repository, + user_repository=user_repository, ) @@ -234,10 +247,10 @@ async def __call__( def authorization( - _static_token: None = Depends(static_token_auth_scheme), - _user_schema: None = Depends(user_auth_scheme), - current_user: UserWithGroups = Depends(group_auth_scheme), - user_service: UserService = Depends(get_user_service), - group_service: GroupService = Depends(get_group_service), + _static_token: None = Depends(static_token_auth_scheme), + _user_schema: None = Depends(user_auth_scheme), + current_user: UserWithGroups = Depends(group_auth_scheme), + user_service: UserService = Depends(get_user_service), + group_service: GroupService = Depends(get_group_service), ) -> AuthorizationService: - return AuthorizationService(current_user=current_user, user_service=user_service, group_service=group_service) \ No newline at end of file + return AuthorizationService(current_user=current_user, user_service=user_service, group_service=group_service) diff --git a/src/foxops/models/group.py b/src/foxops/models/group.py new file mode 100644 index 00000000..6851f898 --- /dev/null +++ b/src/foxops/models/group.py @@ -0,0 +1,9 @@ +from pydantic import BaseModel, ConfigDict + + +class Group(BaseModel): + id: int + system_name: str + display_name: str + + model_config = ConfigDict(from_attributes=True) diff --git a/src/foxops/models/incarnation.py b/src/foxops/models/incarnation.py index 85c5e98b..53995492 100644 --- a/src/foxops/models/incarnation.py +++ b/src/foxops/models/incarnation.py @@ -1,8 +1,14 @@ +from datetime import datetime + from pydantic import BaseModel, ConfigDict, Field +from foxops.database.repositories.change.model import ChangeType +from foxops.database.schema import Permission from foxops.engine import TemplateData from foxops.hosters import ReconciliationStatus from foxops.hosters.types import MergeRequestStatus +from foxops.models.group import Group +from foxops.models.user import User class Incarnation(BaseModel): @@ -14,7 +20,7 @@ class Incarnation(BaseModel): template_repository: str | None commit_sha: str merge_request_id: str | None - + owner: User | None model_config = ConfigDict(from_attributes=True) @@ -32,8 +38,40 @@ class IncarnationBasic(BaseModel): model_config = ConfigDict(from_attributes=True) +class UserPermission(BaseModel): + user: User + type: Permission + + model_config = ConfigDict(from_attributes=True) + + +class GroupPermission(BaseModel): + group: Group + type: Permission + + model_config = ConfigDict(from_attributes=True) + + +class UnresolvedUserPermissions(BaseModel): + user_id: int + type: Permission + + model_config = ConfigDict(from_attributes=True) + + +class UnresolvedGroupPermissions(BaseModel): + group_id: int + type: Permission + + model_config = ConfigDict(from_attributes=True) + + class IncarnationWithDetails(IncarnationBasic): status: ReconciliationStatus = Field(description="DEPRECATED. Use the 'merge_request_status' field instead.") + owner: User + user_permissions: list[UserPermission] + group_permissions: list[GroupPermission] + merge_request_status: MergeRequestStatus | None revision: int | None @@ -44,3 +82,22 @@ class IncarnationWithDetails(IncarnationBasic): template_data_full: TemplateData | None model_config = ConfigDict(from_attributes=True) + + +class IncarnationWithLatestChangeDetails(BaseModel): + id: int + incarnation_repository: str + target_directory: str + template_repository: str + + revision: int + type: ChangeType + requested_version: str + created_at: datetime + + commit_sha: str + commit_url: str + owner: User + + merge_request_id: str | None + merge_request_url: str | None diff --git a/src/foxops/models/user.py b/src/foxops/models/user.py new file mode 100644 index 00000000..70d2e24c --- /dev/null +++ b/src/foxops/models/user.py @@ -0,0 +1,15 @@ +from pydantic import BaseModel, ConfigDict + +from foxops.models.group import Group + + +class User(BaseModel): + id: int + username: str + is_admin: bool + + model_config = ConfigDict(from_attributes=True) + + +class UserWithGroups(User): + groups: list[Group] diff --git a/src/foxops/routers/auth.py b/src/foxops/routers/auth.py index 2d187ea4..e7c487ab 100644 --- a/src/foxops/routers/auth.py +++ b/src/foxops/routers/auth.py @@ -1,9 +1,7 @@ from fastapi import APIRouter, Depends from fastapi.responses import PlainTextResponse -from foxops.dependencies import ( - authorization -) +from foxops.dependencies import authorization #: Holds the router for the version endpoint router = APIRouter(prefix="/auth", tags=["authentication"]) diff --git a/src/foxops/routers/incarnations.py b/src/foxops/routers/incarnations.py index 9fa4ed97..ff900c8d 100644 --- a/src/foxops/routers/incarnations.py +++ b/src/foxops/routers/incarnations.py @@ -3,16 +3,29 @@ from fastapi import APIRouter, Depends, Response, status from pydantic import BaseModel, model_validator +from foxops.database.repositories.group.errors import GroupNotFoundError from foxops.database.repositories.incarnation.errors import IncarnationNotFoundError -from foxops.dependencies import get_change_service, get_hoster, get_incarnation_service +from foxops.database.repositories.user.errors import UserNotFoundError +from foxops.dependencies import ( + authorization, + get_change_service, + get_hoster, + get_incarnation_service, +) from foxops.engine import TemplateData from foxops.engine.errors import ProvidedTemplateDataInvalidError from foxops.errors import IncarnationNotFoundError as IncarnationNotFoundLegacyError from foxops.hosters import Hoster from foxops.logger import bind, get_logger -from foxops.models import IncarnationBasic, IncarnationWithDetails +from foxops.models import IncarnationWithDetails from foxops.models.errors import ApiError +from foxops.models.incarnation import ( + IncarnationWithLatestChangeDetails, + UnresolvedGroupPermissions, + UnresolvedUserPermissions, +) from foxops.routers import changes +from foxops.services.authorization import AuthorizationService from foxops.services.change import ( ChangeRejectedDueToNoChanges, ChangeRejectedDueToPreviousUnfinishedChange, @@ -34,7 +47,7 @@ responses={ status.HTTP_200_OK: { "description": "The list of incarnations in the inventory", - "model": list[IncarnationBasic], + "model": list[IncarnationWithLatestChangeDetails], }, status.HTTP_400_BAD_REQUEST: { "description": "The `incarnation_repository` and `target_directory` settings where inconsistent", @@ -100,11 +113,13 @@ class CreateIncarnationRequest(BaseModel): "model": ApiError, }, }, + status_code=status.HTTP_201_CREATED, ) async def create_incarnation( response: Response, request: CreateIncarnationRequest, change_service: ChangeService = Depends(get_change_service), + authorization_serive: AuthorizationService = Depends(authorization), ): """Initializes a new incarnation and adds it to the inventory. @@ -122,6 +137,7 @@ async def create_incarnation( template_repository=request.template_repository, template_repository_version=request.template_repository_version, template_data=template_data, + owner_id=authorization_serive.current_user.id, ) except ProvidedTemplateDataInvalidError as e: response.status_code = status.HTTP_400_BAD_REQUEST @@ -216,9 +232,9 @@ async def reset_incarnation( message=f"could not initialize the incarnation as the provided template data " f"is invalid: {'; '.join(error_messages)}" ) - except IncarnationNotFoundError: + except IncarnationNotFoundError as exc: response.status_code = status.HTTP_404_NOT_FOUND - return ApiError(message="The incarnation was not found in the inventory") + return ApiError(message=str(exc)) except ChangeRejectedDueToNoChanges: response.status_code = status.HTTP_422_UNPROCESSABLE_ENTITY return ApiError(message="The incarnation does not have any customizations. Nothing to reset.") @@ -279,6 +295,10 @@ class UpdateIncarnationRequest(BaseModel): template_repository_version: str template_data: TemplateData + owner_id: int + + user_permissions: list[UnresolvedUserPermissions] + group_permissions: list[UnresolvedGroupPermissions] automerge: bool @@ -313,6 +333,7 @@ async def update_incarnation( incarnation_id: int, request: UpdateIncarnationRequest, change_service: ChangeService = Depends(get_change_service), + incarnation_service: IncarnationService = Depends(get_incarnation_service), ): """Updates the incarnation to the given version and data. @@ -321,6 +342,34 @@ async def update_incarnation( reusing the previously set variable values), use the PATCH endpoint instead. """ + await incarnation_service.remove_all_permissions(incarnation_id) + try: + await incarnation_service.set_user_permissions(incarnation_id, request.user_permissions) + except UserNotFoundError as exc: + response.status_code = status.HTTP_404_NOT_FOUND + return ApiError(message=str(exc)) + except IncarnationNotFoundError as exc: + response.status_code = status.HTTP_404_NOT_FOUND + return ApiError(message=str(exc)) + + try: + await incarnation_service.set_group_permissions(incarnation_id, request.group_permissions) + except GroupNotFoundError as exc: + response.status_code = status.HTTP_404_NOT_FOUND + return ApiError(message=str(exc)) + except IncarnationNotFoundError as exc: + response.status_code = status.HTTP_404_NOT_FOUND + return ApiError(message=str(exc)) + + try: + await incarnation_service.set_owner(incarnation_id, request.owner_id) + except UserNotFoundError as exc: + response.status_code = status.HTTP_404_NOT_FOUND + return ApiError(message=str(exc)) + except IncarnationNotFoundError as exc: + response.status_code = status.HTTP_404_NOT_FOUND + return ApiError(message=str(exc)) + return await _create_change( incarnation_id=incarnation_id, requested_version=request.template_repository_version, @@ -337,13 +386,27 @@ class PatchIncarnationRequest(BaseModel): requested_version: str | None = None requested_data: TemplateData | None = None + user_permissions: list[UnresolvedUserPermissions] | None = None + group_permissions: list[UnresolvedGroupPermissions] | None = None + owner_id: int | None = None - automerge: bool + automerge: bool | None = None @model_validator(mode="after") def check_either_version_or_data_change_requested(self) -> Self: - if self.requested_version is None and self.requested_data is None: - raise ValueError("Either requested_version or requested_data must be set") + if ( + self.requested_version is None + and self.requested_data is None + and self.user_permissions is None + and self.group_permissions is None + and self.owner_id is None + ): + raise ValueError( + "One of the following fields must be set: requested_version, requested_data, user_permission, group_permission" + ) + + if (self.requested_data is not None or self.requested_version is not None) and self.automerge is None: + raise ValueError("The 'automerge' field must be set if a change is requested") return self @@ -378,6 +441,7 @@ async def patch_incarnation( incarnation_id: int, request: PatchIncarnationRequest, change_service: ChangeService = Depends(get_change_service), + incarnation_service: IncarnationService = Depends(get_incarnation_service), ): """Updates the incarnation to the given version and data. @@ -387,15 +451,54 @@ async def patch_incarnation( requested_data = request.requested_data or {} - return await _create_change( - incarnation_id=incarnation_id, - requested_version=request.requested_version, - requested_data=requested_data, - automerge=request.automerge, - patch=True, - response=response, - change_service=change_service, - ) + if request.group_permissions is not None: + try: + await incarnation_service.remove_all_group_permissions(incarnation_id) + await incarnation_service.set_group_permissions(incarnation_id, request.group_permissions) + except GroupNotFoundError as exc: + response.status_code = status.HTTP_404_NOT_FOUND + return ApiError(message=str(exc)) + except IncarnationNotFoundError as exc: + response.status_code = status.HTTP_404_NOT_FOUND + return ApiError(message=str(exc)) + + if request.user_permissions is not None: + try: + await incarnation_service.remove_all_user_permissions(incarnation_id) + await incarnation_service.set_user_permissions(incarnation_id, request.user_permissions) + except UserNotFoundError as exc: + response.status_code = status.HTTP_404_NOT_FOUND + return ApiError(message=str(exc)) + except IncarnationNotFoundError as exc: + response.status_code = status.HTTP_404_NOT_FOUND + return ApiError(message=str(exc)) + + if request.owner_id is not None: + try: + await incarnation_service.set_owner(incarnation_id, request.owner_id) + except UserNotFoundError as exc: + response.status_code = status.HTTP_404_NOT_FOUND + return ApiError(message=str(exc)) + except IncarnationNotFoundError as exc: + response.status_code = status.HTTP_404_NOT_FOUND + return ApiError(message=str(exc)) + + if request.requested_version is not None or request.requested_data is not None: + return await _create_change( + incarnation_id=incarnation_id, + requested_version=request.requested_version, + requested_data=requested_data, + automerge=request.automerge or False, + patch=True, + response=response, + change_service=change_service, + ) + else: + try: + return await change_service.get_incarnation_with_details(incarnation_id) + except IncarnationNotFoundError as exc: + response.status_code = status.HTTP_404_NOT_FOUND + return ApiError(message=str(exc)) @router.delete( diff --git a/src/foxops/services/authorization.py b/src/foxops/services/authorization.py index 237cf37a..98271224 100644 --- a/src/foxops/services/authorization.py +++ b/src/foxops/services/authorization.py @@ -1,8 +1,9 @@ +from foxops.models.user import UserWithGroups from foxops.services.group import GroupService -from foxops.services.user import UserService, UserWithGroups +from foxops.services.user import UserService -class AuthorizationService: +class AuthorizationService: def __init__(self, current_user: UserWithGroups, user_service: UserService, group_service: GroupService) -> None: self.user_service = user_service self.group_service = group_service @@ -10,9 +11,9 @@ def __init__(self, current_user: UserWithGroups, user_service: UserService, grou def is_admin(self) -> bool: return self.current_user.is_admin - + def is_in_group(self, group_system_name_or_id: str | int) -> bool: if isinstance(group_system_name_or_id, str): return any(group.system_name == group_system_name_or_id for group in self.current_user.groups) else: - return any(group.id == group_system_name_or_id for group in self.current_user.groups) \ No newline at end of file + return any(group.id == group_system_name_or_id for group in self.current_user.groups) diff --git a/src/foxops/services/change.py b/src/foxops/services/change.py index b31b3ab2..d9c5820c 100644 --- a/src/foxops/services/change.py +++ b/src/foxops/services/change.py @@ -6,13 +6,11 @@ import uuid from contextlib import asynccontextmanager from dataclasses import dataclass -from datetime import datetime, timedelta +from datetime import timedelta from pathlib import Path from tempfile import TemporaryDirectory from typing import AsyncIterator -from pydantic import BaseModel - import foxops.engine as fengine from foxops.database.repositories.change.model import ( ChangeType, @@ -20,6 +18,7 @@ ) from foxops.database.repositories.change.repository import ChangeRepository from foxops.database.repositories.incarnation.repository import IncarnationRepository +from foxops.database.repositories.user.repository import UserRepository from foxops.engine import TemplateData from foxops.engine.patching.git_diff_patch import PatchResult from foxops.errors import RetryableError @@ -28,6 +27,13 @@ from foxops.hosters.types import MergeRequestStatus from foxops.models import IncarnationWithDetails from foxops.models.change import Change, ChangeWithMergeRequest +from foxops.models.group import Group +from foxops.models.incarnation import ( + GroupPermission, + IncarnationWithLatestChangeDetails, + UserPermission, +) +from foxops.models.user import User from foxops.utils import get_logger @@ -53,24 +59,6 @@ def __init__(self, conflicting_paths: list[Path], deleted_paths: list[Path]): self.deleted_paths = deleted_paths -class IncarnationWithLatestChangeDetails(BaseModel): - id: int - incarnation_repository: str - target_directory: str - template_repository: str - - revision: int - type: ChangeType - requested_version: str - created_at: datetime - - commit_sha: str - commit_url: str - - merge_request_id: str | None - merge_request_url: str | None - - class ChangeFailed(Exception): pass @@ -114,12 +102,17 @@ async def tmp_empty_dir(): class ChangeService: def __init__( - self, hoster: Hoster, incarnation_repository: IncarnationRepository, change_repository: ChangeRepository + self, + hoster: Hoster, + incarnation_repository: IncarnationRepository, + change_repository: ChangeRepository, + user_repository: UserRepository, ): self._hoster = hoster self._incarnation_repository = incarnation_repository self._change_repository = change_repository + self._user_repository = user_repository self._log = get_logger("change_service") @@ -132,6 +125,8 @@ async def _incarnation_with_latest_change_details_from_dbobj( dbobj.incarnation_repository, dbobj.merge_request_id ) + owner = await self._user_repository.get_by_id(dbobj.owner) + return IncarnationWithLatestChangeDetails( id=dbobj.id, incarnation_repository=dbobj.incarnation_repository, @@ -142,6 +137,7 @@ async def _incarnation_with_latest_change_details_from_dbobj( requested_version=dbobj.requested_version, created_at=dbobj.created_at, commit_sha=dbobj.commit_sha, + owner=User.model_validate(owner), commit_url=await self._hoster.get_commit_url(dbobj.incarnation_repository, dbobj.commit_sha), merge_request_id=dbobj.merge_request_id, merge_request_url=merge_request_url, @@ -166,6 +162,7 @@ async def create_incarnation( template_repository: str, template_repository_version: str, template_data: TemplateData, + owner_id: int, target_directory: str = ".", ) -> Change: if await self._hoster.get_incarnation_state(incarnation_repository, target_directory) is not None: @@ -198,6 +195,7 @@ async def create_incarnation( requested_version=template_repository_version, requested_data=json.dumps(incarnation_state.template_data), template_data_full=json.dumps(incarnation_state.template_data_full), + owner_id=owner_id, ) try: @@ -568,6 +566,27 @@ async def get_incarnation_with_details(self, incarnation_id: int) -> Incarnation pipeline_timeout=timedelta(seconds=10), ) + owner = await self._user_repository.get_by_id(incarnation.owner) + + user_permissions = [ + UserPermission( + type=permission.type, + user=User(id=permission.user_id, username=permission.user_username, is_admin=permission.user_is_admin), + ) + for permission in await self._incarnation_repository.get_user_permissions(incarnation_id) + ] + group_permissions = [ + GroupPermission( + type=permission.type, + group=Group( + id=permission.group_id, + system_name=permission.group_system_name, + display_name=permission.group_display_name, + ), + ) + for permission in await self._incarnation_repository.get_group_permissions(incarnation_id) + ] + return IncarnationWithDetails( id=incarnation.id, incarnation_repository=incarnation.incarnation_repository, @@ -578,6 +597,9 @@ async def get_incarnation_with_details(self, incarnation_id: int) -> Incarnation merge_request_url=merge_request_url, merge_request_status=merge_request_status, status=status, + owner=User.model_validate(owner), + group_permissions=group_permissions, + user_permissions=user_permissions, revision=change.revision, template_repository=incarnation.template_repository, template_repository_version=change.requested_version, diff --git a/src/foxops/services/group.py b/src/foxops/services/group.py index 1b97910b..eecfdba3 100644 --- a/src/foxops/services/group.py +++ b/src/foxops/services/group.py @@ -1,16 +1,7 @@ from typing import Optional -from pydantic import BaseModel, ConfigDict - from foxops.database.repositories.group.repository import GroupRepository - - -class Group(BaseModel): - id: int - system_name: str - display_name: str - - model_config = ConfigDict(from_attributes=True) +from foxops.models.group import Group class GroupService: diff --git a/src/foxops/services/incarnation.py b/src/foxops/services/incarnation.py index 96bbd907..6cb7bbc9 100644 --- a/src/foxops/services/incarnation.py +++ b/src/foxops/services/incarnation.py @@ -1,7 +1,16 @@ from pydantic import BaseModel, ConfigDict +from foxops.database.repositories.group.repository import GroupRepository from foxops.database.repositories.incarnation.repository import IncarnationRepository +from foxops.database.repositories.user.repository import UserRepository from foxops.hosters import Hoster +from foxops.models.incarnation import ( + GroupPermission, + UnresolvedGroupPermissions, + UnresolvedUserPermissions, + UserPermission, +) +from foxops.models.user import User class Incarnation(BaseModel): @@ -10,12 +19,22 @@ class Incarnation(BaseModel): target_directory: str template_repository: str + owner: User + model_config = ConfigDict(from_attributes=True) class IncarnationService: - def __init__(self, incarnation_repository: IncarnationRepository, hoster: Hoster) -> None: + def __init__( + self, + incarnation_repository: IncarnationRepository, + hoster: Hoster, + user_repository: UserRepository, + group_repository: GroupRepository, + ) -> None: self.incarnation_repository = incarnation_repository + self.user_repository = user_repository + self.group_repository = group_repository self.hoster = hoster async def create( @@ -23,6 +42,7 @@ async def create( incarnation_repository: str, target_directory: str, template_repository: str, + owner_id: int, ) -> Incarnation: # verify that the incarnation repository exists await self.hoster.get_repository_metadata(incarnation_repository) @@ -34,12 +54,66 @@ async def create( incarnation_repository=incarnation_repository, target_directory=target_directory, template_repository=template_repository, + owner_id=owner_id, ) return Incarnation.model_validate(incarnation_in_db) async def get_by_id(self, id_: int) -> Incarnation: incarnation_in_db = await self.incarnation_repository.get_by_id(id_) - return Incarnation.model_validate(incarnation_in_db) + + owner = await self.user_repository.get_by_id(incarnation_in_db.owner) + + incarnation_json = incarnation_in_db.model_dump() + del incarnation_json["owner"] + + return Incarnation(owner=owner, **incarnation_json) async def delete(self, incarnation: Incarnation) -> None: await self.incarnation_repository.delete_by_id(incarnation.id) + + async def set_user_permissions( + self, incarnation_id: int, user_permissions: list[UnresolvedUserPermissions] + ) -> None: + if len(user_permissions) == 0: + return + resolved_user_permissions = [ + UserPermission( + user=await self.user_repository.get_by_id(user_permission.user_id), + type=user_permission.type, + ) + for user_permission in user_permissions + ] + + await self.incarnation_repository.get_by_id(incarnation_id) # Validate if the incarnation exists + + await self.incarnation_repository.set_user_permissions(incarnation_id, resolved_user_permissions) + + async def set_group_permissions( + self, incarnation_id: int, group_permissions: list[UnresolvedGroupPermissions] + ) -> None: + if len(group_permissions) == 0: + return + resolved_group_permissions = [ + GroupPermission( + group=await self.group_repository.get_by_id(group_permission.group_id), + type=group_permission.type, + ) + for group_permission in group_permissions + ] + + await self.incarnation_repository.get_by_id(incarnation_id) # Validate if the incarnation exists + + await self.incarnation_repository.set_group_permissions(incarnation_id, resolved_group_permissions) + + async def remove_all_user_permissions(self, incarnation_id: int) -> None: + await self.incarnation_repository.remove_all_user_permissions(incarnation_id) + + async def remove_all_group_permissions(self, incarnation_id: int) -> None: + await self.incarnation_repository.remove_all_group_permissions(incarnation_id) + + async def remove_all_permissions(self, incarnation_id: int) -> None: + await self.remove_all_group_permissions(incarnation_id) + await self.remove_all_user_permissions(incarnation_id) + + async def set_owner(self, incarnation_id: int, user_id: int): + await self.incarnation_repository.set_owner(incarnation_id, user_id) diff --git a/src/foxops/services/user.py b/src/foxops/services/user.py index 10135602..d1197b14 100644 --- a/src/foxops/services/user.py +++ b/src/foxops/services/user.py @@ -1,18 +1,7 @@ -from pydantic import BaseModel, ConfigDict - from foxops.database.repositories.group.repository import GroupRepository from foxops.database.repositories.user.repository import UserRepository -from foxops.services.group import Group - -class User(BaseModel): - id: int - username: str - is_admin: bool +from foxops.models.user import User, UserWithGroups - model_config = ConfigDict(from_attributes=True) - -class UserWithGroups(User): - groups: list[Group] class UserService: def __init__(self, user_repository: UserRepository, group_repository: GroupRepository) -> None: @@ -57,6 +46,6 @@ async def get_user_by_username_with_groups(self, username: str) -> UserWithGroup groups = await self.group_repository.get_by_userid(user.id) return UserWithGroups(groups=groups, **user.model_dump()) - + async def remove_all_groups_from_user(self, user_id: int) -> None: - await self.user_repository.remove_all_groups(user_id) \ No newline at end of file + await self.user_repository.remove_all_groups(user_id) diff --git a/tests/conftest.py b/tests/conftest.py index 0f1b5efc..c112e4b4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -14,15 +14,20 @@ from foxops.__main__ import create_app from foxops.database.engine import create_engine from foxops.database.repositories.change.repository import ChangeRepository +from foxops.database.repositories.group.repository import GroupRepository from foxops.database.repositories.incarnation.repository import IncarnationRepository +from foxops.database.repositories.user.repository import UserRepository from foxops.database.schema import meta from foxops.dependencies import ( get_change_repository, + get_database_engine, get_hoster, get_incarnation_repository, ) from foxops.hosters.local import LocalHoster from foxops.logger import setup_logging +from foxops.services.group import GroupService +from foxops.services.user import UserService pytest_plugins = [ "tests._plugins.fixtures_database", @@ -85,6 +90,26 @@ async def change_repository(test_async_engine: AsyncEngine) -> ChangeRepository: return ChangeRepository(test_async_engine) +@pytest.fixture +async def user_repository(test_async_engine: AsyncEngine) -> UserRepository: + return UserRepository(test_async_engine) + + +@pytest.fixture +async def group_repository(test_async_engine: AsyncEngine) -> GroupRepository: + return GroupRepository(test_async_engine) + + +@pytest.fixture +async def user_service(user_repository: UserRepository, group_repository: GroupRepository) -> UserService: + return UserService(user_repository, group_repository) + + +@pytest.fixture +async def group_service(group_repository: GroupRepository) -> GroupService: + return GroupService(group_repository) + + @pytest.fixture def local_hoster(tmp_path: Path) -> LocalHoster: return LocalHoster(tmp_path) @@ -101,10 +126,16 @@ async def create_unauthenticated_client( incarnation_repository: IncarnationRepository, change_repository: ChangeRepository, local_hoster: LocalHoster, + test_async_engine: AsyncEngine, ) -> AsyncGenerator[AsyncClient, None]: app.dependency_overrides[get_incarnation_repository] = lambda: incarnation_repository app.dependency_overrides[get_change_repository] = lambda: change_repository app.dependency_overrides[get_hoster] = lambda: local_hoster + app.dependency_overrides[ + get_database_engine + ] = ( + lambda: test_async_engine + ) # We need this, since the api now requires a working db -> to create the user in the request\ async with AsyncClient( app=app, @@ -116,6 +147,7 @@ async def create_unauthenticated_client( @pytest.fixture(name="authenticated_client") async def create_authenticated_client(unauthenticated_client: AsyncClient, static_api_token: str) -> AsyncClient: unauthenticated_client.headers["Authorization"] = f"Bearer {static_api_token}" + unauthenticated_client.headers["User"] = "root" return unauthenticated_client diff --git a/tests/database/migrations/test_00ee97d0b7a3.py b/tests/database/migrations/test_00ee97d0b7a3.py index acef7802..4be4c83b 100644 --- a/tests/database/migrations/test_00ee97d0b7a3.py +++ b/tests/database/migrations/test_00ee97d0b7a3.py @@ -4,7 +4,6 @@ from alembic.command import upgrade from alembic.script import ScriptDirectory -from foxops.database.repositories.change.repository import ChangeRepository INSERT_INCARNAION = text( """ @@ -71,15 +70,3 @@ async def test_database_upgrade(alembic_config, database_engine, async_database_ # WHEN # ... running the migration to the target version upgrade(alembic_config, TARGET_REVISION) - - # THEN - # ... the change table should have the new column and all the legacy template data copied to the full data column - cr = ChangeRepository(async_database_engine) - - change = await cr.get_change_by_revision(1, 1) - assert change.template_data_full == json.dumps({"dummydata": "yes"}) - assert change.requested_data == json.dumps({"dummydata": "yes"}) - - change = await cr.get_change_by_revision(1, 2) - assert change.template_data_full == json.dumps({"dummydata": "no"}) - assert change.requested_data == json.dumps({"dummydata": "no"}) diff --git a/tests/database/test_change_repository.py b/tests/database/test_change_repository.py index ebb2331c..4a853732 100644 --- a/tests/database/test_change_repository.py +++ b/tests/database/test_change_repository.py @@ -13,14 +13,22 @@ from foxops.database.repositories.change.repository import ChangeRepository from foxops.database.repositories.incarnation.model import IncarnationInDB from foxops.database.repositories.incarnation.repository import IncarnationRepository +from foxops.database.repositories.user.repository import UserRepository +from foxops.models.user import User + + +@fixture +async def user(user_repository: UserRepository): + return await user_repository.create( + username="test", + is_admin=False, + ) @fixture(scope="function") -async def incarnation(incarnation_repository: IncarnationRepository) -> IncarnationInDB: +async def incarnation(incarnation_repository: IncarnationRepository, user: User) -> IncarnationInDB: return await incarnation_repository.create( - incarnation_repository="test", - target_directory="test", - template_repository="test", + incarnation_repository="test", target_directory="test", template_repository="test", owner_id=user.id ) @@ -179,7 +187,7 @@ async def test_get_latest_change_for_incarnation_throws_exception_when_no_change async def test_list_incarnations_with_change_summary_returns_all_incarnations_with_latest_change_data( - change_repository: ChangeRepository, + change_repository: ChangeRepository, user: User ): # GIVEN incarnation1_change1 = await change_repository.create_incarnation_with_first_change( @@ -191,6 +199,7 @@ async def test_list_incarnations_with_change_summary_returns_all_incarnations_wi requested_version="v1", requested_data=json.dumps({"foo": "bar"}), template_data_full=json.dumps({"foo": "bar"}), + owner_id=user.id, ) incarnation1_change2 = await change_repository.create_change( incarnation_id=incarnation1_change1.id, @@ -215,6 +224,7 @@ async def test_list_incarnations_with_change_summary_returns_all_incarnations_wi requested_version="v1", requested_data=json.dumps({"foo": "bar"}), template_data_full=json.dumps({"foo": "bar"}), + owner_id=user.id, ) # WHEN @@ -328,7 +338,7 @@ async def test_delete_change_raises_exception_when_not_found(change_repository: await change_repository.delete_change(123) -async def test_create_incarnation_with_first_change(change_repository: ChangeRepository): +async def test_create_incarnation_with_first_change(change_repository: ChangeRepository, user: User): # WHEN change = await change_repository.create_incarnation_with_first_change( incarnation_repository="incarnation", @@ -338,6 +348,7 @@ async def test_create_incarnation_with_first_change(change_repository: ChangeRep requested_version_hash="dummy template sha", requested_version="v1", requested_data=json.dumps({"foo": "bar"}), + owner_id=user.id, template_data_full=json.dumps({"foo": "bar"}), ) @@ -353,7 +364,7 @@ async def test_create_incarnation_with_first_change(change_repository: ChangeRep assert change.commit_pushed is False -async def test_delete_incarnation_also_deletes_associated_changes(change_repository: ChangeRepository): +async def test_delete_incarnation_also_deletes_associated_changes(change_repository: ChangeRepository, user: User): # GIVEN change = await change_repository.create_incarnation_with_first_change( incarnation_repository="incarnation", @@ -364,6 +375,7 @@ async def test_delete_incarnation_also_deletes_associated_changes(change_reposit requested_version="v1", requested_data=json.dumps({"foo": "bar"}), template_data_full=json.dumps({"foo": "bar"}), + owner_id=user.id, ) incarnation_id = change.incarnation_id diff --git a/tests/database/test_incarnation_repository.py b/tests/database/test_incarnation_repository.py index a0401214..747b264d 100644 --- a/tests/database/test_incarnation_repository.py +++ b/tests/database/test_incarnation_repository.py @@ -7,24 +7,36 @@ ) from foxops.database.repositories.incarnation.model import IncarnationInDB from foxops.database.repositories.incarnation.repository import IncarnationRepository +from foxops.database.repositories.user.repository import UserRepository +from foxops.models.user import User + + +@fixture +async def user(user_repository: UserRepository): + return await user_repository.create( + username="test", + is_admin=False, + ) @fixture(scope="function", name="incarnation_id") -async def create_incarnation(incarnation_repository: IncarnationRepository) -> int: +async def create_incarnation(incarnation_repository: IncarnationRepository, user: User) -> int: new_incarnation = await incarnation_repository.create( incarnation_repository="incarnation_repo", target_directory="test", template_repository="template_repo", + owner_id=user.id, ) return new_incarnation.id -async def test_create_adds_new_incarnation_to_inventory(incarnation_repository: IncarnationRepository): +async def test_create_adds_new_incarnation_to_inventory(incarnation_repository: IncarnationRepository, user: User): # WHEN new_incarnation = await incarnation_repository.create( incarnation_repository="incarnation_repo", target_directory="test", template_repository="template_repo", + owner_id=user.id, ) # THEN @@ -35,13 +47,14 @@ async def test_create_adds_new_incarnation_to_inventory(incarnation_repository: async def test_create_raises_exception_when_same_incarnation_exists_already( - incarnation_repository: IncarnationRepository, + incarnation_repository: IncarnationRepository, user: User ): # GIVEN await incarnation_repository.create( incarnation_repository="incarnation_repo", target_directory="test", template_repository="template_repo", + owner_id=user.id, ) # THEN @@ -50,11 +63,12 @@ async def test_create_raises_exception_when_same_incarnation_exists_already( incarnation_repository="incarnation_repo", target_directory="test", template_repository="template_repo2", + owner_id=user.id, ) async def test_get_by_id_returns_existing_incarnation( - incarnation_repository: IncarnationRepository, incarnation_id: int + incarnation_repository: IncarnationRepository, incarnation_id: int, user: User ): # WHEN actual_incarnation = await incarnation_repository.get_by_id(incarnation_id) @@ -65,6 +79,7 @@ async def test_get_by_id_returns_existing_incarnation( incarnation_repository="incarnation_repo", target_directory="test", template_repository="template_repo", + owner=user.id, ) diff --git a/tests/e2e/conftest.py b/tests/e2e/conftest.py index c16ec625..4d54ca99 100644 --- a/tests/e2e/conftest.py +++ b/tests/e2e/conftest.py @@ -152,6 +152,7 @@ async def foxops_client(gitlab_address: str, gitlab_access_token: str, foxops_da base_url="http://test", ) as client: client.headers["Authorization"] = f"Bearer {static_token}" + client.headers["User"] = "root" yield client diff --git a/tests/e2e/test_incarnations_api.py b/tests/e2e/test_incarnations_api.py index 8ef21a50..4fc5b9dc 100644 --- a/tests/e2e/test_incarnations_api.py +++ b/tests/e2e/test_incarnations_api.py @@ -290,6 +290,9 @@ async def test_put_incarnation_updates_incarnation_with_merge_request( "template_repository_version": "v2.0.0", "template_data": {"name": "Jon", "age": 18}, "automerge": False, + "owner_id": 1, + "user_permissions": [], + "group_permissions": [], }, ) response.raise_for_status() @@ -334,6 +337,9 @@ async def test_put_incarnation_updates_incarnation_with_merge_request_and_autome "template_repository_version": "v2.0.0", "template_data": {"name": "Jon", "age": 18}, "automerge": True, + "owner_id": 1, + "user_permissions": [], + "group_permissions": [], }, ) response.raise_for_status() @@ -389,8 +395,12 @@ async def test_put_incarnation_creates_merge_request_with_conflicts( "template_repository_version": "v2.0.0", "template_data": {"name": "Jon", "age": 18}, "automerge": automerge, + "owner_id": 1, + "user_permissions": [], + "group_permissions": [], }, ) + response.raise_for_status() incarnation = response.json() @@ -440,6 +450,9 @@ async def test_put_incarnation_fails_if_insufficient_template_data_is_provided( "template_repository_version": "v1.0.0", "template_data": {"name": "Jane"}, "automerge": True, + "owner_id": 1, + "user_permissions": [], + "group_permissions": [], }, ) @@ -460,6 +473,9 @@ async def test_patch_incarnation_returns_error_if_the_previous_one_has_not_been_ "template_repository_version": "v1.0.0", "template_data": {"name": "Jon", "age": 19}, "automerge": False, + "owner_id": 1, + "user_permissions": [], + "group_permissions": [], }, ) response.raise_for_status() @@ -628,6 +644,9 @@ async def test_delete_incarnation_removes_incarnation_when_there_are_changes( "template_repository_version": "v2.0.0", "template_data": {"name": "Jon", "age": 18}, "automerge": True, + "owner_id": 1, + "user_permissions": [], + "group_permissions": [], }, ) response.raise_for_status() diff --git a/tests/routers/test_auth.py b/tests/routers/test_auth.py index 07bf375c..712be76a 100644 --- a/tests/routers/test_auth.py +++ b/tests/routers/test_auth.py @@ -1,6 +1,8 @@ from fastapi import FastAPI, status from httpx import AsyncClient +from foxops.dependencies import group_auth_scheme, user_auth_scheme + async def test_returns_err_if_authorization_header_is_missing(app: FastAPI): # WHEN @@ -63,6 +65,9 @@ async def test_returns_err_if_token_is_wrong(app: FastAPI): async def test_allow_access_if_token_is_correct(app: FastAPI, static_api_token: str): + app.dependency_overrides[group_auth_scheme] = lambda: None # Disable group auth for this test + app.dependency_overrides[user_auth_scheme] = lambda: None # Disable user auth for this test + # WHEN async with AsyncClient(app=app, base_url="http://test", follow_redirects=True) as client: response = await client.get("/auth/test", headers={"Authorization": f"Bearer {static_api_token}"}) diff --git a/tests/routers/test_incarnations.py b/tests/routers/test_incarnations.py index 88dc668e..88c56667 100644 --- a/tests/routers/test_incarnations.py +++ b/tests/routers/test_incarnations.py @@ -11,8 +11,10 @@ from foxops.database.repositories.change.repository import ChangeRepository from foxops.database.repositories.incarnation.errors import IncarnationNotFoundError from foxops.database.repositories.incarnation.repository import IncarnationRepository +from foxops.database.repositories.user.repository import UserRepository from foxops.dependencies import get_change_service from foxops.models.change import Change +from foxops.models.user import User from foxops.services.change import ChangeService, IncarnationAlreadyExists pytestmark = [pytest.mark.api] @@ -41,6 +43,7 @@ async def create_incarnation( template_repository_version: str, template_data: dict[str, str], target_directory: str = ".", + owner_id: int = 1, ) -> Change: return Change( id=1, @@ -55,6 +58,16 @@ async def create_incarnation( ) +@pytest.fixture +async def user(user_repository: UserRepository) -> User: + return User.model_validate( + await user_repository.create( + username="test", + is_admin=False, + ) + ) + + @pytest.fixture def change_service_mock(app: FastAPI): change_service = ChangeServiceMock() @@ -85,9 +98,7 @@ async def test_api_get_incarnations_returns_empty_list_for_empty_incarnation_inv async def test_api_get_incarnations_returns_incarnations_from_inventory( - api_client: AsyncClient, - change_repository: ChangeRepository, - mocker: MockFixture, + api_client: AsyncClient, change_repository: ChangeRepository, mocker: MockFixture, user: User ): # GIVEN await change_repository.create_incarnation_with_first_change( @@ -99,6 +110,7 @@ async def test_api_get_incarnations_returns_incarnations_from_inventory( requested_version_hash="template_commit_sha", requested_data=json.dumps({"foo": "bar"}), template_data_full=json.dumps({"foo": "bar"}), + owner_id=user.id, ) # WHEN @@ -120,6 +132,7 @@ async def test_api_get_incarnations_returns_incarnations_from_inventory( "requested_version": "v1.0", "revision": 1, "type": "direct", + "owner": user.model_dump(), } ] @@ -175,14 +188,11 @@ async def test_api_create_incarnation_returns_conflict_when_incarnation_already_ async def test_api_delete_incarnation_removes_incarnation_from_inventory( - api_client: AsyncClient, - incarnation_repository: IncarnationRepository, + api_client: AsyncClient, incarnation_repository: IncarnationRepository, user: User ): # GIVEN incarnation = await incarnation_repository.create( - incarnation_repository="test", - target_directory="test", - template_repository="template_repo", + incarnation_repository="test", target_directory="test", template_repository="template_repo", owner_id=user.id ) # WHEN diff --git a/tests/services/test_change.py b/tests/services/test_change.py index 87962964..661266e1 100644 --- a/tests/services/test_change.py +++ b/tests/services/test_change.py @@ -7,8 +7,10 @@ from foxops.database.repositories.change.errors import ChangeNotFoundError from foxops.database.repositories.change.repository import ChangeRepository +from foxops.database.repositories.group.repository import GroupRepository from foxops.database.repositories.incarnation.errors import IncarnationNotFoundError from foxops.database.repositories.incarnation.repository import IncarnationRepository +from foxops.database.repositories.user.repository import UserRepository from foxops.engine import IncarnationState from foxops.engine.models.template_config import ( StringVariableDefinition, @@ -19,6 +21,7 @@ from foxops.hosters.types import MergeRequestStatus from foxops.models import Incarnation from foxops.models.change import Change, ChangeWithMergeRequest +from foxops.models.user import User from foxops.services.change import ( CannotRepairChangeException, ChangeRejectedDueToNoChanges, @@ -27,6 +30,7 @@ _construct_merge_request_conflict_description, delete_all_files_in_local_git_repository, ) +from foxops.services.user import UserService @fixture(scope="function") @@ -71,9 +75,14 @@ async def git_repo_template(local_hoster: LocalHoster) -> str: return repo_name +@fixture(scope="function") +async def user(user_service: UserService) -> User: + return await user_service.create_user("test", False) + + @fixture(scope="function") async def initialized_incarnation( - local_hoster: LocalHoster, git_repo_template: str, change_service: ChangeService + local_hoster: LocalHoster, git_repo_template: str, change_service: ChangeService, user: User ) -> Incarnation: repo_name = "incarnation_initialized" await local_hoster.create_repository(repo_name) @@ -83,6 +92,7 @@ async def initialized_incarnation( template_repository=git_repo_template, template_repository_version="v1.0.0", template_data={}, + owner_id=user.id, ) return Incarnation( @@ -92,6 +102,7 @@ async def initialized_incarnation( template_repository=git_repo_template, commit_sha="dummy", merge_request_id=None, + owner=user, ) @@ -113,29 +124,38 @@ async def change_service( test_async_engine: AsyncEngine, incarnation_repository: IncarnationRepository, local_hoster: LocalHoster ) -> ChangeService: change_repository = ChangeRepository(test_async_engine) - + user_repository = UserRepository(test_async_engine) return ChangeService( hoster=local_hoster, incarnation_repository=incarnation_repository, change_repository=change_repository, + user_repository=user_repository, ) +@fixture(scope="function") +async def user_service(test_async_engine: AsyncEngine): + user_repository = UserRepository(test_async_engine) + group_repository = GroupRepository(test_async_engine) + return UserService(user_repository=user_repository, group_repository=group_repository) + + @fixture(scope="function") async def change_service_with_delay( test_async_engine: AsyncEngine, incarnation_repository: IncarnationRepository, local_hoster_with_delay: LocalHoster ) -> ChangeService: change_repository = ChangeRepository(test_async_engine) - + user_repository = UserRepository(test_async_engine) return ChangeService( hoster=local_hoster_with_delay, incarnation_repository=incarnation_repository, change_repository=change_repository, + user_repository=user_repository, ) async def test_create_incarnation_succeeds_when_creating_incarnation_in_root_folder( - change_service: ChangeService, git_repo_template: str, local_hoster: LocalHoster + change_service: ChangeService, git_repo_template: str, local_hoster: LocalHoster, user: User ): # GIVEN incarnation_repo_name = "incarnation" @@ -148,6 +168,7 @@ async def test_create_incarnation_succeeds_when_creating_incarnation_in_root_fol template_repository=git_repo_template, template_repository_version="v1.0.0", template_data={}, + owner_id=user.id, ) # THEN @@ -161,7 +182,7 @@ async def test_create_incarnation_succeeds_when_creating_incarnation_in_root_fol async def test_create_incarnation_succeeds_when_creating_incarnation_in_subfolder( - change_service: ChangeService, git_repo_template: str, local_hoster: LocalHoster + change_service: ChangeService, git_repo_template: str, local_hoster: LocalHoster, user: User ): # GIVEN incarnation_repo_name = "incarnation" @@ -174,6 +195,7 @@ async def test_create_incarnation_succeeds_when_creating_incarnation_in_subfolde template_repository=git_repo_template, template_repository_version="v1.0.0", template_data={}, + owner_id=user.id, ) # THEN @@ -187,7 +209,7 @@ async def test_create_incarnation_succeeds_when_creating_incarnation_in_subfolde async def test_create_incarnation_succeeds_with_internal_retries_when_the_first_push_is_rejected( - change_service_with_delay: ChangeService, local_hoster_with_delay: LocalHoster, git_repo_template: str + change_service_with_delay: ChangeService, local_hoster_with_delay: LocalHoster, git_repo_template: str, user: User ): # GIVEN repo_name = "incarnation_initialized" @@ -201,6 +223,7 @@ async def _delayed_creation(): template_repository=git_repo_template, template_repository_version="v1.0.0", template_data={}, + owner_id=user.id, ) # WHEN @@ -211,6 +234,7 @@ async def _delayed_creation(): template_repository=git_repo_template, template_repository_version="v1.0.0", template_data={}, + owner_id=user.id, ), _delayed_creation(), ) @@ -232,7 +256,7 @@ async def _delayed_creation(): async def test_create_incarnation_fails_if_there_is_already_one_at_the_target( - change_service: ChangeService, git_repo_template: str, local_hoster: LocalHoster + change_service: ChangeService, git_repo_template: str, local_hoster: LocalHoster, user: User ): # GIVEN incarnation_repo_name = "incarnation" @@ -242,6 +266,7 @@ async def test_create_incarnation_fails_if_there_is_already_one_at_the_target( template_repository=git_repo_template, template_repository_version="v1.0.0", template_data={}, + owner_id=user.id, ) # THEN @@ -251,6 +276,7 @@ async def test_create_incarnation_fails_if_there_is_already_one_at_the_target( template_repository=git_repo_template, template_repository_version="v1.1.0", template_data={}, + owner_id=user.id, ) From f9413f3934f266d46800bde80a90bb12dc69c3f5 Mon Sep 17 00:00:00 2001 From: Yannick Mueller Date: Thu, 27 Mar 2025 10:55:41 +0100 Subject: [PATCH 06/23] Adapt change endpoints to new schema --- .../database/repositories/change/model.py | 2 + .../repositories/change/repository.py | 2 + src/foxops/models/change.py | 3 ++ src/foxops/routers/changes.py | 25 ++++++++++-- src/foxops/routers/incarnations.py | 12 +++++- src/foxops/services/change.py | 16 +++++++- tests/routers/test_incarnations.py | 2 +- tests/services/test_change.py | 38 +++++++++---------- 8 files changed, 73 insertions(+), 27 deletions(-) diff --git a/src/foxops/database/repositories/change/model.py b/src/foxops/database/repositories/change/model.py index b788e69f..3e1751d1 100644 --- a/src/foxops/database/repositories/change/model.py +++ b/src/foxops/database/repositories/change/model.py @@ -30,6 +30,8 @@ class ChangeInDB(BaseModel): merge_request_id: str | None merge_request_branch_name: str | None + initialized_by: int | None + model_config = ConfigDict(from_attributes=True) @classmethod diff --git a/src/foxops/database/repositories/change/repository.py b/src/foxops/database/repositories/change/repository.py index f8452a6a..a1f8e9df 100644 --- a/src/foxops/database/repositories/change/repository.py +++ b/src/foxops/database/repositories/change/repository.py @@ -40,6 +40,7 @@ async def create_change( template_data_full: str, merge_request_id: str | None = None, merge_request_branch_name: str | None = None, + initialized_by: int | None = None, ) -> ChangeInDB: """ Create a new change for the given incarnation with the given "revision" number. @@ -67,6 +68,7 @@ async def create_change( commit_pushed=commit_pushed, merge_request_id=merge_request_id, merge_request_branch_name=merge_request_branch_name, + initialized_by=initialized_by, ) .returning(*change.columns) ) diff --git a/src/foxops/models/change.py b/src/foxops/models/change.py index 18ad31ae..7f7495ce 100644 --- a/src/foxops/models/change.py +++ b/src/foxops/models/change.py @@ -4,6 +4,7 @@ from foxops.engine import TemplateData from foxops.hosters.types import MergeRequestStatus +from foxops.models.user import User class Change(BaseModel): @@ -21,6 +22,8 @@ class Change(BaseModel): created_at: datetime commit_sha: str + initialized_by: User | None = None + class ChangeWithMergeRequest(Change): merge_request_id: str diff --git a/src/foxops/routers/changes.py b/src/foxops/routers/changes.py index ecbbd42c..ddfce4b8 100644 --- a/src/foxops/routers/changes.py +++ b/src/foxops/routers/changes.py @@ -7,10 +7,12 @@ from foxops.database.repositories.change.errors import ChangeNotFoundError from foxops.database.repositories.change.model import ChangeType as DatabaseChangeType -from foxops.dependencies import get_change_service +from foxops.dependencies import authorization, get_change_service from foxops.engine import TemplateData from foxops.hosters.types import MergeRequestStatus from foxops.models.change import Change, ChangeWithMergeRequest +from foxops.models.user import User +from foxops.services.authorization import AuthorizationService from foxops.services.change import CannotRepairChangeException, ChangeService router = APIRouter() @@ -73,12 +75,15 @@ class ChangeDetails(BaseModel): created_at: datetime commit_sha: str + initialized_by: User | None = None + merge_request_id: str | None = None merge_request_branch_name: str | None = None merge_request_status: MergeRequestStatus | None = None @classmethod def from_service_object(cls, obj: Change | ChangeWithMergeRequest) -> Self: + print(obj) match obj: case ChangeWithMergeRequest(): return cls(type=ChangeType.MERGE_REQUEST, **obj.model_dump()) @@ -93,19 +98,31 @@ async def create_change( incarnation_id: int, request: CreateChangeRequest, change_service: ChangeService = Depends(get_change_service), + authorization_serive: AuthorizationService = Depends(authorization), ) -> ChangeDetails: match request.change_type: case CreateChangeType.DIRECT: change = await change_service.create_change_direct( - incarnation_id, request.requested_version, request.requested_data + incarnation_id, + request.requested_version, + request.requested_data, + initialized_by=authorization_serive.current_user.id, ) case CreateChangeType.MERGE_REQUEST_MANUAL: change = await change_service.create_change_merge_request( - incarnation_id, request.requested_version, request.requested_data, automerge=False + incarnation_id, + request.requested_version, + request.requested_data, + automerge=False, + initialized_by=authorization_serive.current_user.id, ) case CreateChangeType.MERGE_REQUEST_AUTOMERGE: change = await change_service.create_change_merge_request( - incarnation_id, request.requested_version, request.requested_data, automerge=True + incarnation_id, + request.requested_version, + request.requested_data, + automerge=True, + initialized_by=authorization_serive.current_user.id, ) case _: raise NotImplementedError(f"Unknown change type {request.change_type}") diff --git a/src/foxops/routers/incarnations.py b/src/foxops/routers/incarnations.py index ff900c8d..2f9ff443 100644 --- a/src/foxops/routers/incarnations.py +++ b/src/foxops/routers/incarnations.py @@ -220,10 +220,14 @@ async def reset_incarnation( incarnation_service: IncarnationService = Depends(get_incarnation_service), change_service: ChangeService = Depends(get_change_service), hoster: Hoster = Depends(get_hoster), + authorization_service: AuthorizationService = Depends(authorization), ): try: change = await change_service.reset_incarnation( - incarnation_id, request.requested_version, request.requested_data + incarnation_id, + request.requested_version, + request.requested_data, + initialized_by=authorization_service.current_user.id, ) except ProvidedTemplateDataInvalidError as e: response.status_code = status.HTTP_400_BAD_REQUEST @@ -257,6 +261,7 @@ async def _create_change( patch: bool, response: Response, change_service: ChangeService, + initialized_by: int, ) -> IncarnationWithDetails | ApiError: try: await change_service.create_change_merge_request( @@ -265,6 +270,7 @@ async def _create_change( requested_data=requested_data, automerge=automerge, patch=patch, + initialized_by=initialized_by, ) except ProvidedTemplateDataInvalidError as e: response.status_code = status.HTTP_400_BAD_REQUEST @@ -334,6 +340,7 @@ async def update_incarnation( request: UpdateIncarnationRequest, change_service: ChangeService = Depends(get_change_service), incarnation_service: IncarnationService = Depends(get_incarnation_service), + authorization_serive: AuthorizationService = Depends(authorization), ): """Updates the incarnation to the given version and data. @@ -378,6 +385,7 @@ async def update_incarnation( patch=False, response=response, change_service=change_service, + initialized_by=authorization_serive.current_user.id, ) @@ -442,6 +450,7 @@ async def patch_incarnation( request: PatchIncarnationRequest, change_service: ChangeService = Depends(get_change_service), incarnation_service: IncarnationService = Depends(get_incarnation_service), + authorization_serive: AuthorizationService = Depends(authorization), ): """Updates the incarnation to the given version and data. @@ -492,6 +501,7 @@ async def patch_incarnation( patch=True, response=response, change_service=change_service, + initialized_by=authorization_serive.current_user.id, ) else: try: diff --git a/src/foxops/services/change.py b/src/foxops/services/change.py index d9c5820c..6a6c2463 100644 --- a/src/foxops/services/change.py +++ b/src/foxops/services/change.py @@ -206,7 +206,9 @@ async def create_incarnation( return await self.get_change(change.id) - async def reset_incarnation(self, incarnation_id: int, version: str, data: TemplateData) -> ChangeWithMergeRequest: + async def reset_incarnation( + self, incarnation_id: int, version: str, data: TemplateData, initialized_by: int | None = None + ) -> ChangeWithMergeRequest: """ Resets an incarnation by removing all customizations that were done to it ... and bring it back to a pristine state as if it was just created freshly from the template. @@ -256,6 +258,7 @@ async def reset_incarnation(self, incarnation_id: int, version: str, data: Templ requested_data=json.dumps(incarnation_state.template_data), template_data_full=json.dumps(incarnation_state.template_data_full), merge_request_branch_name=reset_branch_name, + initialized_by=initialized_by, ) await self._push_change_commit_and_update_database(incarnation_git, change_in_db.id) @@ -280,7 +283,7 @@ async def reset_incarnation(self, incarnation_id: int, version: str, data: Templ return await self.get_change_with_merge_request(change_in_db.id) async def create_change_direct( - self, incarnation_id: int, requested_version: str, requested_data: TemplateData + self, incarnation_id: int, requested_version: str, requested_data: TemplateData, initialized_by: int ) -> Change: """ Perform a DIRECT change on the given incarnation. @@ -315,6 +318,7 @@ async def create_change_direct( requested_version=env.to_version, requested_data=json.dumps(env.to_data), template_data_full=json.dumps(env.to_data_full), + initialized_by=initialized_by, ) # if some failure happens after this point, the database object can be cleaned @@ -328,6 +332,7 @@ async def create_change_merge_request( incarnation_id: int, requested_version: str | None, requested_data: TemplateData, + initialized_by: int | None = None, automerge: bool = False, patch: bool = False, ) -> ChangeWithMergeRequest: @@ -354,6 +359,7 @@ async def create_change_merge_request( requested_data=json.dumps(env.to_data), template_data_full=json.dumps(env.to_data_full), merge_request_branch_name=env.branch_name, + initialized_by=initialized_by, ) await self._push_change_commit_and_update_database(env.incarnation_repository, change_in_db.id) @@ -454,6 +460,11 @@ async def get_change(self, change_id: int) -> Change: """ change = await self._change_repository.get_change(change_id) + + initialized_by = None + if change.initialized_by is not None: + initialized_by = await self._user_repository.get_by_id(change.initialized_by) + if not change.commit_pushed: raise IncompleteChange( "the given change is in an incomplete state (commit_pushed=False). " @@ -470,6 +481,7 @@ async def get_change(self, change_id: int) -> Change: template_data_full=json.loads(change.template_data_full), created_at=change.created_at, commit_sha=change.commit_sha, + initialized_by=initialized_by, ) async def get_change_with_merge_request(self, change_id: int) -> ChangeWithMergeRequest: diff --git a/tests/routers/test_incarnations.py b/tests/routers/test_incarnations.py index 88c56667..ba43ba65 100644 --- a/tests/routers/test_incarnations.py +++ b/tests/routers/test_incarnations.py @@ -230,4 +230,4 @@ async def test_api_get_diff_of_non_existing_incarnation_returns_not_found( response = await api_client.get("/incarnations/1/diff") assert response.status_code == HTTPStatus.NOT_FOUND - assert response.json() == {"message": "could not find incarnation in DB with id: 1"} + assert response.json() == {"message": "Incarnation with id '1' not found."} diff --git a/tests/services/test_change.py b/tests/services/test_change.py index 661266e1..21f33d68 100644 --- a/tests/services/test_change.py +++ b/tests/services/test_change.py @@ -281,11 +281,11 @@ async def test_create_incarnation_fails_if_there_is_already_one_at_the_target( async def test_create_change_direct_succeeds_when_updating_the_template_version( - change_service: ChangeService, initialized_incarnation: Incarnation + change_service: ChangeService, initialized_incarnation: Incarnation, user: User ): # WHEN change = await change_service.create_change_direct( - initialized_incarnation.id, requested_version="v1.1.0", requested_data={} + initialized_incarnation.id, requested_version="v1.1.0", requested_data={}, initialized_by=user.id ) # THEN @@ -302,11 +302,11 @@ async def test_create_change_direct_succeeds_when_updating_the_template_version( async def test_create_change_direct_succeeds_and_makes_new_template_variables_visible_in_the_foxops_api( - change_service: ChangeService, initialized_incarnation: Incarnation + change_service: ChangeService, initialized_incarnation: Incarnation, user: User ): # WHEN change = await change_service.create_change_direct( - initialized_incarnation.id, requested_version="v1.3.0", requested_data={} + initialized_incarnation.id, requested_version="v1.3.0", requested_data={}, initialized_by=user.id ) # THEN @@ -324,10 +324,11 @@ async def test_create_change_direct_succeeds_when_updating_to_the_same_branch_na local_hoster: LocalHoster, initialized_incarnation: Incarnation, git_repo_template: str, + user: User, ): # GIVEN initial_change = await change_service.create_change_direct( - initialized_incarnation.id, requested_version="main", requested_data={} + initialized_incarnation.id, requested_version="main", requested_data={}, initialized_by=user.id ) async with local_hoster.cloned_repository(git_repo_template) as repo: (repo.directory / "template" / "README.md").write_text("Hello, world - even more!") @@ -336,7 +337,7 @@ async def test_create_change_direct_succeeds_when_updating_to_the_same_branch_na # WHEN new_change = await change_service.create_change_direct( - initialized_incarnation.id, requested_version="main", requested_data={} + initialized_incarnation.id, requested_version="main", requested_data={}, initialized_by=user.id ) # THEN @@ -352,9 +353,7 @@ async def test_create_change_direct_succeeds_when_updating_to_the_same_branch_na async def test_create_change_direct_succeeds_when_the_previous_change_was_not_merged( - change_service: ChangeService, - initialized_incarnation: Incarnation, - local_hoster: LocalHoster, + change_service: ChangeService, initialized_incarnation: Incarnation, local_hoster: LocalHoster, user: User ): # GIVEN unmerged_change = await change_service.create_change_merge_request( @@ -369,7 +368,7 @@ async def test_create_change_direct_succeeds_when_the_previous_change_was_not_me # WHEN local_hoster.close_merge_request(initialized_incarnation.incarnation_repository, unmerged_change.merge_request_id) change = await change_service.create_change_direct( - initialized_incarnation.id, requested_version="v1.2.0", requested_data={} + initialized_incarnation.id, requested_version="v1.2.0", requested_data={}, initialized_by=user.id ) # THEN @@ -377,21 +376,22 @@ async def test_create_change_direct_succeeds_when_the_previous_change_was_not_me async def test_create_change_direct_succeeds_with_reverting_a_variable_back_to_its_default_value_if_not_explicitly_specified( - change_service: ChangeService, - initialized_incarnation: Incarnation, - local_hoster: LocalHoster, + change_service: ChangeService, initialized_incarnation: Incarnation, local_hoster: LocalHoster, user: User ): # GIVEN # ... the incarnation is at a version that requires a variable - and the variable was explicitly set change = await change_service.create_change_direct( - initialized_incarnation.id, requested_version="v1.3.0", requested_data={"author": "John Doe"} + initialized_incarnation.id, + requested_version="v1.3.0", + requested_data={"author": "John Doe"}, + initialized_by=user.id, ) assert change.template_data_full["author"] == "John Doe" # WHEN # ... creating a change that no longer specifies the variable change = await change_service.create_change_direct( - initialized_incarnation.id, requested_version="v1.3.0", requested_data={} + initialized_incarnation.id, requested_version="v1.3.0", requested_data={}, initialized_by=user.id ) # THEN @@ -514,11 +514,11 @@ async def test_list_changes( async def test_update_incomplete_change_recovers_from_unpushed_direct_change( - local_hoster: LocalHoster, change_service: ChangeService, initialized_incarnation: Incarnation + local_hoster: LocalHoster, change_service: ChangeService, initialized_incarnation: Incarnation, user: User ): # GIVEN change = await change_service.create_change_direct( - initialized_incarnation.id, requested_version="v1.1.0", requested_data={} + initialized_incarnation.id, requested_version="v1.1.0", requested_data={}, initialized_by=user.id ) # remove latest commit from repo @@ -542,11 +542,11 @@ async def test_update_incomplete_change_recovers_from_unpushed_direct_change( async def test_update_incomplete_change_recovers_from_pushed_direct_change( - local_hoster: LocalHoster, change_service: ChangeService, initialized_incarnation: Incarnation + local_hoster: LocalHoster, change_service: ChangeService, initialized_incarnation: Incarnation, user: User ): # GIVEN change = await change_service.create_change_direct( - initialized_incarnation.id, requested_version="v1.1.0", requested_data={} + initialized_incarnation.id, requested_version="v1.1.0", requested_data={}, initialized_by=user.id ) # update database to reflect the missing update about the pushed change From fc37d2bc84fa8582c8fcd31482049d39ee291d47 Mon Sep 17 00:00:00 2001 From: Yannick Mueller Date: Thu, 27 Mar 2025 11:52:10 +0100 Subject: [PATCH 07/23] Use global exception handeling --- .../repositories/change/repository.py | 1 + .../repositories/incarnation/errors.py | 7 +- src/foxops/error_handlers.py | 41 ++++++- src/foxops/routers/changes.py | 23 ++-- src/foxops/routers/incarnations.py | 108 ++++-------------- 5 files changed, 79 insertions(+), 101 deletions(-) diff --git a/src/foxops/database/repositories/change/repository.py b/src/foxops/database/repositories/change/repository.py index a1f8e9df..601ba78e 100644 --- a/src/foxops/database/repositories/change/repository.py +++ b/src/foxops/database/repositories/change/repository.py @@ -131,6 +131,7 @@ async def create_incarnation_with_first_change( template_data_full=template_data_full, commit_sha=commit_sha, commit_pushed=False, + initialized_by=owner_id, ) .returning(*change.columns) ) diff --git a/src/foxops/database/repositories/incarnation/errors.py b/src/foxops/database/repositories/incarnation/errors.py index 4798023d..03c92fc4 100644 --- a/src/foxops/database/repositories/incarnation/errors.py +++ b/src/foxops/database/repositories/incarnation/errors.py @@ -1,8 +1,11 @@ -class IncarnationNotFoundError(Exception): +from foxops.errors import FoxopsError + + +class IncarnationNotFoundError(FoxopsError): def __init__(self, id: int): self.id = id super().__init__(f"Incarnation with id '{id}' not found.") -class IncarnationAlreadyExistsError(Exception): +class IncarnationAlreadyExistsError(FoxopsError): pass diff --git a/src/foxops/error_handlers.py b/src/foxops/error_handlers.py index fbe3c1df..70c3ac96 100644 --- a/src/foxops/error_handlers.py +++ b/src/foxops/error_handlers.py @@ -2,12 +2,41 @@ from fastapi.exceptions import RequestValidationError from fastapi.responses import JSONResponse -from foxops.errors import FoxopsUserError +from foxops.database.repositories.change.errors import ( + ChangeCommitAlreadyPushedError, + ChangeNotFoundError, + IncarnationHasNoChangesError, +) +from foxops.database.repositories.group.errors import ( + GroupAlreadyExistsError, + GroupNotFoundError, +) +from foxops.database.repositories.incarnation.errors import IncarnationNotFoundError +from foxops.database.repositories.user.errors import ( + UserAlreadyExistsError, + UserNotFoundError, +) +from foxops.errors import FoxopsError, FoxopsUserError from foxops.logger import get_logger #: Holds the module logger instance logger = get_logger(__name__) +EXCEPTION_TO_STATUS_CODE = { + # Incarnation errors + IncarnationNotFoundError: status.HTTP_404_NOT_FOUND, + # Group errors + GroupNotFoundError: status.HTTP_404_NOT_FOUND, + GroupAlreadyExistsError: status.HTTP_409_CONFLICT, + # Change errors + ChangeNotFoundError: status.HTTP_404_NOT_FOUND, + ChangeCommitAlreadyPushedError: status.HTTP_409_CONFLICT, + IncarnationHasNoChangesError: status.HTTP_400_BAD_REQUEST, + # User errors + UserNotFoundError: status.HTTP_404_NOT_FOUND, + UserAlreadyExistsError: status.HTTP_409_CONFLICT, +} + async def validation_exception_handler(_: Request, exc: RequestValidationError): return JSONResponse(status_code=status.HTTP_400_BAD_REQUEST, content={"message": str(exc)}) @@ -18,12 +47,22 @@ async def foxops_user_error(_: Request, exc: FoxopsUserError): return JSONResponse(status_code=status.HTTP_400_BAD_REQUEST, content={"message": str(exc)}) +async def catch_all_foxops_exception(_: Request, exc: FoxopsError): + if exc.__class__ in EXCEPTION_TO_STATUS_CODE: + return JSONResponse( + status_code=EXCEPTION_TO_STATUS_CODE[exc.__class__], + content={"message": str(exc)}, + ) + return JSONResponse(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, content={"message": str(exc)}) + + async def catch_all(_: Request, exc: Exception): logger.exception(str(exc)) return JSONResponse(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, content={"message": str(exc)}) __error_handlers__ = { + FoxopsError: catch_all_foxops_exception, RequestValidationError: validation_exception_handler, FoxopsUserError: foxops_user_error, Exception: catch_all, diff --git a/src/foxops/routers/changes.py b/src/foxops/routers/changes.py index ddfce4b8..f0808637 100644 --- a/src/foxops/routers/changes.py +++ b/src/foxops/routers/changes.py @@ -2,15 +2,15 @@ from datetime import datetime from typing import Annotated, Self -from fastapi import APIRouter, Depends, HTTPException, Path, status +from fastapi import APIRouter, Depends, Path, Response, status from pydantic import BaseModel -from foxops.database.repositories.change.errors import ChangeNotFoundError from foxops.database.repositories.change.model import ChangeType as DatabaseChangeType from foxops.dependencies import authorization, get_change_service from foxops.engine import TemplateData from foxops.hosters.types import MergeRequestStatus from foxops.models.change import Change, ChangeWithMergeRequest +from foxops.models.errors import ApiError from foxops.models.user import User from foxops.services.authorization import AuthorizationService from foxops.services.change import CannotRepairChangeException, ChangeService @@ -23,10 +23,7 @@ async def get_change_id( revision: Annotated[int, Path(description="Change revision within the given incarnation")], change_service: Annotated[ChangeService, Depends(get_change_service)], ) -> int: - try: - return await change_service.get_change_id_by_revision(incarnation_id, revision) - except ChangeNotFoundError: - raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Change not found") + return await change_service.get_change_id_by_revision(incarnation_id, revision) async def get_change( @@ -143,7 +140,7 @@ async def list_changes( @router.get( "/{revision}", responses={ - status.HTTP_404_NOT_FOUND: {"description": "Change not found"}, + status.HTTP_404_NOT_FOUND: {"description": "Change not found", "model": ApiError}, }, ) async def get_change_details( @@ -159,15 +156,21 @@ async def get_change_details( "This can happen if the change was started in foxops, but then pushing the change to the " "incarnation repository failed, for example because the hoster was down.", responses={ - status.HTTP_404_NOT_FOUND: {"description": "Change not found"}, - status.HTTP_422_UNPROCESSABLE_ENTITY: {"description": "The change cannot be repaired automatically"}, + status.HTTP_204_NO_CONTENT: {"description": "Change fixed"}, + status.HTTP_404_NOT_FOUND: {"description": "Change not found", "model": ApiError}, + status.HTTP_422_UNPROCESSABLE_ENTITY: { + "description": "The change cannot be repaired automatically", + "model": ApiError, + }, }, ) async def fix_incomplete_change( + response: Response, change_id: Annotated[int, Depends(get_change_id)], change_service: Annotated[ChangeService, Depends(get_change_service)], ): try: await change_service.update_incomplete_change(change_id) except CannotRepairChangeException as e: - raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=str(e)) + response.status_code = status.HTTP_422_UNPROCESSABLE_ENTITY + return ApiError(message=str(e)) diff --git a/src/foxops/routers/incarnations.py b/src/foxops/routers/incarnations.py index 2f9ff443..27ba9343 100644 --- a/src/foxops/routers/incarnations.py +++ b/src/foxops/routers/incarnations.py @@ -3,9 +3,6 @@ from fastapi import APIRouter, Depends, Response, status from pydantic import BaseModel, model_validator -from foxops.database.repositories.group.errors import GroupNotFoundError -from foxops.database.repositories.incarnation.errors import IncarnationNotFoundError -from foxops.database.repositories.user.errors import UserNotFoundError from foxops.dependencies import ( authorization, get_change_service, @@ -178,11 +175,7 @@ async def read_incarnation( change_service: ChangeService = Depends(get_change_service), ): """Returns the details of the incarnation from the inventory.""" - try: - return await change_service.get_incarnation_with_details(incarnation_id) - except IncarnationNotFoundError as exc: - response.status_code = status.HTTP_404_NOT_FOUND - return ApiError(message=str(exc)) + return await change_service.get_incarnation_with_details(incarnation_id) class IncarnationResetRequest(BaseModel): @@ -236,9 +229,6 @@ async def reset_incarnation( message=f"could not initialize the incarnation as the provided template data " f"is invalid: {'; '.join(error_messages)}" ) - except IncarnationNotFoundError as exc: - response.status_code = status.HTTP_404_NOT_FOUND - return ApiError(message=str(exc)) except ChangeRejectedDueToNoChanges: response.status_code = status.HTTP_422_UNPROCESSABLE_ENTITY return ApiError(message="The incarnation does not have any customizations. Nothing to reset.") @@ -279,9 +269,6 @@ async def _create_change( message=f"could not initialize the incarnation as the provided template data " f"is invalid: {'; '.join(error_messages)}" ) - except IncarnationNotFoundError as exc: - response.status_code = status.HTTP_404_NOT_FOUND - return ApiError(message=str(exc)) except ChangeRejectedDueToPreviousUnfinishedChange: response.status_code = status.HTTP_409_CONFLICT return ApiError(message="There is a previous change that is still open. Please merge/close it first.") @@ -350,32 +337,11 @@ async def update_incarnation( """ await incarnation_service.remove_all_permissions(incarnation_id) - try: - await incarnation_service.set_user_permissions(incarnation_id, request.user_permissions) - except UserNotFoundError as exc: - response.status_code = status.HTTP_404_NOT_FOUND - return ApiError(message=str(exc)) - except IncarnationNotFoundError as exc: - response.status_code = status.HTTP_404_NOT_FOUND - return ApiError(message=str(exc)) + await incarnation_service.set_user_permissions(incarnation_id, request.user_permissions) - try: - await incarnation_service.set_group_permissions(incarnation_id, request.group_permissions) - except GroupNotFoundError as exc: - response.status_code = status.HTTP_404_NOT_FOUND - return ApiError(message=str(exc)) - except IncarnationNotFoundError as exc: - response.status_code = status.HTTP_404_NOT_FOUND - return ApiError(message=str(exc)) + await incarnation_service.set_group_permissions(incarnation_id, request.group_permissions) - try: - await incarnation_service.set_owner(incarnation_id, request.owner_id) - except UserNotFoundError as exc: - response.status_code = status.HTTP_404_NOT_FOUND - return ApiError(message=str(exc)) - except IncarnationNotFoundError as exc: - response.status_code = status.HTTP_404_NOT_FOUND - return ApiError(message=str(exc)) + await incarnation_service.set_owner(incarnation_id, request.owner_id) return await _create_change( incarnation_id=incarnation_id, @@ -410,7 +376,8 @@ def check_either_version_or_data_change_requested(self) -> Self: and self.owner_id is None ): raise ValueError( - "One of the following fields must be set: requested_version, requested_data, user_permission, group_permission" + "One of the following fields must be set: requested_version, " + "requested_data, user_permission, group_permission, owner_id" ) if (self.requested_data is not None or self.requested_version is not None) and self.automerge is None: @@ -461,36 +428,15 @@ async def patch_incarnation( requested_data = request.requested_data or {} if request.group_permissions is not None: - try: - await incarnation_service.remove_all_group_permissions(incarnation_id) - await incarnation_service.set_group_permissions(incarnation_id, request.group_permissions) - except GroupNotFoundError as exc: - response.status_code = status.HTTP_404_NOT_FOUND - return ApiError(message=str(exc)) - except IncarnationNotFoundError as exc: - response.status_code = status.HTTP_404_NOT_FOUND - return ApiError(message=str(exc)) + await incarnation_service.remove_all_group_permissions(incarnation_id) + await incarnation_service.set_group_permissions(incarnation_id, request.group_permissions) if request.user_permissions is not None: - try: - await incarnation_service.remove_all_user_permissions(incarnation_id) - await incarnation_service.set_user_permissions(incarnation_id, request.user_permissions) - except UserNotFoundError as exc: - response.status_code = status.HTTP_404_NOT_FOUND - return ApiError(message=str(exc)) - except IncarnationNotFoundError as exc: - response.status_code = status.HTTP_404_NOT_FOUND - return ApiError(message=str(exc)) + await incarnation_service.remove_all_user_permissions(incarnation_id) + await incarnation_service.set_user_permissions(incarnation_id, request.user_permissions) if request.owner_id is not None: - try: - await incarnation_service.set_owner(incarnation_id, request.owner_id) - except UserNotFoundError as exc: - response.status_code = status.HTTP_404_NOT_FOUND - return ApiError(message=str(exc)) - except IncarnationNotFoundError as exc: - response.status_code = status.HTTP_404_NOT_FOUND - return ApiError(message=str(exc)) + await incarnation_service.set_owner(incarnation_id, request.owner_id) if request.requested_version is not None or request.requested_data is not None: return await _create_change( @@ -503,12 +449,8 @@ async def patch_incarnation( change_service=change_service, initialized_by=authorization_serive.current_user.id, ) - else: - try: - return await change_service.get_incarnation_with_details(incarnation_id) - except IncarnationNotFoundError as exc: - response.status_code = status.HTTP_404_NOT_FOUND - return ApiError(message=str(exc)) + + return await change_service.get_incarnation_with_details(incarnation_id) @router.delete( @@ -526,9 +468,9 @@ async def patch_incarnation( "model": ApiError, }, }, + status_code=status.HTTP_204_NO_CONTENT, ) async def delete_incarnation( - response: Response, incarnation_id: int, incarnation_service: IncarnationService = Depends(get_incarnation_service), ): @@ -540,11 +482,7 @@ async def delete_incarnation( incarnation from the inventory. """ - try: - incarnation = await incarnation_service.get_by_id(incarnation_id) - except IncarnationNotFoundError as exc: - response.status_code = status.HTTP_404_NOT_FOUND - return ApiError(message=str(exc)) + incarnation = await incarnation_service.get_by_id(incarnation_id) await incarnation_service.delete(incarnation) return Response(status_code=status.HTTP_204_NO_CONTENT) @@ -564,19 +502,13 @@ async def delete_incarnation( }, ) async def diff_incarnation( - response: Response, incarnation_id: int, change_service: ChangeService = Depends(get_change_service), ): """Returns the diff which shows all changes manually applied to the incarnation.""" - try: - diff = await change_service.diff_incarnation(incarnation_id) + diff = await change_service.diff_incarnation(incarnation_id) - return Response( - content=diff, - media_type="text/plain", - ) - - except IncarnationNotFoundError as exc: - response.status_code = status.HTTP_404_NOT_FOUND - return ApiError(message=str(exc)) + return Response( + content=diff, + media_type="text/plain", + ) From 088c7c86a5886041386887ebb82d96685966806e Mon Sep 17 00:00:00 2001 From: Yannick Mueller Date: Thu, 27 Mar 2025 13:05:25 +0100 Subject: [PATCH 08/23] Update auth endpoint to return user info --- src/foxops/routers/auth.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/foxops/routers/auth.py b/src/foxops/routers/auth.py index e7c487ab..3345b7ed 100644 --- a/src/foxops/routers/auth.py +++ b/src/foxops/routers/auth.py @@ -1,16 +1,13 @@ from fastapi import APIRouter, Depends -from fastapi.responses import PlainTextResponse from foxops.dependencies import authorization +from foxops.models.user import UserWithGroups +from foxops.services.authorization import AuthorizationService #: Holds the router for the version endpoint router = APIRouter(prefix="/auth", tags=["authentication"]) -@router.get( - "/test", - response_class=PlainTextResponse, - dependencies=[Depends(authorization)], -) -def test_authentication_route(): - return "OK" +@router.get("/test") +def test_authentication_route(authorization_service: AuthorizationService = Depends(authorization)) -> UserWithGroups: + return authorization_service.current_user From 65b5ee490fff35792dc6cd0c31e4ff9b4e04267c Mon Sep 17 00:00:00 2001 From: Yannick Mueller Date: Thu, 27 Mar 2025 13:27:22 +0100 Subject: [PATCH 09/23] Fix performance bug in list incarnation --- src/foxops/database/repositories/change/model.py | 6 +++++- src/foxops/database/repositories/change/repository.py | 6 +++++- src/foxops/services/change.py | 4 ++-- tests/routers/test_auth.py | 2 +- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/foxops/database/repositories/change/model.py b/src/foxops/database/repositories/change/model.py index 3e1751d1..35d91a66 100644 --- a/src/foxops/database/repositories/change/model.py +++ b/src/foxops/database/repositories/change/model.py @@ -57,5 +57,9 @@ class IncarnationWithChangesSummary(BaseModel): requested_version: str merge_request_id: str | None created_at: datetime - owner: int + + owner_id: int + owner_username: str + owner_is_admin: bool + model_config = ConfigDict(from_attributes=True) diff --git a/src/foxops/database/repositories/change/repository.py b/src/foxops/database/repositories/change/repository.py index 601ba78e..fb70bf58 100644 --- a/src/foxops/database/repositories/change/repository.py +++ b/src/foxops/database/repositories/change/repository.py @@ -16,7 +16,7 @@ ChangeType, IncarnationWithChangesSummary, ) -from foxops.database.schema import change, incarnations +from foxops.database.schema import change, incarnations, user from foxops.errors import IncarnationNotFoundError from foxops.logger import get_logger @@ -205,6 +205,9 @@ def _incarnations_with_changes_summary_query(self): alias_change.c.commit_sha, alias_change.c.merge_request_id, alias_change.c.created_at, + user.c.username.label("owner_username"), + user.c.is_admin.label("owner_is_admin"), + user.c.id.label("owner_id"), ) .select_from(incarnations) # join incarnations with the corresponding latest change @@ -221,6 +224,7 @@ def _incarnations_with_changes_summary_query(self): # where the joined `change` is already the latest one .where(alias_change_newer.c.id.is_(None)) .order_by(incarnations.c.id) + .join(user, user.c.id == incarnations.c.owner) ), ) diff --git a/src/foxops/services/change.py b/src/foxops/services/change.py index 6a6c2463..ec2490b7 100644 --- a/src/foxops/services/change.py +++ b/src/foxops/services/change.py @@ -125,7 +125,7 @@ async def _incarnation_with_latest_change_details_from_dbobj( dbobj.incarnation_repository, dbobj.merge_request_id ) - owner = await self._user_repository.get_by_id(dbobj.owner) + owner = User(id=dbobj.owner_id, username=dbobj.owner_username, is_admin=dbobj.owner_is_admin) return IncarnationWithLatestChangeDetails( id=dbobj.id, @@ -137,7 +137,7 @@ async def _incarnation_with_latest_change_details_from_dbobj( requested_version=dbobj.requested_version, created_at=dbobj.created_at, commit_sha=dbobj.commit_sha, - owner=User.model_validate(owner), + owner=owner, commit_url=await self._hoster.get_commit_url(dbobj.incarnation_repository, dbobj.commit_sha), merge_request_id=dbobj.merge_request_id, merge_request_url=merge_request_url, diff --git a/tests/routers/test_auth.py b/tests/routers/test_auth.py index 712be76a..6a700258 100644 --- a/tests/routers/test_auth.py +++ b/tests/routers/test_auth.py @@ -74,4 +74,4 @@ async def test_allow_access_if_token_is_correct(app: FastAPI, static_api_token: # THEN assert response.status_code == status.HTTP_200_OK - assert response.text == "OK" + assert response.text From 30bb6a046b80908cdc9f8ddb3b02508f73016dd9 Mon Sep 17 00:00:00 2001 From: Yannick Mueller Date: Thu, 27 Mar 2025 15:07:37 +0100 Subject: [PATCH 10/23] List endpoint only returns Incarnations with access --- .../repositories/change/repository.py | 74 ++++++++++++++----- .../repositories/incarnation/repository.py | 18 +++++ src/foxops/routers/incarnations.py | 26 ++++++- src/foxops/services/authorization.py | 23 +++++- src/foxops/services/change.py | 12 ++- src/foxops/services/incarnation.py | 6 ++ tests/conftest.py | 3 + tests/routers/test_auth.py | 16 +++- 8 files changed, 147 insertions(+), 31 deletions(-) diff --git a/src/foxops/database/repositories/change/repository.py b/src/foxops/database/repositories/change/repository.py index fb70bf58..4755ba8c 100644 --- a/src/foxops/database/repositories/change/repository.py +++ b/src/foxops/database/repositories/change/repository.py @@ -16,7 +16,13 @@ ChangeType, IncarnationWithChangesSummary, ) -from foxops.database.schema import change, incarnations, user +from foxops.database.schema import ( + change, + group_incarnation_permission, + incarnations, + user, + user_incarnation_permission, +) from foxops.errors import IncarnationNotFoundError from foxops.logger import get_logger @@ -185,33 +191,37 @@ async def get_latest_change_for_incarnation(self, incarnation_id: int) -> Change else: return ChangeInDB.model_validate(row) - def _incarnations_with_changes_summary_query(self): + def _incarnations_with_changes_summary_query( + self, incarnation_id: set[int] | None = None, owner_id: int | None = None + ): alias_change = change.alias("change") alias_change_newer = change.alias("change_newer") + query = select( + incarnations.c.id, + incarnations.c.incarnation_repository, + incarnations.c.target_directory, + incarnations.c.template_repository, + incarnations.c.owner, + alias_change.c.revision, + alias_change.c.type, + alias_change.c.requested_version, + alias_change.c.commit_sha, + alias_change.c.merge_request_id, + alias_change.c.created_at, + user.c.username.label("owner_username"), + user.c.is_admin.label("owner_is_admin"), + user.c.id.label("owner_id"), + ).select_from(incarnations) + + if incarnation_id is not None and owner_id is not None: + query = query.where((incarnations.c.id.in_(incarnation_id)) | (incarnations.c.owner == owner_id)) return ( incarnations.c, alias_change.c, ( - select( - incarnations.c.id, - incarnations.c.incarnation_repository, - incarnations.c.target_directory, - incarnations.c.template_repository, - incarnations.c.owner, - alias_change.c.revision, - alias_change.c.type, - alias_change.c.requested_version, - alias_change.c.commit_sha, - alias_change.c.merge_request_id, - alias_change.c.created_at, - user.c.username.label("owner_username"), - user.c.is_admin.label("owner_is_admin"), - user.c.id.label("owner_id"), - ) - .select_from(incarnations) # join incarnations with the corresponding latest change - .join(alias_change, alias_change.c.incarnation_id == incarnations.c.id) + query.join(alias_change, alias_change.c.incarnation_id == incarnations.c.id) .join( alias_change_newer, and_( @@ -235,6 +245,30 @@ async def list_incarnations_with_changes_summary(self) -> AsyncIterator[Incarnat for row in await conn.execute(query): yield IncarnationWithChangesSummary.model_validate(row) + async def list_incarnations_with_changes_summary_and_access( + self, user_id: int, group_ids: list[int] + ) -> AsyncIterator[IncarnationWithChangesSummary]: + user_query = select(user_incarnation_permission.c.incarnation_id).where( + (user_incarnation_permission.c.user_id == user_id) + ) + + group_query = select(group_incarnation_permission.c.incarnation_id).where( + group_incarnation_permission.c.group_id.in_(group_ids) + ) + + async with self.engine.connect() as conn: + user_result = conn.execute(user_query) + group_result = conn.execute(group_query) + + incarnation_ids = set(row[0] for row in await user_result) + + incarnation_ids.update(row[0] for row in await group_result) + + _, _, query = self._incarnations_with_changes_summary_query(incarnation_ids, user_id) + + for row in await conn.execute(query): + yield IncarnationWithChangesSummary.model_validate(row) + async def get_incarnation_by_repo_and_target_dir( self, incarnation_repository: str, target_directory: str ) -> IncarnationWithChangesSummary: diff --git a/src/foxops/database/repositories/incarnation/repository.py b/src/foxops/database/repositories/incarnation/repository.py index bbfba764..081018ee 100644 --- a/src/foxops/database/repositories/incarnation/repository.py +++ b/src/foxops/database/repositories/incarnation/repository.py @@ -117,6 +117,24 @@ async def get_user_permissions(self, incarnation_id: int) -> List[UserPermission rows = await conn.execute(query) return [UserPermissionInDB.model_validate(row) for row in rows] + async def get_user_ids_with_permission(self, incarnation_id: int) -> List[int]: + query = select(user_incarnation_permission.c.user_id).where( + (user_incarnation_permission.c.incarnation_id == incarnation_id) + ) + + async with self.engine.begin() as conn: + rows = await conn.execute(query) + return [row[0] for row in rows] + + async def get_group_ids_with_permission(self, incarnation_id: int) -> List[int]: + query = select(group_incarnation_permission.c.group_id).where( + (group_incarnation_permission.c.incarnation_id == incarnation_id) + ) + + async with self.engine.begin() as conn: + rows = await conn.execute(query) + return [row[0] for row in rows] + async def set_user_permissions(self, incarnation_id: int, user_permissions: List[UserPermission]): query = insert(user_incarnation_permission).values( [ diff --git a/src/foxops/routers/incarnations.py b/src/foxops/routers/incarnations.py index 27ba9343..ad3a7175 100644 --- a/src/foxops/routers/incarnations.py +++ b/src/foxops/routers/incarnations.py @@ -61,6 +61,8 @@ async def list_incarnations( incarnation_repository: str | None = None, target_directory: str = ".", change_service: ChangeService = Depends(get_change_service), + incarnation_service: IncarnationService = Depends(get_incarnation_service), + authorization_service: AuthorizationService = Depends(authorization), ): """Returns a list of all known incarnations. @@ -68,17 +70,33 @@ async def list_incarnations( TODO: implement pagination """ - if incarnation_repository is None: + if incarnation_repository is None and authorization_service.admin: return await change_service.list_incarnations() + elif incarnation_repository is None: + return await change_service.list_incarnations_with_user_access(authorization_service.current_user) try: - return [ - await change_service.get_incarnation_by_repo_and_target_directory(incarnation_repository, target_directory) - ] + incarnation = await change_service.get_incarnation_by_repo_and_target_directory( + incarnation_repository, target_directory + ) + except IncarnationNotFoundLegacyError: response.status_code = status.HTTP_404_NOT_FOUND return ApiError(message="No incarnation found for the given repository and target directory") + if authorization_service.admin or authorization_service == incarnation.owner: + return [incarnation] + + user_with_access = await incarnation_service.get_user_ids_with_access(incarnation.id) + groups_with_access = await incarnation_service.get_group_ids_with_access(incarnation.id) + + if authorization_service.current_user.id in user_with_access or any( + group.id in groups_with_access for group in authorization_service.current_user.groups + ): + return [incarnation] + + return [] + class CreateIncarnationRequest(BaseModel): incarnation_repository: str diff --git a/src/foxops/services/authorization.py b/src/foxops/services/authorization.py index 98271224..b3ae0375 100644 --- a/src/foxops/services/authorization.py +++ b/src/foxops/services/authorization.py @@ -1,6 +1,6 @@ from foxops.models.user import UserWithGroups -from foxops.services.group import GroupService -from foxops.services.user import UserService +from foxops.services.group import Group, GroupService +from foxops.services.user import User, UserService class AuthorizationService: @@ -9,11 +9,28 @@ def __init__(self, current_user: UserWithGroups, user_service: UserService, grou self.group_service = group_service self.current_user = current_user - def is_admin(self) -> bool: + @property + def admin(self) -> bool: return self.current_user.is_admin + @property + def id(self) -> int: + return self.current_user.id + def is_in_group(self, group_system_name_or_id: str | int) -> bool: if isinstance(group_system_name_or_id, str): return any(group.system_name == group_system_name_or_id for group in self.current_user.groups) else: return any(group.id == group_system_name_or_id for group in self.current_user.groups) + + def __eq__(self, value: object): + if isinstance(value, UserWithGroups): + return self.current_user == value + + if isinstance(value, User): + return self.id == value.id + + if isinstance(value, Group): + return any(group.id == value.id for group in self.current_user.groups) + + return NotImplemented diff --git a/src/foxops/services/change.py b/src/foxops/services/change.py index ec2490b7..45c09b96 100644 --- a/src/foxops/services/change.py +++ b/src/foxops/services/change.py @@ -33,7 +33,7 @@ IncarnationWithLatestChangeDetails, UserPermission, ) -from foxops.models.user import User +from foxops.models.user import User, UserWithGroups from foxops.utils import get_logger @@ -149,6 +149,16 @@ async def list_incarnations(self) -> list[IncarnationWithLatestChangeDetails]: async for inc in self._change_repository.list_incarnations_with_changes_summary() ] + async def list_incarnations_with_user_access( + self, user: UserWithGroups + ) -> list[IncarnationWithLatestChangeDetails]: + return [ + await self._incarnation_with_latest_change_details_from_dbobj(inc) + async for inc in self._change_repository.list_incarnations_with_changes_summary_and_access( + user.id, [group.id for group in user.groups] + ) + ] + async def get_incarnation_by_repo_and_target_directory( self, repo: str, target_directory: str ) -> IncarnationWithLatestChangeDetails: diff --git a/src/foxops/services/incarnation.py b/src/foxops/services/incarnation.py index 6cb7bbc9..497cb75d 100644 --- a/src/foxops/services/incarnation.py +++ b/src/foxops/services/incarnation.py @@ -117,3 +117,9 @@ async def remove_all_permissions(self, incarnation_id: int) -> None: async def set_owner(self, incarnation_id: int, user_id: int): await self.incarnation_repository.set_owner(incarnation_id, user_id) + + async def get_user_ids_with_access(self, incarnation_id: int) -> list[int]: + return await self.incarnation_repository.get_user_ids_with_permission(incarnation_id) + + async def get_group_ids_with_access(self, incarnation_id: int) -> list[int]: + return await self.incarnation_repository.get_group_ids_with_permission(incarnation_id) diff --git a/tests/conftest.py b/tests/conftest.py index c112e4b4..eb44fbb4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -9,6 +9,7 @@ import pytest from fastapi import FastAPI from httpx import AsyncClient +from sqlalchemy import text from sqlalchemy.ext.asyncio import AsyncEngine from foxops.__main__ import create_app @@ -69,6 +70,8 @@ async def test_async_engine() -> AsyncGenerator[AsyncEngine, None]: async with async_engine.begin() as conn: await conn.run_sync(meta.create_all) + # Default admin user to have all priviliges for testings + await conn.execute(text("INSERT INTO foxops_user (id, username, is_admin) VALUES (1, 'root', 1)")) yield async_engine diff --git a/tests/routers/test_auth.py b/tests/routers/test_auth.py index 6a700258..cf6fe3d7 100644 --- a/tests/routers/test_auth.py +++ b/tests/routers/test_auth.py @@ -2,6 +2,7 @@ from httpx import AsyncClient from foxops.dependencies import group_auth_scheme, user_auth_scheme +from foxops.models.user import UserWithGroups async def test_returns_err_if_authorization_header_is_missing(app: FastAPI): @@ -65,13 +66,22 @@ async def test_returns_err_if_token_is_wrong(app: FastAPI): async def test_allow_access_if_token_is_correct(app: FastAPI, static_api_token: str): - app.dependency_overrides[group_auth_scheme] = lambda: None # Disable group auth for this test + app.dependency_overrides[group_auth_scheme] = lambda: UserWithGroups( + id=1, username="root", groups=[], is_admin=True + ) # Disable group auth for this test app.dependency_overrides[user_auth_scheme] = lambda: None # Disable user auth for this test # WHEN async with AsyncClient(app=app, base_url="http://test", follow_redirects=True) as client: - response = await client.get("/auth/test", headers={"Authorization": f"Bearer {static_api_token}"}) + response = await client.get( + "/auth/test", headers={"Authorization": f"Bearer {static_api_token}", "User": "root"} + ) # THEN assert response.status_code == status.HTTP_200_OK - assert response.text + assert response.json() == { + "id": 1, + "username": "root", + "groups": [], + "is_admin": True, + } From 3f434eb340825b3b3957f6c2f8e7efacbae45255 Mon Sep 17 00:00:00 2001 From: Yannick Mueller Date: Thu, 27 Mar 2025 17:00:47 +0100 Subject: [PATCH 11/23] Access control for incarnation endpoints --- src/foxops/authz.py | 28 +++++++ .../repositories/incarnation/model.py | 14 ++++ .../repositories/incarnation/repository.py | 30 +++++-- src/foxops/dependencies.py | 4 +- src/foxops/error_handlers.py | 14 +++- src/foxops/errors.py | 18 +++++ src/foxops/models/incarnation.py | 8 ++ src/foxops/routers/incarnations.py | 66 ++++++++++++---- src/foxops/services/authorization.py | 78 ++++++++++++++----- src/foxops/services/incarnation.py | 53 ++++++++++++- tests/routers/test_incarnations.py | 21 ++++- 11 files changed, 284 insertions(+), 50 deletions(-) create mode 100644 src/foxops/authz.py diff --git a/src/foxops/authz.py b/src/foxops/authz.py new file mode 100644 index 00000000..d854d777 --- /dev/null +++ b/src/foxops/authz.py @@ -0,0 +1,28 @@ +from fastapi import Depends + +from foxops.dependencies import authorization, get_incarnation_service +from foxops.errors import ResourceForbiddenError +from foxops.services.authorization import AuthorizationService +from foxops.services.incarnation import IncarnationService + + +async def read_access_on_incarnation( + incarnation_id: int, + authorization_service: AuthorizationService = Depends(authorization), + incarnation_service: IncarnationService = Depends(get_incarnation_service), +) -> None: + permissions = await incarnation_service.get_permissions(incarnation_id) + + if not authorization_service.has_read_access(permissions): + raise ResourceForbiddenError + + +async def write_access_on_incarnation( + incarnation_id: int, + authorization_service: AuthorizationService = Depends(authorization), + incarnation_service: IncarnationService = Depends(get_incarnation_service), +) -> None: + permissions = await incarnation_service.get_permissions(incarnation_id) + + if not authorization_service.has_write_access(permissions): + raise ResourceForbiddenError diff --git a/src/foxops/database/repositories/incarnation/model.py b/src/foxops/database/repositories/incarnation/model.py index 48cdb25a..725a4118 100644 --- a/src/foxops/database/repositories/incarnation/model.py +++ b/src/foxops/database/repositories/incarnation/model.py @@ -28,3 +28,17 @@ class UserPermissionInDB(BaseModel): incarnation_id: int type: Permission model_config = ConfigDict(from_attributes=True) + + +class UnresolvedUserPermissionsInDB(BaseModel): + user_id: int + incarnation_id: int + type: Permission + model_config = ConfigDict(from_attributes=True) + + +class UnresolvedGroupPermissionsInDB(BaseModel): + group_id: int + incarnation_id: int + type: Permission + model_config = ConfigDict(from_attributes=True) diff --git a/src/foxops/database/repositories/incarnation/repository.py b/src/foxops/database/repositories/incarnation/repository.py index 081018ee..d6b8330f 100644 --- a/src/foxops/database/repositories/incarnation/repository.py +++ b/src/foxops/database/repositories/incarnation/repository.py @@ -11,6 +11,8 @@ from foxops.database.repositories.incarnation.model import ( GroupPermissionInDB, IncarnationInDB, + UnresolvedGroupPermissionsInDB, + UnresolvedUserPermissionsInDB, UserPermissionInDB, ) from foxops.database.repositories.user.errors import UserNotFoundError @@ -117,23 +119,23 @@ async def get_user_permissions(self, incarnation_id: int) -> List[UserPermission rows = await conn.execute(query) return [UserPermissionInDB.model_validate(row) for row in rows] - async def get_user_ids_with_permission(self, incarnation_id: int) -> List[int]: - query = select(user_incarnation_permission.c.user_id).where( - (user_incarnation_permission.c.incarnation_id == incarnation_id) + async def get_unresolved_user_permissions(self, incarnation_id: int) -> List[UnresolvedUserPermissionsInDB]: + query = select(user_incarnation_permission).where( + user_incarnation_permission.c.incarnation_id == incarnation_id ) async with self.engine.begin() as conn: rows = await conn.execute(query) - return [row[0] for row in rows] + return [UnresolvedUserPermissionsInDB.model_validate(row) for row in rows] - async def get_group_ids_with_permission(self, incarnation_id: int) -> List[int]: - query = select(group_incarnation_permission.c.group_id).where( - (group_incarnation_permission.c.incarnation_id == incarnation_id) + async def get_unresolved_group_permissions(self, incarnation_id: int) -> List[UnresolvedGroupPermissionsInDB]: + query = select(group_incarnation_permission).where( + group_incarnation_permission.c.incarnation_id == incarnation_id ) async with self.engine.begin() as conn: rows = await conn.execute(query) - return [row[0] for row in rows] + return [UnresolvedGroupPermissionsInDB.model_validate(row) for row in rows] async def set_user_permissions(self, incarnation_id: int, user_permissions: List[UserPermission]): query = insert(user_incarnation_permission).values( @@ -187,3 +189,15 @@ async def set_owner(self, incarnation_id: int, user_id: int): raise IncarnationNotFoundError(incarnation_id) from e return IncarnationInDB.model_validate(row) + + async def get_owner_id(self, incarnation_id: int) -> int: + query = select(incarnations.c.owner).where(incarnations.c.id == incarnation_id) + + async with self.engine.begin() as conn: + result = await conn.execute(query) + try: + row = result.one() + except NoResultFound as e: + raise IncarnationNotFoundError(incarnation_id) from e + + return row[0] diff --git a/src/foxops/dependencies.py b/src/foxops/dependencies.py index ed251cab..2a0fa1a3 100644 --- a/src/foxops/dependencies.py +++ b/src/foxops/dependencies.py @@ -250,7 +250,5 @@ def authorization( _static_token: None = Depends(static_token_auth_scheme), _user_schema: None = Depends(user_auth_scheme), current_user: UserWithGroups = Depends(group_auth_scheme), - user_service: UserService = Depends(get_user_service), - group_service: GroupService = Depends(get_group_service), ) -> AuthorizationService: - return AuthorizationService(current_user=current_user, user_service=user_service, group_service=group_service) + return AuthorizationService(current_user=current_user) diff --git a/src/foxops/error_handlers.py b/src/foxops/error_handlers.py index 70c3ac96..1e9bfa08 100644 --- a/src/foxops/error_handlers.py +++ b/src/foxops/error_handlers.py @@ -16,7 +16,13 @@ UserAlreadyExistsError, UserNotFoundError, ) -from foxops.errors import FoxopsError, FoxopsUserError +from foxops.errors import ( + ForbiddenError, + FoxopsError, + FoxopsUserError, + GeneralForbiddenError, + ResourceForbiddenError, +) from foxops.logger import get_logger #: Holds the module logger instance @@ -35,6 +41,10 @@ # User errors UserNotFoundError: status.HTTP_404_NOT_FOUND, UserAlreadyExistsError: status.HTTP_409_CONFLICT, + # Access + ForbiddenError: status.HTTP_403_FORBIDDEN, + GeneralForbiddenError: status.HTTP_403_FORBIDDEN, + ResourceForbiddenError: status.HTTP_403_FORBIDDEN, } @@ -62,8 +72,8 @@ async def catch_all(_: Request, exc: Exception): __error_handlers__ = { - FoxopsError: catch_all_foxops_exception, RequestValidationError: validation_exception_handler, FoxopsUserError: foxops_user_error, + FoxopsError: catch_all_foxops_exception, Exception: catch_all, } diff --git a/src/foxops/errors.py b/src/foxops/errors.py index 2a433f8e..edd1db1b 100644 --- a/src/foxops/errors.py +++ b/src/foxops/errors.py @@ -48,3 +48,21 @@ def __init__( super().__init__( f"Incarnation at '{incarnation_repository}' and target directory '{target_directory}' already initialized." ) + + +class ForbiddenError(FoxopsError): + pass + + +class ResourceForbiddenError(ForbiddenError): + """Exception raised when a user is not allowed to access a certain resource""" + + def __init__(self): + super().__init__("You are not allowed to perfom the action on this resource") + + +class GeneralForbiddenError(ForbiddenError): + """Exception raised when a user is not allowed to perform a certain action""" + + def __init__(self, message: str): + super().__init__(message) diff --git a/src/foxops/models/incarnation.py b/src/foxops/models/incarnation.py index 53995492..a14a84fb 100644 --- a/src/foxops/models/incarnation.py +++ b/src/foxops/models/incarnation.py @@ -101,3 +101,11 @@ class IncarnationWithLatestChangeDetails(BaseModel): merge_request_id: str | None merge_request_url: str | None + + +class IncarnationPermissions(BaseModel): + user_permissions: list[UnresolvedUserPermissions] + group_permissions: list[UnresolvedGroupPermissions] + owner_id: int + + model_config = ConfigDict(from_attributes=True) diff --git a/src/foxops/routers/incarnations.py b/src/foxops/routers/incarnations.py index ad3a7175..80a72622 100644 --- a/src/foxops/routers/incarnations.py +++ b/src/foxops/routers/incarnations.py @@ -3,6 +3,7 @@ from fastapi import APIRouter, Depends, Response, status from pydantic import BaseModel, model_validator +from foxops.authz import read_access_on_incarnation, write_access_on_incarnation from foxops.dependencies import ( authorization, get_change_service, @@ -11,6 +12,7 @@ ) from foxops.engine import TemplateData from foxops.engine.errors import ProvidedTemplateDataInvalidError +from foxops.errors import GeneralForbiddenError from foxops.errors import IncarnationNotFoundError as IncarnationNotFoundLegacyError from foxops.hosters import Hoster from foxops.logger import bind, get_logger @@ -39,6 +41,10 @@ logger = get_logger(__name__) +class IncarnationWithPermissions(IncarnationWithDetails): + current_user_permissions: dict[str, bool] + + @router.get( "", responses={ @@ -84,18 +90,13 @@ async def list_incarnations( response.status_code = status.HTTP_404_NOT_FOUND return ApiError(message="No incarnation found for the given repository and target directory") - if authorization_service.admin or authorization_service == incarnation.owner: - return [incarnation] + permissions = await incarnation_service.get_permissions(incarnation.id) - user_with_access = await incarnation_service.get_user_ids_with_access(incarnation.id) - groups_with_access = await incarnation_service.get_group_ids_with_access(incarnation.id) - - if authorization_service.current_user.id in user_with_access or any( - group.id in groups_with_access for group in authorization_service.current_user.groups - ): + if authorization_service.has_read_access(permissions): return [incarnation] - return [] + # We don't want to leak metadata to prevent side channel attacks + return ApiError(message="No incarnation found for the given repository and target directory") class CreateIncarnationRequest(BaseModel): @@ -186,14 +187,31 @@ async def create_incarnation( "model": ApiError, }, }, + dependencies=[Depends(read_access_on_incarnation)], ) async def read_incarnation( - response: Response, incarnation_id: int, + show_permissions: bool = False, change_service: ChangeService = Depends(get_change_service), + authorization_service: AuthorizationService = Depends(authorization), ): """Returns the details of the incarnation from the inventory.""" - return await change_service.get_incarnation_with_details(incarnation_id) + incarnation = await change_service.get_incarnation_with_details(incarnation_id) + + if not show_permissions: + return incarnation + + can_write = authorization_service.has_write_access(incarnation) + + return IncarnationWithPermissions( + **incarnation.model_dump(), + current_user_permissions={ + "can_read": True, + "can_update": can_write, + "can_delete": can_write, + "can_reset": can_write, + }, + ) class IncarnationResetRequest(BaseModel): @@ -223,6 +241,7 @@ class IncarnationResetResponse(BaseModel): "model": ApiError, }, }, + dependencies=[Depends(write_access_on_incarnation)], ) async def reset_incarnation( incarnation_id: int, @@ -338,6 +357,7 @@ class UpdateIncarnationRequest(BaseModel): "model": ApiError, }, }, + dependencies=[Depends(write_access_on_incarnation)], ) async def update_incarnation( response: Response, @@ -354,12 +374,20 @@ async def update_incarnation( reusing the previously set variable values), use the PATCH endpoint instead. """ + old_incarnation = await incarnation_service.get_by_id(incarnation_id) + await incarnation_service.remove_all_permissions(incarnation_id) await incarnation_service.set_user_permissions(incarnation_id, request.user_permissions) await incarnation_service.set_group_permissions(incarnation_id, request.group_permissions) - await incarnation_service.set_owner(incarnation_id, request.owner_id) + if old_incarnation.owner.id != request.owner_id: + if authorization_serive.admin or old_incarnation.owner.id == authorization_serive.id: + await incarnation_service.set_owner(incarnation_id, request.owner_id) + else: + raise GeneralForbiddenError( + "Updating the 'owner' field can only be performed by the current owner or an administator." + ) return await _create_change( incarnation_id=incarnation_id, @@ -428,6 +456,7 @@ def check_either_version_or_data_change_requested(self) -> Self: "model": ApiError, }, }, + dependencies=[Depends(write_access_on_incarnation)], ) async def patch_incarnation( response: Response, @@ -445,6 +474,8 @@ async def patch_incarnation( requested_data = request.requested_data or {} + old_incarnation = await incarnation_service.get_by_id(incarnation_id) + if request.group_permissions is not None: await incarnation_service.remove_all_group_permissions(incarnation_id) await incarnation_service.set_group_permissions(incarnation_id, request.group_permissions) @@ -454,7 +485,12 @@ async def patch_incarnation( await incarnation_service.set_user_permissions(incarnation_id, request.user_permissions) if request.owner_id is not None: - await incarnation_service.set_owner(incarnation_id, request.owner_id) + if authorization_serive.admin or authorization_serive.id == old_incarnation.owner.id: + await incarnation_service.set_owner(incarnation_id, request.owner_id) + else: + raise GeneralForbiddenError( + "Updating the 'owner' field can only be performed by the current owner or an administator." + ) if request.requested_version is not None or request.requested_data is not None: return await _create_change( @@ -487,6 +523,7 @@ async def patch_incarnation( }, }, status_code=status.HTTP_204_NO_CONTENT, + dependencies=[Depends(write_access_on_incarnation)], ) async def delete_incarnation( incarnation_id: int, @@ -499,7 +536,6 @@ async def delete_incarnation( This won't delete the incarnation repository itself, but only deletes the incarnation from the inventory. """ - incarnation = await incarnation_service.get_by_id(incarnation_id) await incarnation_service.delete(incarnation) @@ -518,12 +554,14 @@ async def delete_incarnation( "model": ApiError, }, }, + dependencies=[Depends(read_access_on_incarnation)], ) async def diff_incarnation( incarnation_id: int, change_service: ChangeService = Depends(get_change_service), ): """Returns the diff which shows all changes manually applied to the incarnation.""" + diff = await change_service.diff_incarnation(incarnation_id) return Response( diff --git a/src/foxops/services/authorization.py b/src/foxops/services/authorization.py index b3ae0375..0488712c 100644 --- a/src/foxops/services/authorization.py +++ b/src/foxops/services/authorization.py @@ -1,12 +1,10 @@ +from foxops.database.schema import Permission +from foxops.models.incarnation import IncarnationPermissions, IncarnationWithDetails from foxops.models.user import UserWithGroups -from foxops.services.group import Group, GroupService -from foxops.services.user import User, UserService class AuthorizationService: - def __init__(self, current_user: UserWithGroups, user_service: UserService, group_service: GroupService) -> None: - self.user_service = user_service - self.group_service = group_service + def __init__(self, current_user: UserWithGroups) -> None: self.current_user = current_user @property @@ -17,20 +15,64 @@ def admin(self) -> bool: def id(self) -> int: return self.current_user.id - def is_in_group(self, group_system_name_or_id: str | int) -> bool: - if isinstance(group_system_name_or_id, str): - return any(group.system_name == group_system_name_or_id for group in self.current_user.groups) - else: - return any(group.id == group_system_name_or_id for group in self.current_user.groups) + def _read_access_incarnation_with_details(self, incarnation: IncarnationWithDetails) -> bool: + if self.admin or self.id == incarnation.owner.id: + return True - def __eq__(self, value: object): - if isinstance(value, UserWithGroups): - return self.current_user == value + user_ids_with_access = [p.user.id for p in incarnation.user_permissions] + group_ids_with_access = [p.group.id for p in incarnation.group_permissions] + return self.id in user_ids_with_access or any( + group.id in group_ids_with_access for group in self.current_user.groups + ) - if isinstance(value, User): - return self.id == value.id + def _write_access_incarnation_with_details(self, incarnation: IncarnationWithDetails) -> bool: + if self.admin or self.id == incarnation.owner.id: + return True - if isinstance(value, Group): - return any(group.id == value.id for group in self.current_user.groups) + user_ids_with_access = [p.user.id for p in incarnation.user_permissions if p.type == Permission.WRITE] + group_ids_with_access = [p.group.id for p in incarnation.group_permissions if p.type == Permission.WRITE] - return NotImplemented + return self.id in user_ids_with_access or any( + group.id in group_ids_with_access for group in self.current_user.groups + ) + + def _read_access_incarnation_permissions(self, permissions: IncarnationPermissions) -> bool: + if self.admin or self.id == permissions.owner_id: + return True + + user_ids_with_access = [p.user_id for p in permissions.user_permissions] + group_ids_with_access = [p.group_id for p in permissions.group_permissions] + + return self.id in user_ids_with_access or any( + group.id in group_ids_with_access for group in self.current_user.groups + ) + + def _write_access_incarnation_permissions(self, permissions: IncarnationPermissions) -> bool: + if self.admin or self.id == permissions.owner_id: + return True + + user_ids_with_access = [p.user_id for p in permissions.user_permissions if p.type == Permission.WRITE] + + group_ids_with_access = [p.group_id for p in permissions.group_permissions if p.type == Permission.WRITE] + + return self.id in user_ids_with_access or any( + group.id in group_ids_with_access for group in self.current_user.groups + ) + + def has_read_access(self, object: object) -> bool: + match object: + case IncarnationWithDetails(): + return self._read_access_incarnation_with_details(object) + case IncarnationPermissions(): + return self._read_access_incarnation_permissions(object) + case _: + raise NotImplementedError(f"Read access for {type(object)} is not implemented") + + def has_write_access(self, object: object) -> bool: + match object: + case IncarnationWithDetails(): + return self._write_access_incarnation_with_details(object) + case IncarnationPermissions(): + return self._write_access_incarnation_permissions(object) + case _: + raise NotImplementedError(f"Write access for {type(object)} is not implemented") diff --git a/src/foxops/services/incarnation.py b/src/foxops/services/incarnation.py index 497cb75d..83eb5698 100644 --- a/src/foxops/services/incarnation.py +++ b/src/foxops/services/incarnation.py @@ -4,8 +4,10 @@ from foxops.database.repositories.incarnation.repository import IncarnationRepository from foxops.database.repositories.user.repository import UserRepository from foxops.hosters import Hoster +from foxops.models.group import Group from foxops.models.incarnation import ( GroupPermission, + IncarnationPermissions, UnresolvedGroupPermissions, UnresolvedUserPermissions, UserPermission, @@ -118,8 +120,51 @@ async def remove_all_permissions(self, incarnation_id: int) -> None: async def set_owner(self, incarnation_id: int, user_id: int): await self.incarnation_repository.set_owner(incarnation_id, user_id) - async def get_user_ids_with_access(self, incarnation_id: int) -> list[int]: - return await self.incarnation_repository.get_user_ids_with_permission(incarnation_id) + async def get_user_permissions(self, incarnation_id: int) -> list[UserPermission]: + return [ + UserPermission( + user=User( + id=permission.user_id, + username=permission.user_username, + is_admin=permission.user_is_admin, + ), + type=permission.type, + ) + for permission in await self.incarnation_repository.get_user_permissions(incarnation_id) + ] + + async def get_group_permissions(self, incarnation_id: int) -> list[GroupPermission]: + return [ + GroupPermission( + group=Group( + id=permission.group_id, + system_name=permission.group_system_name, + display_name=permission.group_display_name, + ), + type=permission.type, + ) + for permission in await self.incarnation_repository.get_group_permissions(incarnation_id) + ] - async def get_group_ids_with_access(self, incarnation_id: int) -> list[int]: - return await self.incarnation_repository.get_group_ids_with_permission(incarnation_id) + async def get_unresolved_user_permissions(self, incarnation_id: int) -> list[UnresolvedUserPermissions]: + return [ + UnresolvedUserPermissions.model_validate(permission) + for permission in await self.incarnation_repository.get_unresolved_user_permissions(incarnation_id) + ] + + async def get_unresolved_group_permissions(self, incarnation_id: int) -> list[UnresolvedGroupPermissions]: + return [ + UnresolvedGroupPermissions.model_validate(permission) + for permission in await self.incarnation_repository.get_unresolved_group_permissions(incarnation_id) + ] + + async def get_permissions(self, incarnation_id: int) -> IncarnationPermissions: + user_permissions = await self.get_unresolved_user_permissions(incarnation_id) + group_permissions = await self.get_unresolved_group_permissions(incarnation_id) + owner_id = await self.incarnation_repository.get_owner_id(incarnation_id) + + return IncarnationPermissions( + user_permissions=user_permissions, + group_permissions=group_permissions, + owner_id=owner_id, + ) diff --git a/tests/routers/test_incarnations.py b/tests/routers/test_incarnations.py index ba43ba65..5b7e669e 100644 --- a/tests/routers/test_incarnations.py +++ b/tests/routers/test_incarnations.py @@ -12,10 +12,12 @@ from foxops.database.repositories.incarnation.errors import IncarnationNotFoundError from foxops.database.repositories.incarnation.repository import IncarnationRepository from foxops.database.repositories.user.repository import UserRepository -from foxops.dependencies import get_change_service +from foxops.dependencies import get_change_service, get_incarnation_service from foxops.models.change import Change +from foxops.models.incarnation import IncarnationPermissions from foxops.models.user import User from foxops.services.change import ChangeService, IncarnationAlreadyExists +from foxops.services.incarnation import IncarnationService pytestmark = [pytest.mark.api] @@ -86,6 +88,15 @@ def incarnation_repository_mock(app: FastAPI): return incarnation_repository +@pytest.fixture +def incarnation_service_mock(app: FastAPI): + incarnation_service = Mock(spec=IncarnationService) + + app.dependency_overrides[get_incarnation_service] = lambda: incarnation_service + + return incarnation_service + + async def test_api_get_incarnations_returns_empty_list_for_empty_incarnation_inventory( api_client: AsyncClient, ): @@ -207,8 +218,16 @@ async def test_api_delete_incarnation_removes_incarnation_from_inventory( async def test_api_get_diff_returns_diff_for_incarnation( api_client: AsyncClient, change_service_mock: ChangeService, + incarnation_service_mock: IncarnationService, mocker: MockFixture, ): + incarnation_service_mock.get_permissions = mocker.AsyncMock( + return_value=IncarnationPermissions( + owner_id=1, + user_permissions=[], + group_permissions=[], + ) + ) # type: ignore change_service_mock.diff_incarnation = mocker.AsyncMock(return_value=MOCK_DIFF_OUTPUT) # type: ignore response = await api_client.get("/incarnations/1/diff") From d36a7d216a580a711e5ad11fa395295e2b833548 Mon Sep 17 00:00:00 2001 From: Yannick Mueller Date: Fri, 28 Mar 2025 10:57:49 +0100 Subject: [PATCH 12/23] Update Swagger doc --- src/foxops/__main__.py | 18 ++++++++++++++++-- src/foxops/openapi.py | 3 +++ tests/routers/test_incarnations.py | 4 ++-- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/foxops/__main__.py b/src/foxops/__main__.py index 25bb441e..9c7dc13a 100644 --- a/src/foxops/__main__.py +++ b/src/foxops/__main__.py @@ -1,4 +1,4 @@ -from fastapi import APIRouter, Depends, FastAPI +from fastapi import APIRouter, Depends, FastAPI, status from fastapi.middleware.cors import CORSMiddleware from fastapi.staticfiles import StaticFiles from starlette.responses import FileResponse @@ -7,6 +7,7 @@ from foxops.error_handlers import __error_handlers__ from foxops.logger import get_logger, setup_logging from foxops.middlewares import request_id_middleware, request_time_middleware +from foxops.models.errors import ApiError from foxops.openapi import custom_openapi from foxops.routers import auth, incarnations, not_found, version @@ -46,7 +47,20 @@ def create_app(): public_router.include_router(auth.router) # Add routes to the protected router (authentication required) - protected_router = APIRouter(dependencies=[Depends(authorization)]) + protected_router = APIRouter( + dependencies=[Depends(authorization)], + responses={ + status.HTTP_401_UNAUTHORIZED: { + "description": "The provided API Token is invalid or missing. You have to provide a valid API Token in the format 'Bearer \'.", + "model": ApiError, + }, + status.HTTP_403_FORBIDDEN: { + "description": "You are not allowed to access the requested resource or perform the requested action.", + "model": ApiError, + }, + }, + ) + protected_router.include_router(incarnations.router) app.include_router(public_router) diff --git a/src/foxops/openapi.py b/src/foxops/openapi.py index 5763a595..0824c2b5 100644 --- a/src/foxops/openapi.py +++ b/src/foxops/openapi.py @@ -17,6 +17,9 @@ def _wrapper(): openapi_schema["info"]["x-logo"] = { "url": "https://emoji.beeimg.com/%F0%9F%A6%8A", } + openapi_schema["info"]["contact"] = { + "url": "https://foxops.readthedocs.io/en/latest/", + } app.openapi_schema = openapi_schema return app.openapi_schema diff --git a/tests/routers/test_incarnations.py b/tests/routers/test_incarnations.py index 5b7e669e..f8fc5999 100644 --- a/tests/routers/test_incarnations.py +++ b/tests/routers/test_incarnations.py @@ -221,13 +221,13 @@ async def test_api_get_diff_returns_diff_for_incarnation( incarnation_service_mock: IncarnationService, mocker: MockFixture, ): - incarnation_service_mock.get_permissions = mocker.AsyncMock( + incarnation_service_mock.get_permissions = mocker.AsyncMock( # type: ignore return_value=IncarnationPermissions( owner_id=1, user_permissions=[], group_permissions=[], ) - ) # type: ignore + ) change_service_mock.diff_incarnation = mocker.AsyncMock(return_value=MOCK_DIFF_OUTPUT) # type: ignore response = await api_client.get("/incarnations/1/diff") From 2c9d0f987b8820e383e62426967273d8fccbdf0a Mon Sep 17 00:00:00 2001 From: Yannick Mueller Date: Fri, 28 Mar 2025 13:32:53 +0100 Subject: [PATCH 13/23] Access control for change endpoints --- src/foxops/routers/changes.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/foxops/routers/changes.py b/src/foxops/routers/changes.py index f0808637..4a22fc94 100644 --- a/src/foxops/routers/changes.py +++ b/src/foxops/routers/changes.py @@ -14,6 +14,7 @@ from foxops.models.user import User from foxops.services.authorization import AuthorizationService from foxops.services.change import CannotRepairChangeException, ChangeService +from foxops.authz import read_access_on_incarnation, write_access_on_incarnation router = APIRouter() @@ -90,7 +91,7 @@ def from_service_object(cls, obj: Change | ChangeWithMergeRequest) -> Self: raise NotImplementedError(f"Unknown change type {type(obj)}") -@router.post("", status_code=status.HTTP_201_CREATED) +@router.post("", status_code=status.HTTP_201_CREATED, dependencies=[Depends(write_access_on_incarnation)]) async def create_change( incarnation_id: int, request: CreateChangeRequest, @@ -127,7 +128,7 @@ async def create_change( return ChangeDetails.from_service_object(change) -@router.get("") +@router.get("", dependencies=[Depends(read_access_on_incarnation)]) async def list_changes( incarnation_id: int, change_service: ChangeService = Depends(get_change_service), @@ -142,6 +143,7 @@ async def list_changes( responses={ status.HTTP_404_NOT_FOUND: {"description": "Change not found", "model": ApiError}, }, + dependencies=[Depends(read_access_on_incarnation)], ) async def get_change_details( change: Change = Depends(get_change), @@ -163,6 +165,7 @@ async def get_change_details( "model": ApiError, }, }, + dependencies=[Depends(write_access_on_incarnation)], ) async def fix_incomplete_change( response: Response, From 2407da685674081266863c42e681e0671ad6de95 Mon Sep 17 00:00:00 2001 From: Yannick Mueller Date: Fri, 28 Mar 2025 15:11:18 +0100 Subject: [PATCH 14/23] Add API Endpoints for User --- src/foxops/__main__.py | 6 +- src/foxops/authz.py | 7 + .../database/repositories/user/errors.py | 7 + .../database/repositories/user/repository.py | 32 ++++ src/foxops/error_handlers.py | 2 + src/foxops/errors.py | 8 +- src/foxops/routers/changes.py | 3 +- src/foxops/routers/user.py | 140 ++++++++++++++++++ src/foxops/services/user.py | 27 ++++ tests/routers/test_changes.py | 30 +++- 10 files changed, 253 insertions(+), 9 deletions(-) create mode 100644 src/foxops/routers/user.py diff --git a/src/foxops/__main__.py b/src/foxops/__main__.py index 9c7dc13a..53468b53 100644 --- a/src/foxops/__main__.py +++ b/src/foxops/__main__.py @@ -9,7 +9,7 @@ from foxops.middlewares import request_id_middleware, request_time_middleware from foxops.models.errors import ApiError from foxops.openapi import custom_openapi -from foxops.routers import auth, incarnations, not_found, version +from foxops.routers import auth, incarnations, not_found, user, version #: Holds the module logger instance logger = get_logger(__name__) @@ -51,7 +51,8 @@ def create_app(): dependencies=[Depends(authorization)], responses={ status.HTTP_401_UNAUTHORIZED: { - "description": "The provided API Token is invalid or missing. You have to provide a valid API Token in the format 'Bearer \'.", + "description": ("The provided API Token is invalid or missing. " + "You have to provide a valid API Token in the format 'Bearer \\'."), "model": ApiError, }, status.HTTP_403_FORBIDDEN: { @@ -62,6 +63,7 @@ def create_app(): ) protected_router.include_router(incarnations.router) + protected_router.include_router(user.router) app.include_router(public_router) app.include_router(protected_router) diff --git a/src/foxops/authz.py b/src/foxops/authz.py index d854d777..90b1a8e8 100644 --- a/src/foxops/authz.py +++ b/src/foxops/authz.py @@ -26,3 +26,10 @@ async def write_access_on_incarnation( if not authorization_service.has_write_access(permissions): raise ResourceForbiddenError + + +async def access_to_admin_only( + authorization_service: AuthorizationService = Depends(authorization), +) -> None: + if not authorization_service.current_user.is_admin: + raise ResourceForbiddenError("Only administrators are allowed to access this endpoint") diff --git a/src/foxops/database/repositories/user/errors.py b/src/foxops/database/repositories/user/errors.py index 68af1ab3..8c0ffb37 100644 --- a/src/foxops/database/repositories/user/errors.py +++ b/src/foxops/database/repositories/user/errors.py @@ -18,3 +18,10 @@ def __init__(self, id: Optional[int] = None, username: Optional[str] = None): class UserAlreadyExistsError(Exception): def __init__(self, username: str): super().__init__(f"User with username '{username}' already exists.") + + +class UserOwnerOfResourcesError(FoxopsError): + def __init__(self, id: int): + super().__init__( + f"User with id '{id}' is the owner of resources and cannot be deleted. Delete the resources first." + ) diff --git a/src/foxops/database/repositories/user/repository.py b/src/foxops/database/repositories/user/repository.py index 7b402a3a..4a7e9391 100644 --- a/src/foxops/database/repositories/user/repository.py +++ b/src/foxops/database/repositories/user/repository.py @@ -5,6 +5,7 @@ from foxops.database.repositories.user.errors import ( UserAlreadyExistsError, UserNotFoundError, + UserOwnerOfResourcesError, ) from foxops.database.repositories.user.model import UserInDB from foxops.database.schema import group_user, user @@ -70,3 +71,34 @@ async def get_by_id(self, user_id: int) -> UserInDB: raise UserNotFoundError(id=user_id) from e return UserInDB.model_validate(row) + + async def list_paginated(self, limit: int, offset: int) -> list[UserInDB]: + query = select(user).limit(limit).offset(offset) + + async with self.engine.begin() as conn: + result = await conn.execute(query) + rows = result.all() + + return [UserInDB.model_validate(row) for row in rows] + + async def delete_by_id(self, user_id: int) -> None: + async with self.engine.begin() as conn: + query = user.delete().where(user.c.id == user_id) + + try: + await conn.execute(query) + except IntegrityError as e: + raise UserOwnerOfResourcesError(id=user_id) from e + + async def set_is_admin(self, user_id: int, is_admin: bool) -> UserInDB: + async with self.engine.begin() as conn: + query = user.update().where(user.c.id == user_id).values(is_admin=is_admin).returning(user) + + result = await conn.execute(query) + + try: + row = result.one() + except NoResultFound as e: + raise UserNotFoundError(id=user_id) from e + + return UserInDB.model_validate(row) diff --git a/src/foxops/error_handlers.py b/src/foxops/error_handlers.py index 1e9bfa08..bdaf42f4 100644 --- a/src/foxops/error_handlers.py +++ b/src/foxops/error_handlers.py @@ -15,6 +15,7 @@ from foxops.database.repositories.user.errors import ( UserAlreadyExistsError, UserNotFoundError, + UserOwnerOfResourcesError, ) from foxops.errors import ( ForbiddenError, @@ -41,6 +42,7 @@ # User errors UserNotFoundError: status.HTTP_404_NOT_FOUND, UserAlreadyExistsError: status.HTTP_409_CONFLICT, + UserOwnerOfResourcesError: status.HTTP_409_CONFLICT, # Access ForbiddenError: status.HTTP_403_FORBIDDEN, GeneralForbiddenError: status.HTTP_403_FORBIDDEN, diff --git a/src/foxops/errors.py b/src/foxops/errors.py index edd1db1b..381d7972 100644 --- a/src/foxops/errors.py +++ b/src/foxops/errors.py @@ -1,3 +1,6 @@ +from typing import Optional + + class FoxopsError(Exception): """Base class for all foxops errors.""" @@ -57,8 +60,9 @@ class ForbiddenError(FoxopsError): class ResourceForbiddenError(ForbiddenError): """Exception raised when a user is not allowed to access a certain resource""" - def __init__(self): - super().__init__("You are not allowed to perfom the action on this resource") + def __init__(self, message: Optional[str] = None): + message = message or "You are not allowed to perfom the action on this resource" + super().__init__(message) class GeneralForbiddenError(ForbiddenError): diff --git a/src/foxops/routers/changes.py b/src/foxops/routers/changes.py index 4a22fc94..b6eebb94 100644 --- a/src/foxops/routers/changes.py +++ b/src/foxops/routers/changes.py @@ -5,6 +5,7 @@ from fastapi import APIRouter, Depends, Path, Response, status from pydantic import BaseModel +from foxops.authz import read_access_on_incarnation, write_access_on_incarnation from foxops.database.repositories.change.model import ChangeType as DatabaseChangeType from foxops.dependencies import authorization, get_change_service from foxops.engine import TemplateData @@ -14,7 +15,6 @@ from foxops.models.user import User from foxops.services.authorization import AuthorizationService from foxops.services.change import CannotRepairChangeException, ChangeService -from foxops.authz import read_access_on_incarnation, write_access_on_incarnation router = APIRouter() @@ -81,7 +81,6 @@ class ChangeDetails(BaseModel): @classmethod def from_service_object(cls, obj: Change | ChangeWithMergeRequest) -> Self: - print(obj) match obj: case ChangeWithMergeRequest(): return cls(type=ChangeType.MERGE_REQUEST, **obj.model_dump()) diff --git a/src/foxops/routers/user.py b/src/foxops/routers/user.py new file mode 100644 index 00000000..8347db61 --- /dev/null +++ b/src/foxops/routers/user.py @@ -0,0 +1,140 @@ +from fastapi import APIRouter, Depends, Query, Response, status +from pydantic import BaseModel + +from foxops.authz import access_to_admin_only +from foxops.dependencies import authorization, get_user_service +from foxops.models.errors import ApiError +from foxops.models.user import User, UserWithGroups +from foxops.services.authorization import AuthorizationService +from foxops.services.user import UserService + +router = APIRouter(prefix="/api/user", tags=["user"]) + + +@router.get( + "", + dependencies=[Depends(access_to_admin_only)], + responses={ + status.HTTP_200_OK: { + "description": "List of users", + "model": list[User], + }, + status.HTTP_400_BAD_REQUEST: { + "description": "You exceeded the number of pages", + "model": ApiError, + }, + }, +) +async def list_users( + response: Response, + user_service: UserService = Depends(get_user_service), + limit: int = Query(25, ge=1, le=200, description="Number of users to return"), + page: int = Query(1, ge=1, description="Page number"), +): + users = await user_service.list_users_paginated(limit, page) + + # There is not case, where 0 users exist in the db, since at least + # the user, who is calling the api, has to exist. + if len(users) == 0: + response.status_code = status.HTTP_400_BAD_REQUEST + return ApiError( + message="You exceeded the number of pages", + ) + + return users + + +@router.get( + "/{user_id}", + responses={ + status.HTTP_200_OK: { + "description": "User information", + "model": UserWithGroups, + }, + status.HTTP_404_NOT_FOUND: { + "description": "User not found", + "model": ApiError, + }, + }, + dependencies=[Depends(access_to_admin_only)], +) +async def get_user( + user_id: int, + user_service: UserService = Depends(get_user_service), + resolve_groups: bool = Query(False, description="Resolve the groups of the user"), +): + if resolve_groups: + return await user_service.get_user_by_id_with_groups(user_id) + + return await user_service.get_user_by_id(user_id) + + +@router.delete( + "/{user_id}", + status_code=status.HTTP_204_NO_CONTENT, + responses={ + status.HTTP_204_NO_CONTENT: { + "description": "User deleted", + }, + status.HTTP_400_BAD_REQUEST: { + "description": "You can't delete yourself", + "model": ApiError, + }, + status.HTTP_404_NOT_FOUND: { + "description": "User not found", + "model": ApiError, + }, + status.HTTP_409_CONFLICT: { + "description": "User is still owner of some resources", + "model": ApiError, + }, + }, + dependencies=[Depends(access_to_admin_only)], +) +async def delete_user( + user_id: int, + response: Response, + user_service: UserService = Depends(get_user_service), + authorization_service: AuthorizationService = Depends(authorization), +): + if authorization_service.id == user_id: + response.status_code = status.HTTP_400_BAD_REQUEST + return ApiError(message="You can't delete yourself") + + await user_service.delete_user(user_id) + + +class UserUpdatePatchRequest(BaseModel): + is_admin: bool + + +@router.patch( + "/{user_id}", + responses={ + status.HTTP_200_OK: { + "description": "User information", + "model": UserWithGroups, + }, + status.HTTP_400_BAD_REQUEST: { + "description": "You can't change your own admin status", + "model": ApiError, + }, + status.HTTP_404_NOT_FOUND: { + "description": "User not found", + "model": ApiError, + }, + }, + dependencies=[Depends(access_to_admin_only)], +) +async def update_user( + response: Response, + user_id: int, + request: UserUpdatePatchRequest, + user_service: UserService = Depends(get_user_service), + authorization_service: AuthorizationService = Depends(authorization), +): + if authorization_service.id == user_id: + response.status_code = status.HTTP_400_BAD_REQUEST + return ApiError(message="You can't change your own admin status") + + return await user_service.set_is_admin(user_id, request.is_admin) diff --git a/src/foxops/services/user.py b/src/foxops/services/user.py index d1197b14..f7fda6a8 100644 --- a/src/foxops/services/user.py +++ b/src/foxops/services/user.py @@ -47,5 +47,32 @@ async def get_user_by_username_with_groups(self, username: str) -> UserWithGroup return UserWithGroups(groups=groups, **user.model_dump()) + async def get_user_by_id(self, user_id: int) -> User: + user = await self.user_repository.get_by_id(user_id) + + return User.model_validate(user) + + async def get_user_by_id_with_groups(self, user_id: int) -> UserWithGroups: + user = await self.user_repository.get_by_id(user_id) + + groups = await self.group_repository.get_by_userid(user.id) + + return UserWithGroups(groups=groups, **user.model_dump()) + async def remove_all_groups_from_user(self, user_id: int) -> None: await self.user_repository.remove_all_groups(user_id) + + async def list_users_paginated(self, limit: int, page: int) -> list[User]: + users = await self.user_repository.list_paginated(limit=limit, offset=limit * (page - 1)) + + return [User.model_validate(user) for user in users] + + async def delete_user(self, user_id: int) -> None: + await self.user_repository.get_by_id(user_id) # Validate if the user exists + + await self.user_repository.delete_by_id(user_id) + + async def set_is_admin(self, user_id: int, is_admin: bool) -> User: + user = await self.user_repository.set_is_admin(user_id, is_admin) + + return User.model_validate(user) diff --git a/tests/routers/test_changes.py b/tests/routers/test_changes.py index c078bedc..edbe461c 100644 --- a/tests/routers/test_changes.py +++ b/tests/routers/test_changes.py @@ -5,10 +5,13 @@ from fastapi import FastAPI, status from httpx import AsyncClient -from foxops.dependencies import get_change_service +from foxops.dependencies import get_change_service, get_incarnation_service from foxops.hosters.types import MergeRequestStatus from foxops.models.change import Change, ChangeWithMergeRequest +from foxops.models.incarnation import IncarnationPermissions +from foxops.models.user import User from foxops.services.change import ChangeService +from foxops.services.incarnation import IncarnationService @pytest.fixture @@ -20,7 +23,26 @@ def change_service_mock(app: FastAPI): return change_service -async def test_create_change(api_client: AsyncClient, change_service_mock: ChangeService): +@pytest.fixture +def incarnation_service_mock(app: FastAPI): + incarnation_service = Mock(spec_set=IncarnationService) + + incarnation_service.get_permissions = AsyncMock( # type: ignore + return_value=IncarnationPermissions( + owner_id=1, + user_permissions=[], + group_permissions=[], + ) + ) + + app.dependency_overrides[get_incarnation_service] = lambda: incarnation_service + + return incarnation_service + + +async def test_create_change( + api_client: AsyncClient, change_service_mock: ChangeService, incarnation_service_mock: IncarnationService +): # GIVEN change_service_mock.create_change_direct = AsyncMock( # type: ignore return_value=Change( @@ -46,7 +68,9 @@ async def test_create_change(api_client: AsyncClient, change_service_mock: Chang assert response.json()["revision"] == 2 -async def test_list_changes(api_client: AsyncClient, change_service_mock: ChangeService): +async def test_list_changes( + api_client: AsyncClient, change_service_mock: ChangeService, incarnation_service_mock: IncarnationService +): # GIVEN change_service_mock.list_changes = AsyncMock( # type: ignore return_value=[ From f8519c4aa6eda0a5911983f1c8de0611cdb65242 Mon Sep 17 00:00:00 2001 From: Yannick Mueller Date: Fri, 28 Mar 2025 16:07:11 +0100 Subject: [PATCH 15/23] Add API Endpoints for Group --- src/foxops/__main__.py | 9 +- .../database/repositories/group/repository.py | 32 ++++- .../database/repositories/user/repository.py | 9 ++ src/foxops/dependencies.py | 3 +- src/foxops/models/user.py | 10 ++ src/foxops/routers/group.py | 116 ++++++++++++++++++ src/foxops/routers/user.py | 4 +- src/foxops/services/group.py | 39 +++++- tests/conftest.py | 4 +- tests/routers/test_changes.py | 1 - 10 files changed, 216 insertions(+), 11 deletions(-) create mode 100644 src/foxops/routers/group.py diff --git a/src/foxops/__main__.py b/src/foxops/__main__.py index 53468b53..fcfc49c4 100644 --- a/src/foxops/__main__.py +++ b/src/foxops/__main__.py @@ -9,7 +9,7 @@ from foxops.middlewares import request_id_middleware, request_time_middleware from foxops.models.errors import ApiError from foxops.openapi import custom_openapi -from foxops.routers import auth, incarnations, not_found, user, version +from foxops.routers import auth, group, incarnations, not_found, user, version #: Holds the module logger instance logger = get_logger(__name__) @@ -51,8 +51,10 @@ def create_app(): dependencies=[Depends(authorization)], responses={ status.HTTP_401_UNAUTHORIZED: { - "description": ("The provided API Token is invalid or missing. " - "You have to provide a valid API Token in the format 'Bearer \\'."), + "description": ( + "The provided API Token is invalid or missing. " + "You have to provide a valid API Token in the format 'Bearer \\'." + ), "model": ApiError, }, status.HTTP_403_FORBIDDEN: { @@ -64,6 +66,7 @@ def create_app(): protected_router.include_router(incarnations.router) protected_router.include_router(user.router) + protected_router.include_router(group.router) app.include_router(public_router) app.include_router(protected_router) diff --git a/src/foxops/database/repositories/group/repository.py b/src/foxops/database/repositories/group/repository.py index ee8f7139..fb9aeb84 100644 --- a/src/foxops/database/repositories/group/repository.py +++ b/src/foxops/database/repositories/group/repository.py @@ -40,7 +40,7 @@ async def get_by_system_name(self, system_name: str) -> GroupInDB: return GroupInDB.model_validate(row) async def get_by_userid(self, user_id: int) -> list[GroupInDB]: - query = select(group).join(group_user).where(group_user.c.user_id == user_id) + query = select(group_user, group).where(group_user.c.user_id == user_id).join(group) groups = [] async with self.engine.begin() as conn: @@ -61,3 +61,33 @@ async def get_by_id(self, group_id: int) -> GroupInDB: except NoResultFound as e: raise GroupNotFoundError(id=group_id) from e return GroupInDB.model_validate(row) + + async def list_paginated( + self, + limit: int, + offset: int, + ) -> list[GroupInDB]: + query = select(group).limit(limit).offset(offset) + + async with self.engine.begin() as conn: + result = await conn.execute(query) + + return [GroupInDB.model_validate(row) for row in result] + + async def delete(self, group_id: int) -> None: + async with self.engine.begin() as conn: + query = group.delete().where(group.c.id == group_id) + await conn.execute(query) + + async def set_display_name(self, group_id: int, display_name: str) -> GroupInDB: + async with self.engine.begin() as conn: + query = group.update().where(group.c.id == group_id).values(display_name=display_name).returning(group) + + result = await conn.execute(query) + + try: + row = result.one() + except NoResultFound as e: + raise GroupNotFoundError(id=group_id) from e + + return GroupInDB.model_validate(row) diff --git a/src/foxops/database/repositories/user/repository.py b/src/foxops/database/repositories/user/repository.py index 4a7e9391..6fc536ff 100644 --- a/src/foxops/database/repositories/user/repository.py +++ b/src/foxops/database/repositories/user/repository.py @@ -102,3 +102,12 @@ async def set_is_admin(self, user_id: int, is_admin: bool) -> UserInDB: raise UserNotFoundError(id=user_id) from e return UserInDB.model_validate(row) + + async def get_users_of_group(self, group_id: int) -> list[UserInDB]: + query = select(group_user, user).where(group_user.c.group_id == group_id).join(user) + + async with self.engine.begin() as conn: + result = await conn.execute(query) + rows = result.all() + + return [UserInDB.model_validate(row) for row in rows] diff --git a/src/foxops/dependencies.py b/src/foxops/dependencies.py index 2a0fa1a3..572f06bf 100644 --- a/src/foxops/dependencies.py +++ b/src/foxops/dependencies.py @@ -107,8 +107,9 @@ def get_group_repository(database_engine: AsyncEngine = Depends(get_database_eng def get_group_service( group_repository: GroupRepository = Depends(get_group_repository), + user_repository: UserRepository = Depends(get_user_repository), ) -> GroupService: - return GroupService(group_repository=group_repository) + return GroupService(group_repository=group_repository, user_repository=user_repository) def get_user_service( diff --git a/src/foxops/models/user.py b/src/foxops/models/user.py index 70d2e24c..56378ce8 100644 --- a/src/foxops/models/user.py +++ b/src/foxops/models/user.py @@ -11,5 +11,15 @@ class User(BaseModel): model_config = ConfigDict(from_attributes=True) +class GroupWithUsers(Group): + id: int + system_name: str + display_name: str + + users: list[User] + + model_config = ConfigDict(from_attributes=True) + + class UserWithGroups(User): groups: list[Group] diff --git a/src/foxops/routers/group.py b/src/foxops/routers/group.py new file mode 100644 index 00000000..ea30cc81 --- /dev/null +++ b/src/foxops/routers/group.py @@ -0,0 +1,116 @@ +from fastapi import APIRouter, Depends, Query, Response, status +from pydantic import BaseModel + +from foxops.authz import access_to_admin_only +from foxops.dependencies import get_group_service +from foxops.models.errors import ApiError +from foxops.models.group import Group +from foxops.models.user import GroupWithUsers +from foxops.services.group import GroupService + +router = APIRouter(prefix="/api/group", tags=["group"]) + + +@router.get( + "", + responses={ + status.HTTP_200_OK: { + "description": "List of groups", + "model": list[Group], + }, + status.HTTP_400_BAD_REQUEST: { + "description": "You exceeded the number of pages", + "model": ApiError, + }, + }, + dependencies=[Depends(access_to_admin_only)], +) +async def list_groups( + response: Response, + group_service: GroupService = Depends(get_group_service), + limit: int = Query(25, ge=1, le=200, description="Number of groups to return"), + page: int = Query(1, ge=1, description="Page number"), +): + groups = await group_service.list_groups_paginated(limit, page) + + # It is possible, that 0 groups exist in the db. + # So we don't return a 400 if the user is on the first page + if len(groups) == 0 and page > 1: + response.status_code = status.HTTP_400_BAD_REQUEST + return ApiError( + message="You exceeded the number of pages", + ) + + return groups + + +@router.get( + "/{group_id}", + responses={ + status.HTTP_200_OK: { + "description": "Group information", + "model": GroupWithUsers, + }, + status.HTTP_404_NOT_FOUND: { + "description": "Group not found", + "model": ApiError, + }, + }, + dependencies=[Depends(access_to_admin_only)], +) +async def get_group( + group_id: int, + group_service: GroupService = Depends(get_group_service), + resolve_users: bool = Query(False, description="Resolve the users of the group"), +): + if resolve_users: + return await group_service.get_group_by_id_with_users(group_id) + + return await group_service.get_group_by_id(group_id) + + +@router.delete( + "/{group_id}", + responses={ + status.HTTP_204_NO_CONTENT: { + "description": "Group deleted", + }, + status.HTTP_404_NOT_FOUND: { + "description": "Group not found", + "model": ApiError, + }, + }, + dependencies=[Depends(access_to_admin_only)], + status_code=status.HTTP_204_NO_CONTENT, +) +async def delete_group( + group_id: int, + group_service: GroupService = Depends(get_group_service), +): + await group_service.delete_group(group_id) + + +class GroupPatchRequest(BaseModel): + display_name: str + + +@router.patch( + "/{group_id}", + responses={ + status.HTTP_200_OK: { + "description": "Group information", + "model": Group, + }, + status.HTTP_404_NOT_FOUND: { + "description": "Group not found", + "model": ApiError, + }, + }, + dependencies=[Depends(access_to_admin_only)], +) +async def patch_group( + group_id: int, + request: GroupPatchRequest, + group_service: GroupService = Depends(get_group_service), +): + return await group_service.set_display_name(group_id, request.display_name) diff --git a/src/foxops/routers/user.py b/src/foxops/routers/user.py index 8347db61..162a8b55 100644 --- a/src/foxops/routers/user.py +++ b/src/foxops/routers/user.py @@ -104,7 +104,7 @@ async def delete_user( await user_service.delete_user(user_id) -class UserUpdatePatchRequest(BaseModel): +class UserPatchRequest(BaseModel): is_admin: bool @@ -129,7 +129,7 @@ class UserUpdatePatchRequest(BaseModel): async def update_user( response: Response, user_id: int, - request: UserUpdatePatchRequest, + request: UserPatchRequest, user_service: UserService = Depends(get_user_service), authorization_service: AuthorizationService = Depends(authorization), ): diff --git a/src/foxops/services/group.py b/src/foxops/services/group.py index eecfdba3..bfea57e1 100644 --- a/src/foxops/services/group.py +++ b/src/foxops/services/group.py @@ -1,12 +1,15 @@ from typing import Optional from foxops.database.repositories.group.repository import GroupRepository +from foxops.database.repositories.user.repository import UserRepository from foxops.models.group import Group +from foxops.models.user import GroupWithUsers class GroupService: - def __init__(self, group_repository: GroupRepository) -> None: + def __init__(self, group_repository: GroupRepository, user_repository: UserRepository) -> None: self.group_repository = group_repository + self.user_repository = user_repository async def create_group( self, @@ -27,3 +30,37 @@ async def get_group_by_system_name(self, system_name: str) -> Group: group = await self.group_repository.get_by_system_name(system_name) return Group.model_validate(group) + + async def list_groups_paginated( + self, + limit: int, + page: int, + ) -> list[Group]: + groups = await self.group_repository.list_paginated( + limit, + limit * (page - 1), + ) + + return [Group.model_validate(group) for group in groups] + + async def get_group_by_id(self, group_id: int) -> Group: + group = await self.group_repository.get_by_id(group_id) + + return Group.model_validate(group) + + async def get_group_by_id_with_users(self, group_id: int) -> Group: + group = await self.group_repository.get_by_id(group_id) + + users = await self.user_repository.get_users_of_group(group_id) + + return GroupWithUsers(**group.model_dump(), users=users) + + async def delete_group(self, group_id: int) -> None: + await self.group_repository.get_by_id(group_id) # check if group exists + + await self.group_repository.delete(group_id) + + async def set_display_name(self, group_id: int, display_name: str) -> Group: + group = await self.group_repository.set_display_name(group_id, display_name) + + return Group.model_validate(group) diff --git a/tests/conftest.py b/tests/conftest.py index eb44fbb4..78029384 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -109,8 +109,8 @@ async def user_service(user_repository: UserRepository, group_repository: GroupR @pytest.fixture -async def group_service(group_repository: GroupRepository) -> GroupService: - return GroupService(group_repository) +async def group_service(group_repository: GroupRepository, user_repository: UserRepository) -> GroupService: + return GroupService(group_repository, user_repository) @pytest.fixture diff --git a/tests/routers/test_changes.py b/tests/routers/test_changes.py index edbe461c..53e5a9ee 100644 --- a/tests/routers/test_changes.py +++ b/tests/routers/test_changes.py @@ -9,7 +9,6 @@ from foxops.hosters.types import MergeRequestStatus from foxops.models.change import Change, ChangeWithMergeRequest from foxops.models.incarnation import IncarnationPermissions -from foxops.models.user import User from foxops.services.change import ChangeService from foxops.services.incarnation import IncarnationService From b5d90ed8fbb9d138887e54d38034640fbbcc6d75 Mon Sep 17 00:00:00 2001 From: Yannick Mueller Date: Wed, 2 Apr 2025 09:24:03 +0200 Subject: [PATCH 16/23] Add owner filter to list incarnation --- .../repositories/change/repository.py | 43 +++++++++++++++++-- src/foxops/routers/incarnations.py | 18 +++++--- src/foxops/services/change.py | 30 ++++++++----- 3 files changed, 72 insertions(+), 19 deletions(-) diff --git a/src/foxops/database/repositories/change/repository.py b/src/foxops/database/repositories/change/repository.py index 4755ba8c..d7054136 100644 --- a/src/foxops/database/repositories/change/repository.py +++ b/src/foxops/database/repositories/change/repository.py @@ -192,7 +192,7 @@ async def get_latest_change_for_incarnation(self, incarnation_id: int) -> Change return ChangeInDB.model_validate(row) def _incarnations_with_changes_summary_query( - self, incarnation_id: set[int] | None = None, owner_id: int | None = None + self, incarnation_ids: set[int] | None = None, owner_id: int | None = None, owner_filter: int | None = None ): alias_change = change.alias("change") alias_change_newer = change.alias("change_newer") @@ -213,8 +213,12 @@ def _incarnations_with_changes_summary_query( user.c.id.label("owner_id"), ).select_from(incarnations) - if incarnation_id is not None and owner_id is not None: - query = query.where((incarnations.c.id.in_(incarnation_id)) | (incarnations.c.owner == owner_id)) + if incarnation_ids is not None and owner_id is not None: + query = query.where((incarnations.c.id.in_(incarnation_ids)) | (incarnations.c.owner == owner_id)) + elif incarnation_ids is not None: + query = query.where(incarnations.c.id.in_(incarnation_ids)) + if owner_filter is not None: + query = query.where(incarnations.c.owner == owner_filter) return ( incarnations.c, @@ -245,6 +249,15 @@ async def list_incarnations_with_changes_summary(self) -> AsyncIterator[Incarnat for row in await conn.execute(query): yield IncarnationWithChangesSummary.model_validate(row) + async def list_incarnations_with_changes_summary_of_owner( + self, owner_id: int + ) -> AsyncIterator[IncarnationWithChangesSummary]: + _, _, query = self._incarnations_with_changes_summary_query(owner_filter=owner_id) + + async with self.engine.connect() as conn: + for row in await conn.execute(query): + yield IncarnationWithChangesSummary.model_validate(row) + async def list_incarnations_with_changes_summary_and_access( self, user_id: int, group_ids: list[int] ) -> AsyncIterator[IncarnationWithChangesSummary]: @@ -269,6 +282,30 @@ async def list_incarnations_with_changes_summary_and_access( for row in await conn.execute(query): yield IncarnationWithChangesSummary.model_validate(row) + async def list_incarnations_with_changes_summary_and_access_of_owner( + self, user_id: int, group_ids: list[int], owner_id: int + ) -> AsyncIterator[IncarnationWithChangesSummary]: + user_query = select(user_incarnation_permission.c.incarnation_id).where( + (user_incarnation_permission.c.user_id == user_id) + ) + + group_query = select(group_incarnation_permission.c.incarnation_id).where( + group_incarnation_permission.c.group_id.in_(group_ids) + ) + + async with self.engine.connect() as conn: + user_result = conn.execute(user_query) + group_result = conn.execute(group_query) + + incarnation_ids = set(row[0] for row in await user_result) + + incarnation_ids.update(row[0] for row in await group_result) + + _, _, query = self._incarnations_with_changes_summary_query(incarnation_ids, owner_filter=owner_id) + + for row in await conn.execute(query): + yield IncarnationWithChangesSummary.model_validate(row) + async def get_incarnation_by_repo_and_target_dir( self, incarnation_repository: str, target_directory: str ) -> IncarnationWithChangesSummary: diff --git a/src/foxops/routers/incarnations.py b/src/foxops/routers/incarnations.py index 80a72622..85ccda24 100644 --- a/src/foxops/routers/incarnations.py +++ b/src/foxops/routers/incarnations.py @@ -1,6 +1,6 @@ from typing import Self -from fastapi import APIRouter, Depends, Response, status +from fastapi import APIRouter, Depends, Query, Response, status from pydantic import BaseModel, model_validator from foxops.authz import read_access_on_incarnation, write_access_on_incarnation @@ -64,8 +64,10 @@ class IncarnationWithPermissions(IncarnationWithDetails): ) async def list_incarnations( response: Response, - incarnation_repository: str | None = None, - target_directory: str = ".", + incarnation_repository: str + | None = Query(None, description="The incarnation repository of the incarnation to search for"), + target_directory: str = Query(".", description="The target directory of the incarnation to search for"), + owner: str | None = Query(None, description="List all incarnations for this given user with this username"), change_service: ChangeService = Depends(get_change_service), incarnation_service: IncarnationService = Depends(get_incarnation_service), authorization_service: AuthorizationService = Depends(authorization), @@ -76,10 +78,16 @@ async def list_incarnations( TODO: implement pagination """ + if incarnation_repository is not None and owner is not None: + response.status_code = status.HTTP_400_BAD_REQUEST + return ApiError(message="You can only filter by either `incarnation_repository` or `owner`, but not both") + if incarnation_repository is None and authorization_service.admin: - return await change_service.list_incarnations() + return await change_service.list_incarnations(owner_username=owner) elif incarnation_repository is None: - return await change_service.list_incarnations_with_user_access(authorization_service.current_user) + return await change_service.list_incarnations_with_user_access( + authorization_service.current_user, owner_username=owner + ) try: incarnation = await change_service.get_incarnation_by_repo_and_target_directory( diff --git a/src/foxops/services/change.py b/src/foxops/services/change.py index 45c09b96..dae3349e 100644 --- a/src/foxops/services/change.py +++ b/src/foxops/services/change.py @@ -9,7 +9,7 @@ from datetime import timedelta from pathlib import Path from tempfile import TemporaryDirectory -from typing import AsyncIterator +from typing import AsyncIterator, Optional import foxops.engine as fengine from foxops.database.repositories.change.model import ( @@ -143,21 +143,29 @@ async def _incarnation_with_latest_change_details_from_dbobj( merge_request_url=merge_request_url, ) - async def list_incarnations(self) -> list[IncarnationWithLatestChangeDetails]: - return [ - await self._incarnation_with_latest_change_details_from_dbobj(inc) - async for inc in self._change_repository.list_incarnations_with_changes_summary() - ] + async def list_incarnations(self, owner_username: Optional[str] = None) -> list[IncarnationWithLatestChangeDetails]: + if owner_username is not None: + owner = await self._user_repository.get_by_username(owner_username) + generator = self._change_repository.list_incarnations_with_changes_summary_of_owner(owner_id=owner.id) + else: + generator = self._change_repository.list_incarnations_with_changes_summary() + + return [await self._incarnation_with_latest_change_details_from_dbobj(inc) async for inc in generator] async def list_incarnations_with_user_access( - self, user: UserWithGroups + self, user: UserWithGroups, owner_username: Optional[str] = None ) -> list[IncarnationWithLatestChangeDetails]: - return [ - await self._incarnation_with_latest_change_details_from_dbobj(inc) - async for inc in self._change_repository.list_incarnations_with_changes_summary_and_access( + if owner_username is not None: + owner = await self._user_repository.get_by_username(owner_username) + generator = self._change_repository.list_incarnations_with_changes_summary_and_access_of_owner( + user.id, [group.id for group in user.groups], owner.id + ) + else: + generator = self._change_repository.list_incarnations_with_changes_summary_and_access( user.id, [group.id for group in user.groups] ) - ] + + return [await self._incarnation_with_latest_change_details_from_dbobj(inc) async for inc in generator] async def get_incarnation_by_repo_and_target_directory( self, repo: str, target_directory: str From 6f123a200723193af7b0f6fb7d3b9a158ed6315d Mon Sep 17 00:00:00 2001 From: Yannick Mueller Date: Wed, 2 Apr 2025 10:39:05 +0200 Subject: [PATCH 17/23] Code cleanup --- .../database/repositories/group/repository.py | 8 ++--- src/foxops/dependencies.py | 36 ++++++++++++++++--- src/foxops/error_handlers.py | 9 +++-- src/foxops/routers/group.py | 30 +++++++++++++--- src/foxops/routers/incarnations.py | 9 +++-- src/foxops/routers/user.py | 33 ++++++++++++++--- src/foxops/services/authorization.py | 15 ++++---- src/foxops/services/group.py | 2 +- 8 files changed, 111 insertions(+), 31 deletions(-) diff --git a/src/foxops/database/repositories/group/repository.py b/src/foxops/database/repositories/group/repository.py index fb9aeb84..377b33d6 100644 --- a/src/foxops/database/repositories/group/repository.py +++ b/src/foxops/database/repositories/group/repository.py @@ -41,25 +41,23 @@ async def get_by_system_name(self, system_name: str) -> GroupInDB: async def get_by_userid(self, user_id: int) -> list[GroupInDB]: query = select(group_user, group).where(group_user.c.user_id == user_id).join(group) - groups = [] async with self.engine.begin() as conn: result = await conn.execute(query) - for row in result: - groups.append(GroupInDB.model_validate(row)) - - return groups + return [GroupInDB.model_validate(row) for row in result] async def get_by_id(self, group_id: int) -> GroupInDB: query = select(group).where(group.c.id == group_id) async with self.engine.begin() as conn: result = await conn.execute(query) + try: row = result.one() except NoResultFound as e: raise GroupNotFoundError(id=group_id) from e + return GroupInDB.model_validate(row) async def list_paginated( diff --git a/src/foxops/dependencies.py b/src/foxops/dependencies.py index 572f06bf..1a6441d6 100644 --- a/src/foxops/dependencies.py +++ b/src/foxops/dependencies.py @@ -149,7 +149,16 @@ def get_change_service( class StaticTokenHeaderAuth(SecurityBase): def __init__(self): - self.model = APIKey(**{"in": APIKeyIn.header}, name="Authorization") + self.model = APIKey( + **{ + "in": APIKeyIn.header, + "description": ( + "The static token for authentication. This field is required for all endpoints which require authentication. " + "It has to be provided in the format `Bearer `." + ), + }, + name="Authorization", + ) self.scheme_name = self.__class__.__name__ async def __call__(self, request: Request, settings: Settings = Depends(get_settings)) -> None: @@ -171,7 +180,15 @@ async def __call__(self, request: Request, settings: Settings = Depends(get_sett class UserHeaderAuth(SecurityBase): def __init__(self): - self.model = APIKey(**{"in": APIKeyIn.header}, name="User") + self.model = APIKey( + **{ + "in": APIKeyIn.header, + "description": ( + "The Username of the current user. This field is required for all endpoints which require authentication." + ), + }, + name="User", + ) self.scheme_name = self.__class__.__name__ async def __call__(self) -> None: @@ -181,7 +198,18 @@ async def __call__(self) -> None: class GroupHeaderAuth(SecurityBase): def __init__(self): - self.model = APIKey(**{"in": APIKeyIn.header}, name="Groups") + self.model = APIKey( + **{ + "in": APIKeyIn.header, + "description": ( + "The Groups of the current user. Multiple groups can be specified by separating them with commas. " + "This field is optional and may be empty. " + "The groupname must be alphanumeric and may contain underscores, colons and dashes. " + "The groupname **is** case sensitive." + ), + }, + name="Groups", + ) self.scheme_name = self.__class__.__name__ async def get_user(self, user_header: str | None, user_service: UserService) -> User: @@ -204,7 +232,7 @@ async def get_groups(self, group_header: str | None, group_service: GroupService for group in group_header.split(","): group = group.strip() - if not re.match(r"^[a-zA-Z0-9_\-]+$", group): + if not re.match(r"^[a-zA-Z0-9_\-:]+$", group): raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail="Group names must be alphanumeric and may contain underscores and dashes", diff --git a/src/foxops/error_handlers.py b/src/foxops/error_handlers.py index bdaf42f4..0e2cea1d 100644 --- a/src/foxops/error_handlers.py +++ b/src/foxops/error_handlers.py @@ -51,7 +51,7 @@ async def validation_exception_handler(_: Request, exc: RequestValidationError): - return JSONResponse(status_code=status.HTTP_400_BAD_REQUEST, content={"message": str(exc)}) + return JSONResponse(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, content={"message": str(exc)}) async def foxops_user_error(_: Request, exc: FoxopsUserError): @@ -60,6 +60,7 @@ async def foxops_user_error(_: Request, exc: FoxopsUserError): async def catch_all_foxops_exception(_: Request, exc: FoxopsError): + logger.warning(f"A foxops error happend: {str(exc)}") if exc.__class__ in EXCEPTION_TO_STATUS_CODE: return JSONResponse( status_code=EXCEPTION_TO_STATUS_CODE[exc.__class__], @@ -70,7 +71,11 @@ async def catch_all_foxops_exception(_: Request, exc: FoxopsError): async def catch_all(_: Request, exc: Exception): logger.exception(str(exc)) - return JSONResponse(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, content={"message": str(exc)}) + # We don't know what exeption this is and if it contains any sensitive information + # so we just return a generic error message + return JSONResponse( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, content={"message": "An unexpected error occurred."} + ) __error_handlers__ = { diff --git a/src/foxops/routers/group.py b/src/foxops/routers/group.py index ea30cc81..8666f303 100644 --- a/src/foxops/routers/group.py +++ b/src/foxops/routers/group.py @@ -1,4 +1,4 @@ -from fastapi import APIRouter, Depends, Query, Response, status +from fastapi import APIRouter, Depends, Path, Query, Response, status from pydantic import BaseModel from foxops.authz import access_to_admin_only @@ -31,6 +31,11 @@ async def list_groups( limit: int = Query(25, ge=1, le=200, description="Number of groups to return"), page: int = Query(1, ge=1, description="Page number"), ): + """ + Returns a list of all groups in the system. + + The list is paginated, so you can specify the number of groups to return and the page number to return. + """ groups = await group_service.list_groups_paginated(limit, page) # It is possible, that 0 groups exist in the db. @@ -59,10 +64,15 @@ async def list_groups( dependencies=[Depends(access_to_admin_only)], ) async def get_group( - group_id: int, + group_id: int = Path(..., description="The ID of the group"), group_service: GroupService = Depends(get_group_service), resolve_users: bool = Query(False, description="Resolve the users of the group"), ): + """ " + Returns the group with the given ID. + + If `resolve_users` is set to `True`, the users which are part of the group are also returned. + """ if resolve_users: return await group_service.get_group_by_id_with_users(group_id) @@ -84,11 +94,16 @@ async def get_group( status_code=status.HTTP_204_NO_CONTENT, ) async def delete_group( - group_id: int, + group_id: int = Path(..., description="The ID of the group"), group_service: GroupService = Depends(get_group_service), ): + """ + Deletes the group with the given ID. + """ await group_service.delete_group(group_id) + return Response(status_code=status.HTTP_204_NO_CONTENT) + class GroupPatchRequest(BaseModel): display_name: str @@ -109,8 +124,15 @@ class GroupPatchRequest(BaseModel): dependencies=[Depends(access_to_admin_only)], ) async def patch_group( - group_id: int, request: GroupPatchRequest, + group_id: int = Path(..., description="The ID of the group"), group_service: GroupService = Depends(get_group_service), ): + """ + Updates the display name of the group with the given ID. + + It is currently not possible to update the system name of the group. + Updating the display name has no direct effect on the authentication system, + since the api authenticates groups by their system name, which is not inmutable. + """ return await group_service.set_display_name(group_id, request.display_name) diff --git a/src/foxops/routers/incarnations.py b/src/foxops/routers/incarnations.py index 85ccda24..35c14458 100644 --- a/src/foxops/routers/incarnations.py +++ b/src/foxops/routers/incarnations.py @@ -199,7 +199,10 @@ async def create_incarnation( ) async def read_incarnation( incarnation_id: int, - show_permissions: bool = False, + show_permissions: bool = Query( + False, + description="If set to `true`, the response will also include the permissions of the current user.", + ), change_service: ChangeService = Depends(get_change_service), authorization_service: AuthorizationService = Depends(authorization), ): @@ -265,7 +268,7 @@ async def reset_incarnation( incarnation_id, request.requested_version, request.requested_data, - initialized_by=authorization_service.current_user.id, + initialized_by=authorization_service.id, ) except ProvidedTemplateDataInvalidError as e: response.status_code = status.HTTP_400_BAD_REQUEST @@ -279,6 +282,7 @@ async def reset_incarnation( return ApiError(message="The incarnation does not have any customizations. Nothing to reset.") incarnation = await incarnation_service.get_by_id(incarnation_id) + return IncarnationResetResponse( incarnation_id=incarnation_id, merge_request_id=change.merge_request_id, @@ -493,6 +497,7 @@ async def patch_incarnation( await incarnation_service.set_user_permissions(incarnation_id, request.user_permissions) if request.owner_id is not None: + # This additional check is needed, since only the owner or an admin can change the owner if authorization_serive.admin or authorization_serive.id == old_incarnation.owner.id: await incarnation_service.set_owner(incarnation_id, request.owner_id) else: diff --git a/src/foxops/routers/user.py b/src/foxops/routers/user.py index 162a8b55..c1de860f 100644 --- a/src/foxops/routers/user.py +++ b/src/foxops/routers/user.py @@ -1,4 +1,4 @@ -from fastapi import APIRouter, Depends, Query, Response, status +from fastapi import APIRouter, Depends, Path, Query, Response, status from pydantic import BaseModel from foxops.authz import access_to_admin_only @@ -31,6 +31,10 @@ async def list_users( limit: int = Query(25, ge=1, le=200, description="Number of users to return"), page: int = Query(1, ge=1, description="Page number"), ): + """ + Returns a list of all users in the system. + The list is paginated, so you can specify the number of users to return and the page number to return. + """ users = await user_service.list_users_paginated(limit, page) # There is not case, where 0 users exist in the db, since at least @@ -59,10 +63,16 @@ async def list_users( dependencies=[Depends(access_to_admin_only)], ) async def get_user( - user_id: int, + user_id: int = Path(..., description="The ID of the user"), user_service: UserService = Depends(get_user_service), resolve_groups: bool = Query(False, description="Resolve the groups of the user"), ): + """ + Returns the user with the given ID. + If the user is not found, a 404 error is returned. + + If the `resolve_groups` parameter is set to true, the groups of which the user is a member are also returned. + """ if resolve_groups: return await user_service.get_user_by_id_with_groups(user_id) @@ -92,17 +102,28 @@ async def get_user( dependencies=[Depends(access_to_admin_only)], ) async def delete_user( - user_id: int, response: Response, + user_id: int = Path(..., description="The ID of the user"), user_service: UserService = Depends(get_user_service), authorization_service: AuthorizationService = Depends(authorization), ): + """ + Deletes a user with the given ID. + + It is not possible to delete a user which is still the owner of some resources (e.g. incarnations). + If the user is still the owner of some resources, + you first have to either delete the resources or change the owner of the resources. + + It is also not possible to delete yourself. + """ if authorization_service.id == user_id: response.status_code = status.HTTP_400_BAD_REQUEST return ApiError(message="You can't delete yourself") await user_service.delete_user(user_id) + return Response(status_code=status.HTTP_204_NO_CONTENT) + class UserPatchRequest(BaseModel): is_admin: bool @@ -128,11 +149,15 @@ class UserPatchRequest(BaseModel): ) async def update_user( response: Response, - user_id: int, request: UserPatchRequest, + user_id: int = Path(..., description="The ID of the user"), user_service: UserService = Depends(get_user_service), authorization_service: AuthorizationService = Depends(authorization), ): + """ + Give or remove admin rights from a user. + It is not possible to change your own admin status. + """ if authorization_service.id == user_id: response.status_code = status.HTTP_400_BAD_REQUEST return ApiError(message="You can't change your own admin status") diff --git a/src/foxops/services/authorization.py b/src/foxops/services/authorization.py index 0488712c..a75f799c 100644 --- a/src/foxops/services/authorization.py +++ b/src/foxops/services/authorization.py @@ -6,6 +6,7 @@ class AuthorizationService: def __init__(self, current_user: UserWithGroups) -> None: self.current_user = current_user + self.group_ids = set(group.id for group in current_user.groups) @property def admin(self) -> bool: @@ -20,9 +21,9 @@ def _read_access_incarnation_with_details(self, incarnation: IncarnationWithDeta return True user_ids_with_access = [p.user.id for p in incarnation.user_permissions] - group_ids_with_access = [p.group.id for p in incarnation.group_permissions] + return self.id in user_ids_with_access or any( - group.id in group_ids_with_access for group in self.current_user.groups + p.group.id in self.group_ids for p in incarnation.group_permissions ) def _write_access_incarnation_with_details(self, incarnation: IncarnationWithDetails) -> bool: @@ -30,10 +31,9 @@ def _write_access_incarnation_with_details(self, incarnation: IncarnationWithDet return True user_ids_with_access = [p.user.id for p in incarnation.user_permissions if p.type == Permission.WRITE] - group_ids_with_access = [p.group.id for p in incarnation.group_permissions if p.type == Permission.WRITE] return self.id in user_ids_with_access or any( - group.id in group_ids_with_access for group in self.current_user.groups + p.group.id in self.group_ids for p in incarnation.group_permissions if p.type == Permission.WRITE ) def _read_access_incarnation_permissions(self, permissions: IncarnationPermissions) -> bool: @@ -41,10 +41,9 @@ def _read_access_incarnation_permissions(self, permissions: IncarnationPermissio return True user_ids_with_access = [p.user_id for p in permissions.user_permissions] - group_ids_with_access = [p.group_id for p in permissions.group_permissions] return self.id in user_ids_with_access or any( - group.id in group_ids_with_access for group in self.current_user.groups + p.group_id in self.group_ids for p in permissions.group_permissions ) def _write_access_incarnation_permissions(self, permissions: IncarnationPermissions) -> bool: @@ -53,10 +52,8 @@ def _write_access_incarnation_permissions(self, permissions: IncarnationPermissi user_ids_with_access = [p.user_id for p in permissions.user_permissions if p.type == Permission.WRITE] - group_ids_with_access = [p.group_id for p in permissions.group_permissions if p.type == Permission.WRITE] - return self.id in user_ids_with_access or any( - group.id in group_ids_with_access for group in self.current_user.groups + p.group_id in self.group_ids for p in permissions.group_permissions if p.type == Permission.WRITE ) def has_read_access(self, object: object) -> bool: diff --git a/src/foxops/services/group.py b/src/foxops/services/group.py index bfea57e1..43c01133 100644 --- a/src/foxops/services/group.py +++ b/src/foxops/services/group.py @@ -48,7 +48,7 @@ async def get_group_by_id(self, group_id: int) -> Group: return Group.model_validate(group) - async def get_group_by_id_with_users(self, group_id: int) -> Group: + async def get_group_by_id_with_users(self, group_id: int) -> GroupWithUsers: group = await self.group_repository.get_by_id(group_id) users = await self.user_repository.get_users_of_group(group_id) From f8f9890039e4f41b559c128b78d0dcb435b0f2a1 Mon Sep 17 00:00:00 2001 From: Yannick Mueller Date: Wed, 2 Apr 2025 13:47:16 +0200 Subject: [PATCH 18/23] Adapt UI for new Authz --- ui/src/interfaces/authz.types.ts | 5 ++ ui/src/routes/login/Login.tsx | 79 ++++++++++++++++++++++++-------- ui/src/services/api.ts | 19 ++++++-- ui/src/stores/auth.ts | 7 +-- ui/vite.config.ts | 6 +++ 5 files changed, 90 insertions(+), 26 deletions(-) create mode 100644 ui/src/interfaces/authz.types.ts diff --git a/ui/src/interfaces/authz.types.ts b/ui/src/interfaces/authz.types.ts new file mode 100644 index 00000000..37f4362b --- /dev/null +++ b/ui/src/interfaces/authz.types.ts @@ -0,0 +1,5 @@ +export interface AuthorizationToken { + token: string, + user: string, + groups: string[] | null | undefined | string, +} diff --git a/ui/src/routes/login/Login.tsx b/ui/src/routes/login/Login.tsx index 32006e89..1cc9ebd1 100644 --- a/ui/src/routes/login/Login.tsx +++ b/ui/src/routes/login/Login.tsx @@ -1,6 +1,6 @@ import styled from '@emotion/styled' import { Navigate } from 'react-router-dom' -import { useRef, useState } from 'react' +import { useState } from 'react' import { useAuthStore } from '../../stores/auth' import { Button } from '../../components/common/Button/Button' import { Hug } from '../../components/common/Hug/Hug' @@ -8,6 +8,8 @@ import { Logo } from '../../components/common/Logo/Logo' import { TextField } from '../../components/common/TextField/TextField' import { api } from '../../services/api' import { auth } from '../../services/auth' +import { useForm } from 'react-hook-form' +import { AuthorizationToken } from 'interfaces/authz.types' const Wrapper = styled.div({ height: '100vh', @@ -28,6 +30,14 @@ const FormComponent = styled.form({ } }) +const Error = styled.span({ + color: 'red', + fontSize: 12, + marginTop: 4, + display: 'block', + minHeight: 14 +}) + const ProceedLink = styled.span({ textDecoration: 'underline', cursor: 'pointer', @@ -42,48 +52,79 @@ const STAGES = { } export const Login = () => { - const [_token, _setToken] = useState(process.env.FOXOPS_STATIC_TOKEN as string) - const loginRef = useRef(null) - const [error, setError] = useState('') + const { + register, + getValues, + setError, + setFocus, + handleSubmit, + formState: { errors } + } = useForm({ + defaultValues: { + token: process.env.FOXOPS_STATIC_TOKEN || '', + user: process.env.FOXOPS_STATIC_USERNAME || '', + groups: process.env.FOXOPS_STATIC_GROUPS + } + + }) + const [loading, setLoading] = useState(false) const [stage, setStage] = useState(STAGES.FIRST_INTERACTION) const { token, setToken } = useAuthStore() + if (token) return () - const onChangeToken: React.ChangeEventHandler = e => { - setError('') - _setToken(e.target.value) - } const onProceedLinkClick = () => setStage(STAGES.TOKEN_INPUT_SHOWN) const checkToken = async () => { - api.setToken(_token) + const TOKEN = getValues() as AuthorizationToken + const groups = getValues('groups') + + if (groups && !/^ *[a-zA-Z_:\-0-9]+ *(, *[a-zA-Z_:\-0-9]+ *)*$/.test(groups)) { + setError('groups', { + type: 'manual', + message: 'Invalid groups format' + }) + setFocus('groups') + return + } + + api.setToken(TOKEN) setLoading(true) try { await auth.checkToken() - setToken(_token) + setToken(TOKEN) } catch (error) { console.log('error', error) - setError('Invalid token') - loginRef.current?.focus() + setError('token', { + type: 'manual', + message: 'Invalid token' + }) + setFocus('token') api.setToken(null) } finally { setLoading(false) } } - const onFormSubmit: React.FormEventHandler = e => { - e.preventDefault() - checkToken() - } + return ( - + {stage === STAGES.FIRST_INTERACTION && Click to proceed } {stage === STAGES.TOKEN_INPUT_SHOWN && ( <> - - + + + {errors.token && errors.token.message || errors.token?.type} + + + + {errors.user && errors.user.message || errors.user?.type} + + + + {errors.groups && errors.groups.message || errors.groups?.type} diff --git a/ui/src/components/common/Icons/Eye.tsx b/ui/src/components/common/Icons/Eye.tsx new file mode 100644 index 00000000..0fd33f4e --- /dev/null +++ b/ui/src/components/common/Icons/Eye.tsx @@ -0,0 +1,9 @@ +import { FALicense } from './FALicense' +import { SvgIcon, SvgIconProps } from './SvgIcon' + +export const Eye = (props: Partial) => ( + + + + +) diff --git a/ui/src/components/common/Icons/FALicense.tsx b/ui/src/components/common/Icons/FALicense.tsx new file mode 100644 index 00000000..c405ba5c --- /dev/null +++ b/ui/src/components/common/Icons/FALicense.tsx @@ -0,0 +1 @@ +export const FALicense = () =>
' }} /> diff --git a/ui/src/components/common/Icons/Group.tsx b/ui/src/components/common/Icons/Group.tsx new file mode 100644 index 00000000..719f3339 --- /dev/null +++ b/ui/src/components/common/Icons/Group.tsx @@ -0,0 +1,9 @@ +import { FALicense } from './FALicense' +import { SvgIcon, SvgIconProps } from './SvgIcon' + +export const Group = (props: Partial) => ( + + + + +) diff --git a/ui/src/components/common/Icons/Pen.tsx b/ui/src/components/common/Icons/Pen.tsx new file mode 100644 index 00000000..b9366371 --- /dev/null +++ b/ui/src/components/common/Icons/Pen.tsx @@ -0,0 +1,9 @@ +import { FALicense } from './FALicense' +import { SvgIcon, SvgIconProps } from './SvgIcon' + +export const Pen = (props: Partial) => ( + + + + +) diff --git a/ui/src/components/common/Icons/ProfilePicture.tsx b/ui/src/components/common/Icons/ProfilePicture.tsx new file mode 100644 index 00000000..6a887bbf --- /dev/null +++ b/ui/src/components/common/Icons/ProfilePicture.tsx @@ -0,0 +1,7 @@ +import { SvgIcon, SvgIconProps } from './SvgIcon' + +export const ProfilePicture = (props: Partial) => ( + + + +) diff --git a/ui/src/components/common/Icons/SvgIcon.tsx b/ui/src/components/common/Icons/SvgIcon.tsx index a3d45284..08a3199f 100644 --- a/ui/src/components/common/Icons/SvgIcon.tsx +++ b/ui/src/components/common/Icons/SvgIcon.tsx @@ -11,7 +11,7 @@ export interface SvgIconProps extends React.SVGAttributes { height?: number, } export const SvgIcon = ({ children, width = 24, height = 24, ...props }: SvgIconProps) => ( - + {children} ) diff --git a/ui/src/components/common/Icons/User.tsx b/ui/src/components/common/Icons/User.tsx index c9cf068c..5d132373 100644 --- a/ui/src/components/common/Icons/User.tsx +++ b/ui/src/components/common/Icons/User.tsx @@ -1,7 +1,9 @@ +import { FALicense } from './FALicense' import { SvgIcon, SvgIconProps } from './SvgIcon' export const User = (props: Partial) => ( - - + + + ) diff --git a/ui/src/interfaces/group.types.ts b/ui/src/interfaces/group.types.ts new file mode 100644 index 00000000..708baa00 --- /dev/null +++ b/ui/src/interfaces/group.types.ts @@ -0,0 +1,12 @@ +export interface Group { + id: number, + systemName: string, + displayName: string, +} + +export interface GroupApiView { + id: number, + system_name: string, + display_name: string, +} + diff --git a/ui/src/interfaces/incarnations.types.ts b/ui/src/interfaces/incarnations.types.ts index ca9a64bd..14d1fd27 100644 --- a/ui/src/interfaces/incarnations.types.ts +++ b/ui/src/interfaces/incarnations.types.ts @@ -1,3 +1,6 @@ +import { Group, GroupApiView } from './group.types' +import { User, UserApiView } from './user.types' + export interface IncarnationBaseApiView { id: number, incarnation_repository: string, @@ -48,6 +51,11 @@ export interface IncarnationApiView { template_repository_version_hash: string template_data: Record | null, template_data_full: Record | null, + owner: User, + user_permissions: IncarnationUserPermissionApiView[], + group_permissions: IncarnationGroupPermissionApiView[], + current_user_permissions: IncarnationPermissionsAPIView, + } export interface ChangeApiView { @@ -87,6 +95,32 @@ export interface IncarnationResetApiInput { requested_data: Record } +export interface IncarnationUserPermission{ + user: User, + type: 'write' | 'read', +} + +export interface IncarnationGroupPermission{ + group: Group, + type: 'write' | 'read', +} + +export interface IncarnationGroupPermissionApiView{ + group: GroupApiView, + type: 'write' | 'read', +} + +export interface IncarnationUserPermissionApiView{ + user: UserApiView, + type: 'write' | 'read', +} + +export interface IncarnationPermissionsAPIView { + can_read: boolean, + can_update: boolean, + can_reset: boolean, + can_delete: boolean, +} export interface Incarnation { id: number, incarnationRepository: string, @@ -102,13 +136,24 @@ export interface Incarnation { templateRepositoryVersion: string, templateRepositoryVersionHash: string, templateData: Record, - templateDataFull: Record + templateDataFull: Record, + owner: User, + userPermissions: IncarnationUserPermission[], + groupPermissions: IncarnationGroupPermission[], + currentUserPermissions?: IncarnationPermissions, +} + +export interface IncarnationPermissions { + canRead: boolean, + canUpdate: boolean, + canReset: boolean, + canDelete: boolean, } export interface IncarnationUpdateApiInput { - template_repository_version: string, - template_data: Record, - automerge: boolean + requested_version: string, + requested_data: Record, + automerge: boolean, } export interface IncarnationApiInput { diff --git a/ui/src/interfaces/user.types.ts b/ui/src/interfaces/user.types.ts new file mode 100644 index 00000000..3aa1c1d1 --- /dev/null +++ b/ui/src/interfaces/user.types.ts @@ -0,0 +1,11 @@ +export interface User { + id: number, + username: string, + isAdmin: boolean, +} + +export interface UserApiView { + id: number, + username: string, + is_admin: boolean +} diff --git a/ui/src/main.tsx b/ui/src/main.tsx index 2cee5edf..fe67491f 100644 --- a/ui/src/main.tsx +++ b/ui/src/main.tsx @@ -1,6 +1,9 @@ import ReactDOM from 'react-dom/client' import App from './App' +import { StrictMode } from 'react' ReactDOM.createRoot(document.getElementById('root') as HTMLElement).render( - + + + ) diff --git a/ui/src/routes/incarnations/EditForm.tsx b/ui/src/routes/incarnations/EditForm.tsx index 4b8bfb6f..40d14283 100644 --- a/ui/src/routes/incarnations/EditForm.tsx +++ b/ui/src/routes/incarnations/EditForm.tsx @@ -58,6 +58,7 @@ export const EditIncarnationForm = () => { resetIncarnation={(templateVersion: string, templateData: Record) => incarnations.reset(id, templateVersion, templateData)} diffChanges={diffCount} isEdit + incarnation={data} /> ) : (
diff --git a/ui/src/routes/incarnations/Form.tsx b/ui/src/routes/incarnations/Form.tsx index f23a7e5d..abe3c01f 100644 --- a/ui/src/routes/incarnations/Form.tsx +++ b/ui/src/routes/incarnations/Form.tsx @@ -16,6 +16,7 @@ import isUrl from 'is-url' import { OpenInNew } from '../../components/common/Icons/OpenInNew' import { ToggleSwitch } from '../../components/common/ToggleSwitch/ToggleSwitch' import { + Incarnation, IncarnationApiView, MergeRequestStatus } from 'interfaces/incarnations.types' @@ -24,12 +25,22 @@ import { Tabs } from 'components/common/Tabs/Tabs' import { useNavigate } from 'react-router-dom' import { Dialog } from 'components/common/Dialog/Dialog' import { useErrorStore } from 'stores/error' +import { PermissionChip } from './parts/PermissionChip' const DeleteIncarnationLink = styled.span` cursor: pointer; text-decoration: underline; ` +const ReadOnlyMessage = styled.span(({ theme }) => ({ + display: 'inline-block', + color: theme.colors.orange, + width: 'fit-content', + marginLeft: 'auto', + marginTop: '.5rem', + fontStyle: 'italic' +})) + export type DiffChanges = { added: number, removed: number @@ -45,7 +56,8 @@ type FormProps = { incarnationMergeRequestStatus?: MergeRequestStatus | null, mergeRequestUrl?: string | null, commitUrl?: string, - templateDataFull?: Record + templateDataFull?: Record, + incarnation?: Incarnation | null, } type ChangeSquareProps = { @@ -119,7 +131,8 @@ export const IncarnationsForm = ({ incarnationMergeRequestStatus, mergeRequestUrl, commitUrl, - templateDataFull + templateDataFull, + incarnation }: FormProps) => { const { register, @@ -175,7 +188,6 @@ export const IncarnationsForm = ({ const onSubmit: SubmitHandler = async incarnation => { errorStore.clearError() try { - console.log(incarnation) await mutateAsync(incarnation) await delay(1000) queryClient.invalidateQueries(['incarnations']) @@ -189,8 +201,9 @@ export const IncarnationsForm = ({ const buttonTitle = isEdit ? 'Update' : 'Create' let resetIncarnationDisabled = null - - if (diffChanges === null || diffChanges === undefined) { + if (!incarnation?.currentUserPermissions?.canReset) { + resetIncarnationDisabled = 'You do not have permission to reset this incarnation' + } else if (diffChanges === null || diffChanges === undefined) { resetIncarnationDisabled = 'Waiting for changes to be calculated' } else if (diffChanges.added === 0 && diffChanges.removed === 0) { resetIncarnationDisabled = 'No changes to reset' @@ -202,6 +215,12 @@ export const IncarnationsForm = ({ resetIncarnationDisabled = 'There is already an open merge request' } + let buttonTooltip = `${buttonTitle} incarnation` + + if (incarnation?.currentUserPermissions && !incarnation?.currentUserPermissions?.canUpdate) { + buttonTooltip = 'You do not have permission to update this incarnation' + } + const editTemplateDataController = ( )} /> @@ -292,7 +312,7 @@ export const IncarnationsForm = ({ {isEdit && ( - ( - onChange(e.target.checked)} - /> - )} - /> + + )} + { + (isEdit && incarnation) && ( + + { + incarnation?.userPermissions.map(permission => ) + } + { + incarnation?.groupPermissions.map(permission => ) + } + + ) + } {isEdit ? ( @@ -356,34 +383,56 @@ export const IncarnationsForm = ({ )} - - - {isEdit && ( - - + + + { isEdit && ( + + ( + onChange(e.target.checked)} + /> + )} + /> + )} - - - + + {isEdit && ( + + + )} + + + + + + + + { incarnation?.currentUserPermissions && !incarnation?.currentUserPermissions?.canUpdate && You only have read permissions on this incarnation} @@ -437,12 +486,12 @@ export const IncarnationsForm = ({ size="large" /> - + diff --git a/ui/src/constants/incarnations.consts.ts b/ui/src/constants/incarnations.consts.ts index ee8dad35..27f3f710 100644 --- a/ui/src/constants/incarnations.consts.ts +++ b/ui/src/constants/incarnations.consts.ts @@ -1,7 +1,8 @@ +import { Paths } from 'shared/types' import { IncarnationBase } from '../interfaces/incarnations.types' export interface IncarnationTableColumn { - id: keyof IncarnationBase, + id: Paths, // keyof doesn't apply to nested objects! header: string, size: number, minSize?: number @@ -84,5 +85,11 @@ export const INCARNATION_TABLE_COLUMNS: IncarnationTableColumn[] = [ id: 'mergeRequestUrl', size: 250, minSize: 100 + }), + makeColumn({ + header: 'Owner', + id: 'owner.username', + size: 250, + minSize: 100 }) ] diff --git a/ui/src/interfaces/incarnations.types.ts b/ui/src/interfaces/incarnations.types.ts index 14d1fd27..1612c9b7 100644 --- a/ui/src/interfaces/incarnations.types.ts +++ b/ui/src/interfaces/incarnations.types.ts @@ -13,7 +13,8 @@ export interface IncarnationBaseApiView { commit_sha: string, commit_url: string, merge_request_id: null | string, - merge_request_url: null | string + merge_request_url: null | string, + owner: UserApiView, } export type MergeRequestStatus = 'open' | 'merged' | 'closed' | 'unknown' @@ -33,6 +34,7 @@ export interface IncarnationBase { commitUrl: string, mergeRequestId: null | string, mergeRequestUrl: null | string + owner: User templateVersion: string // UI only } export interface IncarnationApiView { diff --git a/ui/src/interfaces/user.types.ts b/ui/src/interfaces/user.types.ts index 3aa1c1d1..fe8890ab 100644 --- a/ui/src/interfaces/user.types.ts +++ b/ui/src/interfaces/user.types.ts @@ -1,3 +1,5 @@ +import { Group, GroupApiView } from './group.types' + export interface User { id: number, username: string, @@ -9,3 +11,11 @@ export interface UserApiView { username: string, is_admin: boolean } + +export interface UserWithGroups extends User { + groups: Group[] +} + +export interface UserWithGroupsApiView extends UserApiView { + groups: GroupApiView[] +} diff --git a/ui/src/routes/login/Login.tsx b/ui/src/routes/login/Login.tsx index 1cc9ebd1..2b9de6fe 100644 --- a/ui/src/routes/login/Login.tsx +++ b/ui/src/routes/login/Login.tsx @@ -70,7 +70,7 @@ export const Login = () => { const [loading, setLoading] = useState(false) const [stage, setStage] = useState(STAGES.FIRST_INTERACTION) - const { token, setToken } = useAuthStore() + const { token, setToken, setUser } = useAuthStore() if (token) return () const onProceedLinkClick = () => setStage(STAGES.TOKEN_INPUT_SHOWN) @@ -90,7 +90,8 @@ export const Login = () => { api.setToken(TOKEN) setLoading(true) try { - await auth.checkToken() + const user = await auth.checkToken() + setUser(user) setToken(TOKEN) } catch (error) { console.log('error', error) diff --git a/ui/src/services/auth.ts b/ui/src/services/auth.ts index 9435c48c..fab3c6d9 100644 --- a/ui/src/services/auth.ts +++ b/ui/src/services/auth.ts @@ -1,5 +1,17 @@ +import { UserWithGroups, UserWithGroupsApiView } from 'interfaces/user.types' import { api } from './api' +export const convertToUiUserWithGroups = (user: UserWithGroupsApiView): UserWithGroups => ({ + username: user.username, + id: user.id, + isAdmin: user.is_admin, + groups: user.groups.map(group => ({ + id: group.id, + displayName: group.display_name, + systemName: group.system_name + })) +}) + export const auth = { - checkToken: () => api.get('/auth/test', { apiPrefix: '', format: 'text' }) + checkToken: async () => convertToUiUserWithGroups(await api.get('/auth/test', { apiPrefix: '' })) } diff --git a/ui/src/services/incarnations.ts b/ui/src/services/incarnations.ts index 27e3c0a6..8d722070 100644 --- a/ui/src/services/incarnations.ts +++ b/ui/src/services/incarnations.ts @@ -3,8 +3,9 @@ import { api } from '../services/api' import { Change, ChangeApiView, Incarnation, IncarnationApiInput, IncarnationApiView, IncarnationBase, IncarnationBaseApiView, IncarnationGroupPermission, IncarnationGroupPermissionApiView, IncarnationPermissions, IncarnationPermissionsAPIView, IncarnationResetApiInput, IncarnationUpdateApiInput, IncarnationUserPermission, IncarnationUserPermissionApiView } from '../interfaces/incarnations.types' import { User, UserApiView } from 'interfaces/user.types' import { Group, GroupApiView } from 'interfaces/group.types' +import { Paths } from 'shared/types' -export const INCARNATION_SEARCH_FIELDS: (keyof IncarnationBase)[] = [ +export const INCARNATION_SEARCH_FIELDS: (Paths)[] = [ 'id', 'incarnationRepository', 'targetDirectory', @@ -49,7 +50,8 @@ export const convertToUiBaseIncarnation = (incarnation: IncarnationBaseApiView): templateRepository: incarnation.template_repository, type: incarnation.type, revison: incarnation.revision, - templateVersion: '' // UI only + templateVersion: '', // UI only, + owner: convertToUiUser(incarnation.owner) }) const convertToUiUser = (user: UserApiView): User => ({ diff --git a/ui/src/shared/types.ts b/ui/src/shared/types.ts index 0c5d4cf2..99a64c63 100644 --- a/ui/src/shared/types.ts +++ b/ui/src/shared/types.ts @@ -1 +1,5 @@ export type ThemeMode = 'light' | 'dark' + +export type Paths = T extends object ? { [K in keyof T]: + `${Exclude}${'' | `.${Paths}`}` + }[keyof T] : never diff --git a/ui/src/stores/auth.ts b/ui/src/stores/auth.ts index a62351b9..105997bd 100644 --- a/ui/src/stores/auth.ts +++ b/ui/src/stores/auth.ts @@ -2,21 +2,26 @@ import { create } from 'zustand' import { persist } from 'zustand/middleware' import { STORAGE_KEYS } from '../services/storage' import { AuthorizationToken } from 'interfaces/authz.types' +import { UserWithGroups } from 'interfaces/user.types' interface AuthStore { token: null | AuthorizationToken, + user: null | UserWithGroups, setToken: (token: null | AuthorizationToken) => void, + setUser: (user: null | UserWithGroups) => void } export const useAuthStore = create()( persist( set => ({ token: null, - setToken: (token: null | AuthorizationToken) => set(() => ({ token })) + user: null, + setToken: (token: null | AuthorizationToken) => set(() => ({ token })), + setUser: (user: null | UserWithGroups) => set(() => ({ user })) }), { name: STORAGE_KEYS.AUTH, - partialize: state => ({ token: state.token }) + partialize: state => ({ token: state.token, user: state.user }) } ) ) diff --git a/ui/src/stores/table-settings.ts b/ui/src/stores/table-settings.ts index dd287893..9d743dc2 100644 --- a/ui/src/stores/table-settings.ts +++ b/ui/src/stores/table-settings.ts @@ -4,8 +4,9 @@ import { INCARNATION_TABLE_COLUMNS, IncarnationTableColumn } from '../constants/ import { IncarnationBase } from '../interfaces/incarnations.types' import { OnChangeFn, PaginationState, SortingState } from '@tanstack/react-table' import { STORAGE_KEYS } from '../services/storage' +import { Paths } from 'shared/types' -const DEFAULT_COLUMNS_IDS: (keyof IncarnationBase)[] = [ +const DEFAULT_COLUMNS_IDS: (Paths)[] = [ 'incarnationRepository', 'targetDirectory', 'templateRepository', @@ -25,7 +26,7 @@ interface TableSettingsStore { withPagination: boolean, pagination: PaginationState sorting: SortingState - setVisibleColumns: (visibleColumns: (keyof IncarnationBase)[]) => void + setVisibleColumns: (visibleColumns: (Paths)[]) => void setDensity: (tableDensity: TableDensity) => void setActionsSimplified: (actionsSimplified: boolean) => void, setWithPagination: (withPagination: boolean) => void, @@ -46,7 +47,7 @@ export const useTableSettingsStore = create()( pageIndex: 0, pageSize: 50 }, - setVisibleColumns: (visibleColumns: (keyof IncarnationBase)[]) => set(() => ({ + setVisibleColumns: (visibleColumns: (Paths)[]) => set(() => ({ visibleColumns: INCARNATION_TABLE_COLUMNS.filter(x => visibleColumns.includes(x.id)) })), setDensity: (tableDensity: TableDensity) => set(() => ({ tableDensity })), diff --git a/ui/src/utils/search-incarnations.test.ts b/ui/src/utils/search-incarnations.test.ts index 5a67c7dc..6a64908d 100644 --- a/ui/src/utils/search-incarnations.test.ts +++ b/ui/src/utils/search-incarnations.test.ts @@ -15,7 +15,12 @@ const data: IncarnationBaseApiView[] = [ created_at: '2021-01-01T00:00:00.000Z', requested_version: 'test-version', revision: 1, - type: 'direct' + type: 'direct', + owner: { + id: 1, + username: 'test-user', + is_admin: false + } }, { id: 1632, @@ -29,7 +34,12 @@ const data: IncarnationBaseApiView[] = [ created_at: '2021-01-01T00:00:00.000Z', requested_version: 'test-version', revision: 1, - type: 'merge_request' + type: 'merge_request', + owner: { + id: 1, + username: 'test-user', + is_admin: false + } }, { id: 659, @@ -43,7 +53,12 @@ const data: IncarnationBaseApiView[] = [ created_at: '2021-02-01T00:00:00.000Z', requested_version: 'test-version', revision: 2, - type: 'direct' + type: 'direct', + owner: { + id: 1, + username: 'test-user', + is_admin: false + } }, { id: 1636, @@ -57,7 +72,12 @@ const data: IncarnationBaseApiView[] = [ created_at: '2021-01-01T00:00:00.000Z', requested_version: 'test-version', revision: 1, - type: 'merge_request' + type: 'merge_request', + owner: { + id: 1, + username: 'test-user', + is_admin: false + } }, { id: 1337, @@ -71,7 +91,12 @@ const data: IncarnationBaseApiView[] = [ created_at: '2021-01-01T00:00:00.000Z', requested_version: 'test-version', revision: 1, - type: 'merge_request' + type: 'merge_request', + owner: { + id: 1, + username: 'test-user', + is_admin: false + } } ] diff --git a/ui/src/utils/search-incarnations.ts b/ui/src/utils/search-incarnations.ts index 1b23acbf..da325765 100644 --- a/ui/src/utils/search-incarnations.ts +++ b/ui/src/utils/search-incarnations.ts @@ -1,3 +1,4 @@ +import { Paths } from 'shared/types' import { searchBy } from '.' import { IncarnationBase } from '../interfaces/incarnations.types' import { INCARNATION_SEARCH_FIELDS } from '../services/incarnations' @@ -18,9 +19,10 @@ const mergeIncarnations = (a: IncarnationBase[], b: IncarnationBase[]) => { return result } -export const makeSortBySemVer = (semVerField: keyof IncarnationBase) => (a: IncarnationBase, b: IncarnationBase) => { - const valA = a[semVerField] - const valB = b[semVerField] +export const makeSortBySemVer = (semVerField: Paths) => (a: IncarnationBase, b: IncarnationBase) => { + const valA = semVerField.split('.').reduce((p: any, c: string) => p && p[c] || null, a) // eslint-disable-line + const valB = semVerField.split('.').reduce((p: any, c: string) => p && p[c] || null, b) // eslint-disable-line + if (typeof valA !== 'string' || typeof valB !== 'string') return 1 const sv1 = semverRegex().exec(valA)?.[0] const sv2 = semverRegex().exec(valB)?.[0] From 2533a5b613fcbfedc257a9091b2106a4b67d322e Mon Sep 17 00:00:00 2001 From: Yannick Mueller Date: Thu, 3 Apr 2025 16:52:42 +0200 Subject: [PATCH 21/23] Add automated tests for API --- tests/conftest.py | 32 ++++- tests/database/test_user_repository.py | 90 +++++++++++++ tests/routers/test_auth.py | 3 + tests/routers/test_authz.py | 174 +++++++++++++++++++++++++ tests/routers/test_incarnations.py | 110 ++++++++++++---- tests/routers/test_user.py | 20 +++ 6 files changed, 400 insertions(+), 29 deletions(-) create mode 100644 tests/database/test_user_repository.py create mode 100644 tests/routers/test_authz.py create mode 100644 tests/routers/test_user.py diff --git a/tests/conftest.py b/tests/conftest.py index 78029384..a4450aca 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -27,6 +27,7 @@ ) from foxops.hosters.local import LocalHoster from foxops.logger import setup_logging +from foxops.models.user import User from foxops.services.group import GroupService from foxops.services.user import UserService @@ -123,6 +124,16 @@ def get_static_api_token() -> str: return "test-token" +@pytest.fixture() +async def priviliged_api_user(user_repository: UserRepository): + return await user_repository.get_by_username("root") + + +@pytest.fixture +async def unprivileged_api_user(user_repository: UserRepository): + return await user_repository.create(username="user", is_admin=False) + + @pytest.fixture(name="unauthenticated_client") async def create_unauthenticated_client( app: FastAPI, @@ -148,9 +159,11 @@ async def create_unauthenticated_client( @pytest.fixture(name="authenticated_client") -async def create_authenticated_client(unauthenticated_client: AsyncClient, static_api_token: str) -> AsyncClient: +async def create_authenticated_client( + unauthenticated_client: AsyncClient, static_api_token: str, priviliged_api_user: User +) -> AsyncClient: unauthenticated_client.headers["Authorization"] = f"Bearer {static_api_token}" - unauthenticated_client.headers["User"] = "root" + unauthenticated_client.headers["User"] = priviliged_api_user.username return unauthenticated_client @@ -158,3 +171,18 @@ async def create_authenticated_client(unauthenticated_client: AsyncClient, stati async def create_api_client(authenticated_client: AsyncClient) -> AsyncClient: authenticated_client.base_url = f"{authenticated_client.base_url}/api" # type: ignore return authenticated_client + + +@pytest.fixture(name="unprivileged_authenticated_client") +async def create_unprivileged_authenticated_client( + unauthenticated_client: AsyncClient, static_api_token: str, unprivileged_api_user: User +) -> AsyncClient: + unauthenticated_client.headers["Authorization"] = f"Bearer {static_api_token}" + unauthenticated_client.headers["User"] = unprivileged_api_user.username + return unauthenticated_client + + +@pytest.fixture(name="unprivileged_api_client") +async def create_unprivileged_api_client(unprivileged_authenticated_client: AsyncClient) -> AsyncClient: + unprivileged_authenticated_client.base_url = f"{unprivileged_authenticated_client.base_url}/api" # type: ignore + return unprivileged_authenticated_client diff --git a/tests/database/test_user_repository.py b/tests/database/test_user_repository.py new file mode 100644 index 00000000..8ce21363 --- /dev/null +++ b/tests/database/test_user_repository.py @@ -0,0 +1,90 @@ +import pytest + +from foxops.database.repositories.group.repository import GroupRepository +from foxops.database.repositories.user.errors import UserAlreadyExistsError +from foxops.database.repositories.user.repository import UserRepository + + +async def test_cant_create_two_users_with_the_same_username( + user_repository: UserRepository, +): + await user_repository.create( + username="user1", + is_admin=False, + ) + + with pytest.raises(UserAlreadyExistsError) as excinfo: + await user_repository.create( + username="user1", + is_admin=False, + ) + + assert str(excinfo.value) == "User with username 'user1' already exists." + + +async def test_default_admin_user_exists( + user_repository: UserRepository, +): + user = await user_repository.get_by_username("root") + + assert user is not None + assert user.username == "root" + assert user.is_admin is True + + +async def test_user_can_join_group( + user_repository: UserRepository, + group_repository: GroupRepository, +): + user = await user_repository.create( + username="user1", + is_admin=False, + ) + + group = await group_repository.create( + system_name="group1", + display_name="group1", + ) + + user_groups = await group_repository.get_by_userid(user.id) + + assert len(user_groups) == 0 + + await user_repository.join_groups( + user_id=user.id, + group_ids=[group.id], + ) + + user_groups = await group_repository.get_by_userid(user.id) + + assert len(user_groups) == 1 + + assert user_groups[0].id == group.id + assert user_groups[0].system_name == group.system_name + assert user_groups[0].display_name == group.display_name + + +async def test_user_can_be_promoted_to_administrator( + user_repository: UserRepository, +): + user = await user_repository.create( + username="user1", + is_admin=False, + ) + + assert user.is_admin is False + + db_user = await user_repository.get_by_id(user.id) + + assert db_user.is_admin is False + + await user_repository.set_is_admin( + user_id=user.id, + is_admin=True, + ) + + db_user = await user_repository.get_by_id(user.id) + + assert db_user.is_admin is True + assert db_user.username == user.username + assert db_user.id == user.id diff --git a/tests/routers/test_auth.py b/tests/routers/test_auth.py index cf6fe3d7..6350048c 100644 --- a/tests/routers/test_auth.py +++ b/tests/routers/test_auth.py @@ -1,9 +1,12 @@ +import pytest from fastapi import FastAPI, status from httpx import AsyncClient from foxops.dependencies import group_auth_scheme, user_auth_scheme from foxops.models.user import UserWithGroups +pytestmark = [pytest.mark.api] + async def test_returns_err_if_authorization_header_is_missing(app: FastAPI): # WHEN diff --git a/tests/routers/test_authz.py b/tests/routers/test_authz.py new file mode 100644 index 00000000..e760b9fe --- /dev/null +++ b/tests/routers/test_authz.py @@ -0,0 +1,174 @@ +import json + +import pytest +from fastapi import status +from httpx import AsyncClient +from pytest_mock import MockFixture + +from foxops.database.repositories.change.repository import ChangeRepository +from foxops.database.repositories.group.repository import GroupRepository +from foxops.database.repositories.incarnation.repository import IncarnationRepository +from foxops.database.repositories.user.errors import UserNotFoundError +from foxops.database.repositories.user.repository import UserRepository +from foxops.database.schema import Permission +from foxops.models.group import Group +from foxops.models.incarnation import UserPermission +from foxops.models.user import User, UserWithGroups + +pytestmark = [pytest.mark.api] + + +@pytest.fixture +async def test_user(user_repository: UserRepository) -> User: + user = await user_repository.create( + username="test_user", + is_admin=False, + ) + return User.model_validate(user) + + +@pytest.fixture +async def test_group(group_repository: GroupRepository) -> Group: + group = await group_repository.create( + system_name="test_group", + display_name="test_group", + ) + return Group.model_validate(group) + + +async def test_user_and_group_get_created_on_request( + unauthenticated_client: AsyncClient, + static_api_token: str, + user_repository: UserRepository, + mocker: MockFixture, + group_repository: GroupRepository, +): + with pytest.raises(UserNotFoundError): + # The user should not exist yet + await user_repository.get_by_username("user1") + + unauthenticated_client.headers["Authorization"] = f"Bearer {static_api_token}" + unauthenticated_client.headers["User"] = "user1" + unauthenticated_client.headers["Groups"] = "group1,group2" + + response = await unauthenticated_client.get("/auth/test") + + assert response.status_code == status.HTTP_200_OK + + response_model = response.json() + + assert response_model == { + "id": mocker.ANY, + "username": "user1", + "groups": [ + { + "id": mocker.ANY, + "system_name": "group1", + "display_name": "group1", + }, + { + "id": mocker.ANY, + "system_name": "group2", + "display_name": "group2", + }, + ], + "is_admin": False, + } + + db_user = await user_repository.get_by_username("user1") + + response_user = UserWithGroups.model_validate(response_model) + + assert db_user.id == response_user.id + assert db_user.username == "user1" + assert db_user.is_admin is False + + db_groups = await group_repository.get_by_userid(db_user.id) + + assert len(db_groups) == 2 + + assert {"group1", "group2"} == {group.system_name for group in db_groups} + assert {"group1", "group2"} == {group.system_name for group in response_user.groups} + + +@pytest.mark.parametrize( + "method,endpoint", + [ + ("GET", "/user"), + ("GET", "/user/1"), + ("PATCH", "/user/1"), + ("DELETE", "/user/1"), + ("GET", "/group"), + ("GET", "/group/1"), + ("PATCH", "/group/1"), + ("DELETE", "/group/1"), + ], +) +async def test_unpriviliged_user_cant_access_group_and_user_endpoints( + method: str, + endpoint: str, + unprivileged_api_client: AsyncClient, +): + response = await unprivileged_api_client.request(method, endpoint) + + assert response.status_code == status.HTTP_403_FORBIDDEN, response.json() + assert response.json() == {"message": "Only administrators are allowed to access this endpoint"} + + +@pytest.mark.parametrize( + "method,endpoint,body,expected_status", + [ + ("GET", "/user", None, status.HTTP_200_OK), + ("GET", "/user/{user_id}", None, status.HTTP_200_OK), + ("PATCH", "/user/{user_id}", {"is_admin": True}, status.HTTP_200_OK), + ("DELETE", "/user/{user_id}", None, status.HTTP_204_NO_CONTENT), + ("GET", "/group", None, status.HTTP_200_OK), + ("GET", "/group/{group_id}", None, status.HTTP_200_OK), + ("PATCH", "/group/{group_id}", {"display_name": "new_display_name"}, status.HTTP_200_OK), + ("DELETE", "/group/{group_id}", None, status.HTTP_204_NO_CONTENT), + ], +) +async def test_priviliged_user_can_access_group_and_user_endpoints( + method: str, + endpoint: str, + api_client: AsyncClient, + test_user: User, + test_group: Group, + body: dict, + expected_status: int, +): + response = await api_client.request( + method, endpoint.format(user_id=test_user.id, group_id=test_group.id), json=body or {} + ) + + assert response.status_code == expected_status + + +async def test_cant_update_owner_field_with_write_permissions( + unprivileged_api_client: AsyncClient, + change_repository: ChangeRepository, + unprivileged_api_user: User, + incarnation_repository: IncarnationRepository, + priviliged_api_user: User, +): + incarnation = await change_repository.create_incarnation_with_first_change( + incarnation_repository="incarnation", + target_directory=".", + template_repository="template", + commit_sha="commit_sha", + requested_version="v1.0", + requested_version_hash="template_commit_sha", + requested_data=json.dumps({"foo": "bar"}), + template_data_full=json.dumps({"foo": "bar"}), + owner_id=priviliged_api_user.id, + ) + + await incarnation_repository.set_user_permissions( + incarnation.id, + [ + UserPermission(user=unprivileged_api_user, type=Permission.WRITE), + ], + ) + response = await unprivileged_api_client.patch(f"/incarnations/{incarnation.id}", json={"owner_id": 2}) + + assert response.status_code == status.HTTP_403_FORBIDDEN, response.request.content diff --git a/tests/routers/test_incarnations.py b/tests/routers/test_incarnations.py index f8fc5999..8f67e719 100644 --- a/tests/routers/test_incarnations.py +++ b/tests/routers/test_incarnations.py @@ -34,32 +34,6 @@ """ -class ChangeServiceMock(Mock): - def __init__(self): - super().__init__(spec=ChangeService) - - async def create_incarnation( - self, - incarnation_repository: str, - template_repository: str, - template_repository_version: str, - template_data: dict[str, str], - target_directory: str = ".", - owner_id: int = 1, - ) -> Change: - return Change( - id=1, - incarnation_id=1, - revision=1, - requested_version_hash="template_commit_sha", - requested_version=template_repository_version, - requested_data=template_data, - template_data_full=template_data, - created_at=datetime.now(), - commit_sha="commit_sha", - ) - - @pytest.fixture async def user(user_repository: UserRepository) -> User: return User.model_validate( @@ -72,7 +46,19 @@ async def user(user_repository: UserRepository) -> User: @pytest.fixture def change_service_mock(app: FastAPI): - change_service = ChangeServiceMock() + change_service = Mock(spec=ChangeService) + + change_service.create_incarnation.return_value = Change( + id=1, + incarnation_id=1, + revision=1, + requested_version_hash="template_commit_sha", + requested_version="test", + requested_data={"foo": "bar"}, + template_data_full={"foo": "bar"}, + created_at=datetime.now(), + commit_sha="commit_sha", + ) app.dependency_overrides[get_change_service] = lambda: change_service @@ -250,3 +236,73 @@ async def test_api_get_diff_of_non_existing_incarnation_returns_not_found( assert response.status_code == HTTPStatus.NOT_FOUND assert response.json() == {"message": "Incarnation with id '1' not found."} + + +async def test_api_create_incarnation_assigns_owner( + api_client: AsyncClient, + mocker: MockFixture, + change_service_mock: ChangeService, + priviliged_api_user: User, +): + requested_incarnation = { + "incarnation_repository": "test", + "target_directory": "test", + "template_repository": "template", + "template_repository_version": "test", + "template_data": {"foo": "bar"}, + } + + change_service_mock.get_incarnation_with_details = mocker.AsyncMock(return_value="dummy-object") # type: ignore + + response = await api_client.post( + "/incarnations", + json=requested_incarnation, + ) + + assert response.status_code == HTTPStatus.CREATED + assert change_service_mock.create_incarnation.call_count == 1 # type: ignore + + called_owner_id = change_service_mock.create_incarnation.call_args.kwargs.get("owner_id") # type: ignore + + assert called_owner_id == priviliged_api_user.id + + +async def test_administrator_see_all_incarnations( + api_client: AsyncClient, + change_repository: ChangeRepository, + priviliged_api_user: User, + unprivileged_api_user: User, +): + incarnation1 = await change_repository.create_incarnation_with_first_change( + incarnation_repository="test1", + target_directory="test1", + template_repository="template1", + commit_sha="commit_sha1", + requested_version="v1.0", + requested_version_hash="template_commit_sha1", + requested_data=json.dumps({"foo": "bar"}), + template_data_full=json.dumps({"foo": "bar"}), + owner_id=priviliged_api_user.id, + ) + + incarnation2 = await change_repository.create_incarnation_with_first_change( + incarnation_repository="test2", + target_directory="test2", + template_repository="template2", + commit_sha="commit_sha2", + requested_version="v1.0", + requested_version_hash="template_commit_sha2", + requested_data=json.dumps({"foo": "bar"}), + template_data_full=json.dumps({"foo": "bar"}), + owner_id=unprivileged_api_user.id, + ) + + response = await api_client.get("/incarnations") + + assert response.status_code == HTTPStatus.OK + + incarnations = response.json() + + assert len(incarnations) == 2 + + assert {incarnation1.id, incarnation2.id} == {i["id"] for i in incarnations} diff --git a/tests/routers/test_user.py b/tests/routers/test_user.py new file mode 100644 index 00000000..53f10fee --- /dev/null +++ b/tests/routers/test_user.py @@ -0,0 +1,20 @@ +from httpx import AsyncClient + +from foxops.database.repositories.user.repository import UserRepository +from foxops.models.user import User + + +async def test_cant_delete_own_user( + api_client: AsyncClient, user_repository: UserRepository, priviliged_api_user: User +): + response = await api_client.delete( + f"/user/{priviliged_api_user.id}", + ) + + assert response.status_code == 400 + assert response.json() == { + "message": "You can't delete yourself", + } + + # The user should still exist + await user_repository.get_by_id(priviliged_api_user.id) From a74b3357b98a557e0d47d600d74d706ece5f4abf Mon Sep 17 00:00:00 2001 From: Yannick Mueller Date: Wed, 9 Apr 2025 13:48:01 +0200 Subject: [PATCH 22/23] Add Documentation for User Concept --- docs/source/index.md | 1 + docs/source/reference/authorization.md | 113 +++++++++++++++++++++++++ 2 files changed, 114 insertions(+) create mode 100644 docs/source/reference/authorization.md diff --git a/docs/source/index.md b/docs/source/index.md index 8ca61d61..4632eae2 100644 --- a/docs/source/index.md +++ b/docs/source/index.md @@ -13,6 +13,7 @@ tutorials/index :caption: References reference/templates +reference/authorization terminology api ``` diff --git a/docs/source/reference/authorization.md b/docs/source/reference/authorization.md new file mode 100644 index 00000000..240dd508 --- /dev/null +++ b/docs/source/reference/authorization.md @@ -0,0 +1,113 @@ +# Authorization +FoxOps supports an authorization system that introduces the concept of users and groups. This allows for limiting access to certain incarnations based on the user or group. + +```{admonition} Missing Authentification +:class: caution +The current authorization system doesn't yet check if the provided users or groups are valid. Additionally, the current static API token is shared between all users, making it technically possible to access all incarnations with the API token. + +This can currently be fixed by using an external component to authenticate a request. While this is a limitation, it is planned to integrate FoxOps with an external SSO service, which would remove this limitation. +``` + +Currently, each incoming request expects three different headers + +| Header | Description | Required | +| ----- | ----------- | -------- | +| `Authorization` | The static API token which is used to authenticate the request. This token can be set with the `FOXOPS_STATIC_TOKEN` environment variable. | ✓ | +| `User` | The user which is used to authenticate the request. This user is checked to see if they have access to the requested incarnation. | ✓ | +| `Groups` | The groups which are used to authenticate the request. These groups are checked to see if the user has access to the requested incarnation. This header can also be provided empty or omitted. | X | + +## User/Group Creation +User and groups get automatically created when a request is made to the API. This means that if a user or group doesn't exist, it will be created automatically. The newly create users don't have admin rights by default. + +However there exists a default user in the database (`root`), which has admin rights. This user is created automatically when the database is initialized. + + +## Basic Authorization Concepts +As previously mentioned, FoxOps uses the provided HTTP headers (`User` and `Groups`) to authorize incoming requests. + +### User +A FoxOps user is uniquely identified by its username. It is also the username that can be used to authorize an incoming request. + +### Groups +Groups in FoxOps are uniquely identified by their system name. This system name can differ from the display name. A group can have zero or more users. + + +### Incarnation +An Incarnation always has a User as an owner. This user is permitted to perform all actions on the Incarnation. It is also possible to assign different users or groups privileges on the Incarnation. These privileges can either be read or write (which also includes read) permissions. + + +### Change +The change model now also stores which user initialized said change. However, it is not possible to give certain groups or users permissions on a change. The change permission is managed by the Incarnation to which the change belongs. + + +### Access Control +The following endpoints are protected with an authorization layer, which requires certain rights. + +```{admonition} Access Control +Users which have admin rights are allowed to perform any actions on any endpoints. +``` + +| Endpoint | Method | Required Permission | +| -------- | ------ | ------------------- | +| `/incarnations` | GET | only returns the Incarnations, to which the User has either read permission or is the owner | +| `/incarnations/{id}` | GET | read or is the owner | +| `/incarnations` | POST | No permissions required. However the current user automatically becomes the owner of the newly created Incarnation. | +| `/incarnations/{id}` | PUT | write or is the owner | +| `/incarnations/{id}` | DELETE | write or is the owner | +| `/incarnations/{id}` | PATCH | write or is the owner | +| `/incarnations/{id}/reset` | POST | write or is the owner | +| `/incarnations/{id}/diff` | GET | read or is the owner | +| `/incarnations/{id}/changes` | GET | read or is the owner | +| `/incarnations/{id}/changes` | POST | write or is the owner | +| `/incarnations/{id}/changes/{revision}` | GET | read or is the owner | +| `/incarnations/{id}/changes/{revision}/fix` | POST | write or is the owner | +| `/user` | GET | Admin only | +| `/user/{id}` | GET | Admin only | +| `/user/{id}` | PATCH | Admin only | +| `/user/{id}` | DELETE | Admin only | +| `/group` | GET | Admin only | +| `/group/{id}` | GET | Admin only | +| `/group/{id}` | PATCH | Admin only | +| `/group/{id}` | DELETE | Admin only | + +## Productive Setup +While the FoxOps API currently doesn't authenticate the User and Groups headers, it is still possible to use this setup in a productive environment. An example of such a setup is described below. + +The following mermaid diagram describes the setup of FoxOps with an external SSO service and a reverse proxy. The reverse proxy is used to authenticate the user and add the User and Groups headers to the request. This ensures that the provided user and groups are valid. + +```{mermaid} +graph TD + A[Client/User] + B[SSO Service] + C[Reverse Proxy] + D[FoxOps API] + + A --> |1. Login and request JWT| B + B --> |2. JWT| A + A --> |3. Request FoxOps API with JWT| C + C --> |4. Validates JWT| C + C --> |5. Request FoxOps API with Headers| D + D --> |6. Response| C + C --> |6. Response| A +``` + +### 1. Login and request JWT +The first step in this setup is to request a new JWT token from the SSO service. An example of such a service would be [Keycloak](https://www.keycloak.org/). + +### 2. JWT +The SSO service then responds with a signed JWT token. This token contains the user and group information. + +### 3. Request FoxOps API with JWT +The client then requests the FoxOps API with the JWT token. This token is used to authenticate the user and groups. The client does **not** provide any of the required headers for the FoxOps API. + +### 4. Validates JWT +The reverse proxy then validates the JWT signature. It parses the user and group information from the JWT token. + +### 5. Request FoxOps API with Headers +The reverse proxy then requests the FoxOps API. It provides the three required headers: +* Authorization: _Static API Token configured in the Reverse Proxy_ +* User: _User from the JWT token_ +* Groups: _Groups from the JWT token_ + +### 6. Response +The API uses the provided user and groups to check if the user has access to the requested incarnation. If the user has access, the API returns the response to the reverse proxy. The reverse proxy then returns the response to the client. From 7e380d93130affbf85a908cb10ea66f268603a12 Mon Sep 17 00:00:00 2001 From: Yannick Mueller Date: Wed, 9 Apr 2025 14:34:51 +0200 Subject: [PATCH 23/23] Fix: Stadartize API Design --- docs/source/reference/authorization.md | 16 ++++++------- src/foxops/routers/group.py | 2 +- src/foxops/routers/user.py | 2 +- tests/routers/test_authz.py | 32 +++++++++++++------------- tests/routers/test_user.py | 2 +- 5 files changed, 27 insertions(+), 27 deletions(-) diff --git a/docs/source/reference/authorization.md b/docs/source/reference/authorization.md index 240dd508..d4669f41 100644 --- a/docs/source/reference/authorization.md +++ b/docs/source/reference/authorization.md @@ -61,14 +61,14 @@ Users which have admin rights are allowed to perform any actions on any endpoint | `/incarnations/{id}/changes` | POST | write or is the owner | | `/incarnations/{id}/changes/{revision}` | GET | read or is the owner | | `/incarnations/{id}/changes/{revision}/fix` | POST | write or is the owner | -| `/user` | GET | Admin only | -| `/user/{id}` | GET | Admin only | -| `/user/{id}` | PATCH | Admin only | -| `/user/{id}` | DELETE | Admin only | -| `/group` | GET | Admin only | -| `/group/{id}` | GET | Admin only | -| `/group/{id}` | PATCH | Admin only | -| `/group/{id}` | DELETE | Admin only | +| `/users` | GET | Admin only | +| `/users/{id}` | GET | Admin only | +| `/users/{id}` | PATCH | Admin only | +| `/users/{id}` | DELETE | Admin only | +| `/groups` | GET | Admin only | +| `/groups/{id}` | GET | Admin only | +| `/groups/{id}` | PATCH | Admin only | +| `/groups/{id}` | DELETE | Admin only | ## Productive Setup While the FoxOps API currently doesn't authenticate the User and Groups headers, it is still possible to use this setup in a productive environment. An example of such a setup is described below. diff --git a/src/foxops/routers/group.py b/src/foxops/routers/group.py index 8666f303..de212be0 100644 --- a/src/foxops/routers/group.py +++ b/src/foxops/routers/group.py @@ -8,7 +8,7 @@ from foxops.models.user import GroupWithUsers from foxops.services.group import GroupService -router = APIRouter(prefix="/api/group", tags=["group"]) +router = APIRouter(prefix="/api/groups", tags=["group"]) @router.get( diff --git a/src/foxops/routers/user.py b/src/foxops/routers/user.py index c1de860f..6b1d3d46 100644 --- a/src/foxops/routers/user.py +++ b/src/foxops/routers/user.py @@ -8,7 +8,7 @@ from foxops.services.authorization import AuthorizationService from foxops.services.user import UserService -router = APIRouter(prefix="/api/user", tags=["user"]) +router = APIRouter(prefix="/api/users", tags=["user"]) @router.get( diff --git a/tests/routers/test_authz.py b/tests/routers/test_authz.py index e760b9fe..7f2c11d5 100644 --- a/tests/routers/test_authz.py +++ b/tests/routers/test_authz.py @@ -94,14 +94,14 @@ async def test_user_and_group_get_created_on_request( @pytest.mark.parametrize( "method,endpoint", [ - ("GET", "/user"), - ("GET", "/user/1"), - ("PATCH", "/user/1"), - ("DELETE", "/user/1"), - ("GET", "/group"), - ("GET", "/group/1"), - ("PATCH", "/group/1"), - ("DELETE", "/group/1"), + ("GET", "/users"), + ("GET", "/users/1"), + ("PATCH", "/users/1"), + ("DELETE", "/users/1"), + ("GET", "/groups"), + ("GET", "/groups/1"), + ("PATCH", "/groups/1"), + ("DELETE", "/groups/1"), ], ) async def test_unpriviliged_user_cant_access_group_and_user_endpoints( @@ -118,14 +118,14 @@ async def test_unpriviliged_user_cant_access_group_and_user_endpoints( @pytest.mark.parametrize( "method,endpoint,body,expected_status", [ - ("GET", "/user", None, status.HTTP_200_OK), - ("GET", "/user/{user_id}", None, status.HTTP_200_OK), - ("PATCH", "/user/{user_id}", {"is_admin": True}, status.HTTP_200_OK), - ("DELETE", "/user/{user_id}", None, status.HTTP_204_NO_CONTENT), - ("GET", "/group", None, status.HTTP_200_OK), - ("GET", "/group/{group_id}", None, status.HTTP_200_OK), - ("PATCH", "/group/{group_id}", {"display_name": "new_display_name"}, status.HTTP_200_OK), - ("DELETE", "/group/{group_id}", None, status.HTTP_204_NO_CONTENT), + ("GET", "/users", None, status.HTTP_200_OK), + ("GET", "/users/{user_id}", None, status.HTTP_200_OK), + ("PATCH", "/users/{user_id}", {"is_admin": True}, status.HTTP_200_OK), + ("DELETE", "/users/{user_id}", None, status.HTTP_204_NO_CONTENT), + ("GET", "/groups", None, status.HTTP_200_OK), + ("GET", "/groups/{group_id}", None, status.HTTP_200_OK), + ("PATCH", "/groups/{group_id}", {"display_name": "new_display_name"}, status.HTTP_200_OK), + ("DELETE", "/groups/{group_id}", None, status.HTTP_204_NO_CONTENT), ], ) async def test_priviliged_user_can_access_group_and_user_endpoints( diff --git a/tests/routers/test_user.py b/tests/routers/test_user.py index 53f10fee..924637c1 100644 --- a/tests/routers/test_user.py +++ b/tests/routers/test_user.py @@ -8,7 +8,7 @@ async def test_cant_delete_own_user( api_client: AsyncClient, user_repository: UserRepository, priviliged_api_user: User ): response = await api_client.delete( - f"/user/{priviliged_api_user.id}", + f"/users/{priviliged_api_user.id}", ) assert response.status_code == 400