Skip to content

Commit 0edfb1b

Browse files
committed
fix: enhance authentication service to handle decryption error
- Added error handling for decryption failures in the authentication service, notifying users and deleting corrupted data. - Ensured that the onDidChange listener is registered even when no stored data exists or decryption fails. - Updated tests to cover new error handling scenarios and listener registration. Signed-off-by: Denis Golovin <dgolovin@redhat.com>
1 parent 61f397e commit 0edfb1b

File tree

3 files changed

+134
-8
lines changed

3 files changed

+134
-8
lines changed

__mocks__/@podman-desktop/api.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ const plugin = {
5050
});
5151
},
5252
showInformationMessage: vi.fn(),
53+
showWarningMessage: vi.fn(),
5354
},
5455
env: {
5556
createTelemetryLogger: vi.fn().mockImplementation(() => ({

src/authentication-service.spec.ts

Lines changed: 98 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
/* eslint-disable sonarjs/no-nested-functions */
1919

2020
import type { ExtensionContext } from '@podman-desktop/api';
21-
import { authentication } from '@podman-desktop/api';
21+
import { authentication, window } from '@podman-desktop/api';
2222
import type { BaseClient, TokenSet } from 'openid-client';
2323
import { Issuer } from 'openid-client';
2424
import { beforeEach, expect, test, vi } from 'vitest';
@@ -110,3 +110,100 @@ test('Authentication service loads tokens form secret storage during initializat
110110
expect(extensionContext.secrets.get).toHaveBeenCalledOnce();
111111
expect((await service.getSessions(['scope1', 'scope3', 'scope4'])).length).toBe(0);
112112
});
113+
114+
test('Authentication service handles decryption errors gracefully', async () => {
115+
const decryptionError = new Error('Error while decrypting the ciphertext provided to safeStorage.decryptString');
116+
117+
vi.mocked(window.showWarningMessage).mockResolvedValue('OK');
118+
119+
const onDidChangeCallback = vi.fn();
120+
const extensionContext: ExtensionContext = {
121+
secrets: {
122+
get: vi.fn().mockRejectedValue(decryptionError),
123+
store: vi.fn(),
124+
delete: vi.fn().mockResolvedValue(undefined),
125+
onDidChange: onDidChangeCallback,
126+
},
127+
subscriptions: [],
128+
} as unknown as ExtensionContext;
129+
130+
const service = await RedHatAuthenticationService.build(extensionContext, getAuthConfig());
131+
await service.initialize();
132+
133+
// Verify secrets.get was called
134+
expect(extensionContext.secrets.get).toHaveBeenCalledOnce();
135+
136+
// Verify warning message was shown to user
137+
expect(window.showWarningMessage).toHaveBeenCalledWith(
138+
expect.stringContaining('could not be restored'),
139+
'OK',
140+
);
141+
142+
// Verify corrupted data was deleted
143+
expect(extensionContext.secrets.delete).toHaveBeenCalledOnce();
144+
145+
// Verify onDidChange listener was still registered
146+
expect(onDidChangeCallback).toHaveBeenCalledOnce();
147+
148+
// Verify no sessions exist after error
149+
expect((await service.getSessions()).length).toBe(0);
150+
});
151+
152+
test('Authentication service registers onDidChange listener even when no stored data exists', async () => {
153+
const onDidChangeCallback = vi.fn();
154+
const extensionContext: ExtensionContext = {
155+
secrets: {
156+
get: vi.fn().mockResolvedValue(undefined),
157+
store: vi.fn(),
158+
delete: vi.fn(),
159+
onDidChange: onDidChangeCallback,
160+
},
161+
subscriptions: [],
162+
} as unknown as ExtensionContext;
163+
164+
const service = await RedHatAuthenticationService.build(extensionContext, getAuthConfig());
165+
await service.initialize();
166+
167+
// Verify secrets.get was called
168+
expect(extensionContext.secrets.get).toHaveBeenCalledOnce();
169+
170+
// Verify onDidChange listener was registered even with no stored data
171+
expect(onDidChangeCallback).toHaveBeenCalledOnce();
172+
173+
// Verify no sessions exist
174+
expect((await service.getSessions()).length).toBe(0);
175+
});
176+
177+
test('Authentication service handles delete error during decryption error recovery', async () => {
178+
const decryptionError = new Error('Error while decrypting the ciphertext');
179+
const deleteError = new Error('Failed to delete');
180+
181+
// Clear mock from previous tests
182+
vi.mocked(window.showWarningMessage).mockClear();
183+
vi.mocked(window.showWarningMessage).mockResolvedValue('OK');
184+
185+
const onDidChangeCallback = vi.fn();
186+
const extensionContext: ExtensionContext = {
187+
secrets: {
188+
get: vi.fn().mockRejectedValue(decryptionError),
189+
store: vi.fn(),
190+
delete: vi.fn().mockRejectedValue(deleteError),
191+
onDidChange: onDidChangeCallback,
192+
},
193+
subscriptions: [],
194+
} as unknown as ExtensionContext;
195+
196+
const service = await RedHatAuthenticationService.build(extensionContext, getAuthConfig());
197+
198+
// Should not throw even if delete fails
199+
await expect(service.initialize()).resolves.not.toThrow();
200+
201+
// Verify warning was still shown
202+
expect(window.showWarningMessage).toHaveBeenCalledOnce();
203+
204+
// Verify delete was attempted
205+
expect(extensionContext.secrets.delete).toHaveBeenCalledOnce();
206+
207+
// Verify onDidChange listener was still registered
208+
expect(onDidChangeCallback).toHaveBeenCalledOnce();
209+
});

src/authentication-service.ts

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,34 @@ export class RedHatAuthenticationService {
155155
}
156156

157157
public async initialize(): Promise<void> {
158-
const storedData = await this.context.secrets.get(this.config.serviceId);
158+
let storedData: string | undefined;
159+
160+
// Try to get stored session data - may throw if decryption fails
161+
try {
162+
storedData = await this.context.secrets.get(this.config.serviceId);
163+
} catch (error) {
164+
// Decryption failed - encryption key likely changed after system update
165+
Logger.error(`Failed to decrypt stored sessions: ${error}`);
166+
167+
// Notify user about the issue
168+
await window.showWarningMessage(
169+
'Your saved Red Hat SSO sessions could not be restored. ' +
170+
'This can happen after a system update changes the encryption key. ' +
171+
'Please sign in again.',
172+
'OK',
173+
);
174+
175+
// Delete the corrupted data from storage
176+
try {
177+
await this.context.secrets.delete(this.config.serviceId);
178+
} catch {
179+
// Ignore delete errors
180+
}
181+
182+
// Don't return - still need to register the onDidChange listener below
183+
storedData = undefined;
184+
}
185+
159186
if (storedData) {
160187
try {
161188
const sessions = this.parseStoredData(storedData);
@@ -197,13 +224,14 @@ export class RedHatAuthenticationService {
197224
Logger.info('Failed to initialize stored data');
198225
await this.clearSessions();
199226
}
200-
201-
this._disposables.push(
202-
this.context.secrets.onDidChange(() => {
203-
this.checkForUpdates().catch(console.error);
204-
}),
205-
);
206227
}
228+
229+
// Always register the listener to detect session changes
230+
this._disposables.push(
231+
this.context.secrets.onDidChange(() => {
232+
this.checkForUpdates().catch(console.error);
233+
}),
234+
);
207235
}
208236

209237
private parseStoredData(data: string): IStoredSession[] {

0 commit comments

Comments
 (0)