Slack pagination and bug fixes (GitbookIO#70)

* Slack pagination and bug fixes

The Slack integration had some issues, which this PR solves. Namely:

- The channels endpoint didn't support pagination and that could lead to some unexpected behaviors due to the inherent weirdness of the slack API. See context here: https://github.com/GitbookIO/support-bucket/issues/961
- The README was missing the signing secret step for publishing an integration.
- There was a bug where the signature validation code would lock the body by reading it, which made it throw once the event tried to read from it again. Solved with a request clone.
- There was an issue in the slack manifests where we would call the events path for `url_verifications`, but that should happen under `events_task`.

* Add changeset

* Improve changeset with the PR description

* Update the handle to keep asynchronicity

* Forgot to remove a test return

* Update integrations/slack/slack-manifest.yaml

* Update integrations/slack/README.md

Author: Minivera

.changeset/rare-bears-type.md

@@ -0,0 +1,10 @@
+---
+'@gitbook/integration-slack': patch
+'@gitbook/runtime': patch
+---
+
+Fixed a few bugs in the slack integration
+- The channels endpoint didn't support pagination and that could lead to some unexpected behaviors due to the inherent weirdness of the slack API. See context here: https://github.com/GitbookIO/support-bucket/issues/961
+- The README was missing the signing secret step for publishing an integration.
+- There was a bug where the signature validation code would lock the body by reading it, which made it throw once the event tried to read from it again. Solved with a request clone.
+- There was an issue in the slack manifests where we would call the events path for `url_verifications`, but that should happen under `events_task`.

integrations/slack/README.md

@@ -3,9 +3,9 @@
 To configure it for your environment, create an application on Slack and use the `slack-manifest.yaml` file to configure it. Don't forget to update the URLs to natch your environment.
 
 
-To publish it, run the command with the `SLACK_CLIENT_ID` and `SLACK_CLIENT_SECRET` environment variables defined:
+To publish it, run the command with the `SLACK_CLIENT_ID`, `SLACK_CLIENT_SECRET`, and `SLACK_SIGNING_SECRET ` environment variables defined:
 
 ```
-SLACK_CLIENT_ID=xxx SLACK_CLIENT_SECRET=xxxx npm run publish-integrations
+SLACK_CLIENT_ID=xxx SLACK_CLIENT_SECRET=xxxx SLACK_SIGNING_SECRET=xxxx npm run publish-integrations
 ```
 

integrations/slack/package.json

@@ -18,4 +18,4 @@
         "publish-integrations-staging": "gitbook publish .",
         "publish-integrations": "gitbook publish ."
     }
-}
+}

integrations/slack/src/events.ts

@@ -5,11 +5,15 @@ import { SlackRuntimeContext } from './configuration';
 /**
  * Handle an event from Slack.
  */
