Allow GitLab connections without hook permissions (#567)

* Allow GitLab connections without hook permissions

Warn instead of fail when connecting a GitLab project that Hookshot
cannot provision a webhook for.

* Mention manual "Secret token" for GitLab webhooks

* Refactor warning pane into a separate component

* Recolour warning pane for better contrast

changelog.d/567.feature

@@ -0,0 +1 @@
+Allow adding connections to GitLab projects even when Hookshot doesn't have permissions to automatically provision a webhook for it. When that occurs, tell the user to ask a project admin to add the webhook.

docs/usage/room_configuration/gitlab_project.md

@@ -14,8 +14,8 @@ To set up a connection to a GitLab project in a new room:
 3. Give the bridge bot moderator permissions or higher (power level 50) (or otherwise configure the room so the bot can edit room state).
 4. Send the command `!hookshot gitlab project https://mydomain/my/project`.
 5. If you have permission to bridge this repo, the bridge will respond with a confirmation message. (Users with `Developer` permissions or greater can bridge projects.)
-  6. If you have configured the bridge with a `publicUrl` inside `gitlab.webhook`, it will automatically provision the webhook for you.
-  7. Otherwise, you'll need to manually configure the webhook to point to your public address for the webhooks listener.
+6. If you have configured the bridge with a `publicUrl` inside `gitlab.webhook`, and you have `Maintainer` permissions or greater on the project, the bot will automatically provision the webhook for you.
+7. Otherwise, you'll need to manually configure the project with a webhook that points to your public address for the webhooks listener, sets the "Secret token" to the one you put in your Hookshot configuration (`gitlab.webhook.secret`), and enables all Triggers that need to be bridged (as Hookshot can only bridge events for enabled Triggers). This can be configured on the GitLab webpage for the project under Settings > Webhook Settings. If you do not have access to this page, you must ask someone who does (i.e. someone with at least `Maintainer` permissions on the project) to add the webhook for you.
 
 ## Configuration
 

src/ConnectionManager.ts

@@ -68,14 +68,14 @@ export class ConnectionManager extends EventEmitter {
      * @param data The data corresponding to the connection state. This will be validated.
      * @returns The resulting connection.
      */
-    public async provisionConnection(roomId: string, userId: string, type: string, data: Record<string, unknown>): Promise<IConnection> {
+    public async provisionConnection(roomId: string, userId: string, type: string, data: Record<string, unknown>) {
         log.info(`Looking to provision connection for ${roomId} ${type} for ${userId} with data ${JSON.stringify(data)}`);
         const connectionType = ConnectionDeclarations.find(c => c.EventTypes.includes(type));
         if (connectionType?.provisionConnection) {
             if (!this.config.checkPermission(userId, connectionType.ServiceCategory, BridgePermissionLevel.manageConnections)) {
                 throw new ApiError(`User is not permitted to provision connections for this type of service.`, ErrCode.ForbiddenUser);
             }
-            const { connection } = await connectionType.provisionConnection(roomId, userId, data, {
+            const result = await connectionType.provisionConnection(roomId, userId, data, {
                 as: this.as,
                 config: this.config,
                 tokenStore: this.tokenStore,
@@ -85,8 +85,8 @@ export class ConnectionManager extends EventEmitter {
                 github: this.github,
                 getAllConnectionsOfType: this.getAllConnectionsOfType.bind(this),
             });
-            this.push(connection);
-            return connection;
+            this.push(result.connection);
+            return result;
         }
         throw new ApiError(`Connection type not known`);
     }

src/Connections/GitlabRepo.ts

@@ -10,7 +10,7 @@ import { BridgeConfigGitLab, GitLabInstance } from "../Config/Config";
 import { IGitlabMergeRequest, IGitlabProject, IGitlabUser, IGitLabWebhookMREvent, IGitLabWebhookNoteEvent, IGitLabWebhookPushEvent, IGitLabWebhookReleaseEvent, IGitLabWebhookTagPushEvent, IGitLabWebhookWikiPageEvent } from "../Gitlab/WebhookTypes";
 import { CommandConnection } from "./CommandConnection";
 import { Connection, IConnection, IConnectionState, InstantiateConnectionOpts, ProvisionConnectionOpts } from "./IConnection";
-import { GetConnectionsResponseItem } from "../provisioning/api";
+import { ConnectionWarning, GetConnectionsResponseItem } from "../provisioning/api";
 import { ErrCode, ApiError, ValidatorApiError } from "../api"
 import { AccessLevel } from "../Gitlab/Types";
 import Ajv, { JSONSchemaType } from "ajv";
@@ -217,7 +217,9 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
         }
 
         // Try to setup a webhook
-        if (gitlabConfig.webhook.publicUrl) {
+        // Requires at least a "Maintainer" role: https://docs.gitlab.com/ee/user/permissions.html
+        let warning: ConnectionWarning | undefined;
+        if (gitlabConfig.webhook.publicUrl && permissionLevel >= AccessLevel.Maintainer) {
             const hooks = await client.projects.hooks.list(project.id);
             const hasHook = hooks.find(h => h.url === gitlabConfig.webhook.publicUrl);
             if (!hasHook) {
@@ -235,11 +237,17 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
                     wiki_page_events: true,
                 });
             }
-        } else {
+        } else if (!gitlabConfig.webhook.publicUrl) {
             log.info(`Not creating webhook, webhookUrl is not defined in config`);
+        } else {
+            warning = {
+                header: "Cannot create webhook",
+                message: "You have insufficient permissions on this project to provision a webhook for it. Ask a Maintainer or Owner of the project to add the webhook for you.",
+            };
+            log.warn(`Not creating webhook, permission level is insufficient (${permissionLevel} < ${AccessLevel.Maintainer})`)
         }
         await as.botIntent.underlyingClient.sendStateEvent(roomId, this.CanonicalEventType, connection.stateKey, validData);
-        return {connection};
+        return {connection, warning};
     }
 
     public static getProvisionerDetails(botUserId: string) {

src/Connections/IConnection.ts

@@ -1,6 +1,6 @@
 import { MatrixEvent, MatrixMessageContent } from "../MatrixEvent";
 import { IssuesOpenedEvent, IssuesEditedEvent } from "@octokit/webhooks-types";
-import { GetConnectionsResponseItem } from "../provisioning/api";
+import { ConnectionWarning, GetConnectionsResponseItem } from "../provisioning/api";
 import { Appservice, IRichReplyMetadata, StateEvent } from "matrix-bot-sdk";
 import { BridgeConfig, BridgePermissionLevel } from "../Config/Config";
 import { UserTokenStore } from "../UserTokenStore";
@@ -80,7 +80,7 @@ export interface IConnection {
 export interface ConnectionDeclaration<C extends IConnection = IConnection> {
     EventTypes: string[];
     ServiceCategory: string;
-    provisionConnection?: (roomId: string, userId: string, data: Record<string, unknown>, opts: ProvisionConnectionOpts) => Promise<{connection: C}>;
+    provisionConnection?: (roomId: string, userId: string, data: Record<string, unknown>, opts: ProvisionConnectionOpts) => Promise<{connection: C, warning?: ConnectionWarning}>;
     createConnectionForState: (roomId: string, state: StateEvent<Record<string, unknown>>, opts: InstantiateConnectionOpts) => C|Promise<C>
 }
 

src/Connections/SetupConnection.ts

@@ -108,9 +108,9 @@ export class SetupConnection extends CommandConnection {
         if (!path) {
             throw new CommandError("Invalid GitLab url", "The GitLab project url you entered was not valid.");
         }
-        const {connection} = await GitLabRepoConnection.provisionConnection(this.roomId, userId, {path, instance: name}, this.provisionOpts);
+        const {connection, warning} = await GitLabRepoConnection.provisionConnection(this.roomId, userId, {path, instance: name}, this.provisionOpts);
         this.pushConnections(connection);
-        await this.as.botClient.sendNotice(this.roomId, `Room configured to bridge ${connection.prettyPath}`);
+        await this.as.botClient.sendNotice(this.roomId, `Room configured to bridge ${connection.prettyPath}` + (warning ? `\n${warning.header}: ${warning.message}` : ""));
     }
 
     private async checkJiraLogin(userId: string, urlStr: string) {

src/Widgets/BridgeWidgetApi.ts

@@ -134,11 +134,14 @@ export class BridgeWidgetApi {
                 throw new ApiError("A JSON body must be provided", ErrCode.BadValue);
             }
             this.connMan.validateCommandPrefix(req.params.roomId, req.body);
-            const connection = await this.connMan.provisionConnection(req.params.roomId as string, req.userId, req.params.type as string, req.body as Record<string, unknown>);
-            if (!connection.getProvisionerDetails) {
+            const result = await this.connMan.provisionConnection(req.params.roomId, req.userId, req.params.type, req.body);
+            if (!result.connection.getProvisionerDetails) {
                 throw new Error('Connection supported provisioning but not getProvisionerDetails');
             }
-            res.send(connection.getProvisionerDetails(true));
+            res.send({
+                ...result.connection.getProvisionerDetails(true),
+                warning: result.warning,
+            });
         } catch (ex) {
             log.error(`Failed to create connection for ${req.params.roomId}`, ex);
             throw ex;

src/provisioning/api.ts

@@ -9,11 +9,17 @@ export interface GetConnectionTypeResponseItem {
     botUserId: string;
 }
 
+export interface ConnectionWarning {
+    header: string,
+    message: string,
+}
+
 export interface GetConnectionsResponseItem<Config = object, Secrets = object> extends GetConnectionTypeResponseItem {
     id: string;
     config: Config;
     secrets?: Secrets;
     canEdit?: boolean;
+    warning?: ConnectionWarning;
 }
 
 const log = new Logger("Provisioner.api");

src/provisioning/provisioner.ts

@@ -129,18 +129,21 @@ export class Provisioner {
         return res.send(connection.getProvisionerDetails());
     }
 
-    private async putConnection(req: Request<{roomId: string, type: string}, unknown, Record<string, unknown>, {userId: string}>, res: Response, next: NextFunction) {
+    private async putConnection(req: Request<{roomId: string, type: string}, unknown, Record<string, unknown>, {userId: string}>, res: Response<GetConnectionsResponseItem>, next: NextFunction) {
         // Need to figure out which connections are available
         try {
             if (!req.body || typeof req.body !== "object") {
-                throw new ApiError("A JSON body must be provided.", ErrCode.BadValue);
+                throw new ApiError("A JSON body must be provided", ErrCode.BadValue);
             }
             this.connMan.validateCommandPrefix(req.params.roomId, req.body);
-            const connection = await this.connMan.provisionConnection(req.params.roomId, req.query.userId, req.params.type, req.body);
-            if (!connection.getProvisionerDetails) {
-                throw new Error('Connection supported provisioning but not getProvisionerDetails.');
+            const result = await this.connMan.provisionConnection(req.params.roomId, req.query.userId, req.params.type, req.body);
+            if (!result.connection.getProvisionerDetails) {
+                throw new Error('Connection supported provisioning but not getProvisionerDetails');
             }
-            res.send(connection.getProvisionerDetails(true));
+            res.send({
+                ...result.connection.getProvisionerDetails(true),
+                warning: result.warning,
+            });
         } catch (ex) {
             log.error(`Failed to create connection for ${req.params.roomId}`, ex);
             return next(ex);

web/BridgeAPI.ts

@@ -117,7 +117,7 @@ export class BridgeAPI {
         return this.request('GET', `/widgetapi/v1/${encodeURIComponent(roomId)}/connections/${encodeURIComponent(service)}`);
     }
 
-    async createConnection(roomId: string, type: string, config: IConnectionState) {
+    async createConnection(roomId: string, type: string, config: IConnectionState): Promise<GetConnectionsResponseItem> {
         return this.request('POST', `/widgetapi/v1/${encodeURIComponent(roomId)}/connections/${encodeURIComponent(type)}`, config);
     }
 

web/components/elements/ErrorPane.tsx

@@ -1,5 +1,5 @@
 import { h, FunctionComponent } from "preact";
-import ErrorBadge from "../../icons/warning-badge.svg";
+import ErrorBadge from "../../icons/error-badge.svg";
 import style from "./ErrorPane.module.scss";
 
 export const ErrorPane: FunctionComponent<{header?: string}> = ({ children, header }) => {

web/components/elements/WarningPane.module.scss

@@ -0,0 +1,4 @@
+.warningPane {
+    max-width: 480px;
+    color: #FF812D;
+}

web/components/elements/WarningPane.tsx

@@ -0,0 +1,9 @@
+import { h, FunctionComponent } from "preact";
+import WarningBadge from "../../icons/warning-badge.svg";
+import style from "./WarningPane.module.scss";
+
+export const WarningPane: FunctionComponent<{header?: string}> = ({ children, header }) => {
+    return <div class={`card error ${style.warningPane}`}>
+        <p><strong><img src={WarningBadge} /> { header || "Problem occured during widget load" }</strong>: {children}</p>
+    </div>;
+};

web/components/elements/index.ts

@@ -2,4 +2,5 @@ export * from "./Button";
 export * from "./ButtonSet";
 export * from "./ErrorPane";
 export * from "./InputField";
-export * from "./ListItem";
+export * from "./ListItem";
+export * from "./WarningPane";

web/components/roomConfig/RoomConfig.tsx

@@ -1,7 +1,7 @@
 import { h, FunctionComponent } from "preact";
 import { useCallback, useEffect, useReducer, useState } from "preact/hooks"
 import { BridgeAPI, BridgeAPIError } from "../../BridgeAPI";
-import { ErrorPane, ListItem } from "../elements";
+import { ErrorPane, ListItem, WarningPane } from "../elements";
 import style from "./RoomConfig.module.scss";
 import { GetConnectionsResponseItem } from "../../../src/provisioning/api";
 import { IConnectionState } from "../../../src/Connections";
@@ -34,18 +34,22 @@ interface IRoomConfigProps<SConfig, ConnectionType extends GetConnectionsRespons
 export const RoomConfig = function<SConfig, ConnectionType extends GetConnectionsResponseItem, ConnectionState extends IConnectionState>(props: IRoomConfigProps<SConfig, ConnectionType, ConnectionState>) {
     const { api, roomId, type, headerImg, text, listItemName, connectionEventType } = props;
     const ConnectionConfigComponent = props.connectionConfigComponent;
-    const [ error, setError ] = useState<null|{header?: string, message: string}>(null);
+    const [ error, setError ] = useState<null|{header?: string, message: string, isWarning?: boolean, forPrevious?: boolean}>(null);
     const [ connections, setConnections ] = useState<ConnectionType[]|null>(null);
     const [ serviceConfig, setServiceConfig ] = useState<SConfig|null>(null);
     const [ canEditRoom, setCanEditRoom ] = useState<boolean>(false);
     // We need to increment this every time we create a connection in order to properly reset the state.
     const [ newConnectionKey, incrementConnectionKey ] = useReducer<number, undefined>(n => n+1, 0);
 
+    const clearCurrentError = () => {
+        setError(error => error?.forPrevious ? error : null);
+    }
+
     useEffect(() => {
         api.getConnectionsForService<ConnectionType>(roomId, type).then(res => {
             setCanEditRoom(res.canEdit);
             setConnections(res.connections);
-            setError(null);
+            clearCurrentError();
         }).catch(ex => {
             console.warn("Failed to fetch existing connections", ex);
             setError({
@@ -58,9 +62,7 @@ export const RoomConfig = function<SConfig, ConnectionType extends GetConnection
     useEffect(() => {
         api.getServiceConfig<SConfig>(type)
             .then(setServiceConfig)
-            .then(() => {
-                setError(null);
-            })
+            .then(clearCurrentError)
             .catch(ex => {
                 console.warn("Failed to fetch service config", ex);
                 setError({
@@ -71,10 +73,15 @@ export const RoomConfig = function<SConfig, ConnectionType extends GetConnection
     }, [api, type]);
 
     const handleSaveOnCreation = useCallback((config: ConnectionState) => {
-        api.createConnection(roomId, connectionEventType, config).then(() => {
+        api.createConnection(roomId, connectionEventType, config).then(result => {
             // Force reload
             incrementConnectionKey(undefined);
-            setError(null);
+            setError(!result.warning ? null : {
+                header: result.warning.header,
+                message: result.warning.message,
+                isWarning: true,
+                forPrevious: true,
+            });
         }).catch(ex => {
             console.warn("Failed to create connection", ex);
             setError({
@@ -86,7 +93,11 @@ export const RoomConfig = function<SConfig, ConnectionType extends GetConnection
 
     return <main>
         {
-            error && <ErrorPane header={error.header || "Error"}>{error.message}</ErrorPane>
+            error &&
+                (!error.isWarning
+                    ? <ErrorPane header={error.header || "Error"}>{error.message}</ErrorPane>
+                    : <WarningPane header={error.header || "Warning"}>{error.message}</WarningPane>
+                )
         }
         <header className={style.header}>
             <img src={headerImg} />

web/icons/error-badge.svg

@@ -0,0 +1,5 @@
+<svg width="16" height="16" viewBox="0 0 16 16" fill="none" xmlns="http://www.w3.org/2000/svg">
+<circle cx="8" cy="8" r="8" fill="#FF4B55"/>
+<rect x="7" y="3" width="2" height="6" rx="1" fill="white"/>
+<rect x="7" y="11" width="2" height="2" rx="1" fill="white"/>
+</svg>

web/icons/warning-badge.svg

@@ -1,5 +1,5 @@
 <svg width="16" height="16" viewBox="0 0 16 16" fill="none" xmlns="http://www.w3.org/2000/svg">
-<circle cx="8" cy="8" r="8" fill="#FF4B55"/>
+<circle cx="8" cy="8" r="8" fill="#FF812D"/>
 <rect x="7" y="3" width="2" height="6" rx="1" fill="white"/>
 <rect x="7" y="11" width="2" height="2" rx="1" fill="white"/>
 </svg>

web/typings/images.d.ts

@@ -1,4 +1,8 @@
 declare module "*.png" {
     const content: string
     export = content
+}
+declare module "*.svg" {
+    const content: string
+    export = content
 }