Embeds auth service into integration service

 - Limits/controls auth access to be through an integration (mainly)

Author: eamodio

src/commands/patches.ts

@@ -323,11 +323,7 @@ export class OpenCloudPatchCommand extends Command {
 				return;
 			}
 
-			const session = await this.container.integrationAuthentication.getSession(
-				integration.id,
-				integration.authProviderDescriptor,
-			);
-
+			const session = await integration.getSession();
 			if (session == null) {
 				void window.showErrorMessage(`Cannot open ${type}; provider not connected.`);
 				return;

src/constants.ts

@@ -878,7 +878,7 @@ export type SupportedAIModels =
 	| 'vscode';
 
 export type SecretKeys =
-	| `gitlens.integration.auth:${string}`
+	| `gitlens.integration.auth:${IntegrationId}|${string}`
 	| `gitlens.${AIProviders}.key`
 	| `gitlens.plus.auth:${Environment}`;
 

src/container.ts

@@ -634,21 +634,14 @@ export class Container {
 		return this._context.extension.id;
 	}
 
-	private _integrationAuthentication: IntegrationAuthenticationService | undefined;
-	get integrationAuthentication() {
-		if (this._integrationAuthentication == null) {
-			this._disposables.push(
-				(this._integrationAuthentication = new IntegrationAuthenticationService(this, this._connection)),
-			);
-		}
-
-		return this._integrationAuthentication;
-	}
-
 	private _integrations: IntegrationService | undefined;
 	get integrations(): IntegrationService {
 		if (this._integrations == null) {
-			this._disposables.push((this._integrations = new IntegrationService(this)));
+			const authenticationService = new IntegrationAuthenticationService(this, this._connection);
+			this._disposables.push(
+				authenticationService,
+				(this._integrations = new IntegrationService(this, authenticationService)),
+			);
 		}
 		return this._integrations;
 	}

src/plus/drafts/draftsService.ts

@@ -113,7 +113,7 @@ export class DraftService implements Disposable {
 				};
 
 				const repo = patchRequests[0].repository;
-				const providerAuth = await this.getProviderAuthFromRepository(repo);
+				const providerAuth = await this.getProviderAuthFromRepoOrIntegrationId(repo);
 				if (providerAuth == null) {
 					throw new Error('No provider integration found');
 				}
@@ -759,17 +759,24 @@ export class DraftService implements Disposable {
 		};
 	}
 
