From 5b294bc05fdf2b76b19fa9106a4658e63b01979d Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Fri, 14 Jan 2022 17:44:15 +0000 Subject: [PATCH] Add support for a tiered permission system (#167) * Add basic permission model * Add permission mapping implementation * Start integrating config checks * Add permission checking for commands * Add warnings for legacy behaviour * changelog * Linting * Fix config build step * Tests * Add documentation * Add complete tests * Add documentation * Add room for room membership permissions * Fixup error * Update sampleConfig --- .github/workflows/main.yml | 2 +- changelog.d/167.feature | 3 + config.sample.yml | 7 + docs/setup.md | 78 +++++++++ src/AdminRoom.ts | 5 +- src/BotCommands.ts | 12 +- src/Bridge.ts | 18 +- src/Config/Config.ts | 68 +++++++- src/Config/Decorators.ts | 11 ++ src/Config/Defaults.ts | 17 +- src/Config/mod.rs | 1 + src/Config/permissions.rs | 164 +++++++++++++++++++ src/ConnectionManager.ts | 34 +++- src/Connections/CommandConnection.ts | 9 +- src/Connections/GithubRepo.ts | 8 +- src/Connections/GitlabRepo.ts | 3 +- src/Connections/IConnection.ts | 4 +- src/Connections/JiraProject.ts | 3 +- src/Connections/SetupConnection.ts | 14 +- src/Notifications/UserNotificationWatcher.ts | 7 +- src/UserTokenStore.ts | 6 +- src/lib.rs | 1 + tests/config/permissions.ts | 123 ++++++++++++++ 23 files changed, 567 insertions(+), 31 deletions(-) create mode 100644 changelog.d/167.feature create mode 100644 src/Config/mod.rs create mode 100644 src/Config/permissions.rs create mode 100644 tests/config/permissions.ts diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 73d7caf3..468a6e6c 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -37,7 +37,7 @@ jobs: uses: actions/setup-node@v1 with: node-version: 16 - - run: yarn --ignore-scripts + - run: yarn # Need to build to get rust bindings - run: yarn --silent ts-node src/Config/Defaults.ts --config | diff config.sample.yml - metrics-docs: diff --git a/changelog.d/167.feature b/changelog.d/167.feature new file mode 100644 index 00000000..b0ac09fe --- /dev/null +++ b/changelog.d/167.feature @@ -0,0 +1,3 @@ +New configuraion option `permissions` to control who can use the bridge. +**Please note**: By default, all users on the same homeserver will be given `admin` permissions (to reflect previous behaviour). Please adjust +your config when updating. \ No newline at end of file diff --git a/config.sample.yml b/config.sample.yml index 1dc1e25c..6e80683c 100644 --- a/config.sample.yml +++ b/config.sample.yml @@ -91,6 +91,13 @@ logging: # (Optional) Logging settings. You can have a severity debug,info,warn,error # level: info +permissions: + # (Optional) Permissions for using the bridge. See docs/setup.md#permissions for help + # + - actor: example.com + services: + - service: "*" + level: admin listeners: # (Optional) HTTP Listener configuration. # Bind resource endpoints to ports and addresses. diff --git a/docs/setup.md b/docs/setup.md index 33dabc23..046f07d3 100644 --- a/docs/setup.md +++ b/docs/setup.md @@ -61,6 +61,84 @@ Copy `registration.sample.yml` into `registration.yml` and fill in: You will need to link the registration file to the homeserver. Consult your homeserver documentation on how to add appservices. [Synapse documents the process here](https://matrix-org.github.io/synapse/latest/application_services.html). +### Permissions + +The bridge supports fine grained permission control over what services a user can access. +By default, any user on the bridge's own homeserver has full permission to use it. + +```yaml +permissions: + - actor: example.com + services: + - service: "*" + level: admin +``` + +You must configure a set of "actors" with access to services. An `actor` can be: +- A MxID (also known as a User ID) e.g. @Half-Shot:half-shot.uk +- A homserver domain e.g. @alice:matrix.org +- A roomId. This will allow any member of this room to complete actions. e.g. `!TlZdPIYrhwNvXlBiEk:half-shot.uk` +- `*`, to match all users. + +Each permission set can have a services. The `service` field can be: +- `github` +- `gitlab` +- `jira` +- `figma` +- `webhooks` +- `*`, for any service. + +The `level` can be: + - `commands` Can run commands within connected rooms, but NOT log into the bridge. + - `login` All the above, and can also log into the bridge. + - `notifications` All the above, and can also bridge their notifications. + - `manageConnections` All the above, and can create and delete connections (either via the provisioner, setup commands, or state events). + - `admin` All permissions. Currently there are no admin features so this exists as a placeholder. + +When permissions are checked, if a user matches any of the permission set and one +of those grants the right level for a service, they are allowed access. If none of the +definitions match, they are denined. + +#### Example + +A typical setup might be. + +```yaml +permissions: + # Allo all users to send commands to existing services + - actor: * + services: + - service: * + level: commands + # Allow any user that is part of this space to manage github connections + - actor: !TlZdPIYrhwNvXlBiEk:half-shot.uk + services: + - service: github + level: manageConnections + # Allow users on this domain to login to jira and github. + - actor: support.example.com + services: + - service: jira + level: login + - service: github + level: commands + # Allow users on this domain to enable notifications on any service. + - actor: engineering.example.com + services: + - service: * + level: notifications + # Allow users on this domain to create connections. + - actor: management.example.com + services: + - service: * + level: manageConnections + # Allow this specific user to do any action + - actor: @alice:example.com + services: + - service: * + level: admin +``` + ### Listeners configuration You will need to configure some listeners to make the bridge functional. diff --git a/src/AdminRoom.ts b/src/AdminRoom.ts index 4b46663b..9aa0f980 100644 --- a/src/AdminRoom.ts +++ b/src/AdminRoom.ts @@ -2,7 +2,7 @@ import "reflect-metadata"; import { AdminAccountData, AdminRoomCommandHandler } from "./AdminRoomCommandHandler"; import { botCommand, compileBotCommands, handleCommand, BotCommands, HelpFunction } from "./BotCommands"; -import { BridgeConfig } from "./Config/Config"; +import { BridgeConfig, BridgePermissionLevel } from "./Config/Config"; import { BridgeRoomState, BridgeRoomStateGitHub } from "./Widgets/BridgeWidgetInterface"; import { Endpoints } from "@octokit/types"; import { FormatUtil } from "./FormatUtil"; @@ -477,7 +477,8 @@ export class AdminRoom extends AdminRoomCommandHandler { } public async handleCommand(eventId: string, command: string) { - const result = await handleCommand(this.userId, command, AdminRoom.botCommands, this); + const checkPermission = (service: string, level: BridgePermissionLevel) => this.config.checkPermission(this.userId, service, level); + const result = await handleCommand(this.userId, command, AdminRoom.botCommands, this, checkPermission); if (!result.handled) { return this.sendNotice("Command not understood"); } diff --git a/src/BotCommands.ts b/src/BotCommands.ts index e5388ecc..e546293a 100644 --- a/src/BotCommands.ts +++ b/src/BotCommands.ts @@ -3,6 +3,8 @@ import stringArgv from "string-argv"; import { CommandError } from "./errors"; import { ApiError } from "./provisioning/api"; import { MatrixMessageContent } from "./MatrixEvent"; +import { BridgePermissionLevel } from "./Config/Config"; +import { PermissionCheckFn } from "./Connections"; const md = new markdown(); @@ -28,6 +30,8 @@ export interface BotCommandOptions { optionalArgs?: string[], includeUserId?: boolean, category?: string, + permissionLevel?: BridgePermissionLevel, + permissionService?: string, } @@ -84,7 +88,9 @@ export function compileBotCommands(...prototypes: Record { if (prefix) { if (!command.startsWith(prefix)) { @@ -98,6 +104,10 @@ export async function handleCommand(userId: string, command: string, botCommands // We have a match! const command = botCommands[prefix]; if (command) { + const permissionService = command.permissionService || defaultPermissionService; + if (permissionService && !permissionCheckFn(permissionService, command.permissionLevel || BridgePermissionLevel.commands)) { + return {handled: true, error: "You do not have permission to use this command"}; + } if (command.requiredArgs && command.requiredArgs.length > parts.length - i) { return {handled: true, error: "Missing args"}; } diff --git a/src/Bridge.ts b/src/Bridge.ts index 5a5852b5..0959206f 100644 --- a/src/Bridge.ts +++ b/src/Bridge.ts @@ -1,7 +1,7 @@ import { AdminAccountData } from "./AdminRoomCommandHandler"; import { AdminRoom, BRIDGE_ROOM_TYPE, LEGACY_BRIDGE_ROOM_TYPE } from "./AdminRoom"; import { Appservice, IAppserviceRegistration, RichRepliesPreprocessor, IRichReplyMetadata, StateEvent, PantalaimonClient, MatrixClient, IAppserviceStorageProvider, EventKind } from "matrix-bot-sdk"; -import { BridgeConfig, GitLabInstance } from "./Config/Config"; +import { BridgeConfig, BridgePermissionLevel, GitLabInstance } from "./Config/Config"; import { BridgeWidgetApi } from "./Widgets/BridgeWidgetApi"; import { CommentProcessor } from "./CommentProcessor"; import { ConnectionManager } from "./ConnectionManager"; @@ -632,24 +632,25 @@ export class Bridge { const processedReply = await this.replyProcessor.processEvent(event, this.as.botClient, EventKind.RoomEvent); const processedReplyMetadata: IRichReplyMetadata = processedReply?.mx_richreply; const adminRoom = this.adminRooms.get(roomId); + const checkPermission = (service: string, level: BridgePermissionLevel) => this.config.checkPermission(event.sender, service, level); if (!adminRoom) { let handled = false; for (const connection of this.connectionManager.getAllConnectionsForRoom(roomId)) { try { if (connection.onMessageEvent) { - handled = await connection.onMessageEvent(event, processedReplyMetadata); + handled = await connection.onMessageEvent(event, checkPermission, processedReplyMetadata); } } catch (ex) { log.warn(`Connection ${connection.toString()} failed to handle message:`, ex); } } - if (!handled) { + if (!handled && this.config.checkPermissionAny(event.sender, BridgePermissionLevel.manageConnections)) { // Divert to the setup room code if we didn't match any of these try { await ( new SetupConnection(roomId, this.as, this.tokenStore, this.config, this.github) - ).onMessageEvent(event); + ).onMessageEvent(event, checkPermission); } catch (ex) { log.warn(`Setup connection failed to handle:`, ex); } @@ -694,6 +695,7 @@ export class Bridge { } private async onRoomJoin(roomId: string, matrixEvent: MatrixEvent) { + this.config.addMemberToCache(roomId, matrixEvent.sender); if (this.as.botUserId !== matrixEvent.sender) { // Only act on bot joins return; @@ -716,14 +718,18 @@ export class Bridge { return; } if (event.state_key !== undefined) { + if (event.type === "m.room.member" && event.content.membership !== "join") { + this.config.removeMemberFromCache(roomId, event.state_key); + return; + } // A state update, hurrah! const existingConnections = this.connectionManager.getInterestedForRoomState(roomId, event.type, event.state_key); for (const connection of existingConnections) { try { if (event.content.disabled === true) { await this.connectionManager.removeConnection(connection.roomId, connection.connectionId); - } else if (connection?.onStateUpdate) { - connection.onStateUpdate(event); + } else { + connection.onStateUpdate?.(event); } } catch (ex) { log.warn(`Connection ${connection.toString()} failed to handle onStateUpdate:`, ex); diff --git a/src/Config/Config.ts b/src/Config/Config.ts index 712afc91..66144c25 100644 --- a/src/Config/Config.ts +++ b/src/Config/Config.ts @@ -1,10 +1,23 @@ import YAML from "yaml"; import { promises as fs } from "fs"; -import { IAppserviceRegistration } from "matrix-bot-sdk"; +import { IAppserviceRegistration, MatrixClient } from "matrix-bot-sdk"; import * as assert from "assert"; -import { configKey } from "./Decorators"; +import { configKey, hideKey } from "./Decorators"; import { BridgeConfigListener, ResourceTypeArray } from "../ListenerService"; import { GitHubRepoConnectionOptions } from "../Connections/GithubRepo"; +import { BridgeConfigActorPermission, BridgePermissions } from "../libRs"; +import LogWrapper from "../LogWrapper"; + +const log = new LogWrapper("Config"); + +// Maps to permission_level_to_int in permissions.rs +export enum BridgePermissionLevel { + "commands" = 1, + login = 2, + notifications = 3, + manageConnections = 4, + admin = 5, +} interface BridgeConfigGitHubYAML { auth: { @@ -154,13 +167,14 @@ export interface BridgeConfigMetrics { port?: number; } -interface BridgeConfigRoot { +export interface BridgeConfigRoot { bot?: BridgeConfigBot; bridge: BridgeConfigBridge; figma?: BridgeConfigFigma; generic?: BridgeGenericWebhooksConfig; github?: BridgeConfigGitHub; gitlab?: BridgeConfigGitLab; + permissions?: BridgeConfigActorPermission[]; provisioning?: BridgeConfigProvisioning; jira?: BridgeConfigJira; logging: BridgeConfigLogging; @@ -179,6 +193,8 @@ export class BridgeConfig { public readonly queue: BridgeConfigQueue; @configKey("Logging settings. You can have a severity debug,info,warn,error", true) public readonly logging: BridgeConfigLogging; + @configKey(`Permissions for using the bridge. See docs/setup.md#permissions for help`, true) + public readonly permissions: BridgeConfigActorPermission[]; @configKey(`A passkey used to encrypt tokens stored inside the bridge. Run openssl genpkey -out passkey.pem -outform PEM -algorithm RSA -pkeyopt rsa_keygen_bits:4096 to generate`) public readonly passFile: string; @@ -208,6 +224,9 @@ export class BridgeConfig { 'resources' may be any of ${ResourceTypeArray.join(', ')}`, true) public readonly listeners: BridgeConfigListener[]; + @hideKey() + private readonly bridgePermissions: BridgePermissions; + constructor(configData: BridgeConfigRoot, env: {[key: string]: string|undefined}) { this.bridge = configData.bridge; assert.ok(this.bridge); @@ -232,6 +251,18 @@ export class BridgeConfig { this.logging = configData.logging || { level: "info", } + this.permissions = configData.permissions || [{ + actor: this.bridge.domain, + services: [{ + service: '*', + level: BridgePermissionLevel[BridgePermissionLevel.admin], + }] + }]; + this.bridgePermissions = new BridgePermissions(this.permissions); + + if (!configData.permissions) { + log.warn(`You have not configured any permissions for the bridge, which by default means all users on ${this.bridge.domain} have admin levels of control. Please adjust your config.`); + } if (!this.github && !this.gitlab && !this.jira && !this.generic && !this.figma) { @@ -254,7 +285,8 @@ export class BridgeConfig { resources: ['webhooks'], port: configData.webhook.port, bindAddress: configData.webhook.bindAddress, - }) + }); + log.warn("The `webhook` configuration still specifies a port/bindAddress. This should be moved to the `listeners` config."); } if (this.provisioning?.port) { @@ -263,6 +295,7 @@ export class BridgeConfig { port: this.provisioning.port, bindAddress: this.provisioning.bindAddress, }) + log.warn("The `provisioning` configuration still specifies a port/bindAddress. This should be moved to the `listeners` config."); } if (this.metrics?.port) { @@ -271,15 +304,39 @@ export class BridgeConfig { port: this.metrics.port, bindAddress: this.metrics.bindAddress, }) + log.warn("The `metrics` configuration still specifies a port/bindAddress. This should be moved to the `listeners` config."); } if (this.widgets?.port) { this.listeners.push({ resources: ['widgets'], port: this.widgets.port, - }) + }); + log.warn("The `widgets` configuration still specifies a port/bindAddress. This should be moved to the `listeners` config."); } + } + public async prefillMembershipCache(client: MatrixClient) { + for(const roomEntry of this.bridgePermissions.getInterestedRooms()) { + const membership = await client.getJoinedRoomMembers(await client.resolveRoom(roomEntry)); + membership.forEach(userId => this.bridgePermissions.addMemberToCache(roomEntry, userId)); + } + } + + public addMemberToCache(roomId: string, userId: string) { + this.bridgePermissions.addMemberToCache(roomId, userId); + } + + public removeMemberFromCache(roomId: string, userId: string) { + this.bridgePermissions.removeMemberFromCache(roomId, userId); + } + + public checkPermissionAny(mxid: string, permission: BridgePermissionLevel) { + return this.bridgePermissions.checkActionAny(mxid, BridgePermissionLevel[permission]); + } + + public checkPermission(mxid: string, service: string, permission: BridgePermissionLevel) { + return this.bridgePermissions.checkAction(mxid, service, BridgePermissionLevel[permission]); } static async parseConfig(filename: string, env: {[key: string]: string|undefined}) { @@ -296,6 +353,7 @@ export async function parseRegistrationFile(filename: string) { // Can be called directly if (require.main === module) { + LogWrapper.configureLogging("info"); BridgeConfig.parseConfig(process.argv[2] || "config.yml", process.env).then(() => { // eslint-disable-next-line no-console console.log('Config successfully validated.'); diff --git a/src/Config/Decorators.ts b/src/Config/Decorators.ts index b8ec0b66..597002a5 100644 --- a/src/Config/Decorators.ts +++ b/src/Config/Decorators.ts @@ -9,4 +9,15 @@ export function configKey(comment?: string, optional = false) { // eslint-disable-next-line @typescript-eslint/no-explicit-any export function getConfigKeyMetadata(target: any, propertyKey: string): [string, boolean] { return Reflect.getMetadata(configKeyMetadataKey, target, propertyKey); +} + +const hideKeyMetadataKey = Symbol("hideKey"); +export function hideKey() { + return Reflect.metadata(hideKeyMetadataKey, true); +} + + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function keyIsHidden(target: any, propertyKey: string): boolean { + return Reflect.getMetadata(hideKeyMetadataKey, target, propertyKey) !== undefined; } \ No newline at end of file diff --git a/src/Config/Defaults.ts b/src/Config/Defaults.ts index 74fd7f8f..13f9f545 100644 --- a/src/Config/Defaults.ts +++ b/src/Config/Defaults.ts @@ -1,6 +1,6 @@ import { BridgeConfig } from "./Config"; import YAML from "yaml"; -import { getConfigKeyMetadata } from "./Decorators"; +import { getConfigKeyMetadata, keyIsHidden } from "./Decorators"; import { Node, YAMLSeq } from "yaml/types"; import { randomBytes } from "crypto"; @@ -20,6 +20,13 @@ export const DefaultConfig = new BridgeConfig({ logging: { level: "info", }, + permissions: [{ + actor: "example.com", + services: [{ + service: "*", + level: "admin" + }], + }], passFile: "passkey.pem", widgets: { publicUrl: "https://example.com/bridge_widget/", @@ -105,6 +112,10 @@ export const DefaultConfig = new BridgeConfig({ function renderSection(doc: YAML.Document, obj: Record, parentNode?: YAMLSeq) { const entries = Object.entries(obj); entries.forEach(([key, value]) => { + if (keyIsHidden(obj, key)) { + return; + } + let newNode: Node; if (typeof value === "object" && !Array.isArray(value)) { newNode = YAML.createNode({}); @@ -112,7 +123,7 @@ function renderSection(doc: YAML.Document, obj: Record, parentN } else { newNode = YAML.createNode(value); } - + const metadata = getConfigKeyMetadata(obj, key); if (metadata) { newNode.commentBefore = `${metadata[1] ? ' (Optional)' : ''} ${metadata[0]}\n`; @@ -141,7 +152,7 @@ async function renderRegistrationFile(configPath?: string) { if (configPath) { bridgeConfig = await BridgeConfig.parseConfig(configPath, process.env); } else { - bridgeConfig = new BridgeConfig(DefaultConfig, process.env); + bridgeConfig = DefaultConfig; } const obj = { as_token: randomBytes(32).toString('hex'), diff --git a/src/Config/mod.rs b/src/Config/mod.rs new file mode 100644 index 00000000..a527b636 --- /dev/null +++ b/src/Config/mod.rs @@ -0,0 +1 @@ +pub mod permissions; diff --git a/src/Config/permissions.rs b/src/Config/permissions.rs new file mode 100644 index 00000000..cce826fb --- /dev/null +++ b/src/Config/permissions.rs @@ -0,0 +1,164 @@ +use std::collections::{HashMap, HashSet}; + +#[derive(Serialize, Deserialize, Clone)] +#[napi(object)] +pub struct BridgeConfigServicePermission { + pub service: Option, + pub level: String, + pub targets: Option>, +} + +#[derive(Serialize, Deserialize, Clone)] +#[napi(object)] +pub struct BridgeConfigActorPermission { + pub actor: String, + pub services: Vec, +} + +#[napi] +pub fn permission_level_to_int(level: String) -> napi::Result { + match level.as_str() { + "commands" => Ok(1), + "login" => Ok(2), + "notifications" => Ok(3), + "manageConnections" => Ok(4), + "admin" => Ok(5), + _ => Err(napi::Error::new( + napi::Status::InvalidArg, + "provided level wasn't valid".to_string(), + )), + } +} + +#[napi] +struct BridgePermissions { + config: Vec, + room_membership: HashMap>, +} + +#[napi] +impl BridgePermissions { + #[napi(constructor)] + pub fn new(config: Vec) -> Self { + let mut room_membership = HashMap::new(); + for entry in config.iter() { + if entry.actor.starts_with("!") { + room_membership.insert(entry.actor.clone(), HashSet::new()); + } + } + BridgePermissions { + config: config, + room_membership: room_membership, + } + } + + fn match_actor( + &self, + actor_permission: &BridgeConfigActorPermission, + domain: &String, + mxid: &String, + ) -> bool { + if actor_permission.actor.starts_with("!") { + match self.room_membership.get(&actor_permission.actor) { + Some(set) => { + if !set.contains(mxid) { + // User not in set. + return false; + } + } + None => { + // No cached data stored...odd. + return false; + } + } + } + return actor_permission.actor.eq(domain) + || actor_permission.actor.eq(mxid) + || actor_permission.actor == "*"; + } + + #[napi] + pub fn get_interested_rooms(&self) -> Vec { + self.room_membership.keys().map(|k| k.clone()).collect() + } + + #[napi] + pub fn add_member_to_cache(&mut self, room_id: String, mxid: String) { + match self.room_membership.get_mut(&room_id) { + Some(set) => { + set.insert(mxid); + } + None => { /* Do nothing, not interested in this one. */ } + } + } + + #[napi] + pub fn remove_member_from_cache(&mut self, room_id: String, mxid: String) { + match self.room_membership.get_mut(&room_id) { + Some(set) => { + set.remove(&mxid); + } + None => { /* Do nothing, not interested in this one. */ } + } + } + + #[napi] + pub fn check_action( + &self, + mxid: String, + service: String, + permission: String, + ) -> napi::Result { + let parts: Vec<&str> = mxid.split(':').collect(); + let domain: String; + let permission_int = permission_level_to_int(permission)?; + if parts.len() > 1 { + domain = parts[1].to_string(); + } else { + domain = parts[0].to_string(); + } + for actor_permission in self.config.iter() { + // Room_id + if !self.match_actor(actor_permission, &domain, &mxid) { + continue; + } + for actor_service in actor_permission.services.iter() { + match &actor_service.service { + Some(actor_service_service) => { + if actor_service_service != &service && actor_service_service != "*" { + continue; + } + } + None => {} + } + if permission_level_to_int(actor_service.level.clone())? >= permission_int { + return Ok(true); + } + } + } + Ok(false) + } + + #[napi] + pub fn check_action_any(&self, mxid: String, permission: String) -> napi::Result { + let parts: Vec<&str> = mxid.split(':').collect(); + let domain: String; + let permission_int = permission_level_to_int(permission)?; + if parts.len() > 1 { + domain = parts[1].to_string(); + } else { + domain = parts[0].to_string(); + } + for actor_permission in self.config.iter() { + if !self.match_actor(actor_permission, &domain, &mxid) { + continue; + } + for actor_service in actor_permission.services.iter() { + if permission_level_to_int(actor_service.level.clone())? >= permission_int { + return Ok(true); + } + } + } + Ok(false) + } +} diff --git a/src/ConnectionManager.ts b/src/ConnectionManager.ts index ef77e3d8..cc81378f 100644 --- a/src/ConnectionManager.ts +++ b/src/ConnectionManager.ts @@ -6,7 +6,7 @@ import { Appservice, StateEvent } from "matrix-bot-sdk"; import { CommentProcessor } from "./CommentProcessor"; -import { BridgeConfig, GitLabInstance } from "./Config/Config"; +import { BridgeConfig, BridgePermissionLevel, GitLabInstance } from "./Config/Config"; import { GenericHookConnection, GitHubDiscussionConnection, GitHubDiscussionSpace, GitHubIssueConnection, GitHubProjectConnection, GitHubRepoConnection, GitHubUserSpace, GitLabIssueConnection, GitLabRepoConnection, IConnection, JiraProjectConnection } from "./Connections"; import { GenericHookAccountData } from "./Connections/GenericHook"; import { GithubInstance } from "./Github/GithubInstance"; @@ -72,6 +72,9 @@ export class ConnectionManager { if (!this.config.jira) { throw Error('JIRA is not configured'); } + if (!this.config.checkPermission(userId, "jira", BridgePermissionLevel.manageConnections)) { + throw new ApiError('User is not permitted to provision connections for Jira', ErrCode.ForbiddenUser); + } const res = await JiraProjectConnection.provisionConnection(roomId, userId, data, this.as, this.tokenStore); await this.as.botIntent.underlyingClient.sendStateEvent(roomId, JiraProjectConnection.CanonicalEventType, res.connection.stateKey, res.stateEventContent); this.push(res.connection); @@ -85,6 +88,9 @@ export class ConnectionManager { if (!this.config.github || !this.config.github.oauth || !this.github) { throw Error('GitHub is not configured'); } + if (!this.config.checkPermission(userId, "github", BridgePermissionLevel.manageConnections)) { + throw new ApiError('User is not permitted to provision connections for GitHub', ErrCode.ForbiddenUser); + } const res = await GitHubRepoConnection.provisionConnection(roomId, userId, data, this.as, this.tokenStore, this.github, this.config.github); await this.as.botIntent.underlyingClient.sendStateEvent(roomId, GitHubRepoConnection.CanonicalEventType, res.connection.stateKey, res.stateEventContent); this.push(res.connection); @@ -94,6 +100,9 @@ export class ConnectionManager { if (!this.config.generic) { throw Error('Generic hook support not supported'); } + if (!this.config.checkPermission(userId, "webhooks", BridgePermissionLevel.manageConnections)) { + throw new ApiError('User is not permitted to provision connections for generic webhooks', ErrCode.ForbiddenUser); + } const res = await GenericHookConnection.provisionConnection(roomId, this.as, data, this.config.generic, this.messageClient); const existing = this.getAllConnectionsOfType(GenericHookConnection).find(c => c.stateKey === res.connection.stateKey); if (existing) { @@ -109,16 +118,27 @@ export class ConnectionManager { throw new ApiError(`Connection type not known`); } + private assertStateAllowed(state: StateEvent, serviceType: "github"|"gitlab"|"jira"|"figma"|"webhooks") { + if (state.sender === this.as.botUserId) { + return; + } + if (!this.config.checkPermission(state.sender, serviceType, BridgePermissionLevel.manageConnections)) { + throw new Error(`User ${state.sender} is disallowed to create state for ${serviceType}`); + } + } + public async createConnectionForState(roomId: string, state: StateEvent) { if (state.content.disabled === true) { log.debug(`${roomId} has disabled state for ${state.type}`); return; } + if (GitHubRepoConnection.EventTypes.includes(state.type)) { if (!this.github || !this.config.github) { throw Error('GitHub is not configured'); } + this.assertStateAllowed(state, "github"); return new GitHubRepoConnection(roomId, this.as, state.content, this.tokenStore, state.stateKey, this.github, this.config.github); } @@ -126,6 +146,7 @@ export class ConnectionManager { if (!this.github) { throw Error('GitHub is not configured'); } + this.assertStateAllowed(state, "github"); return new GitHubDiscussionConnection( roomId, this.as, state.content, state.stateKey, this.tokenStore, this.commentProcessor, this.messageClient, @@ -136,6 +157,7 @@ export class ConnectionManager { if (!this.github) { throw Error('GitHub is not configured'); } + this.assertStateAllowed(state, "github"); return new GitHubDiscussionSpace( await this.as.botClient.getSpace(roomId), state.content, state.stateKey @@ -146,6 +168,8 @@ export class ConnectionManager { if (!this.github) { throw Error('GitHub is not configured'); } + + this.assertStateAllowed(state, "github"); const issue = new GitHubIssueConnection(roomId, this.as, state.content, state.stateKey || "", this.tokenStore, this.commentProcessor, this.messageClient, this.github); await issue.syncIssueState(); return issue; @@ -155,6 +179,8 @@ export class ConnectionManager { if (!this.github) { throw Error('GitHub is not configured'); } + + this.assertStateAllowed(state, "github"); return new GitHubUserSpace( await this.as.botClient.getSpace(roomId), state.content, state.stateKey ); @@ -164,6 +190,8 @@ export class ConnectionManager { if (!this.config.gitlab) { throw Error('GitLab is not configured'); } + + this.assertStateAllowed(state, "gitlab"); const instance = this.config.gitlab.instances[state.content.instance]; if (!instance) { throw Error('Instance name not recognised'); @@ -175,6 +203,7 @@ export class ConnectionManager { if (!this.config.gitlab) { throw Error('GitLab is not configured'); } + this.assertStateAllowed(state, "gitlab"); const instance = this.config.gitlab.instances[state.content.instance]; return new GitLabIssueConnection( roomId, @@ -191,6 +220,7 @@ export class ConnectionManager { if (!this.config.jira) { throw Error('JIRA is not configured'); } + this.assertStateAllowed(state, "jira"); return new JiraProjectConnection(roomId, this.as, state.content, state.stateKey, this.tokenStore); } @@ -198,6 +228,7 @@ export class ConnectionManager { if (!this.config.figma) { throw Error('Figma is not configured'); } + this.assertStateAllowed(state, "figma"); return new FigmaFileConnection(roomId, state.stateKey, state.content, this.config.figma, this.as, this.storage); } @@ -205,6 +236,7 @@ export class ConnectionManager { if (!this.config.generic) { throw Error('Generic webhooks are not configured'); } + this.assertStateAllowed(state, "webhooks"); // Generic hooks store the hookId in the account data const acctData = await this.as.botClient.getSafeRoomAccountData(GenericHookConnection.CanonicalEventType, roomId, {}); // hookId => stateKey diff --git a/src/Connections/CommandConnection.ts b/src/Connections/CommandConnection.ts index 0da6e22d..8c45c9ce 100644 --- a/src/Connections/CommandConnection.ts +++ b/src/Connections/CommandConnection.ts @@ -3,6 +3,7 @@ import LogWrapper from "../LogWrapper"; import { MatrixClient } from "matrix-bot-sdk"; import { MatrixMessageContent, MatrixEvent } from "../MatrixEvent"; import { BaseConnection } from "./BaseConnection"; +import { PermissionCheckFn } from "."; const log = new LogWrapper("CommandConnection"); /** @@ -18,6 +19,7 @@ export abstract class CommandConnection extends BaseConnection { private readonly botCommands: BotCommands, private readonly helpMessage: (prefix: string) => MatrixMessageContent, protected readonly stateCommandPrefix: string, + protected readonly serviceName?: string, ) { super(roomId, stateKey, canonicalStateType); } @@ -26,8 +28,11 @@ export abstract class CommandConnection extends BaseConnection { return this.stateCommandPrefix + " "; } - public async onMessageEvent(ev: MatrixEvent) { - const commandResult = await handleCommand(ev.sender, ev.content.body, this.botCommands, this, this.commandPrefix); + public async onMessageEvent(ev: MatrixEvent, checkPermission: PermissionCheckFn) { + const commandResult = await handleCommand( + ev.sender, ev.content.body, this.botCommands, this,checkPermission, + this.serviceName, this.commandPrefix + ); if (commandResult.handled !== true) { // Not for us. return false; diff --git a/src/Connections/GithubRepo.ts b/src/Connections/GithubRepo.ts index ee22c031..e1d0d3ff 100644 --- a/src/Connections/GithubRepo.ts +++ b/src/Connections/GithubRepo.ts @@ -19,6 +19,7 @@ import { GithubInstance } from "../Github/GithubInstance"; import { GitHubIssueConnection } from "./GithubIssue"; import { BridgeConfigGitHub } from "../Config/Config"; import { ApiError, ErrCode } from "../provisioning/api"; +import { PermissionCheckFn } from "."; const log = new LogWrapper("GitHubRepoConnection"); const md = new markdown(); @@ -277,7 +278,8 @@ export class GitHubRepoConnection extends CommandConnection implements IConnecti as.botClient, GitHubRepoConnection.botCommands, GitHubRepoConnection.helpMessage, - state.commandPrefix || "!gh" + state.commandPrefix || "!gh", + "github", ); } @@ -302,8 +304,8 @@ export class GitHubRepoConnection extends CommandConnection implements IConnecti } - public async onMessageEvent(ev: MatrixEvent, reply?: IRichReplyMetadata) { - if (await super.onMessageEvent(ev)) { + public async onMessageEvent(ev: MatrixEvent, checkPermission: PermissionCheckFn, reply?: IRichReplyMetadata) { + if (await super.onMessageEvent(ev, checkPermission)) { return true; } if (!reply) { diff --git a/src/Connections/GitlabRepo.ts b/src/Connections/GitlabRepo.ts index 9bf71886..319d3bd5 100644 --- a/src/Connections/GitlabRepo.ts +++ b/src/Connections/GitlabRepo.ts @@ -51,7 +51,8 @@ export class GitLabRepoConnection extends CommandConnection { as.botClient, GitLabRepoConnection.botCommands, GitLabRepoConnection.helpMessage, - state.commandPrefix || "!gl" + state.commandPrefix || "!gl", + "gitlab", ) if (!state.path || !state.instance) { throw Error('Invalid state, missing `path` or `instance`'); diff --git a/src/Connections/IConnection.ts b/src/Connections/IConnection.ts index 994521c4..1302504c 100644 --- a/src/Connections/IConnection.ts +++ b/src/Connections/IConnection.ts @@ -2,7 +2,9 @@ import { MatrixEvent, MatrixMessageContent } from "../MatrixEvent"; import { IssuesOpenedEvent, IssuesEditedEvent } from "@octokit/webhooks-types"; import { GetConnectionsResponseItem } from "../provisioning/api"; import { IRichReplyMetadata } from "matrix-bot-sdk"; +import { BridgePermissionLevel } from "../Config/Config"; +export type PermissionCheckFn = (service: string, level: BridgePermissionLevel) => boolean; export interface IConnection { /** * The roomId that this connection serves. @@ -26,7 +28,7 @@ export interface IConnection { * When a room gets a message event. * @returns Was the message handled */ - onMessageEvent?: (ev: MatrixEvent, replyMetadata?: IRichReplyMetadata) => Promise; + onMessageEvent?: (ev: MatrixEvent, checkPermission: PermissionCheckFn, replyMetadata: IRichReplyMetadata) => Promise; onIssueCreated?: (ev: IssuesOpenedEvent) => Promise; diff --git a/src/Connections/JiraProject.ts b/src/Connections/JiraProject.ts index 462ce63a..654423c4 100644 --- a/src/Connections/JiraProject.ts +++ b/src/Connections/JiraProject.ts @@ -138,7 +138,8 @@ export class JiraProjectConnection extends CommandConnection implements IConnect as.botClient, JiraProjectConnection.botCommands, JiraProjectConnection.helpMessage, - state.commandPrefix || "!jira" + state.commandPrefix || "!jira", + "jira" ); if (state.url) { this.projectUrl = new URL(state.url); diff --git a/src/Connections/SetupConnection.ts b/src/Connections/SetupConnection.ts index 66a0e76a..48531f9e 100644 --- a/src/Connections/SetupConnection.ts +++ b/src/Connections/SetupConnection.ts @@ -8,7 +8,7 @@ import { CommandError } from "../errors"; import { UserTokenStore } from "../UserTokenStore"; import { GithubInstance } from "../Github/GithubInstance"; import { v4 as uuid } from "uuid"; -import { BridgeConfig } from "../Config/Config"; +import { BridgeConfig, BridgePermissionLevel } from "../Config/Config"; import markdown from "markdown-it"; import { FigmaFileConnection } from "./FigmaFileConnection"; const md = new markdown(); @@ -43,6 +43,9 @@ export class SetupConnection extends CommandConnection { if (!this.githubInstance || !this.config.github) { throw new CommandError("not-configured", "The bridge is not configured to support GitHub"); } + if (!this.config.checkPermission(userId, "github", BridgePermissionLevel.manageConnections)) { + throw new CommandError('You are not permitted to provision connections for GitHub'); + } if (!await this.as.botClient.userHasPowerLevelFor(userId, this.roomId, "", true)) { throw new CommandError("not-configured", "You must be able to set state in a room ('Change settings') in order to setup new integrations."); } @@ -68,6 +71,9 @@ export class SetupConnection extends CommandConnection { if (!this.config.jira) { throw new CommandError("not-configured", "The bridge is not configured to support Jira"); } + if (!this.config.checkPermission(userId, "jira", BridgePermissionLevel.manageConnections)) { + throw new CommandError('You are not permitted to provision connections for Jira'); + } if (!await this.as.botClient.userHasPowerLevelFor(userId, this.roomId, "", true)) { throw new CommandError("not-configured", "You must be able to set state in a room ('Change settings') in order to setup new integrations."); } @@ -94,6 +100,9 @@ export class SetupConnection extends CommandConnection { if (!this.config.generic?.enabled) { throw new CommandError("not-configured", "The bridge is not configured to support webhooks"); } + if (!this.config.checkPermission(userId, "webhooks", BridgePermissionLevel.manageConnections)) { + throw new CommandError('You are not permitted to provision connections for generic webhooks'); + } if (!await this.as.botClient.userHasPowerLevelFor(userId, this.roomId, "", true)) { throw new CommandError("not-configured", "You must be able to set state in a room ('Change settings') in order to setup new integrations."); } @@ -115,6 +124,9 @@ export class SetupConnection extends CommandConnection { if (!this.config.figma) { throw new CommandError("not-configured", "The bridge is not configured to support Figma"); } + if (!this.config.checkPermission(userId, "figma", BridgePermissionLevel.manageConnections)) { + throw new CommandError('You are not permitted to provision connections for Figma'); + } if (!await this.as.botClient.userHasPowerLevelFor(userId, this.roomId, "", true)) { throw new CommandError("not-configured", "You must be able to set state in a room ('Change settings') in order to setup new integrations."); } diff --git a/src/Notifications/UserNotificationWatcher.ts b/src/Notifications/UserNotificationWatcher.ts index cfe61793..78cc6be3 100644 --- a/src/Notifications/UserNotificationWatcher.ts +++ b/src/Notifications/UserNotificationWatcher.ts @@ -6,7 +6,7 @@ import { NotificationWatcherTask } from "./NotificationWatcherTask"; import { GitHubWatcher } from "./GitHubWatcher"; import { GitHubUserNotification } from "../Github/Types"; import { GitLabWatcher } from "./GitLabWatcher"; -import { BridgeConfig } from "../Config/Config"; +import { BridgeConfig, BridgePermissionLevel } from "../Config/Config"; import Metrics from "../Metrics"; export interface UserNotificationsEvent { roomId: string; @@ -25,7 +25,7 @@ export class UserNotificationWatcher { private matrixMessageSender: MessageSenderClient; private queue: MessageQueue; - constructor(config: BridgeConfig) { + constructor(private readonly config: BridgeConfig) { this.queue = createMessageQueue(config); this.matrixMessageSender = new MessageSenderClient(this.queue); } @@ -74,6 +74,9 @@ Check your token is still valid, and then turn notifications back on.`, "m.notic } public addUser(data: NotificationsEnableEvent) { + if (!this.config.checkPermission(data.userId, data.type, BridgePermissionLevel.notifications)) { + throw Error('User does not have permission enable notifications'); + } let task: NotificationWatcherTask; const key = UserNotificationWatcher.constructMapKey(data.userId, data.type, data.instanceUrl); if (data.type === "github") { diff --git a/src/UserTokenStore.ts b/src/UserTokenStore.ts index 02e2509a..79ac676c 100644 --- a/src/UserTokenStore.ts +++ b/src/UserTokenStore.ts @@ -6,9 +6,10 @@ import { publicEncrypt, privateDecrypt } from "crypto"; import LogWrapper from "./LogWrapper"; import { JiraClient } from "./Jira/Client"; import { JiraOAuthResult } from "./Jira/Types"; -import { BridgeConfig } from "./Config/Config"; +import { BridgeConfig, BridgePermissionLevel } from "./Config/Config"; import { v4 as uuid } from "uuid"; import { GitHubOAuthToken } from "./Github/Types"; +import { ApiError, ErrCode } from "./provisioning/api"; const ACCOUNT_DATA_TYPE = "uk.half-shot.matrix-hookshot.github.password-store:"; const ACCOUNT_DATA_GITLAB_TYPE = "uk.half-shot.matrix-hookshot.gitlab.password-store:"; @@ -50,6 +51,9 @@ export class UserTokenStore { } public async storeUserToken(type: TokenType, userId: string, token: string, instanceUrl?: string): Promise { + if (!this.config.checkPermission(userId, type, BridgePermissionLevel.login)) { + throw new ApiError('User does not have permission to login to service', ErrCode.ForbiddenUser); + } const key = tokenKey(type, userId, false, instanceUrl); const tokenParts: string[] = []; while (token && token.length > 0) { diff --git a/src/lib.rs b/src/lib.rs index 1f9d3543..7cf91108 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,3 +1,4 @@ +pub mod Config; pub mod Github; pub mod Jira; pub mod format_util; diff --git a/tests/config/permissions.ts b/tests/config/permissions.ts new file mode 100644 index 00000000..a79c307a --- /dev/null +++ b/tests/config/permissions.ts @@ -0,0 +1,123 @@ +import { BridgePermissions } from "../../src/libRs"; +import { expect } from "chai"; + +function genBridgePermissions(actor: string, service: string, level: string) { + return new BridgePermissions([ + { + actor, + services: [ + { + service, + level + } + ], + } + ]); +} + +describe("Config/BridgePermissions", () => { + describe("checkAction", () => { + it("will return false for an empty actor set", () => { + const bridgePermissions = new BridgePermissions([]); + expect(bridgePermissions.checkAction("@foo:bar", "empty-service", "commands")).to.be.false; + }); + it("will return false for an insufficent level", () => { + const bridgePermissions = genBridgePermissions('@foo:bar', 'my-service', 'login'); + expect(bridgePermissions.checkAction("@foo:bar", "my-service", "notifications")).to.be.false; + }); + it("will return false if the there are no matching services", () => { + const bridgePermissions = genBridgePermissions('@foo:bar', 'my-service', 'login'); + expect(bridgePermissions.checkAction("@foo:bar", "other-service", "login")).to.be.false; + }); + it("will return false if the target does not match", () => { + const bridgePermissions = genBridgePermissions('@foo:bar', 'my-service', 'login'); + expect(bridgePermissions.checkAction("@foo:baz", "my-service", "login")).to.be.false; + }); + it("will return true if there is a matching level and service", () => { + const bridgePermissions = genBridgePermissions('@foo:bar', 'my-service', 'login'); + expect(bridgePermissions.checkAction("@foo:bar", "my-service", "login")).to.be.true; + }); + it("will return true for a matching actor domain", () => { + const bridgePermissions = genBridgePermissions('bar', 'my-service', 'login'); + expect(bridgePermissions.checkAction("@foo:bar", "my-service", "login")).to.be.true; + }); + it("will return true for a wildcard actor", () => { + const bridgePermissions = genBridgePermissions('*', 'my-service', 'login'); + expect(bridgePermissions.checkAction("@foo:bar", "my-service", "login")).to.be.true; + }); + it("will return true for a wildcard service", () => { + const bridgePermissions = genBridgePermissions('@foo:bar', '*', 'login'); + expect(bridgePermissions.checkAction("@foo:bar", "my-service", "login")).to.be.true; + }); + it("will return false if a user is not present in a room", () => { + const bridgePermissions = genBridgePermissions('!foo:bar', 'my-service', 'login'); + expect(bridgePermissions.checkAction("@foo:bar", "my-service", "login")).to.be.false; + }); + it("will return true if a user is present in a room", () => { + const bridgePermissions = genBridgePermissions('!foo:bar', 'my-service', 'login'); + bridgePermissions.addMemberToCache('!foo:bar', '@foo:bar'); + expect(bridgePermissions.checkAction("@foo:bar", "my-service", "login")).to.be.false; + }); + it("will fall through and return true for multiple permission sets", () => { + const bridgePermissions = new BridgePermissions([ + { + actor: "not-you", + services: [ + { + service: "my-service", + level: "login" + } + ], + }, + { + actor: "or-you", + services: [ + { + service: "my-service", + level: "login" + } + ], + }, + { + actor: "@foo:bar", + services: [ + { + service: "my-service", + level: "commands" + } + ], + } + ]); + expect(bridgePermissions.checkAction("@foo:bar", "my-service", "commands")).to.be.true; + expect(bridgePermissions.checkAction("@foo:bar", "my-service", "login")).to.be.false; + }); + }) + + describe("permissionsCheckActionAny", () => { + it("will return false for an empty actor set", () => { + const bridgePermissions = new BridgePermissions([]); + expect(bridgePermissions.checkActionAny("@foo:bar", "commands")).to.be.false; + }); + it(`will return false for a service with an insufficent level`, () => { + const bridgePermissions = genBridgePermissions("@foo:bar", "fake-service", "commands"); + expect( + bridgePermissions.checkActionAny( + "@foo:bar", + "login" + ) + ).to.be.false; + }); + const checkActorValues = ["@foo:bar", "bar", "*"]; + checkActorValues.forEach(actor => { + it(`will return true for a service defintion of '${actor}' that has a sufficent level`, () => { + const bridgePermissions = genBridgePermissions("@foo:bar", "fake-service", "commands"); + expect( + bridgePermissions.checkActionAny( + "@foo:bar", + "commands" + ) + ).to.be.true; + }); + }); + }) +}) \ No newline at end of file