chore: apply requested PR changes and fix CI

This commit is contained in:
Ralph Khreish
2025-10-15 16:48:10 +02:00
parent 01cbbe97f2
commit b11dacece9
6 changed files with 41 additions and 40 deletions

View File

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

View File

@@ -81,7 +81,8 @@ export class AuthManager {
/** /**
* Get stored authentication credentials * Get stored authentication credentials
* Returns credentials even if expired - Supabase will handle automatic refresh * 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.
*/ */
getCredentials(): AuthCredentials | null { getCredentials(): AuthCredentials | null {
return this.credentialStore.getCredentials(); return this.credentialStore.getCredentials();
@@ -169,7 +170,7 @@ export class AuthManager {
* Check if authenticated * Check if authenticated
*/ */
isAuthenticated(): boolean { isAuthenticated(): boolean {
return this.credentialStore.hasValidCredentials(); return this.credentialStore.hasCredentials();
} }
/** /**

View File

@@ -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');
@@ -108,7 +108,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 +126,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 +146,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 +164,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 +185,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 +203,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 +244,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 +264,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 +279,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);
}); });
}); });
}); });

View File

@@ -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(
@@ -451,7 +451,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 +465,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 +481,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 +495,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 +510,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 +518,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 });
}); });
}); });

View File

@@ -200,9 +200,9 @@ export class CredentialStore {
} }
/** /**
* Check if credentials exist and are valid * Check if credentials exist (regardless of expiration status)
*/ */
hasValidCredentials(): boolean { hasCredentials(): boolean {
const credentials = this.getCredentials({ allowExpired: true }); const credentials = this.getCredentials({ allowExpired: true });
return credentials !== null; return credentials !== null;
} }

View File

@@ -50,7 +50,7 @@ describe('AuthManager Token Refresh', () => {
} }
}); });
it('should return expired credentials for Supabase to refresh', async () => { it('should return expired credentials for Supabase to refresh', () => {
// 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',
@@ -65,14 +65,14 @@ describe('AuthManager Token Refresh', () => {
// Get credentials should return them even if expired // Get credentials should return them even if expired
// Supabase will handle the refresh automatically // Supabase will handle the refresh automatically
const credentials = await authManager.getCredentials(); const credentials = authManager.getCredentials();
expect(credentials).not.toBeNull(); expect(credentials).not.toBeNull();
expect(credentials?.token).toBe('expired_access_token'); expect(credentials?.token).toBe('expired_access_token');
expect(credentials?.refreshToken).toBe('valid_refresh_token'); expect(credentials?.refreshToken).toBe('valid_refresh_token');
}); });
it('should return valid credentials', async () => { it('should return valid credentials', () => {
// 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',
@@ -85,12 +85,12 @@ describe('AuthManager Token Refresh', () => {
credentialStore.saveCredentials(validCredentials); credentialStore.saveCredentials(validCredentials);
const credentials = await authManager.getCredentials(); const credentials = authManager.getCredentials();
expect(credentials?.token).toBe('valid_access_token'); expect(credentials?.token).toBe('valid_access_token');
}); });
it('should return expired credentials even without 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/Supabase to handle // We still return them - it's up to the caller/Supabase to handle
const expiredCredentials: AuthCredentials = { const expiredCredentials: AuthCredentials = {
@@ -104,19 +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 // Returns credentials even if expired
expect(credentials).not.toBeNull(); expect(credentials).not.toBeNull();
expect(credentials?.token).toBe('expired_access_token'); expect(credentials?.token).toBe('expired_access_token');
}); });
it('should return null if no credentials exist', async () => { it('should return null if no credentials exist', () => {
const credentials = await authManager.getCredentials(); const credentials = authManager.getCredentials();
expect(credentials).toBeNull(); expect(credentials).toBeNull();
}); });
it('should return credentials regardless of refresh token validity', async () => { it('should return credentials regardless of refresh token validity', () => {
// 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',
@@ -129,7 +129,7 @@ describe('AuthManager Token Refresh', () => {
credentialStore.saveCredentials(expiredCredentials); credentialStore.saveCredentials(expiredCredentials);
const credentials = await authManager.getCredentials(); const credentials = authManager.getCredentials();
// Returns credentials - Supabase will attempt refresh and handle failure // Returns credentials - Supabase will attempt refresh and handle failure
expect(credentials).not.toBeNull(); expect(credentials).not.toBeNull();