Skip to content

Commit 00fc90a

Browse files
PredixxPredixx
authored
fix(auth): auto-logout on server URL change, drop redundant Clear Credentials prompts (#327) (#329)
Remove the two overlapping "Clear Credentials" notifications shown on an artemis.serverUrl change (one whose button never actually cleared anything). When the server URL changes while logged in, clear the stored credentials, disconnect, and return to the login view; a token issued by the previous server is not valid on a different one. Also remove the now-dead URL-change helper chain: AuthManager.isServerUrlChanged and getStoredLoginServerUrl, the serverUrl parameter and ARTEMIS_SERVER_URL secret persistence, and the unused showLogin callback on AuthFlowHandler.
1 parent c84890e commit 00fc90a

13 files changed

Lines changed: 52 additions & 160 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ All notable changes to the Artemis VS Code extension will be documented in this
77
### Changed
88

99
- **WebSocket status bar:** Removed the `artemis.showWebSocketStatusBar` setting. The connection indicator now appears automatically only when there is a problem; enable `artemis.developerMode` to keep it always visible with full diagnostics on hover. When the connection drops, students now see a plain-language explanation (no "WS" jargon) instead of a technical label.
10+
- **Server URL change:** Removed the manual "Clear Credentials" prompts that appeared when the Artemis server URL changed. Changing the server while logged in now logs you out automatically and returns you to the login view (a session is not valid across servers); the logout command remains for clearing credentials on demand.
1011

1112
## [0.4.8] - 2026-06-24
1213

‎extension/src/extension.ts‎

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import {
2222
detectPlatformCapabilities,
2323
initializeTheiaContext,
2424
} from '@extension/theia';
25-
import { VSCODE_CONFIG } from '@extension/utils';
25+
import { resolveServerUrl, VSCODE_CONFIG } from '@extension/utils';
2626
import { wireDataCollection } from '@dataCollection';
2727
import { createTelemetryManager } from '@telemetry';
2828

@@ -232,9 +232,9 @@ export async function activate(context: vscode.ExtensionContext) {
232232
contextStore,
233233
});
234234

