Skip to content

Commit 798f191

Browse files
TylerLeonhardtTylerLeonhardt
authored
Regenerate Client Id when invalid_client is returned (microsoft#252714)
Fixes microsoft#250960
1 parent faddb17 commit 798f191

File tree

4 files changed

+68
-24
lines changed

4 files changed

+68
-24
lines changed

‎src/vs/workbench/api/browser/mainThreadAuthentication.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,20 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu
233233
await this.dynamicAuthProviderStorageService.setSessionsForDynamicAuthProvider(authProviderId, clientId, sessions);
234234
}
235235

236+
async $sendDidChangeDynamicProviderInfo({ providerId, clientId, authorizationServer, label }: Partial<{ providerId: string; clientId: string; authorizationServer: UriComponents; label: string }>): Promise<void> {
237+
this.logService.info(`Client ID for authentication provider ${providerId} changed to ${clientId}`);
238+
const existing = this.dynamicAuthProviderStorageService.getInteractedProviders().find(p => p.providerId === providerId);
239+
if (!existing) {
240+
throw new Error(`Dynamic authentication provider ${providerId} not found. Has it been registered?`);
241+
}
242+
this.dynamicAuthProviderStorageService.storeClientId(
243+
providerId || existing.providerId,
244+
authorizationServer ? URI.revive(authorizationServer).toString(true) : existing.authorizationServer,
245+
clientId || existing.clientId,
246+
label || existing.label
247+
);
248+
}
249+
236250
private async loginPrompt(provider: IAuthenticationProvider, extensionName: string, recreatingSession: boolean, options?: AuthenticationInteractiveOptions): Promise<boolean> {
237251
let message: string;
238252

‎src/vs/workbench/api/common/extHost.protocol.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,7 @@ export interface MainThreadAuthenticationShape extends IDisposable {
195195
$showDeviceCodeModal(userCode: string, verificationUri: string): Promise<boolean>;
196196
$registerDynamicAuthenticationProvider(id: string, label: string, authorizationServer: UriComponents, clientId: string): Promise<void>;
197197
$setSessionsForDynamicAuthProvider(authProviderId: string, clientId: string, sessions: (IAuthorizationTokenResponse & { created_at: number })[]): Promise<void>;
198+
$sendDidChangeDynamicProviderInfo({ providerId, clientId, authorizationServer, label }: { providerId: string; clientId?: string; authorizationServer?: UriComponents; label?: string }): Promise<void>;
198199
}
199200

200201
export interface MainThreadSecretStateShape extends IDisposable {

‎src/vs/workbench/api/common/extHostAuthentication.ts

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { INTERNAL_AUTH_PROVIDER_PREFIX } from '../../services/authentication/com
1313
import { createDecorator } from '../../../platform/instantiation/common/instantiation.js';
1414
import { IExtHostRpcService } from './extHostRpcService.js';
1515
import { URI, UriComponents } from '../../../base/common/uri.js';
16-
import { fetchDynamicRegistration, getClaimsFromJWT, IAuthorizationJWTClaims, IAuthorizationProtectedResourceMetadata, IAuthorizationServerMetadata, IAuthorizationTokenResponse, isAuthorizationTokenResponse } from '../../../base/common/oauth.js';
16+
import { AuthorizationErrorType, fetchDynamicRegistration, getClaimsFromJWT, IAuthorizationJWTClaims, IAuthorizationProtectedResourceMetadata, IAuthorizationServerMetadata, IAuthorizationTokenResponse, isAuthorizationErrorResponse, isAuthorizationTokenResponse } from '../../../base/common/oauth.js';
1717
import { IExtHostWindow } from './extHostWindow.js';
1818
import { IExtHostInitDataService } from './extHostInitDataService.js';
1919
import { ILogger, ILoggerService, ILogService } from '../../../platform/log/common/log.js';
@@ -213,13 +213,19 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape {
213213

214214
// Use the sequencer to ensure dynamic provider registration is serialized
215215
await this._providerOperations.queue(provider.id, async () => {
216-
const disposable = provider.onDidChangeSessions(e => this._proxy.$sendDidChangeSessions(provider.id, e));
217216
this._authenticationProviders.set(
218217
provider.id,
219218
{
220219
label: provider.label,
221220
provider,
222-
disposable: Disposable.from(provider, disposable),
221+
disposable: Disposable.from(
222+
provider,
223+
provider.onDidChangeSessions(e => this._proxy.$sendDidChangeSessions(provider.id, e)),
224+
provider.onDidChangeClientId(() => this._proxy.$sendDidChangeDynamicProviderInfo({
225+
providerId: provider.id,
226+
clientId: provider.clientId
227+
}))
228+
),
223229
options: { supportsMultipleAccounts: false }
224230
}
225231
);
@@ -256,6 +262,9 @@ export class DynamicAuthProvider implements vscode.AuthenticationProvider {
256262
private _onDidChangeSessions = new Emitter<vscode.AuthenticationProviderAuthenticationSessionsChangeEvent>();
257263
readonly onDidChangeSessions = this._onDidChangeSessions.event;
258264

265+
private readonly _onDidChangeClientId = new Emitter<void>();
266+
readonly onDidChangeClientId = this._onDidChangeClientId.event;
267+
259268
private readonly _tokenStore: TokenStore;
260269

261270
protected readonly _createFlows: Array<{
@@ -276,7 +285,7 @@ export class DynamicAuthProvider implements vscode.AuthenticationProvider {
276285
readonly authorizationServer: URI,
277286
protected readonly _serverMetadata: IAuthorizationServerMetadata,
278287
protected readonly _resourceMetadata: IAuthorizationProtectedResourceMetadata | undefined,
279-
readonly clientId: string,
288+
protected _clientId: string,
280289
onDidDynamicAuthProviderTokensChange: Emitter<{ authProviderId: string; clientId: string; tokens: IAuthorizationToken[] }>,
281290
initialTokens: IAuthorizationToken[],
282291
) {
@@ -292,7 +301,7 @@ export class DynamicAuthProvider implements vscode.AuthenticationProvider {
292301
this._disposable = new DisposableStore();
293302
this._disposable.add(this._onDidChangeSessions);
294303
const scopedEvent = Event.chain(onDidDynamicAuthProviderTokensChange.event, $ => $
295-
.filter(e => e.authProviderId === this.id && e.clientId === clientId)
304+
.filter(e => e.authProviderId === this.id && e.clientId === _clientId)
296305
.map(e => e.tokens)
297306
);
298307
this._tokenStore = this._disposable.add(new TokenStore(
@@ -311,6 +320,10 @@ export class DynamicAuthProvider implements vscode.AuthenticationProvider {
311320
}];
312321
}
313322

323+
get clientId(): string {
324+
return this._clientId;
325+
}
326+
314327
async getSessions(scopes: readonly string[] | undefined, _options: vscode.AuthenticationProviderSessionOptions): Promise<vscode.AuthenticationSession[]> {
315328
this._logger.info(`Getting sessions for scopes: ${scopes?.join(' ') ?? 'all'}`);
316329
if (!scopes) {
@@ -455,7 +468,7 @@ export class DynamicAuthProvider implements vscode.AuthenticationProvider {
455468

456469
// Prepare the authorization request URL
457470
const authorizationUrl = new URL(this._serverMetadata.authorization_endpoint!);
458-
authorizationUrl.searchParams.append('client_id', this.clientId);
471+
authorizationUrl.searchParams.append('client_id', this._clientId);
459472
authorizationUrl.searchParams.append('response_type', 'code');
460473
authorizationUrl.searchParams.append('state', state.toString());
461474
authorizationUrl.searchParams.append('code_challenge', codeChallenge);
@@ -546,7 +559,7 @@ export class DynamicAuthProvider implements vscode.AuthenticationProvider {
546559
}
547560

548561
const tokenRequest = new URLSearchParams();
549-
tokenRequest.append('client_id', this.clientId);
562+
tokenRequest.append('client_id', this._clientId);
550563
tokenRequest.append('grant_type', 'authorization_code');
551564
tokenRequest.append('code', code);
552565
tokenRequest.append('redirect_uri', redirectUri);
@@ -569,6 +582,10 @@ export class DynamicAuthProvider implements vscode.AuthenticationProvider {
569582
const result = await response.json();
570583
if (isAuthorizationTokenResponse(result)) {
571584
return result;
585+
} else if (isAuthorizationErrorResponse(result) && result.error === AuthorizationErrorType.InvalidClient) {
586+
this._logger.warn(`Client ID (${this._clientId}) was invalid, generated a new one.`);
587+
await this._generateNewClientId();
588+
throw new Error(`Client ID was invalid, generated a new one. Please try again.`);
572589
}
573590
throw new Error(`Invalid authorization token response: ${JSON.stringify(result)}`);
574591
}
@@ -579,7 +596,7 @@ export class DynamicAuthProvider implements vscode.AuthenticationProvider {
579596
}
580597

581598
const tokenRequest = new URLSearchParams();
582-
tokenRequest.append('client_id', this.clientId);
599+
tokenRequest.append('client_id', this._clientId);
583600
tokenRequest.append('grant_type', 'refresh_token');
584601
tokenRequest.append('refresh_token', refreshToken);
585602

@@ -592,20 +609,30 @@ export class DynamicAuthProvider implements vscode.AuthenticationProvider {
592609
body: tokenRequest.toString()
593610
});
594611

595-
if (!response.ok) {
596-
const text = await response.text();
597-
throw new Error(`Token exchange failed: ${response.status} ${response.statusText} - ${text}`);
598-
}
599-
600612
const result = await response.json();
601613
if (isAuthorizationTokenResponse(result)) {
602614
return {
603615
...result,
604616
created_at: Date.now(),
605617
};
618+
} else if (isAuthorizationErrorResponse(result) && result.error === AuthorizationErrorType.InvalidClient) {
619+
this._logger.warn(`Client ID (${this._clientId}) was invalid, generated a new one.`);
620+
await this._generateNewClientId();
621+
throw new Error(`Client ID was invalid, generated a new one. Please try again.`);
606622
}
607623
throw new Error(`Invalid authorization token response: ${JSON.stringify(result)}`);
608624
}
625+
626+
protected async _generateNewClientId(): Promise<void> {
627+
try {
628+
const registration = await fetchDynamicRegistration(this._serverMetadata, this._initData.environment.appName, this._resourceMetadata?.scopes_supported);
629+
this._clientId = registration.client_id;
630+
this._onDidChangeClientId.fire();
631+
} catch (err) {
632+
this._logger.error(`Failed to fetch new client ID: ${err}`);
633+
throw new Error(`Failed to fetch new client ID: ${err}`);
634+
}
635+
}
609636
}
610637

611638
type IAuthorizationToken = IAuthorizationTokenResponse & {

‎src/vs/workbench/api/node/extHostAuthentication.ts

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { IExtHostWindow } from '../common/extHostWindow.js';
1313
import { IExtHostUrlsService } from '../common/extHostUrls.js';
1414
import { ILoggerService, ILogService } from '../../../platform/log/common/log.js';
1515
import { MainThreadAuthenticationShape } from '../common/extHost.protocol.js';
16-
import { IAuthorizationServerMetadata, IAuthorizationProtectedResourceMetadata, IAuthorizationTokenResponse, IAuthorizationDeviceResponse, isAuthorizationDeviceResponse, isAuthorizationTokenResponse, IAuthorizationDeviceTokenErrorResponse } from '../../../base/common/oauth.js';
16+
import { IAuthorizationServerMetadata, IAuthorizationProtectedResourceMetadata, IAuthorizationTokenResponse, IAuthorizationDeviceResponse, isAuthorizationDeviceResponse, isAuthorizationTokenResponse, IAuthorizationDeviceTokenErrorResponse, AuthorizationErrorType, AuthorizationDeviceCodeErrorType } from '../../../base/common/oauth.js';
1717
import { Emitter } from '../../../base/common/event.js';
1818
import { raceCancellationError } from '../../../base/common/async.js';
1919
import { IExtHostProgress } from '../common/extHostProgress.js';
@@ -88,7 +88,7 @@ export class NodeDynamicAuthProvider extends DynamicAuthProvider {
8888

8989
// Prepare the authorization request URL
9090
const authorizationUrl = new URL(this._serverMetadata.authorization_endpoint!);
91-
authorizationUrl.searchParams.append('client_id', this.clientId);
91+
authorizationUrl.searchParams.append('client_id', this._clientId);
9292
authorizationUrl.searchParams.append('response_type', 'code');
9393
authorizationUrl.searchParams.append('code_challenge', codeChallenge);
9494
authorizationUrl.searchParams.append('code_challenge_method', 'S256');
@@ -173,7 +173,7 @@ export class NodeDynamicAuthProvider extends DynamicAuthProvider {
173173

174174
// Step 1: Request device and user codes
175175
const deviceCodeRequest = new URLSearchParams();
176-
deviceCodeRequest.append('client_id', this.clientId);
176+
deviceCodeRequest.append('client_id', this._clientId);
177177
if (scopeString) {
178178
deviceCodeRequest.append('scope', scopeString);
179179
}
@@ -243,7 +243,7 @@ export class NodeDynamicAuthProvider extends DynamicAuthProvider {
243243
const tokenRequest = new URLSearchParams();
244244
tokenRequest.append('grant_type', 'urn:ietf:params:oauth:grant-type:device_code');
245245
tokenRequest.append('device_code', deviceCodeData.device_code);
246-
tokenRequest.append('client_id', this.clientId);
246+
tokenRequest.append('client_id', this._clientId);
247247

248248
try {
249249
const tokenResponse = await fetch(this._serverMetadata.token_endpoint, {
@@ -273,17 +273,21 @@ export class NodeDynamicAuthProvider extends DynamicAuthProvider {
273273
}
274274

275275
// Handle known error cases
276-
if (errorData.error === 'authorization_pending') {
276+
if (errorData.error === AuthorizationDeviceCodeErrorType.AuthorizationPending) {
277277
// User hasn't completed authorization yet, continue polling
278278
continue;
279-
} else if (errorData.error === 'slow_down') {
279+
} else if (errorData.error === AuthorizationDeviceCodeErrorType.SlowDown) {
280280
// Server is asking us to slow down
281281
await new Promise(resolve => setTimeout(resolve, pollInterval));
282282
continue;
283-
} else if (errorData.error === 'expired_token') {
283+
} else if (errorData.error === AuthorizationDeviceCodeErrorType.ExpiredToken) {
284284
throw new Error('Device code expired. Please try again.');
285-
} else if (errorData.error === 'access_denied') {
285+
} else if (errorData.error === AuthorizationDeviceCodeErrorType.AccessDenied) {
286286
throw new CancellationError();
287+
} else if (errorData.error === AuthorizationErrorType.InvalidClient) {
288+
this._logger.warn(`Client ID (${this._clientId}) was invalid, generated a new one.`);
289+
await this._generateNewClientId();
290+
throw new Error(`Client ID was invalid, generated a new one. Please try again.`);
287291
} else {
288292
throw new Error(`Token request failed: ${errorData.error_description || errorData.error || 'Unknown error'}`);
289293
}
@@ -292,9 +296,7 @@ export class NodeDynamicAuthProvider extends DynamicAuthProvider {
292296
if (isCancellationError(error)) {
293297
throw error;
294298
}
295-
this._logger.error(`Error polling for token: ${error}`);
296-
// Continue polling on network errors
297-
continue;
299+
throw new Error(`Error polling for token: ${error}`);
298300
}
299301
}
300302

0 commit comments

Comments
 (0)