-
Notifications
You must be signed in to change notification settings - Fork 12
fix(auth): replace user_id query param with JWT-based identity. Relat… #372
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
80908ff
41a243b
a96be5b
a80412a
44e9fb1
b709034
6c80402
8a84b02
9146582
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| """Allow global paper feedback without an active track. | ||
|
|
||
| Revision ID: 0027_global_paper_feedback | ||
| Revises: 0026_embedding_endpoint_settings | ||
| Create Date: 2026-03-13 | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from alembic import op | ||
| import sqlalchemy as sa | ||
|
|
||
|
|
||
| revision = "0027_global_paper_feedback" | ||
| down_revision = "0026_embedding_endpoint_settings" | ||
| branch_labels = None | ||
| depends_on = None | ||
|
|
||
|
|
||
| def upgrade() -> None: | ||
| conn = op.get_bind() | ||
| inspector = sa.inspect(conn) | ||
| if "paper_feedback" not in inspector.get_table_names(): | ||
| return | ||
|
|
||
| columns = {column["name"]: column for column in inspector.get_columns("paper_feedback")} | ||
| track_column = columns.get("track_id") | ||
| if not track_column or bool(track_column.get("nullable")): | ||
| return | ||
|
|
||
| with op.batch_alter_table("paper_feedback") as batch_op: | ||
| batch_op.alter_column("track_id", existing_type=sa.Integer(), nullable=True) | ||
|
|
||
|
|
||
| def downgrade() -> None: | ||
| conn = op.get_bind() | ||
| inspector = sa.inspect(conn) | ||
| if "paper_feedback" not in inspector.get_table_names(): | ||
| return | ||
|
|
||
| columns = {column["name"]: column for column in inspector.get_columns("paper_feedback")} | ||
| track_column = columns.get("track_id") | ||
| if not track_column or not bool(track_column.get("nullable")): | ||
| return | ||
|
|
||
| null_count = conn.execute(sa.text("SELECT COUNT(*) FROM paper_feedback WHERE track_id IS NULL")).scalar() | ||
| if null_count: | ||
| raise RuntimeError("Cannot downgrade: global paper_feedback rows with NULL track_id exist") | ||
|
|
||
| with op.batch_alter_table("paper_feedback") as batch_op: | ||
| batch_op.alter_column("track_id", existing_type=sa.Integer(), nullable=False) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,19 +6,19 @@ | |
| from pathlib import Path | ||
| from typing import Optional | ||
|
|
||
| from fastapi import APIRouter, Request | ||
| from fastapi import APIRouter, Depends, Request | ||
| from pydantic import BaseModel | ||
|
|
||
| from paperbot.application.collaboration.message_schema import new_run_id, new_trace_id | ||
| from paperbot.core.abstractions import AgentRunContext | ||
| from paperbot.api.auth.dependencies import get_required_user_id | ||
|
|
||
| from ..streaming import StreamEvent, sse_response | ||
|
|
||
| router = APIRouter() | ||
|
|
||
|
|
||
| class GenCodeRequest(BaseModel): | ||
| user_id: str = "default" | ||
| title: str | ||
| abstract: str | ||
| method_section: Optional[str] = None | ||
|
|
@@ -28,7 +28,7 @@ class GenCodeRequest(BaseModel): | |
|
|
||
|
|
||
| async def gen_code_stream( | ||
| request: GenCodeRequest, *, event_log=None, run_id: str = "", trace_id: str = "" | ||
| request: GenCodeRequest, *, user_id: str, event_log=None, run_id: str = "", trace_id: str = "" | ||
| ): | ||
| """Stream code generation progress""" | ||
|
Comment on lines
30
to
33
|
||
| try: | ||
|
|
@@ -94,7 +94,7 @@ async def gen_code_stream( | |
| result = await agent.reproduce_from_paper( | ||
| paper_context, | ||
| output_dir=output_dir, | ||
| user_id=request.user_id, | ||
| user_id=user_id, | ||
| event_log=event_log, | ||
| run_id=run_id, | ||
| trace_id=trace_id, | ||
|
|
@@ -181,7 +181,11 @@ async def gen_code_stream( | |
|
|
||
|
|
||
| @router.post("/gen-code") | ||
| async def generate_code(request: GenCodeRequest, http_request: Request): | ||
| async def generate_code( | ||
| request: GenCodeRequest, | ||
| http_request: Request, | ||
| user_id: str = Depends(get_required_user_id), | ||
| ): | ||
| """ | ||
| Generate code from paper and stream progress. | ||
|
|
||
|
|
@@ -190,8 +194,9 @@ async def generate_code(request: GenCodeRequest, http_request: Request): | |
| event_log = getattr(http_request.app.state, "event_log", None) | ||
| run_id = new_run_id() | ||
| trace_id = new_trace_id() | ||
|
|
||
| return sse_response( | ||
| gen_code_stream(request, event_log=event_log, run_id=run_id, trace_id=trace_id), | ||
| gen_code_stream(request, user_id=user_id, event_log=event_log, run_id=run_id, trace_id=trace_id), | ||
| workflow="gen_code", | ||
| run_id=run_id, | ||
| trace_id=trace_id, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,7 +13,7 @@ | |
|
|
||
| from typing import Any, Dict, List, Optional | ||
|
|
||
| from fastapi import APIRouter, HTTPException, Query, Request | ||
| from fastapi import APIRouter, Depends, HTTPException, Query, Request | ||
| from pydantic import BaseModel, Field | ||
|
|
||
| from paperbot.api.streaming import StreamEvent, sse_response | ||
|
|
@@ -23,6 +23,7 @@ | |
| HarvestPipeline, | ||
| HarvestProgress, | ||
| ) | ||
| from paperbot.api.auth.dependencies import get_required_user_id | ||
| from paperbot.infrastructure.stores.paper_store import PaperStore, paper_to_dict | ||
| from paperbot.utils.logging_config import LogFiles, Logger, clear_trace_id, set_trace_id | ||
|
|
||
|
|
@@ -324,12 +325,9 @@ class LibraryResponse(BaseModel): | |
| offset: int | ||
|
|
||
|
|
||
| # TODO(auth): user_id is accepted from client without authentication. | ||
| # This is intentional for the MVP single-user setup. For multi-user production, | ||
| # user_id should come from an authenticated session or JWT token. | ||
| @router.get("/papers/library", response_model=LibraryResponse) | ||
| def get_user_library( | ||
| user_id: str = Query("default", description="User ID"), | ||
| user_id: str = Depends(get_required_user_id), | ||
| track_id: Optional[int] = Query(None, description="Filter by track"), | ||
| actions: Optional[str] = Query(None, description="Filter by actions (comma-separated)"), | ||
|
Comment on lines
328
to
332
|
||
| sort_by: str = Query("saved_at", description="Sort field"), | ||
|
|
@@ -392,13 +390,15 @@ def get_paper(paper_id: int): | |
| class SavePaperRequest(BaseModel): | ||
| """Request to save paper to library.""" | ||
|
|
||
| # TODO(auth): user_id from client without auth - intentional for MVP single-user setup | ||
| user_id: str = Field("default", description="User ID") | ||
| track_id: Optional[int] = Field(None, description="Associated track ID") | ||
|
|
||
|
|
||
| @router.post("/papers/{paper_id}/save") | ||
| def save_paper_to_library(paper_id: int, request: SavePaperRequest): | ||
| def save_paper_to_library( | ||
| paper_id: int, | ||
| request: SavePaperRequest, | ||
| user_id: str = Depends(get_required_user_id), | ||
| ): | ||
|
Comment on lines
396
to
+401
|
||
| """ | ||
| Save a paper to user's library. | ||
|
|
||
|
|
@@ -413,7 +413,7 @@ def save_paper_to_library(paper_id: int, request: SavePaperRequest): | |
| # Use research store to record feedback | ||
| research_store = _get_research_store() | ||
| feedback = research_store.record_paper_feedback( | ||
| user_id=request.user_id, | ||
| user_id=user_id, | ||
| paper_id=str(paper_id), | ||
| action="save", | ||
| track_id=request.track_id, | ||
|
|
@@ -422,13 +422,10 @@ def save_paper_to_library(paper_id: int, request: SavePaperRequest): | |
| return {"success": True, "feedback": feedback} | ||
|
|
||
|
|
||
| # TODO(auth): user_id accepted from query string without authentication. | ||
| # Intentional for MVP single-user setup. For multi-user production, user_id | ||
| # should come from authenticated session/JWT, not query parameters. | ||
| @router.delete("/papers/{paper_id}/save") | ||
| def remove_paper_from_library( | ||
| paper_id: int, | ||
| user_id: str = Query("default", description="User ID"), | ||
| user_id: str = Depends(get_required_user_id), | ||
| ): | ||
|
Comment on lines
425
to
429
|
||
| """Remove a paper from user's library.""" | ||
| store = _get_paper_store() | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add route tests for the new auth dependency path.
This changed endpoint behavior (dependency-required user identity and payload override), but no corresponding chat-route test updates are included in the provided changes.
Please add tests for: unauthenticated request rejection, authenticated request success, and precedence of dependency-injected
user_idover any body-provided value.As per coding guidelines
{src,web/src}/**/*.{py,ts,tsx}: “If behavior changes, add or update tests”.🤖 Prompt for AI Agents