Merge branch 'yasu/slack-webhook-to-api'

* yasu/slack-webhook-to-api:
  update docs with slack access token setup and secrets file changes
  tests: promote w/ new `channel` field in notification payload
  enforce presence of either webhooks or access token in secrets file
  use the new notification sending interface throughout the codebase
  send slack notifications with web api if token is configured
  rename slack req/res atd types so they're not webhook-specific

Author: yasunariw

README.md

@@ -22,7 +22,23 @@ Run the `_build/default/src/monorobot.exe` binary. The following commands are su
 
 - `run`: Launch the HTTP server
 - `check_gh <GH_PAYLOAD>`: read a Github notification from a file and display the actions that will be taken (used for testing)
-- `check_slack <SLACK_PAYLOAD> <SLACK_WEBHOOK>`: read a Slack notification from a file and send it to a webhook (used for testing)
+- `check_slack <SLACK_PAYLOAD>`: read a Slack notification from a file and send it to a channel (used for testing)
+
+## Getting Started
+
+1. Commit a **repository configuration** file to the root of your target repository.
+2. Place a **secrets** file locally on the server.
+3. Configure GitHub
+    1. If targeting a private repository, set up a [personal access token](https://docs.github.com/en/free-pro-team@latest/github/authenticating-to-github/creating-a-personal-access-token) with `repo` scope and store it in the `gh_token` field of the secrets file.
+    2. [Create a webhook](https://docs.github.com/en/free-pro-team@latest/developers/webhooks-and-events/creating-webhooks#setting-up-a-webhook) for the repository you are targeting. Set the *Payload URL* to be `<server_domain>/github`.
+    3. You can optionally [secure the webhook](https://docs.github.com/en/free-pro-team@latest/developers/webhooks-and-events/securing-your-webhooks) with a token, and store it in the `gh_hook_token` field of the secrets file.
+4. Configure Slack
+    1. [Create a Slack app](https://api.slack.com/apps?new_app=1).
+    2. Click "Install to Workspace", and when prompted to grant permissions to your workspace, click "Allow".
+    3. Set up notifications with one of the following methods:
+        - **Web API (recommended):** To use Slack's [Web API](https://api.slack.com/web), click on "OAuth & Permissions" in your app dashboard's sidebar. Give your bot a *Bot Token Scope* of `chat:write`. Copy the generated OAuth access token (`xoxb-XXXX`) to the `slack_access_token` field of your secrets file. This token is used by the bot to authenticate to the workspace, and remains valid until the token is revoked or the app is uninstalled.
+        - **Incoming Webhooks:** To use [incoming webhooks](https://api.slack.com/messaging/webhooks), enable them in your app dashboard and create one for each channel you want to notify. Store them in the `slack_hooks` field of your secrets file. If you decide to notify additional channels later, you will need to update the secrets file with the new webhooks and restart the server.
+
 
 ### Documentation
 

documentation/config_docs.md

@@ -87,7 +87,7 @@ A **label rule** specifies whether or not a Slack channel should be notified, ba
 |-|-|-|-|
 | `match` | if notifications have any label in this list, they should be routed to the channel | Yes | all labels matched if no list provided |
 | `ignore` | if notifications have any label in this list, they shouldn't be routed to the channel (even if they have any `match` labels) | Yes | - |
-| `channel` | channel to use as webhook if the rule is matched | No | - |
+| `channel` | channel to notify if the rule is matched | No | - |
 
 ## Prefix Options
 
@@ -126,7 +126,7 @@ A **prefix rule** specifies whether or not a Slack channel should be notified, b
 |-|-|-|-|
 | `match` | if commit files have any prefix in this list, they should be routed to the channel | Yes | all prefixes matched if no list provided |
 | `ignore` | if commit files have any prefix in this list, they shouldn't be routed to the channel (even if they have any `match` prefixes) | Yes | - |
-| `channel` | channel to use as webhook if the rule is matched | No | - |
+| `channel` | channel to notify if the rule is matched | No | - |
 
 ## Status Options
 

documentation/secret_docs.md

@@ -8,44 +8,19 @@ A secrets file stores sensitive information. Unlike the repository configuration
 
 ```json
 {
-    "slack_hooks": [
-        {
-            "url": "https://slack_webhook_url",
-            "channel": "default"
-        },
-        {
-            "url": "https://slack_webhook_url",
-            "channel": "aa"
-        },
-        {
-            "url": "https://slack_webhook_url",
-            "channel": "backend"
-        },
-        {
-            "url": "https://slack_webhook_url",
-            "channel": "all-push-events"
-        },
-        {
-            "url": "https://slack_webhook_url",
-            "channel": "frontend-bot"
-        },
-        {
-            "url": "https://slack_webhook_url",
-            "channel": "aa-git"
-        },
-        {
-            "url": "https://slack_webhook_url",
-            "channel": "siren"
-        }
-    ]
+    "gh_token": "",
+    "slack_access_token": ""
 }
 ```
 
 | value | description | optional | default |
 |-|-|-|-|
-| `slack_hooks` | list of channel names (`channel`) and their corresponding webhook endpoint (`url`) | No | - |
 | `gh_token` | specify to grant the bot access to private repositories; omit for public repositories | Yes | - |
 | `gh_hook_token` | specify to ensure the bot only receives GitHub notifications from pre-approved repositories | Yes | - |
+| `slack_access_token` | slack bot access token to enable message posting to the workspace | Yes | try to use webhooks defined in `slack_hooks` instead |
+| `slack_hooks` | list of channel names and their corresponding webhook endpoint | Yes | try to use token defined in `slack_access_token` instead |
+
+Note that either `slack_access_token` or `slack_hooks` must be defined.
 
 ## `gh_token`
 
@@ -54,3 +29,29 @@ Some operations, such as fetching a config file from a private repository, or th
 ## `gh_hook_token`
 
 Refer [here](https://docs.github.com/en/free-pro-team@latest/developers/webhooks-and-events/securing-your-webhooks) for more information on securing webhooks with a token.
+
+## `slack_access_token`
+
+Refer [here](https://api.slack.com/authentication/oauth-v2) for obtaining an access token via OAuth.
+
+## `slack_hooks`
+
+*Note: If `slack_access_token` is also defined, the bot will authenticate over Slack's Web API and this option will not be used.*
+
+Expected format:
+
+```json
+[
+    {
+        "channel": "channel name",
+        "url": "webhook url"
+    },
+    {
+        "channel": "channel name",
+        "url": "webhook url"
+    },
+    ...
+]
+```
+
+Refer [here](https://api.slack.com/messaging/webhooks) for obtaining a webhook for a channel.

lib/action.ml

@@ -152,41 +152,29 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct
     let cfg = Context.get_config_exn ctx in
     match req with
     | Github.Push n ->
-      partition_push cfg n |> List.map ~f:(fun (webhook, n) -> webhook, generate_push_notification n) |> Lwt.return
-    | Pull_request n ->
-      partition_pr cfg n |> List.map ~f:(fun webhook -> webhook, generate_pull_request_notification n) |> Lwt.return
-    | PR_review n ->
-      partition_pr_review cfg n |> List.map ~f:(fun webhook -> webhook, generate_pr_review_notification n) |> Lwt.return
+      partition_push cfg n |> List.map ~f:(fun (channel, n) -> generate_push_notification n channel) |> Lwt.return
+    | Pull_request n -> partition_pr cfg n |> List.map ~f:(generate_pull_request_notification n) |> Lwt.return
+    | PR_review n -> partition_pr_review cfg n |> List.map ~f:(generate_pr_review_notification n) |> Lwt.return
     | PR_review_comment n ->
-      partition_pr_review_comment cfg n
-      |> List.map ~f:(fun webhook -> webhook, generate_pr_review_comment_notification n)
-      |> Lwt.return
-    | Issue n ->
-      partition_issue cfg n |> List.map ~f:(fun webhook -> webhook, generate_issue_notification n) |> Lwt.return
+      partition_pr_review_comment cfg n |> List.map ~f:(generate_pr_review_comment_notification n) |> Lwt.return
+    | Issue n -> partition_issue cfg n |> List.map ~f:(generate_issue_notification n) |> Lwt.return
     | Issue_comment n ->
-      partition_issue_comment cfg n
-      |> List.map ~f:(fun webhook -> webhook, generate_issue_comment_notification n)
-      |> Lwt.return
+      partition_issue_comment cfg n |> List.map ~f:(generate_issue_comment_notification n) |> Lwt.return
     | Commit_comment n ->
-      let%lwt webhooks, api_commit = partition_commit_comment ctx n in
-      let%lwt notif = generate_commit_comment_notification api_commit n in
-      let notifs = List.map ~f:(fun webhook -> webhook, notif) webhooks in
+      let%lwt channels, api_commit = partition_commit_comment ctx n in
+      let notifs = List.map ~f:(generate_commit_comment_notification api_commit n) channels in
       Lwt.return notifs
     | Status n ->
-      let%lwt webhooks = partition_status ctx n in
-      let notifs = List.map ~f:(fun webhook -> webhook, generate_status_notification cfg n) webhooks in
+      let%lwt channels = partition_status ctx n in
+      let notifs = List.map ~f:(generate_status_notification cfg n) channels in
       Lwt.return notifs
     | _ -> Lwt.return []
 
   let send_notifications (ctx : Context.t) notifications =
-    let notify (chan, msg) =
-      match Context.hook_of_channel ctx chan with
-      | None -> Printf.ksprintf action_error "webhook not defined for Slack channel '%s'" chan
-      | Some url ->
-        ( match%lwt Slack_api.send_notification ~chan ~msg ~url with
-        | Ok () -> Lwt.return_unit
-        | Error e -> action_error e
-        )
+    let notify (msg : Slack_t.post_message_req) =
+      match%lwt Slack_api.send_notification ~ctx ~msg with
+      | Ok () -> Lwt.return_unit
+      | Error e -> action_error e
     in
     Lwt_list.iter_s notify notifications
 

lib/api.ml

@@ -9,5 +9,5 @@ module type Github = sig
 end
 
 module type Slack = sig
-  val send_notification : chan:string -> msg:webhook_notification -> url:string -> (unit, string) Result.t Lwt.t
+  val send_notification : ctx:Context.t -> msg:post_message_req -> (unit, string) Result.t Lwt.t
 end

lib/api_local.ml

@@ -21,20 +21,18 @@ module Github : Api.Github = struct
 end
 
 module Slack : Api.Slack = struct
-  let send_notification ~chan ~msg ~url:_ =
-    let json =
-      msg |> Slack_j.string_of_webhook_notification |> Yojson.Basic.from_string |> Yojson.Basic.pretty_to_string
-    in
-    Stdio.printf "will notify #%s\n" chan;
+  let send_notification ~ctx:_ ~msg =
+    let json = msg |> Slack_j.string_of_post_message_req |> Yojson.Basic.from_string |> Yojson.Basic.pretty_to_string in
+    Stdio.printf "will notify #%s\n" msg.channel;
     Stdio.printf "%s\n" json;
     Lwt.return @@ Ok ()
 end
 
 module Slack_simple : Api.Slack = struct
   let log = Log.from "slack"
 
-  let send_notification ~chan ~msg ~url:_ =
-    log#info "will notify %s%s" chan
+  let send_notification ~ctx:_ ~(msg : Slack_t.post_message_req) =
+    log#info "will notify %s%s" msg.channel
       ( match msg.Slack_t.text with
       | None -> ""
       | Some s -> Printf.sprintf " with %S" s
@@ -45,9 +43,9 @@ end
 module Slack_json : Api.Slack = struct
   let log = Log.from "slack"
 
-  let send_notification ~chan ~msg ~url:_ =
-    let json = Slack_j.string_of_webhook_notification msg in
-    log#info "will notify %s" chan;
+  let send_notification ~ctx:_ ~(msg : Slack_t.post_message_req) =
+    log#info "will notify %s" msg.channel;
+    let json = Slack_j.string_of_post_message_req msg in
     let url = Uri.of_string "https://api.slack.com/docs/messages/builder" in
     let url = Uri.add_query_param url ("msg", [ json ]) in
     log#info "%s" (Uri.to_string url);

lib/api_remote.ml

@@ -50,12 +50,35 @@ end
 module Slack : Api.Slack = struct
   let log = Log.from "slack"
 
-  let send_notification ~chan ~msg ~url =
-    let data = Slack_j.string_of_webhook_notification msg in
-    let body = `Raw ("application/json", data) in
-    log#info "sending to %s : %s" chan data;
-    match%lwt http_request ~body `POST url with
-    | Ok _ -> Lwt.return @@ Ok ()
-    | Error e ->
-      Lwt.return @@ fmt_error "error while querying remote: %s\nfailed to send Slack notification to %s" e url
+  let bearer_token_header access_token = sprintf "Authorization: Bearer %s" (Uri.pct_decode access_token)
+
+  (** `send_notification ctx msg` notifies `msg.channel` with the payload `msg`;
+      uses web API with access token if available, or with webhook otherwise *)
+  let send_notification ~(ctx : Context.t) ~(msg : Slack_t.post_message_req) =
+    log#info "sending to %s" msg.channel;
+    let build_error e = fmt_error "%s\nfailed to send Slack notification" e in
+    let build_query_error url e = build_error @@ sprintf "error while querying %s: %s" url e in
+    let secrets = Context.get_secrets_exn ctx in
+    let headers, url, webhook_mode =
+      match secrets.slack_access_token with
+      | Some access_token -> [ bearer_token_header access_token ], Some "https://slack.com/api/chat.postMessage", false
+      | None -> [], Context.hook_of_channel ctx msg.channel, true
+    in
+    match url with
+    | None -> Lwt.return @@ build_error @@ sprintf "no token or webhook configured to notify channel %s" msg.channel
+    | Some url ->
+      let data = Slack_j.string_of_post_message_req msg in
+      let body = `Raw ("application/json", data) in
+      log#info "data: %s" data;
+      ( match%lwt http_request ~body ~headers `POST url with
+      (* error detection in response: slack uses status codes for webhooks versus a 200 code w/ `error` field for web api *)
+      | Ok s ->
+        if webhook_mode then Lwt.return @@ Ok ()
+        else (
+          let res = Slack_j.post_message_res_of_string s in
+          if res.ok then Lwt.return @@ Ok ()
+          else Lwt.return @@ build_query_error url (Option.value ~default:"an unknown error occurred" res.error)
+        )
+      | Error e -> Lwt.return @@ build_query_error url e
+      )
 end

lib/config.atd

@@ -39,7 +39,12 @@ type webhook = {
 (* This is the structure of the secrets file which stores sensitive information, and
    shouldn't be checked into version control.  *)
 type secrets = {
-  slack_hooks : webhook list;
-  ?gh_token : string option; (* GitHub personal access token, if repo access requires it *)
-  ?gh_hook_token : string option; (* GitHub webhook token to secure the webhook *)
+  (* GitHub personal access token, if repo access requires it *)
+  ?gh_token : string nullable;
+  (* GitHub webhook token to secure the webhook *)
+  ?gh_hook_token : string nullable;
+  (* list of Slack webhook & channel name pairs *)
+  ~slack_hooks <ocaml default="[]"> : webhook list;
+  (* Slack bot token obtained via OAuth, enabling message posting to the workspace *)
+  ?slack_access_token : string nullable;
 }

lib/context.ml

@@ -67,8 +67,14 @@ let refresh_secrets ctx =
   match get_local_file path with
   | Error e -> fmt_error "error while getting local file: %s\nfailed to get secrets from file %s" e path
   | Ok file ->
-    ctx.secrets <- Some (Config_j.secrets_of_string file);
-    Ok ctx
+    let secrets = Config_j.secrets_of_string file in
+    begin
+      match secrets.slack_access_token, secrets.slack_hooks with
+      | None, [] -> fmt_error "either slack_access_token or slack_hooks must be defined in file '%s'" path
+      | _ ->
+        ctx.secrets <- Some secrets;
+        Ok ctx
+    end
 
 let refresh_state ctx =
   match ctx.state_filepath with

lib/slack.atd

@@ -1,9 +1,9 @@
-type webhook_notification_field = {
+type message_field = {
   ?title: string nullable;
   value: string;
 }
 
-type webhook_notification_attachment = {
+type message_attachment = {
   fallback: string nullable;
   ?mrkdwn_in: string list nullable;
   ?color: string nullable;
@@ -14,18 +14,18 @@ type webhook_notification_attachment = {
   ?title: string nullable;
   ?title_link: string nullable;
   ?text: string nullable;
-  ?fields: webhook_notification_field list nullable;
+  ?fields: message_field list nullable;
   ?image_url: string nullable;
   ?thumb_url: string nullable;
   ?ts: int nullable;
   ?footer: string nullable;
 }
 
-type notification_section_block_type = [
+type message_section_block_type = [
   Section <json name="section">
 ] <ocaml repr="classic">
 
-type notification_divider_block_type = [
+type message_divider_block_type = [
   Divider <json name="divider">
 ] <ocaml repr="classic">
 
@@ -39,22 +39,29 @@ type text_object = {
   text: string;
 }
 
-type webhook_notification_text_block = {
-  notification_type <json name="type"> : notification_section_block_type;
+type message_text_block = {
+  message_type <json name="type"> : message_section_block_type;
   text: text_object;
 }
 
-type webhook_notification_divider_block = {
-  notification_type <json name="type"> : notification_divider_block_type;
+type message_divider_block = {
+  message_type <json name="type"> : message_divider_block_type;
 }
 
-type webhook_notification_block = [
-  Text of webhook_notification_text_block
-  | Divider of webhook_notification_divider_block
+type message_block = [
+  Text of message_text_block
+  | Divider of message_divider_block
 ] <json adapter.ocaml="Atdgen_runtime.Json_adapter.Type_field">
 
-type webhook_notification = {
+type post_message_req = {
+  channel: string;
   ?text: string nullable;
-  ?attachments: webhook_notification_attachment list nullable;
-  ?blocks: webhook_notification_block list nullable;
+  ?attachments: message_attachment list nullable;
+  ?blocks: message_block list nullable;
+}
+
+type post_message_res = {
+  ok: bool;
+  ?channel: string nullable;
+  ?error: string nullable;
 }