From a6c44e0e6930449b450afd74b1f08d6bd5d319b5 Mon Sep 17 00:00:00 2001 From: Tanuj Mittal Date: Fri, 29 May 2020 19:16:50 -0700 Subject: [PATCH] Add support for changing GitHub OAuth permissions (#4) --- .../Controls/GitHub/GitHubReposComponent.tsx | 7 +++++-- src/GitHub/GitHubOAuthService.test.ts | 12 ++++++------ src/GitHub/GitHubOAuthService.ts | 19 +++++++++++++++++-- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/Explorer/Controls/GitHub/GitHubReposComponent.tsx b/src/Explorer/Controls/GitHub/GitHubReposComponent.tsx index 4373bf13e..c3e8ff6d0 100644 --- a/src/Explorer/Controls/GitHub/GitHubReposComponent.tsx +++ b/src/Explorer/Controls/GitHub/GitHubReposComponent.tsx @@ -1,9 +1,9 @@ -import { DefaultButton, IButtonProps, PrimaryButton } from "office-ui-fabric-react"; +import { DefaultButton, IButtonProps, Link, PrimaryButton } from "office-ui-fabric-react"; import * as React from "react"; import { IGitHubBranch, IGitHubRepo } from "../../../GitHub/GitHubClient"; import { AddRepoComponent, AddRepoComponentProps } from "./AddRepoComponent"; import { AuthorizeAccessComponent, AuthorizeAccessComponentProps } from "./AuthorizeAccessComponent"; -import { ChildrenMargin, ButtonsFooterStyle, ContentFooterStyle } from "./GitHubStyleConstants"; +import { ButtonsFooterStyle, ChildrenMargin, ContentFooterStyle } from "./GitHubStyleConstants"; import { ReposListComponent, ReposListComponentProps } from "./ReposListComponent"; export interface GitHubReposComponentProps { @@ -45,6 +45,9 @@ export class GitHubReposComponent extends React.Component

{GitHubReposComponent.ManageGitHubRepoDescription}

+ + {GitHubReposComponent.ManageGitHubRepoResetConnection} + ); diff --git a/src/GitHub/GitHubOAuthService.test.ts b/src/GitHub/GitHubOAuthService.test.ts index 58c2fdba1..9ff5cca4b 100644 --- a/src/GitHub/GitHubOAuthService.test.ts +++ b/src/GitHub/GitHubOAuthService.test.ts @@ -60,20 +60,20 @@ describe("GitHubOAuthService", () => { expect(gitHubOAuthService.getTokenObservable()()).toBeUndefined(); }); - it("startOAuth resets OAuth state", () => { + it("startOAuth resets OAuth state", async () => { let url: string; const windowOpenCallback = jest.fn().mockImplementation((value: string) => { url = value; }); window.open = windowOpenCallback; - gitHubOAuthService.startOAuth("scope"); + await gitHubOAuthService.startOAuth("scope"); expect(windowOpenCallback).toBeCalled(); const initialParams = new URLSearchParams(new URL(url).search); expect(initialParams.get("state")).toBeDefined(); - gitHubOAuthService.startOAuth("another scope"); + await gitHubOAuthService.startOAuth("another scope"); expect(windowOpenCallback).toBeCalled(); const newParams = new URLSearchParams(new URL(url).search); @@ -106,7 +106,7 @@ describe("GitHubOAuthService", () => { junoClient.getGitHubToken = getGitHubTokenCallback; const initialToken = gitHubOAuthService.getTokenObservable()(); - const state = gitHubOAuthService.startOAuth("scope"); + const state = await gitHubOAuthService.startOAuth("scope"); const params: IGitHubConnectorParams = { state, @@ -120,7 +120,7 @@ describe("GitHubOAuthService", () => { }); it("finishOAuth updates token to error if state doesn't match", async () => { - gitHubOAuthService.startOAuth("scope"); + await gitHubOAuthService.startOAuth("scope"); const params: IGitHubConnectorParams = { state: "state", @@ -135,7 +135,7 @@ describe("GitHubOAuthService", () => { const getGitHubTokenCallback = jest.fn().mockReturnValue({ status: HttpStatusCodes.NotFound }); junoClient.getGitHubToken = getGitHubTokenCallback; - const state = gitHubOAuthService.startOAuth("scope"); + const state = await gitHubOAuthService.startOAuth("scope"); const params: IGitHubConnectorParams = { state, diff --git a/src/GitHub/GitHubOAuthService.ts b/src/GitHub/GitHubOAuthService.ts index d085688cf..b151ebd3d 100644 --- a/src/GitHub/GitHubOAuthService.ts +++ b/src/GitHub/GitHubOAuthService.ts @@ -2,6 +2,7 @@ import ko from "knockout"; import { HttpStatusCodes } from "../Common/Constants"; import { Logger } from "../Common/Logger"; import { config } from "../Config"; +import { AuthorizeAccessComponent } from "../Explorer/Controls/GitHub/AuthorizeAccessComponent"; import { ConsoleDataType } from "../Explorer/Menus/NotificationConsole/NotificationConsoleComponent"; import { JunoClient } from "../Juno/JunoClient"; import { isInvalidParentFrameOrigin } from "../Utils/MessageValidation"; @@ -39,7 +40,19 @@ export class GitHubOAuthService { this.token = ko.observable(); } - public startOAuth(scope: string): string { + public async startOAuth(scope: string): Promise { + // If attempting to change scope from "Public & private repos" to "Public only" we need to delete app authorization. + // Otherwise OAuth app still retains the "public & private repos" permissions. + if ( + this.token()?.scope === AuthorizeAccessComponent.Scopes.PublicAndPrivate.key && + scope === AuthorizeAccessComponent.Scopes.Public.key + ) { + const logoutSuccessful = await this.logout(); + if (!logoutSuccessful) { + return undefined; + } + } + const params = { scope, client_id: config.GITHUB_CLIENT_ID, @@ -76,7 +89,7 @@ export class GitHubOAuthService { return this.token; } - public async logout() { + public async logout(): Promise { try { const response = await this.junoClient.deleteAppAuthorization(this.token()?.access_token); if (response.status !== HttpStatusCodes.NoContent) { @@ -84,10 +97,12 @@ export class GitHubOAuthService { } this.resetToken(); + return true; } catch (error) { const message = `Failed to delete app authorization: ${error}`; Logger.logError(message, "GitHubOAuthService/logout"); NotificationConsoleUtils.logConsoleMessage(ConsoleDataType.Error, message); + return false; } }