Fix GitHub account ids being numbers (#228045)

For a long time the account id wasn't handled correctly. It should be a string, but the API returns a number. This ensures it's a string and does some migration logic.
This commit is contained in:
Tyler James Leonhardt
2024-09-09 19:39:37 -07:00
committed by GitHub
parent d8af24e15d
commit c4d1cc2e67
2 changed files with 23 additions and 6 deletions

View File

@@ -18,7 +18,9 @@ interface SessionData {
account?: {
label?: string;
displayName?: string;
id: string;
// Unfortunately, for some time the id was a number, so we need to support both.
// This can be removed once we are confident that all users have migrated to the new id.
id: string | number;
};
scopes: string[];
accessToken: string;
@@ -239,9 +241,14 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid
return [];
}
// Unfortunately, we were using a number secretly for the account id for some time... this is due to a bad `any`.
// AuthenticationSession's account id is a string, so we need to detect when there is a number accountId and re-store
// the sessions to migrate away from the bad number usage.
// TODO@TylerLeonhardt: Remove this after we are confident that all users have migrated to the new id.
let seenNumberAccountId: boolean = false;
// TODO: eventually remove this Set because we should only have one session per set of scopes.
const scopesSeen = new Set<string>();
const sessionPromises = sessionData.map(async (session: SessionData) => {
const sessionPromises = sessionData.map(async (session: SessionData): Promise<vscode.AuthenticationSession | undefined> => {
// For GitHub scope list, order doesn't matter so we immediately sort the scopes
const scopesStr = [...session.scopes].sort().join(' ');
if (!this._supportsMultipleAccounts && scopesSeen.has(scopesStr)) {
@@ -262,13 +269,23 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid
this._logger.trace(`Read the following session from the keychain with the following scopes: ${scopesStr}`);
scopesSeen.add(scopesStr);
let accountId: string;
if (session.account?.id) {
if (typeof session.account.id === 'number') {
seenNumberAccountId = true;
}
accountId = `${session.account.id}`;
} else {
accountId = userInfo?.id ?? '<unknown>';
}
return {
id: session.id,
account: {
label: session.account
? session.account.label ?? session.account.displayName ?? '<unknown>'
: userInfo?.accountName ?? '<unknown>',
id: session.account?.id ?? userInfo?.id ?? '<unknown>'
id: accountId
},
// we set this to session.scopes to maintain the original order of the scopes requested
// by the extension that called getSession()
@@ -283,7 +300,7 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid
.filter(<T>(p?: T): p is T => Boolean(p));
this._logger.info(`Got ${verifiedSessions.length} verified sessions.`);
if (verifiedSessions.length !== sessionData.length) {
if (seenNumberAccountId || verifiedSessions.length !== sessionData.length) {
await this.storeSessions(verifiedSessions);
}

View File

@@ -228,9 +228,9 @@ export class GitHubServer implements IGitHubServer {
if (result.ok) {
try {
const json = await result.json();
const json = await result.json() as { id: number; login: string };
this._logger.info('Got account info!');
return { id: json.id, accountName: json.login };
return { id: `${json.id}`, accountName: json.login };
} catch (e) {
this._logger.error(`Unexpected error parsing response from GitHub: ${e.message ?? e}`);
throw e;