github-authentication - simplify avatar persistence

extensions/github-authentication/src/github.ts

@@ -14,16 +14,8 @@ import { crypto } from './node/crypto';
 import { TIMED_OUT_ERROR, USER_CANCELLATION_ERROR } from './common/errors';
 import { GitHubSocialSignInProvider, isSocialSignInProvider } from './flows';
 
-/**
- * The stored (JSON) form of a vscode.Uri pointing to the account's avatar.
- */
-interface StoredAccountIcon {
-	scheme: string;
-	authority?: string;
-	path?: string;
-	query?: string;
-	fragment?: string;
-}
+// `vscode` doesn't publicly export `UriComponents`, so derive the exact shape from `Uri.from`.
+type UriComponents = Parameters<typeof vscode.Uri.from>[0];
 
 interface SessionData {
 	id: string;
@@ -33,31 +25,12 @@ interface SessionData {
 		// 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;
-		// `undefined` means the avatar has not been looked up yet, `null` means a lookup
-		// completed and found no avatar, and a `StoredAccountIcon` is a resolved avatar.
-		icon?: StoredAccountIcon | null;
+		icon?: UriComponents;
 	};
 	scopes: string[];
 	accessToken: string;
 }
 
-/**
- * Whether a stored session's account icon still needs to be looked up.
- */
-export function needsAccountIconLookup(session: SessionData): boolean {
-	return !session.account || session.account.icon === undefined;
-}
-
-/**
- * Serializes an account icon for storage, using `null` to mark a completed lookup that found no avatar.
- */
-export function serializeAccountIcon(icon: vscode.Uri | undefined, hasNoAvatar: boolean): StoredAccountIcon | null | undefined {
-	if (icon) {
-		return { scheme: icon.scheme, authority: icon.authority, path: icon.path, query: icon.query, fragment: icon.fragment };
-	}
-	return hasNoAvatar ? null : undefined;
-}
-
 export enum AuthProviderType {
 	github = 'github',
 	githubEnterprise = 'github-enterprise'
@@ -165,7 +138,6 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid
 	private readonly _telemetryReporter: ExperimentationTelemetry;
 	private readonly _keychain: Keychain;
 	private readonly _accountsSeen = new Set<string>();
-	private readonly _sessionsWithoutAvatars = new WeakSet<vscode.AuthenticationSession>();
 	private readonly _disposable: vscode.Disposable | undefined;
 
 	private _sessionsPromise: Promise<vscode.AuthenticationSession[]>;
@@ -310,25 +282,21 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid
 		// 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;
-		// Sessions that were stored before the account icon was introduced are re-stored
-		// once an icon has been fetched so that we don't refetch it on every read.
-		let seenIconUpdate: boolean = false;
+		// Re-store newly verified accounts so future reads do not need another lookup.
+		let seenAccountUpdate: 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): Promise<vscode.AuthenticationSession | undefined> => {
 			// For GitHub scope list, order doesn't matter so we immediately sort the scopes
 			const scopesStr = [...session.scopes].sort().join(' ');
 			let userInfo: { id: string; accountName: string; avatarUrl: string | undefined } | undefined;
-			if (needsAccountIconLookup(session)) {
-				const needsAccount = !session.account;
+			if (!session.account) {
 				try {
 					userInfo = await this._githubServer.getUserInfo(session.accessToken);
-					seenIconUpdate = true;
-					if (needsAccount) {
-						this._logger.info(`Verified session with the following scopes: ${scopesStr}`);
-					}
+					seenAccountUpdate = true;
+					this._logger.info(`Verified session with the following scopes: ${scopesStr}`);
 				} catch (e) {
-					if (e.message === 'Unauthorized' && needsAccount) {
+					if (e.message === 'Unauthorized') {
 						return undefined;
 					}
 				}
@@ -346,13 +314,10 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid
 			} else {
 				accountId = userInfo?.id ?? '<unknown>';
 			}
-			let icon: vscode.Uri | undefined;
-			if (session.account?.icon?.scheme) {
-				icon = vscode.Uri.from(session.account.icon);
-			} else if (userInfo?.avatarUrl) {
-				icon = vscode.Uri.parse(userInfo.avatarUrl);
-			}
-			const resolvedSession: vscode.AuthenticationSession = {
+			const icon = session.account?.icon
+				? vscode.Uri.from(session.account.icon)
+				: userInfo?.avatarUrl ? vscode.Uri.parse(userInfo.avatarUrl) : undefined;
+			return {
 				id: session.id,
 				account: {
 					label: session.account
@@ -366,10 +331,6 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid
 				scopes: session.scopes,
 				accessToken: session.accessToken
 			};
-			if (!icon && (session.account?.icon === null || userInfo)) {
-				this._sessionsWithoutAvatars.add(resolvedSession);
-			}
-			return resolvedSession;
 		});
 
 		const verifiedSessions = (await Promise.allSettled(sessionPromises))
@@ -378,7 +339,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 (seenNumberAccountId || seenIconUpdate || verifiedSessions.length !== sessionData.length) {
+		if (seenNumberAccountId || seenAccountUpdate || verifiedSessions.length !== sessionData.length) {
 			await this.storeSessions(verifiedSessions);
 		}
 
@@ -388,17 +349,7 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid
 	private async storeSessions(sessions: vscode.AuthenticationSession[]): Promise<void> {
 		this._logger.info(`Storing ${sessions.length} sessions...`);
 		this._sessionsPromise = Promise.resolve(sessions);
-		const storedSessions: SessionData[] = sessions.map(session => ({
-			id: session.id,
-			account: {
-				label: session.account.label,
-				id: session.account.id,
-				icon: serializeAccountIcon(session.account.icon, this._sessionsWithoutAvatars.has(session)),
-			},
-			scopes: [...session.scopes],
-			accessToken: session.accessToken
-		}));
-		await this._keychain.setToken(JSON.stringify(storedSessions));
+		await this._keychain.setToken(JSON.stringify(sessions));
 		this._logger.info(`Stored ${sessions.length} sessions!`);
 	}
 
@@ -468,16 +419,12 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid
 
 	private async tokenToSession(token: string, scopes: string[]): Promise<vscode.AuthenticationSession> {
 		const userInfo = await this._githubServer.getUserInfo(token);
-		const session: vscode.AuthenticationSession = {
+		return {
 			id: crypto.getRandomValues(new Uint32Array(2)).reduce((prev, curr) => prev += curr.toString(16), ''),
 			accessToken: token,
 			account: { label: userInfo.accountName, id: userInfo.id, icon: userInfo.avatarUrl ? vscode.Uri.parse(userInfo.avatarUrl) : undefined },
 			scopes
 		};
-		if (!session.account.icon) {
-			this._sessionsWithoutAvatars.add(session);
-		}
-		return session;
 	}
 
 	public async removeSession(id: string) {