Compare commits
4 Commits
ralph/feat
...
ralph/fix/
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
93097dfeb5 | ||
|
|
fa2b63de40 | ||
|
|
b11dacece9 | ||
|
|
01cbbe97f2 |
5
.changeset/dirty-hairs-know.md
Normal file
5
.changeset/dirty-hairs-know.md
Normal file
@@ -0,0 +1,5 @@
|
|||||||
|
---
|
||||||
|
"task-master-ai": patch
|
||||||
|
---
|
||||||
|
|
||||||
|
Improve auth token refresh flow
|
||||||
@@ -143,7 +143,7 @@ export class AuthCommand extends Command {
|
|||||||
*/
|
*/
|
||||||
private async executeStatus(): Promise<void> {
|
private async executeStatus(): Promise<void> {
|
||||||
try {
|
try {
|
||||||
const result = await this.displayStatus();
|
const result = this.displayStatus();
|
||||||
this.setLastResult(result);
|
this.setLastResult(result);
|
||||||
} catch (error: any) {
|
} catch (error: any) {
|
||||||
this.handleError(error);
|
this.handleError(error);
|
||||||
@@ -171,8 +171,8 @@ export class AuthCommand extends Command {
|
|||||||
/**
|
/**
|
||||||
* Display authentication status
|
* Display authentication status
|
||||||
*/
|
*/
|
||||||
private async displayStatus(): Promise<AuthResult> {
|
private displayStatus(): AuthResult {
|
||||||
const credentials = await this.authManager.getCredentials();
|
const credentials = this.authManager.getCredentials();
|
||||||
|
|
||||||
console.log(chalk.cyan('\n🔐 Authentication Status\n'));
|
console.log(chalk.cyan('\n🔐 Authentication Status\n'));
|
||||||
|
|
||||||
@@ -325,7 +325,7 @@ export class AuthCommand extends Command {
|
|||||||
]);
|
]);
|
||||||
|
|
||||||
if (!continueAuth) {
|
if (!continueAuth) {
|
||||||
const credentials = await this.authManager.getCredentials();
|
const credentials = this.authManager.getCredentials();
|
||||||
ui.displaySuccess('Using existing authentication');
|
ui.displaySuccess('Using existing authentication');
|
||||||
|
|
||||||
if (credentials) {
|
if (credentials) {
|
||||||
@@ -490,7 +490,7 @@ export class AuthCommand extends Command {
|
|||||||
/**
|
/**
|
||||||
* Get current credentials (for programmatic usage)
|
* Get current credentials (for programmatic usage)
|
||||||
*/
|
*/
|
||||||
getCredentials(): Promise<AuthCredentials | null> {
|
getCredentials(): AuthCredentials | null {
|
||||||
return this.authManager.getCredentials();
|
return this.authManager.getCredentials();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -115,7 +115,7 @@ export class ContextCommand extends Command {
|
|||||||
*/
|
*/
|
||||||
private async executeShow(): Promise<void> {
|
private async executeShow(): Promise<void> {
|
||||||
try {
|
try {
|
||||||
const result = await this.displayContext();
|
const result = this.displayContext();
|
||||||
this.setLastResult(result);
|
this.setLastResult(result);
|
||||||
} catch (error: any) {
|
} catch (error: any) {
|
||||||
this.handleError(error);
|
this.handleError(error);
|
||||||
@@ -126,7 +126,7 @@ export class ContextCommand extends Command {
|
|||||||
/**
|
/**
|
||||||
* Display current context
|
* Display current context
|
||||||
*/
|
*/
|
||||||
private async displayContext(): Promise<ContextResult> {
|
private displayContext(): ContextResult {
|
||||||
// Check authentication first
|
// Check authentication first
|
||||||
if (!this.authManager.isAuthenticated()) {
|
if (!this.authManager.isAuthenticated()) {
|
||||||
console.log(chalk.yellow('✗ Not authenticated'));
|
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'));
|
console.log(chalk.cyan('\n🌍 Workspace Context\n'));
|
||||||
|
|
||||||
@@ -250,7 +250,7 @@ export class ContextCommand extends Command {
|
|||||||
]);
|
]);
|
||||||
|
|
||||||
// Update context
|
// Update context
|
||||||
await this.authManager.updateContext({
|
this.authManager.updateContext({
|
||||||
orgId: selectedOrg.id,
|
orgId: selectedOrg.id,
|
||||||
orgName: selectedOrg.name,
|
orgName: selectedOrg.name,
|
||||||
// Clear brief when changing org
|
// Clear brief when changing org
|
||||||
@@ -263,7 +263,7 @@ export class ContextCommand extends Command {
|
|||||||
return {
|
return {
|
||||||
success: true,
|
success: true,
|
||||||
action: 'select-org',
|
action: 'select-org',
|
||||||
context: (await this.authManager.getContext()) || undefined,
|
context: this.authManager.getContext() || undefined,
|
||||||
message: `Selected organization: ${selectedOrg.name}`
|
message: `Selected organization: ${selectedOrg.name}`
|
||||||
};
|
};
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
@@ -284,7 +284,7 @@ export class ContextCommand extends Command {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Check if org is selected
|
// Check if org is selected
|
||||||
const context = await this.authManager.getContext();
|
const context = this.authManager.getContext();
|
||||||
if (!context?.orgId) {
|
if (!context?.orgId) {
|
||||||
ui.displayError(
|
ui.displayError(
|
||||||
'No organization selected. Run "tm context org" first.'
|
'No organization selected. Run "tm context org" first.'
|
||||||
@@ -343,7 +343,7 @@ export class ContextCommand extends Command {
|
|||||||
if (selectedBrief) {
|
if (selectedBrief) {
|
||||||
// Update context with brief
|
// Update context with brief
|
||||||
const briefName = `Brief ${selectedBrief.id.slice(0, 8)}`;
|
const briefName = `Brief ${selectedBrief.id.slice(0, 8)}`;
|
||||||
await this.authManager.updateContext({
|
this.authManager.updateContext({
|
||||||
briefId: selectedBrief.id,
|
briefId: selectedBrief.id,
|
||||||
briefName: briefName
|
briefName: briefName
|
||||||
});
|
});
|
||||||
@@ -353,12 +353,12 @@ export class ContextCommand extends Command {
|
|||||||
return {
|
return {
|
||||||
success: true,
|
success: true,
|
||||||
action: 'select-brief',
|
action: 'select-brief',
|
||||||
context: (await this.authManager.getContext()) || undefined,
|
context: this.authManager.getContext() || undefined,
|
||||||
message: `Selected brief: ${selectedBrief.name}`
|
message: `Selected brief: ${selectedBrief.name}`
|
||||||
};
|
};
|
||||||
} else {
|
} else {
|
||||||
// Clear brief selection
|
// Clear brief selection
|
||||||
await this.authManager.updateContext({
|
this.authManager.updateContext({
|
||||||
briefId: undefined,
|
briefId: undefined,
|
||||||
briefName: undefined
|
briefName: undefined
|
||||||
});
|
});
|
||||||
@@ -368,7 +368,7 @@ export class ContextCommand extends Command {
|
|||||||
return {
|
return {
|
||||||
success: true,
|
success: true,
|
||||||
action: 'select-brief',
|
action: 'select-brief',
|
||||||
context: (await this.authManager.getContext()) || undefined,
|
context: this.authManager.getContext() || undefined,
|
||||||
message: 'Cleared brief selection'
|
message: 'Cleared brief selection'
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
@@ -491,7 +491,7 @@ export class ContextCommand extends Command {
|
|||||||
|
|
||||||
// Update context: set org and brief
|
// Update context: set org and brief
|
||||||
const briefName = `Brief ${brief.id.slice(0, 8)}`;
|
const briefName = `Brief ${brief.id.slice(0, 8)}`;
|
||||||
await this.authManager.updateContext({
|
this.authManager.updateContext({
|
||||||
orgId: brief.accountId,
|
orgId: brief.accountId,
|
||||||
orgName,
|
orgName,
|
||||||
briefId: brief.id,
|
briefId: brief.id,
|
||||||
@@ -508,7 +508,7 @@ export class ContextCommand extends Command {
|
|||||||
this.setLastResult({
|
this.setLastResult({
|
||||||
success: true,
|
success: true,
|
||||||
action: 'set',
|
action: 'set',
|
||||||
context: (await this.authManager.getContext()) || undefined,
|
context: this.authManager.getContext() || undefined,
|
||||||
message: 'Context set from brief'
|
message: 'Context set from brief'
|
||||||
});
|
});
|
||||||
} catch (error: any) {
|
} 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');
|
ui.displaySuccess('Context updated');
|
||||||
|
|
||||||
// Display what was set
|
// Display what was set
|
||||||
@@ -631,7 +631,7 @@ export class ContextCommand extends Command {
|
|||||||
return {
|
return {
|
||||||
success: true,
|
success: true,
|
||||||
action: 'set',
|
action: 'set',
|
||||||
context: (await this.authManager.getContext()) || undefined,
|
context: this.authManager.getContext() || undefined,
|
||||||
message: 'Context updated'
|
message: 'Context updated'
|
||||||
};
|
};
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
@@ -682,7 +682,7 @@ export class ContextCommand extends Command {
|
|||||||
/**
|
/**
|
||||||
* Get current context (for programmatic usage)
|
* Get current context (for programmatic usage)
|
||||||
*/
|
*/
|
||||||
getContext(): Promise<UserContext | null> {
|
getContext(): UserContext | null {
|
||||||
return this.authManager.getContext();
|
return this.authManager.getContext();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -35,7 +35,7 @@ vi.mock('./credential-store.js', () => {
|
|||||||
}
|
}
|
||||||
saveCredentials() {}
|
saveCredentials() {}
|
||||||
clearCredentials() {}
|
clearCredentials() {}
|
||||||
hasValidCredentials() {
|
hasCredentials() {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -29,8 +29,6 @@ export class AuthManager {
|
|||||||
private oauthService: OAuthService;
|
private oauthService: OAuthService;
|
||||||
private supabaseClient: SupabaseAuthClient;
|
private supabaseClient: SupabaseAuthClient;
|
||||||
private organizationService?: OrganizationService;
|
private organizationService?: OrganizationService;
|
||||||
private logger = getLogger('AuthManager');
|
|
||||||
private refreshPromise: Promise<AuthCredentials> | null = null;
|
|
||||||
|
|
||||||
private constructor(config?: Partial<AuthConfig>) {
|
private constructor(config?: Partial<AuthConfig>) {
|
||||||
this.credentialStore = CredentialStore.getInstance(config);
|
this.credentialStore = CredentialStore.getInstance(config);
|
||||||
@@ -83,60 +81,10 @@ export class AuthManager {
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Get stored authentication credentials
|
* 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> {
|
getCredentials(): 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 {
|
|
||||||
return this.credentialStore.getCredentials();
|
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 {
|
isAuthenticated(): boolean {
|
||||||
return this.credentialStore.hasValidCredentials();
|
return this.credentialStore.hasCredentials();
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Get the current user context (org/brief selection)
|
* Get the current user context (org/brief selection)
|
||||||
*/
|
*/
|
||||||
async getContext(): Promise<UserContext | null> {
|
getContext(): UserContext | null {
|
||||||
const credentials = await this.getCredentials();
|
const credentials = this.getCredentials();
|
||||||
return credentials?.selectedContext || null;
|
return credentials?.selectedContext || null;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Update the user context (org/brief selection)
|
* Update the user context (org/brief selection)
|
||||||
*/
|
*/
|
||||||
async updateContext(context: Partial<UserContext>): Promise<void> {
|
updateContext(context: Partial<UserContext>): void {
|
||||||
const credentials = await this.getCredentials();
|
const credentials = this.getCredentials();
|
||||||
if (!credentials) {
|
if (!credentials) {
|
||||||
throw new AuthenticationError('Not authenticated', 'NOT_AUTHENTICATED');
|
throw new AuthenticationError('Not authenticated', 'NOT_AUTHENTICATED');
|
||||||
}
|
}
|
||||||
@@ -262,8 +211,8 @@ export class AuthManager {
|
|||||||
/**
|
/**
|
||||||
* Clear the user context
|
* Clear the user context
|
||||||
*/
|
*/
|
||||||
async clearContext(): Promise<void> {
|
clearContext(): void {
|
||||||
const credentials = await this.getCredentials();
|
const credentials = this.getCredentials();
|
||||||
if (!credentials) {
|
if (!credentials) {
|
||||||
throw new AuthenticationError('Not authenticated', 'NOT_AUTHENTICATED');
|
throw new AuthenticationError('Not authenticated', 'NOT_AUTHENTICATED');
|
||||||
}
|
}
|
||||||
@@ -280,7 +229,7 @@ export class AuthManager {
|
|||||||
private async getOrganizationService(): Promise<OrganizationService> {
|
private async getOrganizationService(): Promise<OrganizationService> {
|
||||||
if (!this.organizationService) {
|
if (!this.organizationService) {
|
||||||
// First check if we have credentials with a token
|
// First check if we have credentials with a token
|
||||||
const credentials = await this.getCredentials();
|
const credentials = this.getCredentials();
|
||||||
if (!credentials || !credentials.token) {
|
if (!credentials || !credentials.token) {
|
||||||
throw new AuthenticationError('Not authenticated', 'NOT_AUTHENTICATED');
|
throw new AuthenticationError('Not authenticated', 'NOT_AUTHENTICATED');
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -52,7 +52,7 @@ describe('CredentialStore - Token Expiration', () => {
|
|||||||
|
|
||||||
credentialStore.saveCredentials(expiredCredentials);
|
credentialStore.saveCredentials(expiredCredentials);
|
||||||
|
|
||||||
const retrieved = credentialStore.getCredentials();
|
const retrieved = credentialStore.getCredentials({ allowExpired: false });
|
||||||
|
|
||||||
expect(retrieved).toBeNull();
|
expect(retrieved).toBeNull();
|
||||||
});
|
});
|
||||||
@@ -69,7 +69,7 @@ describe('CredentialStore - Token Expiration', () => {
|
|||||||
|
|
||||||
credentialStore.saveCredentials(validCredentials);
|
credentialStore.saveCredentials(validCredentials);
|
||||||
|
|
||||||
const retrieved = credentialStore.getCredentials();
|
const retrieved = credentialStore.getCredentials({ allowExpired: false });
|
||||||
|
|
||||||
expect(retrieved).not.toBeNull();
|
expect(retrieved).not.toBeNull();
|
||||||
expect(retrieved?.token).toBe('valid-token');
|
expect(retrieved?.token).toBe('valid-token');
|
||||||
@@ -92,6 +92,25 @@ describe('CredentialStore - Token Expiration', () => {
|
|||||||
expect(retrieved).not.toBeNull();
|
expect(retrieved).not.toBeNull();
|
||||||
expect(retrieved?.token).toBe('expired-token');
|
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', () => {
|
describe('Clock Skew Tolerance', () => {
|
||||||
@@ -108,7 +127,7 @@ describe('CredentialStore - Token Expiration', () => {
|
|||||||
|
|
||||||
credentialStore.saveCredentials(almostExpiredCredentials);
|
credentialStore.saveCredentials(almostExpiredCredentials);
|
||||||
|
|
||||||
const retrieved = credentialStore.getCredentials();
|
const retrieved = credentialStore.getCredentials({ allowExpired: false });
|
||||||
|
|
||||||
expect(retrieved).toBeNull();
|
expect(retrieved).toBeNull();
|
||||||
});
|
});
|
||||||
@@ -126,7 +145,7 @@ describe('CredentialStore - Token Expiration', () => {
|
|||||||
|
|
||||||
credentialStore.saveCredentials(validCredentials);
|
credentialStore.saveCredentials(validCredentials);
|
||||||
|
|
||||||
const retrieved = credentialStore.getCredentials();
|
const retrieved = credentialStore.getCredentials({ allowExpired: false });
|
||||||
|
|
||||||
expect(retrieved).not.toBeNull();
|
expect(retrieved).not.toBeNull();
|
||||||
expect(retrieved?.token).toBe('valid-token');
|
expect(retrieved?.token).toBe('valid-token');
|
||||||
@@ -146,7 +165,7 @@ describe('CredentialStore - Token Expiration', () => {
|
|||||||
|
|
||||||
credentialStore.saveCredentials(credentials);
|
credentialStore.saveCredentials(credentials);
|
||||||
|
|
||||||
const retrieved = credentialStore.getCredentials();
|
const retrieved = credentialStore.getCredentials({ allowExpired: false });
|
||||||
|
|
||||||
expect(retrieved).not.toBeNull();
|
expect(retrieved).not.toBeNull();
|
||||||
expect(typeof retrieved?.expiresAt).toBe('number'); // Normalized to number
|
expect(typeof retrieved?.expiresAt).toBe('number'); // Normalized to number
|
||||||
@@ -164,7 +183,7 @@ describe('CredentialStore - Token Expiration', () => {
|
|||||||
|
|
||||||
credentialStore.saveCredentials(credentials);
|
credentialStore.saveCredentials(credentials);
|
||||||
|
|
||||||
const retrieved = credentialStore.getCredentials();
|
const retrieved = credentialStore.getCredentials({ allowExpired: false });
|
||||||
|
|
||||||
expect(retrieved).not.toBeNull();
|
expect(retrieved).not.toBeNull();
|
||||||
expect(typeof retrieved?.expiresAt).toBe('number');
|
expect(typeof retrieved?.expiresAt).toBe('number');
|
||||||
@@ -185,7 +204,7 @@ describe('CredentialStore - Token Expiration', () => {
|
|||||||
mode: 0o600
|
mode: 0o600
|
||||||
});
|
});
|
||||||
|
|
||||||
const retrieved = credentialStore.getCredentials();
|
const retrieved = credentialStore.getCredentials({ allowExpired: false });
|
||||||
|
|
||||||
expect(retrieved).toBeNull();
|
expect(retrieved).toBeNull();
|
||||||
});
|
});
|
||||||
@@ -203,7 +222,7 @@ describe('CredentialStore - Token Expiration', () => {
|
|||||||
mode: 0o600
|
mode: 0o600
|
||||||
});
|
});
|
||||||
|
|
||||||
const retrieved = credentialStore.getCredentials();
|
const retrieved = credentialStore.getCredentials({ allowExpired: false });
|
||||||
|
|
||||||
expect(retrieved).toBeNull();
|
expect(retrieved).toBeNull();
|
||||||
});
|
});
|
||||||
@@ -244,15 +263,15 @@ describe('CredentialStore - Token Expiration', () => {
|
|||||||
|
|
||||||
credentialStore.saveCredentials(credentials);
|
credentialStore.saveCredentials(credentials);
|
||||||
|
|
||||||
const retrieved = credentialStore.getCredentials();
|
const retrieved = credentialStore.getCredentials({ allowExpired: false });
|
||||||
|
|
||||||
// Should be normalized to number for runtime use
|
// Should be normalized to number for runtime use
|
||||||
expect(typeof retrieved?.expiresAt).toBe('number');
|
expect(typeof retrieved?.expiresAt).toBe('number');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('hasValidCredentials', () => {
|
describe('hasCredentials', () => {
|
||||||
it('should return false for expired credentials', () => {
|
it('should return true for expired credentials', () => {
|
||||||
const expiredCredentials: AuthCredentials = {
|
const expiredCredentials: AuthCredentials = {
|
||||||
token: 'expired-token',
|
token: 'expired-token',
|
||||||
refreshToken: 'refresh-token',
|
refreshToken: 'refresh-token',
|
||||||
@@ -264,7 +283,7 @@ describe('CredentialStore - Token Expiration', () => {
|
|||||||
|
|
||||||
credentialStore.saveCredentials(expiredCredentials);
|
credentialStore.saveCredentials(expiredCredentials);
|
||||||
|
|
||||||
expect(credentialStore.hasValidCredentials()).toBe(false);
|
expect(credentialStore.hasCredentials()).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should return true for valid credentials', () => {
|
it('should return true for valid credentials', () => {
|
||||||
@@ -279,11 +298,11 @@ describe('CredentialStore - Token Expiration', () => {
|
|||||||
|
|
||||||
credentialStore.saveCredentials(validCredentials);
|
credentialStore.saveCredentials(validCredentials);
|
||||||
|
|
||||||
expect(credentialStore.hasValidCredentials()).toBe(true);
|
expect(credentialStore.hasCredentials()).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should return false when no credentials exist', () => {
|
it('should return false when no credentials exist', () => {
|
||||||
expect(credentialStore.hasValidCredentials()).toBe(false);
|
expect(credentialStore.hasCredentials()).toBe(false);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -197,7 +197,7 @@ describe('CredentialStore', () => {
|
|||||||
JSON.stringify(mockCredentials)
|
JSON.stringify(mockCredentials)
|
||||||
);
|
);
|
||||||
|
|
||||||
const result = store.getCredentials();
|
const result = store.getCredentials({ allowExpired: false });
|
||||||
|
|
||||||
expect(result).toBeNull();
|
expect(result).toBeNull();
|
||||||
expect(mockLogger.warn).toHaveBeenCalledWith(
|
expect(mockLogger.warn).toHaveBeenCalledWith(
|
||||||
@@ -226,6 +226,31 @@ describe('CredentialStore', () => {
|
|||||||
expect(result).not.toBeNull();
|
expect(result).not.toBeNull();
|
||||||
expect(result?.token).toBe('expired-token');
|
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', () => {
|
describe('saveCredentials with timestamp normalization', () => {
|
||||||
@@ -451,7 +476,7 @@ describe('CredentialStore', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('hasValidCredentials', () => {
|
describe('hasCredentials', () => {
|
||||||
it('should return true when valid unexpired credentials exist', () => {
|
it('should return true when valid unexpired credentials exist', () => {
|
||||||
const futureDate = new Date(Date.now() + 3600000); // 1 hour from now
|
const futureDate = new Date(Date.now() + 3600000); // 1 hour from now
|
||||||
const credentials = {
|
const credentials = {
|
||||||
@@ -465,10 +490,10 @@ describe('CredentialStore', () => {
|
|||||||
vi.mocked(fs.existsSync).mockReturnValue(true);
|
vi.mocked(fs.existsSync).mockReturnValue(true);
|
||||||
vi.mocked(fs.readFileSync).mockReturnValue(JSON.stringify(credentials));
|
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 pastDate = new Date(Date.now() - 3600000); // 1 hour ago
|
||||||
const credentials = {
|
const credentials = {
|
||||||
token: 'expired-token',
|
token: 'expired-token',
|
||||||
@@ -481,13 +506,13 @@ describe('CredentialStore', () => {
|
|||||||
vi.mocked(fs.existsSync).mockReturnValue(true);
|
vi.mocked(fs.existsSync).mockReturnValue(true);
|
||||||
vi.mocked(fs.readFileSync).mockReturnValue(JSON.stringify(credentials));
|
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', () => {
|
it('should return false when no credentials exist', () => {
|
||||||
vi.mocked(fs.existsSync).mockReturnValue(false);
|
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', () => {
|
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.readFileSync).mockReturnValue('invalid json {');
|
||||||
vi.mocked(fs.renameSync).mockImplementation(() => undefined);
|
vi.mocked(fs.renameSync).mockImplementation(() => undefined);
|
||||||
|
|
||||||
expect(store.hasValidCredentials()).toBe(false);
|
expect(store.hasCredentials()).toBe(false);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should return false for credentials without expiry', () => {
|
it('should return false for credentials without expiry', () => {
|
||||||
@@ -510,7 +535,7 @@ describe('CredentialStore', () => {
|
|||||||
vi.mocked(fs.readFileSync).mockReturnValue(JSON.stringify(credentials));
|
vi.mocked(fs.readFileSync).mockReturnValue(JSON.stringify(credentials));
|
||||||
|
|
||||||
// Credentials without expiry are considered invalid
|
// Credentials without expiry are considered invalid
|
||||||
expect(store.hasValidCredentials()).toBe(false);
|
expect(store.hasCredentials()).toBe(false);
|
||||||
|
|
||||||
// Should log warning about missing expiration
|
// Should log warning about missing expiration
|
||||||
expect(mockLogger.warn).toHaveBeenCalledWith(
|
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
|
// Spy on getCredentials to verify it's called with correct params
|
||||||
const getCredentialsSpy = vi.spyOn(store, 'getCredentials');
|
const getCredentialsSpy = vi.spyOn(store, 'getCredentials');
|
||||||
|
|
||||||
vi.mocked(fs.existsSync).mockReturnValue(false);
|
vi.mocked(fs.existsSync).mockReturnValue(false);
|
||||||
store.hasValidCredentials();
|
store.hasCredentials();
|
||||||
|
|
||||||
expect(getCredentialsSpy).toHaveBeenCalledWith({ allowExpired: false });
|
expect(getCredentialsSpy).toHaveBeenCalledWith({ allowExpired: true });
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -54,9 +54,12 @@ export class CredentialStore {
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Get stored authentication credentials
|
* Get stored authentication credentials
|
||||||
|
* @param options.allowExpired - Whether to return expired credentials (default: true)
|
||||||
* @returns AuthCredentials with expiresAt as number (milliseconds) for runtime use
|
* @returns AuthCredentials with expiresAt as number (milliseconds) for runtime use
|
||||||
*/
|
*/
|
||||||
getCredentials(options?: { allowExpired?: boolean }): AuthCredentials | null {
|
getCredentials({
|
||||||
|
allowExpired = true
|
||||||
|
}: { allowExpired?: boolean } = {}): AuthCredentials | null {
|
||||||
try {
|
try {
|
||||||
if (!fs.existsSync(this.config.configFile)) {
|
if (!fs.existsSync(this.config.configFile)) {
|
||||||
return null;
|
return null;
|
||||||
@@ -90,7 +93,6 @@ export class CredentialStore {
|
|||||||
|
|
||||||
// Check if the token has expired (with clock skew tolerance)
|
// Check if the token has expired (with clock skew tolerance)
|
||||||
const now = Date.now();
|
const now = Date.now();
|
||||||
const allowExpired = options?.allowExpired ?? false;
|
|
||||||
if (now >= expiresAtMs - this.CLOCK_SKEW_MS && !allowExpired) {
|
if (now >= expiresAtMs - this.CLOCK_SKEW_MS && !allowExpired) {
|
||||||
this.logger.warn(
|
this.logger.warn(
|
||||||
'Authentication token has expired or is about to expire',
|
'Authentication token has expired or is about to expire',
|
||||||
@@ -103,7 +105,7 @@ export class CredentialStore {
|
|||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Return valid token
|
// Return credentials (even if expired) to enable refresh flows
|
||||||
return authData;
|
return authData;
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
this.logger.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 {
|
hasCredentials(): boolean {
|
||||||
const credentials = this.getCredentials({ allowExpired: false });
|
const credentials = this.getCredentials({ allowExpired: true });
|
||||||
return credentials !== null;
|
return credentials !== null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -281,15 +281,26 @@ export class OAuthService {
|
|||||||
// Exchange code for session using PKCE
|
// Exchange code for session using PKCE
|
||||||
const session = await this.supabaseClient.exchangeCodeForSession(code);
|
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
|
// Save authentication data
|
||||||
const authData: AuthCredentials = {
|
const authData: AuthCredentials = {
|
||||||
token: session.access_token,
|
token: session.access_token,
|
||||||
refreshToken: session.refresh_token,
|
refreshToken: session.refresh_token,
|
||||||
userId: session.user.id,
|
userId: session.user.id,
|
||||||
email: session.user.email,
|
email: session.user.email,
|
||||||
expiresAt: session.expires_at
|
expiresAt,
|
||||||
? new Date(session.expires_at * 1000).toISOString()
|
|
||||||
: undefined,
|
|
||||||
tokenType: 'standard',
|
tokenType: 'standard',
|
||||||
savedAt: new Date().toISOString()
|
savedAt: new Date().toISOString()
|
||||||
};
|
};
|
||||||
@@ -340,10 +351,18 @@ export class OAuthService {
|
|||||||
// Get user info from the session
|
// Get user info from the session
|
||||||
const user = await this.supabaseClient.getUser();
|
const user = await this.supabaseClient.getUser();
|
||||||
|
|
||||||
// Calculate expiration time
|
// Calculate expiration time - can be overridden with TM_TOKEN_EXPIRY_MINUTES
|
||||||
const expiresAt = expiresIn
|
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()
|
? new Date(Date.now() + parseInt(expiresIn) * 1000).toISOString()
|
||||||
: undefined;
|
: undefined;
|
||||||
|
}
|
||||||
|
|
||||||
// Save authentication data
|
// Save authentication data
|
||||||
const authData: AuthCredentials = {
|
const authData: AuthCredentials = {
|
||||||
@@ -351,7 +370,7 @@ export class OAuthService {
|
|||||||
refreshToken: refreshToken || undefined,
|
refreshToken: refreshToken || undefined,
|
||||||
userId: user?.id || 'unknown',
|
userId: user?.id || 'unknown',
|
||||||
email: user?.email,
|
email: user?.email,
|
||||||
expiresAt: expiresAt,
|
expiresAt,
|
||||||
tokenType: 'standard',
|
tokenType: 'standard',
|
||||||
savedAt: new Date().toISOString()
|
savedAt: new Date().toISOString()
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -98,11 +98,11 @@ export class SupabaseSessionStorage implements SupportedStorage {
|
|||||||
// Only handle Supabase session keys
|
// Only handle Supabase session keys
|
||||||
if (key === STORAGE_KEY || key.includes('auth-token')) {
|
if (key === STORAGE_KEY || key.includes('auth-token')) {
|
||||||
try {
|
try {
|
||||||
|
this.logger.info('Supabase called setItem - storing refreshed session');
|
||||||
|
|
||||||
// Parse the session and update our credentials
|
// Parse the session and update our credentials
|
||||||
const sessionUpdates = this.parseSessionToCredentials(value);
|
const sessionUpdates = this.parseSessionToCredentials(value);
|
||||||
const existingCredentials = this.store.getCredentials({
|
const existingCredentials = this.store.getCredentials();
|
||||||
allowExpired: true
|
|
||||||
});
|
|
||||||
|
|
||||||
if (sessionUpdates.token) {
|
if (sessionUpdates.token) {
|
||||||
const updatedCredentials: AuthCredentials = {
|
const updatedCredentials: AuthCredentials = {
|
||||||
@@ -113,6 +113,9 @@ export class SupabaseSessionStorage implements SupportedStorage {
|
|||||||
} as AuthCredentials;
|
} as AuthCredentials;
|
||||||
|
|
||||||
this.store.saveCredentials(updatedCredentials);
|
this.store.saveCredentials(updatedCredentials);
|
||||||
|
this.logger.info(
|
||||||
|
'Successfully saved refreshed credentials from Supabase'
|
||||||
|
);
|
||||||
}
|
}
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
this.logger.error('Error setting session:', error);
|
this.logger.error('Error setting session:', error);
|
||||||
|
|||||||
@@ -17,10 +17,11 @@ export class SupabaseAuthClient {
|
|||||||
private client: SupabaseJSClient | null = null;
|
private client: SupabaseJSClient | null = null;
|
||||||
private sessionStorage: SupabaseSessionStorage;
|
private sessionStorage: SupabaseSessionStorage;
|
||||||
private logger = getLogger('SupabaseAuthClient');
|
private logger = getLogger('SupabaseAuthClient');
|
||||||
|
private credentialStore: CredentialStore;
|
||||||
|
|
||||||
constructor() {
|
constructor() {
|
||||||
const credentialStore = CredentialStore.getInstance();
|
this.credentialStore = CredentialStore.getInstance();
|
||||||
this.sessionStorage = new SupabaseSessionStorage(credentialStore);
|
this.sessionStorage = new SupabaseSessionStorage(this.credentialStore);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -73,7 +73,7 @@ export class StorageFactory {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
// Use auth token from AuthManager (synchronous - no auto-refresh here)
|
// Use auth token from AuthManager (synchronous - no auto-refresh here)
|
||||||
const credentials = authManager.getCredentialsSync();
|
const credentials = authManager.getCredentials();
|
||||||
if (credentials) {
|
if (credentials) {
|
||||||
// Merge with existing storage config, ensuring required fields
|
// Merge with existing storage config, ensuring required fields
|
||||||
const nextStorage: StorageSettings = {
|
const nextStorage: StorageSettings = {
|
||||||
@@ -103,7 +103,7 @@ export class StorageFactory {
|
|||||||
|
|
||||||
// Then check if authenticated via AuthManager
|
// Then check if authenticated via AuthManager
|
||||||
if (authManager.isAuthenticated()) {
|
if (authManager.isAuthenticated()) {
|
||||||
const credentials = authManager.getCredentialsSync();
|
const credentials = authManager.getCredentials();
|
||||||
if (credentials) {
|
if (credentials) {
|
||||||
// Configure API storage with auth credentials
|
// Configure API storage with auth credentials
|
||||||
const nextStorage: StorageSettings = {
|
const nextStorage: StorageSettings = {
|
||||||
|
|||||||
@@ -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
|
// Set up expired credentials with refresh token
|
||||||
const expiredCredentials: AuthCredentials = {
|
const expiredCredentials: AuthCredentials = {
|
||||||
token: 'expired_access_token',
|
token: 'expired_access_token',
|
||||||
@@ -63,50 +63,16 @@ describe('AuthManager Token Refresh', () => {
|
|||||||
|
|
||||||
credentialStore.saveCredentials(expiredCredentials);
|
credentialStore.saveCredentials(expiredCredentials);
|
||||||
|
|
||||||
// Mock the refreshToken method to track calls
|
// Get credentials should return them even if expired
|
||||||
const refreshTokenSpy = vi.spyOn(authManager as any, 'refreshToken');
|
// Refresh will be handled by explicit calls or client operations
|
||||||
const mockSession: Session = {
|
const credentials = authManager.getCredentials();
|
||||||
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()
|
|
||||||
}
|
|
||||||
};
|
|
||||||
|
|
||||||
refreshTokenSpy.mockResolvedValue({
|
expect(credentials).not.toBeNull();
|
||||||
token: mockSession.access_token,
|
expect(credentials?.token).toBe('expired_access_token');
|
||||||
refreshToken: mockSession.refresh_token,
|
expect(credentials?.refreshToken).toBe('valid_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
|
it('should return valid credentials', () => {
|
||||||
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);
|
|
||||||
});
|
|
||||||
|
|
||||||
it('should return valid credentials without attempting refresh', async () => {
|
|
||||||
// Set up valid (non-expired) credentials
|
// Set up valid (non-expired) credentials
|
||||||
const validCredentials: AuthCredentials = {
|
const validCredentials: AuthCredentials = {
|
||||||
token: 'valid_access_token',
|
token: 'valid_access_token',
|
||||||
@@ -119,17 +85,14 @@ describe('AuthManager Token Refresh', () => {
|
|||||||
|
|
||||||
credentialStore.saveCredentials(validCredentials);
|
credentialStore.saveCredentials(validCredentials);
|
||||||
|
|
||||||
// Spy on refreshToken to ensure it's not called
|
const credentials = authManager.getCredentials();
|
||||||
const refreshTokenSpy = vi.spyOn(authManager as any, 'refreshToken');
|
|
||||||
|
|
||||||
const credentials = await authManager.getCredentials();
|
|
||||||
|
|
||||||
expect(credentials?.token).toBe('valid_access_token');
|
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
|
// Set up expired credentials WITHOUT refresh token
|
||||||
|
// We still return them - it's up to the caller to handle
|
||||||
const expiredCredentials: AuthCredentials = {
|
const expiredCredentials: AuthCredentials = {
|
||||||
token: 'expired_access_token',
|
token: 'expired_access_token',
|
||||||
refreshToken: undefined,
|
refreshToken: undefined,
|
||||||
@@ -141,17 +104,19 @@ describe('AuthManager Token Refresh', () => {
|
|||||||
|
|
||||||
credentialStore.saveCredentials(expiredCredentials);
|
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();
|
expect(credentials).toBeNull();
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should return null if no credentials exist', async () => {
|
it('should return credentials regardless of refresh token validity', () => {
|
||||||
const credentials = await authManager.getCredentials();
|
|
||||||
expect(credentials).toBeNull();
|
|
||||||
});
|
|
||||||
|
|
||||||
it('should handle refresh failures gracefully', async () => {
|
|
||||||
// Set up expired credentials with refresh token
|
// Set up expired credentials with refresh token
|
||||||
const expiredCredentials: AuthCredentials = {
|
const expiredCredentials: AuthCredentials = {
|
||||||
token: 'expired_access_token',
|
token: 'expired_access_token',
|
||||||
@@ -164,13 +129,11 @@ describe('AuthManager Token Refresh', () => {
|
|||||||
|
|
||||||
credentialStore.saveCredentials(expiredCredentials);
|
credentialStore.saveCredentials(expiredCredentials);
|
||||||
|
|
||||||
// Mock refreshToken to throw an error
|
const credentials = authManager.getCredentials();
|
||||||
const refreshTokenSpy = vi.spyOn(authManager as any, 'refreshToken');
|
|
||||||
refreshTokenSpy.mockRejectedValue(new Error('Refresh failed'));
|
|
||||||
|
|
||||||
const credentials = await authManager.getCredentials();
|
// Returns credentials - refresh will be attempted by the client which will handle failure
|
||||||
|
expect(credentials).not.toBeNull();
|
||||||
expect(credentials).toBeNull();
|
expect(credentials?.token).toBe('expired_access_token');
|
||||||
expect(refreshTokenSpy).toHaveBeenCalledTimes(1);
|
expect(credentials?.refreshToken).toBe('invalid_refresh_token');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -76,7 +76,7 @@ describe('AuthManager - Token Auto-Refresh Integration', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
describe('Expired Token Detection', () => {
|
describe('Expired Token Detection', () => {
|
||||||
it('should detect expired token', async () => {
|
it('should return expired token for Supabase to refresh', () => {
|
||||||
// Set up expired credentials
|
// Set up expired credentials
|
||||||
const expiredCredentials: AuthCredentials = {
|
const expiredCredentials: AuthCredentials = {
|
||||||
token: 'expired-token',
|
token: 'expired-token',
|
||||||
@@ -91,24 +91,15 @@ describe('AuthManager - Token Auto-Refresh Integration', () => {
|
|||||||
|
|
||||||
authManager = AuthManager.getInstance();
|
authManager = AuthManager.getInstance();
|
||||||
|
|
||||||
// Mock the Supabase refreshSession to return new tokens
|
// Get credentials returns them even if expired
|
||||||
const mockRefreshSession = vi
|
const credentials = authManager.getCredentials();
|
||||||
.fn()
|
|
||||||
.mockResolvedValue(mockRefreshedSession);
|
|
||||||
vi.spyOn(
|
|
||||||
authManager['supabaseClient'],
|
|
||||||
'refreshSession'
|
|
||||||
).mockImplementation(mockRefreshSession);
|
|
||||||
|
|
||||||
// Get credentials should trigger refresh
|
|
||||||
const credentials = await authManager.getCredentials();
|
|
||||||
|
|
||||||
expect(mockRefreshSession).toHaveBeenCalledTimes(1);
|
|
||||||
expect(credentials).not.toBeNull();
|
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
|
// Set up valid credentials
|
||||||
const validCredentials: AuthCredentials = {
|
const validCredentials: AuthCredentials = {
|
||||||
token: 'valid-token',
|
token: 'valid-token',
|
||||||
@@ -123,22 +114,14 @@ describe('AuthManager - Token Auto-Refresh Integration', () => {
|
|||||||
|
|
||||||
authManager = AuthManager.getInstance();
|
authManager = AuthManager.getInstance();
|
||||||
|
|
||||||
// Mock refresh to ensure it's not called
|
const credentials = authManager.getCredentials();
|
||||||
const mockRefreshSession = vi.fn();
|
|
||||||
vi.spyOn(
|
|
||||||
authManager['supabaseClient'],
|
|
||||||
'refreshSession'
|
|
||||||
).mockImplementation(mockRefreshSession);
|
|
||||||
|
|
||||||
const credentials = await authManager.getCredentials();
|
|
||||||
|
|
||||||
expect(mockRefreshSession).not.toHaveBeenCalled();
|
|
||||||
expect(credentials?.token).toBe('valid-token');
|
expect(credentials?.token).toBe('valid-token');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('Token Refresh Flow', () => {
|
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 = {
|
const expiredCredentials: AuthCredentials = {
|
||||||
token: 'old-token',
|
token: 'old-token',
|
||||||
refreshToken: 'old-refresh-token',
|
refreshToken: 'old-refresh-token',
|
||||||
@@ -162,23 +145,24 @@ describe('AuthManager - Token Auto-Refresh Integration', () => {
|
|||||||
'refreshSession'
|
'refreshSession'
|
||||||
).mockResolvedValue(mockRefreshedSession);
|
).mockResolvedValue(mockRefreshedSession);
|
||||||
|
|
||||||
const refreshedCredentials = await authManager.getCredentials();
|
// Explicitly call refreshToken() method
|
||||||
|
const refreshedCredentials = await authManager.refreshToken();
|
||||||
|
|
||||||
expect(refreshedCredentials).not.toBeNull();
|
expect(refreshedCredentials).not.toBeNull();
|
||||||
expect(refreshedCredentials?.token).toBe('new-access-token-xyz');
|
expect(refreshedCredentials.token).toBe('new-access-token-xyz');
|
||||||
expect(refreshedCredentials?.refreshToken).toBe('new-refresh-token-xyz');
|
expect(refreshedCredentials.refreshToken).toBe('new-refresh-token-xyz');
|
||||||
|
|
||||||
// Verify context was preserved
|
// Verify context was preserved
|
||||||
expect(refreshedCredentials?.selectedContext?.orgId).toBe('test-org');
|
expect(refreshedCredentials.selectedContext?.orgId).toBe('test-org');
|
||||||
expect(refreshedCredentials?.selectedContext?.briefId).toBe('test-brief');
|
expect(refreshedCredentials.selectedContext?.briefId).toBe('test-brief');
|
||||||
|
|
||||||
// Verify new expiration is in the future
|
// 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();
|
const now = Date.now();
|
||||||
expect(newExpiry).toBeGreaterThan(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 = {
|
const expiredCredentials: AuthCredentials = {
|
||||||
token: 'expired-token',
|
token: 'expired-token',
|
||||||
refreshToken: 'invalid-refresh-token',
|
refreshToken: 'invalid-refresh-token',
|
||||||
@@ -198,12 +182,11 @@ describe('AuthManager - Token Auto-Refresh Integration', () => {
|
|||||||
'refreshSession'
|
'refreshSession'
|
||||||
).mockRejectedValue(new Error('Refresh token expired'));
|
).mockRejectedValue(new Error('Refresh token expired'));
|
||||||
|
|
||||||
const credentials = await authManager.getCredentials();
|
// Explicit refreshToken() call should throw
|
||||||
|
await expect(authManager.refreshToken()).rejects.toThrow();
|
||||||
expect(credentials).toBeNull();
|
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should return null if no refresh token available', async () => {
|
it('should return expired credentials even without refresh token', () => {
|
||||||
const expiredCredentials: AuthCredentials = {
|
const expiredCredentials: AuthCredentials = {
|
||||||
token: 'expired-token',
|
token: 'expired-token',
|
||||||
// No refresh token
|
// No refresh token
|
||||||
@@ -217,18 +200,21 @@ describe('AuthManager - Token Auto-Refresh Integration', () => {
|
|||||||
|
|
||||||
authManager = AuthManager.getInstance();
|
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 = {
|
const credentialsWithoutExpiry: AuthCredentials = {
|
||||||
token: 'test-token',
|
token: 'test-token',
|
||||||
refreshToken: 'refresh-token',
|
refreshToken: 'refresh-token',
|
||||||
userId: 'test-user-id',
|
userId: 'test-user-id',
|
||||||
email: 'test@example.com',
|
email: 'test@example.com',
|
||||||
// Missing expiresAt
|
// Missing expiresAt - invalid token
|
||||||
savedAt: new Date().toISOString()
|
savedAt: new Date().toISOString()
|
||||||
} as any;
|
} as any;
|
||||||
|
|
||||||
@@ -236,16 +222,17 @@ describe('AuthManager - Token Auto-Refresh Integration', () => {
|
|||||||
|
|
||||||
authManager = AuthManager.getInstance();
|
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();
|
expect(credentials).toBeNull();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('Clock Skew Tolerance', () => {
|
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)
|
// Token expires in 15 seconds (within 30-second buffer)
|
||||||
|
// Supabase will handle refresh automatically
|
||||||
const almostExpiredCredentials: AuthCredentials = {
|
const almostExpiredCredentials: AuthCredentials = {
|
||||||
token: 'almost-expired-token',
|
token: 'almost-expired-token',
|
||||||
refreshToken: 'valid-refresh-token',
|
refreshToken: 'valid-refresh-token',
|
||||||
@@ -259,23 +246,16 @@ describe('AuthManager - Token Auto-Refresh Integration', () => {
|
|||||||
|
|
||||||
authManager = AuthManager.getInstance();
|
authManager = AuthManager.getInstance();
|
||||||
|
|
||||||
const mockRefreshSession = vi
|
const credentials = authManager.getCredentials();
|
||||||
.fn()
|
|
||||||
.mockResolvedValue(mockRefreshedSession);
|
|
||||||
vi.spyOn(
|
|
||||||
authManager['supabaseClient'],
|
|
||||||
'refreshSession'
|
|
||||||
).mockImplementation(mockRefreshSession);
|
|
||||||
|
|
||||||
const credentials = await authManager.getCredentials();
|
// Credentials are returned (Supabase handles auto-refresh in background)
|
||||||
|
expect(credentials).not.toBeNull();
|
||||||
// Should trigger refresh due to 30-second buffer
|
expect(credentials?.token).toBe('almost-expired-token');
|
||||||
expect(mockRefreshSession).toHaveBeenCalledTimes(1);
|
expect(credentials?.refreshToken).toBe('valid-refresh-token');
|
||||||
expect(credentials?.token).toBe('new-access-token-xyz');
|
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should not refresh token well before expiry', async () => {
|
it('should return valid token well before expiry', () => {
|
||||||
// Token expires in 5 minutes (well outside 30-second buffer)
|
// Token expires in 5 minutes
|
||||||
const validCredentials: AuthCredentials = {
|
const validCredentials: AuthCredentials = {
|
||||||
token: 'valid-token',
|
token: 'valid-token',
|
||||||
refreshToken: 'valid-refresh-token',
|
refreshToken: 'valid-refresh-token',
|
||||||
@@ -289,21 +269,17 @@ describe('AuthManager - Token Auto-Refresh Integration', () => {
|
|||||||
|
|
||||||
authManager = AuthManager.getInstance();
|
authManager = AuthManager.getInstance();
|
||||||
|
|
||||||
const mockRefreshSession = vi.fn();
|
const credentials = authManager.getCredentials();
|
||||||
vi.spyOn(
|
|
||||||
authManager['supabaseClient'],
|
|
||||||
'refreshSession'
|
|
||||||
).mockImplementation(mockRefreshSession);
|
|
||||||
|
|
||||||
const credentials = await authManager.getCredentials();
|
// Valid credentials are returned as-is
|
||||||
|
expect(credentials).not.toBeNull();
|
||||||
expect(mockRefreshSession).not.toHaveBeenCalled();
|
|
||||||
expect(credentials?.token).toBe('valid-token');
|
expect(credentials?.token).toBe('valid-token');
|
||||||
|
expect(credentials?.refreshToken).toBe('valid-refresh-token');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('Synchronous vs Async Methods', () => {
|
describe('Synchronous vs Async Methods', () => {
|
||||||
it('getCredentialsSync should not trigger refresh', () => {
|
it('getCredentials should return expired credentials', () => {
|
||||||
const expiredCredentials: AuthCredentials = {
|
const expiredCredentials: AuthCredentials = {
|
||||||
token: 'expired-token',
|
token: 'expired-token',
|
||||||
refreshToken: 'valid-refresh-token',
|
refreshToken: 'valid-refresh-token',
|
||||||
@@ -317,40 +293,17 @@ describe('AuthManager - Token Auto-Refresh Integration', () => {
|
|||||||
|
|
||||||
authManager = AuthManager.getInstance();
|
authManager = AuthManager.getInstance();
|
||||||
|
|
||||||
// Synchronous call should return null without refresh
|
// Returns credentials even if expired - Supabase will handle refresh
|
||||||
const credentials = authManager.getCredentialsSync();
|
const credentials = authManager.getCredentials();
|
||||||
|
|
||||||
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();
|
|
||||||
|
|
||||||
expect(credentials).not.toBeNull();
|
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', () => {
|
describe('Multiple Concurrent Calls', () => {
|
||||||
it('should handle concurrent getCredentials calls gracefully', async () => {
|
it('should handle concurrent getCredentials calls gracefully', () => {
|
||||||
const expiredCredentials: AuthCredentials = {
|
const expiredCredentials: AuthCredentials = {
|
||||||
token: 'expired-token',
|
token: 'expired-token',
|
||||||
refreshToken: 'valid-refresh-token',
|
refreshToken: 'valid-refresh-token',
|
||||||
@@ -364,29 +317,20 @@ describe('AuthManager - Token Auto-Refresh Integration', () => {
|
|||||||
|
|
||||||
authManager = AuthManager.getInstance();
|
authManager = AuthManager.getInstance();
|
||||||
|
|
||||||
const mockRefreshSession = vi
|
// Make multiple concurrent calls (synchronous now)
|
||||||
.fn()
|
const creds1 = authManager.getCredentials();
|
||||||
.mockResolvedValue(mockRefreshedSession);
|
const creds2 = authManager.getCredentials();
|
||||||
vi.spyOn(
|
const creds3 = authManager.getCredentials();
|
||||||
authManager['supabaseClient'],
|
|
||||||
'refreshSession'
|
|
||||||
).mockImplementation(mockRefreshSession);
|
|
||||||
|
|
||||||
// Make multiple concurrent calls
|
// All should get the same credentials (even if expired)
|
||||||
const [creds1, creds2, creds3] = await Promise.all([
|
expect(creds1?.token).toBe('expired-token');
|
||||||
authManager.getCredentials(),
|
expect(creds2?.token).toBe('expired-token');
|
||||||
authManager.getCredentials(),
|
expect(creds3?.token).toBe('expired-token');
|
||||||
authManager.getCredentials()
|
|
||||||
]);
|
|
||||||
|
|
||||||
// All should get the refreshed token
|
// All include refresh token for Supabase to use
|
||||||
expect(creds1?.token).toBe('new-access-token-xyz');
|
expect(creds1?.refreshToken).toBe('valid-refresh-token');
|
||||||
expect(creds2?.token).toBe('new-access-token-xyz');
|
expect(creds2?.refreshToken).toBe('valid-refresh-token');
|
||||||
expect(creds3?.token).toBe('new-access-token-xyz');
|
expect(creds3?.refreshToken).toBe('valid-refresh-token');
|
||||||
|
|
||||||
// Refresh might be called multiple times, but that's okay
|
|
||||||
// (ideally we'd debounce, but this is acceptable behavior)
|
|
||||||
expect(mockRefreshSession).toHaveBeenCalled();
|
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user