Merge pull request #969 from pendulum-chain/copilot/sub-pr-912

ebma · web-flow · commit 70568bd83ac7 · 2025-12-16T09:05:17.000+01:00
Address code review comments: security, performance, and bug fixes
diff --git a/apps/api/src/api/controllers/brla.controller.ts b/apps/api/src/api/controllers/brla.controller.ts
@@ -205,6 +205,14 @@ export const recordInitialKycAttempt = async (
     });
 
     if (!taxIdRecord) {
+      // Validate that user is authenticated since userId is required in the schema
+      if (!req.userId) {
+        res.status(httpStatus.UNAUTHORIZED).json({ 
+          error: "Authentication required to record KYC attempt" 
+        });
+        return;
+      }
+
       const accountType = isValidCnpj(taxId)
         ? AveniaAccountType.COMPANY
         : isValidCpf(taxId)
@@ -220,9 +228,6 @@ export const recordInitialKycAttempt = async (
           internalStatus: TaxIdInternalStatus.Consulted,
           subAccountId: "",
           taxId,
-          // @ts-ignore: Assume userId is passed in body for now, or use empty string if logic permits (but schema is NOT NULL)
-          // Actually, if Auth is first, we should have userId.
-          // Using a placeholder assertion as we can't change the request type easily here without bigger refactor.
           userId: req.userId
         });
       }
diff --git a/apps/api/src/api/middlewares/supabaseAuth.ts b/apps/api/src/api/middlewares/supabaseAuth.ts
@@ -1,4 +1,5 @@
 import { NextFunction, Request, Response } from "express";
+import logger from "../../config/logger";
 import { SupabaseAuthService } from "../services/auth";
 
 declare global {
@@ -60,6 +61,17 @@ export async function optionalAuth(req: Request, res: Response, next: NextFuncti
 
     next();
   } catch (error) {
+    // Log truncated token for security - only show first/last few characters
+    const authHeader = req.headers.authorization;
+    const truncatedAuth = authHeader
+      ? `${authHeader.substring(0, 15)}...${authHeader.substring(authHeader.length - 4)}`
+      : undefined;
+
+    logger.warn("optionalAuth middleware: authentication error", {
+      authorization: truncatedAuth,
+      error,
+      path: req.path
+    });
     next();
   }
 }
