Compare commits

...

4 Commits

Author SHA1 Message Date
Ralph Khreish
93097dfeb5 chore: apply requested changes p3 2025-10-15 17:17:34 +02:00
Ralph Khreish
fa2b63de40 chore: apply requested changes p2 2025-10-15 17:01:56 +02:00
Ralph Khreish
b11dacece9 chore: apply requested PR changes and fix CI 2025-10-15 16:48:10 +02:00
Ralph Khreish
01cbbe97f2 fix: auth refresh 2025-10-15 15:28:32 +02:00
14 changed files with 239 additions and 308 deletions

View File

@@ -0,0 +1,5 @@
---
"task-master-ai": patch
---
Improve auth token refresh flow

View File

@@ -143,7 +143,7 @@ export class AuthCommand extends Command {
*/
private async executeStatus(): Promise<void> {
try {
const result = await this.displayStatus();
const result = this.displayStatus();
this.setLastResult(result);
} catch (error: any) {
this.handleError(error);
@@ -171,8 +171,8 @@ export class AuthCommand extends Command {
/**
* Display authentication status
*/
private async displayStatus(): Promise<AuthResult> {
const credentials = await this.authManager.getCredentials();
private displayStatus(): AuthResult {
const credentials = this.authManager.getCredentials();
console.log(chalk.cyan('\n🔐 Authentication Status\n'));
@@ -325,7 +325,7 @@ export class AuthCommand extends Command {
]);
if (!continueAuth) {
const credentials = await this.authManager.getCredentials();
const credentials = this.authManager.getCredentials();
ui.displaySuccess('Using existing authentication');
if (credentials) {
@@ -490,7 +490,7 @@ export class AuthCommand extends Command {
/**
* Get current credentials (for programmatic usage)
*/
getCredentials(): Promise<AuthCredentials | null> {
getCredentials(): AuthCredentials | null {
return this.authManager.getCredentials();
}

View File

@@ -115,7 +115,7 @@ export class ContextCommand extends Command {
*/
private async executeShow(): Promise<void> {
try {
const result = await this.displayContext();
const result = this.displayContext();
this.setLastResult(result);
} catch (error: any) {
this.handleError(error);
@@ -126,7 +126,7 @@ export class ContextCommand extends Command {
/**
* Display current context
*/
private async displayContext(): Promise<ContextResult> {
private displayContext(): ContextResult {
// Check authentication first
if (!this.authManager.isAuthenticated()) {
console.log(chalk.yellow('✗ Not authenticated'));
@@ -139,7 +139,7 @@ export class ContextCommand extends Command {
};
}
const context = await this.authManager.getContext();
const context = this.authManager.getContext();
console.log(chalk.cyan('\n🌍 Workspace Context\n'));
@@ -250,7 +250,7 @@ export class ContextCommand extends Command {
]);
// Update context
await this.authManager.updateContext({
this.authManager.updateContext({
orgId: selectedOrg.id,
orgName: selectedOrg.name,
// Clear brief when changing org
@@ -263,7 +263,7 @@ export class ContextCommand extends Command {
return {
success: true,
action: 'select-org',
context: (await this.authManager.getContext()) || undefined,
context: this.authManager.getContext() || undefined,
message: `Selected organization: ${selectedOrg.name}`
};
} catch (error) {
@@ -284,7 +284,7 @@ export class ContextCommand extends Command {
}
// Check if org is selected
const context = await this.authManager.getContext();
const context = this.authManager.getContext();
if (!context?.orgId) {
ui.displayError(
'No organization selected. Run "tm context org" first.'
@@ -343,7 +343,7 @@ export class ContextCommand extends Command {
if (selectedBrief) {
// Update context with brief
const briefName = `Brief ${selectedBrief.id.slice(0, 8)}`;
await this.authManager.updateContext({
this.authManager.updateContext({
briefId: selectedBrief.id,
briefName: briefName
});
@@ -353,12 +353,12 @@ export class ContextCommand extends Command {
return {
success: true,
action: 'select-brief',
context: (await this.authManager.getContext()) || undefined,
context: this.authManager.getContext() || undefined,
message: `Selected brief: ${selectedBrief.name}`
};
} else {
// Clear brief selection
await this.authManager.updateContext({
this.authManager.updateContext({
briefId: undefined,
briefName: undefined
});
@@ -368,7 +368,7 @@ export class ContextCommand extends Command {
return {
success: true,
action: 'select-brief',
context: (await this.authManager.getContext()) || undefined,
context: this.authManager.getContext() || undefined,
message: 'Cleared brief selection'
};
}
@@ -491,7 +491,7 @@ export class ContextCommand extends Command {
// Update context: set org and brief
const briefName = `Brief ${brief.id.slice(0, 8)}`;
await this.authManager.updateContext({
this.authManager.updateContext({
orgId: brief.accountId,
orgName,
briefId: brief.id,
@@ -508,7 +508,7 @@ export class ContextCommand extends Command {
this.setLastResult({
success: true,
action: 'set',
context: (await this.authManager.getContext()) || undefined,
context: this.authManager.getContext() || undefined,
message: 'Context set from brief'
});
} catch (error: any) {
@@ -613,7 +613,7 @@ export class ContextCommand extends Command {
};
}
await this.authManager.updateContext(context);
this.authManager.updateContext(context);
ui.displaySuccess('Context updated');
// Display what was set
@@ -631,7 +631,7 @@ export class ContextCommand extends Command {
return {
success: true,
action: 'set',
context: (await this.authManager.getContext()) || undefined,
context: this.authManager.getContext() || undefined,
message: 'Context updated'
};
} catch (error) {
@@ -682,7 +682,7 @@ export class ContextCommand extends Command {
/**
* Get current context (for programmatic usage)
*/
getContext(): Promise<UserContext | null> {
getContext(): UserContext | null {
return this.authManager.getContext();
}

View File

@@ -35,7 +35,7 @@ vi.mock('./credential-store.js', () => {
}
saveCredentials() {}
clearCredentials() {}
hasValidCredentials() {
hasCredentials() {
return false;
}
}

View File

@@ -29,8 +29,6 @@ export class AuthManager {
private oauthService: OAuthService;
private supabaseClient: SupabaseAuthClient;
private organizationService?: OrganizationService;
private logger = getLogger('AuthManager');
private refreshPromise: Promise<AuthCredentials> | null = null;
private constructor(config?: Partial<AuthConfig>) {
this.credentialStore = CredentialStore.getInstance(config);
@@ -83,60 +81,10 @@ export class AuthManager {
/**
* Get stored authentication credentials
* Automatically refreshes the token if expired
* Returns credentials as-is (even if expired). Refresh must be triggered explicitly
* via refreshToken() or will occur automatically when using the Supabase client for API calls.
*/
async getCredentials(): Promise<AuthCredentials | null> {
const credentials = this.credentialStore.getCredentials({
allowExpired: true
});
if (!credentials) {
return null;
}
// Check if credentials are expired (with 30-second clock skew buffer)
const CLOCK_SKEW_MS = 30_000;
const isExpired = credentials.expiresAt
? new Date(credentials.expiresAt).getTime() <= Date.now() + CLOCK_SKEW_MS
: false;
// If expired and we have a refresh token, attempt refresh
if (isExpired && credentials.refreshToken) {
// Return existing refresh promise if one is in progress
if (this.refreshPromise) {
try {
return await this.refreshPromise;
} catch {
return null;
}
}
try {
this.logger.info('Token expired, attempting automatic refresh...');
this.refreshPromise = this.refreshToken();
const result = await this.refreshPromise;
return result;
} catch (error) {
this.logger.warn('Automatic token refresh failed:', error);
return null;
} finally {
this.refreshPromise = null;
}
}
// Return null if expired and no refresh token
if (isExpired) {
return null;
}
return credentials;
}
/**
* Get stored authentication credentials (synchronous version)
* Does not attempt automatic refresh
*/
getCredentialsSync(): AuthCredentials | null {
getCredentials(): AuthCredentials | null {
return this.credentialStore.getCredentials();
}
@@ -219,25 +167,26 @@ export class AuthManager {
}
/**
* Check if authenticated
* Check if authenticated (credentials exist, regardless of expiration)
* @returns true if credentials are stored, including expired credentials
*/
isAuthenticated(): boolean {
return this.credentialStore.hasValidCredentials();
return this.credentialStore.hasCredentials();
}
/**
* Get the current user context (org/brief selection)
*/
async getContext(): Promise<UserContext | null> {
const credentials = await this.getCredentials();
getContext(): UserContext | null {
const credentials = this.getCredentials();
return credentials?.selectedContext || null;
}
/**
* Update the user context (org/brief selection)
*/
async updateContext(context: Partial<UserContext>): Promise<void> {
const credentials = await this.getCredentials();
updateContext(context: Partial<UserContext>): void {
const credentials = this.getCredentials();
if (!credentials) {
throw new AuthenticationError('Not authenticated', 'NOT_AUTHENTICATED');
}
@@ -262,8 +211,8 @@ export class AuthManager {
/**
* Clear the user context
*/
async clearContext(): Promise<void> {
const credentials = await this.getCredentials();
clearContext(): void {
const credentials = this.getCredentials();
if (!credentials) {
throw new AuthenticationError('Not authenticated', 'NOT_AUTHENTICATED');
}
@@ -280,7 +229,7 @@ export class AuthManager {
private async getOrganizationService(): Promise<OrganizationService> {
if (!this.organizationService) {
// First check if we have credentials with a token
const credentials = await this.getCredentials();
const credentials = this.getCredentials();
if (!credentials || !credentials.token) {
throw new AuthenticationError('Not authenticated', 'NOT_AUTHENTICATED');
}

View File

@@ -52,7 +52,7 @@ describe('CredentialStore - Token Expiration', () => {
credentialStore.saveCredentials(expiredCredentials);
const retrieved = credentialStore.getCredentials();
const retrieved = credentialStore.getCredentials({ allowExpired: false });
expect(retrieved).toBeNull();
});
@@ -69,7 +69,7 @@ describe('CredentialStore - Token Expiration', () => {
credentialStore.saveCredentials(validCredentials);
const retrieved = credentialStore.getCredentials();
const retrieved = credentialStore.getCredentials({ allowExpired: false });
expect(retrieved).not.toBeNull();
expect(retrieved?.token).toBe('valid-token');
@@ -92,6 +92,25 @@ describe('CredentialStore - Token Expiration', () => {
expect(retrieved).not.toBeNull();
expect(retrieved?.token).toBe('expired-token');
});
it('should return expired token by default (allowExpired defaults to true)', () => {
const expiredCredentials: AuthCredentials = {
token: 'expired-token-default',
refreshToken: 'refresh-token',
userId: 'test-user',
email: 'test@example.com',
expiresAt: new Date(Date.now() - 60000).toISOString(),
savedAt: new Date().toISOString()
};
credentialStore.saveCredentials(expiredCredentials);
// Call without options - should default to allowExpired: true
const retrieved = credentialStore.getCredentials();
expect(retrieved).not.toBeNull();
expect(retrieved?.token).toBe('expired-token-default');
});
});
describe('Clock Skew Tolerance', () => {
@@ -108,7 +127,7 @@ describe('CredentialStore - Token Expiration', () => {
credentialStore.saveCredentials(almostExpiredCredentials);
const retrieved = credentialStore.getCredentials();
const retrieved = credentialStore.getCredentials({ allowExpired: false });
expect(retrieved).toBeNull();
});
@@ -126,7 +145,7 @@ describe('CredentialStore - Token Expiration', () => {
credentialStore.saveCredentials(validCredentials);
const retrieved = credentialStore.getCredentials();
const retrieved = credentialStore.getCredentials({ allowExpired: false });
expect(retrieved).not.toBeNull();
expect(retrieved?.token).toBe('valid-token');
@@ -146,7 +165,7 @@ describe('CredentialStore - Token Expiration', () => {
credentialStore.saveCredentials(credentials);
const retrieved = credentialStore.getCredentials();
const retrieved = credentialStore.getCredentials({ allowExpired: false });
expect(retrieved).not.toBeNull();
expect(typeof retrieved?.expiresAt).toBe('number'); // Normalized to number
@@ -164,7 +183,7 @@ describe('CredentialStore - Token Expiration', () => {
credentialStore.saveCredentials(credentials);
const retrieved = credentialStore.getCredentials();
const retrieved = credentialStore.getCredentials({ allowExpired: false });
expect(retrieved).not.toBeNull();
expect(typeof retrieved?.expiresAt).toBe('number');
@@ -185,7 +204,7 @@ describe('CredentialStore - Token Expiration', () => {
mode: 0o600
});
const retrieved = credentialStore.getCredentials();
const retrieved = credentialStore.getCredentials({ allowExpired: false });
expect(retrieved).toBeNull();
});
@@ -203,7 +222,7 @@ describe('CredentialStore - Token Expiration', () => {
mode: 0o600
});
const retrieved = credentialStore.getCredentials();
const retrieved = credentialStore.getCredentials({ allowExpired: false });
expect(retrieved).toBeNull();
});
@@ -244,15 +263,15 @@ describe('CredentialStore - Token Expiration', () => {
credentialStore.saveCredentials(credentials);
const retrieved = credentialStore.getCredentials();
const retrieved = credentialStore.getCredentials({ allowExpired: false });
// Should be normalized to number for runtime use
expect(typeof retrieved?.expiresAt).toBe('number');
});
});
describe('hasValidCredentials', () => {
it('should return false for expired credentials', () => {
describe('hasCredentials', () => {
it('should return true for expired credentials', () => {
const expiredCredentials: AuthCredentials = {
token: 'expired-token',
refreshToken: 'refresh-token',
@@ -264,7 +283,7 @@ describe('CredentialStore - Token Expiration', () => {
credentialStore.saveCredentials(expiredCredentials);
expect(credentialStore.hasValidCredentials()).toBe(false);
expect(credentialStore.hasCredentials()).toBe(true);
});
it('should return true for valid credentials', () => {
@@ -279,11 +298,11 @@ describe('CredentialStore - Token Expiration', () => {
credentialStore.saveCredentials(validCredentials);
expect(credentialStore.hasValidCredentials()).toBe(true);
expect(credentialStore.hasCredentials()).toBe(true);
});
it('should return false when no credentials exist', () => {
expect(credentialStore.hasValidCredentials()).toBe(false);
expect(credentialStore.hasCredentials()).toBe(false);
});
});
});

View File

@@ -197,7 +197,7 @@ describe('CredentialStore', () => {
JSON.stringify(mockCredentials)
);
const result = store.getCredentials();
const result = store.getCredentials({ allowExpired: false });
expect(result).toBeNull();
expect(mockLogger.warn).toHaveBeenCalledWith(
@@ -226,6 +226,31 @@ describe('CredentialStore', () => {
expect(result).not.toBeNull();
expect(result?.token).toBe('expired-token');
});
it('should return expired tokens by default (allowExpired defaults to true)', () => {
const expiredTimestamp = Date.now() - 3600000; // 1 hour ago
const mockCredentials = {
token: 'expired-token-default',
userId: 'user-expired',
expiresAt: expiredTimestamp,
tokenType: 'standard',
savedAt: new Date().toISOString()
};
vi.mocked(fs.existsSync).mockReturnValue(true);
vi.mocked(fs.readFileSync).mockReturnValue(
JSON.stringify(mockCredentials)
);
// Call without options - should default to allowExpired: true
const result = store.getCredentials();
expect(result).not.toBeNull();
expect(result?.token).toBe('expired-token-default');
expect(mockLogger.warn).not.toHaveBeenCalledWith(
expect.stringContaining('Authentication token has expired')
);
});
});
describe('saveCredentials with timestamp normalization', () => {
@@ -451,7 +476,7 @@ describe('CredentialStore', () => {
});
});
describe('hasValidCredentials', () => {
describe('hasCredentials', () => {
it('should return true when valid unexpired credentials exist', () => {
const futureDate = new Date(Date.now() + 3600000); // 1 hour from now
const credentials = {
@@ -465,10 +490,10 @@ describe('CredentialStore', () => {
vi.mocked(fs.existsSync).mockReturnValue(true);
vi.mocked(fs.readFileSync).mockReturnValue(JSON.stringify(credentials));
expect(store.hasValidCredentials()).toBe(true);
expect(store.hasCredentials()).toBe(true);
});
it('should return false when credentials are expired', () => {
it('should return true when credentials are expired', () => {
const pastDate = new Date(Date.now() - 3600000); // 1 hour ago
const credentials = {
token: 'expired-token',
@@ -481,13 +506,13 @@ describe('CredentialStore', () => {
vi.mocked(fs.existsSync).mockReturnValue(true);
vi.mocked(fs.readFileSync).mockReturnValue(JSON.stringify(credentials));
expect(store.hasValidCredentials()).toBe(false);
expect(store.hasCredentials()).toBe(true);
});
it('should return false when no credentials exist', () => {
vi.mocked(fs.existsSync).mockReturnValue(false);
expect(store.hasValidCredentials()).toBe(false);
expect(store.hasCredentials()).toBe(false);
});
it('should return false when file contains invalid JSON', () => {
@@ -495,7 +520,7 @@ describe('CredentialStore', () => {
vi.mocked(fs.readFileSync).mockReturnValue('invalid json {');
vi.mocked(fs.renameSync).mockImplementation(() => undefined);
expect(store.hasValidCredentials()).toBe(false);
expect(store.hasCredentials()).toBe(false);
});
it('should return false for credentials without expiry', () => {
@@ -510,7 +535,7 @@ describe('CredentialStore', () => {
vi.mocked(fs.readFileSync).mockReturnValue(JSON.stringify(credentials));
// Credentials without expiry are considered invalid
expect(store.hasValidCredentials()).toBe(false);
expect(store.hasCredentials()).toBe(false);
// Should log warning about missing expiration
expect(mockLogger.warn).toHaveBeenCalledWith(
@@ -518,14 +543,14 @@ describe('CredentialStore', () => {
);
});
it('should use allowExpired=false by default', () => {
it('should use allowExpired=true', () => {
// Spy on getCredentials to verify it's called with correct params
const getCredentialsSpy = vi.spyOn(store, 'getCredentials');
vi.mocked(fs.existsSync).mockReturnValue(false);
store.hasValidCredentials();
store.hasCredentials();
expect(getCredentialsSpy).toHaveBeenCalledWith({ allowExpired: false });
expect(getCredentialsSpy).toHaveBeenCalledWith({ allowExpired: true });
});
});

View File

@@ -54,9 +54,12 @@ export class CredentialStore {
/**
* Get stored authentication credentials
* @param options.allowExpired - Whether to return expired credentials (default: true)
* @returns AuthCredentials with expiresAt as number (milliseconds) for runtime use
*/
getCredentials(options?: { allowExpired?: boolean }): AuthCredentials | null {
getCredentials({
allowExpired = true
}: { allowExpired?: boolean } = {}): AuthCredentials | null {
try {
if (!fs.existsSync(this.config.configFile)) {
return null;
@@ -90,7 +93,6 @@ export class CredentialStore {
// Check if the token has expired (with clock skew tolerance)
const now = Date.now();
const allowExpired = options?.allowExpired ?? false;
if (now >= expiresAtMs - this.CLOCK_SKEW_MS && !allowExpired) {
this.logger.warn(
'Authentication token has expired or is about to expire',
@@ -103,7 +105,7 @@ export class CredentialStore {
return null;
}
// Return valid token
// Return credentials (even if expired) to enable refresh flows
return authData;
} catch (error) {
this.logger.error(
@@ -199,10 +201,11 @@ export class CredentialStore {
}
/**
* Check if credentials exist and are valid
* Check if credentials exist (regardless of expiration status)
* @returns true if credentials are stored, including expired credentials
*/
hasValidCredentials(): boolean {
const credentials = this.getCredentials({ allowExpired: false });
hasCredentials(): boolean {
const credentials = this.getCredentials({ allowExpired: true });
return credentials !== null;
}

View File

@@ -281,15 +281,26 @@ export class OAuthService {
// Exchange code for session using PKCE
const session = await this.supabaseClient.exchangeCodeForSession(code);
// Calculate expiration - can be overridden with TM_TOKEN_EXPIRY_MINUTES
let expiresAt: string | undefined;
const tokenExpiryMinutes = process.env.TM_TOKEN_EXPIRY_MINUTES;
if (tokenExpiryMinutes) {
const minutes = parseInt(tokenExpiryMinutes);
expiresAt = new Date(Date.now() + minutes * 60 * 1000).toISOString();
this.logger.warn(`Token expiry overridden to ${minutes} minute(s)`);
} else {
expiresAt = session.expires_at
? new Date(session.expires_at * 1000).toISOString()
: undefined;
}
// Save authentication data
const authData: AuthCredentials = {
token: session.access_token,
refreshToken: session.refresh_token,
userId: session.user.id,
email: session.user.email,
expiresAt: session.expires_at
? new Date(session.expires_at * 1000).toISOString()
: undefined,
expiresAt,
tokenType: 'standard',
savedAt: new Date().toISOString()
};
@@ -340,10 +351,18 @@ export class OAuthService {
// Get user info from the session
const user = await this.supabaseClient.getUser();
// Calculate expiration time
const expiresAt = expiresIn
? new Date(Date.now() + parseInt(expiresIn) * 1000).toISOString()
: undefined;
// Calculate expiration time - can be overridden with TM_TOKEN_EXPIRY_MINUTES
let expiresAt: string | undefined;
const tokenExpiryMinutes = process.env.TM_TOKEN_EXPIRY_MINUTES;
if (tokenExpiryMinutes) {
const minutes = parseInt(tokenExpiryMinutes);
expiresAt = new Date(Date.now() + minutes * 60 * 1000).toISOString();
this.logger.warn(`Token expiry overridden to ${minutes} minute(s)`);
} else {
expiresAt = expiresIn
? new Date(Date.now() + parseInt(expiresIn) * 1000).toISOString()
: undefined;
}
// Save authentication data
const authData: AuthCredentials = {
@@ -351,7 +370,7 @@ export class OAuthService {
refreshToken: refreshToken || undefined,
userId: user?.id || 'unknown',
email: user?.email,
expiresAt: expiresAt,
expiresAt,
tokenType: 'standard',
savedAt: new Date().toISOString()
};

View File

@@ -98,11 +98,11 @@ export class SupabaseSessionStorage implements SupportedStorage {
// Only handle Supabase session keys
if (key === STORAGE_KEY || key.includes('auth-token')) {
try {
this.logger.info('Supabase called setItem - storing refreshed session');
// Parse the session and update our credentials
const sessionUpdates = this.parseSessionToCredentials(value);
const existingCredentials = this.store.getCredentials({
allowExpired: true
});
const existingCredentials = this.store.getCredentials();
if (sessionUpdates.token) {
const updatedCredentials: AuthCredentials = {
@@ -113,6 +113,9 @@ export class SupabaseSessionStorage implements SupportedStorage {
} as AuthCredentials;
this.store.saveCredentials(updatedCredentials);
this.logger.info(
'Successfully saved refreshed credentials from Supabase'
);
}
} catch (error) {
this.logger.error('Error setting session:', error);

View File

@@ -17,10 +17,11 @@ export class SupabaseAuthClient {
private client: SupabaseJSClient | null = null;
private sessionStorage: SupabaseSessionStorage;
private logger = getLogger('SupabaseAuthClient');
private credentialStore: CredentialStore;
constructor() {
const credentialStore = CredentialStore.getInstance();
this.sessionStorage = new SupabaseSessionStorage(credentialStore);
this.credentialStore = CredentialStore.getInstance();
this.sessionStorage = new SupabaseSessionStorage(this.credentialStore);
}
/**

View File

@@ -73,7 +73,7 @@ export class StorageFactory {
);
}
// Use auth token from AuthManager (synchronous - no auto-refresh here)
const credentials = authManager.getCredentialsSync();
const credentials = authManager.getCredentials();
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.getCredentialsSync();
const credentials = authManager.getCredentials();
if (credentials) {
// Configure API storage with auth credentials
const nextStorage: StorageSettings = {

View File

@@ -50,7 +50,7 @@ describe('AuthManager Token Refresh', () => {
}
});
it('should not make concurrent refresh requests', async () => {
it('should return expired credentials to enable refresh flows', () => {
// Set up expired credentials with refresh token
const expiredCredentials: AuthCredentials = {
token: 'expired_access_token',
@@ -63,50 +63,16 @@ describe('AuthManager Token Refresh', () => {
credentialStore.saveCredentials(expiredCredentials);
// Mock the refreshToken method to track calls
const refreshTokenSpy = vi.spyOn(authManager as any, 'refreshToken');
const mockSession: Session = {
access_token: 'new_access_token',
refresh_token: 'new_refresh_token',
expires_at: Math.floor(Date.now() / 1000) + 3600,
user: {
id: 'test-user-id',
email: 'test@example.com',
app_metadata: {},
user_metadata: {},
aud: 'authenticated',
created_at: new Date().toISOString()
}
};
// Get credentials should return them even if expired
// Refresh will be handled by explicit calls or client operations
const credentials = authManager.getCredentials();
refreshTokenSpy.mockResolvedValue({
token: mockSession.access_token,
refreshToken: mockSession.refresh_token,
userId: mockSession.user.id,
email: mockSession.user.email,
expiresAt: new Date(mockSession.expires_at! * 1000).toISOString(),
savedAt: new Date().toISOString()
});
// Make multiple concurrent calls to getCredentials
const promises = [
authManager.getCredentials(),
authManager.getCredentials(),
authManager.getCredentials()
];
const results = await Promise.all(promises);
// Verify all calls returned the same new credentials
expect(results[0]?.token).toBe('new_access_token');
expect(results[1]?.token).toBe('new_access_token');
expect(results[2]?.token).toBe('new_access_token');
// Verify refreshToken was only called once, not three times
expect(refreshTokenSpy).toHaveBeenCalledTimes(1);
expect(credentials).not.toBeNull();
expect(credentials?.token).toBe('expired_access_token');
expect(credentials?.refreshToken).toBe('valid_refresh_token');
});
it('should return valid credentials without attempting refresh', async () => {
it('should return valid credentials', () => {
// Set up valid (non-expired) credentials
const validCredentials: AuthCredentials = {
token: 'valid_access_token',
@@ -119,17 +85,14 @@ describe('AuthManager Token Refresh', () => {
credentialStore.saveCredentials(validCredentials);
// Spy on refreshToken to ensure it's not called
const refreshTokenSpy = vi.spyOn(authManager as any, 'refreshToken');
const credentials = await authManager.getCredentials();
const credentials = authManager.getCredentials();
expect(credentials?.token).toBe('valid_access_token');
expect(refreshTokenSpy).not.toHaveBeenCalled();
});
it('should return null if credentials are expired with no refresh token', async () => {
it('should return expired credentials even without refresh token', () => {
// Set up expired credentials WITHOUT refresh token
// We still return them - it's up to the caller to handle
const expiredCredentials: AuthCredentials = {
token: 'expired_access_token',
refreshToken: undefined,
@@ -141,17 +104,19 @@ describe('AuthManager Token Refresh', () => {
credentialStore.saveCredentials(expiredCredentials);
const credentials = await authManager.getCredentials();
const credentials = authManager.getCredentials();
// Returns credentials even if expired
expect(credentials).not.toBeNull();
expect(credentials?.token).toBe('expired_access_token');
});
it('should return null if no credentials exist', () => {
const credentials = authManager.getCredentials();
expect(credentials).toBeNull();
});
it('should return null if no credentials exist', async () => {
const credentials = await authManager.getCredentials();
expect(credentials).toBeNull();
});
it('should handle refresh failures gracefully', async () => {
it('should return credentials regardless of refresh token validity', () => {
// Set up expired credentials with refresh token
const expiredCredentials: AuthCredentials = {
token: 'expired_access_token',
@@ -164,13 +129,11 @@ describe('AuthManager Token Refresh', () => {
credentialStore.saveCredentials(expiredCredentials);
// Mock refreshToken to throw an error
const refreshTokenSpy = vi.spyOn(authManager as any, 'refreshToken');
refreshTokenSpy.mockRejectedValue(new Error('Refresh failed'));
const credentials = authManager.getCredentials();
const credentials = await authManager.getCredentials();
expect(credentials).toBeNull();
expect(refreshTokenSpy).toHaveBeenCalledTimes(1);
// Returns credentials - refresh will be attempted by the client which will handle failure
expect(credentials).not.toBeNull();
expect(credentials?.token).toBe('expired_access_token');
expect(credentials?.refreshToken).toBe('invalid_refresh_token');
});
});

View File

@@ -76,7 +76,7 @@ describe('AuthManager - Token Auto-Refresh Integration', () => {
});
describe('Expired Token Detection', () => {
it('should detect expired token', async () => {
it('should return expired token for Supabase to refresh', () => {
// Set up expired credentials
const expiredCredentials: AuthCredentials = {
token: 'expired-token',
@@ -91,24 +91,15 @@ describe('AuthManager - Token Auto-Refresh Integration', () => {
authManager = AuthManager.getInstance();
// Mock the Supabase refreshSession to return new tokens
const mockRefreshSession = vi
.fn()
.mockResolvedValue(mockRefreshedSession);
vi.spyOn(
authManager['supabaseClient'],
'refreshSession'
).mockImplementation(mockRefreshSession);
// Get credentials returns them even if expired
const credentials = authManager.getCredentials();
// Get credentials should trigger refresh
const credentials = await authManager.getCredentials();
expect(mockRefreshSession).toHaveBeenCalledTimes(1);
expect(credentials).not.toBeNull();
expect(credentials?.token).toBe('new-access-token-xyz');
expect(credentials?.token).toBe('expired-token');
expect(credentials?.refreshToken).toBe('valid-refresh-token');
});
it('should not refresh valid token', async () => {
it('should return valid token', () => {
// Set up valid credentials
const validCredentials: AuthCredentials = {
token: 'valid-token',
@@ -123,22 +114,14 @@ describe('AuthManager - Token Auto-Refresh Integration', () => {
authManager = AuthManager.getInstance();
// Mock refresh to ensure it's not called
const mockRefreshSession = vi.fn();
vi.spyOn(
authManager['supabaseClient'],
'refreshSession'
).mockImplementation(mockRefreshSession);
const credentials = authManager.getCredentials();
const credentials = await authManager.getCredentials();
expect(mockRefreshSession).not.toHaveBeenCalled();
expect(credentials?.token).toBe('valid-token');
});
});
describe('Token Refresh Flow', () => {
it('should refresh expired token and save new credentials', async () => {
it('should manually refresh expired token and save new credentials', async () => {
const expiredCredentials: AuthCredentials = {
token: 'old-token',
refreshToken: 'old-refresh-token',
@@ -162,23 +145,24 @@ describe('AuthManager - Token Auto-Refresh Integration', () => {
'refreshSession'
).mockResolvedValue(mockRefreshedSession);
const refreshedCredentials = await authManager.getCredentials();
// Explicitly call refreshToken() method
const refreshedCredentials = await authManager.refreshToken();
expect(refreshedCredentials).not.toBeNull();
expect(refreshedCredentials?.token).toBe('new-access-token-xyz');
expect(refreshedCredentials?.refreshToken).toBe('new-refresh-token-xyz');
expect(refreshedCredentials.token).toBe('new-access-token-xyz');
expect(refreshedCredentials.refreshToken).toBe('new-refresh-token-xyz');
// Verify context was preserved
expect(refreshedCredentials?.selectedContext?.orgId).toBe('test-org');
expect(refreshedCredentials?.selectedContext?.briefId).toBe('test-brief');
expect(refreshedCredentials.selectedContext?.orgId).toBe('test-org');
expect(refreshedCredentials.selectedContext?.briefId).toBe('test-brief');
// Verify new expiration is in the future
const newExpiry = new Date(refreshedCredentials!.expiresAt!).getTime();
const newExpiry = new Date(refreshedCredentials.expiresAt!).getTime();
const now = Date.now();
expect(newExpiry).toBeGreaterThan(now);
});
it('should return null if refresh fails', async () => {
it('should throw error if manual refresh fails', async () => {
const expiredCredentials: AuthCredentials = {
token: 'expired-token',
refreshToken: 'invalid-refresh-token',
@@ -198,12 +182,11 @@ describe('AuthManager - Token Auto-Refresh Integration', () => {
'refreshSession'
).mockRejectedValue(new Error('Refresh token expired'));
const credentials = await authManager.getCredentials();
expect(credentials).toBeNull();
// Explicit refreshToken() call should throw
await expect(authManager.refreshToken()).rejects.toThrow();
});
it('should return null if no refresh token available', async () => {
it('should return expired credentials even without refresh token', () => {
const expiredCredentials: AuthCredentials = {
token: 'expired-token',
// No refresh token
@@ -217,18 +200,21 @@ describe('AuthManager - Token Auto-Refresh Integration', () => {
authManager = AuthManager.getInstance();
const credentials = await authManager.getCredentials();
const credentials = authManager.getCredentials();
expect(credentials).toBeNull();
// Credentials are returned even without refresh token
expect(credentials).not.toBeNull();
expect(credentials?.token).toBe('expired-token');
expect(credentials?.refreshToken).toBeUndefined();
});
it('should return null if credentials missing expiresAt', async () => {
it('should return null if credentials missing expiresAt', () => {
const credentialsWithoutExpiry: AuthCredentials = {
token: 'test-token',
refreshToken: 'refresh-token',
userId: 'test-user-id',
email: 'test@example.com',
// Missing expiresAt
// Missing expiresAt - invalid token
savedAt: new Date().toISOString()
} as any;
@@ -236,16 +222,17 @@ describe('AuthManager - Token Auto-Refresh Integration', () => {
authManager = AuthManager.getInstance();
const credentials = await authManager.getCredentials();
const credentials = authManager.getCredentials();
// Should return null because no valid expiration
// Tokens without valid expiration are considered invalid
expect(credentials).toBeNull();
});
});
describe('Clock Skew Tolerance', () => {
it('should refresh token within 30-second expiry window', async () => {
it('should return credentials within 30-second expiry window', () => {
// Token expires in 15 seconds (within 30-second buffer)
// Supabase will handle refresh automatically
const almostExpiredCredentials: AuthCredentials = {
token: 'almost-expired-token',
refreshToken: 'valid-refresh-token',
@@ -259,23 +246,16 @@ describe('AuthManager - Token Auto-Refresh Integration', () => {
authManager = AuthManager.getInstance();
const mockRefreshSession = vi
.fn()
.mockResolvedValue(mockRefreshedSession);
vi.spyOn(
authManager['supabaseClient'],
'refreshSession'
).mockImplementation(mockRefreshSession);
const credentials = authManager.getCredentials();
const credentials = await authManager.getCredentials();
// Should trigger refresh due to 30-second buffer
expect(mockRefreshSession).toHaveBeenCalledTimes(1);
expect(credentials?.token).toBe('new-access-token-xyz');
// Credentials are returned (Supabase handles auto-refresh in background)
expect(credentials).not.toBeNull();
expect(credentials?.token).toBe('almost-expired-token');
expect(credentials?.refreshToken).toBe('valid-refresh-token');
});
it('should not refresh token well before expiry', async () => {
// Token expires in 5 minutes (well outside 30-second buffer)
it('should return valid token well before expiry', () => {
// Token expires in 5 minutes
const validCredentials: AuthCredentials = {
token: 'valid-token',
refreshToken: 'valid-refresh-token',
@@ -289,21 +269,17 @@ describe('AuthManager - Token Auto-Refresh Integration', () => {
authManager = AuthManager.getInstance();
const mockRefreshSession = vi.fn();
vi.spyOn(
authManager['supabaseClient'],
'refreshSession'
).mockImplementation(mockRefreshSession);
const credentials = authManager.getCredentials();
const credentials = await authManager.getCredentials();
expect(mockRefreshSession).not.toHaveBeenCalled();
// Valid credentials are returned as-is
expect(credentials).not.toBeNull();
expect(credentials?.token).toBe('valid-token');
expect(credentials?.refreshToken).toBe('valid-refresh-token');
});
});
describe('Synchronous vs Async Methods', () => {
it('getCredentialsSync should not trigger refresh', () => {
it('getCredentials should return expired credentials', () => {
const expiredCredentials: AuthCredentials = {
token: 'expired-token',
refreshToken: 'valid-refresh-token',
@@ -317,40 +293,17 @@ describe('AuthManager - Token Auto-Refresh Integration', () => {
authManager = AuthManager.getInstance();
// Synchronous call should return null without refresh
const credentials = authManager.getCredentialsSync();
expect(credentials).toBeNull();
});
it('getCredentials async should trigger refresh', async () => {
const expiredCredentials: AuthCredentials = {
token: 'expired-token',
refreshToken: 'valid-refresh-token',
userId: 'test-user-id',
email: 'test@example.com',
expiresAt: new Date(Date.now() - 60000).toISOString(),
savedAt: new Date().toISOString()
};
credentialStore.saveCredentials(expiredCredentials);
authManager = AuthManager.getInstance();
vi.spyOn(
authManager['supabaseClient'],
'refreshSession'
).mockResolvedValue(mockRefreshedSession);
const credentials = await authManager.getCredentials();
// Returns credentials even if expired - Supabase will handle refresh
const credentials = authManager.getCredentials();
expect(credentials).not.toBeNull();
expect(credentials?.token).toBe('new-access-token-xyz');
expect(credentials?.token).toBe('expired-token');
expect(credentials?.refreshToken).toBe('valid-refresh-token');
});
});
describe('Multiple Concurrent Calls', () => {
it('should handle concurrent getCredentials calls gracefully', async () => {
it('should handle concurrent getCredentials calls gracefully', () => {
const expiredCredentials: AuthCredentials = {
token: 'expired-token',
refreshToken: 'valid-refresh-token',
@@ -364,29 +317,20 @@ describe('AuthManager - Token Auto-Refresh Integration', () => {
authManager = AuthManager.getInstance();
const mockRefreshSession = vi
.fn()
.mockResolvedValue(mockRefreshedSession);
vi.spyOn(
authManager['supabaseClient'],
'refreshSession'
).mockImplementation(mockRefreshSession);
// Make multiple concurrent calls (synchronous now)
const creds1 = authManager.getCredentials();
const creds2 = authManager.getCredentials();
const creds3 = authManager.getCredentials();
// Make multiple concurrent calls
const [creds1, creds2, creds3] = await Promise.all([
authManager.getCredentials(),
authManager.getCredentials(),
authManager.getCredentials()
]);
// All should get the same credentials (even if expired)
expect(creds1?.token).toBe('expired-token');
expect(creds2?.token).toBe('expired-token');
expect(creds3?.token).toBe('expired-token');
// All should get the refreshed token
expect(creds1?.token).toBe('new-access-token-xyz');
expect(creds2?.token).toBe('new-access-token-xyz');
expect(creds3?.token).toBe('new-access-token-xyz');
// Refresh might be called multiple times, but that's okay
// (ideally we'd debounce, but this is acceptable behavior)
expect(mockRefreshSession).toHaveBeenCalled();
// All include refresh token for Supabase to use
expect(creds1?.refreshToken).toBe('valid-refresh-token');
expect(creds2?.refreshToken).toBe('valid-refresh-token');
expect(creds3?.refreshToken).toBe('valid-refresh-token');
});
});
});