From 9787cf31594473d98289102b5a711221ea0006c0 Mon Sep 17 00:00:00 2001 From: Jinhwan Ko Date: Tue, 14 Apr 2026 00:13:34 +0900 Subject: [PATCH] feat: add portfolio ownership model, auth guards, and migration --- README.md | 20 --- app/api/portfolios.py | 82 +++++++++-- app/domain/models.py | 1 + app/domain/services.py | 12 +- app/repositories/portfolio_repository.py | 10 +- app/repositories/postgres.py | 49 +++++-- app/schemas/portfolio.py | 1 + app/services/portfolio_service.py | 19 ++- docker-compose.local-test.yml | 50 +++++++ ...-004_portfolio-ownership-access-control.md | 74 ++++++++++ docs/SRS/index.md | 5 +- docs/SRS/traceability-matrix.md | 1 + docs/technical/api-contracts.md | 7 +- docs/technical/auth-and-token-lifecycle.md | 7 +- docs/technical/index.md | 2 + docs/technical/persistence-and-migrations.md | 15 +- .../portfolio-ownership-and-auth-design.md | 137 ++++++++++++++++++ .../20260413_0010_portfolio_owner_user_id.py | 97 +++++++++++++ tests/test_cli.py | 14 +- tests/test_domain_services.py | 12 +- tests/test_portfolio_comments.py | 5 +- tests/test_portfolio_ownership.py | 131 +++++++++++++++++ 22 files changed, 681 insertions(+), 70 deletions(-) create mode 100644 docker-compose.local-test.yml create mode 100644 docs/SRS/Interface/REQ-004_portfolio-ownership-access-control.md create mode 100644 docs/technical/portfolio-ownership-and-auth-design.md create mode 100644 migrations/versions/20260413_0010_portfolio_owner_user_id.py create mode 100644 tests/test_portfolio_ownership.py diff --git a/README.md b/README.md index 95fec15..ad00e9f 100644 --- a/README.md +++ b/README.md @@ -57,26 +57,6 @@ uvicorn app.main:app --reload pytest ``` -## Production Deployment (HTTPS with Caddy) - -For production, it is recommended to run PortfolioHub behind [Caddy](https://caddyserver.com/), which handles TLS certificate provisioning automatically. - -A minimal `Caddyfile`: - -``` -your-domain.com { - reverse_proxy localhost:8000 -} -``` - -See **[docs/technical/deployment-and-operations.md](docs/technical/deployment-and-operations.md)** for a full guide covering: - -- Caddy installation on Linux/Ubuntu -- CORS configuration (`PORTFOLIOHUB_ALLOWED_ORIGINS`) -- Running `uvicorn` as a systemd service -- Docker Compose setup with a Caddy sidecar container -- Firewall port configuration (80, 443) - ## CLI The repository now includes a `portfoliohub` CLI that mirrors the main portfolio flows from `jsbot-kis`. diff --git a/app/api/portfolios.py b/app/api/portfolios.py index 230384a..be392b5 100644 --- a/app/api/portfolios.py +++ b/app/api/portfolios.py @@ -2,7 +2,7 @@ from fastapi import APIRouter, Depends, HTTPException, Request, Response, status -from app.core.auth import Permission, authorize_request +from app.core.auth import AuthContext, Permission, authorize_request from app.core.config import Settings, get_settings from app.repositories.factory import get_portfolio_repository_from_settings from app.schemas.portfolio import ( @@ -41,6 +41,28 @@ def _close_repository(repository: object) -> None: repository.close() +def _ownership_required(settings: Settings) -> bool: + return settings.db_enabled + + +def _require_owner_context(auth_context: AuthContext) -> str: + if auth_context.user_id is None: + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="Ownership context is required") + return auth_context.user_id + + +def _ensure_portfolio_owned(repository: object, portfolio_id: str, auth_context: AuthContext) -> None: + if auth_context.permission == Permission.ADMIN: + return + owner_user_id = _require_owner_context(auth_context) + try: + portfolio = repository.get(portfolio_id) + except ValueError as exc: + _raise_portfolio_error(exc) + if portfolio.owner_user_id != owner_user_id: + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="Portfolio ownership mismatch") + + @router.get("", response_model=list[PortfolioListItem]) def list_portfolios( request: Request, @@ -48,10 +70,13 @@ def list_portfolios( status_filter: str | None = None, settings: Settings = Depends(get_settings), ) -> list[PortfolioListItem]: - authorize_request(Permission.READ, request, settings) + auth_context = authorize_request(Permission.READ, request, settings) service, repository = _get_service(settings) try: - return service.list_portfolios(broker=broker, status=status_filter) + owner_user_id: str | None = None + if _ownership_required(settings) and auth_context.permission != Permission.ADMIN: + owner_user_id = _require_owner_context(auth_context) + return service.list_portfolios(broker=broker, status=status_filter, owner_user_id=owner_user_id) finally: _close_repository(repository) @@ -64,9 +89,11 @@ def get_portfolio( recent_trades: int = 5, settings: Settings = Depends(get_settings), ) -> PortfolioDetailResponse: - authorize_request(Permission.READ, request, settings) + auth_context = authorize_request(Permission.READ, request, settings) service, repository = _get_service(settings) try: + if _ownership_required(settings): + _ensure_portfolio_owned(repository, portfolio_id, auth_context) return service.get_portfolio_status(portfolio_id=portfolio_id, recent_trades=recent_trades) finally: _close_repository(repository) @@ -83,10 +110,13 @@ def create_portfolio( *, settings: Settings = Depends(get_settings), ) -> PortfolioDetailResponse: - authorize_request(Permission.TRADE_WRITE, request, settings) + auth_context = authorize_request(Permission.TRADE_WRITE, request, settings) service, repository = _get_service(settings) try: - return service.create_portfolio(payload) + owner_user_id = auth_context.user_id or "system" + if _ownership_required(settings): + owner_user_id = _require_owner_context(auth_context) + return service.create_portfolio(payload, owner_user_id=owner_user_id) finally: _close_repository(repository) @@ -99,9 +129,11 @@ def update_portfolio( *, settings: Settings = Depends(get_settings), ) -> PortfolioDetailResponse: - authorize_request(Permission.TRADE_WRITE, request, settings) + auth_context = authorize_request(Permission.TRADE_WRITE, request, settings) service, repository = _get_service(settings) try: + if _ownership_required(settings): + _ensure_portfolio_owned(repository, portfolio_id, auth_context) return service.update_portfolio(portfolio_id, payload) finally: _close_repository(repository) @@ -115,9 +147,11 @@ def buy_portfolio_asset( *, settings: Settings = Depends(get_settings), ) -> PortfolioDetailResponse: - authorize_request(Permission.TRADE_WRITE, request, settings) + auth_context = authorize_request(Permission.TRADE_WRITE, request, settings) service, repository = _get_service(settings) try: + if _ownership_required(settings): + _ensure_portfolio_owned(repository, portfolio_id, auth_context) return service.buy(portfolio_id, payload) finally: _close_repository(repository) @@ -131,9 +165,11 @@ def sell_portfolio_asset( *, settings: Settings = Depends(get_settings), ) -> PortfolioDetailResponse: - authorize_request(Permission.TRADE_WRITE, request, settings) + auth_context = authorize_request(Permission.TRADE_WRITE, request, settings) service, repository = _get_service(settings) try: + if _ownership_required(settings): + _ensure_portfolio_owned(repository, portfolio_id, auth_context) return service.sell(portfolio_id, payload) finally: _close_repository(repository) @@ -147,9 +183,11 @@ def invest_buffer( *, settings: Settings = Depends(get_settings), ) -> PortfolioDetailResponse: - authorize_request(Permission.TRADE_WRITE, request, settings) + auth_context = authorize_request(Permission.TRADE_WRITE, request, settings) service, repository = _get_service(settings) try: + if _ownership_required(settings): + _ensure_portfolio_owned(repository, portfolio_id, auth_context) return service.invest(portfolio_id, payload) finally: _close_repository(repository) @@ -163,9 +201,11 @@ def transfer_buffer( *, settings: Settings = Depends(get_settings), ) -> PortfolioDetailResponse: - authorize_request(Permission.TRADE_WRITE, request, settings) + auth_context = authorize_request(Permission.TRADE_WRITE, request, settings) service, repository = _get_service(settings) try: + if _ownership_required(settings): + _ensure_portfolio_owned(repository, portfolio_id, auth_context) return service.transfer(portfolio_id, payload) finally: _close_repository(repository) @@ -179,9 +219,11 @@ def reset_portfolio( *, settings: Settings = Depends(get_settings), ) -> PortfolioDetailResponse: - authorize_request(Permission.TRADE_WRITE, request, settings) + auth_context = authorize_request(Permission.TRADE_WRITE, request, settings) service, repository = _get_service(settings) try: + if _ownership_required(settings): + _ensure_portfolio_owned(repository, portfolio_id, auth_context) return service.reset(portfolio_id, payload) finally: _close_repository(repository) @@ -216,9 +258,11 @@ def create_comment( *, settings: Settings = Depends(get_settings), ) -> CommentResponse: - authorize_request(Permission.TRADE_WRITE, request, settings) + auth_context = authorize_request(Permission.TRADE_WRITE, request, settings) service, repository = _get_service(settings) try: + if _ownership_required(settings): + _ensure_portfolio_owned(repository, portfolio_id, auth_context) return service.create_comment(portfolio_id, payload) except ValueError as exc: _raise_portfolio_error(exc) @@ -233,9 +277,11 @@ def list_comments( *, settings: Settings = Depends(get_settings), ) -> list[CommentResponse]: - authorize_request(Permission.READ, request, settings) + auth_context = authorize_request(Permission.READ, request, settings) service, repository = _get_service(settings) try: + if _ownership_required(settings): + _ensure_portfolio_owned(repository, portfolio_id, auth_context) return service.list_comments(portfolio_id) except ValueError as exc: _raise_portfolio_error(exc) @@ -252,9 +298,11 @@ def update_comment( *, settings: Settings = Depends(get_settings), ) -> CommentResponse: - authorize_request(Permission.TRADE_WRITE, request, settings) + auth_context = authorize_request(Permission.TRADE_WRITE, request, settings) service, repository = _get_service(settings) try: + if _ownership_required(settings): + _ensure_portfolio_owned(repository, portfolio_id, auth_context) return service.update_comment(portfolio_id, comment_id, payload) except ValueError as exc: _raise_portfolio_error(exc) @@ -270,9 +318,11 @@ def delete_comment( *, settings: Settings = Depends(get_settings), ) -> Response: - authorize_request(Permission.TRADE_WRITE, request, settings) + auth_context = authorize_request(Permission.TRADE_WRITE, request, settings) service, repository = _get_service(settings) try: + if _ownership_required(settings): + _ensure_portfolio_owned(repository, portfolio_id, auth_context) service.delete_comment(portfolio_id, comment_id) except ValueError as exc: _raise_portfolio_error(exc) diff --git a/app/domain/models.py b/app/domain/models.py index 8559438..d78e91a 100644 --- a/app/domain/models.py +++ b/app/domain/models.py @@ -72,6 +72,7 @@ class Portfolio: status: str name: str symbol: str + owner_user_id: str | None = None cash_balance: float = 0.0 asset_quantity: float = 0.0 price: float = 0.0 diff --git a/app/domain/services.py b/app/domain/services.py index 4bb94e5..6eecaab 100644 --- a/app/domain/services.py +++ b/app/domain/services.py @@ -27,12 +27,14 @@ class PortfolioDomainRepository(Protocol): status: str = "enabled", active_strategy_id: str | None = None, avabile_strategies: list[str] | None = None, + owner_user_id: str | None = None, initial_cash: float = 0.0, ) -> Portfolio: ... def create_portfolio( self, portfolio_id: str, *, + owner_user_id: str, broker: str, status: str, name: str, @@ -45,7 +47,13 @@ class PortfolioDomainRepository(Protocol): avabile_strategies: list[str] | None, ) -> Portfolio: ... def update_portfolio(self, portfolio_id: str, **fields: object) -> Portfolio: ... - def list_portfolios(self, *, broker: str | None = None, status: str | None = None) -> list[Portfolio]: ... + def list_portfolios( + self, + *, + broker: str | None = None, + status: str | None = None, + owner_user_id: str | None = None, + ) -> list[Portfolio]: ... def get_strategy_set(self, id: str) -> StrategySet | None: ... def get_or_create_strategy_set(self, name: str, config: str) -> StrategySet: ... def save( @@ -103,6 +111,7 @@ def apply_buy( status=portfolio.status, name=portfolio.name, symbol=portfolio.symbol, + owner_user_id=portfolio.owner_user_id, cash_balance=float(portfolio.cash_balance) - total_cost, asset_quantity=float(portfolio.asset_quantity) + quantity, price=price, @@ -164,6 +173,7 @@ def apply_sell( status=portfolio.status, name=portfolio.name, symbol=portfolio.symbol, + owner_user_id=portfolio.owner_user_id, cash_balance=float(portfolio.cash_balance) + net_proceeds, asset_quantity=float(portfolio.asset_quantity) - quantity, price=price, diff --git a/app/repositories/portfolio_repository.py b/app/repositories/portfolio_repository.py index 4bdc36b..187fea4 100644 --- a/app/repositories/portfolio_repository.py +++ b/app/repositories/portfolio_repository.py @@ -91,6 +91,7 @@ class PortfolioRepository(Protocol): status: str = "enabled", active_strategy_id: str | None = None, avabile_strategies: list[str] | None = None, + owner_user_id: str | None = None, initial_cash: float = 0.0, ) -> Portfolio: ... @@ -98,6 +99,7 @@ class PortfolioRepository(Protocol): self, portfolio_id: str, *, + owner_user_id: str, broker: str, status: str, name: str, @@ -112,7 +114,13 @@ class PortfolioRepository(Protocol): def update_portfolio(self, portfolio_id: str, **fields: object) -> Portfolio: ... - def list_portfolios(self, *, broker: str | None = None, status: str | None = None) -> list[Portfolio]: ... + def list_portfolios( + self, + *, + broker: str | None = None, + status: str | None = None, + owner_user_id: str | None = None, + ) -> list[Portfolio]: ... def get_or_create_strategy_set(self, name: str, config: str) -> StrategySet: ... diff --git a/app/repositories/postgres.py b/app/repositories/postgres.py index 4422097..4a54005 100644 --- a/app/repositories/postgres.py +++ b/app/repositories/postgres.py @@ -70,6 +70,7 @@ _SELECT_PORTFOLIO = """ profit_buffer, active_strategy_id, avabile_strategies, + owner_user_id, created_at, updated_at, last_trade_at @@ -217,6 +218,7 @@ class PostgresPortfolioRepository: status TEXT NOT NULL DEFAULT 'enabled', name TEXT NOT NULL, symbol TEXT NOT NULL, + owner_user_id TEXT, cash_balance DOUBLE PRECISION NOT NULL, asset_quantity DOUBLE PRECISION NOT NULL, asset_price DOUBLE PRECISION NOT NULL DEFAULT 0, @@ -230,6 +232,7 @@ class PostgresPortfolioRepository: """ ) cur.execute("ALTER TABLE portfolios ADD COLUMN IF NOT EXISTS name TEXT") + cur.execute("ALTER TABLE portfolios ADD COLUMN IF NOT EXISTS owner_user_id TEXT") cur.execute("UPDATE portfolios SET name = symbol WHERE name IS NULL OR name = ''") cur.execute("ALTER TABLE portfolios ALTER COLUMN name SET NOT NULL") cur.execute( @@ -363,9 +366,10 @@ class PostgresPortfolioRepository: profit_buffer=float(row[8]), active_strategy_id=row[9] if len(row) > 9 else None, avabile_strategies=avabile_strategies, - created_at=row[11] if len(row) > 11 else None, - updated_at=row[12] if len(row) > 12 else None, - last_trade_at=row[13] if len(row) > 13 else None, + owner_user_id=row[11] if len(row) > 11 else None, + created_at=row[12] if len(row) > 12 else None, + updated_at=row[13] if len(row) > 13 else None, + last_trade_at=row[14] if len(row) > 14 else None, ) @staticmethod @@ -726,6 +730,7 @@ class PostgresPortfolioRepository: status: str = "enabled", active_strategy_id: str | None = None, avabile_strategies: list[str] | None = None, + owner_user_id: str | None = None, initial_cash: float = 0.0, ) -> Portfolio: self.ensure_schema() @@ -734,10 +739,10 @@ class PostgresPortfolioRepository: """ INSERT INTO portfolios ( portfolio_id, broker, status, name, symbol, - cash_balance, asset_quantity, asset_price, profit_buffer, + owner_user_id, cash_balance, asset_quantity, asset_price, profit_buffer, active_strategy_id, avabile_strategies ) - VALUES (%s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s) + VALUES (%s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s) ON CONFLICT (portfolio_id) DO NOTHING """, ( @@ -746,6 +751,7 @@ class PostgresPortfolioRepository: status, name or symbol, symbol, + owner_user_id, float(initial_cash), 0.0, 0.0, @@ -762,6 +768,7 @@ class PostgresPortfolioRepository: self, portfolio_id: str, *, + owner_user_id: str, broker: str, status: str, name: str, @@ -782,13 +789,13 @@ class PostgresPortfolioRepository: """ INSERT INTO portfolios ( portfolio_id, broker, status, name, symbol, - cash_balance, asset_quantity, asset_price, profit_buffer, + owner_user_id, cash_balance, asset_quantity, asset_price, profit_buffer, active_strategy_id, avabile_strategies - ) VALUES (%s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s) + ) VALUES (%s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s) RETURNING portfolio_id, broker, status, name, symbol, cash_balance, asset_quantity, asset_price, profit_buffer, - active_strategy_id, avabile_strategies, created_at, updated_at, last_trade_at + active_strategy_id, avabile_strategies, owner_user_id, created_at, updated_at, last_trade_at """, ( portfolio_id, @@ -796,6 +803,7 @@ class PostgresPortfolioRepository: status, name, symbol, + owner_user_id, float(cash_balance), float(asset_quantity), float(asset_price), @@ -842,14 +850,20 @@ class PostgresPortfolioRepository: RETURNING portfolio_id, broker, status, name, symbol, cash_balance, asset_quantity, asset_price, profit_buffer, - active_strategy_id, avabile_strategies, created_at, updated_at, last_trade_at + active_strategy_id, avabile_strategies, owner_user_id, created_at, updated_at, last_trade_at """, (*params, portfolio_id), ) row = cur.fetchone() return self._row_to_portfolio(row) - def list_portfolios(self, *, broker: str | None = None, status: str | None = None) -> list[Portfolio]: + def list_portfolios( + self, + *, + broker: str | None = None, + status: str | None = None, + owner_user_id: str | None = None, + ) -> list[Portfolio]: self.ensure_schema() query = _SELECT_PORTFOLIO clauses: list[str] = [] @@ -860,6 +874,9 @@ class PostgresPortfolioRepository: if status is not None: clauses.append("status = %s") params.append(status) + if owner_user_id is not None: + clauses.append("owner_user_id = %s") + params.append(owner_user_id) if clauses: query += "WHERE " + " AND ".join(clauses) + " " query += "ORDER BY updated_at DESC, portfolio_id ASC" @@ -1041,9 +1058,9 @@ class PostgresPortfolioRepository: last_trade_at = NULL WHERE portfolio_id = %s RETURNING - portfolio_id, broker, status, symbol, + portfolio_id, broker, status, name, symbol, cash_balance, asset_quantity, asset_price, profit_buffer, - active_strategy_id, avabile_strategies, created_at, updated_at, last_trade_at + active_strategy_id, avabile_strategies, owner_user_id, created_at, updated_at, last_trade_at """, (portfolio_id,), ) @@ -1080,9 +1097,9 @@ class PostgresPortfolioRepository: updated_at = NOW() WHERE portfolio_id = %s RETURNING - portfolio_id, broker, status, symbol, + portfolio_id, broker, status, name, symbol, cash_balance, asset_quantity, asset_price, profit_buffer, - active_strategy_id, avabile_strategies, created_at, updated_at, last_trade_at + active_strategy_id, avabile_strategies, owner_user_id, created_at, updated_at, last_trade_at """, (updated_buffer, portfolio_id), ) @@ -1149,9 +1166,9 @@ class PostgresPortfolioRepository: updated_at = NOW() WHERE portfolio_id = %s RETURNING - portfolio_id, broker, status, symbol, + portfolio_id, broker, status, name, symbol, cash_balance, asset_quantity, asset_price, profit_buffer, - active_strategy_id, avabile_strategies, created_at, updated_at, last_trade_at + active_strategy_id, avabile_strategies, owner_user_id, created_at, updated_at, last_trade_at """, (cash, buf, portfolio_id), ) diff --git a/app/schemas/portfolio.py b/app/schemas/portfolio.py index 2f77f19..12fb573 100644 --- a/app/schemas/portfolio.py +++ b/app/schemas/portfolio.py @@ -45,6 +45,7 @@ class PortfolioSummary(BaseModel): status: str name: str symbol: str + owner_user_id: str | None = None cash_balance: float = 0.0 asset_quantity: float = 0.0 price: float = 0.0 diff --git a/app/services/portfolio_service.py b/app/services/portfolio_service.py index 7308781..3f04d6e 100644 --- a/app/services/portfolio_service.py +++ b/app/services/portfolio_service.py @@ -49,6 +49,7 @@ def _to_portfolio_summary(portfolio: Portfolio): status=portfolio.status, name=portfolio.name, symbol=portfolio.symbol, + owner_user_id=portfolio.owner_user_id, cash_balance=portfolio.cash_balance, asset_quantity=portfolio.asset_quantity, price=portfolio.price, @@ -137,8 +138,14 @@ class PortfolioService: def __init__(self, repository: PortfolioRepository) -> None: self._repository = repository - def list_portfolios(self, *, broker: str | None = None, status: str | None = None) -> list[PortfolioListItem]: - portfolios = self._repository.list_portfolios(broker=broker, status=status) + def list_portfolios( + self, + *, + broker: str | None = None, + status: str | None = None, + owner_user_id: str | None = None, + ) -> list[PortfolioListItem]: + portfolios = self._repository.list_portfolios(broker=broker, status=status, owner_user_id=owner_user_id) return [ PortfolioListItem( **_to_portfolio_summary(portfolio).model_dump(), @@ -151,7 +158,12 @@ class PortfolioService: status_result = get_portfolio_status(self._repository, portfolio_id, recent_trades=recent_trades) return _to_detail_response(status_result) - def create_portfolio(self, payload: CreatePortfolioRequest) -> PortfolioDetailResponse: + def create_portfolio( + self, + payload: CreatePortfolioRequest, + *, + owner_user_id: str = "system", + ) -> PortfolioDetailResponse: active_strategy_id = payload.active_strategy_id if active_strategy_id is None: strategy_set = self._repository.get_or_create_strategy_set( @@ -162,6 +174,7 @@ class PortfolioService: avabile_strategies = _merge_strategy_ids(active_strategy_id, payload.avabile_strategies) portfolio = self._repository.create_portfolio( payload.portfolio_id, + owner_user_id=owner_user_id, broker=payload.broker, status=payload.status, name=payload.name or payload.symbol, diff --git a/docker-compose.local-test.yml b/docker-compose.local-test.yml new file mode 100644 index 0000000..a2d82a1 --- /dev/null +++ b/docker-compose.local-test.yml @@ -0,0 +1,50 @@ +services: + timescaledb: + image: timescale/timescaledb:latest-pg16 + container_name: portfolio-hub-timescaledb + environment: + POSTGRES_DB: portfolio_hub + POSTGRES_USER: portfolio_hub_user + POSTGRES_PASSWORD: change-me + ports: + - "127.0.0.1:5432:5432" + volumes: + - timescaledb_data:/var/lib/postgresql/data + healthcheck: + test: ["CMD-SHELL", "pg_isready -U portfolio_hub_user -d portfolio_hub"] + interval: 10s + timeout: 5s + retries: 5 + start_period: 10s + restart: unless-stopped + + redis-stack: + image: redis/redis-stack:latest + container_name: portfolio-hub-redis-stack + ports: + - "127.0.0.1:6379:6379" + - "127.0.0.1:8001:8001" + volumes: + - redis_stack_data:/data + healthcheck: + test: ["CMD", "redis-cli", "ping"] + interval: 10s + timeout: 5s + retries: 5 + start_period: 5s + restart: unless-stopped + + loki: + image: grafana/loki:2.9.8 + container_name: portfolio-hub-loki + command: -config.file=/etc/loki/local-config.yaml + ports: + - "127.0.0.1:3100:3100" + volumes: + - loki_data:/loki + restart: unless-stopped + +volumes: + timescaledb_data: + redis_stack_data: + loki_data: diff --git a/docs/SRS/Interface/REQ-004_portfolio-ownership-access-control.md b/docs/SRS/Interface/REQ-004_portfolio-ownership-access-control.md new file mode 100644 index 0000000..a90be43 --- /dev/null +++ b/docs/SRS/Interface/REQ-004_portfolio-ownership-access-control.md @@ -0,0 +1,74 @@ +# REQ-004: 포트폴리오 소유주 기반 접근 제어 + +- Status: Proposed +- Type: Interface +- Priority: High +- Owner: Justin + +## Intent +- 시스템은 각 포트폴리오의 소유주를 명시적으로 저장하고, 인증된 주체의 권한과 소유권을 함께 평가해 접근 제어를 수행해야 한다. +- 동일 서버에서 운영되는 frontend와 portfolio-hub가 공통 인증 기반을 공유하더라도, 포트폴리오 단위 데이터 격리가 보장되도록 한다. + +## Requirement +- 시스템은 각 `portfolio` 레코드에 소유주 식별자 `owner_user_id`를 저장해야 한다. +- 시스템은 신규 포트폴리오 생성 시 요청 본문으로 소유주를 임의 지정받지 않고, 인증 컨텍스트의 사용자 식별자를 `owner_user_id`로 저장해야 한다. +- 시스템은 `admin` 권한을 제외한 모든 사용자/토큰 요청에서 `owner_user_id`가 인증 주체와 일치하는 포트폴리오만 조회/변경/삭제 가능해야 한다. +- 시스템은 `admin` 권한 요청에서 소유권 제약 없이 포트폴리오 전체를 조회/변경/삭제할 수 있어야 한다. +- 시스템은 소유권 불일치로 거부된 요청에 대해 `403 Forbidden`을 반환해야 한다. +- 시스템은 인증 컨텍스트에 사용자 식별자가 없는 토큰이 소유권이 필요한 포트폴리오 작업을 요청하면 `403 Forbidden`을 반환해야 한다. +- 시스템은 기존 포트폴리오 데이터에 대해 마이그레이션 시 `owner_user_id`를 null 허용으로 추가한 뒤, 백필 완료 후 not null 제약을 적용할 수 있어야 한다. + +## Acceptance Criteria +1. Given 인증된 일반 사용자(또는 해당 사용자에 귀속된 토큰)가 `POST /portfolios`를 호출할 때, When 포트폴리오가 생성되면, Then 저장된 `owner_user_id`는 요청자 사용자 식별자와 동일해야 하며 요청 본문으로 다른 소유주를 지정할 수 없어야 한다. +2. Given 서로 다른 두 사용자 A와 B가 존재하고 A 소유 포트폴리오가 있을 때, When B가 `GET /portfolios/{portfolio_id}` 또는 변경 API를 호출하면, Then 시스템은 `403`을 반환하고 데이터를 노출/변경하지 않아야 한다. +3. Given 일반 사용자 A가 목록 조회를 수행할 때, When `GET /portfolios`를 호출하면, Then 응답 목록에는 A 소유 포트폴리오만 포함되어야 한다. +4. Given `admin` 권한 사용자가 존재할 때, When `GET /portfolios` 또는 특정 포트폴리오 변경 API를 호출하면, Then 시스템은 소유권 검사로 요청을 거부하지 않아야 한다. +5. Given 마이그레이션 적용 전 생성된 레거시 포트폴리오가 존재할 때, When ownership 마이그레이션을 적용하면, Then 데이터 손실 없이 `owner_user_id`가 백필되고 이후 읽기/쓰기 API가 실패 없이 동작해야 한다. +6. Given 인증 컨텍스트에 사용자 식별자가 없는 bot 토큰이 포트폴리오 생성/수정/삭제를 요청할 때, When 소유권이 필요한 API를 호출하면, Then 시스템은 `403`을 반환해야 한다. + +## Impacted Area +- Docs: `docs/SRS/index.md`, `docs/SRS/traceability-matrix.md`, `docs/technical/auth-and-token-lifecycle.md`, `docs/technical/api-contracts.md`, `docs/technical/persistence-and-migrations.md`, `docs/technical/portfolio-ownership-and-auth-design.md` +- Modules: `app/core/auth.py`, `app/api/portfolios.py`, `app/services/portfolio_service.py`, `app/repositories/portfolio_repository.py`, `app/repositories/postgres.py`, `app/domain/models.py`, `app/schemas/portfolio.py`, `migrations/versions/*` +- Tests: `tests/test_app.py`, `tests/test_auth.py`, `tests/test_cli.py`, `tests/test_domain_services.py` + +## Notes +- 본 requirement는 인증 서비스 분리 여부와 무관하게 portfolio-hub 내부에서 반드시 충족되어야 하는 리소스 소유권 계약이다. +- 소유권 기반 제어는 기존 RBAC(`read`, `trade-write`, `admin`)를 대체하지 않고 보완한다. + +## Change Log + +### CHG-20260413-01 +- Date: 2026-04-13 +- Author: GitHub Copilot + +#### Change Summary +- 포트폴리오 소유주 필드와 소유권 기반 접근 제어를 정의하는 신규 requirement를 추가했다. + +#### Intent +- frontend와 API가 동일 인증 체계를 사용할 때도 포트폴리오 단위 데이터 격리를 강제하기 위함이다. + +#### Acceptance Criteria Delta +- Added: + - 소유주 저장 규칙 + - 일반 사용자 소유권 격리 규칙 + - admin 예외 규칙 + - 레거시 백필 규칙 + - user_id 없는 주체 거부 규칙 +- Updated: + - 없음 +- Removed: + - 없음 + +#### Impacted Area Delta +- Docs: + - `docs/SRS/Interface/REQ-004_portfolio-ownership-access-control.md` + - `docs/SRS/index.md` + - `docs/SRS/traceability-matrix.md` + - `docs/technical/portfolio-ownership-and-auth-design.md` +- Modules: + - ownership 검사 경로 전반 +- Tests: + - API/권한/백필 검증 시나리오 + +#### Validation Plan +- 사용자별 소유권 격리 통합 테스트, admin 우회 테스트, 마이그레이션 백필 테스트를 통해 계약 충족 여부를 확인한다. diff --git a/docs/SRS/index.md b/docs/SRS/index.md index c45327e..27cc8be 100644 --- a/docs/SRS/index.md +++ b/docs/SRS/index.md @@ -15,7 +15,7 @@ - 최근 변경 요약은 최신 항목을 위에 두고 최대 5건만 유지한다. ## Next Requirement ID -- `REQ-004` +- `REQ-005` ## Requirement Register @@ -24,13 +24,14 @@ | REQ-001 | 포트폴리오 댓글형 메모 이력 지원 | Interface | Approved | Medium | Justin | `docs/SRS/Interface/REQ-001_portfolio-comment-history.md` | | REQ-002 | 포트폴리오 태깅 및 태그 기반 검색 지원 | Interface | Proposed | Medium | Justin | `docs/SRS/Interface/REQ-002_portfolio-tagging-and-search.md` | | REQ-003 | 포트폴리오 필수 필드 및 거래소/시장 데이터 정의 | Data | Proposed | Medium | Justin | `docs/SRS/Data/REQ-003_portfolio-required-fields.md` | +| REQ-004 | 포트폴리오 소유주 기반 접근 제어 | Interface | Proposed | High | Justin | `docs/SRS/Interface/REQ-004_portfolio-ownership-access-control.md` | ## Recent Change Log Summary +- 2026-04-13 — `REQ-004` 포트폴리오 소유주(`owner_user_id`) 및 소유권 기반 접근 제어 requirement를 추가했다. - 2026-04-13 — `REQ-003`에서 태그 필터 기반 조회의 결정성 유지 책임을 소비 서비스(stock-picker)로 이관했다. - 2026-04-13 — `REQ-003`에서 `sourceVersion`/`sourceTimestamp` 요구를 제거하고 태그 필터 기반 조회 결정성 계약으로 정리했다. - 2026-04-13 — `REQ-003`의 소비자 표현을 특정 서비스 전용에서 태그 기반 조회 소비 서비스 일반 계약으로 정리했다. - 2026-04-13 — `REQ-002`의 소비자 표현을 특정 서비스 전용에서 다운스트림 소비자 일반 계약으로 정리했다. -- 2026-04-13 — `REQ-002`에 includeTags/excludeTags 태그 필터 계약을 추가했다. ## Status Transition Evidence - `Proposed` → `Approved`: Owner가 지정되고 requirement 문서 리뷰가 완료되며 acceptance criteria가 누락 없이 검토되어야 한다. diff --git a/docs/SRS/traceability-matrix.md b/docs/SRS/traceability-matrix.md index c5f0978..30ff263 100644 --- a/docs/SRS/traceability-matrix.md +++ b/docs/SRS/traceability-matrix.md @@ -14,6 +14,7 @@ | REQ-001 | AC1, AC2, AC3, AC4, AC5, AC6, AC7 | `docs/SRS/Interface/REQ-001_portfolio-comment-history.md` | #2 | TBD | TBD | Approved | In Progress | 2026-04-10 | | REQ-002 | AC1, AC2, AC3, AC4, AC5, AC6, AC7, AC8, AC9 | `docs/SRS/Interface/REQ-002_portfolio-tagging-and-search.md` | TBD | TBD | TBD | Proposed | Planned | 2026-04-10 | | REQ-003 | AC1, AC2, AC3, AC4, AC5 | `docs/SRS/Data/REQ-003_portfolio-required-fields.md` | TBD | TBD | TBD | Proposed | Planned | 2026-04-10 | +| REQ-004 | AC1, AC2, AC3, AC4, AC5, AC6 | `docs/SRS/Interface/REQ-004_portfolio-ownership-access-control.md` | TBD | TBD | `tests/test_portfolio_ownership.py`, `tests/test_auth.py`, `tests/test_portfolio_comments.py`, local `pytest` pass (2026-04-14) | Proposed | In Progress | 2026-04-14 | ## Update Rules - REQ ID must exactly match the SRS requirement file prefix. diff --git a/docs/technical/api-contracts.md b/docs/technical/api-contracts.md index 061173e..04dfabf 100644 --- a/docs/technical/api-contracts.md +++ b/docs/technical/api-contracts.md @@ -1,11 +1,12 @@ # API Contracts -- Related REQ: `REQ-001`, `REQ-002`, `REQ-003` +- Related REQ: `REQ-001`, `REQ-002`, `REQ-003`, `REQ-004` ## 공통 원칙 - 응답은 JSON 기반으로 반환한다. - 인증이 필요한 엔드포인트는 `Authorization: Bearer `을 사용한다. - 권한 계층은 `read` < `trade-write` < `admin`이다. +- `admin`이 아닌 요청은 포트폴리오 리소스 접근 시 `owner_user_id` 소유권 검사를 추가로 통과해야 한다. - 에러 응답은 HTTP 상태 코드와 함께 원인 메시지를 제공한다. - Base URL 예시는 `http://localhost:8000`을 기준으로 한다. - JSON 예시는 대표 형태이며 일부 선택 필드를 생략할 수 있다. @@ -95,6 +96,7 @@ #### `GET /portfolios` - Auth: `read` +- Ownership: `admin`은 전체 조회, 그 외는 본인 소유(`owner_user_id == auth_context.user_id`)만 조회 - Query: - `broker: string | null` - `status_filter: string | null` @@ -131,6 +133,7 @@ curl -X GET "http://localhost:8000/portfolios?broker=upbit&status_filter=enabled #### `GET /portfolios/{portfolio_id}` - Auth: `read` +- Ownership: `admin`이 아니고 소유주 불일치 시 `403` - Query: - `recent_trades: int = 5` - 응답 `200`: `PortfolioDetailResponse` @@ -148,6 +151,7 @@ curl -X GET "http://localhost:8000/portfolios/krw-btc?recent_trades=5" \ "status": "enabled", "name": "Bitcoin", "symbol": "KRW-BTC", + "owner_user_id": "user-1", "cash_balance": 1000000.0, "asset_quantity": 0.1, "price": 100000000.0, @@ -188,6 +192,7 @@ curl -X GET "http://localhost:8000/portfolios/krw-btc?recent_trades=5" \ #### `POST /portfolios` - Auth: `trade-write` +- Ownership: 요청 body로 owner를 받지 않고 인증 컨텍스트 사용자로 `owner_user_id`를 강제 설정 - Body: `CreatePortfolioRequest` - 응답 `201`: `PortfolioDetailResponse` diff --git a/docs/technical/auth-and-token-lifecycle.md b/docs/technical/auth-and-token-lifecycle.md index 7893a28..1f96072 100644 --- a/docs/technical/auth-and-token-lifecycle.md +++ b/docs/technical/auth-and-token-lifecycle.md @@ -1,6 +1,6 @@ # Auth and Token Lifecycle -- Related REQ: N/A +- Related REQ: `REQ-004` ## 목표 정적 토큰과 DB 발급 토큰을 병행하는 현재 인증 모델의 동작 기준을 명확히 한다. @@ -13,6 +13,11 @@ | `trade-write` | 조회 + 포트폴리오 변경 API | | `admin` | 전체 API + 사용자/봇 관리 | +소유권 결합 정책: + +- `admin`이 아닌 주체는 포트폴리오 접근 시 리소스 소유주(`owner_user_id`) 일치 조건을 추가로 만족해야 한다. +- `admin`은 소유권 제약 없이 전체 포트폴리오를 접근할 수 있다. + ## 인증 방식 - 보호된 API는 `Authorization: Bearer ` 헤더를 사용한다. diff --git a/docs/technical/index.md b/docs/technical/index.md index 8a36f82..759cbac 100644 --- a/docs/technical/index.md +++ b/docs/technical/index.md @@ -21,6 +21,7 @@ | `api-contracts.md` | HTTP API 상세 계약, request/response 모델, 대표 payload 예시 | `REQ-001`, `REQ-002`, `REQ-003` | | `persistence-and-migrations.md` | PostgreSQL/Timescale 연동, 리포지토리 스위치, Alembic 운영, 스키마 baseline | `REQ-001`, `REQ-003` | | `auth-and-token-lifecycle.md` | 로그인/토큰/봇 주체 라이프사이클 및 권한 모델 | N/A | +| `portfolio-ownership-and-auth-design.md` | 포트폴리오 소유주(`owner_user_id`) 도입, 소유권 접근 제어, 검증 시나리오 설계 | `REQ-004` | | `management-cli-operations.md` | `portfoliohub` CLI 명령 책임, 종료코드, 운영 플로우 | N/A | | `deployment-and-operations.md` | Linux systemd, Caddy reverse proxy, Docker Compose 운영 절차 | N/A | @@ -31,6 +32,7 @@ | `REQ-001` | `api-contracts.md`, `persistence-and-migrations.md` | | `REQ-002` | `api-contracts.md` | | `REQ-003` | `architecture-overview.md`, `api-contracts.md`, `persistence-and-migrations.md` | +| `REQ-004` | `auth-and-token-lifecycle.md`, `api-contracts.md`, `persistence-and-migrations.md`, `portfolio-ownership-and-auth-design.md` | ## 권장 작성 순서 1. `project-structure-baseline.md` diff --git a/docs/technical/persistence-and-migrations.md b/docs/technical/persistence-and-migrations.md index 9eec26a..ffb0723 100644 --- a/docs/technical/persistence-and-migrations.md +++ b/docs/technical/persistence-and-migrations.md @@ -1,6 +1,6 @@ # Persistence and Migrations -- Related REQ: `REQ-001`, `REQ-003` +- Related REQ: `REQ-001`, `REQ-003`, `REQ-004` ## 목표 DB 기반 영속성 경로를 안정적으로 운영하고, 스키마 변경 이력과 문서 동기화를 일관되게 유지한다. @@ -28,7 +28,7 @@ DB 기반 영속성 경로를 안정적으로 운영하고, 스키마 변경 이 ## 스키마 문서 기준 - Source of truth: Alembic revisions in `migrations/versions` -- Current baseline revision: `20260410_0009` +- Current baseline revision: `20260413_0010` - 상세 테이블/컬럼/인덱스 제약은 이 문서가 Alembic revision과 동기화된 기준 문서다. ## 마이그레이션 운영 흐름 @@ -158,7 +158,7 @@ Purpose: Current baseline note: -- 아래 컬럼은 revision `20260403_0008`까지의 마이그레이션 기준이다. +- 아래 컬럼은 revision `20260413_0010`까지의 마이그레이션 기준이다. | Column | Type | Null | Default | Constraints / Notes | | --- | --- | --- | --- | --- | @@ -167,6 +167,7 @@ Current baseline note: | status | Text | NO | `enabled` | Portfolio status | | name | Text | NO | - | Asset name | | symbol | Text | NO | - | Trading symbol | +| owner_user_id | Text | NO | - | FK to `users.id` | | cash_balance | Float | NO | - | Current cash balance | | asset_quantity | Float | NO | - | Current asset quantity | | asset_price | Float | NO | 0 | Current price snapshot | @@ -180,7 +181,7 @@ Current baseline note: Constraints: - Primary key: `portfolio_id` -- Foreign key constraints: none in baseline +- Foreign key: `fk_portfolios_owner_user` on (`owner_user_id`) -> `users(id)` #### portfolio_ledger @@ -239,6 +240,7 @@ Constraints: | `idx_bot_principals_active_created` | `bot_principals` | `is_active`, `created_at` | no | active/inactive bot filtering | | `idx_portfolio_ledger_portfolio_time` | `portfolio_ledger` | `portfolio_id`, `recorded_at DESC` | no | recent ledger query | | `idx_portfolios_broker_status_updated` | `portfolios` | `broker`, `status`, `updated_at DESC` | no | broker/status recency filtering | +| `idx_portfolios_owner_updated` | `portfolios` | `owner_user_id`, `updated_at DESC` | no | owner-scoped recency filtering | | `idx_portfolio_comments_portfolio_created` | `portfolio_comments` | `portfolio_id`, `created_at DESC` | no | reverse chronological comments | Conditional migration behavior: @@ -248,9 +250,9 @@ Conditional migration behavior: ## Integrity Notes -현재 baseline은 DB-level FK constraint를 의도적으로 두지 않는다. +현재 baseline은 필수 ownership 제약(`portfolios.owner_user_id` -> `users.id`)만 DB-level FK를 사용한다. -- logical relation은 application/repository logic이 관리한다. +- ownership 외 logical relation은 application/repository logic이 관리한다. - 향후 FK constraint를 추가하면 migration ID와 함께 본 문서를 갱신한다. - application layer cascade: - 포트폴리오 삭제 시 연관 `portfolio_comments`를 hard delete 한다. @@ -273,6 +275,7 @@ Conditional migration behavior: | 20260403_0007 | migrations/versions/20260403_0007_bot_principal_permission_only.py | Remove `owner_user_id` and enforce bot-local permission model | | 20260403_0008 | migrations/versions/20260403_0008_portfolio_name_column.py | Add non-null `portfolios.name` and backfill from `symbol` | | 20260410_0009 | migrations/versions/20260410_0009_portfolio_comments_table.py | Add `portfolio_comments` table for per-portfolio memo history | +| 20260413_0010 | migrations/versions/20260413_0010_portfolio_owner_user_id.py | Add `portfolios.owner_user_id`, backfill owner, create owner index, and add owner FK | ## Open Implementation Points - `portfolio_ledger`의 Timescale hypertable 강제 여부와 보존 정책(retention)을 확정한다. diff --git a/docs/technical/portfolio-ownership-and-auth-design.md b/docs/technical/portfolio-ownership-and-auth-design.md new file mode 100644 index 0000000..9add3f2 --- /dev/null +++ b/docs/technical/portfolio-ownership-and-auth-design.md @@ -0,0 +1,137 @@ +# Portfolio Ownership and Auth Design + +- Related REQ: `REQ-004` + +## 목표 +- 포트폴리오 리소스에 소유주(`owner_user_id`)를 도입해 사용자 단위 데이터 격리를 강제한다. +- 기존 RBAC(`read`, `trade-write`, `admin`)와 소유권 검사 규칙을 결합해 접근 제어를 표준화한다. +- frontend와 portfolio-hub가 같은 서버에서 개별 컨테이너로 운영되는 구조에서도 단일 인증 체계로 일관된 권한 모델을 유지한다. + +## 범위 +- 포함: + - DB 스키마 변경 및 레거시 데이터 백필 + - API/서비스/리포지토리 ownership 정책 + - 통합 테스트 및 회귀 테스트 시나리오 +- 제외: + - 외부 IdP/OIDC 연동 + - 인증 서비스를 별도 프로세스로 분리하는 배포 작업 + +## DB 마이그레이션 설계 + +## Revision Plan +1. `owner_user_id` nullable 추가 +2. 레거시 row 백필 +3. 인덱스 추가 +4. not null 제약 적용 +5. 필요 시 FK 제약(`users.id`) 적용 + +## 권장 DDL 단계 +1. 컬럼 추가 + +```sql +ALTER TABLE portfolios +ADD COLUMN owner_user_id TEXT; +``` + +2. 백필 기준 사용자 결정 +- 환경변수 기반 bootstrap admin이 있으면 해당 사용자 id를 우선 사용한다. +- 없으면 `users.created_at ASC` 첫 사용자 id를 사용한다. +- 사용자 자체가 없으면 migration을 실패시키고 운영자가 사용자 생성 후 재시도한다. + +3. 레거시 데이터 백필 + +```sql +UPDATE portfolios +SET owner_user_id = :fallback_user_id +WHERE owner_user_id IS NULL; +``` + +4. 제약 및 인덱스 적용 + +```sql +ALTER TABLE portfolios +ALTER COLUMN owner_user_id SET NOT NULL; + +CREATE INDEX idx_portfolios_owner_updated +ON portfolios (owner_user_id, updated_at DESC); +``` + +5. 선택적 FK 제약 + +```sql +ALTER TABLE portfolios +ADD CONSTRAINT fk_portfolios_owner_user +FOREIGN KEY (owner_user_id) REFERENCES users(id); +``` + +## 롤백 전략 +- downgrade에서는 FK, 인덱스, not null 제약 해제 후 컬럼 제거 순서로 진행한다. +- 운영 데이터 보존이 필요하면 컬럼 삭제 대신 nullable 유지 downgrade를 별도 운영 방침으로 둔다. + +## API 권한 규칙 설계 + +## 공통 정책 +- 1단계: RBAC 검사 통과 필요 +- 2단계: admin이 아니면 ownership 검사 수행 +- ownership 검사 기준: `portfolio.owner_user_id == auth_context.user_id` +- `auth_context.user_id`가 없는 요청은 ownership 필요한 엔드포인트에서 `403` + +## Endpoint Policy Matrix + +| Endpoint | RBAC 최소 권한 | Ownership 적용 | 비고 | +| --- | --- | --- | --- | +| `GET /portfolios` | read | 적용 | admin은 전체, 그 외 owner만 필터 | +| `GET /portfolios/{portfolio_id}` | read | 적용 | owner 불일치 시 403 | +| `POST /portfolios` | trade-write | 생성 시 owner 고정 | owner_user_id는 인증 주체로 강제 | +| `PATCH /portfolios/{portfolio_id}` | trade-write | 적용 | owner 불일치 시 403 | +| `POST /portfolios/{portfolio_id}/buy` | trade-write | 적용 | owner 불일치 시 403 | +| `POST /portfolios/{portfolio_id}/sell` | trade-write | 적용 | owner 불일치 시 403 | +| `POST /portfolios/{portfolio_id}/invest` | trade-write | 적용 | owner 불일치 시 403 | +| `POST /portfolios/{portfolio_id}/transfer` | trade-write | 적용 | owner 불일치 시 403 | +| `POST /portfolios/{portfolio_id}/reset` | trade-write | 적용 | owner 불일치 시 403 | +| `DELETE /portfolios/{portfolio_id}` | admin | admin only | 기존 정책 유지 | +| `POST/GET/PATCH/DELETE /portfolios/{id}/comments*` | read or trade-write | 적용 | 상위 포트폴리오 ownership을 재사용 | + +## 서비스/리포지토리 반영 포인트 +- Domain model: + - `Portfolio`에 `owner_user_id` 추가 +- Schema: + - `PortfolioSummary`/`CreatePortfolioRequest` 응답/입력 계약에 owner 노출 정책 정의 + - 외부 입력으로 owner_user_id 수동 지정은 금지 +- Repository: + - create 시 owner_user_id 필수 인자 + - list/get 시 요청자 컨텍스트 기반 owner 필터 함수 제공 +- Service/API: + - request principal을 service 계층으로 전달 + - 공통 ownership 가드 함수 도입 + +## frontend 연계 방침 +- frontend는 credential 저장/검증 자체를 수행하지 않고 portfolio-hub 인증 API를 사용한다. +- 브라우저 경로는 HttpOnly + Secure cookie 기반 세션을 우선 고려한다. +- 동일 서버 내 별도 컨테이너 운영 시 reverse proxy에서 CORS/Cookie domain 정책을 일관되게 관리한다. + +## 테스트 케이스 목록 + +## Migration +1. 기존 portfolios row가 존재할 때 owner_user_id 백필 후 null row가 0건인지 검증 +2. users가 없는 상태에서 migration 실행 시 실패 동작 검증 +3. downgrade 시 인덱스/제약 해제가 순서대로 수행되는지 검증 + +## API Authorization +1. 사용자 A 소유 portfolio를 사용자 B가 조회하면 403 +2. 사용자 A 소유 portfolio를 사용자 B가 수정/매매/리셋하면 403 +3. admin이 타 사용자 portfolio 조회/수정 가능 +4. 일반 사용자의 `GET /portfolios`는 본인 소유만 반환 +5. user_id 없는 bot principal 요청이 ownership 필요 API 호출 시 403 + +## API Creation +1. `POST /portfolios`에서 owner_user_id가 인증 사용자로 저장되는지 검증 +2. 요청 body에 owner_user_id를 넣어도 무시되거나 validation 실패하는지 검증 + +## Regression +1. 기존 REQ-001 댓글 API가 ownership 정책 적용 후에도 owner 경계 내에서 정상 동작 +2. 기존 RBAC 오류(`401`, `403`) 계약이 ownership 도입 후에도 유지 +3. admin delete 동작이 ownership 변경과 충돌 없이 유지 + +## 추적 항목 +- 본 설계 문서 구현 시 `docs/SRS/traceability-matrix.md`의 `REQ-004` 행에 Issue/PR/Test evidence를 갱신한다. diff --git a/migrations/versions/20260413_0010_portfolio_owner_user_id.py b/migrations/versions/20260413_0010_portfolio_owner_user_id.py new file mode 100644 index 0000000..47f2550 --- /dev/null +++ b/migrations/versions/20260413_0010_portfolio_owner_user_id.py @@ -0,0 +1,97 @@ +"""add portfolio owner user id + +Revision ID: 20260413_0010 +Revises: 20260410_0009 +Create Date: 2026-04-13 01:00:00 +""" + +from __future__ import annotations + +import sqlalchemy as sa +from alembic import op +from sqlalchemy import inspect + + +revision = "20260413_0010" +down_revision = "20260410_0009" +branch_labels = None +depends_on = None + + +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 _index_exists(inspector, table_name: str, index_name: str) -> bool: + return any(index["name"] == index_name for index in inspector.get_indexes(table_name)) + + +def _fk_exists(inspector, table_name: str, fk_name: str) -> bool: + return any(fk.get("name") == fk_name for fk in inspector.get_foreign_keys(table_name)) + + +def upgrade() -> None: + bind = op.get_bind() + inspector = inspect(bind) + + if not _column_exists(inspector, "portfolios", "owner_user_id"): + op.add_column("portfolios", sa.Column("owner_user_id", sa.Text(), nullable=True)) + + bind.execute( + sa.text( + """ + WITH fallback_owner AS ( + SELECT id + FROM users + ORDER BY created_at ASC, id ASC + LIMIT 1 + ) + UPDATE portfolios p + SET owner_user_id = f.id + FROM fallback_owner f + WHERE p.owner_user_id IS NULL + """ + ) + ) + + unresolved = bind.execute(sa.text("SELECT COUNT(*) FROM portfolios WHERE owner_user_id IS NULL")).scalar() + if unresolved and int(unresolved) > 0: + raise RuntimeError("owner_user_id backfill failed: users 테이블에 fallback 소유주가 필요합니다") + + op.alter_column("portfolios", "owner_user_id", existing_type=sa.Text(), nullable=False) + + inspector = inspect(bind) + if not _index_exists(inspector, "portfolios", "idx_portfolios_owner_updated"): + op.create_index( + "idx_portfolios_owner_updated", + "portfolios", + ["owner_user_id", sa.text("updated_at DESC")], + unique=False, + ) + + inspector = inspect(bind) + if not _fk_exists(inspector, "portfolios", "fk_portfolios_owner_user"): + op.create_foreign_key( + "fk_portfolios_owner_user", + "portfolios", + "users", + ["owner_user_id"], + ["id"], + ondelete="RESTRICT", + ) + + +def downgrade() -> None: + bind = op.get_bind() + inspector = inspect(bind) + + if _fk_exists(inspector, "portfolios", "fk_portfolios_owner_user"): + op.drop_constraint("fk_portfolios_owner_user", "portfolios", type_="foreignkey") + + inspector = inspect(bind) + if _index_exists(inspector, "portfolios", "idx_portfolios_owner_updated"): + op.drop_index("idx_portfolios_owner_updated", table_name="portfolios") + + inspector = inspect(bind) + if _column_exists(inspector, "portfolios", "owner_user_id"): + op.drop_column("portfolios", "owner_user_id") diff --git a/tests/test_cli.py b/tests/test_cli.py index 0f5cf38..568f40c 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -117,6 +117,7 @@ class FakeRepo: status: str = "enabled", active_strategy_id: str | None = None, avabile_strategies: list[str] | None = None, + owner_user_id: str | None = None, initial_cash: float = 0.0, ) -> Portfolio: if self._portfolio is None: @@ -126,6 +127,7 @@ class FakeRepo: status=status, name=name or symbol, symbol=symbol, + owner_user_id=owner_user_id, cash_balance=initial_cash, active_strategy_id=active_strategy_id, avabile_strategies=avabile_strategies, @@ -136,6 +138,7 @@ class FakeRepo: self, portfolio_id: str, *, + owner_user_id: str, broker: str, status: str, name: str, @@ -153,6 +156,7 @@ class FakeRepo: status=status, name=name, symbol=symbol, + owner_user_id=owner_user_id, cash_balance=cash_balance, asset_quantity=asset_quantity, price=asset_price, @@ -169,13 +173,21 @@ class FakeRepo: setattr(portfolio, key, value) return portfolio - def list_portfolios(self, *, broker: str | None = None, status: str | None = None) -> list[Portfolio]: + def list_portfolios( + self, + *, + broker: str | None = None, + status: str | None = None, + owner_user_id: str | None = None, + ) -> list[Portfolio]: if self._portfolio is None: return [] if broker is not None and self._portfolio.broker != broker: return [] if status is not None and self._portfolio.status != status: return [] + if owner_user_id is not None and self._portfolio.owner_user_id != owner_user_id: + return [] return [self._portfolio] def get_or_create_strategy_set(self, name: str, config: str) -> StrategySet: diff --git a/tests/test_domain_services.py b/tests/test_domain_services.py index ee9e31f..480869f 100644 --- a/tests/test_domain_services.py +++ b/tests/test_domain_services.py @@ -31,6 +31,7 @@ class FakeRepo: status: str = "enabled", active_strategy_id: str | None = None, avabile_strategies: list[str] | None = None, + owner_user_id: str | None = None, initial_cash: float = 0.0, ) -> Portfolio: self._portfolio = Portfolio( @@ -39,6 +40,7 @@ class FakeRepo: status=status, name=name or symbol, symbol=symbol, + owner_user_id=owner_user_id, cash_balance=initial_cash, active_strategy_id=active_strategy_id, avabile_strategies=avabile_strategies, @@ -55,9 +57,17 @@ class FakeRepo: setattr(portfolio, key, value) return portfolio - def list_portfolios(self, *, broker: str | None = None, status: str | None = None) -> list[Portfolio]: + def list_portfolios( + self, + *, + broker: str | None = None, + status: str | None = None, + owner_user_id: str | None = None, + ) -> list[Portfolio]: if self._portfolio is None: return [] + if owner_user_id is not None and self._portfolio.owner_user_id != owner_user_id: + return [] return [self._portfolio] def get_strategy_set(self, id: str) -> StrategySet | None: diff --git a/tests/test_portfolio_comments.py b/tests/test_portfolio_comments.py index d9d30d2..8e56989 100644 --- a/tests/test_portfolio_comments.py +++ b/tests/test_portfolio_comments.py @@ -50,7 +50,9 @@ class FakeRepo: setattr(portfolio, key, value) return portfolio - def list_portfolios(self, *, broker=None, status=None): + def list_portfolios(self, *, broker=None, status=None, owner_user_id=None): + if owner_user_id is not None and self._portfolio and self._portfolio.owner_user_id != owner_user_id: + return [] return [self._portfolio] if self._portfolio else [] def get_or_create_strategy_set(self, name: str, config: str) -> StrategySet: @@ -181,6 +183,7 @@ def _portfolio() -> Portfolio: status="enabled", name="Samsung Electronics", symbol="005930", + owner_user_id="legacy-owner", cash_balance=1_000_000.0, asset_quantity=1.0, price=70_000.0, diff --git a/tests/test_portfolio_ownership.py b/tests/test_portfolio_ownership.py new file mode 100644 index 0000000..a66fe90 --- /dev/null +++ b/tests/test_portfolio_ownership.py @@ -0,0 +1,131 @@ +from __future__ import annotations + +from datetime import datetime, timezone + +from fastapi.testclient import TestClient + +import app.api.portfolios as portfolios_api +import app.core.auth as auth_core +from app.core.config import get_settings +from app.core.security import hash_token +from app.domain.models import LedgerEntry, Portfolio, StrategySet, TokenPrincipal +from app.main import app + + +class FakeRepo: + def __init__(self, *, portfolio: Portfolio, principals: dict[str, TokenPrincipal]) -> None: + self._portfolio = portfolio + self._principals = principals + self._strategy = StrategySet(id="set-1", name="target_value", config="{}", nick="target_value:demo") + + def close(self) -> None: + return None + + def list_users(self): + return [] + + def get_token_principal(self, token_hash: str): + return self._principals.get(token_hash) + + def get(self, portfolio_id: str) -> Portfolio: + if self._portfolio.portfolio_id != portfolio_id: + raise ValueError("포트폴리오를 찾을 수 없습니다") + return self._portfolio + + def list_recent_trades(self, portfolio_id: str, limit: int = 5) -> list[LedgerEntry]: + return [] + + def get_strategy_set(self, id: str): + if id == self._strategy.id: + return self._strategy + return None + + +def _portfolio(owner_user_id: str) -> Portfolio: + return Portfolio( + portfolio_id="p1", + broker="kis", + status="enabled", + name="Samsung Electronics", + symbol="005930", + owner_user_id=owner_user_id, + cash_balance=1_000_000.0, + asset_quantity=1.0, + price=70_000.0, + profit_buffer=0.0, + active_strategy_id="set-1", + avabile_strategies=["set-1"], + created_at=datetime.now(timezone.utc), + updated_at=datetime.now(timezone.utc), + ) + + +def _configure_client(monkeypatch, repo: FakeRepo) -> TestClient: + monkeypatch.setenv("PORTFOLIOHUB_DB_ENABLED", "true") + monkeypatch.delenv("PORTFOLIOHUB_READ_TOKENS", raising=False) + monkeypatch.delenv("PORTFOLIOHUB_TRADE_WRITE_TOKENS", raising=False) + monkeypatch.delenv("PORTFOLIOHUB_ADMIN_TOKENS", raising=False) + get_settings.cache_clear() + app.dependency_overrides.clear() + monkeypatch.setattr(portfolios_api, "get_portfolio_repository_from_settings", lambda settings: repo) + monkeypatch.setattr(auth_core, "get_portfolio_repository_from_settings", lambda settings: repo) + return TestClient(app) + + +def test_owner_can_read_owned_portfolio_on_db_auth(monkeypatch) -> None: + token = "owner-db-token" + principal = TokenPrincipal( + token_id="t1", + user_id="owner-1", + permission="read", + token_type="client", + ) + repo = FakeRepo(portfolio=_portfolio("owner-1"), principals={hash_token(token): principal}) + client = _configure_client(monkeypatch, repo) + + response = client.get("/portfolios/p1", headers={"Authorization": f"Bearer {token}"}) + + app.dependency_overrides.clear() + assert response.status_code == 200 + assert response.json()["portfolio"]["owner_user_id"] == "owner-1" + + +def test_non_owner_gets_403_on_db_auth(monkeypatch) -> None: + token = "other-db-token" + principal = TokenPrincipal( + token_id="t2", + user_id="other-user", + permission="read", + token_type="client", + ) + repo = FakeRepo(portfolio=_portfolio("owner-1"), principals={hash_token(token): principal}) + client = _configure_client(monkeypatch, repo) + + response = client.get("/portfolios/p1", headers={"Authorization": f"Bearer {token}"}) + + app.dependency_overrides.clear() + assert response.status_code == 403 + assert response.json()["detail"] == "Portfolio ownership mismatch" + + +def test_db_enabled_env_token_without_user_context_is_rejected(monkeypatch) -> None: + repo = FakeRepo(portfolio=_portfolio("owner-1"), principals={}) + monkeypatch.setenv("PORTFOLIOHUB_DB_ENABLED", "true") + monkeypatch.setenv("PORTFOLIOHUB_TRADE_WRITE_TOKENS", "trade-env-token") + monkeypatch.delenv("PORTFOLIOHUB_READ_TOKENS", raising=False) + monkeypatch.delenv("PORTFOLIOHUB_ADMIN_TOKENS", raising=False) + get_settings.cache_clear() + app.dependency_overrides.clear() + monkeypatch.setattr(portfolios_api, "get_portfolio_repository_from_settings", lambda settings: repo) + monkeypatch.setattr(auth_core, "get_portfolio_repository_from_settings", lambda settings: repo) + client = TestClient(app) + + response = client.post( + "/portfolios", + headers={"Authorization": "Bearer trade-env-token"}, + json={"portfolio_id": "new-1", "symbol": "005930"}, + ) + + app.dependency_overrides.clear() + assert response.status_code == 403 + assert response.json()["detail"] == "Ownership context is required"