Skip to content

Commit f270146

Browse files
sergeibbbsergeibbb
committed
Enhances authentication token handling with metadata
Introduces `TokenInfo` and `TokenWithInfo` types to encapsulate authentication access tokens along with their relevant metadata, such as provider ID, cloud status, scopes, and expiration. This refactoring replaces direct `accessToken` strings with the new `TokenWithInfo` object across various API calls within integration providers. Benefits include: - Provides richer context in `AuthenticationError` messages for improved debugging. - Increases type safety and clarity by explicitly passing token metadata. (#4880, #4896)
1 parent 1925be4 commit f270146

File tree

26 files changed

+891
-654
lines changed

26 files changed

+891
-654
lines changed

‎src/errors.ts‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { CancellationError as _CancellationError } from 'vscode';
44
import type { Response } from '@env/fetch.js';
55
import type { RequiredSubscriptionPlanIds, Subscription } from './plus/gk/models/subscription.js';
66
import { isSubscriptionPaidPlan } from './plus/gk/utils/subscription.utils.js';
7+
import type { TokenInfo } from './plus/integrations/authentication/models.js';
78

89
export class AccessDeniedError extends Error {
910
public readonly subscription: Subscription;
@@ -54,27 +55,38 @@ export class AuthenticationError extends Error {
5455
readonly original?: Error;
5556
readonly reason: AuthenticationErrorReason | undefined;
5657

57-
constructor(id: string, reason?: AuthenticationErrorReason, original?: Error);
58-
constructor(id: string, message?: string, original?: Error);
59-
constructor(id: string, messageOrReason: string | AuthenticationErrorReason | undefined, original?: Error) {
58+
constructor(info: TokenInfo, reason?: AuthenticationErrorReason, original?: Error);
59+
constructor(info: TokenInfo, message?: string, original?: Error);
60+
constructor(info: TokenInfo, messageOrReason: string | AuthenticationErrorReason | undefined, original?: Error) {
61+
const { providerId: id, type, cloud, scopes, expiresAt } = info;
62+
const tokenDetails = [
63+
cloud ? 'cloud' : 'self-managed',
64+
info.microHash && `@${info.microHash}`,
65+
type && ` ${type}`,
66+
expiresAt && `expires at ${expiresAt.toISOString()}`,
67+
scopes && `[${scopes.join(',')}]`,
68+
]
69+
.filter(v => v)
70+
.join(', ');
71+
const authInfo = `(token details: ${tokenDetails})`;
6072
let message;
6173
let reason: AuthenticationErrorReason | undefined;
6274
if (messageOrReason == null) {
63-
message = `Unable to get required authentication session for '${id}'`;
75+
message = `Unable to get required authentication session for '${id}' ${authInfo}`;
6476
} else if (typeof messageOrReason === 'string') {
65-
message = messageOrReason;
77+
message = `"${messageOrReason}" for '${id}' ${authInfo}`;
6678
reason = undefined;
6779
} else {
6880
reason = messageOrReason;
6981
switch (reason) {
7082
case AuthenticationErrorReason.UserDidNotConsent:
71-
message = `'${id}' authentication is required for this operation`;
83+
message = `'${id}' authentication is required for this operation ${authInfo}`;
7284
break;
7385
case AuthenticationErrorReason.Unauthorized:
74-
message = `Your '${id}' credentials are either invalid or expired`;
86+
message = `Your '${id}' credentials are either invalid or expired ${authInfo}`;
7587
break;
7688
case AuthenticationErrorReason.Forbidden:
77-
message = `Your '${id}' credentials do not have the required access`;
89+
message = `Your '${id}' credentials do not have the required access ${authInfo}`;
7890
break;
7991
}
8092
}

‎src/plus/gk/serverConnection.ts‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ import { memoize } from '../../system/decorators/memoize.js';
2828
import { Logger } from '../../system/logger.js';
2929
import type { LogScope } from '../../system/logger.scope.js';
3030
import { getLogScope } from '../../system/logger.scope.js';
31+
import type { TokenInfo } from '../integrations/authentication/models.js';
32+
import { toTokenInfo } from '../integrations/authentication/models.js';
3133
import type { UrlsProvider } from './urlsProvider.js';
3234

3335
interface FetchOptions {
@@ -185,7 +187,7 @@ export class ServerConnection implements Disposable {
185187
}
186188
return rsp;
187189
} catch (ex) {
188-
this.handleGkRequestError('gitkraken', ex, scope);
190+
this.handleGkRequestError(options?.token || undefined, ex, scope);
189191
throw ex;
190192
}
191193
}
@@ -266,6 +268,11 @@ export class ServerConnection implements Disposable {
266268
if (ex instanceof CancellationError) throw ex;
267269
if (ex.name === 'AbortError') throw new CancellationError(ex);
268270

271+
const gitkrakenTokenInfo: TokenInfo<'gitkraken'> = toTokenInfo('gitkraken', token, {
272+
cloud: true,
273+
scopes: undefined,
274+
});
275+
269276
switch (ex.status) {
270277
case 404: // Not found
271278
throw new RequestNotFoundError(ex);
@@ -274,7 +281,7 @@ export class ServerConnection implements Disposable {
274281
case 422: // Unprocessable Entity
275282
throw new RequestUnprocessableEntityError(ex);
276283
case 401: // Unauthorized
277-
throw new AuthenticationError('gitkraken', AuthenticationErrorReason.Unauthorized, ex);
284+
throw new AuthenticationError(gitkrakenTokenInfo, AuthenticationErrorReason.Unauthorized, ex);
278285
case 429: //Too Many Requests
279286
this.trackRequestException();
280287
throw this.buildRequestRateLimitError(token, ex);
@@ -283,7 +290,7 @@ export class ServerConnection implements Disposable {
283290
this.trackRequestException();
284291
throw this.buildRequestRateLimitError(token, ex);
285292
}
286-
throw new AuthenticationError('gitkraken', AuthenticationErrorReason.Forbidden, ex);
293+
throw new AuthenticationError(gitkrakenTokenInfo, AuthenticationErrorReason.Forbidden, ex);
287294
case 500: // Internal Server Error
288295
Logger.error(ex, scope);
289296
if (ex.response != null) {

‎src/plus/integrations/authentication/models.ts‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { AuthenticationSession } from 'vscode';
2+
import { md5 } from '@env/crypto.js';
23
import type { IntegrationIds, SupportedCloudIntegrationIds } from '../../../constants.integrations.js';
34
import {
45
GitCloudHostIntegrationId,
@@ -16,6 +17,61 @@ export interface ProviderAuthenticationSession extends AuthenticationSession {
1617
readonly protocol?: string;
1718
}
1819

20+
export interface TokenInfo<T extends IntegrationIds | 'gitkraken' = IntegrationIds | 'gitkraken'> {
21+
readonly providerId: T;
22+
/**
23+
* The first 3 characters of the md5 hash of the token.
24+
* It's a very obfuscated representation of the token that we can use in logs
25+
* to see whether the token survives refreshing or gets updated.
26+
*/
27+
readonly microHash: string | undefined;
28+
readonly cloud: boolean;
29+
readonly scopes: readonly string[] | undefined;
30+
readonly type?: CloudIntegrationAuthType;
31+
readonly expiresAt?: Date;
32+
}
33+
34+
export interface TokenWithInfo<T extends IntegrationIds = IntegrationIds> extends TokenInfo<T> {
35+
readonly accessToken: string;
36+
}
37+
38+
export function toTokenInfo<T extends IntegrationIds | 'gitkraken'>(
39+
providerId: T,
40+
accessToken: string | undefined,
41+
info: { cloud: boolean; scopes?: readonly string[]; expiresAt?: Date },
42+
): TokenInfo<T> {
43+
return {
44+
providerId: providerId,
45+
microHash: microhash(accessToken),
46+
cloud: info.cloud,
47+
scopes: info.scopes,
48+
expiresAt: info.expiresAt,
49+
};
50+
}
51+
52+
export function toTokenWithInfo<T extends IntegrationIds>(
53+
providerId: T,
54+
session: ProviderAuthenticationSession,
55+
altToken?: string,
56+
): TokenWithInfo<T> {
57+
const accessToken = altToken ?? session.accessToken;
58+
return {
59+
...toTokenInfo(providerId, accessToken, session),
60+
accessToken: accessToken,
61+
};
62+
}
63+
64+
function microhash(token: undefined): undefined;
65+
function microhash(token: string): string;
66+
function microhash(token: string | undefined): string | undefined;
67+
function microhash(token: string | undefined): string | undefined {
68+
return !token ? undefined : md5(token, 'hex').substring(0, 3);
69+
}
70+
71+
export type TokenOptInfo<T extends IntegrationIds = IntegrationIds> =
72+
| TokenWithInfo<T>
73+
| { providerId: T; accessToken?: undefined };
74+
1975
export interface ConfiguredIntegrationDescriptor {
2076
readonly cloud: boolean;
2177
readonly integrationId: IntegrationIds;

‎src/plus/integrations/models/gitHostIntegration.ts‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ export abstract class GitHostIntegration<
337337

338338
let userAccount: ProviderAccount | undefined;
339339
try {
340-
userAccount = await api.getCurrentUserForInstance(providerId, organization);
340+
userAccount = await api.getCurrentUserForInstance({ providerId: providerId }, organization);
341341
} catch (ex) {
342342
Logger.error(ex, 'getIssuesForRepos');
343343
return undefined;
@@ -380,7 +380,7 @@ export abstract class GitHostIntegration<
380380
await Promise.all(
381381
projectInputs.map(async projectInput => {
382382
const results = await api.getIssuesForAzureProject(
383-
providerId,
383+
{ providerId: providerId },
384384
projectInput.namespace,
385385
projectInput.project,
386386
{
@@ -415,7 +415,7 @@ export abstract class GitHostIntegration<
415415
if (options?.filters != null) {
416416
let userAccount: ProviderAccount | undefined;
417417
try {
418-
userAccount = await api.getCurrentUser(providerId);
418+
userAccount = await api.getCurrentUser({ providerId: providerId });
419419
} catch (ex) {
420420
Logger.error(ex, 'getIssuesForRepos');
421421
return undefined;
@@ -453,7 +453,7 @@ export abstract class GitHostIntegration<
453453
const data: ProviderIssue[] = [];
454454
await Promise.all(
455455
repoInputs.map(async repoInput => {
456-
const results = await api.getIssuesForRepo(providerId, repoInput.repo, {
456+
const results = await api.getIssuesForRepo({ providerId: providerId }, repoInput.repo, {
457457
...getIssuesOptions,
458458
cursor: repoInput.cursor,
459459
baseUrl: options?.customUrl,
@@ -480,7 +480,7 @@ export abstract class GitHostIntegration<
480480
}
481481

482482
try {
483-
return await api.getIssuesForRepos(providerId, reposOrRepoIds, {
483+
return await api.getIssuesForRepos({ providerId: providerId }, reposOrRepoIds, {
484484
...getIssuesOptions,
485485
cursor: options?.cursor,
486486
baseUrl: options?.customUrl,
@@ -540,14 +540,14 @@ export abstract class GitHostIntegration<
540540

541541
const organization: string = first(organizations.values())!;
542542
try {
543-
userAccount = await api.getCurrentUserForInstance(providerId, organization);
543+
userAccount = await api.getCurrentUserForInstance({ providerId: providerId }, organization);
544544
} catch (ex) {
545545
Logger.error(ex, 'getPullRequestsForRepos');
546546
return undefined;
547547
}
548548
} else {
549549
try {
550-
userAccount = await api.getCurrentUser(providerId);
550+
userAccount = await api.getCurrentUser({ providerId: providerId });
551551
} catch (ex) {
552552
Logger.error(ex, 'getPullRequestsForRepos');
553553
return undefined;
@@ -602,7 +602,7 @@ export abstract class GitHostIntegration<
602602
const data: ProviderPullRequest[] = [];
603603
await Promise.all(
604604
repoInputs.map(async repoInput => {
605-
const results = await api.getPullRequestsForRepo(providerId, repoInput.repo, {
605+
const results = await api.getPullRequestsForRepo({ providerId: providerId }, repoInput.repo, {
606606
...getPullRequestsOptions,
607607
cursor: repoInput.cursor,
608608
baseUrl: options?.customUrl,
@@ -629,7 +629,7 @@ export abstract class GitHostIntegration<
629629
}
630630

631631
try {
632-
return await api.getPullRequestsForRepos(providerId, reposOrRepoIds, {
632+
return await api.getPullRequestsForRepos({ providerId: providerId }, reposOrRepoIds, {
633633
...getPullRequestsOptions,
634634
cursor: options?.cursor,
635635
baseUrl: options?.customUrl,

‎src/plus/integrations/providers/azure/azure.ts‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import { Logger } from '../../../../system/logger.js';
2626
import type { LogScope } from '../../../../system/logger.scope.js';
2727
import { getLogScope } from '../../../../system/logger.scope.js';
2828
import { maybeStopWatch } from '../../../../system/stopwatch.js';
29+
import type { TokenInfo, TokenWithInfo } from '../../authentication/models.js';
2930
import type {
3031
AzureGitCommit,
3132
AzureProjectDescriptor,
@@ -82,7 +83,7 @@ export class AzureDevOpsApi implements Disposable {
8283
@debug<AzureDevOpsApi['getPullRequestForBranch']>({ args: { 0: p => p.name, 1: '<token>' } })
8384
public async getPullRequestForBranch(
8485
provider: Provider,
85-
token: string,
86+
token: TokenWithInfo,
8687
owner: string,
8788
repo: string,
8889
branch: string,
@@ -136,7 +137,7 @@ export class AzureDevOpsApi implements Disposable {
136137
@debug<AzureDevOpsApi['getPullRequestForCommit']>({ args: { 0: p => p.name, 1: '<token>' } })
137138
async getPullRequestForCommit(
138139
provider: Provider,
139-
token: string,
140+
token: TokenWithInfo,
140141
owner: string,
141142
repo: string,
142143
rev: string,
@@ -193,7 +194,7 @@ export class AzureDevOpsApi implements Disposable {
193194
@debug<AzureDevOpsApi['getIssueOrPullRequest']>({ args: { 0: p => p.name, 1: '<token>' } })
194195
public async getIssueOrPullRequest(
195196
provider: Provider,
196-
token: string,
197+
token: TokenWithInfo,
197198
owner: string,
198199
repo: string,
199200
id: string,
@@ -295,7 +296,7 @@ export class AzureDevOpsApi implements Disposable {
295296
@debug<AzureDevOpsApi['getIssue']>({ args: { 0: p => p.name, 1: '<token>' } })
296297
public async getIssue(
297298
provider: Provider,
298-
token: string,
299+
token: TokenWithInfo,
299300
project: AzureProjectDescriptor,
300301
id: string,
301302
options: {
@@ -344,7 +345,7 @@ export class AzureDevOpsApi implements Disposable {
344345
@debug<AzureDevOpsApi['getAccountForCommit']>({ args: { 0: p => p.name, 1: '<token>' } })
345346
async getAccountForCommit(
346347
provider: Provider,
347-
token: string,
348+
token: TokenWithInfo,
348349
owner: string,
349350
repo: string,
350351
rev: string,
@@ -394,7 +395,7 @@ export class AzureDevOpsApi implements Disposable {
394395
@debug<AzureDevOpsApi['getCurrentUserOnServer']>({ args: { 0: p => p.name, 1: '<token>' } })
395396
async getCurrentUserOnServer(
396397
provider: Provider,
397-
token: string,
398+
token: TokenWithInfo,
398399
baseUrl: string,
399400
): Promise<{ id: string; name?: string; email?: string; username?: string; avatarUrl?: string } | undefined> {
400401
const scope = getLogScope();
@@ -450,7 +451,7 @@ export class AzureDevOpsApi implements Disposable {
450451
issueType: string,
451452
state: string,
452453
provider: Provider,
453-
token: string,
454+
token: TokenWithInfo,
454455
owner: string,
455456
projectName: string,
456457
options: {
@@ -470,7 +471,7 @@ export class AzureDevOpsApi implements Disposable {
470471
private async retrieveWorkItemTypeStates(
471472
workItemType: string,
472473
provider: Provider,
473-
token: string,
474+
token: TokenWithInfo,
474475
owner: string,
475476
projectName: string,
476477
options: {
@@ -500,13 +501,14 @@ export class AzureDevOpsApi implements Disposable {
500501

501502
private async request<T>(
502503
provider: Provider,
503-
token: string,
504+
token: TokenWithInfo,
504505
baseUrl: string | undefined,
505506
route: string,
506507
options: { method: RequestInit['method'] } & Record<string, unknown>,
507508
scope: LogScope | undefined,
508509
cancellation?: CancellationToken | undefined,
509510
): Promise<T | undefined> {
511+
const { accessToken, ...tokenInfo } = token;
510512
const url = baseUrl ? `${baseUrl}/${route}` : route;
511513

512514
let rsp: Response;
@@ -526,7 +528,7 @@ export class AzureDevOpsApi implements Disposable {
526528
rsp = await wrapForForcedInsecureSSL(provider.getIgnoreSSLErrors(), () =>
527529
fetch(url, {
528530
headers: {
529-
Authorization: `Basic ${base64(`PAT:${token}`)}`,
531+
Authorization: `Basic ${base64(`PAT:${accessToken}`)}`,
530532
'Content-Type': 'application/json',
531533
},
532534
agent: agent,
@@ -546,7 +548,7 @@ export class AzureDevOpsApi implements Disposable {
546548
}
547549
} catch (ex) {
548550
if (ex instanceof ProviderFetchError || ex.name === 'AbortError') {
549-
this.handleRequestError(provider, token, ex, scope);
551+
this.handleRequestError(provider, tokenInfo, ex, scope);
550552
} else if (Logger.isDebugging) {
551553
void window.showErrorMessage(`AzureDevOps request failed: ${ex.message}`);
552554
}
@@ -557,7 +559,7 @@ export class AzureDevOpsApi implements Disposable {
557559

558560
private handleRequestError(
559561
provider: Provider | undefined,
560-
_token: string,
562+
tokenInfo: TokenInfo,
561563
ex: ProviderFetchError | (Error & { name: 'AbortError' }),
562564
scope: LogScope | undefined,
563565
): void {
@@ -569,7 +571,7 @@ export class AzureDevOpsApi implements Disposable {
569571
case 422: // Unprocessable Entity
570572
throw new RequestNotFoundError(ex);
571573
case 401: // Unauthorized
572-
throw new AuthenticationError('azureDevOps', AuthenticationErrorReason.Unauthorized, ex);
574+
throw new AuthenticationError(tokenInfo, AuthenticationErrorReason.Unauthorized, ex);
573575
case 403: // Forbidden
574576
// TODO: Learn the Azure API docs and put it in order:
575577
// if (ex.message.includes('rate limit')) {
@@ -585,7 +587,7 @@ export class AzureDevOpsApi implements Disposable {
585587

586588
// throw new RequestRateLimitError(ex, token, resetAt);
587589
// }
588-
throw new AuthenticationError('azure', AuthenticationErrorReason.Forbidden, ex);
590+
throw new AuthenticationError(tokenInfo, AuthenticationErrorReason.Forbidden, ex);
589591
case 500: // Internal Server Error
590592
Logger.error(ex, scope);
591593
if (ex.response != null) {

0 commit comments

Comments
 (0)