235-
// Configuration listener
235+
// Configuration listener for the Artemis server URL.
236236
if (theiaEnv.isManagedEnvironment) {
237-
// In managed Theia environments, revert unauthorized changes to locked settings
237+
// In managed Theia environments, revert unauthorized changes to the locked setting.
238238
context.subscriptions.push(vscode.workspace.onDidChangeConfiguration(event => {
239239
if (event.affectsConfiguration(`${VSCODE_CONFIG.ARTEMIS_SECTION}.${VSCODE_CONFIG.SERVER_URL_KEY}`) && theiaEnv.artemisUrl) {
240240
const config = vscode.workspace.getConfiguration(VSCODE_CONFIG.ARTEMIS_SECTION);
@@ -243,26 +243,31 @@ export async function activate(context: vscode.ExtensionContext) {
243243
}
244244
}));
245245
} else {
246-
// In VS Code: prompt user to clear credentials when server URL changes
247-
context.subscriptions.push(vscode.workspace.onDidChangeConfiguration(event => {
248-
if (event.affectsConfiguration(`${VSCODE_CONFIG.ARTEMIS_SECTION}.${VSCODE_CONFIG.SERVER_URL_KEY}`)) {
249-
logger.info('Artemis server URL configuration changed', LogCategory.CONFIG);
250-
const config = vscode.workspace.getConfiguration(VSCODE_CONFIG.ARTEMIS_SECTION);
251-
const newServerUrl = config.get<string>(VSCODE_CONFIG.SERVER_URL_KEY);
252-
if (newServerUrl) {
253-
vscode.window.showInformationMessage(
254-
`Artemis server URL updated to: ${newServerUrl}. You may need to log in again if you were authenticated to a different server.`,
255-
'Clear Credentials'
256-
).then(selection => {
257-
if (selection === 'Clear Credentials') {
258-
authManager.clear().then(async () => {
259-
await updateAuthContext(false);
260-
vscode.window.showInformationMessage('Stored credentials cleared. Please log in again.');
261-
artemisWebviewProvider.showLogin();
262-
});
263-
}
264-
});
246+
// On Desktop the server URL is freely configurable. When it actually changes
247+
// while the user is logged in, clear the stored credentials and return to the
248+
// login view: a token issued by the previous server is not valid on a different
249+
// one. The last URL is tracked so a no-op settings save does not log the user out.
250+
let lastServerUrl = resolveServerUrl();
251+
context.subscriptions.push(vscode.workspace.onDidChangeConfiguration(async event => {
252+
if (!event.affectsConfiguration(`${VSCODE_CONFIG.ARTEMIS_SECTION}.${VSCODE_CONFIG.SERVER_URL_KEY}`)) {
253+
return;
254+
}
255+
const newServerUrl = resolveServerUrl();
256+
if (newServerUrl === lastServerUrl) {
257+
return;
258+
}
259+
lastServerUrl = newServerUrl;
260+
try {
261+
if (!(await authManager.hasAuthToken())) {
262+
return;
265263
}
264+
logger.info('Artemis server URL changed; clearing credentials stored for the previous server', LogCategory.CONFIG);
265+
await authManager.clear();
266+
await updateAuthContext(false);
267+
artemisWebviewProvider.showLogin();
268+
vscode.window.showInformationMessage('Artemis server changed. Please log in again.');
269+
} catch (error) {
270+
logger.error('Failed to clear credentials after server URL change', LogCategory.AUTH, error);
266271
}
267272
}));
268273
}

‎extension/src/extension/api/artemisApi.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,7 @@ export class ArtemisApiService {
380380
}
381381

382382
// Store as cookie string — Desktop auth sends Cookie header, not Bearer
383-
await this.authManager.storeArtemisCredentials(jwtCookie, this.getServerUrl(), rememberMe);
383+
await this.authManager.storeArtemisCredentials(jwtCookie, rememberMe);
384384

385385
return { success: true };
386386
}

‎extension/src/extension/provider/artemisWebviewProvider.ts‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,6 @@ export class ArtemisWebviewProvider extends BaseWebviewProvider implements vscod
235235
{
236236
onAuthenticated: (userInfo) => this._navigationFacade.navigateToStartPage(userInfo),
237237
hideLoadingAndSendServerUrl: () => this._navigationFacade.hideLoadingAndSendServerUrl(),
238-
showLogin: () => this._navigationFacade.showLogin(),
239238
},
240239
);
241240

@@ -298,9 +297,6 @@ export class ArtemisWebviewProvider extends BaseWebviewProvider implements vscod
298297
this._postMessageSafe(message);
299298
});
300299

301-
// Check if server URL has changed and clear credentials if needed
302-
this._authFlowHandler.checkServerUrlChange();
303-
304300
// Check for existing authentication and auto-login if valid
305301
this._authFlowHandler.checkExistingAuthentication();
306302

‎extension/src/extension/services/auth/authFlowHandler.ts‎

Lines changed: 1 addition & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,11 @@
1-
import * as vscode from 'vscode';
2-
31
import type { ExtensionToWebviewMessage } from '@shared/messageContracts';
42
import { ExtensionMsg } from '@shared/messageContracts';
53

64
import { ArtemisApiService } from '@extension/api';
75
import type { UserInfo } from '@extension/controller/appStateManager';
86
import { LogCategory, logger } from '@extension/services/loggingService';
9-
import { getTheiaEnvironment } from '@extension/theia';
107
import { ApiError } from '@extension/types';
11-
import { CONFIG, resolveServerUrl, VSCODE_CONFIG } from '@extension/utils';
8+
import { resolveServerUrl } from '@extension/utils';
129

1310
import { AuthManager } from './authManager';
1411

@@ -21,38 +18,9 @@ export class AuthFlowHandler {
2118
private readonly _callbacks: {
2219
onAuthenticated: (userInfo: UserInfo) => Promise<void>;
2320
hideLoadingAndSendServerUrl: () => void;
24-
showLogin: () => void;
2521
},
2622
) {}
2723

28-
public async checkServerUrlChange(): Promise<void> {
29-
// In Theia, the server URL is environment-managed — skip change detection
30-
if (getTheiaEnvironment().isTheia) { return; }
31-
32-
try {
33-
const hasAuth = await this._authManager.hasAuthToken();
34-
if (hasAuth) {
35-
const isServerUrlChanged = await this._authManager.isServerUrlChanged(resolveServerUrl());
36-
if (isServerUrlChanged) {
37-
const config = vscode.workspace.getConfiguration(VSCODE_CONFIG.ARTEMIS_SECTION);
38-
const currentServerUrl = config.get<string>(VSCODE_CONFIG.SERVER_URL_KEY, CONFIG.ARTEMIS_SERVER_URL_DEFAULT);
39-
40-
vscode.window.showWarningMessage(
41-
`The Artemis server URL has changed to ${currentServerUrl}. Your stored credentials may no longer be valid.`,
42-
'Clear Credentials',
43-
'Keep Credentials'
44-
).then(selection => {
45-
if (selection === 'Clear Credentials') {
46-
this._callbacks.showLogin();
47-
}
48-
});
49-
}
50-
}
51-
} catch (error) {
52-
logger.error('Error checking server URL change', LogCategory.AUTH, error);
53-
}
54-
}
55-
5624
public async checkExistingAuthentication(): Promise<void> {
5725
try {
5826
let hasAuth = false;

‎extension/src/extension/services/auth/authManager.ts‎

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -31,26 +31,6 @@ export class AuthManager {
3131
return !!stored;
3232
}
3333

34-
/**
35-
* Returns the server URL that was active at the time of last successful login.
36-
* Used exclusively for URL-change detection: if the user changes their
37-
* `artemis.serverUrl` setting after login, stored credentials may be stale.
38-
* The live server URL is always resolved via `resolveServerUrl()`.
39-
*/
40-
public async getStoredLoginServerUrl(): Promise<string | undefined> {
41-
return await this.context.secrets.get(CONFIG.SECRET_KEYS.ARTEMIS_SERVER_URL);
42-
}
43-
44-
/**
45-
* Checks whether the user changed the server URL since their last login.
46-
* @param currentUrl The currently resolved server URL from settings/env.
47-
*/
48-
public async isServerUrlChanged(currentUrl: string): Promise<boolean> {
49-
const storedUrl = await this.getStoredLoginServerUrl();
50-
if (!storedUrl) { return false; }
51-
return storedUrl !== currentUrl;
52-
}
53-
5434
/**
5535
* Returns the raw JWT string (without any "jwt=" cookie prefix), suitable
5636
* for use in a `Cookie: jwt=<value>` or `Authorization: Bearer <value>` header.
@@ -98,19 +78,17 @@ export class AuthManager {
9878
return { 'Cookie': token };
9979
}
10080

101-
public async storeArtemisCredentials(token: string, serverUrl: string, persist: boolean): Promise<void> {
81+
public async storeArtemisCredentials(token: string, persist: boolean): Promise<void> {
10282
this.memoryToken = token;
10383
if (persist) {
10484
await this.context.secrets.store(CONFIG.SECRET_KEYS.ARTEMIS_TOKEN, token);
105-
await this.context.secrets.store(CONFIG.SECRET_KEYS.ARTEMIS_SERVER_URL, serverUrl);
10685
}
10786
}
10887

10988
public async clear(): Promise<void> {
11089
this.memoryToken = undefined;
11190
try {
11291
await this.context.secrets.delete(CONFIG.SECRET_KEYS.ARTEMIS_TOKEN);
113-
await this.context.secrets.delete(CONFIG.SECRET_KEYS.ARTEMIS_SERVER_URL);
11492
} catch (err) {
11593
logger.error('Failed to clear auth credentials from secrets:', LogCategory.AUTH, err);
11694
}

‎extension/src/extension/theia/theiaAuthProvider.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ export async function authenticateFromEnvironment(
3131
// Store raw JWT in memory only — never persist ENV tokens to SecretStorage
3232
await authManager.storeArtemisCredentials(
3333
theiaEnv.artemisToken,
34-
theiaEnv.artemisUrl,
3534
false,
3635
);
3736

‎extension/src/extension/utils/constants.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ export const CONFIG = {
44
AUTH_COOKIE_NAME: 'jwt',
55
SECRET_KEYS: {
66
ARTEMIS_TOKEN: 'artemis-auth-token',
7-
ARTEMIS_SERVER_URL: 'artemis-server-url',
87
},
98
WEBVIEW: {
109
VIEW_TYPE: 'artemis.loginView',

‎extension/test/unit/api/artemisApi.test.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -611,8 +611,6 @@ suite('Artemis API Service Test Suite', () => {
611611
await apiService.markMessageHelpful(sessionId, messageId, true);
612612
});
613613

614-
// isServerUrlChanged moved to AuthManager — tested in authManager.test.ts
615-
616614
test('should fallback to creating VCS token when none exists', async () => {
617615
const participationId = 9;
618616
let attempt = 0;

‎extension/test/unit/services/artemisWebsocketService.test.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ suite('Artemis WebSocket Service Test Suite', () => {
147147
wsService = new TestableArtemisWebsocketService(authManager);
148148

149149
// Mock auth
150-
await authManager.storeArtemisCredentials('jwt=token', 'https://artemis.example.com', true);
150+
await authManager.storeArtemisCredentials('jwt=token', true);
151151

152152
// Track state changes via EventEmitter (no immediate replay on subscribe)
153153
const states: boolean[] = [];
@@ -176,7 +176,7 @@ suite('Artemis WebSocket Service Test Suite', () => {
176176

177177
test('should subscribe to topics and receive messages', async () => {
178178
wsService = new TestableArtemisWebsocketService(authManager);
179-
await authManager.storeArtemisCredentials('jwt=token', 'https://artemis.example.com', true);
179+
await authManager.storeArtemisCredentials('jwt=token', true);
180180
const p = wsService.connect();
181181
await flushMicrotasks();
182182
wsService.mockClient!.simulateConnect();
@@ -218,7 +218,7 @@ suite('Artemis WebSocket Service Test Suite', () => {
218218

219219
test('should handle disconnection', async () => {
220220
wsService = new TestableArtemisWebsocketService(authManager);
221-
await authManager.storeArtemisCredentials('jwt=token', 'https://artemis.example.com', true);
221+
await authManager.storeArtemisCredentials('jwt=token', true);
222222
const p = wsService.connect();
223223
await flushMicrotasks();
224224
wsService.mockClient!.simulateConnect();
@@ -234,7 +234,7 @@ suite('Artemis WebSocket Service Test Suite', () => {
234234

235235
test('should handle STOMP errors', async () => {
236236
wsService = new TestableArtemisWebsocketService(authManager);
237-
await authManager.storeArtemisCredentials('jwt=token', 'https://artemis.example.com', true);
237+
await authManager.storeArtemisCredentials('jwt=token', true);
238238
const p = wsService.connect();
239239
await flushMicrotasks();
240240
wsService.mockClient!.simulateConnect();
@@ -263,7 +263,7 @@ suite('Artemis WebSocket Service Test Suite', () => {
263263

264264
test('should connect when disconnected', async () => {
265265
wsService = new TestableArtemisWebsocketService(authManager);
266-
await authManager.storeArtemisCredentials('jwt=token', 'https://artemis.example.com', true);
266+
await authManager.storeArtemisCredentials('jwt=token', true);
267267

268268
// Not connected yet
269269
assert.strictEqual(wsService.isConnected(), false);
@@ -285,7 +285,7 @@ suite('Artemis WebSocket Service Test Suite', () => {
285285

286286
test('should subscribe to Iris session and receive messages', async () => {
287287
wsService = new TestableArtemisWebsocketService(authManager);
288-
await authManager.storeArtemisCredentials('jwt=token', 'https://artemis.example.com', true);
288+
await authManager.storeArtemisCredentials('jwt=token', true);
289289
const p = wsService.connect();
290290
await flushMicrotasks();
291291
wsService.mockClient!.simulateConnect();
@@ -332,7 +332,7 @@ suite('Artemis WebSocket Service Test Suite', () => {
332332

333333
test('stale unsubscribe should not remove active subscription', async () => {
334334
wsService = new TestableArtemisWebsocketService(authManager);
335-
await authManager.storeArtemisCredentials('jwt=token', 'https://artemis.example.com', true);
335+
await authManager.storeArtemisCredentials('jwt=token', true);
336336
const p = wsService.connect();
337337
await flushMicrotasks();
338338
wsService.mockClient!.simulateConnect();

0 commit comments

Comments
 (0)