-export function createSlackEventsHandler(handlers: {
-    [type: string]: (event: object, context: SlackRuntimeContext) => Promise<any>;
-}): FetchEventCallback {
+export function createSlackEventsHandler(
+    handlers: {
+        [type: string]: (event: object, context: SlackRuntimeContext) => Promise<any>;
+    },
+    fallback?: FetchEventCallback
+): FetchEventCallback {
     return async (request, context) => {
-        const event = await request.json();
+        // Clone the request so its body is still available to the fallback
+        const event = await request.clone().json<{ event?: { type: string }; type?: string }>();
 
         if (!event.type) {
             return new Response(`Invalid event`, {
@@ -18,8 +22,13 @@ export function createSlackEventsHandler(handlers: {
         }
 
         const eventType = event.event?.type || event.type;
+        // Find the handle for the event type, or use the fallback if that's missing
         const handler = handlers[eventType];
         if (!handler) {
+            if (fallback) {
+                return fallback(request, context);
+            }
+
             return new Response(`No handler for event type "${eventType}"`, {
                 status: 404,
             });

integrations/slack/src/middlewares.ts

@@ -1,4 +1,3 @@
-import { Request } from 'itty-router';
 import { sha256 } from 'js-sha256';
 
 import { SlackRuntimeContext } from './configuration';
@@ -7,7 +6,10 @@ import { SlackRuntimeContext } from './configuration';
  * Verify the authenticity of a Slack request.
  * Reference - https://api.slack.com/authentication/verifying-requests-from-slack
  */
-export async function verifySlackRequest(req: Request, { environment }: SlackRuntimeContext) {
+export async function verifySlackRequest(request: Request, { environment }: SlackRuntimeContext) {
+    // Clone the request as to not use up the only read the body allows for future requests
+    const req = request.clone();
+
     // @ts-ignore
     const slackSignature = req.headers.get('x-slack-signature');
     // @ts-ignore
@@ -48,7 +50,9 @@ export async function acknowledgeSlackRequest(req: Request) {
     });
 
     return new Response(JSON.stringify({ acknowledged: true }), {
-        'Content-Type': 'application/json',
+        headers: {
+            'Content-Type': 'application/json',
+        },
     });
 }
 

integrations/slack/src/router.ts

@@ -5,7 +5,7 @@ import { createOAuthHandler, FetchEventCallback } from '@gitbook/runtime';
 import { createSlackEventsHandler } from './events';
 import { unfurlLink } from './links';
 import { acknowledgeSlackRequest, verifySlackRequest } from './middlewares';
-import { slackAPI } from './slack';
+import { getChannelsPaginated } from './slack';
 
 /**
  * Handle incoming HTTP requests:
@@ -46,24 +46,26 @@ export const handleFetchEvent: FetchEventCallback = async (request, context) =>
         })
     );
 
-    router.post('/events', acknowledgeSlackRequest);
+    router.post(
+        '/events',
+        verifySlackRequest,
+        createSlackEventsHandler(
+            {
+                url_verification: async (event: { challenge: string }) => {
+                    return { challenge: event.challenge };
+                },
+            },
+            acknowledgeSlackRequest
+        )
+    );
 
     /*
      * List the channels the user can select in the configuration flow.
      */
     router.get('/channels', async () => {
-        // TODO: list from all pages
-        const result = await slackAPI(context, {
-            method: 'GET',
-            path: 'conversations.list',
-            payload: {
-                limit: 1000,
-                exclude_archived: true,
-                types: 'public_channel,private_channel',
-            },
-        });
+        const channels = await getChannelsPaginated(context);
 
-        const completions = result?.channels.map((channel) => ({
+        const completions = channels.map((channel) => ({
             label: channel.name,
             value: channel.id,
         }));
@@ -82,9 +84,6 @@ export const handleFetchEvent: FetchEventCallback = async (request, context) =>
         '/events_task',
         verifySlackRequest,
         createSlackEventsHandler({
-            url_verification: async (event) => {
-                return { challenge: event.challenge };
-            },
             link_shared: unfurlLink,
         })
     );

integrations/slack/src/slack.ts

@@ -4,6 +4,59 @@ import { SlackRuntimeContext } from './configuration';
 
 const logger = Logger('slack:api');
 
+/**
+ * Cloudflare workers have a maximum number of subrequests we can call (20 according to my
+ * tests) https://developers.cloudflare.com/workers/platform/limits/#how-many-subrequests-can-i-make
+ * TODO: Test with 50
+ */
+const maximumSubrequests = 20;
+
+/**
+ * Executes a Slack API request to fetch channels, handles pagination, then returns the merged
+ * results.
+ */
+export async function getChannelsPaginated(context: SlackRuntimeContext) {
+    const channels = [];
+
+    let response = await slackAPI(context, {
+        method: 'GET',
+        path: 'conversations.list',
+        payload: {
+            limit: 1000,
+            exclude_archived: true,
+            types: 'public_channel,private_channel',
+        },
+    });
+    channels.push(...response?.channels);
+
+    let numberOfCalls = 0;
+    // Handle pagination. Slack can be weird when it comes to paginating requests and not return
+    // all the channels even if we tell it to fetch a thousand of them. Pagination may be called
+    // even with workspaces with less than 1000 channels. We also keep a reference to number of
+    // subsequent calls to avoid hitting the hard limit of calls on workers.
+    while (response.response_metadata.next_cursor && numberOfCalls < maximumSubrequests) {
+        logger.debug('Request was paginated, calling the next cursor');
+        response = await slackAPI(context, {
+            method: 'GET',
+            path: 'conversations.list',
+            payload: {
+                limit: 1000,
+                exclude_archived: true,
+                types: 'public_channel,private_channel',
+                cursor: response.response_metadata.next_cursor,
+            },
+        });
+        channels.push(...response?.channels);
+        numberOfCalls++;
+    }
+
+    // Remove any duplicate as the pagination API could return duplicated channels
+    return channels.filter(
+        (value, index, self) =>
+            index === self.findIndex((t) => t.place === value.place && t.id === value.id)
+    );
+}
+
 /**
  * Execute a Slack API request and return the result.
  */

packages/runtime/src/oauth.ts

@@ -3,6 +3,18 @@ import { RequestUpdateIntegrationInstallation } from '@gitbook/api';
 import { RuntimeCallback } from './context';
 import { Logger } from './logger';
 
+/**
+ * Utility interface for typing the response from a Slack OAuth call.
+ * TODO: Use the official Slack type
+ */
+export interface OAuthResponse {
+    ok?: boolean;
+    access_token: string;
+    team: {
+        id: string;
+    };
+}
+
 export interface OAuthConfig {
     /**
      * Redirect URL to use. When the OAuth identity provider only accept a static one.
@@ -33,7 +45,7 @@ export interface OAuthConfig {
      * Extract the credentials from the code exchange response.
      */
     extractCredentials?: (
-        response: object
+        response: OAuthResponse
     ) => RequestUpdateIntegrationInstallation | Promise<RequestUpdateIntegrationInstallation>;
 }
 
@@ -110,7 +122,7 @@ export function createOAuthHandler(
                 throw new Error('Failed to exchange code for access token');
             }
 
-            const json = await response.json();
+            const json = await response.json<OAuthResponse>();
 
             if (!json.ok) {
                 throw new Error(`Failed to exchange code for access token ${JSON.stringify(json)}`);