Skip to content

Commit

Permalink
Address feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@jeanp413
jeanp413 committed Apr 20, 2022
1 parent 71b6ac8 commit ac5fa95
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 33 deletions.
36 changes: 27 additions & 9 deletions extensions/gitpod/src/authentication.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import Log from './common/logger';
import { arrayEquals } from './common/utils';
import { Disposable } from './common/dispose';
import TelemetryReporter from './telemetryReporter';
import { getGitpodHost } from './settingsSync';

interface SessionData {
id: string;
Expand All @@ -37,13 +36,26 @@ export default class GitpodAuthenticationProvider extends Disposable implements

this._logger = logger;
this._telemetry = telemetry;
this._gitpodServer = new GitpodServer(this._logger);
this._keychain = new Keychain(this.context, `gitpod.auth.${getGitpodHost().host}`, this._logger);
this._keychain = new Keychain(this.context, `gitpod.auth`, this._logger);

const serviceUrl = vscode.workspace.getConfiguration('gitpod').get<string>('host')!;
this._gitpodServer = new GitpodServer(serviceUrl, this._logger);
this._register(vscode.workspace.onDidChangeConfiguration(async e => {
if (e.affectsConfiguration('gitpod.host')) {
const serviceUrl = vscode.workspace.getConfiguration('gitpod').get<string>('host')!;
this._gitpodServer.dispose();
this._gitpodServer = new GitpodServer(serviceUrl, this._logger);

// Remove all sessions when gitpod.host changes
const sessions = await this._sessionsPromise;
await this._keychain.deleteToken();
this._sessionChangeEmitter.fire({ added: [], removed: sessions, changed: [] });
}
}));

// Contains the current state of the sessions we have available.
this._sessionsPromise = this.readSessions();

this._register(this._gitpodServer);
this._register(vscode.authentication.registerAuthenticationProvider('gitpod', 'Gitpod', this, { supportsMultipleAccounts: false }));
this._register(this.context.secrets.onDidChange(() => this.checkForUpdates()));
}
Expand Down Expand Up @@ -174,7 +186,8 @@ export default class GitpodAuthenticationProvider extends Disposable implements
// For the Gitpod scope list, order doesn't matter so we immediately sort the scopes
const sortedScopes = scopes.sort();