diff --git a/apps/api/src/api/services/auth/supabase.service.ts b/apps/api/src/api/services/auth/supabase.service.ts
@@ -6,14 +6,22 @@ export class SupabaseAuthService {
    */
   static async checkUserExists(email: string): Promise<boolean> {
     try {
-      const { data, error } = await supabaseAdmin.auth.admin.listUsers();
+      // Query the profiles table directly for better performance
+      const { data, error } = await supabaseAdmin
+        .from("profiles")
+        .select("id")
+        .eq("email", email)
+        .single();
 
       if (error) {
+        // If error is "PGRST116" (no rows returned), user doesn't exist
+        if (error.code === "PGRST116") {
+          return false;
+        }
         throw error;
       }
 
-      const userExists = data.users.some(user => user.email === email);
-      return userExists;
+      return !!data;
     } catch (error) {
       console.error("Error checking user existence:", error);
       throw error;
diff --git a/apps/api/src/database/migrations/013-fix-tax-ids-table.ts b/apps/api/src/database/migrations/013-fix-tax-ids-table.ts
@@ -11,8 +11,19 @@ export async function up(queryInterface: QueryInterface): Promise<void> {
       END IF;
     END $$;
 
-    -- Add the 'COMPANY' value to the existing enum type safely
-    ALTER TYPE "enum_tax_ids_account_type" ADD VALUE IF NOT EXISTS 'COMPANY';
+    -- Add the 'COMPANY' value to the existing enum type safely (compatible with PostgreSQL <12)
+    DO $$
+    BEGIN
+      IF NOT EXISTS (
+        SELECT 1 FROM pg_enum
+        WHERE enumlabel = 'COMPANY'
+          AND enumtypid = (
+            SELECT oid FROM pg_type WHERE typname = 'enum_tax_ids_account_type'
+          )
+      ) THEN
+        ALTER TYPE "enum_tax_ids_account_type" ADD VALUE 'COMPANY';
+      END IF;
+    END $$;
   `);
 }
 
diff --git a/apps/api/src/database/migrations/022-add-user-id-to-entities.ts b/apps/api/src/database/migrations/022-add-user-id-to-entities.ts
@@ -2,10 +2,11 @@ import {DataTypes, QueryInterface} from "sequelize";
 import {v4 as uuidv4} from "uuid";
 
 export async function up(queryInterface: QueryInterface): Promise<void> {
-  // Generate a dummy user ID for migration
-  const DUMMY_USER_ID = uuidv4();
+  // Use a well-known sentinel UUID for the migration placeholder user
+  // This UUID is specifically reserved for migration purposes
+  const DUMMY_USER_ID = "00000000-0000-0000-0000-000000000001";
 
-  console.log(`Using dummy user ID for migration: ${DUMMY_USER_ID}`);
+  console.log(`Using sentinel migration user ID: ${DUMMY_USER_ID}`);
 
   // Add user_id to kyc_level_2
   await queryInterface.addColumn("kyc_level_2", "user_id", {
@@ -14,10 +15,11 @@ export async function up(queryInterface: QueryInterface): Promise<void> {
   });
 
   // Insert dummy user to satisfy foreign key constraint
+  // Use ON CONFLICT to handle cases where migration is re-run
   const timestamp = new Date().toISOString();
   await queryInterface.sequelize.query(`
     INSERT INTO profiles (id, email, created_at, updated_at)
-    VALUES ('${DUMMY_USER_ID}', 'migration_placeholder_${DUMMY_USER_ID}@example.com', '${timestamp}', '${timestamp}')
+    VALUES ('${DUMMY_USER_ID}', 'migration_placeholder@vortex.internal', '${timestamp}', '${timestamp}')
     ON CONFLICT (id) DO NOTHING;
   `);
 
diff --git a/apps/frontend/src/components/widget-steps/AuthEmailStep/index.tsx b/apps/frontend/src/components/widget-steps/AuthEmailStep/index.tsx
@@ -41,51 +41,48 @@ export const AuthEmailStep = ({ className }: AuthEmailStepProps) => {
       className={cn("relative flex min-h-[506px] grow flex-col", className)}
       style={{ "--quote-summary-height": `${quoteSummaryHeight}px` } as React.CSSProperties}
     >
-      <div className="flex-1 pb-36">
+      <form className="flex flex-1 flex-col pb-36" onSubmit={handleSubmit}>
         <div className="mt-4 text-center">
           <h1 className="mb-4 font-bold text-3xl text-blue-700">Enter Your Email</h1>
-          <p className="text-gray-600 mb-6">We'll send you a one-time code to verify your identity</p>
+          <p className="mb-6 text-gray-600">We'll send you a one-time code to verify your identity</p>
         </div>
 
         <div className="flex flex-col items-center">
-          <div className="w-full max-w-md">
-            <form className="space-y-4" onSubmit={handleSubmit}>
-              <div>
-                <label className="block text-sm font-medium text-gray-700 mb-2" htmlFor="email">
-                  Email Address
-                </label>
-                <input
-                  autoFocus
-                  className="w-full px-4 py-3 border border-gray-300 rounded-lg focus:outline-none focus:ring-2 focus:ring-blue-500 focus:border-transparent"
-                  disabled={isLoading}
-                  id="email"
-                  onChange={e => setEmail(e.target.value)}
-                  placeholder="you@example.com"
-                  type="email"
-                  value={email}
-                />
-                {(localError || errorMessage) && <p className="mt-2 text-sm text-red-600">{localError || errorMessage}</p>}
-              </div>
-            </form>
+          <div className="w-full max-w-md space-y-4">
+            <div>
+              <label className="mb-2 block font-medium text-gray-700 text-sm" htmlFor="email">
+                Email Address
+              </label>
+              <input
+                autoFocus
+                className="w-full rounded-lg border border-gray-300 px-4 py-3 focus:border-transparent focus:outline-none focus:ring-2 focus:ring-blue-500"
+                disabled={isLoading}
+                id="email"
+                onChange={e => setEmail(e.target.value)}
+                placeholder="you@example.com"
+                type="email"
+                value={email}
+              />
+              {(localError || errorMessage) && <p className="mt-2 text-red-600 text-sm">{localError || errorMessage}</p>}
+            </div>
           </div>
         </div>
-      </div>
 
-      <div
-        className="absolute right-0 left-0 z-[5] flex flex-col items-center mb-4"
-        style={{ bottom: `calc(var(--quote-summary-height, 100px) + 2rem)` }}
-      >
-        <div className="w-full max-w-md">
-          <button
-            className="w-full bg-blue-600 text-white py-3 px-4 rounded-lg font-medium hover:bg-blue-700 disabled:bg-gray-400 disabled:cursor-not-allowed transition-colors"
-            disabled={isLoading}
-            onClick={handleSubmit}
-            type="button"
-          >
-            {isLoading ? "Sending..." : "Continue"}
-          </button>
+        <div
+          className="absolute right-0 left-0 z-[5] mb-4 flex flex-col items-center"
+          style={{ bottom: `calc(var(--quote-summary-height, 100px) + 2rem)` }}
+        >
+          <div className="w-full max-w-md">
+            <button
+              className="w-full rounded-lg bg-blue-600 px-4 py-3 font-medium text-white transition-colors hover:bg-blue-700 disabled:cursor-not-allowed disabled:bg-gray-400"
+              disabled={isLoading}
+              type="submit"
+            >
+              {isLoading ? "Sending..." : "Continue"}
+            </button>
+          </div>
         </div>
-      </div>
+      </form>
 
       {quote && <QuoteSummary onHeightChange={setQuoteSummaryHeight} quote={quote} />}
     </div>
diff --git a/apps/frontend/src/hooks/useAuthTokens.ts b/apps/frontend/src/hooks/useAuthTokens.ts
@@ -1,5 +1,5 @@
 import { useSelector } from "@xstate/react";
-import { useCallback, useEffect } from "react";
+import { useCallback, useEffect, useRef } from "react";
 import type { ActorRefFrom } from "xstate";
 import { supabase } from "../config/supabase";
 import type { rampMachine } from "../machines/ramp.machine";
@@ -12,25 +12,39 @@ export function useAuthTokens(actorRef: ActorRefFrom<typeof rampMachine>) {
     userId: state.context.userId
   }));
 
+  // Track if we've already restored the session to avoid running multiple times
+  const hasRestoredSession = useRef(false);
+
   // Check for tokens in URL on mount (magic link callback)
   useEffect(() => {
     const urlTokens = AuthService.handleUrlTokens();
     if (urlTokens) {
-      supabase.auth.getSession().then(({ data }) => {
-        if (data.session) {
-          const tokens = {
-            access_token: data.session.access_token,
-            refresh_token: data.session.refresh_token,
-            user_id: data.session.user.id
-          };
+      // Use the URL tokens to set session with Supabase, then get full user details
+      supabase.auth
+        .setSession({
+          access_token: urlTokens.accessToken,
+          refresh_token: urlTokens.refreshToken
+        })
+        .then(({ data, error }) => {
+          if (error) {
+            console.error("Failed to set session from URL tokens:", error);
+            return;
+          }
+
+          if (data.session) {
+            const tokens = {
+              accessToken: data.session.access_token,
+              refreshToken: data.session.refresh_token,
+              userId: data.session.user.id
+            };
 
-          AuthService.storeTokens(tokens);
-          actorRef.send({ tokens, type: "AUTH_SUCCESS" });
+            AuthService.storeTokens(tokens);
+            actorRef.send({ tokens, type: "AUTH_SUCCESS" });
 
-          // Clean URL
-          window.history.replaceState({}, "", window.location.pathname);
-        }
-      });
+            // Clean URL
+            window.history.replaceState({}, "", window.location.pathname);
+          }
+        });
     }
   }, [actorRef]);
 
@@ -42,16 +56,20 @@ export function useAuthTokens(actorRef: ActorRefFrom<typeof rampMachine>) {
 
   // Restore session from localStorage on mount
   useEffect(() => {
-    const tokens = AuthService.getTokens();
-    if (tokens && !isAuthenticated) {
-      actorRef.send({
-        tokens: {
-          access_token: tokens.access_token,
-          refresh_token: tokens.refresh_token,
-          user_id: tokens.user_id
-        },
-        type: "AUTH_SUCCESS"
-      });
+    // Only restore once on initial mount to avoid infinite loops
+    if (!hasRestoredSession.current && !isAuthenticated) {
+      const tokens = AuthService.getTokens();
+      if (tokens) {
+        hasRestoredSession.current = true;
+        actorRef.send({
+          tokens: {
+            accessToken: tokens.accessToken,
+            refreshToken: tokens.refreshToken,
+            userId: tokens.userId
+          },
+          type: "AUTH_SUCCESS"
+        });
+      }
     }
   }, [actorRef, isAuthenticated]);
 
diff --git a/apps/frontend/src/machines/ramp.machine.ts b/apps/frontend/src/machines/ramp.machine.ts
@@ -32,8 +32,8 @@ const getInitialAuthState = () => {
   if (tokens) {
     const authState = {
       isAuthenticated: true,
-      userEmail: tokens.user_email,
-      userId: tokens.user_id
+      userEmail: tokens.userEmail,
+      userId: tokens.userId
     };
     return authState;
   }
@@ -145,7 +145,7 @@ export type RampMachineEvents =
   | { type: "EMAIL_VERIFIED" }
   | { type: "OTP_SENT" }
   | { type: "VERIFY_OTP"; code: string }
-  | { type: "AUTH_SUCCESS"; tokens: { access_token: string; refresh_token: string; user_id: string } }
+  | { type: "AUTH_SUCCESS"; tokens: { accessToken: string; refreshToken: string; userId: string } }
   | { type: "AUTH_ERROR"; error: string }
   | { type: "CHANGE_EMAIL" }
   | { type: "LOGOUT" };
@@ -244,7 +244,7 @@ export const rampMachine = setup({
     AUTH_SUCCESS: {
       actions: assign({
         isAuthenticated: true,
-        userId: ({ event }) => event.tokens.user_id
+        userId: ({ event }) => event.tokens.userId
       })
     },
     EXPIRE_QUOTE: {
@@ -706,15 +706,15 @@ export const rampMachine = setup({
             assign({
               errorMessage: undefined,
               isAuthenticated: true,
-              userId: ({ event }) => event.output.user_id
+              userId: ({ event }) => event.output.userId
             }),
             ({ event, context }) => {
               // Store tokens in localStorage for session persistence
               AuthService.storeTokens({
-                access_token: event.output.access_token,
-                refresh_token: event.output.refresh_token,
-                user_email: context.userEmail,
-                user_id: event.output.user_id
+                accessToken: event.output.accessToken,
+                refreshToken: event.output.refreshToken,
+                userEmail: context.userEmail,
+                userId: event.output.userId
               });
             }
           ],
diff --git a/apps/frontend/src/machines/types.ts b/apps/frontend/src/machines/types.ts
@@ -71,7 +71,7 @@ export type RampMachineEvents =
   | { type: "EMAIL_VERIFIED" }
   | { type: "OTP_SENT" }
   | { type: "VERIFY_OTP"; code: string }
-  | { type: "AUTH_SUCCESS"; tokens: { access_token: string; refresh_token: string; user_id: string } }
+  | { type: "AUTH_SUCCESS"; tokens: { accessToken: string; refreshToken: string; userId: string } }
   | { type: "AUTH_ERROR"; error: string };
 
 export type RampMachineActor = ActorRef<any, RampMachineEvents>;
diff --git a/apps/frontend/src/services/api/api-client.ts b/apps/frontend/src/services/api/api-client.ts
@@ -20,8 +20,8 @@ apiClient.interceptors.request.use(
   config => {
     // Add Authorization header if user is authenticated
     const tokens = AuthService.getTokens();
-    if (tokens?.access_token) {
-      config.headers.Authorization = `Bearer ${tokens.access_token}`;
+    if (tokens?.accessToken) {
+      config.headers.Authorization = `Bearer ${tokens.accessToken}`;
     }
     return config;
   },
diff --git a/apps/frontend/src/services/api/auth.api.ts b/apps/frontend/src/services/api/auth.api.ts
diff --git a/apps/frontend/src/services/auth.ts b/apps/frontend/src/services/auth.ts
diff --git a/supabase/config.toml b/supabase/config.toml
