Auth Integration Audit: 6 Critical Issues

by Editorial Team 42 views
Iklan Headers

Hey folks! During the implementation of Issue #35 (Multi-Tenant User Scoping), we ran into a few snags with our auth integration. This article is your one-stop shop for all the nitty-gritty details, outlining the issues we found and the fixes we need to get everything working smoothly. We're diving deep, so buckle up!

Critical Issues: The Big Ones

1. JWT Token Format Mismatch (CRITICAL) - The Authentication Headache

File: web/lib/auth.ts:140-163

The Problem: Alright, so here's the deal. The getAuthToken() function is supposed to hand us the token from the NextAuth session cookie. The catch? NextAuth loves to encrypt those JWT tokens by default, using the JWE format. The backend, on the other hand, is expecting a plain, old-school HS256 JWT. Talk about a communication breakdown!

# src/sage/api/auth.py:60-64
payload = jwt.decode(
    token,
    self.settings.nextauth_secret,
    algorithms=["HS256"],  # Won't work with encrypted JWE!
)

Fixing the Mess: We've got a few options here, each with its own quirks:

  1. Option 1: Go Plain JWT (Not Recommended): We could configure NextAuth to use plain JWTs. But honestly, that's like leaving your front door unlocked – not the best for security.
  2. Option 2: Custom Backend Token (Recommended): This is the path we're leaning towards. After a user logs in via NextAuth, we'll hit the backend's /api/auth/token endpoint to grab a simple JWT containing the learner_id. We'll then store this in memory or a cookie and use it for all subsequent API calls. Simple, secure, and effective.
  3. Option 3: Use NextAuth's encode/decode Functions (Complex): We could try using NextAuth's built-in encode and decode functions on the backend. It's doable, but it adds a layer of complexity we'd rather avoid.

The Winner: Option 2 is our champion. It's the most secure and straightforward solution.

2. Chat Page's Fake Session IDs (Issue #110) - Chat's Identity Crisis

File: web/app/chat/page.tsx:57-62

The Problem: Heads up! The frontend of the chat page is generating its own session IDs in the format session-{timestamp}-{random}. The thing is, it should be calling POST /api/sessions to create these sessions. This issue has been filed as Issue #110, so we're on it.

3. Graph Page's Mock Data (Medium Priority) - The Data Disconnect

File: web/app/graph/page.tsx:29-104

The Problem: The knowledge graph page is currently living in a world of mock data. It's using hardcoded data for nodes, edges, and outcomes. We need to hook this up to the real API.

const MOCK_NODES: KnowledgeNode[] = [...]
const MOCK_EDGES: KnowledgeEdge[] = [...]
const MOCK_OUTCOMES: OutcomeSnapshot[] = [...]

Fixing the Problem: The fix is simple: we need to replace the mock data with a call to api.getLearnerGraph(learnerId). We'll get the learner_id from the NextAuth session, and boom, real data.

4. Missing /api/scenarios Proxy in next.config.mjs - Proxy Problems

File: web/next.config.mjs

The Problem: Our frontend is missing a proxy for the scenarios API routes. This means any calls to /api/scenarios/* are going straight to Next.js (and getting a 404) instead of our FastAPI backend.

// Missing from rewrites:
{ source: "/api/scenarios/:path*", destination: "http://localhost:8000/api/scenarios/:path*" }

Fixing the Problem: We need to add the missing rewrite to next.config.mjs to ensure the frontend correctly routes requests to the backend API.

5. useLearner Hook's LocalStorage Addiction - The Session Sabotage

File: web/hooks/useLearner.ts:7-39

The Problem: The useLearner hook is currently storing and retrieving the learner_id from localStorage. This is a big no-no when we have a proper auth system in place.

const LEARNER_ID_KEY = "sage_learner_id";
const storedId = localStorage.getItem(LEARNER_ID_KEY);

Fixing the Problem: We need to swap this out and get the learner_id from the NextAuth session. Here's how:

import { useSession } from "next-auth/react";
const { data: session } = useSession();
const learnerId = session?.user?.learner_id;

6. Practice Mode's Unnecessary Parameter - The Redundant Request

File: web/hooks/usePracticeMode.ts:79

The Problem: The usePracticeMode hook is passing the learner_id to the API. However, the backend already knows the learner_id from the auth token.

const response = await api.startPractice({
  // ...
  learner_id: learnerId,  // Unnecessary - backend gets from auth
});

Fixing the Problem: We can simply remove the learner_id from the startPractice request. The backend will handle it, and we'll have cleaner code.

Summary Table: Issues at a Glance

# Issue Severity File Status
1 JWT token format mismatch CRITICAL auth.ts, auth.py Open
2 Chat generates fake session IDs HIGH chat/page.tsx Issue #110
3 Graph page uses mock data MEDIUM graph/page.tsx Open
4 Missing scenarios proxy MEDIUM next.config.mjs Open
5 useLearner uses localStorage MEDIUM useLearner.ts Open
6 Unnecessary learner_id param LOW usePracticeMode.ts Open

Recommended Fix Order: The Action Plan

Here's the order we should tackle these issues:

  1. Fix the JWT Token Issue (#1): This is the most critical issue and everything else depends on it.
  2. Add the Scenarios Proxy (#4): A quick win to get our API routes working.
  3. Fix Chat Session Creation (#2): Already in progress, so let's keep the ball rolling.
  4. Update the useLearner Hook (#5): This affects multiple components, so it's a priority.
  5. Fix the Graph Page (#3): Requires the useLearner hook to be fixed first.
  6. Clean Up Practice Mode (#6): Low priority, just a bit of cleanup.

Related Issues: For More Context

  • Issue #35 - The parent auth implementation issue.
  • Issue #110 - The chat WebSocket session bug.

That's all for now, folks! We'll keep you updated on our progress. Stay tuned!