fix: auth refresh token
This commit is contained in:
@@ -29,6 +29,7 @@ export class AuthManager {
|
||||
private oauthService: OAuthService;
|
||||
private supabaseClient: SupabaseAuthClient;
|
||||
private organizationService?: OrganizationService;
|
||||
private logger = getLogger('AuthManager');
|
||||
|
||||
private constructor(config?: Partial<AuthConfig>) {
|
||||
this.credentialStore = CredentialStore.getInstance(config);
|
||||
@@ -78,8 +79,39 @@ export class AuthManager {
|
||||
|
||||
/**
|
||||
* Get stored authentication credentials
|
||||
* Automatically refreshes the token if expired
|
||||
*/
|
||||
getCredentials(): AuthCredentials | null {
|
||||
async getCredentials(): Promise<AuthCredentials | null> {
|
||||
const credentials = this.credentialStore.getCredentials();
|
||||
|
||||
// If credentials exist but are expired, try to refresh
|
||||
if (!credentials) {
|
||||
const expiredCredentials = this.credentialStore.getCredentials({
|
||||
allowExpired: true
|
||||
});
|
||||
|
||||
// Only attempt refresh if we have expired credentials with a refresh token
|
||||
if (expiredCredentials && expiredCredentials.refreshToken) {
|
||||
try {
|
||||
this.logger.info('Token expired, attempting automatic refresh...');
|
||||
return await this.refreshToken();
|
||||
} catch (error) {
|
||||
this.logger.warn('Automatic token refresh failed:', error);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
return credentials;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get stored authentication credentials (synchronous version)
|
||||
* Does not attempt automatic refresh
|
||||
*/
|
||||
getCredentialsSync(): AuthCredentials | null {
|
||||
return this.credentialStore.getCredentials();
|
||||
}
|
||||
|
||||
@@ -171,8 +203,8 @@ export class AuthManager {
|
||||
/**
|
||||
* Get the current user context (org/brief selection)
|
||||
*/
|
||||
getContext(): UserContext | null {
|
||||
const credentials = this.getCredentials();
|
||||
async getContext(): Promise<UserContext | null> {
|
||||
const credentials = await this.getCredentials();
|
||||
return credentials?.selectedContext || null;
|
||||
}
|
||||
|
||||
@@ -180,7 +212,7 @@ export class AuthManager {
|
||||
* Update the user context (org/brief selection)
|
||||
*/
|
||||
async updateContext(context: Partial<UserContext>): Promise<void> {
|
||||
const credentials = this.getCredentials();
|
||||
const credentials = await this.getCredentials();
|
||||
if (!credentials) {
|
||||
throw new AuthenticationError('Not authenticated', 'NOT_AUTHENTICATED');
|
||||
}
|
||||
@@ -206,7 +238,7 @@ export class AuthManager {
|
||||
* Clear the user context
|
||||
*/
|
||||
async clearContext(): Promise<void> {
|
||||
const credentials = this.getCredentials();
|
||||
const credentials = await this.getCredentials();
|
||||
if (!credentials) {
|
||||
throw new AuthenticationError('Not authenticated', 'NOT_AUTHENTICATED');
|
||||
}
|
||||
@@ -223,7 +255,7 @@ export class AuthManager {
|
||||
private async getOrganizationService(): Promise<OrganizationService> {
|
||||
if (!this.organizationService) {
|
||||
// First check if we have credentials with a token
|
||||
const credentials = this.getCredentials();
|
||||
const credentials = await this.getCredentials();
|
||||
if (!credentials || !credentials.token) {
|
||||
throw new AuthenticationError('Not authenticated', 'NOT_AUTHENTICATED');
|
||||
}
|
||||
|
||||
289
packages/tm-core/src/auth/credential-store.spec.ts
Normal file
289
packages/tm-core/src/auth/credential-store.spec.ts
Normal file
@@ -0,0 +1,289 @@
|
||||
/**
|
||||
* @fileoverview Unit tests for CredentialStore token expiration handling
|
||||
*/
|
||||
|
||||
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
|
||||
import fs from 'fs';
|
||||
import path from 'path';
|
||||
import os from 'os';
|
||||
import { CredentialStore } from './credential-store';
|
||||
import type { AuthCredentials } from './types';
|
||||
|
||||
describe('CredentialStore - Token Expiration', () => {
|
||||
let credentialStore: CredentialStore;
|
||||
let tmpDir: string;
|
||||
let authFile: string;
|
||||
|
||||
beforeEach(() => {
|
||||
// Create temp directory for test credentials
|
||||
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'tm-cred-test-'));
|
||||
authFile = path.join(tmpDir, 'auth.json');
|
||||
|
||||
// Create instance with test config
|
||||
CredentialStore.resetInstance();
|
||||
credentialStore = CredentialStore.getInstance({
|
||||
configDir: tmpDir,
|
||||
configFile: authFile
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
// Clean up
|
||||
try {
|
||||
if (fs.existsSync(tmpDir)) {
|
||||
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||
}
|
||||
} catch {
|
||||
// Ignore cleanup errors
|
||||
}
|
||||
CredentialStore.resetInstance();
|
||||
});
|
||||
|
||||
describe('Expiration Detection', () => {
|
||||
it('should return null for expired token', () => {
|
||||
const expiredCredentials: AuthCredentials = {
|
||||
token: 'expired-token',
|
||||
refreshToken: 'refresh-token',
|
||||
userId: 'test-user',
|
||||
email: 'test@example.com',
|
||||
expiresAt: new Date(Date.now() - 60000).toISOString(), // 1 minute ago
|
||||
savedAt: new Date().toISOString()
|
||||
};
|
||||
|
||||
credentialStore.saveCredentials(expiredCredentials);
|
||||
|
||||
const retrieved = credentialStore.getCredentials();
|
||||
|
||||
expect(retrieved).toBeNull();
|
||||
});
|
||||
|
||||
it('should return credentials for valid token', () => {
|
||||
const validCredentials: AuthCredentials = {
|
||||
token: 'valid-token',
|
||||
refreshToken: 'refresh-token',
|
||||
userId: 'test-user',
|
||||
email: 'test@example.com',
|
||||
expiresAt: new Date(Date.now() + 3600000).toISOString(), // 1 hour from now
|
||||
savedAt: new Date().toISOString()
|
||||
};
|
||||
|
||||
credentialStore.saveCredentials(validCredentials);
|
||||
|
||||
const retrieved = credentialStore.getCredentials();
|
||||
|
||||
expect(retrieved).not.toBeNull();
|
||||
expect(retrieved?.token).toBe('valid-token');
|
||||
});
|
||||
|
||||
it('should return expired token when allowExpired is true', () => {
|
||||
const expiredCredentials: AuthCredentials = {
|
||||
token: 'expired-token',
|
||||
refreshToken: 'refresh-token',
|
||||
userId: 'test-user',
|
||||
email: 'test@example.com',
|
||||
expiresAt: new Date(Date.now() - 60000).toISOString(),
|
||||
savedAt: new Date().toISOString()
|
||||
};
|
||||
|
||||
credentialStore.saveCredentials(expiredCredentials);
|
||||
|
||||
const retrieved = credentialStore.getCredentials({ allowExpired: true });
|
||||
|
||||
expect(retrieved).not.toBeNull();
|
||||
expect(retrieved?.token).toBe('expired-token');
|
||||
});
|
||||
});
|
||||
|
||||
describe('Clock Skew Tolerance', () => {
|
||||
it('should reject token expiring within 30-second buffer', () => {
|
||||
// Token expires in 15 seconds (within 30-second buffer)
|
||||
const almostExpiredCredentials: AuthCredentials = {
|
||||
token: 'almost-expired-token',
|
||||
refreshToken: 'refresh-token',
|
||||
userId: 'test-user',
|
||||
email: 'test@example.com',
|
||||
expiresAt: new Date(Date.now() + 15000).toISOString(),
|
||||
savedAt: new Date().toISOString()
|
||||
};
|
||||
|
||||
credentialStore.saveCredentials(almostExpiredCredentials);
|
||||
|
||||
const retrieved = credentialStore.getCredentials();
|
||||
|
||||
expect(retrieved).toBeNull();
|
||||
});
|
||||
|
||||
it('should accept token expiring outside 30-second buffer', () => {
|
||||
// Token expires in 60 seconds (outside 30-second buffer)
|
||||
const validCredentials: AuthCredentials = {
|
||||
token: 'valid-token',
|
||||
refreshToken: 'refresh-token',
|
||||
userId: 'test-user',
|
||||
email: 'test@example.com',
|
||||
expiresAt: new Date(Date.now() + 60000).toISOString(),
|
||||
savedAt: new Date().toISOString()
|
||||
};
|
||||
|
||||
credentialStore.saveCredentials(validCredentials);
|
||||
|
||||
const retrieved = credentialStore.getCredentials();
|
||||
|
||||
expect(retrieved).not.toBeNull();
|
||||
expect(retrieved?.token).toBe('valid-token');
|
||||
});
|
||||
});
|
||||
|
||||
describe('Timestamp Format Handling', () => {
|
||||
it('should handle ISO string timestamps', () => {
|
||||
const credentials: AuthCredentials = {
|
||||
token: 'test-token',
|
||||
refreshToken: 'refresh-token',
|
||||
userId: 'test-user',
|
||||
email: 'test@example.com',
|
||||
expiresAt: new Date(Date.now() + 3600000).toISOString(),
|
||||
savedAt: new Date().toISOString()
|
||||
};
|
||||
|
||||
credentialStore.saveCredentials(credentials);
|
||||
|
||||
const retrieved = credentialStore.getCredentials();
|
||||
|
||||
expect(retrieved).not.toBeNull();
|
||||
expect(typeof retrieved?.expiresAt).toBe('number'); // Normalized to number
|
||||
});
|
||||
|
||||
it('should handle numeric timestamps', () => {
|
||||
const credentials: AuthCredentials = {
|
||||
token: 'test-token',
|
||||
refreshToken: 'refresh-token',
|
||||
userId: 'test-user',
|
||||
email: 'test@example.com',
|
||||
expiresAt: Date.now() + 3600000,
|
||||
savedAt: new Date().toISOString()
|
||||
};
|
||||
|
||||
credentialStore.saveCredentials(credentials);
|
||||
|
||||
const retrieved = credentialStore.getCredentials();
|
||||
|
||||
expect(retrieved).not.toBeNull();
|
||||
expect(typeof retrieved?.expiresAt).toBe('number');
|
||||
});
|
||||
|
||||
it('should return null for invalid timestamp format', () => {
|
||||
// Manually write invalid timestamp to file
|
||||
const invalidCredentials = {
|
||||
token: 'test-token',
|
||||
refreshToken: 'refresh-token',
|
||||
userId: 'test-user',
|
||||
email: 'test@example.com',
|
||||
expiresAt: 'invalid-date',
|
||||
savedAt: new Date().toISOString()
|
||||
};
|
||||
|
||||
fs.writeFileSync(authFile, JSON.stringify(invalidCredentials), {
|
||||
mode: 0o600
|
||||
});
|
||||
|
||||
const retrieved = credentialStore.getCredentials();
|
||||
|
||||
expect(retrieved).toBeNull();
|
||||
});
|
||||
|
||||
it('should return null for missing expiresAt', () => {
|
||||
const credentialsWithoutExpiry = {
|
||||
token: 'test-token',
|
||||
refreshToken: 'refresh-token',
|
||||
userId: 'test-user',
|
||||
email: 'test@example.com',
|
||||
savedAt: new Date().toISOString()
|
||||
};
|
||||
|
||||
fs.writeFileSync(authFile, JSON.stringify(credentialsWithoutExpiry), {
|
||||
mode: 0o600
|
||||
});
|
||||
|
||||
const retrieved = credentialStore.getCredentials();
|
||||
|
||||
expect(retrieved).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('Storage Persistence', () => {
|
||||
it('should persist expiresAt as ISO string', () => {
|
||||
const expiryTime = Date.now() + 3600000;
|
||||
const credentials: AuthCredentials = {
|
||||
token: 'test-token',
|
||||
refreshToken: 'refresh-token',
|
||||
userId: 'test-user',
|
||||
email: 'test@example.com',
|
||||
expiresAt: expiryTime,
|
||||
savedAt: new Date().toISOString()
|
||||
};
|
||||
|
||||
credentialStore.saveCredentials(credentials);
|
||||
|
||||
// Read raw file to verify format
|
||||
const fileContent = fs.readFileSync(authFile, 'utf-8');
|
||||
const parsed = JSON.parse(fileContent);
|
||||
|
||||
// Should be stored as ISO string
|
||||
expect(typeof parsed.expiresAt).toBe('string');
|
||||
expect(parsed.expiresAt).toMatch(/^\d{4}-\d{2}-\d{2}T/); // ISO format
|
||||
});
|
||||
|
||||
it('should normalize timestamp on retrieval', () => {
|
||||
const credentials: AuthCredentials = {
|
||||
token: 'test-token',
|
||||
refreshToken: 'refresh-token',
|
||||
userId: 'test-user',
|
||||
email: 'test@example.com',
|
||||
expiresAt: new Date(Date.now() + 3600000).toISOString(),
|
||||
savedAt: new Date().toISOString()
|
||||
};
|
||||
|
||||
credentialStore.saveCredentials(credentials);
|
||||
|
||||
const retrieved = credentialStore.getCredentials();
|
||||
|
||||
// Should be normalized to number for runtime use
|
||||
expect(typeof retrieved?.expiresAt).toBe('number');
|
||||
});
|
||||
});
|
||||
|
||||
describe('hasValidCredentials', () => {
|
||||
it('should return false for expired credentials', () => {
|
||||
const expiredCredentials: AuthCredentials = {
|
||||
token: 'expired-token',
|
||||
refreshToken: 'refresh-token',
|
||||
userId: 'test-user',
|
||||
email: 'test@example.com',
|
||||
expiresAt: new Date(Date.now() - 60000).toISOString(),
|
||||
savedAt: new Date().toISOString()
|
||||
};
|
||||
|
||||
credentialStore.saveCredentials(expiredCredentials);
|
||||
|
||||
expect(credentialStore.hasValidCredentials()).toBe(false);
|
||||
});
|
||||
|
||||
it('should return true for valid credentials', () => {
|
||||
const validCredentials: AuthCredentials = {
|
||||
token: 'valid-token',
|
||||
refreshToken: 'refresh-token',
|
||||
userId: 'test-user',
|
||||
email: 'test@example.com',
|
||||
expiresAt: new Date(Date.now() + 3600000).toISOString(),
|
||||
savedAt: new Date().toISOString()
|
||||
};
|
||||
|
||||
credentialStore.saveCredentials(validCredentials);
|
||||
|
||||
expect(credentialStore.hasValidCredentials()).toBe(true);
|
||||
});
|
||||
|
||||
it('should return false when no credentials exist', () => {
|
||||
expect(credentialStore.hasValidCredentials()).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -47,8 +47,8 @@ export class SupabaseTaskRepository {
|
||||
* Gets the current brief ID from auth context
|
||||
* @throws {Error} If no brief is selected
|
||||
*/
|
||||
private getBriefIdOrThrow(): string {
|
||||
const context = this.authManager.getContext();
|
||||
private async getBriefIdOrThrow(): Promise<string> {
|
||||
const context = await this.authManager.getContext();
|
||||
if (!context?.briefId) {
|
||||
throw new Error(
|
||||
'No brief selected. Please select a brief first using: tm context brief'
|
||||
@@ -61,7 +61,7 @@ export class SupabaseTaskRepository {
|
||||
_projectId?: string,
|
||||
options?: LoadTasksOptions
|
||||
): Promise<Task[]> {
|
||||
const briefId = this.getBriefIdOrThrow();
|
||||
const briefId = await this.getBriefIdOrThrow();
|
||||
|
||||
// Build query with filters
|
||||
let query = this.supabase
|
||||
@@ -114,7 +114,7 @@ export class SupabaseTaskRepository {
|
||||
}
|
||||
|
||||
async getTask(_projectId: string, taskId: string): Promise<Task | null> {
|
||||
const briefId = this.getBriefIdOrThrow();
|
||||
const briefId = await this.getBriefIdOrThrow();
|
||||
|
||||
const { data, error } = await this.supabase
|
||||
.from('tasks')
|
||||
@@ -157,7 +157,7 @@ export class SupabaseTaskRepository {
|
||||
taskId: string,
|
||||
updates: Partial<Task>
|
||||
): Promise<Task> {
|
||||
const briefId = this.getBriefIdOrThrow();
|
||||
const briefId = await this.getBriefIdOrThrow();
|
||||
|
||||
// Validate updates using Zod schema
|
||||
try {
|
||||
|
||||
@@ -105,7 +105,7 @@ export class ExportService {
|
||||
}
|
||||
|
||||
// Get current context
|
||||
const context = this.authManager.getContext();
|
||||
const context = await this.authManager.getContext();
|
||||
|
||||
// Determine org and brief IDs
|
||||
let orgId = options.orgId || context?.orgId;
|
||||
@@ -232,7 +232,7 @@ export class ExportService {
|
||||
hasBrief: boolean;
|
||||
context: UserContext | null;
|
||||
}> {
|
||||
const context = this.authManager.getContext();
|
||||
const context = await this.authManager.getContext();
|
||||
|
||||
return {
|
||||
hasOrg: !!context?.orgId,
|
||||
@@ -379,7 +379,7 @@ export class ExportService {
|
||||
};
|
||||
|
||||
// Get auth token
|
||||
const credentials = this.authManager.getCredentials();
|
||||
const credentials = await this.authManager.getCredentials();
|
||||
if (!credentials || !credentials.token) {
|
||||
throw new Error('Not authenticated');
|
||||
}
|
||||
|
||||
@@ -119,7 +119,7 @@ export class ApiStorage implements IStorage {
|
||||
private async loadTagsIntoCache(): Promise<void> {
|
||||
try {
|
||||
const authManager = AuthManager.getInstance();
|
||||
const context = authManager.getContext();
|
||||
const context = await authManager.getContext();
|
||||
|
||||
// If we have a selected brief, create a virtual "tag" for it
|
||||
if (context?.briefId) {
|
||||
@@ -152,7 +152,7 @@ export class ApiStorage implements IStorage {
|
||||
|
||||
try {
|
||||
const authManager = AuthManager.getInstance();
|
||||
const context = authManager.getContext();
|
||||
const context = await authManager.getContext();
|
||||
|
||||
// If no brief is selected in context, throw an error
|
||||
if (!context?.briefId) {
|
||||
@@ -318,7 +318,7 @@ export class ApiStorage implements IStorage {
|
||||
|
||||
try {
|
||||
const authManager = AuthManager.getInstance();
|
||||
const context = authManager.getContext();
|
||||
const context = await authManager.getContext();
|
||||
|
||||
// In our API-based system, we only have one "tag" at a time - the current brief
|
||||
if (context?.briefId) {
|
||||
|
||||
@@ -72,8 +72,8 @@ export class StorageFactory {
|
||||
{ storageType: 'api', missing }
|
||||
);
|
||||
}
|
||||
// Use auth token from AuthManager
|
||||
const credentials = authManager.getCredentials();
|
||||
// Use auth token from AuthManager (synchronous - no auto-refresh here)
|
||||
const credentials = authManager.getCredentialsSync();
|
||||
if (credentials) {
|
||||
// Merge with existing storage config, ensuring required fields
|
||||
const nextStorage: StorageSettings = {
|
||||
@@ -103,7 +103,7 @@ export class StorageFactory {
|
||||
|
||||
// Then check if authenticated via AuthManager
|
||||
if (authManager.isAuthenticated()) {
|
||||
const credentials = authManager.getCredentials();
|
||||
const credentials = authManager.getCredentialsSync();
|
||||
if (credentials) {
|
||||
// Configure API storage with auth credentials
|
||||
const nextStorage: StorageSettings = {
|
||||
|
||||
Reference in New Issue
Block a user