Skip to content

Commit

Permalink
Handle incomplete login requests gracefully, fixes microsoft#109102
Browse files Browse the repository at this point in the history
  • Loading branch information
Rachel Macfarlane committed Oct 23, 2020
1 parent 0124f68 commit 102e0e6
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 34 deletions.
79 changes: 49 additions & 30 deletions extensions/github-authentication/src/githubServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,36 +25,7 @@ export const uriHandler = new UriEventHandler;

const onDidManuallyProvideToken = new vscode.EventEmitter<string>();

const exchangeCodeForToken: (state: string) => PromiseAdapter<vscode.Uri, string> =
(state) => async (uri, resolve, reject) => {
Logger.info('Exchanging code for token...');
const query = parseQuery(uri);
const code = query.code;

if (query.state !== state) {
reject('Received mismatched state');
return;
}

try {
const result = await fetch(`https://${AUTH_RELAY_SERVER}/token?code=${code}&state=${state}`, {
method: 'POST',
headers: {
Accept: 'application/json'
}
});

if (result.ok) {
const json = await result.json();
Logger.info('Token exchange success!');
resolve(json.access_token);
} else {
reject(result.statusText);
}
} catch (ex) {
reject(ex);
}
};

function parseQuery(uri: vscode.Uri) {
return uri.query.split('&').reduce((prev: any, current) => {
Expand All @@ -67,6 +38,9 @@ function parseQuery(uri: vscode.Uri) {
export class GitHubServer {
private _statusBarItem: vscode.StatusBarItem | undefined;

private _pendingStates = new Map<string, string[]>();
private _codeExchangePromises = new Map<string, Promise<string>>();

private isTestEnvironment(url: vscode.Uri): boolean {
return url.authority === 'vscode-web-test-playground.azurewebsites.net' || url.authority.startsWith('localhost:');
}
Expand All @@ -91,18 +65,63 @@ export class GitHubServer {
this.updateStatusBarItem(false);
return token;
} else {
const existingStates = this._pendingStates.get(scopes) || [];
this._pendingStates.set(scopes, [...existingStates, state]);

const uri = vscode.Uri.parse(`https://${AUTH_RELAY_SERVER}/authorize/?callbackUri=${encodeURIComponent(callbackUri.toString())}&scope=${scopes}&state=${state}&responseType=code&authServer=https://github.com`);
await vscode.env.openExternal(uri);
}

// Register a single listener for the URI callback, in case the user starts the login process multiple times
// before completing it.
let existingPromise = this._codeExchangePromises.get(scopes);
if (!existingPromise) {
existingPromise = promiseFromEvent(uriHandler.event, this.exchangeCodeForToken(scopes));
this._codeExchangePromises.set(scopes, existingPromise);
}

return Promise.race([
promiseFromEvent(uriHandler.event, exchangeCodeForToken(state)),
existingPromise,
promiseFromEvent<string, string>(onDidManuallyProvideToken.event)
]).finally(() => {
this._pendingStates.delete(scopes);
this._codeExchangePromises.delete(scopes);
this.updateStatusBarItem(false);
});
}

private exchangeCodeForToken: (scopes: string) => PromiseAdapter<vscode.Uri, string> =
(scopes) => async (uri, resolve, reject) => {
Logger.info('Exchanging code for token...');
const query = parseQuery(uri);
const code = query.code;

const acceptedStates = this._pendingStates.get(scopes) || [];
if (!acceptedStates.includes(query.state)) {
reject('Received mismatched state');
return;
}

try {
const result = await fetch(`https://${AUTH_RELAY_SERVER}/token?code=${code}&state=${query.state}`, {
method: 'POST',
headers: {
Accept: 'application/json'
}
});

if (result.ok) {
const json = await result.json();
Logger.info('Token exchange success!');
resolve(json.access_token);
} else {
reject(result.statusText);
}
} catch (ex) {
reject(ex);
}
};

private updateStatusBarItem(isStart?: boolean) {
if (isStart && !this._statusBarItem) {
this._statusBarItem = vscode.window.createStatusBarItem(vscode.StatusBarAlignment.Left);
Expand Down
37 changes: 33 additions & 4 deletions extensions/microsoft-authentication/src/AADHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@ export class AzureActiveDirectoryService {
private _uriHandler: UriEventHandler;
private _disposables: vscode.Disposable[] = [];

// Used to keep track of current requests when not using the local server approach.
private _pendingStates = new Map<string, string[]>();
private _codeExchangePromises = new Map<string, Promise<vscode.AuthenticationSession>>();
private _codeVerfifiers = new Map<string, string>();

constructor() {
this._uriHandler = new UriEventHandler();
this._disposables.push(vscode.window.registerUriHandler(this._uriHandler));
Expand Down Expand Up @@ -385,23 +390,47 @@ export class AzureActiveDirectoryService {
}, 1000 * 60 * 5);
});

return Promise.race([this.handleCodeResponse(state, codeVerifier, scope), timeoutPromise]);
const existingStates = this._pendingStates.get(scope) || [];
this._pendingStates.set(scope, [...existingStates, state]);

// Register a single listener for the URI callback, in case the user starts the login process multiple times
// before completing it.
let existingPromise = this._codeExchangePromises.get(scope);
if (!existingPromise) {
existingPromise = this.handleCodeResponse(scope);
this._codeExchangePromises.set(scope, existingPromise);
}

this._codeVerfifiers.set(state, codeVerifier);

return Promise.race([existingPromise, timeoutPromise])
.finally(() => {
this._pendingStates.delete(scope);
this._codeExchangePromises.delete(scope);
this._codeVerfifiers.delete(state);
});
}

private async handleCodeResponse(state: string, codeVerifier: string, scope: string): Promise<vscode.AuthenticationSession> {
private async handleCodeResponse(scope: string): Promise<vscode.AuthenticationSession> {
let uriEventListener: vscode.Disposable;
return new Promise((resolve: (value: vscode.AuthenticationSession) => void, reject) => {
uriEventListener = this._uriHandler.event(async (uri: vscode.Uri) => {
try {
const query = parseQuery(uri);
const code = query.code;

const acceptedStates = this._pendingStates.get(scope) || [];
// Workaround double encoding issues of state in web
if (query.state !== state && decodeURIComponent(query.state) !== state) {
if (!acceptedStates.includes(query.state) && !acceptedStates.includes(decodeURIComponent(query.state))) {
throw new Error('State does not match.');
}

const token = await this.exchangeCodeForToken(code, codeVerifier, scope);
const verifier = this._codeVerfifiers.get(query.state) ?? this._codeVerfifiers.get(decodeURIComponent(query.state));
if (!verifier) {
throw new Error('No available code verifier');
}

const token = await this.exchangeCodeForToken(code, verifier, scope);
this.setToken(token, scope);

const session = await this.convertToSession(token);
Expand Down

0 comments on commit 102e0e6

Please sign in to comment.