Skip to content

Commit 866b394

Browse files
authored
Add API authorization rules and email comparison utility (#162)
# Summary This branch fixes the QC checks failing server side due to an auth issue. In addition, refine the prompt for the QC check to make it more reliable and provide more context about the fields of the draft note. This commit introduces a new file for API authorization rules, detailing the authentication provider pairing and user-scoped versus playlist-scoped API authorization. It also adds a utility function for case-insensitive email comparison to enhance authorization checks. ## Changes - New file: `.cursor/rules/api-auth-and-qc.mdc` with detailed API authorization guidelines. - New utility: `emails_match` function in `backend/src/dna/auth/email.py` for comparing email addresses. - Updated backend authorization checks to use `emails_match` for user email comparisons in several endpoints. - Added tests for the new email comparison function and updated tests for QC checks to reflect the new authorization logic. ## Testing - [X] Changes tested locally - [X] All relevant automated tests run successfully - [X] Verified no existing workflows are broken ## How I Tested Deployed branch to server --------- Signed-off-by: James Spadafora <spadjv@gmail.com>
1 parent 6a13373 commit 866b394

10 files changed

Lines changed: 285 additions & 36 deletions

File tree

.cursor/rules/api-auth-and-qc.mdc

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
---
2+
description: Auth provider pairing and user-scoped vs playlist-scoped API authorization.
3+
globs: backend/src/**/*.py,frontend/packages/**/*
4+
alwaysApply: false
5+
---
6+
7+
# API auth and Note QC
8+
9+
## Match frontend and backend auth in every environment
10+
11+
Local dev uses noop auth; GCP production uses Google OAuth. Both sides must agree or you will see 401/403 on protected routes.
12+
13+
| Environment | Backend `AUTH_PROVIDER` | Frontend `VITE_AUTH_PROVIDER` |
14+
|---------------|-------------------------|-------------------------------|
15+
| Local (default) | `none` | `none` |
16+
| GCP / production | `google` | `google` (baked at Docker build; see `frontend/Dockerfile`) |
17+
18+
Also set `GOOGLE_CLIENT_ID` (backend) and `VITE_GOOGLE_CLIENT_ID` (frontend build) to the **same** OAuth client. The GCP deploy workflow passes the secret into the frontend image build.
19+
20+
## Two authorization patterns
21+
22+
**User-scoped** (`/users/{user_email}/…`): settings, QC check definitions. The path `user_email` must match the authenticated user. Use `emails_match()` from `dna.auth.email` (case-insensitive); do not use raw `!=`.
23+
24+
**Playlist / draft-scoped** (`/playlists/…`, `publish-notes`, `run-qc-checks`): any authenticated user may act on drafts in the playlist. Do **not** require `body.user_email == current_user` on `run-qc-checks`.
25+
26+
## Note QC: `user_email` means draft owner
27+
28+
- **CRUD** `/users/{user_email}/qc-checks`: `user_email` is the signed-in user configuring their checks.
29+
- **Run** `POST …/run-qc-checks` with `{ "user_email": "…" }`: identifies which **draft note** to check (owner), same as draft-note paths. The publish dialog runs QC for every selected draft; the caller’s token email may differ.
30+
31+
Frontend: `useNoteQCChecks` correctly passes `d.user_email` from each `DraftNote`. Settings / `NoteQCTab` use the auth user’s email.
32+
33+
## Debugging 403 Forbidden on QC
34+
35+
1. **Settings tab** (`GET /users/…/qc-checks`): path email ≠ token email (check case; use `emails_match` on the backend).
36+
2. **Publish dialog** (`run-qc-checks`): caller was wrongly required to own the draft — fixed by playlist-scoped auth above.
37+
3. **401** (not 403): missing Bearer token, wrong `AUTH_PROVIDER`, or invalid Google token / client ID mismatch.
38+
39+
When adding endpoints, copy the pattern from `publish_notes` (auth only) or user settings (auth + `emails_match`), never invent a third rule without documenting it here.

backend/src/dna/auth/email.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
"""Email comparison helpers for authorization checks."""
2+
3+
4+
def emails_match(a: str, b: str) -> bool:
5+
"""Return True when two emails refer to the same mailbox (case-insensitive)."""
6+
return a.strip().lower() == b.strip().lower()

backend/src/dna/llm_providers/llm_provider_base.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,7 @@ async def generate_structured_with_tools(
251251
max_iterations: int = 5,
252252
temperature: float = 0.2,
253253
max_tool_result_chars: int = DEFAULT_MAX_TOOL_RESULT_CHARS,
254+
extraction_user_message: str | None = None,
254255
) -> T:
255256
"""Tool-use phase then instructor-validated structured extraction."""
256257
messages: list[dict[str, Any]] = [
@@ -311,7 +312,8 @@ async def generate_structured_with_tools(
311312
extraction_messages.append(
312313
{
313314
"role": "user",
314-
"content": (
315+
"content": extraction_user_message
316+
or (
315317
"Provide your final quality-check result for this draft and check. "
316318
"Fill every required field in the structured response schema."
317319
),

backend/src/dna/models/qc_check.py

Lines changed: 55 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -46,27 +46,67 @@ class NoteQCCheck(BaseModel):
4646
class NoteQCAttributeSuggestion(BaseModel):
4747
"""Suggested updates to draft note metadata."""
4848

49-
to: Optional[str] = None
50-
cc: Optional[str] = None
51-
subject: Optional[str] = None
49+
to: Optional[str] = Field(
50+
default=None,
51+
description=(
52+
"Primary recipients when the note is directed at specific users. "
53+
'JSON string of a JSON array: [{"type":"User","id":123,"name":"Name"}]. '
54+
"Resolve users via search_entities before suggesting."
55+
),
56+
)
57+
cc: Optional[str] = Field(
58+
default=None,
59+
description=(
60+
"Users who should see the note but are not primary To recipients "
61+
"(mentioned in passing, stakeholders). Same JSON array string format as to. "
62+
"Do not duplicate users already in to."
63+
),
64+
)
65+
subject: Optional[str] = Field(
66+
default=None,
67+
description="Suggested subject line; only when the check implies it should change.",
68+
)
5269
version_status: Optional[str] = Field(
5370
default=None,
5471
description="Suggested version status (maps to draft version_status).",
5572
)
56-
links: Optional[list[DraftNoteLink]] = None
73+
links: Optional[list[DraftNoteLink]] = Field(
74+
default=None,
75+
description=(
76+
"Non-user entity links (Shot, Asset, Version, Task, Playlist). "
77+
"Do not put users here—use to or cc for users."
78+
),
79+
)
5780

5881

5982
class NoteQCLLMOutput(BaseModel):
6083
"""Structured QC verdict returned by the LLM (instructor-validated)."""
6184

62-
passed: bool
63-
issue: Optional[str] = None
64-
evidence: Optional[str] = None
85+
passed: bool = Field(
86+
description="True only if the check passes and no draft changes are needed.",
87+
)
88+
issue: Optional[str] = Field(
89+
default=None,
90+
description="What failed; required when passed is false.",
91+
)
92+
evidence: Optional[str] = Field(
93+
default=None,
94+
description="Quote or reference from transcript/draft supporting the issue.",
95+
)
6596
note_suggestion: Optional[str] = Field(
6697
default=None,
67-
description="Full suggested note body when the check fails.",
98+
description=(
99+
"Full corrected note body when content must change. Use @[Name](type:id) "
100+
"for mentions (e.g. user:484). Omit when only metadata changes."
101+
),
102+
)
103+
attribute_suggestion: Optional[NoteQCAttributeSuggestion] = Field(
104+
default=None,
105+
description=(
106+
"Metadata fixes only: include fields that differ from the current draft. "
107+
"Use to for direct addressees, cc for other users, links for non-user entities."
108+
),
68109
)
69-
attribute_suggestion: Optional[NoteQCAttributeSuggestion] = None
70110

71111

72112
class NoteQCResult(BaseModel):
@@ -97,9 +137,12 @@ class RunQCChecksResponse(BaseModel):
97137
DEFAULT_ACTION_ITEM_CHECK = NoteQCCheckCreate(
98138
name="Action Item Check",
99139
prompt=(
100-
"Review the transcript and note below. "
101-
"If the transcript mentions an action item, task, or decision that is NOT "
102-
"reflected in the note, report it. Otherwise respond with passed=true."
140+
"Review the transcript and draft note. If the transcript mentions an action "
141+
"item, task, owner, or decision that is missing or unclear in the note, fail "
142+
"the check. Suggest a corrected note body that includes the missing items. "
143+
"If someone is assigned the action, add them to To; if others are only "
144+
"mentioned for visibility, add them to Cc. The check only passes when the note "
145+
"fully captures all action items and owners from the transcript."
103146
),
104147
severity="warning",
105148
enabled=True,

backend/src/dna/qc/qc_prompt.py

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
"""System and extraction prompts for note quality checks."""
2+
3+
from __future__ import annotations
4+
5+
QC_EXTRACTION_USER_MESSAGE = """\
6+
Provide your final quality-check result for this draft and check.
7+
8+
When passed is false, include actionable suggestions that resolve every issue \
9+
described in the check instructions:
10+
- Use note_suggestion for the full corrected note body when content must change.
11+
- Use attribute_suggestion only for metadata fields that must change (to, cc, \
12+
links, subject, version_status). Omit fields that should stay as they are.
13+
- Resolve users and entities with search_entities / get_entity before suggesting \
14+
ids in links or recipient lists.
15+
16+
When passed is true, leave note_suggestion and attribute_suggestion null.\
17+
"""
18+
19+
QC_SYSTEM_INSTRUCTIONS = """\
20+
You are a quality-check assistant for VFX dailies notes. Your job is to \
21+
evaluate the current draft against the check instructions in the user message, \
22+
then propose concrete fixes when the check fails.
23+
24+
## Goal
25+
26+
When the check fails, return suggestions the author can apply to resolve every \
27+
issue found. Do not only describe problems—supply corrected content and metadata \
28+
where needed. When the check passes, set passed=true and omit suggestions.
29+
30+
## Inputs
31+
32+
- Version context: shot/task/version metadata for the note being published.
33+
- Transcript: spoken dailies discussion (primary source for missing facts).
34+
- Current draft note (JSON): content, subject, to, cc, links, version_status.
35+
36+
Use the transcript, the current draft note, and the version context together with the check instructions. \
37+
You may call search_entities and get_entity to resolve production-tracker \
38+
users, shots, assets, versions, tasks, and playlists before suggesting ids.
39+
40+
## How to choose which fields to change
41+
42+
Infer which draft fields need updates from the issue type. Only suggest fields \
43+
that actually change.
44+
45+
**note_suggestion (note body / content)**
46+
- Use for wording, missing action items, decisions, feedback, or @-mentions in \
47+
the note text.
48+
- Return the full proposed note body (not a diff). Preserve correct existing \
49+
text; fix or extend what the check targets.
50+
- User mentions in the body use: @[Display Name](type:id) with lowercase type \
51+
and numeric id, e.g. @[Jane Doe](user:484). Resolve users via tools first.
52+
53+
**attribute_suggestion.to**
54+
- Primary recipients: people the note is directed to or who own the action \
55+
("tell Maya to…", "John needs to fix…", direct feedback to one person).
56+
- Value: JSON string of a JSON array of User objects, each \
57+
{"type":"User","id":<int>,"name":"<display name>"}. Include existing To \
58+
recipients unless the check requires replacing them.
59+
60+
**attribute_suggestion.cc**
61+
- Secondary visibility: people mentioned in the transcript or note who should \
62+
see the note but are not the primary addressee (mentioned in passing, \
63+
stakeholders, "loop in" / "cc" intent).
64+
- Do not put users in cc when they should be To (direct addressee). Do not \
65+
duplicate the same user in both to and cc.
66+
- Same JSON array string format as to.
67+
68+
**attribute_suggestion.links**
69+
- Non-user production references: shots, assets, versions, tasks, playlists.
70+
- Do not put users in links; users belong in to or cc.
71+
- Each item: {"entity_type":"<Type>","entity_id":<int>,"entity_name":"<name>"} \
72+
where entity_type matches DNA types (e.g. Shot, Asset, Version, Task, Playlist).
73+
- Also reference linked entities in the note body with @[Name](type:id) when \
74+
appropriate.
75+
76+
**attribute_suggestion.subject**
77+
- Only when the check or transcript implies the subject line is wrong or incomplete.
78+
79+
**attribute_suggestion.version_status**
80+
- Only when the check or transcript implies the version status should change.
81+
82+
## Response rules
83+
84+
- passed: true only if the check is satisfied and no fixes are required.
85+
- issue: short summary of what failed (null if passed).
86+
- evidence: brief quote or reference from transcript and/or draft (null if passed).
87+
- When passed is false, provide at least one of note_suggestion or \
88+
attribute_suggestion with fields that differ from the current draft.
89+
- Prefer attribute_suggestion for routing (to/cc/links) and note_suggestion for \
90+
prose; use both when the check requires both.
91+
- Do not embed a full JSON note inside note_suggestion unless you also split \
92+
fields correctly; prefer separate note_suggestion and attribute_suggestion.
93+
"""
94+
95+
96+
def build_qc_system_prompt(
97+
version_context: str,
98+
transcript_text: str,
99+
draft_json: str,
100+
) -> str:
101+
"""Assemble the QC system prompt with runtime context."""
102+
return (
103+
f"{QC_SYSTEM_INSTRUCTIONS}\n\n"
104+
"--- Version context ---\n"
105+
f"{version_context}\n\n"
106+
"--- Transcript ---\n"
107+
f"{transcript_text}\n\n"
108+
"--- Current draft note (JSON) ---\n"
109+
f"{draft_json}\n"
110+
)

backend/src/dna/qc/qc_runner.py

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
NoteQCResult,
1515
)
1616
from dna.prodtrack_providers.prodtrack_provider_base import ProdtrackProviderBase
17+
from dna.qc.qc_prompt import QC_EXTRACTION_USER_MESSAGE, build_qc_system_prompt
1718

1819
QC_TOOL_DEFINITIONS: list[dict[str, Any]] = [
1920
{
@@ -134,18 +135,7 @@ async def _run_one_check(
134135
if isinstance(pid, int):
135136
project_id = pid
136137

137-
system_prompt = (
138-
"You are a quality-check assistant for VFX dailies notes. "
139-
"Use the provided version context, transcript, and draft note. "
140-
"Follow the user's check instructions in the user message. "
141-
"You may call tools to look up production tracker entities when needed.\n\n"
142-
"--- Version context ---\n"
143-
f"{version_context}\n\n"
144-
"--- Transcript ---\n"
145-
f"{transcript_text}\n\n"
146-
"--- Current draft note (JSON) ---\n"
147-
f"{draft_json}\n"
148-
)
138+
system_prompt = build_qc_system_prompt(version_context, transcript_text, draft_json)
149139
user_message = f"Check name: {check.name}\n\nCheck instructions:\n{check.prompt}"
150140

151141
tool_executor = _make_tool_executor(prodtrack_provider, project_id)
@@ -158,6 +148,7 @@ async def _run_one_check(
158148
response_model=NoteQCLLMOutput,
159149
max_iterations=5,
160150
temperature=0.2,
151+
extraction_user_message=QC_EXTRACTION_USER_MESSAGE,
161152
)
162153
except Exception as exc: # pragma: no cover - network/provider errors
163154
return _failed_run_result(

backend/src/main.py

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
from fastapi.responses import FileResponse
2222
from fastapi.security import HTTPAuthorizationCredentials, HTTPBearer
2323

24+
from dna.auth.email import emails_match
2425
from dna.auth_providers.auth_provider_base import AuthProviderBase, get_auth_provider
2526
from dna.cors_settings import get_cors_middleware_kwargs
2627
from dna.events import EventType, get_event_publisher
@@ -1297,7 +1298,7 @@ async def get_user_settings(
12971298
current_user: CurrentUserDep,
12981299
) -> UserSettingsResponse:
12991300
"""Get user settings."""
1300-
if user_email != current_user:
1301+
if not emails_match(user_email, current_user):
13011302
raise HTTPException(status_code=403, detail="Forbidden")
13021303
stored = await provider.get_user_settings(user_email)
13031304
if stored is None:
@@ -1319,7 +1320,7 @@ async def upsert_user_settings(
13191320
current_user: CurrentUserDep,
13201321
) -> UserSettingsResponse:
13211322
"""Create or update user settings."""
1322-
if user_email != current_user:
1323+
if not emails_match(user_email, current_user):
13231324
raise HTTPException(status_code=403, detail="Forbidden")
13241325
updated = await provider.upsert_user_settings(user_email, data)
13251326
return _user_settings_to_response(updated)
@@ -1338,7 +1339,7 @@ async def delete_user_settings(
13381339
current_user: CurrentUserDep,
13391340
) -> bool:
13401341
"""Delete user settings."""
1341-
if user_email != current_user:
1342+
if not emails_match(user_email, current_user):
13421343
raise HTTPException(status_code=403, detail="Forbidden")
13431344
deleted = await provider.delete_user_settings(user_email)
13441345
if not deleted:
@@ -1357,7 +1358,7 @@ async def list_qc_checks(
13571358
storage_provider: StorageProviderDep,
13581359
current_user: CurrentUserDep,
13591360
) -> list[NoteQCCheck]:
1360-
if user_email != current_user:
1361+
if not emails_match(user_email, current_user):
13611362
raise HTTPException(status_code=403, detail="Forbidden")
13621363
return await storage_provider.get_qc_checks(user_email)
13631364

@@ -1375,7 +1376,7 @@ async def create_qc_check(
13751376
storage_provider: StorageProviderDep,
13761377
current_user: CurrentUserDep,
13771378
) -> NoteQCCheck:
1378-
if user_email != current_user:
1379+
if not emails_match(user_email, current_user):
13791380
raise HTTPException(status_code=403, detail="Forbidden")
13801381
return await storage_provider.create_qc_check(user_email, data)
13811382

@@ -1393,7 +1394,7 @@ async def update_qc_check(
13931394
storage_provider: StorageProviderDep,
13941395
current_user: CurrentUserDep,
13951396
) -> NoteQCCheck:
1396-
if user_email != current_user:
1397+
if not emails_match(user_email, current_user):
13971398
raise HTTPException(status_code=403, detail="Forbidden")
13981399
updated = await storage_provider.update_qc_check(user_email, check_id, data)
13991400
if updated is None:
@@ -1413,7 +1414,7 @@ async def delete_qc_check(
14131414
storage_provider: StorageProviderDep,
14141415
current_user: CurrentUserDep,
14151416
) -> None:
1416-
if user_email != current_user:
1417+
if not emails_match(user_email, current_user):
14171418
raise HTTPException(status_code=403, detail="Forbidden")
14181419
deleted = await storage_provider.delete_qc_check(user_email, check_id)
14191420
if not deleted:
@@ -1435,8 +1436,8 @@ async def run_qc_checks(
14351436
llm_provider: LLMProviderDep,
14361437
current_user: CurrentUserDep,
14371438
) -> RunQCChecksResponse:
1438-
if body.user_email != current_user:
1439-
raise HTTPException(status_code=403, detail="Forbidden")
1439+
# Authenticated callers may QC any draft in the playlist (same as publish-notes).
1440+
# body.user_email identifies the draft owner, not the caller.
14401441
draft = await storage_provider.get_draft_note(
14411442
body.user_email, playlist_id, version_id
14421443
)

backend/tests/test_auth_email.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
"""Tests for email authorization helpers."""
2+
3+
from dna.auth.email import emails_match
4+
5+
6+
def test_emails_match_case_insensitive():
7+
assert emails_match("Test@Example.com", "test@example.com")
8+
assert emails_match(" user@corp.com ", "user@corp.com")
9+
10+
11+
def test_emails_match_different_addresses():
12+
assert not emails_match("a@example.com", "b@example.com")

0 commit comments

Comments
 (0)