this._telemetry.sendTelemetryEvent('login', {
this._telemetry.sendTelemetryEvent('gitpod_desktop_auth', {
kind: 'login',
scopes: JSON.stringify(sortedScopes),
});

Expand All @@ -199,11 +212,11 @@ export default class GitpodAuthenticationProvider extends Disposable implements
} catch (e) {
// If login was cancelled, do not notify user.
if (e === 'Cancelled' || e.message === 'Cancelled') {
this._telemetry.sendTelemetryEvent('login_cancelled');
this._telemetry.sendTelemetryEvent('gitpod_desktop_auth', { kind: 'login_cancelled' });
throw e;
}

this._telemetry.sendTelemetryEvent('login_failed');
this._telemetry.sendTelemetryEvent('gitpod_desktop_auth', { kind: 'login_failed' });

vscode.window.showErrorMessage(`Sign in failed: ${e}`);
this._logger.error(e);
Expand All @@ -223,7 +236,7 @@ export default class GitpodAuthenticationProvider extends Disposable implements

public async removeSession(id: string) {
try {
this._telemetry.sendTelemetryEvent('logout');
this._telemetry.sendTelemetryEvent('gitpod_desktop_auth', { kind: 'logout' });

this._logger.info(`Logging out of ${id}`);

Expand All @@ -240,7 +253,7 @@ export default class GitpodAuthenticationProvider extends Disposable implements
this._logger.error('Session not found');
}
} catch (e) {
this._telemetry.sendTelemetryEvent('logout_failed');
this._telemetry.sendTelemetryEvent('gitpod_desktop_auth', { kind: 'logout_failed' });

vscode.window.showErrorMessage(`Sign out failed: ${e}`);
this._logger.error(e);
Expand All @@ -251,4 +264,9 @@ export default class GitpodAuthenticationProvider extends Disposable implements
public handleUri(uri: vscode.Uri) {
this._gitpodServer.hadleUri(uri);
}

public override dispose() {
super.dispose();
this._gitpodServer.dispose();
}
}
9 changes: 3 additions & 6 deletions extensions/gitpod/src/common/telemetry.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -363,26 +363,24 @@ export class BaseTelemetryReporter extends Disposable {
* Properties are sanitized on best-effort basis to remove sensitive data prior to sending.
* @param eventName The name of the event
* @param properties The properties to send with the event
* @param measurements The measurements (numeric values) to send with the event
*/
public sendTelemetryEvent(eventName: string, properties?: TelemetryEventProperties): void {
if (this.userOptIn && eventName !== '') {
properties = { ...properties, ...this.getCommonProperties() };
const cleanProperties = this.cloneAndChange(properties, (_key: string, prop: string) => this.anonymizeFilePaths(prop, false));
this.telemetryAppender.logEvent(`${this.extensionId}/${eventName}`, { properties: this.removePropertiesWithPossibleUserInfo(cleanProperties) });
this.telemetryAppender.logEvent(`${eventName}`, { properties: this.removePropertiesWithPossibleUserInfo(cleanProperties) });
}
}

/**
* Given an event name, some properties, and measurements sends a raw (unsanitized) telemetry event
* @param eventName The name of the event
* @param properties The properties to send with the event
* @param measurements The measurements (numeric values) to send with the event
*/
public sendRawTelemetryEvent(eventName: string, properties?: RawTelemetryEventProperties): void {
if (this.userOptIn && eventName !== '') {
properties = { ...properties, ...this.getCommonProperties() };
this.telemetryAppender.logEvent(`${this.extensionId}/${eventName}`, { properties });
this.telemetryAppender.logEvent(`${eventName}`, { properties });
}
}

Expand All @@ -409,15 +407,14 @@ export class BaseTelemetryReporter extends Disposable {

return this.anonymizeFilePaths(prop, false);
});
this.telemetryAppender.logEvent(`${this.extensionId}/${eventName}`, { properties: this.removePropertiesWithPossibleUserInfo(cleanProperties) });
this.telemetryAppender.logEvent(`${eventName}`, { properties: this.removePropertiesWithPossibleUserInfo(cleanProperties) });
}
}

