Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
194 changes: 109 additions & 85 deletions backend/routes/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
Restricts sign-in to @bu.edu email accounts only.
"""

import hashlib
import json
import base64
import hashlib
import secrets
import traceback
from urllib.parse import urlencode

from fastapi import APIRouter, HTTPException, Query
Expand Down Expand Up @@ -47,25 +48,29 @@ def _google_client_config() -> dict:
}


def _generate_pkce_pair():
code_verifier = base64.urlsafe_b64encode(secrets.token_bytes(32)).rstrip(b"=").decode()
code_challenge = base64.urlsafe_b64encode(
hashlib.sha256(code_verifier.encode()).digest()
).rstrip(b"=").decode()
return code_verifier, code_challenge


def _encode_state(data: dict) -> str:
payload = json.dumps(data)
return base64.urlsafe_b64encode(payload.encode()).decode()
return base64.urlsafe_b64encode(json.dumps(data).encode()).decode()


def _decode_state(state: str) -> dict:
try:
payload = base64.urlsafe_b64decode(state.encode()).decode()
return json.loads(payload)
return json.loads(base64.urlsafe_b64decode(state.encode()).decode())
except Exception:
return {}


def _generate_pkce_pair():
code_verifier = base64.urlsafe_b64encode(secrets.token_bytes(32)).rstrip(b'=').decode()
code_challenge = base64.urlsafe_b64encode(
hashlib.sha256(code_verifier.encode()).digest()
).rstrip(b'=').decode()
return code_verifier, code_challenge
def _email_log_value(email: str) -> str:
if "@" in email:
return email.split("@")[-1]
return "unknown"


@router.get("/google")
Expand All @@ -80,7 +85,7 @@ def google_login():
auth_url, _ = flow.authorization_url(
prompt="consent",
access_type="offline",
state=_encode_state({"action": "signin", "cv": code_verifier}),
state=_encode_state({"cv": code_verifier}),
code_challenge=code_challenge,
code_challenge_method="S256",
)
Expand All @@ -89,83 +94,102 @@ def google_login():

@router.get("/google/callback")
def google_callback(code: str = Query(...), state: str = Query(None)):
"""Exchange auth code for tokens, validate @bu.edu, upsert user."""
"""Exchange auth code for tokens, validate .edu email, upsert user."""
if not GOOGLE_AVAILABLE:
return RedirectResponse(f"{FRONTEND_URL}/signin?error=google_not_configured")
return RedirectResponse(f"{FRONTEND_URL}/?error=google_not_configured")

state_data = _decode_state(state) if state else {}
code_verifier = state_data.get("cv")

flow = Flow.from_client_config(_google_client_config(), scopes=AUTH_SCOPES)
flow.redirect_uri = GOOGLE_AUTH_REDIRECT_URI
flow.fetch_token(code=code, code_verifier=code_verifier)
creds = flow.credentials

# Fetch user info from Google
service = build("oauth2", "v2", credentials=creds)
user_info = service.userinfo().get().execute()

email = user_info.get("email", "")
google_id = user_info.get("id", "")
name = user_info.get("name", "")
avatar_url = user_info.get("picture", "")

# Restrict to @bu.edu accounts
if not email.endswith("@bu.edu"):
return RedirectResponse(
f"{FRONTEND_URL}/signin?error=invalid_domain"
)

# Determine user_id: check if this Google ID already exists
existing = table("users").select("id", filters={"google_id": f"eq.{google_id}"})
if existing:
user_id = existing[0]["id"]
# Update name/avatar in case they changed
table("users").update(
{"name": name, "avatar_url": avatar_url, "email": email},
filters={"id": f"eq.{user_id}"},
)
else:
# Check if a user with this email exists (migration from old system)
email_match = table("users").select("id", filters={"email": f"eq.{email}"})
if email_match:
user_id = email_match[0]["id"]
try:
print("[auth] step 1: fetching token")
code_verifier = _decode_state(state).get("cv") if state else None
print(f"[auth] code_verifier present: {bool(code_verifier)}")
flow = Flow.from_client_config(_google_client_config(), scopes=AUTH_SCOPES)
flow.redirect_uri = GOOGLE_AUTH_REDIRECT_URI
flow.fetch_token(code=code, code_verifier=code_verifier)
creds = flow.credentials
print("[auth] step 2: token fetched OK")

# Fetch user info from Google
service = build("oauth2", "v2", credentials=creds)
user_info = service.userinfo().get().execute()

email = user_info.get("email", "")
email_log_value = _email_log_value(email)
print(f"[auth] step 3: user_info email_domain={email_log_value}")
google_id = user_info.get("id", "")
name = user_info.get("name", "")
avatar_url = user_info.get("picture", "")

# Restrict to .edu accounts
if not email.endswith(".edu"):
print(f"[auth] rejected: not .edu (email_domain={email_log_value})")
return RedirectResponse(f"{FRONTEND_URL}/?error=invalid_domain")

is_new_user = False

# Determine user_id: check if this Google ID already exists
print("[auth] step 4: checking DB")
computed_id = f"user_{google_id}"
existing = table("users").select("id", filters={"google_id": f"eq.{google_id}"})
if existing:
user_id = existing[0]["id"]
print(f"[auth] existing user (google_id match): {user_id}")
table("users").update(
{
"google_id": google_id,
"name": name,
"avatar_url": avatar_url,
"auth_provider": "google",
},
{"name": name, "avatar_url": avatar_url, "email": email},
filters={"id": f"eq.{user_id}"},
)
Comment on lines 136 to 139

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Inconsistent auth_provider update across branches.

When a user is found by google_id match, the update (lines 136-139) does not set auth_provider, while all other code paths explicitly set auth_provider="google" (lines 146, 156, 169). This could leave legacy users without the field populated.

🔧 Proposed fix for consistency
         table("users").update(
-            {"name": name, "avatar_url": avatar_url, "email": email},
+            {"name": name, "avatar_url": avatar_url, "email": email, "auth_provider": "google"},
             filters={"id": f"eq.{user_id}"},
         )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/routes/auth.py` around lines 136 - 139, The update in the google_id
branch uses table("users").update({...}) but omits setting auth_provider,
causing inconsistency with other branches that set auth_provider="google";
modify the update payload in that branch to include "auth_provider": "google"
alongside name, avatar_url, and email (the same fields used in the other update
branches) for the record identified by user_id so legacy users receive the
auth_provider value.

else:
# Create new user
user_id = f"user_{google_id}"
table("users").insert({
"id": user_id,
"name": name,
"email": email,
"google_id": google_id,
"avatar_url": avatar_url,
"auth_provider": "google",
})

# Store OAuth tokens (calendar access included)
table("oauth_tokens").upsert(
{
"user_id": user_id,
"access_token": creds.token,
"refresh_token": creds.refresh_token or "",
"expires_at": creds.expiry.isoformat() if creds.expiry else "",
},
on_conflict="user_id",
)
email_match = table("users").select("id", filters={"email": f"eq.{email}"})
if email_match:
user_id = email_match[0]["id"]
print(f"[auth] existing user (email match): {user_id}")
table("users").update(
{"google_id": google_id, "name": name, "avatar_url": avatar_url, "auth_provider": "google"},
filters={"id": f"eq.{user_id}"},
)
else:
id_match = table("users").select("id", filters={"id": f"eq.{computed_id}"})
if id_match:
# Partially created in a previous failed auth — treat as existing
user_id = id_match[0]["id"]
print(f"[auth] existing user (id match, partial prev insert): {user_id}")
table("users").update(
{"google_id": google_id, "name": name, "avatar_url": avatar_url, "email": email, "auth_provider": "google"},
filters={"id": f"eq.{user_id}"},
)
else:
is_new_user = True
user_id = computed_id
print(f"[auth] creating new user: {user_id}")
table("users").insert({
"id": user_id,
"name": name,
"email": email,
"google_id": google_id,
"avatar_url": avatar_url,
"auth_provider": "google",
})

print("[auth] step 5: upserting tokens")
table("oauth_tokens").upsert(
{
"user_id": user_id,
"access_token": creds.token,
"refresh_token": creds.refresh_token or "",
"expires_at": creds.expiry.isoformat() if creds.expiry else "",
},
on_conflict="user_id",
)

# Redirect to frontend with user info
params = urlencode({
"user_id": user_id,
"name": name,
"avatar": avatar_url,
})
return RedirectResponse(f"{FRONTEND_URL}/signin/callback?{params}")
print(f"[auth] step 6: done, is_new={is_new_user}, redirecting")
params = urlencode({
"user_id": user_id,
"name": name,
"avatar": avatar_url,
"is_new": "true" if is_new_user else "false",
})
return RedirectResponse(f"{FRONTEND_URL}/signin/callback?{params}")

except Exception as e:
traceback.print_exc()
print(f"[auth] FAILED at step above: {e}")
return RedirectResponse(f"{FRONTEND_URL}/?error=auth_failed")
24 changes: 17 additions & 7 deletions backend/routes/calendar.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"""

