Add a sign in command and improve the auth flow. Fixes #188

Adds a sign in command, and also adds a "Sign in" tree node
if there are no credentials when expanding the tree.

Author: shana

package.json

@@ -133,6 +133,16 @@
         "command": "pr.deleteLocalBranch",
         "title": "Delete Local Branch",
         "category": "GitHub Pull Requests"
+      },
+      {
+        "command": "pr.signin",
+        "title": "Sign in to GitHub",
+        "category": "GitHub Pull Requests"
+      },
+      {
+        "command": "pr.signinAndRefreshList",
+        "title": "Sign in and Refresh",
+        "category": "GitHub Pull Requests"
       }
     ],
     "menus": {
@@ -176,6 +186,10 @@
         {
           "command": "pr.refreshChanges",
           "when": "false"
+        },
+        {
+          "command": "pr.signin",
+          "when": "config.git.enabled && github:hasGitHubRemotes"
         }
       ],
       "view/title": [

src/authentication/vsConfiguration.ts

@@ -55,6 +55,14 @@ export class VSCodeConfiguration extends Configuration {
 		return this._hosts.get(host.toLocaleLowerCase());
 	}
 
+	public removeHost(host: string): void {
+		this._hosts.delete(host);
+		if (host === this.host) {
+			super.update(undefined, undefined, false);
+		}
+		this.saveConfiguration();
+	}
+
 	public update(username: string | undefined, token: string | undefined, raiseEvent: boolean = true): void {
 		super.update(username, token, raiseEvent);
 		this.saveConfiguration();

src/commands.ts

@@ -137,4 +137,14 @@ export function registerCommands(context: vscode.ExtensionContext, prManager: IP
 		});
 		return vscode.commands.executeCommand('vscode.diff', previousFileUri, fileChange.filePath, `${fileChange.fileName} from ${commit.substr(0, 8)}`, { preserveFocus: true });
 	}));
+
+	context.subscriptions.push(vscode.commands.registerCommand('pr.signin', async () => {
+		await prManager.authenticate();
+	}));
+
+	context.subscriptions.push(vscode.commands.registerCommand('pr.signinAndRefreshList', async () => {
+		if (await prManager.authenticate()) {
+			vscode.commands.executeCommand('pr.refreshList');
+		}
+	}));
 }

src/common/authentication.ts

@@ -0,0 +1,7 @@
+export class AuthenticationError extends Error {
+    name: string;
+    stack?: string;
+    constructor(public message: string) {
+        super(message);
+    }
+}

src/common/remote.ts

@@ -16,6 +16,11 @@ export class Remote {
 		return this.gitProtocol.repositoryName;
 	}
 