/**
* Given an error, properties, and measurements. Sends an exception event
* @param error The error to send
* @param properties The properties to send with the event
* @param measurements The measurements (numeric values) to send with the event
*/
public sendTelemetryException(error: Error, properties?: TelemetryEventProperties): void {
if (this.shouldSendErrorTelemetry() && this.errorOptIn && error) {
Expand Down
2 changes: 1 addition & 1 deletion extensions/gitpod/src/extension.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export async function activate(context: vscode.ExtensionContext) {
/* Gitpod settings sync */
await updateSyncContext();
context.subscriptions.push(vscode.workspace.onDidChangeConfiguration(e => {
if (e.affectsConfiguration('gitpod')) {
if (e.affectsConfiguration('gitpod.host') || e.affectsConfiguration('configurationSync.store')) {
updateSyncContext();
}
}));
Expand Down
18 changes: 9 additions & 9 deletions extensions/gitpod/src/gitpodServer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { withServerApi } from './internalApi';
import pkceChallenge from 'pkce-challenge';
import { v4 as uuid } from 'uuid';
import { Disposable } from './common/dispose';
import { getGitpodHost } from './settingsSync';

interface ExchangeTokenResponse {
token_type: 'Bearer';
Expand All @@ -33,21 +32,15 @@ export class GitpodServer extends Disposable {
public static AUTH_COMPLETE_PATH = '/complete-gitpod-auth';

private _serviceUrl: string;

private _pendingStates = new Map<string, string[]>();
private _pendingVerifiers = new Map<string, string>();
private _codeExchangePromises = new Map<string, { promise: Promise<string>; cancel: vscode.EventEmitter<void> }>();
private _uriEmitter = this._register(new vscode.EventEmitter<vscode.Uri>());

constructor(private readonly _logger: Log) {
constructor(serviceUrl: string, private readonly _logger: Log) {
super();

this._serviceUrl = getGitpodHost().toString();
this._register(vscode.workspace.onDidChangeConfiguration(e => {
if (e.affectsConfiguration('gitpod')) {
this._serviceUrl = getGitpodHost().toString();
}
}));
this._serviceUrl = new URL(serviceUrl).toString().replace(/\/$/, '');
}

public async login(scopes: string): Promise<string> {
Expand Down Expand Up @@ -173,4 +166,11 @@ export class GitpodServer extends Disposable {
public hadleUri(uri: vscode.Uri) {
this._uriEmitter.fire(uri);
}

public override dispose() {
super.dispose();
for (const [, { cancel }] of this._codeExchangePromises) {
cancel.fire();
}
}
}
15 changes: 7 additions & 8 deletions extensions/gitpod/src/settingsSync.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,8 @@ interface ConfigurationSyncStore {
authenticationProviders: Record<string, { scopes: string[] }>;
}

export const getGitpodHost = () => new URL(vscode.workspace.getConfiguration('gitpod').get<string>('host') || 'https://gitpod.io/');

function getGitpodSyncProviderConfig(): ConfigurationSyncStore {
const serviceUrl = getGitpodHost().toString();
const syncStoreURL = `${serviceUrl}/code-sync`;
function getGitpodSyncProviderConfig(serviceUrl: string): ConfigurationSyncStore {
const syncStoreURL = `${new URL(serviceUrl).toString().replace(/\/$/, '')}/code-sync`;
return {
url: syncStoreURL,
stableUrl: syncStoreURL,
Expand All @@ -42,7 +39,8 @@ function getGitpodSyncProviderConfig(): ConfigurationSyncStore {
export async function updateSyncContext() {
const config = vscode.workspace.getConfiguration();
const syncProviderConfig = config.get('configurationSync.store');
const gitpodSyncProviderConfig = getGitpodSyncProviderConfig();
const serviceUrl = config.get<string>('gitpod.host')!;
const gitpodSyncProviderConfig = getGitpodSyncProviderConfig(serviceUrl);
const addedSyncProvider = syncProviderConfig && JSON.stringify(syncProviderConfig) === JSON.stringify(gitpodSyncProviderConfig);
await vscode.commands.executeCommand('setContext', 'gitpod.addedSyncProvider', !!addedSyncProvider);
}
Expand All @@ -66,7 +64,8 @@ export async function enableSettingsSync(enabled: boolean, telemetry: TelemetryR
const config = vscode.workspace.getConfiguration();
const currentSyncProviderConfig: ConfigurationSyncStore | undefined = config.get('configurationSync.store');
const currentIgnoredSettingsConfig: string[] | undefined = config.get('settingsSync.ignoredSettings');
const gitpodSyncProviderConfig = getGitpodSyncProviderConfig();
const serviceUrl = config.get<string>('gitpod.host')!;
const gitpodSyncProviderConfig = getGitpodSyncProviderConfig(serviceUrl);
if (enabled) {
if (JSON.stringify(currentSyncProviderConfig) === JSON.stringify(gitpodSyncProviderConfig)) {
return;
Expand All @@ -87,7 +86,7 @@ export async function enableSettingsSync(enabled: boolean, telemetry: TelemetryR
await config.update('settingsSync.ignoredSettings', newIgnoredSettingsConfig, vscode.ConfigurationTarget.Global);
await config.update('configurationSync.store', newSyncProviderConfig, vscode.ConfigurationTarget.Global);

telemetry.sendTelemetryEvent('sync_provider_enabled', { enabled: String(enabled) });
telemetry.sendTelemetryEvent('gitpod_desktop_settings_sync', { enabled: String(enabled) });

await promptToCloseAll();
}

0 comments on commit ac5fa95

Please sign in to comment.