import json
import logging
from datetime import datetime, timezone, timedelta

from fastapi import APIRouter, File, Form, HTTPException, Query, UploadFile
Expand All @@ -29,6 +30,7 @@
GOOGLE_AVAILABLE = False

router = APIRouter()
logger = logging.getLogger(__name__)


# ── Helpers ───────────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -115,13 +117,21 @@ def save_assignments(body: SaveAssignmentsBody):
@router.get("/upcoming/{user_id}")
def get_upcoming(user_id: str):
today = datetime.utcnow().strftime("%Y-%m-%d")
rows = table("assignments").select(
"*",
filters={"user_id": f"eq.{user_id}", "due_date": f"gte.{today}"},
order="due_date.asc",
limit=20,
)
return {"assignments": rows}
try:
rows = table("assignments").select(
"*",
filters={"user_id": f"eq.{user_id}", "due_date": f"gte.{today}"},
order="due_date.asc",
limit=20,
)
return {"assignments": rows or []}
except Exception:
logger.exception(
"Failed to fetch upcoming assignments for user_id=%s today=%s",
user_id,
today,
)
return {"assignments": []}
Comment on lines +120 to +134

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify blind catches and missing telemetry in these route handlers.
rg -n -C2 'except Exception' backend/routes/calendar.py backend/routes/graph.py
rg -n 'logger\.(exception|error|warning)\(' backend/routes/calendar.py backend/routes/graph.py

