From aeb4f10fd003dc3e62d86974f019bac65f4d9876 Mon Sep 17 00:00:00 2001 From: Justin Ko Date: Fri, 3 Apr 2026 13:21:03 +0000 Subject: [PATCH] refactor(auth): remove bot owner linkage and enforce bot-level permissions --- app/api/auth_routes.py | 2 +- app/domain/models.py | 2 +- app/repositories/portfolio_repository.py | 2 +- app/repositories/postgres.py | 31 +++++----- app/schemas/auth.py | 4 +- app/services/auth_service.py | 14 ++--- docs/api-examples.md | 6 +- docs/db-schema-spec.md | 6 +- docs/portfoliohub-spec.md | 4 +- ...0403_0006_bot_principals_and_bot_tokens.py | 2 +- ...0403_0007_bot_principal_permission_only.py | 62 +++++++++++++++++++ tests/test_users.py | 12 ++-- 12 files changed, 104 insertions(+), 43 deletions(-) create mode 100644 migrations/versions/20260403_0007_bot_principal_permission_only.py diff --git a/app/api/auth_routes.py b/app/api/auth_routes.py index 835a156..57b4756 100644 --- a/app/api/auth_routes.py +++ b/app/api/auth_routes.py @@ -57,8 +57,8 @@ def _to_auth_token_response(token: AuthToken) -> AuthTokenResponse: def _to_bot_principal_response(bot: BotPrincipal) -> BotPrincipalResponse: return BotPrincipalResponse( id=bot.id, - owner_user_id=bot.owner_user_id, name=bot.name, + permission=bot.permission, description=bot.description, is_active=bot.is_active, created_at=bot.created_at, diff --git a/app/domain/models.py b/app/domain/models.py index 023d38d..e182b68 100644 --- a/app/domain/models.py +++ b/app/domain/models.py @@ -56,8 +56,8 @@ class TokenPrincipal: @dataclass class BotPrincipal: id: str - owner_user_id: str name: str + permission: str description: str | None = None is_active: bool = True created_at: datetime | None = None diff --git a/app/repositories/portfolio_repository.py b/app/repositories/portfolio_repository.py index b7076a6..3cbd985 100644 --- a/app/repositories/portfolio_repository.py +++ b/app/repositories/portfolio_repository.py @@ -62,8 +62,8 @@ class PortfolioRepository(Protocol): def create_bot_principal( self, *, - owner_user_id: str, name: str, + permission: str, description: str | None = None, ) -> BotPrincipal: ... diff --git a/app/repositories/postgres.py b/app/repositories/postgres.py index a6ace20..2f2a1c7 100644 --- a/app/repositories/postgres.py +++ b/app/repositories/postgres.py @@ -47,8 +47,8 @@ _SELECT_AUTH_TOKEN = """ _SELECT_BOT_PRINCIPAL = """ SELECT id, - owner_user_id, name, + permission, description, is_active, created_at, @@ -158,8 +158,8 @@ class PostgresPortfolioRepository: """ CREATE TABLE IF NOT EXISTS bot_principals ( id TEXT PRIMARY KEY, - owner_user_id TEXT NOT NULL, name TEXT NOT NULL UNIQUE, + permission TEXT NOT NULL, description TEXT, is_active BOOLEAN NOT NULL DEFAULT TRUE, created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), @@ -169,6 +169,9 @@ class PostgresPortfolioRepository: """ ) cur.execute("ALTER TABLE auth_tokens ADD COLUMN IF NOT EXISTS bot_principal_id TEXT") + cur.execute("ALTER TABLE bot_principals ADD COLUMN IF NOT EXISTS permission TEXT") + cur.execute("UPDATE bot_principals SET permission = 'read' WHERE permission IS NULL") + cur.execute("ALTER TABLE bot_principals ALTER COLUMN permission SET NOT NULL") cur.execute( """ CREATE INDEX IF NOT EXISTS idx_auth_tokens_user_created @@ -304,8 +307,8 @@ class PostgresPortfolioRepository: raise ValueError("봇을 찾을 수 없습니다") return BotPrincipal( id=row[0], - owner_user_id=row[1], - name=row[2], + name=row[1], + permission=row[2], description=row[3] if len(row) > 3 else None, is_active=bool(row[4]) if len(row) > 4 else True, created_at=row[5] if len(row) > 5 else None, @@ -515,9 +518,10 @@ class PostgresPortfolioRepository: ) -> AuthToken: self.ensure_schema() with self._connect().cursor() as cur: - cur.execute("SELECT 1 FROM users WHERE id = %s", (user_id,)) - if cur.fetchone() is None: - raise ValueError("사용자를 찾을 수 없습니다") + if token_type != "bot": + cur.execute("SELECT 1 FROM users WHERE id = %s", (user_id,)) + if cur.fetchone() is None: + raise ValueError("사용자를 찾을 수 없습니다") if bot_principal_id is not None: cur.execute("SELECT 1 FROM bot_principals WHERE id = %s", (bot_principal_id,)) if cur.fetchone() is None: @@ -562,15 +566,12 @@ class PostgresPortfolioRepository: def create_bot_principal( self, *, - owner_user_id: str, name: str, + permission: str, description: str | None = None, ) -> BotPrincipal: self.ensure_schema() with self._connect().cursor() as cur: - cur.execute("SELECT 1 FROM users WHERE id = %s", (owner_user_id,)) - if cur.fetchone() is None: - raise ValueError("소유 사용자를 찾을 수 없습니다") cur.execute("SELECT 1 FROM bot_principals WHERE name = %s", (name,)) if cur.fetchone() is not None: raise ValueError(f"이미 존재하는 bot name입니다: {name}") @@ -578,11 +579,11 @@ class PostgresPortfolioRepository: bot_id = str(uuid.uuid4()) cur.execute( """ - INSERT INTO bot_principals (id, owner_user_id, name, description) + INSERT INTO bot_principals (id, name, permission, description) VALUES (%s, %s, %s, %s) - RETURNING id, owner_user_id, name, description, is_active, created_at, updated_at, deactivated_at + RETURNING id, name, permission, description, is_active, created_at, updated_at, deactivated_at """, - (bot_id, owner_user_id, name, description), + (bot_id, name, permission, description), ) row = cur.fetchone() return self._row_to_bot_principal(row) @@ -615,7 +616,7 @@ class PostgresPortfolioRepository: UPDATE bot_principals SET is_active = FALSE, deactivated_at = NOW(), updated_at = NOW() WHERE id = %s - RETURNING id, owner_user_id, name, description, is_active, created_at, updated_at, deactivated_at + RETURNING id, name, permission, description, is_active, created_at, updated_at, deactivated_at """, (bot_principal_id,), ) diff --git a/app/schemas/auth.py b/app/schemas/auth.py index 609f2c5..3257aa9 100644 --- a/app/schemas/auth.py +++ b/app/schemas/auth.py @@ -66,8 +66,8 @@ class IssuedClientTokenResponse(BaseModel): class CreateBotPrincipalRequest(BaseModel): - owner_user_id: str = Field(min_length=1) name: str = Field(min_length=1) + permission: Literal["read", "trade-write", "admin"] description: str | None = None @field_validator("name") @@ -81,8 +81,8 @@ class CreateBotPrincipalRequest(BaseModel): class BotPrincipalResponse(BaseModel): id: str - owner_user_id: str name: str + permission: Literal["read", "trade-write", "admin"] description: str | None = None is_active: bool created_at: datetime | None = None diff --git a/app/services/auth_service.py b/app/services/auth_service.py index a4ef1cb..e238d9f 100644 --- a/app/services/auth_service.py +++ b/app/services/auth_service.py @@ -103,10 +103,9 @@ class AuthService: self._repository.revoke_auth_token(auth_context.user_id, token_id) def create_bot_principal(self, payload: CreateBotPrincipalRequest) -> BotPrincipal: - self._repository.get_user(payload.owner_user_id) return self._repository.create_bot_principal( - owner_user_id=payload.owner_user_id, name=payload.name, + permission=payload.permission, description=payload.description, ) @@ -124,16 +123,15 @@ class AuthService: if not bot.is_active: raise ValueError("비활성화된 봇에는 토큰을 발급할 수 없습니다") - owner = self._repository.get_user(bot.owner_user_id) - owner_permission = Permission(owner.permission) - requested = Permission(payload.permission or owner.permission) - if not _has_permission(owner_permission, requested): - raise ValueError("요청한 토큰 권한이 봇 소유자의 권한을 초과합니다") + bot_permission = Permission(bot.permission) + requested = Permission(payload.permission or bot.permission) + if not _has_permission(bot_permission, requested): + raise ValueError("요청한 토큰 권한이 봇 권한을 초과합니다") raw_token = generate_token() expires_at = self._build_expiration(payload.expires_in_days, self._settings.bot_token_default_ttl_days) auth_token = self._repository.create_auth_token( - owner.id, + bot.id, name=payload.name, token_hash=hash_token(raw_token), token_prefix=token_prefix(raw_token), diff --git a/docs/api-examples.md b/docs/api-examples.md index b5aba89..701d9f9 100644 --- a/docs/api-examples.md +++ b/docs/api-examples.md @@ -363,8 +363,8 @@ curl -X POST "http://localhost:8000/auth/bots" \ -H "Authorization: Bearer " \ -H "Content-Type: application/json" \ -d '{ - "owner_user_id": "user-1", "name": "market-bot", + "permission": "trade-write", "description": "External trading bot" }' ``` @@ -374,8 +374,8 @@ Response 201: ```json { "id": "bot-1", - "owner_user_id": "user-1", "name": "market-bot", + "permission": "trade-write", "description": "External trading bot", "is_active": true, "created_at": "2026-04-03T00:00:00Z", @@ -466,8 +466,8 @@ Response 200: ```json { "id": "bot-1", - "owner_user_id": "user-1", "name": "market-bot", + "permission": "trade-write", "description": "External trading bot", "is_active": false, "created_at": "2026-04-03T00:00:00Z", diff --git a/docs/db-schema-spec.md b/docs/db-schema-spec.md index 346ef74..6dbd40c 100644 --- a/docs/db-schema-spec.md +++ b/docs/db-schema-spec.md @@ -5,7 +5,7 @@ This document defines the database schema in an AI-agent-friendly format. - Source of truth: Alembic revisions in `migrations/versions` -- Current baseline revision: `20260403_0006` +- Current baseline revision: `20260403_0007` - Baseline files: - `migrations/versions/20260403_0001_initial_portfolio_schema.py` - `migrations/versions/20260403_0002_strategy_set_columns.py` @@ -13,6 +13,7 @@ This document defines the database schema in an AI-agent-friendly format. - `migrations/versions/20260403_0004_users_table.py` - `migrations/versions/20260403_0005_auth_user_fields_and_tokens.py` - `migrations/versions/20260403_0006_bot_principals_and_bot_tokens.py` + - `migrations/versions/20260403_0007_bot_principal_permission_only.py` ## 2. Scope @@ -101,8 +102,8 @@ Columns: | Column | Type | Null | Default | Constraints / Notes | |---|---|---|---|---| | id | Text | NO | - | Primary Key | -| owner_user_id | Text | NO | - | Logical owner reference to users.id | | name | Text | NO | - | Unique bot name | +| permission | Text | NO | - | Bot permission ceiling | | description | Text | YES | - | Optional operator description | | is_active | Boolean | NO | TRUE | When false, bot tokens are rejected | | created_at | DateTime(timezone=True) | NO | NOW() | Creation timestamp | @@ -292,3 +293,4 @@ When schema changes: | 20260403_0004 | migrations/versions/20260403_0004_users_table.py | Add `users` table with unique username/email and created_at index | | 20260403_0005 | migrations/versions/20260403_0005_auth_user_fields_and_tokens.py | Add password/permission/login fields to `users` and create `auth_tokens` table | | 20260403_0006 | migrations/versions/20260403_0006_bot_principals_and_bot_tokens.py | Add `bot_principals` table and `auth_tokens.bot_principal_id` for bot API key management | +| 20260403_0007 | migrations/versions/20260403_0007_bot_principal_permission_only.py | Remove `bot_principals.owner_user_id` and enforce bot-local `permission` model | diff --git a/docs/portfoliohub-spec.md b/docs/portfoliohub-spec.md index 41c4907..636576c 100644 --- a/docs/portfoliohub-spec.md +++ b/docs/portfoliohub-spec.md @@ -441,8 +441,8 @@ Bootstrap rule: `CreateBotPrincipalRequest` -- `owner_user_id: str` - `name: str` +- `permission: "read" | "trade-write" | "admin"` - `description: str | null` `CreateBotTokenRequest` @@ -529,8 +529,8 @@ Bootstrap rule: `BotPrincipalResponse` - `id: str` -- `owner_user_id: str` - `name: str` +- `permission: "read" | "trade-write" | "admin"` - `description: str | null` - `is_active: bool` - `created_at: datetime | null` diff --git a/migrations/versions/20260403_0006_bot_principals_and_bot_tokens.py b/migrations/versions/20260403_0006_bot_principals_and_bot_tokens.py index d25f439..b883035 100644 --- a/migrations/versions/20260403_0006_bot_principals_and_bot_tokens.py +++ b/migrations/versions/20260403_0006_bot_principals_and_bot_tokens.py @@ -38,8 +38,8 @@ def upgrade() -> None: op.create_table( "bot_principals", sa.Column("id", sa.Text(), nullable=False), - sa.Column("owner_user_id", sa.Text(), nullable=False), sa.Column("name", sa.Text(), nullable=False), + sa.Column("permission", sa.Text(), nullable=False, server_default="read"), sa.Column("description", sa.Text(), nullable=True), sa.Column("is_active", sa.Boolean(), nullable=False, server_default=sa.text("TRUE")), sa.Column("created_at", sa.TIMESTAMP(timezone=True), nullable=False, server_default=sa.text("NOW()")), diff --git a/migrations/versions/20260403_0007_bot_principal_permission_only.py b/migrations/versions/20260403_0007_bot_principal_permission_only.py new file mode 100644 index 0000000..08458eb --- /dev/null +++ b/migrations/versions/20260403_0007_bot_principal_permission_only.py @@ -0,0 +1,62 @@ +"""remove bot owner link and enforce bot permission + +Revision ID: 20260403_0007 +Revises: 20260403_0006 +Create Date: 2026-04-03 14:20:00 +""" + +from __future__ import annotations + +import sqlalchemy as sa +from alembic import op +from sqlalchemy import inspect + + +revision = "20260403_0007" +down_revision = "20260403_0006" +branch_labels = None +depends_on = None + + +def _table_exists(inspector, table_name: str) -> bool: + return table_name in inspector.get_table_names() + + +def _column_exists(inspector, table_name: str, column_name: str) -> bool: + return any(column["name"] == column_name for column in inspector.get_columns(table_name)) + + +def upgrade() -> None: + bind = op.get_bind() + inspector = inspect(bind) + + if not _table_exists(inspector, "bot_principals"): + return + + if not _column_exists(inspector, "bot_principals", "permission"): + op.add_column("bot_principals", sa.Column("permission", sa.Text(), nullable=True)) + + op.execute("UPDATE bot_principals SET permission = 'read' WHERE permission IS NULL") + op.alter_column("bot_principals", "permission", existing_type=sa.Text(), nullable=False) + + inspector = inspect(bind) + if _column_exists(inspector, "bot_principals", "owner_user_id"): + op.drop_column("bot_principals", "owner_user_id") + + +def downgrade() -> None: + bind = op.get_bind() + inspector = inspect(bind) + + if not _table_exists(inspector, "bot_principals"): + return + + if not _column_exists(inspector, "bot_principals", "owner_user_id"): + op.add_column("bot_principals", sa.Column("owner_user_id", sa.Text(), nullable=True)) + + op.execute("UPDATE bot_principals SET owner_user_id = 'legacy-owner' WHERE owner_user_id IS NULL") + op.alter_column("bot_principals", "owner_user_id", existing_type=sa.Text(), nullable=False) + + inspector = inspect(bind) + if _column_exists(inspector, "bot_principals", "permission"): + op.drop_column("bot_principals", "permission") diff --git a/tests/test_users.py b/tests/test_users.py index cfdf920..72cd094 100644 --- a/tests/test_users.py +++ b/tests/test_users.py @@ -168,18 +168,17 @@ class FakeUserRepo: def create_bot_principal( self, *, - owner_user_id: str, name: str, + permission: str, description: str | None = None, ) -> BotPrincipal: - self.get_user(owner_user_id) if any(item.name == name for item in self.bots.values()): raise ValueError(f"이미 존재하는 bot name입니다: {name}") now = datetime.now(timezone.utc) bot = BotPrincipal( id=f"bot-{self.next_bot_id}", - owner_user_id=owner_user_id, name=name, + permission=permission, description=description, is_active=True, created_at=now, @@ -331,7 +330,6 @@ def test_login_and_issue_client_token_flow(monkeypatch) -> None: def test_admin_can_create_bot_and_issue_token(monkeypatch) -> None: repo = FakeUserRepo() - owner = repo.create_user("bot-owner", password_hash=hash_password("password123"), permission="trade-write") admin = repo.create_user("admin", password_hash=hash_password("admin123!"), permission="admin") client = _client_with_repo(monkeypatch, repo) @@ -342,7 +340,7 @@ def test_admin_can_create_bot_and_issue_token(monkeypatch) -> None: bot_response = client.post( "/auth/bots", headers={"Authorization": f"Bearer {admin_token}"}, - json={"owner_user_id": owner.id, "name": "market-bot", "description": "external execution bot"}, + json={"name": "market-bot", "permission": "trade-write", "description": "external execution bot"}, ) assert bot_response.status_code == 201 bot_id = bot_response.json()["id"] @@ -359,12 +357,12 @@ def test_admin_can_create_bot_and_issue_token(monkeypatch) -> None: assert payload["token"] assert payload["auth_token"]["token_type"] == "bot" assert payload["auth_token"]["bot_principal_id"] == bot_id + assert payload["auth_token"]["permission"] == "read" assert payload["auth_token"]["expires_at"] is not None def test_deactivated_bot_token_is_rejected(monkeypatch) -> None: repo = FakeUserRepo() - owner = repo.create_user("bot-owner", password_hash=hash_password("password123"), permission="read") admin = repo.create_user("admin", password_hash=hash_password("admin123!"), permission="admin") client = _client_with_repo(monkeypatch, repo) @@ -375,7 +373,7 @@ def test_deactivated_bot_token_is_rejected(monkeypatch) -> None: bot_response = client.post( "/auth/bots", headers={"Authorization": f"Bearer {admin_token}"}, - json={"owner_user_id": owner.id, "name": "ops-bot"}, + json={"name": "ops-bot", "permission": "read"}, ) assert bot_response.status_code == 201 bot_id = bot_response.json()["id"]