Fix oauth login urls for GitHub cloud having the wrong domain (#377)

* Ensure we use a single function for generating oauth urls

* Add tests

* changelog

* Fix test
This commit is contained in:
Will Hunt 2022-06-27 15:17:01 +01:00 committed by GitHub
parent 5e63508f49
commit c36a5e7d60
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 96 additions and 22 deletions

1
changelog.d/377.bugfix Normal file
View File

@ -0,0 +1 @@
GitHub OAuth URLs for Cloud now use the correct endpoint.

View File

@ -7,18 +7,6 @@ import { GitHubOAuthToken } from "./Types";
import LogWrapper from "../LogWrapper";
const log = new LogWrapper('GitHubBotCommands');
export function generateGitHubOAuthUrl(clientId: string, redirectUri: string, baseUrl: URL, state: string) {
const q = qs.stringify({
client_id: clientId,
redirect_uri: redirectUri,
state: state,
});
const url = `${new URL("/login/oauth/authorize", baseUrl)}?${q}`;
return url;
}
export class GitHubBotCommands extends AdminRoomCommandHandler {
@botCommand("github login", {help: "Log in to GitHub", category: Category.Github})
public async loginCommand() {
@ -29,7 +17,16 @@ export class GitHubBotCommands extends AdminRoomCommandHandler {
throw new CommandError("no-github-support", "The bridge is not configured with GitHub OAuth support.");
}
const state = this.tokenStore.createStateForOAuth(this.userId);
return this.sendNotice(`Open ${generateGitHubOAuthUrl(this.config.github.oauth.client_id, this.config.github.oauth.redirect_uri, this.config.github.baseUrl, state)} to link your account to the bridge.`);
const url = GithubInstance.generateOAuthUrl(
this.config.github.baseUrl,
"authorize",
{
state,
client_id: this.config.github.oauth.client_id,
redirect_uri: this.config.github.oauth.redirect_uri,
}
);
return this.sendNotice(`Open ${url} to link your account to the bridge.`);
}
@botCommand("github setpersonaltoken", {help: "Set your personal access token for GitHub", requiredArgs: ['accessToken'], category: Category.Github})

View File

@ -21,6 +21,16 @@ interface Installation {
matchesRepository: string[];
}
interface OAuthUrlParameters {
[key: string]: string|undefined;
state?: string;
client_id?: string;
redirect_uri?: string;
client_secret?: string,
refresh_token?: string,
grant_type?: 'refresh_token',
}
export class GithubInstance {
private internalOctokit!: Octokit;
private readonly installationsCache = new Map<number, Installation>();
@ -61,13 +71,13 @@ export class GithubInstance {
}
public static async refreshAccessToken(refreshToken: string, clientId: string, clientSecret: string, baseUrl: URL): Promise<GitHubOAuthTokenResponse> {
const url = new URL("/login/oauth/access_token", baseUrl);
const accessTokenRes = await axios.post(`${url}?${qs.encode({
const url = GithubInstance.generateOAuthUrl(baseUrl, "access_token", {
client_id: clientId,
client_secret: clientSecret,
refresh_token: refreshToken,
grant_type: 'refresh_token',
})}`);
});
const accessTokenRes = await axios.post(`${url}?${qs.encode()}`);
return qs.decode(accessTokenRes.data) as unknown as GitHubOAuthTokenResponse;
}
@ -168,6 +178,16 @@ export class GithubInstance {
// Enterprise (yes, i know right)
return new URL(`/github-apps/${this.appSlug}/installations/new`, this.baseUrl);
}
public static generateOAuthUrl(baseUrl: URL, action: "authorize"|"access_token", params: OAuthUrlParameters) {
const q = qs.stringify(params);
if (baseUrl.hostname === GITHUB_CLOUD_URL.hostname) {
// Cloud doesn't use `api.` for oauth.
baseUrl = new URL("https://github.com");
}
const rawUrl = baseUrl.toString();
return rawUrl + `${rawUrl.endsWith('/') ? '' : '/'}` + `login/oauth/${action}?${q}`;
}
}
export class GithubGraphQLClient {

View File

@ -2,7 +2,6 @@ import { Router, Request, Response, NextFunction } from "express";
import { BridgeConfigGitHub } from "../Config/Config";
import { ApiError, ErrCode } from "../api";
import { UserTokenStore } from "../UserTokenStore";
import { generateGitHubOAuthUrl } from "./AdminCommands";
import LogWrapper from "../LogWrapper";
import { GithubInstance } from "./GithubInstance";
@ -45,8 +44,17 @@ export class GitHubProvisionerRouter {
if (!this.config.oauth) {
throw new ApiError("GitHub is not configured to support OAuth", ErrCode.UnsupportedOperation);
}
const userUrl = GithubInstance.generateOAuthUrl(
this.config.baseUrl,
"authorize",
{
redirect_uri: this.config.oauth.redirect_uri,
client_id: this.config.oauth.client_id,
state: this.tokenStore.createStateForOAuth(req.query.userId)
}
);
res.send({
user_url: generateGitHubOAuthUrl(this.config.oauth.client_id, this.config.oauth.redirect_uri, this.config.baseUrl, this.tokenStore.createStateForOAuth(req.query.userId)),
user_url: userUrl,
org_url: this.githubInstance.newInstallationUrl.toString(),
});
}

View File

@ -14,6 +14,7 @@ import { GitHubOAuthTokenResponse } from "./Github/Types";
import Metrics from "./Metrics";
import { FigmaWebhooksRouter } from "./figma/router";
import { GenericWebhooksRouter } from "./generic/Router";
import { GithubInstance } from "./Github/GithubInstance";
const log = new LogWrapper("Webhooks");
@ -191,15 +192,14 @@ export class Webhooks extends EventEmitter {
if (!exists) {
return res.status(404).send(`<p>Could not find user which authorised this request. Has it timed out?</p>`);
}
const accessTokenUrl = new URL("/login/oauth/access_token", this.config.github.baseUrl);
const accessTokenRes = await axios.post(`${accessTokenUrl}?${qs.encode({
const accessTokenUrl = GithubInstance.generateOAuthUrl(this.config.github.baseUrl, "access_token", {
client_id: this.config.github.oauth.client_id,
client_secret: this.config.github.oauth.client_secret,
code: req.query.code as string,
redirect_uri: this.config.github.oauth.redirect_uri,
state: req.query.state as string,
})}`);
// eslint-disable-next-line camelcase
});
const accessTokenRes = await axios.post(accessTokenUrl);
const result = qs.parse(accessTokenRes.data) as GitHubOAuthTokenResponse|{error: string, error_description: string, error_uri: string};
if ("error" in result) {
return res.status(500).send(`<p><b>GitHub Error</b>: ${result.error} ${result.error_description}</p>`);

View File

@ -0,0 +1,48 @@
import { expect } from "chai";
import { GithubInstance } from "../../src/Github/GithubInstance";
import { GITHUB_CLOUD_URL } from "../../src/Github/GithubInstance";
describe("GitHub", () => {
describe("AdminCommands", () => {
it("can generate an authorize URL for the cloud URL", () => {
expect(
GithubInstance.generateOAuthUrl(GITHUB_CLOUD_URL, "authorize", {
state: "my_state",
client_id: "123",
redirect_uri: "456",
})
).equals('https://github.com/login/oauth/authorize?state=my_state&client_id=123&redirect_uri=456');
});
it("can generate an authorize URL for enterprise URLs", () => {
expect(
GithubInstance.generateOAuthUrl(new URL("https://mygithuburl.com/foo/bar"), "authorize", {
state: "my_state",
client_id: "123",
redirect_uri: "456",
})
).equals('https://mygithuburl.com/foo/bar/login/oauth/authorize?state=my_state&client_id=123&redirect_uri=456');
});
it("can generate an access_token URL for the cloud URL", () => {
expect(
GithubInstance.generateOAuthUrl(GITHUB_CLOUD_URL, "access_token", {
client_id: "123",
client_secret: "the-secret",
code: "the-code",
redirect_uri: "456",
state: "my_state",
})
).equals('https://github.com/login/oauth/access_token?client_id=123&client_secret=the-secret&code=the-code&redirect_uri=456&state=my_state');
});
it("can generate an access_token URL for enterprise URLs", () => {
expect(
GithubInstance.generateOAuthUrl(new URL("https://mygithuburl.com/foo/bar"), "access_token", {
client_id: "123",
client_secret: "the-secret",
code: "the-code",
redirect_uri: "456",
state: "my_state",
})
).equals('https://mygithuburl.com/foo/bar/login/oauth/access_token?client_id=123&client_secret=the-secret&code=the-code&redirect_uri=456&state=my_state');
});
});
});