Compare commits
4 Commits
task-maste
...
ralph/fix/
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
08d3720f92 | ||
|
|
0a9ec7f44b | ||
|
|
8ee37d6812 | ||
|
|
9986b9838b |
@@ -1,11 +1,11 @@
|
|||||||
{
|
{
|
||||||
"mode": "exit",
|
"mode": "pre",
|
||||||
"tag": "rc",
|
"tag": "rc",
|
||||||
"initialVersions": {
|
"initialVersions": {
|
||||||
"task-master-ai": "0.28.0",
|
"task-master-ai": "0.29.0-rc.0",
|
||||||
"@tm/cli": "",
|
"@tm/cli": "",
|
||||||
"docs": "0.0.5",
|
"docs": "0.0.5",
|
||||||
"extension": "0.25.5",
|
"extension": "0.25.6-rc.0",
|
||||||
"@tm/ai-sdk-provider-grok-cli": "",
|
"@tm/ai-sdk-provider-grok-cli": "",
|
||||||
"@tm/build-config": "",
|
"@tm/build-config": "",
|
||||||
"@tm/claude-code-plugin": "0.0.1",
|
"@tm/claude-code-plugin": "0.0.1",
|
||||||
@@ -16,6 +16,7 @@
|
|||||||
"mean-planes-wave",
|
"mean-planes-wave",
|
||||||
"nice-ways-hope",
|
"nice-ways-hope",
|
||||||
"plain-falcons-serve",
|
"plain-falcons-serve",
|
||||||
|
"silent-bushes-grow",
|
||||||
"smart-owls-relax"
|
"smart-owls-relax"
|
||||||
]
|
]
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,5 +1,11 @@
|
|||||||
# task-master-ai
|
# task-master-ai
|
||||||
|
|
||||||
|
## 0.29.0-rc.1
|
||||||
|
|
||||||
|
### Patch Changes
|
||||||
|
|
||||||
|
- [#1299](https://github.com/eyaltoledano/claude-task-master/pull/1299) [`a6c5152`](https://github.com/eyaltoledano/claude-task-master/commit/a6c5152f20edd8717cf1aea34e7c178b1261aa99) Thanks [@Crunchyman-ralph](https://github.com/Crunchyman-ralph)! - Improve refresh token when authenticating
|
||||||
|
|
||||||
## 0.29.0-rc.0
|
## 0.29.0-rc.0
|
||||||
|
|
||||||
### Minor Changes
|
### Minor Changes
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
{
|
{
|
||||||
"name": "task-master-ai",
|
"name": "task-master-ai",
|
||||||
"version": "0.29.0-rc.0",
|
"version": "0.29.0-rc.1",
|
||||||
"description": "A task management system for ambitious AI-driven development that doesn't overwhelm and confuse Cursor.",
|
"description": "A task management system for ambitious AI-driven development that doesn't overwhelm and confuse Cursor.",
|
||||||
"main": "index.js",
|
"main": "index.js",
|
||||||
"type": "module",
|
"type": "module",
|
||||||
|
|||||||
@@ -21,11 +21,16 @@ const CredentialStoreSpy = vi.fn();
|
|||||||
vi.mock('./credential-store.js', () => {
|
vi.mock('./credential-store.js', () => {
|
||||||
return {
|
return {
|
||||||
CredentialStore: class {
|
CredentialStore: class {
|
||||||
|
static getInstance(config?: any) {
|
||||||
|
return new (this as any)(config);
|
||||||
|
}
|
||||||
|
static resetInstance() {
|
||||||
|
// Mock reset instance method
|
||||||
|
}
|
||||||
constructor(config: any) {
|
constructor(config: any) {
|
||||||
CredentialStoreSpy(config);
|
CredentialStoreSpy(config);
|
||||||
this.getCredentials = vi.fn(() => null);
|
|
||||||
}
|
}
|
||||||
getCredentials() {
|
getCredentials(_options?: any) {
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
saveCredentials() {}
|
saveCredentials() {}
|
||||||
@@ -85,7 +90,7 @@ describe('AuthManager Singleton', () => {
|
|||||||
expect(instance1).toBe(instance2);
|
expect(instance1).toBe(instance2);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should use config on first call', () => {
|
it('should use config on first call', async () => {
|
||||||
const config = {
|
const config = {
|
||||||
baseUrl: 'https://test.auth.com',
|
baseUrl: 'https://test.auth.com',
|
||||||
configDir: '/test/config',
|
configDir: '/test/config',
|
||||||
@@ -101,7 +106,7 @@ describe('AuthManager Singleton', () => {
|
|||||||
|
|
||||||
// Verify the config is passed to internal components through observable behavior
|
// Verify the config is passed to internal components through observable behavior
|
||||||
// getCredentials would look in the configured file path
|
// getCredentials would look in the configured file path
|
||||||
const credentials = instance.getCredentials();
|
const credentials = await instance.getCredentials();
|
||||||
expect(credentials).toBeNull(); // File doesn't exist, but config was propagated correctly
|
expect(credentials).toBeNull(); // File doesn't exist, but config was propagated correctly
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -30,6 +30,7 @@ export class AuthManager {
|
|||||||
private supabaseClient: SupabaseAuthClient;
|
private supabaseClient: SupabaseAuthClient;
|
||||||
private organizationService?: OrganizationService;
|
private organizationService?: OrganizationService;
|
||||||
private logger = getLogger('AuthManager');
|
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);
|
||||||
@@ -37,7 +38,10 @@ export class AuthManager {
|
|||||||
this.oauthService = new OAuthService(this.credentialStore, config);
|
this.oauthService = new OAuthService(this.credentialStore, config);
|
||||||
|
|
||||||
// Initialize Supabase client with session restoration
|
// Initialize Supabase client with session restoration
|
||||||
this.initializeSupabaseSession();
|
// Fire-and-forget with catch handler to prevent unhandled rejections
|
||||||
|
this.initializeSupabaseSession().catch(() => {
|
||||||
|
// Errors are already logged in initializeSupabaseSession
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -82,38 +86,49 @@ export class AuthManager {
|
|||||||
* Automatically refreshes the token if expired
|
* Automatically refreshes the token if expired
|
||||||
*/
|
*/
|
||||||
async getCredentials(): Promise<AuthCredentials | null> {
|
async getCredentials(): Promise<AuthCredentials | null> {
|
||||||
const credentials = this.credentialStore.getCredentials();
|
const credentials = this.credentialStore.getCredentials({
|
||||||
|
allowExpired: true
|
||||||
|
});
|
||||||
|
|
||||||
// If credentials exist but are expired, try to refresh
|
|
||||||
if (!credentials) {
|
if (!credentials) {
|
||||||
const expiredCredentials = this.credentialStore.getCredentials({
|
return null;
|
||||||
allowExpired: true
|
}
|
||||||
});
|
|
||||||
|
|
||||||
// Check if we have any credentials at all
|
// Check if credentials are expired (with 30-second clock skew buffer)
|
||||||
if (!expiredCredentials) {
|
const CLOCK_SKEW_MS = 30_000;
|
||||||
// No credentials found
|
const isExpired = credentials.expiresAt
|
||||||
return null;
|
? 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;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check if refresh token is available
|
|
||||||
if (!expiredCredentials.refreshToken) {
|
|
||||||
this.logger.warn(
|
|
||||||
'Token expired but no refresh token available. Please re-authenticate.'
|
|
||||||
);
|
|
||||||
return null;
|
|
||||||
}
|
|
||||||
|
|
||||||
// Attempt refresh
|
|
||||||
try {
|
try {
|
||||||
this.logger.info('Token expired, attempting automatic refresh...');
|
this.logger.info('Token expired, attempting automatic refresh...');
|
||||||
return await this.refreshToken();
|
this.refreshPromise = this.refreshToken();
|
||||||
|
const result = await this.refreshPromise;
|
||||||
|
return result;
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
this.logger.warn('Automatic token refresh failed:', error);
|
this.logger.warn('Automatic token refresh failed:', error);
|
||||||
return null;
|
return null;
|
||||||
|
} finally {
|
||||||
|
this.refreshPromise = null;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Return null if expired and no refresh token
|
||||||
|
if (isExpired) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
return credentials;
|
return credentials;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
176
packages/tm-core/tests/auth/auth-refresh.test.ts
Normal file
176
packages/tm-core/tests/auth/auth-refresh.test.ts
Normal file
@@ -0,0 +1,176 @@
|
|||||||
|
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||||
|
import fs from 'fs';
|
||||||
|
import os from 'os';
|
||||||
|
import path from 'path';
|
||||||
|
import type { Session } from '@supabase/supabase-js';
|
||||||
|
import { AuthManager } from '../../src/auth/auth-manager';
|
||||||
|
import { CredentialStore } from '../../src/auth/credential-store';
|
||||||
|
import type { AuthCredentials } from '../../src/auth/types';
|
||||||
|
|
||||||
|
describe('AuthManager Token Refresh', () => {
|
||||||
|
let authManager: AuthManager;
|
||||||
|
let credentialStore: CredentialStore;
|
||||||
|
let tmpDir: string;
|
||||||
|
let authFile: string;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
// Reset singletons
|
||||||
|
AuthManager.resetInstance();
|
||||||
|
CredentialStore.resetInstance();
|
||||||
|
|
||||||
|
// Create temporary directory for test isolation
|
||||||
|
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'tm-auth-refresh-'));
|
||||||
|
authFile = path.join(tmpDir, 'auth.json');
|
||||||
|
|
||||||
|
// Initialize AuthManager with test config (this will create CredentialStore internally)
|
||||||
|
authManager = AuthManager.getInstance({
|
||||||
|
configDir: tmpDir,
|
||||||
|
configFile: authFile
|
||||||
|
});
|
||||||
|
|
||||||
|
// Get the CredentialStore instance that AuthManager created
|
||||||
|
credentialStore = CredentialStore.getInstance();
|
||||||
|
credentialStore.clearCredentials();
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
// Clean up
|
||||||
|
try {
|
||||||
|
credentialStore.clearCredentials();
|
||||||
|
} catch {
|
||||||
|
// Ignore cleanup errors
|
||||||
|
}
|
||||||
|
AuthManager.resetInstance();
|
||||||
|
CredentialStore.resetInstance();
|
||||||
|
vi.restoreAllMocks();
|
||||||
|
|
||||||
|
// Remove temporary directory
|
||||||
|
if (tmpDir && fs.existsSync(tmpDir)) {
|
||||||
|
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should not make concurrent refresh requests', async () => {
|
||||||
|
// Set up expired credentials with refresh token
|
||||||
|
const expiredCredentials: AuthCredentials = {
|
||||||
|
token: 'expired_access_token',
|
||||||
|
refreshToken: 'valid_refresh_token',
|
||||||
|
userId: 'test-user-id',
|
||||||
|
email: 'test@example.com',
|
||||||
|
expiresAt: new Date(Date.now() - 1000).toISOString(), // Expired 1 second ago
|
||||||
|
savedAt: new Date().toISOString()
|
||||||
|
};
|
||||||
|
|
||||||
|
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()
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
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);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should return valid credentials without attempting refresh', async () => {
|
||||||
|
// Set up valid (non-expired) credentials
|
||||||
|
const validCredentials: AuthCredentials = {
|
||||||
|
token: 'valid_access_token',
|
||||||
|
refreshToken: 'valid_refresh_token',
|
||||||
|
userId: 'test-user-id',
|
||||||
|
email: 'test@example.com',
|
||||||
|
expiresAt: new Date(Date.now() + 3600000).toISOString(), // Expires in 1 hour
|
||||||
|
savedAt: new Date().toISOString()
|
||||||
|
};
|
||||||
|
|
||||||
|
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();
|
||||||
|
|
||||||
|
expect(credentials?.token).toBe('valid_access_token');
|
||||||
|
expect(refreshTokenSpy).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should return null if credentials are expired with no refresh token', async () => {
|
||||||
|
// Set up expired credentials WITHOUT refresh token
|
||||||
|
const expiredCredentials: AuthCredentials = {
|
||||||
|
token: 'expired_access_token',
|
||||||
|
refreshToken: undefined,
|
||||||
|
userId: 'test-user-id',
|
||||||
|
email: 'test@example.com',
|
||||||
|
expiresAt: new Date(Date.now() - 1000).toISOString(), // Expired 1 second ago
|
||||||
|
savedAt: new Date().toISOString()
|
||||||
|
};
|
||||||
|
|
||||||
|
credentialStore.saveCredentials(expiredCredentials);
|
||||||
|
|
||||||
|
const credentials = await 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 () => {
|
||||||
|
// Set up expired credentials with refresh token
|
||||||
|
const expiredCredentials: AuthCredentials = {
|
||||||
|
token: 'expired_access_token',
|
||||||
|
refreshToken: 'invalid_refresh_token',
|
||||||
|
userId: 'test-user-id',
|
||||||
|
email: 'test@example.com',
|
||||||
|
expiresAt: new Date(Date.now() - 1000).toISOString(),
|
||||||
|
savedAt: new Date().toISOString()
|
||||||
|
};
|
||||||
|
|
||||||
|
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 = await authManager.getCredentials();
|
||||||
|
|
||||||
|
expect(credentials).toBeNull();
|
||||||
|
expect(refreshTokenSpy).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -6,6 +6,9 @@
|
|||||||
*/
|
*/
|
||||||
|
|
||||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||||
|
import fs from 'fs';
|
||||||
|
import os from 'os';
|
||||||
|
import path from 'path';
|
||||||
import type { Session } from '@supabase/supabase-js';
|
import type { Session } from '@supabase/supabase-js';
|
||||||
import { AuthManager } from '../../src/auth/auth-manager';
|
import { AuthManager } from '../../src/auth/auth-manager';
|
||||||
import { CredentialStore } from '../../src/auth/credential-store';
|
import { CredentialStore } from '../../src/auth/credential-store';
|
||||||
@@ -14,6 +17,8 @@ import type { AuthCredentials } from '../../src/auth/types';
|
|||||||
describe('AuthManager - Token Auto-Refresh Integration', () => {
|
describe('AuthManager - Token Auto-Refresh Integration', () => {
|
||||||
let authManager: AuthManager;
|
let authManager: AuthManager;
|
||||||
let credentialStore: CredentialStore;
|
let credentialStore: CredentialStore;
|
||||||
|
let tmpDir: string;
|
||||||
|
let authFile: string;
|
||||||
|
|
||||||
// Mock Supabase session that will be returned on refresh
|
// Mock Supabase session that will be returned on refresh
|
||||||
const mockRefreshedSession: Session = {
|
const mockRefreshedSession: Session = {
|
||||||
@@ -34,10 +39,21 @@ describe('AuthManager - Token Auto-Refresh Integration', () => {
|
|||||||
};
|
};
|
||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
// Reset AuthManager singleton
|
// Reset singletons
|
||||||
AuthManager.resetInstance();
|
AuthManager.resetInstance();
|
||||||
|
CredentialStore.resetInstance();
|
||||||
|
|
||||||
// Clear any existing credentials
|
// Create temporary directory for test isolation
|
||||||
|
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'tm-auth-integration-'));
|
||||||
|
authFile = path.join(tmpDir, 'auth.json');
|
||||||
|
|
||||||
|
// Initialize AuthManager with test config (this will create CredentialStore internally)
|
||||||
|
authManager = AuthManager.getInstance({
|
||||||
|
configDir: tmpDir,
|
||||||
|
configFile: authFile
|
||||||
|
});
|
||||||
|
|
||||||
|
// Get the CredentialStore instance that AuthManager created
|
||||||
credentialStore = CredentialStore.getInstance();
|
credentialStore = CredentialStore.getInstance();
|
||||||
credentialStore.clearCredentials();
|
credentialStore.clearCredentials();
|
||||||
});
|
});
|
||||||
@@ -50,7 +66,13 @@ describe('AuthManager - Token Auto-Refresh Integration', () => {
|
|||||||
// Ignore cleanup errors
|
// Ignore cleanup errors
|
||||||
}
|
}
|
||||||
AuthManager.resetInstance();
|
AuthManager.resetInstance();
|
||||||
|
CredentialStore.resetInstance();
|
||||||
vi.restoreAllMocks();
|
vi.restoreAllMocks();
|
||||||
|
|
||||||
|
// Remove temporary directory
|
||||||
|
if (tmpDir && fs.existsSync(tmpDir)) {
|
||||||
|
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||||
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('Expired Token Detection', () => {
|
describe('Expired Token Detection', () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user