-	async getProviderAuthFromRepository(repository: Repository): Promise<ProviderAuth | undefined> {
-		const remoteProvider = await repository.getBestRemoteWithIntegration();
-		if (remoteProvider == null) return undefined;
+	async getProviderAuthFromRepoOrIntegrationId(
+		repoOrIntegrationId: Repository | IntegrationId,
+	): Promise<ProviderAuth | undefined> {
+		let integration;
+		if (isRepository(repoOrIntegrationId)) {
+			const remoteProvider = await repoOrIntegrationId.getBestRemoteWithIntegration();
+			if (remoteProvider == null) return undefined;
 
-		const integration = await remoteProvider.getIntegration();
+			integration = await remoteProvider.getIntegration();
+		} else {
+			const metadata = providersMetadata[repoOrIntegrationId];
+			if (metadata == null) return undefined;
+
+			integration = await this.container.integrations.get(repoOrIntegrationId, metadata.domain);
+		}
 		if (integration == null) return undefined;
 
-		const session = await this.container.integrationAuthentication.getSession(
-			integration.id,
-			integration.authProviderDescriptor,
-		);
+		const session = await integration.getSession();
 		if (session == null) return undefined;
 
 		return {
@@ -778,21 +785,6 @@ export class DraftService implements Disposable {
 		};
 	}
 
-	async getProviderAuthForIntegration(integrationId: IntegrationId): Promise<ProviderAuth | undefined> {
-		const metadata = providersMetadata[integrationId];
-		if (metadata == null) return undefined;
-		const session = await this.container.integrationAuthentication.getSession(integrationId, {
-			domain: metadata.domain,
-			scopes: metadata.scopes,
-		});
-		if (session == null) return undefined;
-
-		return {
-			provider: integrationId,
-			token: session.accessToken,
-		};
-	}
-
 	async getProviderAuthForDraft(draft: Draft): Promise<ProviderAuth | undefined> {
 		if (draft.changesets == null || draft.changesets.length === 0) {
 			return undefined;
@@ -829,7 +821,7 @@ export class DraftService implements Disposable {
 			repo = repositoryOrIdentity;
 		}
 
-		return this.getProviderAuthFromRepository(repo);
+		return this.getProviderAuthFromRepoOrIntegrationId(repo);
 	}
 
 	async getCodeSuggestions(
@@ -850,9 +842,7 @@ export class DraftService implements Disposable {
 	): Promise<Draft[]> {
 		const entityIdentifier = getEntityIdentifierInput(item);
 		const prEntityId = EntityIdentifierUtils.encode(entityIdentifier);
-		const providerAuth = isRepository(repositoryOrIntegrationId)
-			? await this.getProviderAuthFromRepository(repositoryOrIntegrationId)
-			: await this.getProviderAuthForIntegration(repositoryOrIntegrationId);
+		const providerAuth = await this.getProviderAuthFromRepoOrIntegrationId(repositoryOrIntegrationId);
 
 		// swallowing this error as we don't need to fail here
 		try {

src/plus/integrations/authentication/integrationAuthentication.ts

@@ -143,7 +143,7 @@ export class IntegrationAuthenticationService implements Disposable {
 		}
 	}
 
-	private getSecretKey(providerId: string, id: string): `gitlens.integration.auth:${string}` {
+	private getSecretKey(providerId: IntegrationId, id: string): `gitlens.integration.auth:${IntegrationId}|${string}` {
 		return `gitlens.integration.auth:${providerId}|${id}`;
 	}
 

src/plus/integrations/integration.ts

@@ -24,6 +24,7 @@ import type { LogScope } from '../../system/logger.scope';
 import { getLogScope } from '../../system/logger.scope';
 import type {
 	IntegrationAuthenticationProviderDescriptor,
+	IntegrationAuthenticationService,
 	IntegrationAuthenticationSessionDescriptor,
 } from './authentication/integrationAuthentication';
 import type { ProviderAuthenticationSession } from './authentication/models';
@@ -88,6 +89,7 @@ export abstract class IntegrationBase<
 
 	constructor(
 		protected readonly container: Container,
+		protected readonly authenticationService: IntegrationAuthenticationService,
 		protected readonly getProvidersApi: () => Promise<ProvidersApi>,
 	) {}
 
@@ -125,7 +127,7 @@ export abstract class IntegrationBase<
 	}
 
 	protected _session: ProviderAuthenticationSession | null | undefined;
-	protected session() {
+	getSession() {
 		if (this._session === undefined) {
 			return this.ensureSession(false);
 		}
@@ -151,40 +153,38 @@ export abstract class IntegrationBase<
 
 		const connected = this._session != null;
 
-		if (connected && !options?.silent) {
-			if (options?.currentSessionOnly) {
-				void showIntegrationDisconnectedTooManyFailedRequestsWarningMessage(this.name);
+		let signOut = !options?.currentSessionOnly;
+
+		if (connected && !options?.currentSessionOnly && !options?.silent) {
+			const disable = { title: 'Disable' };
+			const disableAndSignOut = { title: 'Disable & Sign Out' };
+			const cancel = { title: 'Cancel', isCloseAffordance: true };
+
+			let result: MessageItem | undefined;
+			if (this.authenticationService.supports(this.authProvider.id)) {
+				result = await window.showWarningMessage(
+					`Are you sure you want to disable the rich integration with ${this.name}?\n\nNote: signing out clears the saved authentication.`,
+					{ modal: true },
+					disable,
+					disableAndSignOut,
+					cancel,
+				);
 			} else {
-				const disable = { title: 'Disable' };
-				const signout = { title: 'Disable & Sign Out' };
-				const cancel = { title: 'Cancel', isCloseAffordance: true };
-
-				let result: MessageItem | undefined;
-				if (this.container.integrationAuthentication.supports(this.authProvider.id)) {
-					result = await window.showWarningMessage(
-						`Are you sure you want to disable the rich integration with ${this.name}?\n\nNote: signing out clears the saved authentication.`,
-						{ modal: true },
-						disable,
-						signout,
-						cancel,
-					);
-				} else {
-					result = await window.showWarningMessage(
-						`Are you sure you want to disable the rich integration with ${this.name}?`,
-						{ modal: true },
-						disable,
-						cancel,
-					);
-				}
-
-				if (result == null || result === cancel) return;
-				if (result === signout) {
-					void this.container.integrationAuthentication.deleteSession(
-						this.authProvider.id,
-						this.authProviderDescriptor,
-					);
-				}
+				result = await window.showWarningMessage(
+					`Are you sure you want to disable the rich integration with ${this.name}?`,
+					{ modal: true },
+					disable,
+					cancel,
+				);
 			}
+
+			if (result == null || result === cancel) return;
+
+			signOut = result === disableAndSignOut;
+		}
+
+		if (signOut) {
+			void this.authenticationService.deleteSession(this.authProvider.id, this.authProviderDescriptor);
 		}
 
 		this.resetRequestExceptionCount();
@@ -241,14 +241,15 @@ export abstract class IntegrationBase<
 		this.requestExceptionCount++;
 
 		if (this.requestExceptionCount >= 5 && this._session !== null) {
+			void showIntegrationDisconnectedTooManyFailedRequestsWarningMessage(this.name);
 			void this.disconnect({ currentSessionOnly: true });
 		}
 	}
 
 	@gate()
 	@debug({ exit: true })
 	async isConnected(): Promise<boolean> {
-		return (await this.session()) != null;
+		return (await this.getSession()) != null;
 	}
 
 	@gate()
@@ -267,11 +268,10 @@ export abstract class IntegrationBase<
 
 		let session: ProviderAuthenticationSession | undefined | null;
 		try {
-			session = await this.container.integrationAuthentication.getSession(
-				this.authProvider.id,
-				this.authProviderDescriptor,
-				{ createIfNeeded: createIfNeeded, forceNewSession: forceNewSession },
-			);
+			session = await this.authenticationService.getSession(this.authProvider.id, this.authProviderDescriptor, {
+				createIfNeeded: createIfNeeded,
+				forceNewSession: forceNewSession,
+			});
 		} catch (ex) {
 			await this.container.storage.deleteWorkspace(this.connectedKey);
 

src/plus/integrations/integrationService.ts

@@ -12,6 +12,7 @@ import { debug, log } from '../../system/decorators/log';
 import { take } from '../../system/event';
 import { filterMap, flatten } from '../../system/iterable';
 import type { SubscriptionChangeEvent } from '../gk/account/subscriptionService';
+import type { IntegrationAuthenticationService } from './authentication/integrationAuthentication';
 import { supportedCloudIntegrationIds, toIntegrationId } from './authentication/models';
 import type {
 	HostingIntegration,
@@ -51,7 +52,10 @@ export class IntegrationService implements Disposable {
 	private readonly _disposable: Disposable;
 	private _integrations = new Map<IntegrationKey, Integration>();
 
-	constructor(private readonly container: Container) {
+	constructor(
+		private readonly container: Container,
+		private readonly authenticationService: IntegrationAuthenticationService,
+	) {
 		this._disposable = Disposable.from(
 			configuration.onDidChange(e => {
 				if (configuration.changed(e, 'remotes')) {
@@ -222,39 +226,53 @@ export class IntegrationService implements Disposable {
 				case HostingIntegrationId.GitHub:
 					integration = new (
 						await import(/* webpackChunkName: "integrations" */ './providers/github')
-					).GitHubIntegration(this.container, this.getProvidersApi.bind(this));
+					).GitHubIntegration(this.container, this.authenticationService, this.getProvidersApi.bind(this));
 					break;
 				case SelfHostedIntegrationId.GitHubEnterprise:
 					if (domain == null) throw new Error(`Domain is required for '${id}' integration`);
 					integration = new (
 						await import(/* webpackChunkName: "integrations" */ './providers/github')
-					).GitHubEnterpriseIntegration(this.container, this.getProvidersApi.bind(this), domain);
+					).GitHubEnterpriseIntegration(
+						this.container,
+						this.authenticationService,
+						this.getProvidersApi.bind(this),
+						domain,
+					);
 					break;
 				case HostingIntegrationId.GitLab:
 					integration = new (
 						await import(/* webpackChunkName: "integrations" */ './providers/gitlab')
-					).GitLabIntegration(this.container, this.getProvidersApi.bind(this));
+					).GitLabIntegration(this.container, this.authenticationService, this.getProvidersApi.bind(this));
 					break;
 				case SelfHostedIntegrationId.GitLabSelfHosted:
 					if (domain == null) throw new Error(`Domain is required for '${id}' integration`);
 					integration = new (
 						await import(/* webpackChunkName: "integrations" */ './providers/gitlab')
-					).GitLabSelfHostedIntegration(this.container, this.getProvidersApi.bind(this), domain);
+					).GitLabSelfHostedIntegration(
+						this.container,
+						this.authenticationService,
+						this.getProvidersApi.bind(this),
+						domain,
+					);
 					break;
 				case HostingIntegrationId.Bitbucket:
 					integration = new (
 						await import(/* webpackChunkName: "integrations" */ './providers/bitbucket')
-					).BitbucketIntegration(this.container, this.getProvidersApi.bind(this));
+					).BitbucketIntegration(this.container, this.authenticationService, this.getProvidersApi.bind(this));
 					break;
 				case HostingIntegrationId.AzureDevOps:
 					integration = new (
 						await import(/* webpackChunkName: "integrations" */ './providers/azureDevOps')
-					).AzureDevOpsIntegration(this.container, this.getProvidersApi.bind(this));
+					).AzureDevOpsIntegration(
+						this.container,
+						this.authenticationService,
+						this.getProvidersApi.bind(this),
+					);
 					break;
 				case IssueIntegrationId.Jira:
 					integration = new (
 						await import(/* webpackChunkName: "integrations" */ './providers/jira')
-					).JiraIntegration(this.container, this.getProvidersApi.bind(this));
+					).JiraIntegration(this.container, this.authenticationService, this.getProvidersApi.bind(this));
 					break;
 				default:
 					throw new Error(`Integration with '${id}' is not supported`);
@@ -269,10 +287,11 @@ export class IntegrationService implements Disposable {
 	private async getProvidersApi() {
 		if (this._providersApi == null) {
 			const container = this.container;
+			const authenticationService = this.authenticationService;
 			async function load() {
 				return new (
 					await import(/* webpackChunkName: "integrations-api" */ './providers/providersApi')
-				).ProvidersApi(container);
+				).ProvidersApi(container, authenticationService);
 			}
 
 			this._providersApi = load();

src/plus/integrations/providers/github.ts

@@ -9,7 +9,10 @@ import { PullRequest } from '../../../git/models/pullRequest';
 import type { RepositoryMetadata } from '../../../git/models/repositoryMetadata';
 import { log } from '../../../system/decorators/log';
 import { ensurePaidPlan } from '../../utils';
-import type { IntegrationAuthenticationProviderDescriptor } from '../authentication/integrationAuthentication';
+import type {
+	IntegrationAuthenticationProviderDescriptor,
+	IntegrationAuthenticationService,
+} from '../authentication/integrationAuthentication';
 import type { SupportedIntegrationIds } from '../integration';
 import { HostingIntegration } from '../integration';
 import { HostingIntegrationId, providersMetadata, SelfHostedIntegrationId } from './models';
@@ -282,10 +285,11 @@ export class GitHubEnterpriseIntegration extends GitHubIntegrationBase<SelfHoste
 
 	constructor(
 		container: Container,
+		authenticationService: IntegrationAuthenticationService,
 		getProvidersApi: () => Promise<ProvidersApi>,
 		private readonly _domain: string,
 	) {
-		super(container, getProvidersApi);
+		super(container, authenticationService, getProvidersApi);
 	}
 
 	@log()