From 390bde88b5b6ef13bd9b048cb9b847632f9b3082 Mon Sep 17 00:00:00 2001 From: David Cox Date: Tue, 24 Sep 2024 13:00:20 -0700 Subject: [PATCH] refactor: consolidate auth errors and logging --- client/src/auth.ts | 65 +++++++++++++++++++++------- client/src/errorUtils.ts | 49 +++++++++++++++++++++ client/src/types.ts | 3 +- client/tests/keyhippo-client.test.ts | 2 +- 4 files changed, 102 insertions(+), 17 deletions(-) create mode 100644 client/src/errorUtils.ts diff --git a/client/src/auth.ts b/client/src/auth.ts index 5ae7f16..e27dca1 100644 --- a/client/src/auth.ts +++ b/client/src/auth.ts @@ -1,5 +1,9 @@ import { SupabaseClient, createClient } from "@supabase/supabase-js"; import { AppError, AuthResult, Logger } from "./types"; +import { + createUnauthorizedError, + createAuthenticationError, +} from "./errorUtils"; export const authenticate = async ( headers: Headers, @@ -10,8 +14,9 @@ export const authenticate = async ( const authHeader = headers.get("Authorization"); if (authHeader && authHeader.startsWith("Bearer ")) { const apiKey = authHeader.split(" ")[1]; - supabase = createClient( - (supabase as any).supabaseUrl, // cast away protected + // Reinitialize Supabase client with the API key + const authenticatedSupabase = createClient( + (supabase as any).supabaseUrl, // Cast away protected (supabase as any).supabaseKey, { global: { headers: { Authorization: apiKey } }, @@ -23,36 +28,66 @@ export const authenticate = async ( }, ); - const { data: userId, error: apiKeyError } = await supabase + // Call the RPC function to get the user ID + const { data: userId, error: apiKeyError } = await authenticatedSupabase .schema("keyhippo") .rpc("get_uid_for_key", { user_api_key: apiKey }); - if (apiKeyError) throw apiKeyError; + if (apiKeyError) { + throw createUnauthorizedError("Invalid API key."); + } + if (!userId) { - throw new Error(`Invalid API key: ${apiKey}`); + throw createUnauthorizedError( + "API key does not correspond to any user.", + ); } logger.info(`User authenticated: ${userId}`); - return { userId, supabase }; + return { userId, supabase: authenticatedSupabase }; } else { const { data: { user }, error, } = await supabase.auth.getUser(); - if (error) throw error; - if (!user) throw new Error("User not authenticated"); + if (error) { + throw createAuthenticationError( + "Failed to retrieve authenticated user.", + ); + } + + if (!user) { + throw createUnauthorizedError("User not authenticated."); + } logger.info(`User authenticated: ${user.id}`); return { userId: user.id, supabase }; } } catch (error) { - logger.warn( - `Authentication failed: ${error instanceof Error ? error.message : String(error)}`, - ); - throw { - _tag: "UnauthorizedError", - message: `Authentication failed: ${error instanceof Error ? error.message : String(error)}`, - } as AppError; + // Type assertion with runtime check + if ( + error && + typeof error === "object" && + "_tag" in error && + typeof (error as AppError)._tag === "string" && + "message" in error && + typeof (error as AppError).message === "string" + ) { + // If the error is already an AppError, rethrow it + throw error; + } else if (error instanceof Error) { + // Handle standard Error objects + logger.error(`Authentication failed: ${error.message}`); + throw createAuthenticationError( + "Authentication failed due to an unexpected error.", + ); + } else { + // Handle non-Error, non-AppError objects + logger.error(`Authentication failed: ${String(error)}`); + throw createAuthenticationError( + "Authentication failed due to an unknown error.", + ); + } } }; diff --git a/client/src/errorUtils.ts b/client/src/errorUtils.ts new file mode 100644 index 0000000..039f2ee --- /dev/null +++ b/client/src/errorUtils.ts @@ -0,0 +1,49 @@ +import { AppError } from "./types"; + +export const isAppError = (error: any): error is AppError => { + return ( + error && + typeof error === "object" && + "_tag" in error && + typeof error._tag === "string" && + "message" in error && + typeof error.message === "string" + ); +}; + +export const isUnauthorizedError = ( + error: any, +): error is { _tag: "UnauthorizedError"; message: string } => { + return isAppError(error) && error._tag === "UnauthorizedError"; +}; + +export const isAuthenticationError = ( + error: any, +): error is { _tag: "AuthenticationError"; message: string } => { + return isAppError(error) && error._tag === "AuthenticationError"; +}; + +export const createUnauthorizedError = (message: string): AppError => ({ + _tag: "UnauthorizedError", + message, +}); + +export const createAuthenticationError = (message: string): AppError => ({ + _tag: "AuthenticationError", + message, +}); + +export const createDatabaseError = (message: string): AppError => ({ + _tag: "DatabaseError", + message, +}); + +export const createValidationError = (message: string): AppError => ({ + _tag: "ValidationError", + message, +}); + +export const createNetworkError = (message: string): AppError => ({ + _tag: "NetworkError", + message, +}); diff --git a/client/src/types.ts b/client/src/types.ts index 4eee143..eee8593 100644 --- a/client/src/types.ts +++ b/client/src/types.ts @@ -57,7 +57,8 @@ export type AppError = | { _tag: "DatabaseError"; message: string } | { _tag: "UnauthorizedError"; message: string } | { _tag: "ValidationError"; message: string } - | { _tag: "NetworkError"; message: string }; + | { _tag: "NetworkError"; message: string } + | { _tag: "AuthenticationError"; message: string }; export type AuthResult = { userId: string; diff --git a/client/tests/keyhippo-client.test.ts b/client/tests/keyhippo-client.test.ts index 0b6c44d..a1ce52a 100644 --- a/client/tests/keyhippo-client.test.ts +++ b/client/tests/keyhippo-client.test.ts @@ -219,7 +219,7 @@ describe("KeyHippo Client Tests", () => { Authorization: `Bearer ${keyInfo.apiKey}`, }); await expect(testSetup.keyHippo.authenticate(mockHeaders)).rejects.toThrow( - "Authentication failed: Invalid API key", + "API key does not correspond to any user.", ); });