+	public get normalizedHost(): string {
+		const normalizedUri = this.gitProtocol.normalizeUri();
+		return `${normalizedUri.scheme}://${normalizedUri.authority}`;
+	}
+
 	constructor(
 		public readonly remoteName: string,
 		public readonly url: string,

src/github/credentials.ts

@@ -12,7 +12,7 @@ import { Remote } from '../common/remote';
 import { VSCodeConfiguration } from '../authentication/vsConfiguration';
 import Logger from '../common/logger';
 
-const SIGNIN_COMMAND = 'Sign in';
+const TRY_AGAIN = 'Try again?';
 
 export class CredentialStore {
 	private _octokits: Map<string, Octokit>;
@@ -26,28 +26,31 @@ export class CredentialStore {
 		this._octokits = new Map<string, Octokit>();
 	}
 
-	public async getOctokit(remote: Remote): Promise<Octokit> {
+	public async hasOctokit(remote: Remote): Promise<boolean> {
 		// the remote url might be http[s]/git/ssh but we always go through https for the api
 		// so use a normalized http[s] url regardless of the original protocol
 		const normalizedUri = remote.gitProtocol.normalizeUri();
 		const host = `${normalizedUri.scheme}://${normalizedUri.authority}`;
 
-		// for authentication purposes only the host portion matters
 		if (this._octokits.has(host)) {
-			return this._octokits.get(host);
+			return true;
 		}
 
 		this._configuration.setHost(host);
 
-		let octokit: Octokit;
 		const creds: IHostConfiguration = this._configuration;
 		const server = new GitHubServer(host);
-		let error: string;
+		let octokit: Octokit;
 
-		if (creds.token && await server.validate(creds.username, creds.token)) {
-			octokit = this.createOctokit('token', creds);
-		} else {
+		if (creds.token) {
+			if (await server.validate(creds.username, creds.token)) {
+				octokit = this.createOctokit('token', creds);
+			} else {
+				this._configuration.removeHost(creds.host);
+			}
+		}
 
+		if (!octokit) {
 			// see if the system keychain has something we can use
 			const data = await fill(host);
 			if (data) {
@@ -57,49 +60,56 @@ export class CredentialStore {
 					this._configuration.update(login.username, login.token, false);
 				}
 			}
+		}
 
-			const result = await vscode.window.showInformationMessage(
-				`In order to use the Pull Requests functionality, you need to sign in to ${normalizedUri.authority}`,
-				SIGNIN_COMMAND);
-
-			if (result === SIGNIN_COMMAND) {
-				try {
-					const login = await server.login();
-					if (login) {
-						octokit = this.createOctokit('token', login)
-						this._configuration.update(login.username, login.token, false);
-						vscode.window.showInformationMessage(`You are now signed in to ${normalizedUri.authority}`);
-					}
-				} catch (e) {
-					error = e;
-				}
-			}
+		if (octokit) {
+			this._octokits.set(host, octokit);
 		}
 
-		if (!octokit) {
+		return this._octokits.has(host);
+	}
 
-			Logger.appendLine(`Error signing in to ${normalizedUri.authority}: ${error}`);
+	public getOctokit(remote: Remote): Octokit {
+		const normalizedUri = remote.gitProtocol.normalizeUri();
+		const host = `${normalizedUri.scheme}://${normalizedUri.authority}`;
+		return this._octokits.get(host);
+	}
 
-			// anonymous access, not guaranteed to work for everything, and rate limited
-			if (await server.checkAnonymousAccess()) {
-				octokit = this.createOctokit('token', creds);
-				if (error) {
-					vscode.window.showWarningMessage(`Error signing in to ${normalizedUri.authority}: ${error}. Pull Requests functionality might not work correctly for this server.`)
-				} else {
-					vscode.window.showWarningMessage(`Not signed in to ${normalizedUri.authority}. Pull Requests functionality might not work correctly for this server.`)
-				}
+	public async login(remote: Remote): Promise<Octokit> {
+		// the remote url might be http[s]/git/ssh but we always go through https for the api
+		// so use a normalized http[s] url regardless of the original protocol
+		const normalizedUri = remote.gitProtocol.normalizeUri();
+		const host = `${normalizedUri.scheme}://${normalizedUri.authority}`;
 
-				// the server does not support anonymous access, disable everything
-			} else {
-				if (error) {
-					vscode.window.showErrorMessage(`Error signing in to ${normalizedUri.authority}: ${error}`);
-				} else {
-					vscode.window.showWarningMessage(`Not signed in to ${normalizedUri.authority}. Pull Requests functionality is disabled for this server.`)
+		let retry: boolean = true;
+		let octokit: Octokit;
+		const server = new GitHubServer(host);
+
+		while (retry) {
+			let error: string;
+
+			try {
+				const login = await server.login();
+				if (login) {
+					octokit = this.createOctokit('token', login)
+					this._configuration.update(login.username, login.token, false);
+					vscode.window.showInformationMessage(`You are now signed in to ${normalizedUri.authority}`);
 				}
+			} catch (e) {
+				error = e;
+			}
+
+			if (octokit) {
+				retry = false;
+			} else if (retry) {
+				Logger.appendLine(`Error signing in to ${normalizedUri.authority}: ${error}`);
+				retry = (await vscode.window.showErrorMessage(`Error signing in to ${normalizedUri.authority}`, TRY_AGAIN)) === TRY_AGAIN;
 			}
 		}
 
-		this._octokits.set(host, octokit);
+		if (octokit) {
+			this._octokits.set(host, octokit);
+		}
 		return octokit;
 	}
 

src/github/githubRepository.ts

@@ -7,21 +7,29 @@ import * as vscode from 'vscode';
 import * as Octokit from '@octokit/rest';
 import Logger from "../common/logger";
 import { Remote } from "../common/remote";
-import { PRType } from "./interface";
+import { PRType, IGitHubRepository } from "./interface";
 import { PullRequestModel } from "./pullRequestModel";
 import { CredentialStore } from './credentials';
+import { AuthenticationError } from '../common/authentication';
 
 export const PULL_REQUEST_PAGE_SIZE = 20;
+const SIGNIN_COMMAND = 'Sign in';
 
 export interface PullRequestData {
 	pullRequests: PullRequestModel[];
 	hasMorePages: boolean;
 }
-export class GitHubRepository {
+
+export class GitHubRepository implements IGitHubRepository {
 	private _octokit: Octokit;
+	private _initialized: boolean;
 	public get octokit(): Octokit {
 		if (this._octokit === undefined) {
-			throw new Error("Call ensure() before accessing this property.");
+			if (!this._initialized) {
+				throw new Error("Call ensure() before accessing this property.");
+			} else {
+				throw new AuthenticationError("Not authenticated.");
+			}
 		}
 		return this._octokit;
 	}
@@ -30,10 +38,34 @@ export class GitHubRepository {
 	}
 
 	async ensure(): Promise<GitHubRepository> {
-		this._octokit = await this._credentialStore.getOctokit(this.remote);
+		this._initialized = true;
+
+		if (!await this._credentialStore.hasOctokit(this.remote)) {
+			const normalizedUri = this.remote.gitProtocol.normalizeUri();
+			const result = await vscode.window.showInformationMessage(
+				`In order to use the Pull Requests functionality, you need to sign in to ${normalizedUri.authority}`,
+				SIGNIN_COMMAND);
+
+			if (result === SIGNIN_COMMAND) {
+				this._octokit = await this._credentialStore.login(this.remote);
+			}
+		} else {
+			this._octokit = await this._credentialStore.getOctokit(this.remote);
+		}
+
 		return this;
 	}
 
+	async authenticate(): Promise<boolean> {
+		this._initialized = true;
+		if (!await this._credentialStore.hasOctokit(this.remote)) {
+			this._octokit = await this._credentialStore.login(this.remote);
+		} else {
+			this._octokit = this._credentialStore.getOctokit(this.remote);
+		}
+		return this._octokit !== undefined;
+	}
+
 	async getDefaultBranch(): Promise<string> {
 		try {
 			const { octokit, remote } = await this.ensure();

src/github/interface.ts

@@ -129,6 +129,10 @@ export interface IPullRequestsPagingOptions {
 	fetchNextPage: boolean;
 }
 
+export interface IGitHubRepository {
+	authenticate(): Promise<boolean>;
+}
+
 export interface IPullRequestManager {
 	activePullRequest?: IPullRequestModel;
 	readonly onDidChangeActivePullRequest: vscode.Event<void>;
@@ -160,6 +164,7 @@ export interface IPullRequestManager {
 	 */
 	fullfillPullRequestMissingInfo(pullRequest: IPullRequestModel): Promise<void>;
 	updateRepositories(): Promise<void>;
+	authenticate(): Promise<boolean>;
 
 	/**
 	 * git related APIs

src/github/pullRequestManager.ts

@@ -87,6 +87,15 @@ export class PullRequestManager implements IPullRequestManager {
 		return Promise.resolve();
 	}
 
+	async authenticate(): Promise<boolean> {
+		let ret = false;
+		this._credentialStore.reset();
+		for (let repository of uniqBy(this._githubRepositories, x => x.remote.normalizedHost)) {
+			ret = await repository.authenticate() || ret;
+		}
+		return ret;
+	}
+
 	async getLocalPullRequests(): Promise<IPullRequestModel[]> {
 		const githubRepositories = this._githubRepositories;
 

src/view/treeNodes/categoryNode.ts

@@ -9,10 +9,12 @@ import { IPullRequestManager, IPullRequestModel, PRType } from '../../github/int
 import { PRNode } from './pullRequestNode';
 import { TreeNode } from './treeNode';
 import { formatError } from '../../common/utils';
+import { AuthenticationError } from '../../common/authentication';
 
 export enum PRCategoryActionType {
 	Empty,
-	More
+	More,
+	Login
 }
 
 export class PRCategoryActionNode extends TreeNode implements vscode.TreeItem {
@@ -40,6 +42,14 @@ export class PRCategoryActionNode extends TreeNode implements vscode.TreeItem {
 					]
 				}
 				break;
+			case PRCategoryActionType.Login:
+				this.label = 'Sign in';
+				this.command = {
+					title: 'Sign in',
+					command: 'pr.signinAndRefreshList',
+					arguments: []
+				}
+				break;
 			default:
 				break;
 		}
@@ -95,11 +105,13 @@ export class CategoryTreeNode extends TreeNode implements vscode.TreeItem {
 
 	async getChildren(): Promise<TreeNode[]> {
 		let hasMorePages = false;
+		let needLogin = false;
 		if (this._type === PRType.LocalPullRequest) {
 			try {
 				this.prs = await this._prManager.getLocalPullRequests();
 			} catch (e) {
 				vscode.window.showErrorMessage(`Fetching local pull requests failed: ${formatError(e)}`);
+				needLogin = e instanceof AuthenticationError;
 			}
 		} else {
 			if (!this.fetchNextPage) {
@@ -109,6 +121,7 @@ export class CategoryTreeNode extends TreeNode implements vscode.TreeItem {
 					hasMorePages = ret[1];
 				} catch (e) {
 					vscode.window.showErrorMessage(`Fetching pull requests failed: ${formatError(e)}`);
+					needLogin = e instanceof AuthenticationError;
 				}
 			} else {
 				try {
@@ -117,6 +130,7 @@ export class CategoryTreeNode extends TreeNode implements vscode.TreeItem {
 					hasMorePages = ret[1];
 				} catch (e) {
 					vscode.window.showErrorMessage(`Fetching pull requests failed: ${formatError(e)}`);
+					needLogin = e instanceof AuthenticationError;
 				}
 
 				this.fetchNextPage = false;
@@ -132,7 +146,8 @@ export class CategoryTreeNode extends TreeNode implements vscode.TreeItem {
 			this.childrenDisposables = nodes;
 			return nodes;
 		} else {
-			let result = [new PRCategoryActionNode(PRCategoryActionType.Empty)];
+			let category = needLogin ? PRCategoryActionType.Login : PRCategoryActionType.Empty;
+			let result = [new PRCategoryActionNode(category)];
 
 			this.childrenDisposables = result;
 			return result;