Refactor hookshot to include permission grant system (#655)

* Add support for checking connection grants

* Make this less noisy

* Remove import

* Trying to generic-ize the grant system

* Implement for Figma

* More grant reformatting

* Add tests

* changelog

* Fix todo

* Refactor grants

* Add logging to grant checkers

* Ensure we provide a sender

changelog.d/655.feature

@@ -0,0 +1 @@
+Implement grant system to internally record all approved connections in hookshot.

src/Bridge.ts

@@ -1,6 +1,6 @@
 import { AdminAccountData } from "./AdminRoomCommandHandler";
 import { AdminRoom, BRIDGE_ROOM_TYPE, LEGACY_BRIDGE_ROOM_TYPE } from "./AdminRoom";
-import { Appservice, RichRepliesPreprocessor, IRichReplyMetadata, StateEvent, EventKind, PowerLevelsEvent, IAppserviceRegistration, Intent } from "matrix-bot-sdk";
+import { Appservice, RichRepliesPreprocessor, IRichReplyMetadata, StateEvent, EventKind, PowerLevelsEvent, Intent } from "matrix-bot-sdk";
 import BotUsersManager from "./Managers/BotUsersManager";
 import { BridgeConfig, BridgePermissionLevel, GitLabInstance } from "./Config/Config";
 import { BridgeWidgetApi } from "./Widgets/BridgeWidgetApi";
@@ -494,7 +494,7 @@ export class Bridge {
                         this.tokenStore,
                         this.commentProcessor,
                         this.messageClient,
-                        this.config.github,
+                        this.config,
                     );
                     connManager.push(discussionConnection);
                 } catch (ex) {
@@ -1294,7 +1294,7 @@ export class Bridge {
         adminRoom.on("open.project", async (project: ProjectsGetResponseData) => {
             const [connection] = this.connectionManager?.getForGitHubProject(project.id) || [];
             if (!connection) {
-                const connection = await GitHubProjectConnection.onOpenProject(project, this.as, intent, adminRoom.userId);
+                const connection = await GitHubProjectConnection.onOpenProject(project, this.as, intent, this.config, adminRoom.userId);
                 this.connectionManager?.push(connection);
             } else {
                 await intent.underlyingClient.inviteUser(adminRoom.userId, connection.roomId);
@@ -1318,7 +1318,7 @@ export class Bridge {
                 this.tokenStore,
                 this.commentProcessor,
                 this.messageClient,
-                this.config.gitlab,
+                this.config,
             );
             this.connectionManager?.push(newConnection);
             return intent.underlyingClient.inviteUser(adminRoom.userId, newConnection.roomId);

src/Config/Config.ts

@@ -515,15 +515,15 @@ export class BridgeConfig {
     @hideKey()
     private readonly bridgePermissions: BridgePermissions;
 
-    constructor(configData: BridgeConfigRoot, env: {[key: string]: string|undefined}) {
+    constructor(configData: BridgeConfigRoot, env?: {[key: string]: string|undefined}) {
         this.bridge = configData.bridge;
         assert.ok(this.bridge);
         this.github = configData.github && new BridgeConfigGitHub(configData.github);
-        if (this.github?.auth && env["GITHUB_PRIVATE_KEY_FILE"]) {
-            this.github.auth.privateKeyFile = env["GITHUB_PRIVATE_KEY_FILE"];
+        if (this.github?.auth && env?.["GITHUB_PRIVATE_KEY_FILE"]) {
+            this.github.auth.privateKeyFile = env?.["GITHUB_PRIVATE_KEY_FILE"];
         }
-        if (this.github?.oauth && env["GITHUB_OAUTH_REDIRECT_URI"]) {
-            this.github.oauth.redirect_uri = env["GITHUB_OAUTH_REDIRECT_URI"];
+        if (this.github?.oauth && env?.["GITHUB_OAUTH_REDIRECT_URI"]) {
+            this.github.oauth.redirect_uri = env?.["GITHUB_OAUTH_REDIRECT_URI"];
         }
         this.gitlab = configData.gitlab && new BridgeConfigGitLab(configData.gitlab);
         this.figma = configData.figma;
@@ -577,10 +577,10 @@ For more details, see https://github.com/matrix-org/matrix-hookshot/issues/594.
         }
 
         // TODO: Formalize env support
-        if (env.CFG_QUEUE_MONOLITHIC && ["false", "off", "no"].includes(env.CFG_QUEUE_MONOLITHIC)) {
+        if (env?.CFG_QUEUE_MONOLITHIC && ["false", "off", "no"].includes(env.CFG_QUEUE_MONOLITHIC)) {
             this.queue.monolithic = false;
-            this.queue.host = env.CFG_QUEUE_HOST;
-            this.queue.port = env.CFG_QUEUE_POST ? parseInt(env.CFG_QUEUE_POST, 10) : undefined;
+            this.queue.host = env?.CFG_QUEUE_HOST;
+            this.queue.port = env?.CFG_QUEUE_POST ? parseInt(env?.CFG_QUEUE_POST, 10) : undefined;
         }
 
         this.goNebMigrator = configData.goNebMigrator;

src/Config/Defaults.ts

@@ -1,4 +1,4 @@
-import { BridgeConfig } from "./Config";
+import { BridgeConfig, BridgeConfigRoot } from "./Config";
 import YAML from "yaml";
 import { getConfigKeyMetadata, keyIsHidden } from "./Decorators";
 import { Node, YAMLSeq } from "yaml/types";
@@ -8,7 +8,7 @@ import { DefaultDisallowedIpRanges } from "matrix-appservice-bridge";
 const serverName = "example.com";
 const hookshotWebhooksUrl = "https://example.com";
 
-export const DefaultConfig = new BridgeConfig({
+export const DefaultConfigRoot: BridgeConfigRoot = {
     bridge: {
         domain: serverName,
         url: "http://localhost:8008",
@@ -147,7 +147,9 @@ export const DefaultConfig = new BridgeConfig({
             resources: ['widgets'],
         }
     ]
-}, {});
+};
+
+export const DefaultConfig = new BridgeConfig(DefaultConfigRoot);
 
 function renderSection(doc: YAML.Document, obj: Record<string, unknown>, parentNode?: YAMLSeq) {
     const entries = Object.entries(obj);

src/ConnectionManager.ts

@@ -166,7 +166,14 @@ export class ConnectionManager extends EventEmitter {
         }
     }
 
-    public createConnectionForState(roomId: string, state: StateEvent<any>, rollbackBadState: boolean) {
+    /**
+     * This is called ONLY when we spot new state in a room and want to create a connection for it.
+     * @param roomId 
+     * @param state 
+     * @param rollbackBadState 
+     * @returns 
+     */
+    public async createConnectionForState(roomId: string, state: StateEvent<any>, rollbackBadState: boolean) {
         // Empty object == redacted
         if (state.content.disabled === true || Object.keys(state.content).length === 0) {
             log.debug(`${roomId} has disabled state for ${state.type}`);
@@ -188,7 +195,7 @@ export class ConnectionManager extends EventEmitter {
             return;
         }
 
-        return connectionType.createConnectionForState(roomId, state, {
+        const connection = await connectionType.createConnectionForState(roomId, state, {
             as: this.as,
             intent: botUser.intent,
             config: this.config,
@@ -198,8 +205,19 @@ export class ConnectionManager extends EventEmitter {
             storage: this.storage,
             github: this.github,
         });
+
+        // Finally, ensure the connection is allowed by us.
+        await connection.ensureGrant?.(state.sender);
+        return connection;
     }
 
+    /**
+     * This is called when hookshot starts up, or a hookshot service bot has left
+     * and we need to recalculate the right bots for all the connections in a room.
+     * @param roomId 
+     * @param rollbackBadState 
+     * @returns 
+     */
     public async createConnectionsForRoomId(roomId: string, rollbackBadState: boolean) {
         const botUser = this.botUsersManager.getBotUserInRoom(roomId);
         if (!botUser) {

src/Connections/CommandConnection.ts

@@ -36,10 +36,10 @@ export abstract class CommandConnection<StateType extends IConnectionState = ICo
     }
 
     public async onStateUpdate(stateEv: MatrixEvent<unknown>) {
-        this.state = this.validateConnectionState(stateEv.content);
+        this.state = await this.validateConnectionState(stateEv.content);
     }
 
-    protected abstract validateConnectionState(content: unknown): ValidatedStateType;
+    protected abstract validateConnectionState(content: unknown): Promise<ValidatedStateType>|ValidatedStateType;
 
     public async onMessageEvent(ev: MatrixEvent<MatrixMessageContent>, checkPermission: PermissionCheckFn) {
         const commandResult = await handleCommand(

src/Connections/FigmaFileConnection.ts

@@ -5,8 +5,9 @@ import { BaseConnection } from "./BaseConnection";
 import { IConnection, IConnectionState } from ".";
 import { Logger } from "matrix-appservice-bridge";
 import { IBridgeStorageProvider } from "../Stores/StorageProvider";
-import { BridgeConfigFigma } from "../Config/Config";
+import { BridgeConfig } from "../Config/Config";
 import { Connection, InstantiateConnectionOpts, ProvisionConnectionOpts } from "./IConnection";
+import { ConfigGrantChecker, GrantChecker } from "../grants/GrantCheck";
 
 const log = new Logger("FigmaFileConnection");
 
@@ -47,7 +48,7 @@ export class FigmaFileConnection extends BaseConnection implements IConnection {
         if (!config.figma) {
             throw Error('Figma is not configured');
         }
-        return new FigmaFileConnection(roomId, event.stateKey, event.content, config.figma, as, intent, storage);
+        return new FigmaFileConnection(roomId, event.stateKey, event.content, config, as, intent, storage);
     }
 
     static async provisionConnection(roomId: string, userId: string, data: Record<string, unknown> = {}, {as, intent, config, storage}: ProvisionConnectionOpts) {
@@ -55,7 +56,8 @@ export class FigmaFileConnection extends BaseConnection implements IConnection {
             throw Error('Figma is not configured');
         }
         const validState = this.validateState(data);
-        const connection = new FigmaFileConnection(roomId, validState.fileId, validState, config.figma, as, intent, storage);
+        const connection = new FigmaFileConnection(roomId, validState.fileId, validState, config, as, intent, storage);
+        await new GrantChecker(as.botIntent, "figma").grantConnection(roomId, { fileId: validState.fileId, instanceName: validState.instanceName  || "none"});
         await intent.underlyingClient.sendStateEvent(roomId, FigmaFileConnection.CanonicalEventType, validState.fileId, validState);
         return {
             connection,
@@ -63,12 +65,13 @@ export class FigmaFileConnection extends BaseConnection implements IConnection {
         }
     }
 
+    private readonly grantChecker: GrantChecker<{fileId: string, instanceName: string}> = new ConfigGrantChecker("figma", this.as, this.config);
 
     constructor(
         roomId: string,
         stateKey: string,
         private state: FigmaFileConnectionState,
-        private readonly config: BridgeConfigFigma,
+        private readonly config: BridgeConfig,
         private readonly as: Appservice,
         private readonly intent: Intent,
         private readonly storage: IBridgeStorageProvider) {
@@ -91,6 +94,14 @@ export class FigmaFileConnection extends BaseConnection implements IConnection {
         return this.state.priority || super.priority;
     }
 
+    public async ensureGrant(sender?: string) {
+        return this.grantChecker.assertConnectionGranted(this.roomId, { fileId: this.state.fileId, instanceName: this.state.instanceName || "none"}, sender);
+    }
+
+    public async onRemove() {
+        return this.grantChecker.ungrantConnection(this.roomId, { fileId: this.state.fileId, instanceName: this.state.instanceName || "none"});
+    }
+
     public async handleNewComment(payload: FigmaPayload) {
         // We need to check if the comment was actually new.
         // There isn't a way to tell how the comment has changed, so for now check the timestamps
@@ -102,8 +113,8 @@ export class FigmaFileConnection extends BaseConnection implements IConnection {
         }
 
         let intent;
-        if (this.config.overrideUserId) {
-            intent = this.as.getIntentForUserId(this.config.overrideUserId);
+        if (this.config.figma?.overrideUserId) {
+            intent = this.as.getIntentForUserId(this.config.figma.overrideUserId);
         } else {
             intent = this.intent;
         }

src/Connections/GithubDiscussion.ts

@@ -12,7 +12,9 @@ import { DiscussionCommentCreatedEvent } from "@octokit/webhooks-types";
 import { GithubGraphQLClient } from "../Github/GithubInstance";
 import { Logger } from "matrix-appservice-bridge";
 import { BaseConnection } from "./BaseConnection";
-import { BridgeConfigGitHub } from "../Config/Config";
+import { BridgeConfig, BridgeConfigGitHub } from "../Config/Config";
+import { ConfigGrantChecker, GrantChecker } from "../grants/GrantCheck";
+import QuickLRU from "@alloc/quick-lru";
 export interface GitHubDiscussionConnectionState {
     owner: string;
     repo: string;
@@ -48,21 +50,20 @@ export class GitHubDiscussionConnection extends BaseConnection implements IConne
         }
         return new GitHubDiscussionConnection(
             roomId, as, intent, event.content, event.stateKey, tokenStore, commentProcessor,
-            messageClient, config.github,
+            messageClient, config,
         );
     }
 
-    readonly sentEvents = new Set<string>(); //TODO: Set some reasonable limits
 
     public static async createDiscussionRoom(
         as: Appservice, intent: Intent, userId: string|null, owner: string, repo: string, discussion: Discussion,
         tokenStore: UserTokenStore, commentProcessor: CommentProcessor, messageClient: MessageSenderClient,
-        config: BridgeConfigGitHub,
+        config: BridgeConfig,
     ) {
         const commentIntent = await getIntentForUser({
             login: discussion.user.login,
             avatarUrl: discussion.user.avatar_url,
-        }, as, config.userIdPrefix);
+        }, as, config.github?.userIdPrefix);
         const state: GitHubDiscussionConnectionState = {
             owner,
             repo,
@@ -94,9 +95,19 @@ export class GitHubDiscussionConnection extends BaseConnection implements IConne
             format: 'org.matrix.custom.html',
         });
         await intent.ensureJoined(roomId);
+
         return new GitHubDiscussionConnection(roomId, as, intent, state, '', tokenStore, commentProcessor, messageClient, config);
     }
 
+    private static grantKey(state: GitHubDiscussionConnectionState) {
+        return `${this.CanonicalEventType}/${state.owner}/${state.repo}`;
+    }
+
+    private readonly sentEvents = new QuickLRU<string, undefined>({ maxSize: 128 });
+    private readonly grantChecker: GrantChecker;
+
+    private readonly config: BridgeConfigGitHub;
+
     constructor(
         roomId: string,
         private readonly as: Appservice,
@@ -106,9 +117,14 @@ export class GitHubDiscussionConnection extends BaseConnection implements IConne
         private readonly tokenStore: UserTokenStore,
         private readonly commentProcessor: CommentProcessor,
         private readonly messageClient: MessageSenderClient,
-        private readonly config: BridgeConfigGitHub,
+        bridgeConfig: BridgeConfig,
     ) {
         super(roomId, stateKey, GitHubDiscussionConnection.CanonicalEventType);
+        if (!bridgeConfig.github) {
+            throw Error('Expected github to be enabled in config');
+        }
+        this.config = bridgeConfig.github;
+        this.grantChecker = new ConfigGrantChecker("github", this.as, bridgeConfig);
     }
 
     public isInterestedInStateEvent(eventType: string, stateKey: string) {
@@ -125,7 +141,7 @@ export class GitHubDiscussionConnection extends BaseConnection implements IConne
         const qlClient = new GithubGraphQLClient(octokit);
         const commentId = await qlClient.addDiscussionComment(this.state.internalId, ev.content.body);
         log.info(`Sent ${commentId} for ${ev.event_id} (${ev.sender})`);
-        this.sentEvents.add(commentId);
+        this.sentEvents.set(commentId, undefined);
         return true;
     }
 
@@ -162,6 +178,7 @@ export class GitHubDiscussionConnection extends BaseConnection implements IConne
 
     public async onRemove() {
         log.info(`Removing ${this.toString()} for ${this.roomId}`);
+        await this.grantChecker.ungrantConnection(this.roomId, GitHubDiscussionConnection.grantKey(this.state));
         // Do a sanity check that the event exists.
         try {
             await this.intent.underlyingClient.getRoomStateEvent(this.roomId, GitHubDiscussionConnection.CanonicalEventType, this.stateKey);
@@ -171,4 +188,8 @@ export class GitHubDiscussionConnection extends BaseConnection implements IConne
             await this.intent.underlyingClient.sendStateEvent(this.roomId, GitHubDiscussionConnection.LegacyCanonicalEventType, this.stateKey, { disabled: true });
         }
     }
+
+    public async ensureGrant(sender?: string) {
+        await this.grantChecker.assertConnectionGranted(this.roomId, GitHubDiscussionConnection.grantKey(this.state), sender);
+    }
 }

src/Connections/GithubDiscussionSpace.ts

@@ -6,6 +6,8 @@ import axios from "axios";
 import { GitHubDiscussionConnection } from "./GithubDiscussion";
 import { GithubInstance } from "../Github/GithubInstance";
 import { BaseConnection } from "./BaseConnection";
+import { ConfigGrantChecker, GrantChecker } from "../grants/GrantCheck";
+import { BridgeConfig } from "../Config/Config";
 
 const log = new Logger("GitHubDiscussionSpace");
 
@@ -35,12 +37,13 @@ export class GitHubDiscussionSpace extends BaseConnection implements IConnection
         if (!github || !config.github) {
             throw Error('GitHub is not configured');
         }
+        await new GrantChecker(as.botIntent, 'github').grantConnection(roomId, this.grantKey(event.content));
         return new GitHubDiscussionSpace(
-            await intent.underlyingClient.getSpace(roomId), event.content, event.stateKey
+            as, config, await intent.underlyingClient.getSpace(roomId), event.content, event.stateKey
         );
     }
 
-    static async onQueryRoom(result: RegExpExecArray, opts: {githubInstance: GithubInstance, as: Appservice}): Promise<Record<string, unknown>> {
+    public static async onQueryRoom(result: RegExpExecArray, opts: {githubInstance: GithubInstance, as: Appservice}): Promise<Record<string, unknown>> {
         if (!result || result.length < 2) {
             log.error(`Invalid alias pattern '${result}'`);
             throw Error("Could not find issue");
@@ -141,10 +144,19 @@ export class GitHubDiscussionSpace extends BaseConnection implements IConnection
         };
     }
 
-    constructor(public readonly space: Space,
+    private static grantKey(state: GitHubDiscussionSpaceConnectionState) {
+        return `${this.CanonicalEventType}/${state.owner}/${state.repo}`;
+    }
+
+    private readonly grantChecker: GrantChecker;
+
+    constructor(as: Appservice,
+        config: BridgeConfig,
+        public readonly space: Space,
         private state: GitHubDiscussionSpaceConnectionState,
         stateKey: string) {
             super(space.roomId, stateKey, GitHubDiscussionSpace.CanonicalEventType)
+            this.grantChecker = new ConfigGrantChecker("github", as, config);
         }
 
     public isInterestedInStateEvent(eventType: string, stateKey: string) {
@@ -167,9 +179,15 @@ export class GitHubDiscussionSpace extends BaseConnection implements IConnection
         log.info(`Adding connection to ${this.toString()}`);
         await this.space.addChildRoom(discussion.roomId);
     }
+    
+
+    public async ensureGrant(sender?: string) {
+        await this.grantChecker.assertConnectionGranted(this.roomId, GitHubDiscussionSpace.grantKey(this.state), sender);
+    }
 
     public async onRemove() {
         log.info(`Removing ${this.toString()} for ${this.roomId}`);
+        this.grantChecker.ungrantConnection(this.roomId, GitHubDiscussionSpace.grantKey(this.state));
         // Do a sanity check that the event exists.
         try {
 

src/Connections/GithubProject.ts

@@ -3,6 +3,8 @@ import { Appservice, Intent, StateEvent } from "matrix-bot-sdk";
 import { Logger } from "matrix-appservice-bridge";
 import { ProjectsGetResponseData } from "../Github/Types";
 import { BaseConnection } from "./BaseConnection";
+import { ConfigGrantChecker, GrantChecker } from "../grants/GrantCheck";
+import { BridgeConfig } from "../Config/Config";
 
 export interface GitHubProjectConnectionState {
     // eslint-disable-next-line camelcase
@@ -28,10 +30,14 @@ export class GitHubProjectConnection extends BaseConnection implements IConnecti
         if (!config.github) {
             throw Error('GitHub is not configured');
         }
-        return new GitHubProjectConnection(roomId, as, intent, event.content, event.stateKey);
+        return new GitHubProjectConnection(roomId, as, intent, config, event.content, event.stateKey);
     }
 
-    static async onOpenProject(project: ProjectsGetResponseData, as: Appservice, intent: Intent, inviteUser: string): Promise<GitHubProjectConnection> {
+    public static getGrantKey(projectId: number) {
+        return `${this.CanonicalEventType}/${projectId}`;
+    }
+
+    static async onOpenProject(project: ProjectsGetResponseData, as: Appservice, intent: Intent, config: BridgeConfig, inviteUser: string): Promise<GitHubProjectConnection> {
         log.info(`Fetching ${project.name} ${project.id}`);
 
         // URL hack so we don't need to fetch the repo itself.
@@ -55,22 +61,31 @@ export class GitHubProjectConnection extends BaseConnection implements IConnecti
                 },
             ],
         });
+        await new GrantChecker(as.botIntent, 'github').grantConnection(roomId, this.getGrantKey(project.id));
 
-        return new GitHubProjectConnection(roomId, as, intent, state, project.url)
+        return new GitHubProjectConnection(roomId, as, intent, config, state, project.url)
     }
 
     get projectId() {
         return this.state.project_id;
     }
 
+    private readonly grantChecker: GrantChecker;
+
     constructor(
         public readonly roomId: string,
         as: Appservice,
         intent: Intent,
+        config: BridgeConfig,
         private state: GitHubProjectConnectionState,
         stateKey: string,
     ) {
         super(roomId, stateKey, GitHubProjectConnection.CanonicalEventType);
+        this.grantChecker = new ConfigGrantChecker("github", as, config);
+    }
+
+    public ensureGrant(sender?: string) {
+        return this.grantChecker.assertConnectionGranted(this.roomId, GitHubProjectConnection.getGrantKey(this.state.project_id), sender);
     }
 
     public isInterestedInStateEvent(eventType: string, stateKey: string) {

src/Connections/GithubRepo.ts

@@ -28,6 +28,8 @@ import { PermissionCheckFn } from ".";
 import { MinimalGitHubIssue, MinimalGitHubRepo } from "../libRs";
 import Ajv, { JSONSchemaType } from "ajv";
 import { HookFilter } from "../HookFilter";
+import { GrantChecker } from "../grants/GrantCheck";
+import { GitHubGrantChecker } from "../Github/GrantChecker";
 
 const log = new Logger("GitHubRepoConnection");
 const md = new markdown();
@@ -359,11 +361,7 @@ export class GitHubRepoConnection extends CommandConnection<GitHubRepoConnection
         throw new ValidatorApiError(validator.errors);
     }
 
-    static async provisionConnection(roomId: string, userId: string, data: Record<string, unknown>, {as, intent, tokenStore, github, config}: ProvisionConnectionOpts) {
-        if (!github || !config.github) {
-            throw Error('GitHub is not configured');
-        }
-        const validData = this.validateState(data);
+    static async assertUserHasAccessToRepo(userId: string, org: string, repo: string, github: GithubInstance, tokenStore: UserTokenStore) {
         const octokit = await tokenStore.getOctokitForUser(userId);
         if (!octokit) {
             throw new ApiError("User is not authenticated with GitHub", ErrCode.ForbiddenUser);
@@ -371,8 +369,8 @@ export class GitHubRepoConnection extends CommandConnection<GitHubRepoConnection
         const me = await octokit.users.getAuthenticated();
         let permissionLevel;
         try {
-            const repo = await octokit.repos.getCollaboratorPermissionLevel({owner: validData.org, repo: validData.repo, username: me.data.login });
-            permissionLevel = repo.data.permission;
+            const githubRepo = await octokit.repos.getCollaboratorPermissionLevel({owner: org, repo, username: me.data.login });
+            permissionLevel = githubRepo.data.permission;
         } catch (ex) {
             throw new ApiError("Could not determine if the user has access to this repository, does the repository exist?", ErrCode.ForbiddenUser);
         }
@@ -380,6 +378,14 @@ export class GitHubRepoConnection extends CommandConnection<GitHubRepoConnection
         if (permissionLevel !== "admin" && permissionLevel !== "write") {
             throw new ApiError("You must at least have write permissions to bridge this repository", ErrCode.ForbiddenUser);
         }
+    }
+
+    static async provisionConnection(roomId: string, userId: string, data: Record<string, unknown>, {as, intent, tokenStore, github, config}: ProvisionConnectionOpts) {
+        if (!github || !config.github) {
+            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) {
             throw new ApiError(
@@ -393,6 +399,7 @@ export class GitHubRepoConnection extends CommandConnection<GitHubRepoConnection
             );
         }
         const stateEventKey = `${validData.org}/${validData.repo}`;
+        await new GrantChecker(as.botIntent, 'github').grantConnection(roomId, this.getGrantKey(validData.org, validData.repo));
         await intent.underlyingClient.sendStateEvent(roomId, this.CanonicalEventType, stateEventKey, validData);
         return {
             stateEventContent: validData,
@@ -413,7 +420,10 @@ export class GitHubRepoConnection extends CommandConnection<GitHubRepoConnection
         if (!github || !config.github) {
             throw Error('GitHub is not configured');
         }
-        return new GitHubRepoConnection(roomId, as, intent, this.validateState(state.content, true), tokenStore, state.stateKey, github, config.github);
+
+        const connectionState = this.validateState(state.content, true);
+    
+        return new GitHubRepoConnection(roomId, as, intent, connectionState, tokenStore, state.stateKey, github, config.github);
     }
 
     static async onQueryRoom(result: RegExpExecArray, opts: IQueryRoomOpts): Promise<unknown> {
@@ -501,6 +511,8 @@ 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);
+
     constructor(
         roomId: string,
         private readonly as: Appservice,
@@ -557,8 +569,15 @@ export class GitHubRepoConnection extends CommandConnection<GitHubRepoConnection
         return this.state.priority || super.priority;
     }
 
-    protected validateConnectionState(content: unknown) {
-        return GitHubRepoConnection.validateState(content);
+    public async ensureGrant(sender?: string, state = this.state) {
+        await this.grantChecker.assertConnectionGranted(this.roomId, state, sender);
+    }
+
+    protected async validateConnectionState(content: unknown) {
+        const state = GitHubRepoConnection.validateState(content);
+        // Validate the permissions of this state
+        await this.ensureGrant(undefined, state);
+        return state;
     }
 
     public async onStateUpdate(stateEv: MatrixEvent<unknown>) {
@@ -1373,6 +1392,7 @@ export class GitHubRepoConnection extends CommandConnection<GitHubRepoConnection
 
     public async onRemove() {
         log.info(`Removing ${this.toString()} for ${this.roomId}`);
+        await this.grantChecker.ungrantConnection(this.roomId, { org: this.org, repo: this.repo });
         // Do a sanity check that the event exists.
         try {
             await this.intent.underlyingClient.getRoomStateEvent(this.roomId, GitHubRepoConnection.CanonicalEventType, this.stateKey);
@@ -1395,6 +1415,10 @@ export class GitHubRepoConnection extends CommandConnection<GitHubRepoConnection
         }
         return true;
     }
+
+    public static getGrantKey(org: string, repo: string) {
+        return `${this.CanonicalEventType}/${org}/${repo}`;
+    }
 }
 
 // eslint-disable-next-line @typescript-eslint/no-explicit-any

src/Connections/GithubUserSpace.ts

@@ -5,6 +5,8 @@ import axios from "axios";
 import { GitHubDiscussionSpace } from ".";
 import { GithubInstance } from "../Github/GithubInstance";
 import { BaseConnection } from "./BaseConnection";
+import { ConfigGrantChecker, GrantChecker } from "../grants/GrantCheck";
+import { BridgeConfig } from "../Config/Config";
 
 const log = new Logger("GitHubOwnerSpace");
 
@@ -28,13 +30,17 @@ export class GitHubUserSpace extends BaseConnection implements IConnection {
     static readonly QueryRoomRegex = /#github_(.+):.*/;
     static readonly ServiceCategory = "github";
 
+    private static grantKey(state: GitHubUserSpaceConnectionState) {
+        return `${this.CanonicalEventType}/${state.username}`;
+    }
+
     public static async createConnectionForState(roomId: string, event: StateEvent<any>, {
-        github, config, intent}: InstantiateConnectionOpts) {
+        github, config, intent, as}: InstantiateConnectionOpts) {
         if (!github || !config.github) {
             throw Error('GitHub is not configured');
         }
         return new GitHubUserSpace(
-            await intent.underlyingClient.getSpace(roomId), event.content, event.stateKey
+            as, config, await intent.underlyingClient.getSpace(roomId), event.content, event.stateKey
         );
     }
 
@@ -137,12 +143,21 @@ export class GitHubUserSpace extends BaseConnection implements IConnection {
         };
     }
 
-    constructor(public readonly space: Space,
+    private readonly grantChecker: GrantChecker;
+
+    constructor(as: Appservice,
+        config: BridgeConfig,
+        public readonly space: Space,
         private state: GitHubUserSpaceConnectionState,
         stateKey: string) {
             super(space.roomId, stateKey, GitHubUserSpace.CanonicalEventType);
+            this.grantChecker = new ConfigGrantChecker("github", as, config);
         }
 
+    public ensureGrant(sender?: string) {
+        return this.grantChecker.assertConnectionGranted(this.roomId, GitHubUserSpace.grantKey(this.state), sender);
+    }
+
     public isInterestedInStateEvent(eventType: string, stateKey: string) {
         return GitHubUserSpace.EventTypes.includes(eventType) && this.stateKey === stateKey;
     }

src/Connections/GitlabIssue.ts

@@ -5,11 +5,12 @@ import { UserTokenStore } from "../UserTokenStore";
 import { Logger } from "matrix-appservice-bridge";
 import { CommentProcessor } from "../CommentProcessor";
 import { MessageSenderClient } from "../MatrixSender";
-import { BridgeConfigGitLab, GitLabInstance } from "../Config/Config";
+import { BridgeConfig, BridgeConfigGitLab, GitLabInstance } from "../Config/Config";
 import { GetIssueResponse } from "../Gitlab/Types";
 import { IGitLabWebhookNoteEvent } from "../Gitlab/WebhookTypes";
 import { ensureUserIsInRoom, getIntentForUser } from "../IntentUtils";
 import { BaseConnection } from "./BaseConnection";
+import { ConfigGrantChecker, GrantChecker } from "../grants/GrantCheck";
 
 export interface GitLabIssueConnectionState {
     instance: string;
@@ -70,7 +71,7 @@ export class GitLabIssueConnection extends BaseConnection implements IConnection
             commentProcessor,
             messageClient,
             instance,
-            config.gitlab,
+            config,
         );
     }
 
@@ -84,7 +85,7 @@ export class GitLabIssueConnection extends BaseConnection implements IConnection
         tokenStore: UserTokenStore,
         commentProcessor: CommentProcessor,
         messageSender: MessageSenderClient,
-        config: BridgeConfigGitLab,
+        config: BridgeConfig,
     ) {
         const state: GitLabIssueConnectionState = {
             projects,
@@ -109,6 +110,11 @@ export class GitLabIssueConnection extends BaseConnection implements IConnection
                 },
             ],
         });
+        await new GrantChecker(as.botIntent, "gitlab").grantConnection(roomId, {
+            instance: state.instance,
+            project: state.projects[0].toString(),
+            issue: state.iid.toString(),
+        });
 
         return new GitLabIssueConnection(roomId, as, intent, state, issue.web_url, tokenStore, commentProcessor, messageSender, instance, config);
     }
@@ -121,6 +127,9 @@ export class GitLabIssueConnection extends BaseConnection implements IConnection
         return this.instance.url;
     }
 
+    private readonly grantChecker: GrantChecker<{instance: string, project: string, issue: string}>;
+    private readonly config: BridgeConfigGitLab;
+
     constructor(
         roomId: string,
         private readonly as: Appservice,
@@ -131,9 +140,22 @@ export class GitLabIssueConnection extends BaseConnection implements IConnection
         private commentProcessor: CommentProcessor,
         private messageClient: MessageSenderClient,
         private instance: GitLabInstance,
-        private config: BridgeConfigGitLab,
+        config: BridgeConfig,
     ) {
         super(roomId, stateKey, GitLabIssueConnection.CanonicalEventType);
+        this.grantChecker = new ConfigGrantChecker("gitlab", as, config);
+        if (!config.gitlab) {
+            throw Error('No gitlab config!');
+        }
+        this.config = config.gitlab;
+    }
+
+    public ensureGrant(sender?: string) {
+        return this.grantChecker.assertConnectionGranted(this.roomId, {
+            instance: this.state.instance,
+            project: this.state.projects[0],
+            issue: this.state.iid.toString(),
+        }, sender);
     }
 
     public isInterestedInStateEvent(eventType: string, stateKey: string) {

src/Connections/GitlabRepo.ts

@@ -20,6 +20,7 @@ import { HookFilter } from "../HookFilter";
 import { GitLabClient } from "../Gitlab/Client";
 import { IBridgeStorageProvider } from "../Stores/StorageProvider";
 import axios from "axios";
+import { GitLabGrantChecker } from "../Gitlab/GrantChecker";
 
 export interface GitLabRepoConnectionState extends IConnectionState {
     instance: string;
@@ -202,7 +203,7 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
         throw new ValidatorApiError(validator.errors);
     }
 
-    static async createConnectionForState(roomId: string, event: StateEvent<Record<string, unknown>>, {intent, tokenStore, config}: InstantiateConnectionOpts) {
+    static async createConnectionForState(roomId: string, event: StateEvent<Record<string, unknown>>, {as, intent, tokenStore, config}: InstantiateConnectionOpts) {
         if (!config.gitlab) {
             throw Error('GitLab is not configured');
         }
@@ -211,18 +212,16 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
         if (!instance) {
             throw Error('Instance name not recognised');
         }
-        return new GitLabRepoConnection(roomId, event.stateKey, intent, state, tokenStore, instance);
+        return new GitLabRepoConnection(roomId, event.stateKey, as, config.gitlab, intent, state, tokenStore, instance);
     }
 
-    public static async provisionConnection(roomId: string, requester: string, data: Record<string, unknown>, { config, intent, tokenStore, getAllConnectionsOfType }: ProvisionConnectionOpts) {
-        if (!config.gitlab) {
-            throw Error('GitLab is not configured');
-        }
-        const gitlabConfig = config.gitlab;
-        const validData = this.validateState(data);
-        const instance = gitlabConfig.instances[validData.instance];
+    public static async assertUserHasAccessToProject(
+        instanceName: string, path: string, requester: string,
+        tokenStore: UserTokenStore, config: BridgeConfigGitLab
+    ) {
+        const instance = config.instances[instanceName];
         if (!instance) {
-            throw Error(`provisionConnection provided an instanceName of ${validData.instance} but the instance does not exist`);
+            throw Error(`provisionConnection provided an instanceName of ${instanceName} but the instance does not exist`);
         }
         const client = await tokenStore.getGitLabForUser(requester, instance.url);
         if (!client) {
@@ -230,7 +229,7 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
         }
         let permissionLevel;
         try {
-            permissionLevel = await client.projects.getMyAccessLevel(validData.path);
+            permissionLevel = await client.projects.getMyAccessLevel(path);
         } catch (ex) {
             throw new ApiError("Could not determine if the user has access to this project, does the project exist?", ErrCode.ForbiddenUser);
         }
@@ -238,11 +237,28 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
         if (permissionLevel < AccessLevel.Developer) {
             throw new ApiError("You must at least have developer access to bridge this project", ErrCode.ForbiddenUser);
         }
+        return permissionLevel;
+    }
+
+    public static async provisionConnection(roomId: string, requester: string, data: Record<string, unknown>, { as, config, intent, tokenStore, getAllConnectionsOfType }: ProvisionConnectionOpts) {
+        if (!config.gitlab) {
+            throw Error('GitLab is not configured');
+        }
+        const validData = this.validateState(data);
+        const gitlabConfig = config.gitlab;
+        const instance = gitlabConfig.instances[validData.instance];
+        if (!instance) {
+            throw Error(`provisionConnection provided an instanceName of ${validData.instance} but the instance does not exist`);
+        }
+        const permissionLevel = await this.assertUserHasAccessToProject(validData.instance, validData.path, requester, tokenStore, gitlabConfig);
+        const client = await tokenStore.getGitLabForUser(requester, instance.url);
+        if (!client) {
+            throw new ApiError("User is not authenticated with GitLab", ErrCode.ForbiddenUser);
+        }
 
         const project = await client.projects.get(validData.path);
-
         const stateEventKey = `${validData.instance}/${validData.path}`;
-        const connection = new GitLabRepoConnection(roomId, stateEventKey, intent, validData, tokenStore, instance);
+        const connection = new GitLabRepoConnection(roomId, stateEventKey, as, gitlabConfig, intent, validData, tokenStore, instance);
         const existingConnections = getAllConnectionsOfType(GitLabRepoConnection);
         const existing = existingConnections.find(c => c.roomId === roomId && c.instance.url === connection.instance.url && c.path === connection.path);
 
@@ -282,6 +298,7 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
             };
             log.warn(`Not creating webhook, permission level is insufficient (${permissionLevel} < ${AccessLevel.Maintainer})`)
         }
+        await new GitLabGrantChecker(as, gitlabConfig, tokenStore).grantConnection(roomId, { instance: validData.instance, path: validData.path })
         await intent.underlyingClient.sendStateEvent(roomId, this.CanonicalEventType, connection.stateKey, validData);
         return {connection, warning};
     }
@@ -377,9 +394,13 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
     private readonly mergeRequestSeenDiscussionIds = new QuickLRU<string, undefined>({ maxSize: 100 });
     private readonly hookFilter: HookFilter<AllowedEventsNames>;
 
+    private readonly grantChecker;
+
     constructor(
         roomId: string,
         stateKey: string,
+        as: Appservice,
+        config: BridgeConfigGitLab,
         private readonly intent: Intent,
         state: ConnectionStateValidated,
         private readonly tokenStore: UserTokenStore,
@@ -397,6 +418,7 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
             "!gl",
             "gitlab",
         )
+        this.grantChecker = new GitLabGrantChecker(as, config, tokenStore);
         if (!state.path || !state.instance) {
             throw Error('Invalid state, missing `path` or `instance`');
         }
@@ -431,6 +453,11 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
     }
 
     public async onStateUpdate(stateEv: MatrixEvent<unknown>) {
+        const validatedState = GitLabRepoConnection.validateState(stateEv.content);
+        await this.grantChecker.assertConnectionGranted(this.roomId, {
+            instance: validatedState.instance,
+            path: validatedState.path,
+        } , stateEv.sender);
         await super.onStateUpdate(stateEv);
         this.hookFilter.enabledHooks = this.state.enableHooks;
     }
@@ -856,6 +883,7 @@ ${data.description}`;
 
     public async onRemove() {
         log.info(`Removing ${this.toString()} for ${this.roomId}`);
+        await this.grantChecker.ungrantConnection(this.roomId, { instance: this.state.instance, path: this.path });
         // Do a sanity check that the event exists.
         try {
             await this.intent.underlyingClient.getRoomStateEvent(this.roomId, GitLabRepoConnection.CanonicalEventType, this.stateKey);

src/Connections/IConnection.ts

@@ -25,6 +25,19 @@ export interface IConnection {
 
     priority: number;
 
+    /**
+     * Ensures that the current state loaded into the connection has been granted by
+     * the remote service. I.e. If the room is bridged into a GitHub repository,
+     * check that the *sender* has permission to bridge it.
+     *
+     * If a grant cannot be found, it may be determined by doing an API lookup against
+     * the remote service.
+     * 
+     * @param sender The matrix ID of the sender of the event.
+     * @throws If the grant cannot be found, and cannot be detetermined, this will throw.
+     */
+    ensureGrant?: (sender?: string) => void;
+
     /**
      * The unique connection ID. This is a opaque hash of the roomId, connection type and state key.
      */

src/Connections/JiraProject.ts

@@ -16,6 +16,8 @@ import JiraApi from "jira-client";
 import { GetConnectionsResponseItem } from "../provisioning/api";
 import { BridgeConfigJira } from "../Config/Config";
 import { HookshotJiraApi } from "../Jira/Client";
+import { GrantChecker } from "../grants/GrantCheck";
+import { JiraGrantChecker } from "../Jira/GrantChecker";
 
 type JiraAllowedEventsNames =
     "issue_created" |
@@ -94,8 +96,6 @@ const md = new markdownit();
  */
 @Connection
 export class JiraProjectConnection extends CommandConnection<JiraProjectConnectionState> implements IConnection {
-
-
     static readonly CanonicalEventType = "uk.half-shot.matrix-hookshot.jira.project";
     static readonly LegacyCanonicalEventType = "uk.half-shot.matrix-github.jira.project";
 
@@ -107,35 +107,41 @@ export class JiraProjectConnection extends CommandConnection<JiraProjectConnecti
     static botCommands: BotCommands;
     static helpMessage: (cmdPrefix?: string) => MatrixMessageContent;
 
+    static async assertUserHasAccessToProject(tokenStore: UserTokenStore, userId: string, urlStr: string) {
+        const url = new URL(urlStr);
+        const jiraClient = await tokenStore.getJiraForUser(userId, url.toString());
+        if (!jiraClient) {
+            throw new ApiError("User is not authenticated with JIRA", ErrCode.ForbiddenUser);
+        }
+        const jiraResourceClient = await jiraClient.getClientForUrl(url);
+        if (!jiraResourceClient) {
+            throw new ApiError("User is not authenticated with this JIRA instance", ErrCode.ForbiddenUser);
+        }
+        const projectKey = JiraProjectConnection.getProjectKeyForUrl(url);
+        if (!projectKey) {
+            throw new ApiError("URL did not contain a valid project key", ErrCode.BadValue);
+        }
+        try {
+            // Need to check that the user can access this.
+            const project = await jiraResourceClient.getProject(projectKey);
+            return project;
+        } catch (ex) {
+            throw new ApiError("Requested project was not found", ErrCode.ForbiddenUser);
+        }
+    }
+
     static async provisionConnection(roomId: string, userId: string, data: Record<string, unknown>, {as, intent, tokenStore, config}: ProvisionConnectionOpts) {
         if (!config.jira) {
             throw new ApiError('JIRA integration is not configured', ErrCode.DisabledFeature);
         }
         const validData = validateJiraConnectionState(data);
         log.info(`Attempting to provisionConnection for ${roomId} ${validData.url} on behalf of ${userId}`);
-        const jiraClient = await tokenStore.getJiraForUser(userId, validData.url);
-        if (!jiraClient) {
-            throw new ApiError("User is not authenticated with JIRA", ErrCode.ForbiddenUser);
-        }
-        const jiraResourceClient = await jiraClient.getClientForUrl(new URL(validData.url));
-        if (!jiraResourceClient) {
-            throw new ApiError("User is not authenticated with this JIRA instance", ErrCode.ForbiddenUser);
-        }
+        const project = await this.assertUserHasAccessToProject(tokenStore,  userId, validData.url);
         const connection = new JiraProjectConnection(roomId, as, intent, validData, validData.url, tokenStore);
-        log.debug(`projectKey for ${validData.url} is ${connection.projectKey}`);
-        if (!connection.projectKey) {
-            throw Error('Expected projectKey to be defined');
-        }
-        try {
-            // Need to check that the user can access this.
-            const project = await jiraResourceClient.getProject(connection.projectKey);
-            // Fetch the project's id now, to support events that identify projects by id instead of url
-            if (connection.state.id !== undefined && connection.state.id !== project.id) {
-                log.warn(`Updating ID of project ${connection.projectKey} from ${connection.state.id} to ${project.id}`);
-                connection.state.id = project.id;
-            }
-        } catch (ex) {
-            throw new ApiError("Requested project was not found", ErrCode.ForbiddenUser);
+        // Fetch the project's id now, to support events that identify projects by id instead of url
+        if (connection.state.id !== undefined && connection.state.id !== project.id) {
+            log.warn(`Updating ID of project ${connection.projectKey} from ${connection.state.id} to ${project.id}`);
+            connection.state.id = project.id;
         }
         await intent.underlyingClient.sendStateEvent(roomId, JiraProjectConnection.CanonicalEventType, connection.stateKey, validData);
         log.info(`Created connection via provisionConnection ${connection.toString()}`);
@@ -200,13 +206,15 @@ export class JiraProjectConnection extends CommandConnection<JiraProjectConnecti
      */
     private projectUrl?: URL;
 
+    private readonly grantChecker: GrantChecker<{url: string}>;
+
     constructor(
         roomId: string,
         private readonly as: Appservice,
         private readonly intent: Intent,
         state: JiraProjectConnectionState,
         stateKey: string,
-        private readonly tokenStore: UserTokenStore,
+        private readonly tokenStore: UserTokenStore
     ) {
         super(
             roomId,
@@ -227,6 +235,7 @@ export class JiraProjectConnection extends CommandConnection<JiraProjectConnecti
         } else {
             throw Error('State is missing both id and url, cannot create connection');
         }
+        this.grantChecker = new JiraGrantChecker(as, tokenStore);
     }
 
     public isInterestedInStateEvent(eventType: string, stateKey: string) {
@@ -237,6 +246,12 @@ export class JiraProjectConnection extends CommandConnection<JiraProjectConnecti
         return validateJiraConnectionState(content);
     }
 
+    public ensureGrant(sender?: string) {
+        return this.grantChecker.assertConnectionGranted(this.roomId, {
+            url: this.state.url,
+        }, sender);
+    }
+
     public async onJiraIssueCreated(data: JiraIssueEvent) {
         // NOTE This is the only event type that shouldn't be skipped if the state object is missing,
         //      for backwards compatibility with issue creation having been the only supported Jira event type,
@@ -499,6 +514,9 @@ export class JiraProjectConnection extends CommandConnection<JiraProjectConnecti
 
     public async onRemove() {
         log.info(`Removing ${this.toString()} for ${this.roomId}`);
+        await this.grantChecker.ungrantConnection(this.roomId, {
+            url: this.state.url,
+        });
         // Do a sanity check that the event exists.
         try {
             await this.intent.underlyingClient.getRoomStateEvent(this.roomId, JiraProjectConnection.CanonicalEventType, this.stateKey);

src/Github/GrantChecker.ts

@@ -0,0 +1,39 @@
+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');
+
+interface GitHubGrantConnectionId {
+    org: string;
+    repo: string;
+}
+
+
+export class GitHubGrantChecker extends GrantChecker<GitHubGrantConnectionId> {
+    constructor(private readonly as: Appservice, private readonly github: GithubInstance, private readonly tokenStore: UserTokenStore) {
+        super(as.botIntent, "github")
+    }
+
+    protected async checkFallback(roomId: string, connectionId: GitHubGrantConnectionId, sender?: string) {
+        if (!sender) {
+            log.debug(`Tried to check fallback for ${roomId} with a missing sender`);
+            // Cannot validate without a sender.
+            return false;
+        }
+        if (this.as.isNamespacedUser(sender)) {
+            // Bridge is always valid.
+            return true;
+        }
+        try {
+            await GitHubRepoConnection.assertUserHasAccessToRepo(sender, connectionId.org, connectionId.repo, this.github, 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);
+            return false;
+        }
+    }
+}

src/Gitlab/GrantChecker.ts

@@ -0,0 +1,40 @@
+import { Logger } from "matrix-appservice-bridge";
+import { Appservice } from "matrix-bot-sdk";
+import { BridgeConfigGitLab } from "../Config/Config";
+import { GitLabRepoConnection } from "../Connections";
+import { GrantChecker } from "../grants/GrantCheck";
+import { UserTokenStore } from "../UserTokenStore";
+
+const log = new Logger('GitLabGrantChecker');
+
+interface GitLabGrantConnectionId{
+    instance: string;
+    path: string;
+}
+
+
+
+export class GitLabGrantChecker extends GrantChecker<GitLabGrantConnectionId> {
+    constructor(private readonly as: Appservice, private readonly config: BridgeConfigGitLab, private readonly tokenStore: UserTokenStore) {
+        super(as.botIntent, "gitlab")
+    }
+
+    protected async checkFallback(roomId: string, connectionId: GitLabGrantConnectionId, sender?: string) {
+        if (!sender) {
+            log.debug(`Tried to check fallback for ${roomId} with a missing sender`);
+            // Cannot validate without a sender.
+            return false;
+        }
+        if (this.as.isNamespacedUser(sender)) {
+            // Bridge is always valid.
+            return true;
+        }
+        try {
+            await GitLabRepoConnection.assertUserHasAccessToProject(connectionId.instance, connectionId.path, sender, this.tokenStore, this.config);
+            return true;
+        } catch (ex) {
+            log.info(`${sender} does not have access to ${connectionId.instance}/${connectionId.path}`, ex);
+            return false;
+        }
+    }
+}

src/IntentUtils.ts

@@ -35,9 +35,9 @@ export async function ensureUserIsInRoom(targetIntent: Intent, botClient: Matrix
     }
 }
 
-export async function getIntentForUser(user: {avatarUrl?: string, login: string}, as: Appservice, prefix: string) {
+export async function getIntentForUser(user: {avatarUrl?: string, login: string}, as: Appservice, prefix?: string) {
     const domain = as.botUserId.split(":")[1];
-    const intent = as.getIntentForUserId(`@${prefix}${user.login}:${domain}`);
+    const intent = as.getIntentForUserId(`@${prefix ?? ''}${user.login}:${domain}`);
     const displayName = `${user.login}`;
     // Verify up-to-date profile
     let profile;

src/Jira/GrantChecker.ts

@@ -0,0 +1,33 @@
+import { Appservice } from "matrix-bot-sdk";
+import { JiraProjectConnection } from "../Connections";
+import { GrantChecker } from "../grants/GrantCheck";
+import { UserTokenStore } from "../UserTokenStore";
+
+interface JiraGrantConnectionId{
+    url: string;
+}
+
+
+
+export class JiraGrantChecker extends GrantChecker<JiraGrantConnectionId> {
+    constructor(private readonly as: Appservice, private readonly tokenStore: UserTokenStore) {
+        super(as.botIntent, "jira")
+    }
+
+    protected async checkFallback(roomId: string, connectionId: JiraGrantConnectionId, sender?: string) {
+        if (!sender) {
+            // Cannot validate without a sender.
+            return false;
+        }
+        if (this.as.isNamespacedUser(sender)) {
+            // Bridge is always valid.
+            return true;
+        }
+        try {
+            await JiraProjectConnection.assertUserHasAccessToProject(this.tokenStore, sender, connectionId.url);
+            return true;
+        } catch (ex) {
+            return false;
+        }
+    }
+}

src/Managers/BotUsersManager.ts

@@ -119,7 +119,7 @@ export default class BotUsersManager {
      * @param roomId
      */
     onRoomJoin(botUser: BotUser, roomId: string): void {
-        log.info(`Bot user ${botUser.userId} joined room ${roomId}`);
+        log.debug(`Bot user ${botUser.userId} joined room ${roomId}`);
         const botUsers = this._botsInRooms.get(roomId) ?? new Set<BotUser>();
         botUsers.add(botUser);
         this._botsInRooms.set(roomId, botUsers);

src/grants/GrantCheck.ts

@@ -0,0 +1,114 @@
+import { Logger } from "matrix-appservice-bridge";
+import { Appservice, Intent, MatrixError } from "matrix-bot-sdk";
+import { BridgeConfig, BridgePermissionLevel } from "../Config/Config";
+import { FormatUtil } from "../FormatUtil";
+
+const GRANT_ACCOUNT_DATA_KEY = "uk.half-shot.matrix-hookshot.grant";
+
+interface GrantContent {
+    granted: boolean;
+}
+
+const log = new Logger("GrantChecker");
+
+export class GrantRejectedError extends Error {
+    constructor(public readonly roomId: string, public readonly connectionId: string) {
+        super(`No grant exists for ${roomId}/${connectionId}. Rejecting`);
+    }
+}
+
+
+type ConnectionId = string|object;
+
+export class GrantChecker<cId extends ConnectionId = ConnectionId> {
+    private static stringifyConnectionId<cId = ConnectionId>(connId: cId) {
+        if (typeof connId === "string") {
+            return FormatUtil.hashId(connId.toString());
+        }
+        return FormatUtil.hashId(Object.entries(connId as Record<string, unknown>).map((data) => `${data[0]}:${data[1]}`).join(''));
+    }
+
+    constructor(private readonly intent: Intent, protected readonly grantType: string) { }
+
+    /**
+     * If the connection hasn't been previously granted, we can use this function to check
+     * their permissions in the moment.
+     * 
+     * By default, this always returns false.
+     */
+    // eslint-disable-next-line @typescript-eslint/no-unused-vars
+    protected checkFallback(_roomId: string, _connectionId: cId, _sender?: string): Promise<boolean>|boolean {
+        return false;
+    }
+
+    private getKey(connectionIdStr: string): string {
+        return `${GRANT_ACCOUNT_DATA_KEY}/${this.grantType}/${connectionIdStr}`.toLowerCase();
+    }
+
+    public async assertConnectionGranted(roomId: string, connectionId: cId, sender?: string) {
+        const connId = GrantChecker.stringifyConnectionId(connectionId);
+        try {
+            const content = await this.intent.underlyingClient.getRoomAccountData<GrantContent>(this.getKey(connId), roomId);
+            if (!content.granted) {
+                // Previously granted but now stale.
+                throw new GrantRejectedError(roomId, connId);
+            }
+        } catch (ex) {
+            if (ex instanceof MatrixError && ex.errcode === "M_NOT_FOUND") {
+                if (!await this.checkFallback?.(roomId, connectionId, sender)) {
+                    throw new GrantRejectedError(roomId, connId);
+                } else {
+                    log.info(`Grant fallback succeeded for ${roomId}/${connectionId}`);
+                    await this.grantConnection(roomId, connectionId);
+                }
+            } else {
+                log.warn(`Failed to check grant in ${roomId}/${connectionId}`, ex);
+                throw new GrantRejectedError(roomId, connId);
+            }
+        }
+    }
+
+    public async grantConnection(roomId: string, connectionId: cId) {
+        const cidStr = GrantChecker.stringifyConnectionId(connectionId);
+        log.info(`Granting ${roomId}/${cidStr}`);
+        await this.intent.underlyingClient.setRoomAccountData(
+            this.getKey(cidStr),
+            roomId,
+            { granted: true } as GrantContent
+        );
+    }
+
+    public async ungrantConnection(roomId: string, connectionId: cId) {
+        const cidStr = GrantChecker.stringifyConnectionId(connectionId);
+        log.info(`Ungranting ${roomId}/${cidStr}`);
+        await this.intent.underlyingClient.setRoomAccountData(
+            this.getKey(cidStr),
+            roomId,
+            { granted: false } as GrantContent
+        );
+    }
+}
+
+/**
+ * Check the grant of a given connection, falling back to checking the permissions of the user
+ * across the bridge.
+ */
+export class ConfigGrantChecker<cId extends ConnectionId = ConnectionId> extends GrantChecker<cId> {
+    static ConfigMinAccessLevel = BridgePermissionLevel.admin;
+
+    constructor(grantType: string, private readonly as: Appservice, private readonly config: BridgeConfig) {
+        super(as.botIntent, grantType)
+    }
+
+    protected checkFallback(_roomId: string, _connectionId: cId, sender?: string) {
+        if (!sender) {
+            // Cannot validate without a sender.
+            return false;
+        }
+        if (this.as.isNamespacedUser(sender)) {
+            // Bridge is always valid.
+            return true;
+        }
+        return this.config.checkPermission(sender, this.grantType, ConfigGrantChecker.ConfigMinAccessLevel);
+    }
+}

tests/connections/GitlabRepoTest.ts

@@ -4,6 +4,7 @@ import { AppserviceMock } from "../utils/AppserviceMock";
 import { ApiError, ErrCode, ValidatorApiError } from "../../src/api";
 import { GitLabRepoConnection, GitLabRepoConnectionState } from "../../src/Connections";
 import { expect } from "chai";
+import { BridgeConfigGitLab } from "../../src/Config/Config";
 
 const ROOM_ID = "!foo:bar";
 
@@ -42,6 +43,8 @@ function createConnection(state: Record<string, unknown> = {}, isExistingState=f
 	const connection = new GitLabRepoConnection(
 		ROOM_ID,
 		"state_key",
+		as,
+		{} as BridgeConfigGitLab,
 		intent,
 		GitLabRepoConnection.validateState({
 			instance: "bar",

tests/grants/GrantChecker.spec.ts

@@ -0,0 +1,177 @@
+import { expect } from "chai";
+import { BridgeConfig } from "../../src/Config/Config";
+import { DefaultConfigRoot } from "../../src/Config/Defaults";
+import { FormatUtil } from "../../src/FormatUtil";
+import { ConfigGrantChecker, GrantChecker, GrantRejectedError } from '../../src/grants/GrantCheck';
+import { AppserviceMock } from "../utils/AppserviceMock";
+import { IntentMock } from "../utils/IntentMock";
+
+const ROOM_ID = '!a-room:bar';
+const CONNECTION_ID = '!a-room:bar';
+const ALWAYS_GRANT_USER = '@grant_me:bar';
+const GRANT_SERVICE_USER = '@grant_service_user:bar';
+const GRANT_SERVCE_LOW_PERMS = '@grant_service_user_without_perms:bar';
+const GRANT_WRONG_SERVCE_USER = '@grant_wrong_service_user:bar';
+const ALICE_USERID = '@alice:bar';
+const GRANT_SERVICE = 'example-grant';
+
+async function doesAssert(checker: GrantChecker<string>, roomId: string, connectionId: string, sender?: string) {
+    try {
+        await checker.assertConnectionGranted(roomId, connectionId, sender);
+        throw Error(`Expected ${roomId}/${connectionId} to have thrown an error`)
+    } catch (ex) {
+        expect(ex).instanceOf(GrantRejectedError, 'Error thrown, but was not a grant rejected error');
+        expect(ex.roomId).to.equal(roomId, "Grant rejected, but roomId didn't match");
+        // connectionIds are always hashed
+        expect(ex.connectionId).to.equal(FormatUtil.hashId(connectionId), "Grant rejected, but connectionId didn't match");
+        return true;
+    }
+}
+
+class TestGrantChecker extends GrantChecker {
+    protected checkFallback(roomId: string, connectionId: string | object, sender?: string | undefined) {
+        return sender === ALWAYS_GRANT_USER;
+    }
+}
+
+describe("GrantChecker", () => {
+    describe('base grant system', () => {
+        let check: GrantChecker<string>;
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        let intent: any;
+        beforeEach(() => {
+            intent = IntentMock.create('@foo:bar');
+            check = new TestGrantChecker(intent, GRANT_SERVICE);
+        });
+
+        it('will grant a connection', async () => {
+            await check.grantConnection(ROOM_ID, CONNECTION_ID);
+            // And then to check that the grant has now been allowed.
+            await check.assertConnectionGranted(ROOM_ID, CONNECTION_ID);
+        });
+
+        it('will assert on a missing grant', async () => {
+            await doesAssert(
+                check,
+                ROOM_ID,
+                CONNECTION_ID
+            );
+        });
+
+        it('will allow a missing grant if sender matches', async () => {
+            // Use the special user to grant the connection
+            await check.assertConnectionGranted(ROOM_ID, CONNECTION_ID, ALWAYS_GRANT_USER);
+
+            // And then to check that the grant has now been allowed.
+            await check.assertConnectionGranted(ROOM_ID, CONNECTION_ID);
+        });
+
+        it('will not conflict with another connection id', async () => {
+            await check.grantConnection(ROOM_ID, CONNECTION_ID);
+            await doesAssert(
+                check,
+                ROOM_ID,
+                CONNECTION_ID + "2",
+            );
+        });
+
+        it('will not conflict with another room', async () => {
+            await check.grantConnection(ROOM_ID, CONNECTION_ID);
+            await doesAssert(
+                check,
+                ROOM_ID + "2",
+                CONNECTION_ID
+            );
+        });
+
+        it('will not conflict with another grant service', async () => {
+            const anotherchecker = new TestGrantChecker(intent, GRANT_SERVICE + "-two");
+            await check.grantConnection(ROOM_ID, CONNECTION_ID);
+
+            await doesAssert(
+                anotherchecker,
+                ROOM_ID,
+                CONNECTION_ID
+            );
+        });
+    });
+    describe('config fallback', () => {
+        let check: GrantChecker<string>;
+        let as: AppserviceMock;
+        beforeEach(() => {
+            const mockAs = AppserviceMock.create();
+            as = mockAs;
+            const config = new BridgeConfig(
+                {
+                    ...DefaultConfigRoot,
+                    permissions: [{
+                        actor: ALWAYS_GRANT_USER,
+                        services: [{
+                            service: '*',
+                            level: "admin",
+                        }],
+                    },
+                    {
+                            actor: GRANT_SERVICE_USER,
+                            services: [{
+                                service: GRANT_SERVICE,
+                                level: "admin",
+                            }]
+                    },
+                    {
+                        actor: GRANT_SERVCE_LOW_PERMS,
+                        services: [{
+                            service: GRANT_SERVICE,
+                            level: 'notifications',
+                        }]
+                    },
+                    {
+                        actor: GRANT_WRONG_SERVCE_USER,
+                        services: [{
+                            service: 'another-service',
+                            level: "admin",
+                        }]
+                    }],
+                }
+            );
+            check = new ConfigGrantChecker(GRANT_SERVICE, mockAs, config);
+        });
+
+        it('will deny a missing grant if the sender is not provided', async () => {
+            await doesAssert(
+                check,
+                ROOM_ID,
+                CONNECTION_ID
+            );
+        });
+
+        it('will deny a missing grant if the sender is not in the appservice whitelist', async () => {
+            await doesAssert(
+                check,
+                ROOM_ID,
+                CONNECTION_ID,
+                ALICE_USERID,
+            );
+        });
+
+        it('will grant if the user is part of the appservice', async () => {
+            await check.assertConnectionGranted(ROOM_ID, CONNECTION_ID,  as.namespace + "bot");
+        });
+
+        it('will grant if the user has access to all services', async () => {
+            await check.assertConnectionGranted(ROOM_ID, CONNECTION_ID,  ALWAYS_GRANT_USER);
+        });
+
+        it('will grant if the user has access to this service', async () => {
+            await check.assertConnectionGranted(ROOM_ID, CONNECTION_ID,  GRANT_SERVICE_USER);
+        });
+
+        it('will not grant if the user has low access to this service', async () => {
+            await doesAssert(check, ROOM_ID, CONNECTION_ID, GRANT_SERVCE_LOW_PERMS);
+        });
+
+        it('will not grant if the user has access to a different service', async () => {
+            await doesAssert(check, ROOM_ID, CONNECTION_ID, GRANT_WRONG_SERVCE_USER);
+        });
+    });
+});

tests/utils/AppserviceMock.ts

@@ -4,6 +4,8 @@ export class AppserviceMock {
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     public readonly intentMap = new Map<string, any>();
     public readonly botIntent = IntentMock.create(`@bot:example.com`);
+    public namespace = "@hookshot_";
+    
     static create(){
         // eslint-disable-next-line @typescript-eslint/no-explicit-any
         return new this() as any;
@@ -26,4 +28,8 @@ export class AppserviceMock {
         this.intentMap.set(userId, intent);
         return intent;
     }
+
+    public isNamespacedUser(userId: string) {
+        return userId.startsWith(this.namespace);
+    }
 }

tests/utils/IntentMock.ts

@@ -1,5 +1,6 @@
 
 import { expect } from "chai";
+import { MatrixError } from "matrix-bot-sdk";
 export class MatrixClientMock {
 
     static create(){
@@ -9,6 +10,7 @@ export class MatrixClientMock {
 
     // map room Id → user Ids
     private joinedMembers: Map<string, string[]> = new Map();
+    public readonly roomAccountData: Map<string, string> = new Map();
 
     async setDisplayName() {
         return;
@@ -28,6 +30,21 @@ export class MatrixClientMock {
         roomMembers.push(userId);
         this.joinedMembers.set(roomId, roomMembers);
     }
+
+    async getRoomAccountData(key: string, roomId: string): Promise<string> {
+        const data = this.roomAccountData.get(roomId+key);
+        if (data) {
+            return data;
+        }
+        throw new MatrixError({
+            errcode: 'M_NOT_FOUND',
+            error: 'Test error: No account data',
+        }, 404);
+    }
+
+    async setRoomAccountData(key: string, roomId: string, value: string): Promise<void> {
+        this.roomAccountData.set(roomId+key, value);
+    }
 }
 
 export class IntentMock {