Skip to content

Commit d67e4ef

Browse files
Merge pull request microsoft#188111 from microsoft/tyler/motionless-crawdad
Try to revoke tokens that are getting deleted
2 parents 11e6a95 + ac27273 commit d67e4ef

File tree

7 files changed

+85
-3
lines changed

7 files changed

+85
-3
lines changed

extensions/github-authentication/extension-browser.webpack.config.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ module.exports = withBrowserDefaults({
2121
'uuid': path.resolve(__dirname, 'node_modules/uuid/dist/esm-browser/index.js'),
2222
'./node/authServer': path.resolve(__dirname, 'src/browser/authServer'),
2323
'./node/crypto': path.resolve(__dirname, 'src/browser/crypto'),
24-
'./node/fetch': path.resolve(__dirname, 'src/browser/fetch')
24+
'./node/fetch': path.resolve(__dirname, 'src/browser/fetch'),
25+
'./node/buffer': path.resolve(__dirname, 'src/browser/buffer'),
2526
}
2627
}
2728
});
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
export function base64Encode(text: string): string {
7+
return btoa(text);
8+
}

extensions/github-authentication/src/common/logger.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,4 +26,7 @@ export class Log {
2626
this.output.error(message);
2727
}
2828

29+
public warn(message: string): void {
30+
this.output.warn(message);
31+
}
2932
}

extensions/github-authentication/src/flows.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,8 @@ const allFlows: IFlow[] = [
201201
supportsGitHubEnterpriseServer: false,
202202
supportsHostedGitHubEnterprise: true,
203203
supportsRemoteExtensionHost: true,
204-
supportsWebWorkerExtensionHost: true,
204+
// Web worker can't open a port to listen for the redirect
205+
supportsWebWorkerExtensionHost: false,
205206
// exchanging a code for a token requires a client secret
206207
supportsNoClientSecret: false,
207208
supportsSupportedClients: true,

extensions/github-authentication/src/github.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,7 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid
363363
sessions.splice(sessionIndex, 1);
364364

365365
await this.storeSessions(sessions);
366+
await this._githubServer.logout(session);
366367

367368
this._sessionChangeEmitter.fire({ added: [], removed: [session], changed: [] });
368369
} else {

extensions/github-authentication/src/githubServer.ts

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import { crypto } from './node/crypto';
1212
import { fetching } from './node/fetch';
1313
import { ExtensionHost, GitHubTarget, getFlows } from './flows';
1414
import { NETWORK_ERROR, USER_CANCELLATION_ERROR } from './common/errors';
15+
import { Config } from './config';
16+
import { base64Encode } from './node/buffer';
1517

1618
// This is the error message that we throw if the login was cancelled for any reason. Extensions
1719
// calling `getSession` can handle this error to know that the user cancelled the login.
@@ -22,6 +24,7 @@ const REDIRECT_URL_INSIDERS = 'https://insiders.vscode.dev/redirect';
2224

2325
export interface IGitHubServer {
2426
login(scopes: string): Promise<string>;
27+
logout(session: vscode.AuthenticationSession): Promise<void>;
2528
getUserInfo(token: string): Promise<{ id: string; accountName: string }>;
2629
sendAdditionalTelemetryInfo(session: vscode.AuthenticationSession): Promise<void>;
2730
friendlyName: string;
@@ -78,9 +81,14 @@ export class GitHubServer implements IGitHubServer {
7881
}
7982

8083
// TODO@joaomoreno TODO@TylerLeonhardt
84+
private _isNoCorsEnvironment: boolean | undefined;
8185
private async isNoCorsEnvironment(): Promise<boolean> {
86+
if (this._isNoCorsEnvironment !== undefined) {
87+
return this._isNoCorsEnvironment;
88+
}
8289
const uri = await vscode.env.asExternalUri(vscode.Uri.parse(`${vscode.env.uriScheme}://vscode.github-authentication/dummy`));
83-
return (uri.scheme === 'https' && /^((insiders\.)?vscode|github)\./.test(uri.authority)) || (uri.scheme === 'http' && /^localhost/.test(uri.authority));
90+
this._isNoCorsEnvironment = (uri.scheme === 'https' && /^((insiders\.)?vscode|github)\./.test(uri.authority)) || (uri.scheme === 'http' && /^localhost/.test(uri.authority));
91+
return this._isNoCorsEnvironment;
8492
}
8593

8694
public async login(scopes: string): Promise<string> {
@@ -144,6 +152,58 @@ export class GitHubServer implements IGitHubServer {
144152
throw new Error(userCancelled ? CANCELLATION_ERROR : 'No auth flow succeeded.');
145153
}
146154

155+
public async logout(session: vscode.AuthenticationSession): Promise<void> {
156+
this._logger.trace(`Deleting session (${session.id}) from server...`);
157+
158+
if (!Config.gitHubClientSecret) {
159+
this._logger.warn('No client secret configured for GitHub authentication. The token has been deleted with best effort on this system, but we are unable to delete the token on server without the client secret.');
160+
return;
161+
}
162+
163+
// Only attempt to delete OAuth tokens. They are always prefixed with `gho_`.
164+
// https://docs.github.com/en/rest/apps/oauth-applications#about-oauth-apps-and-oauth-authorizations-of-github-apps
165+
if (!session.accessToken.startsWith('gho_')) {
166+
this._logger.warn('The token being deleted is not an OAuth token. It has been deleted locally, but we cannot delete it on server.');
167+
return;
168+
}
169+
170+
if (!isSupportedTarget(this._type, this._ghesUri)) {
171+
this._logger.trace('GitHub.com and GitHub hosted GitHub Enterprise are the only options that support deleting tokens on the server. Skipping.');
172+
return;
173+
}
174+
175+
const authHeader = 'Basic ' + base64Encode(`${Config.gitHubClientId}:${Config.gitHubClientSecret}`);
176+
const uri = this.getServerUri(`/applications/${Config.gitHubClientId}/token`);
177+
178+
try {
179+
// Defined here: https://docs.github.com/en/rest/apps/oauth-applications?apiVersion=2022-11-28#delete-an-app-token
180+
const result = await fetching(uri.toString(true), {
181+
method: 'DELETE',
182+
headers: {
183+
Accept: 'application/vnd.github+json',
184+
Authorization: authHeader,
185+
'X-GitHub-Api-Version': '2022-11-28',
186+
'User-Agent': `${vscode.env.appName} (${vscode.env.appHost})`
187+
},
188+
body: JSON.stringify({ access_token: session.accessToken }),
189+
});
190+
191+
if (result.status === 204) {
192+
this._logger.trace(`Successfully deleted token from session (${session.id}) from server.`);
193+
return;
194+
}
195+
196+
try {
197+
const body = await result.text();
198+
throw new Error(body);
199+
} catch (e) {
200+
throw new Error(`${result.status} ${result.statusText}`);
201+
}
202+
} catch (e) {
203+
this._logger.warn('Failed to delete token from server.' + e.message ?? e);
204+
}
205+
}
206+
147207
private getServerUri(path: string = '') {
148208
const apiUri = this.baseUri;
149209
// github.com and Hosted GitHub Enterprise instances
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
export function base64Encode(text: string): string {
7+
return Buffer.from(text, 'binary').toString('base64');
8+
}

0 commit comments

Comments
 (0)