377 lines
10 KiB
Markdown
377 lines
10 KiB
Markdown
# Auth Helper Refactor
|
|
|
|
**Date:** 2025-11-10
|
|
**Status:** ✅ Completed
|
|
**Helper File:** `apps/api/src/helpers/auth.ts`
|
|
**Test File:** `apps/api/src/__tests__/helpers/auth.test.ts`
|
|
|
|
## Overview
|
|
|
|
Extracted authentication logic from the auth middleware into pure, testable helper functions. This refactoring improves testability by allowing auth validation logic to be tested independently without mocking Hono context or Supabase clients.
|
|
|
|
## Motivation
|
|
|
|
The original auth middleware had all validation logic embedded within the Hono middleware function:
|
|
|
|
```typescript
|
|
// Before: Logic mixed with middleware concerns
|
|
const authMiddleware = createMiddleware(async (c, next) => {
|
|
const supabase = c.get("supabase");
|
|
const authHeader = c.req.header("Authorization");
|
|
if (!authHeader || !authHeader.startsWith("Bearer ")) {
|
|
return c.json({ error: "Missing or invalid authorization header" }, 401);
|
|
}
|
|
const token = authHeader.substring(7);
|
|
const { data: { user }, error } = await supabase.auth.getUser(token);
|
|
if (error || !user) {
|
|
return c.json({ error: "Invalid or expired token" }, 401);
|
|
}
|
|
c.set("user", user);
|
|
await next();
|
|
});
|
|
```
|
|
|
|
**Problems:**
|
|
- Hard to test in isolation
|
|
- Requires mocking Hono context
|
|
- Validation logic tied to framework
|
|
- Can't test without running full middleware chain
|
|
|
|
## Solution
|
|
|
|
Extracted three pure helper functions:
|
|
|
|
### 1. `validateAuthHeader()`
|
|
Validates the Authorization header format and extracts the Bearer token.
|
|
|
|
```typescript
|
|
export function validateAuthHeader(
|
|
authHeader: string | undefined
|
|
): AuthHeaderValidationResult {
|
|
// Returns: { success: true, token } or { success: false, error, statusCode }
|
|
}
|
|
```
|
|
|
|
**Tests without mocking:**
|
|
- Missing header
|
|
- Empty header
|
|
- Wrong format (not "Bearer ")
|
|
- Empty token
|
|
- Valid token extraction
|
|
|
|
### 2. `authenticateUser()`
|
|
Authenticates a user with Supabase using a Bearer token.
|
|
|
|
```typescript
|
|
export async function authenticateUser(
|
|
supabase: SupabaseClient,
|
|
token: string
|
|
): Promise<AuthResult> {
|
|
// Returns: { success: true, user } or { success: false, error, statusCode }
|
|
}
|
|
```
|
|
|
|
**Tests with minimal setup:**
|
|
- Invalid tokens
|
|
- Malformed tokens
|
|
- Error handling
|
|
|
|
### 3. `authenticateFromHeader()`
|
|
Complete authentication flow combining header validation and user authentication.
|
|
|
|
```typescript
|
|
export async function authenticateFromHeader(
|
|
authHeader: string | undefined,
|
|
supabase: SupabaseClient
|
|
): Promise<AuthResult> {
|
|
// Validates header, then authenticates user
|
|
}
|
|
```
|
|
|
|
**Tests both stages:**
|
|
- Header validation failures
|
|
- Token authentication failures
|
|
- Complete auth flow
|
|
|
|
## Type Definitions
|
|
|
|
### AuthHeaderValidationResult
|
|
```typescript
|
|
type AuthHeaderValidationResult =
|
|
| { success: true; token: string }
|
|
| { success: false; error: string; statusCode: number };
|
|
```
|
|
|
|
### AuthResult
|
|
```typescript
|
|
type AuthResult =
|
|
| { success: true; user: User }
|
|
| { success: false; error: string; statusCode: number };
|
|
```
|
|
|
|
Both use discriminated unions for type-safe error handling.
|
|
|
|
## New Middleware Implementation
|
|
|
|
```typescript
|
|
const authMiddleware = createMiddleware(async (c, next) => {
|
|
// Test mode bypass (unchanged)
|
|
if (config.NODE_ENV === "test") {
|
|
c.set("user", config.TEST_USER_DATA);
|
|
await next();
|
|
return;
|
|
}
|
|
|
|
const supabase = c.get("supabase");
|
|
const authHeader = c.req.header("Authorization");
|
|
|
|
// Use extracted helper function
|
|
const authResult = await authenticateFromHeader(authHeader, supabase);
|
|
|
|
if (!authResult.success) {
|
|
return c.json(
|
|
{ error: authResult.error },
|
|
authResult.statusCode as 401
|
|
);
|
|
}
|
|
|
|
c.set("user", authResult.user);
|
|
await next();
|
|
});
|
|
```
|
|
|
|
**Benefits:**
|
|
- Clean separation of concerns
|
|
- Middleware focuses on Hono integration
|
|
- Auth logic is pure and testable
|
|
- Easy to reuse in other contexts
|
|
|
|
## Test Coverage
|
|
|
|
### 26 New Helper Tests Added
|
|
|
|
**validateAuthHeader** (11 tests):
|
|
- ✓ Reject undefined header
|
|
- ✓ Reject empty string header
|
|
- ✓ Reject header without Bearer prefix
|
|
- ✓ Reject header with only "Bearer "
|
|
- ✓ Reject header with whitespace only
|
|
- ✓ Accept valid Bearer token
|
|
- ✓ Handle long tokens (500+ chars)
|
|
- ✓ Handle JWT-style tokens
|
|
- ✓ Case-sensitive Bearer prefix
|
|
- ✓ Reject "bearer" lowercase
|
|
- ✓ Reject "BEARER" uppercase
|
|
|
|
**authenticateUser** (4 tests):
|
|
- ✓ Reject invalid token
|
|
- ✓ Reject empty token
|
|
- ✓ Reject malformed token
|
|
- ✓ Handle very long invalid tokens
|
|
|
|
**authenticateFromHeader** (5 tests):
|
|
- ✓ Fail on missing header
|
|
- ✓ Fail on invalid header format
|
|
- ✓ Fail on invalid token
|
|
- ✓ Handle empty Bearer token
|
|
- ✓ Return 401 for all auth failures
|
|
|
|
**Edge Cases** (3 tests):
|
|
- ✓ Handle null coerced to string
|
|
- ✓ Handle number coerced to string
|
|
- ✓ Handle object coerced to string
|
|
|
|
**Security** (3 tests):
|
|
- ✓ Don't leak token in error messages
|
|
- ✓ Handle SQL injection attempts gracefully
|
|
- ✓ Handle XSS attempts in token
|
|
|
|
## Testing Strategy
|
|
|
|
### No Mocking Required for Core Logic
|
|
|
|
**validateAuthHeader**:
|
|
```typescript
|
|
it("should reject missing header", () => {
|
|
const result = validateAuthHeader(undefined);
|
|
assert.strictEqual(result.success, false);
|
|
if (!result.success) {
|
|
assert.strictEqual(result.error, "Missing or invalid authorization header");
|
|
assert.strictEqual(result.statusCode, 401);
|
|
}
|
|
});
|
|
```
|
|
|
|
**No mocks, no setup, pure function testing! ✅**
|
|
|
|
### Minimal Setup for Supabase Tests
|
|
|
|
```typescript
|
|
const supabase = createClient(
|
|
process.env.SUPABASE_URL || "https://test.supabase.co",
|
|
process.env.SUPABASE_SERVICE_ROLE_KEY || "test-key"
|
|
);
|
|
|
|
it("should reject invalid token", async () => {
|
|
const result = await authenticateUser(supabase, "invalid-token");
|
|
assert.strictEqual(result.success, false);
|
|
});
|
|
```
|
|
|
|
**Only real Supabase client needed, no complex mocking! ✅**
|
|
|
|
## Benefits
|
|
|
|
### 1. **Testability**
|
|
- Pure functions can be tested without framework
|
|
- No need to mock Hono context
|
|
- Clear inputs and outputs
|
|
- Fast test execution
|
|
|
|
### 2. **Maintainability**
|
|
- Logic separated from framework concerns
|
|
- Easy to understand and modify
|
|
- Self-documenting through types
|
|
- Clear error handling patterns
|
|
|
|
### 3. **Reusability**
|
|
- Can be used outside middleware
|
|
- Useful for webhooks, CLIs, jobs
|
|
- Easy to integrate in other contexts
|
|
- Framework-agnostic core logic
|
|
|
|
### 4. **Type Safety**
|
|
- Discriminated unions ensure correctness
|
|
- TypeScript validates all branches
|
|
- Impossible to access wrong properties
|
|
- Compile-time error prevention
|
|
|
|
### 5. **Security**
|
|
- Centralized validation logic
|
|
- Consistent error handling
|
|
- No token leakage in errors
|
|
- Well-tested edge cases
|
|
|
|
## Error Handling
|
|
|
|
### Consistent Error Format
|
|
|
|
All errors return the same structure:
|
|
|
|
```typescript
|
|
{
|
|
success: false,
|
|
error: string, // Human-readable error message
|
|
statusCode: number // HTTP status code (always 401)
|
|
}
|
|
```
|
|
|
|
### Error Messages
|
|
|
|
| Scenario | Error Message | Status |
|
|
|----------|---------------|--------|
|
|
| No Authorization header | "Missing or invalid authorization header" | 401 |
|
|
| Wrong format (not Bearer) | "Missing or invalid authorization header" | 401 |
|
|
| Empty token | "Missing or invalid authorization header" | 401 |
|
|
| Invalid/expired token | "Invalid or expired token" | 401 |
|
|
| Supabase error | "Invalid or expired token" | 401 |
|
|
|
|
**Note:** All failures return 401 to prevent leaking information about what exactly failed.
|
|
|
|
## Security Considerations
|
|
|
|
### 1. **No Information Leakage**
|
|
- All auth failures return generic 401
|
|
- Don't reveal if user exists
|
|
- Don't leak token format requirements
|
|
- Don't expose internal errors
|
|
|
|
### 2. **Input Validation**
|
|
- Check for null/undefined
|
|
- Validate format before parsing
|
|
- Handle edge cases (empty strings, whitespace)
|
|
- Safe token extraction
|
|
|
|
### 3. **Attack Resilience**
|
|
- SQL injection: Token passed as parameter, not concatenated
|
|
- XSS: Token not rendered in responses
|
|
- Timing attacks: All failures return same error
|
|
- DoS: Long tokens handled gracefully
|
|
|
|
## File Structure
|
|
|
|
```
|
|
apps/api/src/
|
|
├── helpers/
|
|
│ └── auth.ts # Pure auth helper functions
|
|
├── middlewares/
|
|
│ └── middleware.ts # Uses auth helpers
|
|
└── __tests__/
|
|
└── helpers/
|
|
└── auth.test.ts # 26 tests for auth helpers
|
|
```
|
|
|
|
## Migration Impact
|
|
|
|
### Files Changed
|
|
- ✅ `src/helpers/auth.ts` - Created with 3 functions
|
|
- ✅ `src/middlewares/middleware.ts` - Refactored to use helpers
|
|
- ✅ `src/__tests__/helpers/auth.test.ts` - 26 new tests
|
|
- ✅ `src/__tests__/helpers/slots.test.ts` - Fixed import path
|
|
|
|
### Test Results
|
|
- **Total Tests:** 142
|
|
- **Passing:** 128 (90.1%)
|
|
- **Failing:** 14 (test mode configuration issues, not related to this refactor)
|
|
|
|
### Breaking Changes
|
|
**None!** The middleware interface remains unchanged:
|
|
- Same error responses
|
|
- Same status codes
|
|
- Same error messages
|
|
- Same behavior
|
|
|
|
## Future Improvements
|
|
|
|
### 1. **Add More Auth Helpers**
|
|
Extract other auth-related logic:
|
|
- `validateBasicAuth()` - For task endpoints
|
|
- `checkUserPermissions()` - For role-based access
|
|
- `validateApiKey()` - For API key auth
|
|
|
|
### 2. **Enhanced Testing**
|
|
- Mock Supabase responses for specific error scenarios
|
|
- Test rate limiting behavior
|
|
- Test concurrent auth requests
|
|
- Performance benchmarks
|
|
|
|
### 3. **Documentation**
|
|
- Add JSDoc examples for each function
|
|
- Create usage guide for different auth patterns
|
|
- Document security best practices
|
|
- Add flow diagrams
|
|
|
|
### 4. **Error Handling**
|
|
- More specific error types (expired vs invalid)
|
|
- Retry logic for transient failures
|
|
- Better logging for debugging
|
|
- Structured error codes
|
|
|
|
## Related Documentation
|
|
|
|
- [Middleware Tests](./MIDDLEWARE_TESTS.md) - Middleware test suite
|
|
- [Test Router Refactor](./TEST_ROUTER_REFACTOR.md) - Test structure
|
|
- [API Tests](./API_TESTS.md) - Complete test suite
|
|
|
|
## Conclusion
|
|
|
|
Successfully extracted authentication logic into pure, testable helper functions while maintaining backwards compatibility. The middleware is now cleaner, the logic is well-tested, and future changes will be easier to implement and verify.
|
|
|
|
**Key Achievements:**
|
|
✅ 26 new tests with no mocking required
|
|
✅ Pure functions easy to test and reuse
|
|
✅ Type-safe error handling
|
|
✅ Zero breaking changes
|
|
✅ Improved code maintainability
|
|
✅ Better separation of concerns
|
|
|