nodejs/hookshot
nodejs/hookshot
1
0
Fork 0
mirror of https://github.com/matrix-org/matrix-hookshot.git synced 2025-03-10 13:17:08 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix case where GitHub repos aren't connectable if the GitHub App was manually approved (#718)

Browse Source 
* Fix GitHub connections failing if the installation cache is stale

* changelog
This commit is contained in:
Will Hunt 2023-04-21 10:27:57 +01:00 committed by GitHub
parent 9baa21bb30
commit c6a04c2ebd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 25 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -0,0 +1 @@
Fix cases of GitHub repos not being bridgable if the GitHub App had to be manually approved.

28
src/Connections/GithubRepo.ts
View File

@ -383,7 +383,7 @@ export class GitHubRepoConnection extends CommandConnection<GitHubRepoConnection
throw new ValidatorApiError(validator.errors);
}
static async assertUserHasAccessToRepo(userId: string, org: string, repo: string, github: GithubInstance, tokenStore: UserTokenStore) {
static async assertUserHasAccessToRepo(userId: string, org: string, repo: string, tokenStore: UserTokenStore) {
const octokit = await tokenStore.getOctokitForUser(userId);
if (!octokit) {
throw new ApiError("User is not authenticated with GitHub", ErrCode.ForbiddenUser);
@ -407,9 +407,25 @@ export class GitHubRepoConnection extends CommandConnection<GitHubRepoConnection
throw Error('GitHub is not configured');
}
const validData = this.validateState(data);
await this.assertUserHasAccessToRepo(userId, validData.org, validData.repo, github, tokenStore);
const appOctokit = await github.getSafeOctokitForRepo(validData.org, validData.repo);
if (!appOctokit) {
await this.assertUserHasAccessToRepo(userId, validData.org, validData.repo, tokenStore);
const userOctokit = await tokenStore.getOctokitForUser(userId);
if (!userOctokit) {
// Given we assert the above, this is unlikely.
throw new ApiError("User is not authenticated with GitHub", ErrCode.ForbiddenUser);
}
const ownSelf = await userOctokit.users.getAuthenticated();
let installationId = 0;
if (ownSelf.data.login === validData.org) {
installationId = (await github.appOctokit.apps.getUserInstallation({ username: ownSelf.data.login })).data.id;
} else {
// Github will error if the authed user tries to list repos of a disallowed installation, even
// if we got the installation ID from the app's instance.
installationId = (await github.appOctokit.apps.getOrgInstallation({ org: validData.org })).data.id;
}
if (!installationId) {
throw new ApiError(
"You need to add a GitHub App to this organisation / repository before you can bridge it. Open the link to add the app, and then retry this request",
ErrCode.AdditionalActionRequired,
@ -421,7 +437,7 @@ export class GitHubRepoConnection extends CommandConnection<GitHubRepoConnection
);
}
const stateEventKey = `${validData.org}/${validData.repo}`;
await new GitHubGrantChecker(as, github, tokenStore).grantConnection(roomId, { org: validData.org, repo: validData.repo });
await new GitHubGrantChecker(as, tokenStore).grantConnection(roomId, { org: validData.org, repo: validData.repo });
await intent.underlyingClient.sendStateEvent(roomId, this.CanonicalEventType, stateEventKey, validData);
return {
stateEventContent: validData,
@ -533,7 +549,7 @@ export class GitHubRepoConnection extends CommandConnection<GitHubRepoConnection
public debounceOnIssueLabeled = new Map<number, {labels: Set<string>, timeout: NodeJS.Timeout}>();
private readonly grantChecker = new GitHubGrantChecker(this.as, this.githubInstance, this.tokenStore);
private readonly grantChecker = new GitHubGrantChecker(this.as, this.tokenStore);
constructor(
roomId: string,

5
src/Github/GrantChecker.ts
View File

@ -2,7 +2,6 @@ import { Appservice } from "matrix-bot-sdk";
import { GitHubRepoConnection } from "../Connections";
import { GrantChecker } from "../grants/GrantCheck";
import { UserTokenStore } from "../UserTokenStore";
import { GithubInstance } from "./GithubInstance";
import { Logger } from 'matrix-appservice-bridge';
const log = new Logger('GitHubGrantChecker');
@ -14,7 +13,7 @@ interface GitHubGrantConnectionId {
export class GitHubGrantChecker extends GrantChecker<GitHubGrantConnectionId> {
constructor(private readonly as: Appservice, private readonly github: GithubInstance, private readonly tokenStore: UserTokenStore) {
constructor(private readonly as: Appservice, private readonly tokenStore: UserTokenStore) {
super(as.botIntent, "github")
}
@ -29,7 +28,7 @@ export class GitHubGrantChecker extends GrantChecker<GitHubGrantConnectionId> {
return true;
}
try {
await GitHubRepoConnection.assertUserHasAccessToRepo(sender, connectionId.org, connectionId.repo, this.github, this.tokenStore);
await GitHubRepoConnection.assertUserHasAccessToRepo(sender, connectionId.org, connectionId.repo, this.tokenStore);
return true;
} catch (ex) {
log.info(`Tried to check fallback for ${roomId}: ${sender} does not have access to ${connectionId.org}/${connectionId.repo}`, ex);