Skip to content

Commit

Permalink
Improve GHES flow (microsoft#172959)
Browse files Browse the repository at this point in the history
* Improve GHES flow

It's clear to me now that a lot of users of the GHES flow have auth behind a VPN. Something that we cannot support in the OAuth flow because we require making a request to vscode.dev who makes a request to the GHES instance.

In light of this, we will remove the OAuth flow for GHES. Fear not, though... because both the Device Code Flow and the PAT flow are there and the PAT flow has been improved in 1.75... so in conclusion, I think GHES is handled as best we can with this PR.

Additionally, there is some "Hosted GitHub Enterprise" scenarios popping up, and this future proofs that by allowing "Hosted GitHub Enterprise" experiences to go through the OAuth flow because we know for a fact that they will be accessible from vscode.dev.

Fixes microsoft#169619
Fixes microsoft/vscode-internalbacklog#3398

* Use api. sub domain and update telemetry logic
  • Loading branch information
TylerLeonhardt authored Jan 31, 2023
1 parent ab060f0 commit 06eb374
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 44 deletions.
10 changes: 9 additions & 1 deletion extensions/github-authentication/src/common/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import { Uri } from 'vscode';
import { AuthProviderType } from '../github';

const VALID_DESKTOP_CALLBACK_SCHEMES = [
'vscode',
Expand All @@ -15,7 +16,7 @@ const VALID_DESKTOP_CALLBACK_SCHEMES = [
'vscode-exploration'
];

export function isSupportedEnvironment(uri: Uri): boolean {
export function isSupportedClient(uri: Uri): boolean {
return (
VALID_DESKTOP_CALLBACK_SCHEMES.includes(uri.scheme) ||
// vscode.dev & insiders.vscode.dev
Expand All @@ -24,3 +25,10 @@ export function isSupportedEnvironment(uri: Uri): boolean {
/(?:^|\.)github\.dev$/.test(uri.authority)
);
}

export function isSupportedTarget(type: AuthProviderType, gheUri?: Uri): boolean {
return (
type === AuthProviderType.github ||
/\.ghe\.com$/.test(gheUri!.authority)
);
}
80 changes: 37 additions & 43 deletions extensions/github-authentication/src/githubServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ import { PromiseAdapter, promiseFromEvent } from './common/utils';
import { ExperimentationTelemetry } from './common/experimentationService';
import { AuthProviderType, UriEventHandler } from './github';
import { Log } from './common/logger';
import { isSupportedEnvironment } from './common/env';
import { isSupportedClient, isSupportedTarget } from './common/env';
import { LoopbackAuthServer } from './node/authServer';
import { crypto } from './node/crypto';
import { fetching } from './node/fetch';

const CLIENT_ID = '01ab8ac9400c4e429b23';
const GITHUB_TOKEN_URL = 'https://vscode.dev/codeExchangeProxyEndpoints/github/login/oauth/access_token';
const GITHUB_TOKEN_URL = 'http://localhost:3000/codeExchangeProxyEndpoints/github/login/oauth/access_token';
const NETWORK_ERROR = 'network error';

const REDIRECT_URL_STABLE = 'https://vscode.dev/redirect';
Expand Down Expand Up @@ -96,30 +96,15 @@ export class GitHubServer implements IGitHubServer {
if (proxyEndpoints?.github && new URL(proxyEndpoints.github).hostname === 'insiders.vscode.dev') {
this._redirectEndpoint = REDIRECT_URL_INSIDERS;
}
return this._redirectEndpoint;
} else {
// GHES
const result = await fetching(this.getServerUri('/meta').toString(true));
if (result.ok) {
try {
const json: { installed_version: string } = await result.json();
const [majorStr, minorStr, _patch] = json.installed_version.split('.');
const major = Number(majorStr);
const minor = Number(minorStr);
if (major >= 4 || major === 3 && minor >= 8
) {
// GHES 3.8 and above used vscode.dev/redirect as the route.
// It only supports a single redirect endpoint, so we can't use
// insiders.vscode.dev/redirect when we're running in Insiders, unfortunately.
this._redirectEndpoint = 'https://vscode.dev/redirect';
}
} catch (e) {
this._logger.error(e);
}
}

// TODO in like 1 year change the default vscode.dev/redirect maybe
this._redirectEndpoint = 'https://vscode-auth.github.com/';
// GHE only supports a single redirect endpoint, so we can't use
// insiders.vscode.dev/redirect when we're running in Insiders, unfortunately.
// Additionally, we make the assumption that this function will only be used
// in flows that target supported GHE targets, not on-prem GHES. Because of this
// assumption, we can assume that the GHE version used is at least 3.8 which is
// the version that changed the redirect endpoint to this URI from the old
// GitHub maintained server.
this._redirectEndpoint = 'https://vscode.dev/redirect';
}
return this._redirectEndpoint;
}
Expand Down Expand Up @@ -154,8 +139,9 @@ export class GitHubServer implements IGitHubServer {
const nonce: string = crypto.getRandomValues(new Uint32Array(2)).reduce((prev, curr) => prev += curr.toString(16), '');
const callbackUri = await vscode.env.asExternalUri(vscode.Uri.parse(`${vscode.env.uriScheme}://vscode.github-authentication/did-authenticate?nonce=${encodeURIComponent(nonce)}`));

const supported = isSupportedEnvironment(callbackUri);
if (supported) {
const supportedClient = isSupportedClient(callbackUri);
const supportedTarget = isSupportedTarget(this._type, this._ghesUri);
if (supportedClient && supportedTarget) {
try {
return await this.doLoginWithoutLocalServer(scopes, nonce, callbackUri);
} catch (e) {
Expand All @@ -167,9 +153,11 @@ export class GitHubServer implements IGitHubServer {
// Starting a local server is only supported if:
// 1. We are in a UI extension because we need to open a port on the machine that has the browser
// 2. We are in a node runtime because we need to open a port on the machine
// 3. code exchange can only be done with a supported target
if (
this._extensionKind === vscode.ExtensionKind.UI &&
typeof navigator === 'undefined'
typeof navigator === 'undefined' &&
supportedTarget
) {
try {
await promptToContinue();
Expand All @@ -193,7 +181,7 @@ export class GitHubServer implements IGitHubServer {

// In a supported environment, we can't use PAT auth because we use this auth for Settings Sync and it doesn't support PATs.
// With that said, GitHub Enterprise isn't used by Settings Sync so we can use PATs for that.
if (!supported || this._type === AuthProviderType.githubEnterprise) {
if (!supportedClient || this._type === AuthProviderType.githubEnterprise) {
try {
await promptToContinue();
return await this.doLoginWithPat(scopes);
Expand Down Expand Up @@ -502,11 +490,12 @@ export class GitHubServer implements IGitHubServer {
}

private getServerUri(path: string = '') {
if (this._type === AuthProviderType.github) {
return vscode.Uri.parse('https://api.github.com').with({ path });
}
// GHES
const apiUri = this.baseUri;
// github.com and Hosted GitHub Enterprise instances
if (isSupportedTarget(this._type, this._ghesUri)) {
return vscode.Uri.parse(`${apiUri.scheme}://api.${apiUri.authority}`).with({ path });
}
// GitHub Enterprise Server (aka on-prem)
return vscode.Uri.parse(`${apiUri.scheme}://${apiUri.authority}/api/v3${path}`);
}

Expand Down Expand Up @@ -613,28 +602,33 @@ export class GitHubServer implements IGitHubServer {

private async checkEnterpriseVersion(token: string): Promise<void> {
try {
let version: string;
if (!isSupportedTarget(this._type, this._ghesUri)) {
const result = await fetching(this.getServerUri('/meta').toString(), {
headers: {
Authorization: `token ${token}`,
'User-Agent': 'Visual-Studio-Code'
}
});

const result = await fetching(this.getServerUri('/meta').toString(), {
headers: {
Authorization: `token ${token}`,
'User-Agent': 'Visual-Studio-Code'
if (!result.ok) {
return;
}
});

if (!result.ok) {
return;
const json: { verifiable_password_authentication: boolean; installed_version: string } = await result.json();
version = json.installed_version;
} else {
version = 'hosted';
}

const json: { verifiable_password_authentication: boolean; installed_version: string } = await result.json();

/* __GDPR__
"ghe-session" : {
"owner": "TylerLeonhardt",
"version": { "classification": "SystemMetaData", "purpose": "FeatureInsight" }
}
*/
this._telemetryReporter.sendTelemetryEvent('ghe-session', {
version: json.installed_version
version
});
} catch {
// No-op
Expand Down

0 comments on commit 06eb374

Please sign in to comment.