Repository: SaplingLearn/Sapling

Length of output: 1279


🏁 Script executed:

# Find test files for calendar routes
find . -type f -name "*test*calendar*" -o -name "*calendar*test*" | head -20

Repository: SaplingLearn/Sapling

Length of output: 103


🏁 Script executed:

# Check if logging is configured in the backend
rg -n "import logging" backend/ | head -10
rg -n "^logger = " backend/ | head -10

Repository: SaplingLearn/Sapling

Length of output: 176


🏁 Script executed:

# Look for pytest/test configuration
fd -t f "(conftest|pytest)" backend/

Repository: SaplingLearn/Sapling

Length of output: 89


🏁 Script executed:

cat -n backend/tests/test_calendar_routes.py

Repository: SaplingLearn/Sapling

Length of output: 8933


🏁 Script executed:

# Check calendar.py to see the full context of the get_upcoming function
cat -n backend/routes/calendar.py | head -150

Repository: SaplingLearn/Sapling

Length of output: 5870


🏁 Script executed:

# Look at how documents.py handles exceptions (shows logging pattern)
rg -B5 -A5 'except Exception' backend/routes/documents.py

Repository: SaplingLearn/Sapling

Length of output: 922


Add logging and exception path test coverage to /upcoming/{user_id} endpoint.

Line 126–127 silently swallows all exceptions, hiding real database/service failures and making incident detection impossible. Tests also lack coverage for the exception path.

Specific issues
  1. Missing telemetry: Exceptions are not logged; only the empty fallback is returned, eliminating visibility into failures.
  2. Incomplete test coverage: TestGetUpcoming tests the success and empty-result cases but does not test the exception handler path (line 126–127 behavior).

The logging pattern is already established in backend/routes/documents.py (import logging, logger exception calls with context).

Suggested minimal fix
+import logging
+
+logger = logging.getLogger(__name__)
+
 `@router.get`("/upcoming/{user_id}")
 def get_upcoming(user_id: str):
     try:
         today = datetime.utcnow().strftime("%Y-%m-%d")
         rows = table("assignments").select(
             "*",
             filters={"user_id": f"eq.{user_id}", "due_date": f"gte.{today}"},
             order="due_date.asc",
             limit=20,
         )
         return {"assignments": rows or []}
-    except Exception:
-        return {"assignments": []}
+    except HTTPException:
+        raise
+    except Exception:
+        logger.exception("calendar.get_upcoming_failed", extra={"user_id": user_id})
+        return {"assignments": []}

Add test case in backend/tests/test_calendar_routes.py:

def test_returns_empty_on_db_failure(self):
    with patch("routes.calendar.table") as t:
        t.return_value.select.side_effect = Exception("DB connection error")
        r = client.get("/api/calendar/upcoming/user_andres")
    assert r.status_code == 200
    assert r.json()["assignments"] == []
🧰 Tools
🪛 Ruff (0.15.10)

[warning] 126-126: Do not catch blind exception: Exception

(BLE001)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/routes/calendar.py` around lines 117 - 127, The endpoint in
backend/routes/calendar.py swallows all exceptions without logging; update the
try/except around the table("assignments").select call to import logging, use a
module-level logger, and call logger.exception(...) (including user_id and the
query params like today) inside the except before returning {"assignments": []};
then add a test in backend/tests/test_calendar_routes.py (e.g.,
test_returns_empty_on_db_failure) that patches routes.calendar.table so select
raises Exception("DB connection error") and asserts the response is 200 with
{"assignments": []} to cover the exception path.



@router.post("/suggest-study-blocks")
Expand Down
17 changes: 14 additions & 3 deletions backend/routes/graph.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
from fastapi import APIRouter
import logging

from fastapi import APIRouter, HTTPException
from pydantic import BaseModel
from typing import Optional

Expand All @@ -8,16 +10,25 @@
)

router = APIRouter()
logger = logging.getLogger(__name__)


@router.get("/{user_id}")
def get_user_graph(user_id: str):
return get_graph(user_id)
try:
return get_graph(user_id)
except Exception as e:
logger.exception("get_graph failed for user_id=%s", user_id)
raise HTTPException(status_code=500, detail=str(e) or "Failed to fetch graph")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid exposing raw backend exception text in API responses.

Using detail=str(e) can leak internal error content to clients. Since you already log the full exception, return a stable generic message instead. (If changed, update assertions in backend/tests/test_graph_routes.py at Line 20 and Line 27 accordingly.)

Also applies to: 31-31

🧰 Tools
🪛 Ruff (0.15.10)

[warning] 22-22: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/routes/graph.py` at line 22, The route currently raises HTTPException
with detail=str(e), which can leak internal errors; change both occurrences of
raise HTTPException(status_code=500, detail=str(e) or "Failed to fetch graph")
to raise HTTPException(status_code=500, detail="Failed to fetch graph") (i.e.,
return a stable generic message) and keep full exception logging as-is; also
update the assertions in backend/tests/test_graph_routes.py (the assertions at
Line 20 and Line 27) to expect the new generic message.



@router.get("/{user_id}/recommendations")
def get_user_recommendations(user_id: str):
return {"recommendations": get_recommendations(user_id)}
try:
return {"recommendations": get_recommendations(user_id)}
except Exception as e:
logger.exception("get_recommendations failed for user_id=%s", user_id)
raise HTTPException(status_code=500, detail=str(e) or "Failed to fetch recommendations")


# ── Course endpoints ──────────────────────────────────────────────────────────
Expand Down
8 changes: 8 additions & 0 deletions backend/tests/test_calendar_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,14 @@ def test_returns_empty_list_when_none(self):
assert r.status_code == 200
assert r.json()["assignments"] == []

def test_returns_empty_on_db_failure(self):
with patch("routes.calendar.table") as t:
t.return_value.select.side_effect = Exception("DB connection error")
r = client.get("/api/calendar/upcoming/user_andres")

assert r.status_code == 200
assert r.json() == {"assignments": []}


# ── POST /api/calendar/suggest-study-blocks ───────────────────────────────────

Expand Down
27 changes: 27 additions & 0 deletions backend/tests/test_graph_routes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
"""
Unit tests for routes/graph.py
"""

from unittest.mock import patch

from fastapi.testclient import TestClient

from main import app

client = TestClient(app)


class TestGraphRoutes:
def test_get_user_graph_returns_500_on_failure(self):
with patch("routes.graph.get_graph", side_effect=Exception("graph failed")):
r = client.get("/api/graph/user_andres")

assert r.status_code == 500
assert r.json() == {"detail": "graph failed"}

def test_get_user_recommendations_returns_500_on_failure(self):
with patch("routes.graph.get_recommendations", side_effect=Exception("recommendations failed")):
r = client.get("/api/graph/user_andres/recommendations")

assert r.status_code == 500
assert r.json() == {"detail": "recommendations failed"}
10 changes: 10 additions & 0 deletions frontend/src/app/api/auth/google/callback/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export const dynamic = 'force-static';

export async function GET() {
return new Response('Static export build: Google auth callback is handled by the backend redirect.', {
status: 410,
headers: {
'Content-Type': 'text/plain; charset=utf-8',
},
});
}
10 changes: 10 additions & 0 deletions frontend/src/app/api/auth/google/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export const dynamic = 'force-static';

export async function GET() {
return new Response('Static export build: use the frontend Google sign-in button instead.', {
status: 410,
headers: {
'Content-Type': 'text/plain; charset=utf-8',
},
});
}
Loading
Loading