More backend changes to support Slack app integrations (#19592)

resources/slack-manifest.yaml

+_metadata:
+  major_version: 1
+  minor_version: 1
+display_information:
+  name: Metabase
+  description: Bringing the power of Metabase to your Slack #channels!
+  background_color: "#509EE3"
+features:
+  bot_user:
+    display_name: Metabase
+oauth_config:
+  scopes:
+    bot:
+      - users:read
+      - channels:read
+      - channels:join
+      - files:write
+      - chat:write
+      - chat:write.customize
+      - chat:write.public

src/metabase/api/slack.clj

 (ns metabase.api.slack
   "/api/slack endpoints"
-  (:require [compojure.core :refer [PUT]]
+  (:require [clojure.java.io :as io]
+            [compojure.core :refer [PUT]]
             [metabase.api.common :as api]
             [metabase.config :as config]
             [metabase.integrations.slack :as slack]
@@ -10,21 +11,34 @@
 
 (api/defendpoint PUT "/settings"
   "Update Slack related settings. You must be a superuser to do this."
-  [:as {{slack-app-token :slack-app-token} :body}]
-  {slack-app-token     (s/maybe su/NonBlankString)}
+  [:as {{slack-app-token :slack-app-token, slack-files-channel :slack-files-channel} :body}]
+  {slack-app-token     (s/maybe su/NonBlankString)
+   slack-files-channel (s/maybe su/NonBlankString)}
   (api/check-superuser)
-  (if-not slack-app-token
-    (slack/slack-app-token nil)
-    (try
-      (when-not config/is-test?
-        (when-not (slack/valid-token? slack-app-token)
-          (throw (ex-info (tru "Invalid Slack token.")
-          {:errors {:slack-app-token (tru "invalid token")}}))))
-      ;; Clear the deprecated `slack-token` when setting a new `slack-app-token`
-      (slack/slack-token nil)
-      (slack/slack-app-token slack-app-token)
-      {:ok true}
-      (catch clojure.lang.ExceptionInfo info
-        {:status 400, :body (ex-data info)}))))
+  (try
+    (when (and slack-app-token (not config/is-test?))
+      (when-not (slack/valid-token? slack-app-token)
+        (throw (ex-info (tru "Invalid Slack token.")
+                        {:errors {:slack-app-token (tru "invalid token")}
+                         :status-code 400}))))
+    (slack/slack-app-token slack-app-token)
+    (when slack-app-token
+      (do
+        (slack/slack-token-valid? true)
+        ;; Clear the deprecated `slack-token` when setting a new `slack-app-token`
+        (slack/slack-token nil)))
+    (slack/slack-files-channel slack-files-channel)
+    {:ok true}
+    (catch clojure.lang.ExceptionInfo info
+      {:status 400, :body (ex-data info)})))
+
+(def ^:private slack-manifest
+  (delay (slurp (io/resource "slack-manifest.yaml"))))
+
+(api/defendpoint GET "/manifest"
+  "Returns the YAML manifest file that should be used to bootstrap new Slack apps"
+  []
+  (api/check-superuser)
+  @slack-manifest)
 
 (api/define-routes)

src/metabase/email/messages.clj

@@ -651,3 +651,15 @@
   [alert user {:keys [first_name last_name] :as archiver}]
   (let [edited-text (format "the question was edited by %s %s" first_name last_name)]
     (send-email! user not-working-subject stopped-template (assoc (common-alert-context alert) :deletionCause edited-text))))
+
+(defn send-slack-token-error-emails!
+  "Email all admins when a Slack API call fails due to a revoked token or other auth error"
+  []
+  (email/send-message!
+   :subject (trs "Your Slack connection stopped working")
+   :recipients (all-admin-recipients)
+   :message-type :html
+   :message (stencil/render-file "metabase/email/slack_token_error.mustache"
+                                 (merge (common-context)
+                                        {:logoHeader  true
+                                         :settingsUrl (str (public-settings/site-url) "/admin/settings/slack")}))))

src/metabase/email/slack_token_error.mustache

+{{> metabase/email/_header }}
+  <div style="text-align: center; margin: 0 auto; max-width: 500px">
+    <div style="text-align: left">
+      <h2 style="font-weight: normal; color: #4C545B; line-height: 34px;">Your Slack connection stopped working</h2>
+      <p style="line-height: 22px; margin-bottom: 40px">This may affect existing dashboard subscriptions. Follow the steps in settings to reconnect Slack and get things up and running again.</p>
+    </div>
+    <a style="{{buttonStyle}}" href="{{settingsUrl}}">
+      Go to settings
+    </a>
+  </div>
+{{> metabase/email/_footer }}

src/metabase/integrations/slack.clj

@@ -3,8 +3,10 @@
             [clj-http.client :as http]
             [clojure.core.memoize :as memoize]
             [clojure.java.io :as io]
+            [clojure.string :as str]
             [clojure.tools.logging :as log]
             [medley.core :as m]
+            [metabase.email.messages :as messages]
             [metabase.models.setting :as setting :refer [defsetting]]
             [metabase.util :as u]
             [metabase.util.i18n :refer [deferred-tru trs tru]]
@@ -22,26 +24,62 @@
        " "
        (deferred-tru "This should be used for all new Slack integrations starting in Metabase v0.42.0.")))
 
+(defsetting slack-token-valid?
+  (str (deferred-tru "Whether the current Slack app token, if set, is valid.")
+       " "
+       (deferred-tru "Set to 'false' if a Slack API request returns an auth error."))
+  :type :boolean)
+
+(defsetting slack-files-channel
+  (deferred-tru "The name of the channel to which Metabase files should be initially uploaded")
+  :default "metabase_files"
+  :setter (fn [channel]
+            (if (str/blank? channel)
+              (setting/set-value-of-type! :string :slack-files-channel nil)
+              ;; Strip leading # if present, since the Slack API doesn't like it
+              (setting/set-value-of-type! :string
+                                          :slack-files-channel
+                                          (if (str/starts-with? channel "#") (subs channel 1) channel)))))
+
 (def ^:private ^String slack-api-base-url "https://slack.com/api")
-(def ^:private ^String files-channel-name "metabase_files")
 
 (defn slack-configured?
   "Is Slack integration configured?"
   []
   (boolean (or (seq (slack-app-token)) (seq (slack-token)))))
 
+(def ^:private slack-token-error-codes
+  "List of error codes that indicate an invalid or revoked Slack token."
+  ;; If any of these error codes are received from the Slack API, we send an email to all admins indicating that the
+  ;; Slack integration is broken. In practice, the "account_inactive" error code is the one that is most likely to be
+  ;; received. This would happen if access to the Slack workspace is manually revoked via the Slack UI.
+  #{"invalid_auth", "account_inactive", "token_revoked", "token_expired"})
+
+(def ^:private ^:dynamic *send-token-error-emails?*
+  "Whether to send an email to all admins when an invalid or revoked token error is received in response to a Slack
+  API call. Should be set to false when checking if an unsaved token is valid. (Default: `true`)"
+  true)
+
 (defn- handle-error [body]
-  (let [invalid-token? (= (:error body) "invalid_auth")
+  (let [invalid-token? (slack-token-error-codes (:error body))
         message        (if invalid-token?
-                         (tru "Invalid token")
-                         (tru "Slack API error: {0}" (:error body)))
+                         (trs "Invalid token")
+                         (trs "Slack API error: {0}" (:error body)))
         error          (if invalid-token?
                          {:error-code (:error body)
                           :errors     {:slack-token message}}
                          {:error-code (:error body)
                           :message    message
                           :response   body})]
-    (log/warn (u/pprint-to-str 'red error))
+    (when (and invalid-token? *send-token-error-emails?*)
+      ;; Check `slack-token-valid?` before sending emails to avoid sending repeat emails for the same invalid token.
+      ;; We should send an email if `slack-token-valid?` is `true` or `nil` (i.e. a pre-existing bot integration is
+      ;; being used)
+      (when (not (false? (slack-token-valid?))) (messages/send-slack-token-error-emails!))
+      (slack-token-valid? false))
+    (if invalid-token?
+      (log/warn (u/pprint-to-str 'red (trs "🔒 Your Slack authorization token is invalid or has been revoked. Please update your integration in Admin Settings -> Slack.")))
+      (log/warn (u/pprint-to-str 'red error)))
     (throw (ex-info message error))))
 
 (defn- handle-response [{:keys [status body]}]
@@ -129,9 +167,10 @@
   "Check whether a Slack token is valid by checking whether we can call `conversations.list` with it."
   [token :- su/NonBlankString]
   (try
-    (boolean (take 1 (conversations-list :limit 1, :token token)))
+    (binding [*send-token-error-emails?* false]
+      (boolean (take 1 (conversations-list :limit 1, :token token))))
     (catch Throwable e
-      (if (= (:error-code (ex-data e)) "invalid_auth")
+      (if (slack-token-error-codes (:error-code (ex-data e)))
         false
         (throw e)))))
 
@@ -144,19 +183,7 @@
        (filter (complement :deleted))
        (filter (complement :is_bot))))
 
-(defn- files-channel* []
-  (or (channel-with-name files-channel-name)
-      (let [message (str (tru "Slack channel named `metabase_files` is missing!")
-                         " "
-                         (tru "Please create or unarchive the channel in order to complete the Slack integration.")
-                         " "
-                         (tru "The channel is used for storing images that are included in dashboard subscriptions."))]
-        (log/error (u/format-color 'red message))
-        (throw (ex-info message {:status-code 400})))))
-
-(def ^{:arglists '([])} files-channel
-  "Calls Slack api `channels.info` to check whether a channel named #metabase_files exists. If it doesn't, throws an
-  error that advices an admin to create it."
+(def ^:private ^{:arglists '([channel-name])} files-channel*
   ;; If the channel has successfully been created we can cache the information about it from the API response. We need
   ;; this information every time we send out a pulse, but making a call to the `conversations.list` endpoint everytime we
   ;; send a Pulse can result in us seeing 429 (rate limiting) status codes -- see
@@ -164,7 +191,27 @@
   ;;
   ;; Of course, if `files-channel*` *fails* (because the channel is not created), this won't get cached; this is what
   ;; we want -- to remind people to create it
-  (memoize/ttl files-channel* :ttl/threshold (u/hours->ms 6)))
+  ;;
+  ;; The memoized function is paramterized by the channel name so that if the name is changed, the cached channel details
+  ;; will be refetched.
+  (memoize/ttl
+   (fn [channel-name]
+     (or (when channel-name (channel-with-name channel-name))
+         (let [message (str (tru "Slack channel named `{0}` is missing!" channel-name)
+                            " "
+                            (tru "Please create or unarchive the channel in order to complete the Slack integration.")
+                            " "
+                            (tru "The channel is used for storing images that are included in dashboard subscriptions."))]
+           (log/error (u/format-color 'red message))
+           (throw (ex-info message {:status-code 400})))))
+   :ttl/threshold (u/hours->ms 6)))
+
+(defn files-channel
+  "Calls Slack api `channels.info` to check whether a channel exists with the expected name from the
+  [[slack-files-channel]] setting. If it does, returns the channel details as a map. If it doesn't, throws an error
+  that advices an admin to create it."
+  []
+  (files-channel* (slack-files-channel)))
 
 (def ^:private NonEmptyByteArray
   (s/constrained

test/metabase/api/slack_test.clj

 (ns metabase.api.slack-test
-  (:require [clojure.test :refer :all]
+  (:require [clojure.string :as str]
+            [clojure.test :refer :all]
+            [metabase.config :as config]
             [metabase.integrations.slack :as slack]
             [metabase.test :as mt]))
 
@@ -13,4 +15,46 @@
           (mt/user-http-request :crowberto :put 200 "slack/settings"
                                 {:slack-app-token "fake-token"})
           (is (= "fake-token" (slack/slack-app-token)))
-          (is (= nil (slack/slack-token))))))))
+          (is (= nil (slack/slack-token))))))
+
+    (testing "A 400 error is returned if the Slack app token is invalid"
+      (with-redefs [slack/valid-token? (constantly false)
+                    ;; Token validation is skipped by default in test environments; overriding `is-test?` ensures
+                    ;; that validation occurs
+                    config/is-test?    false]
+        (mt/user-http-request :crowberto :put 400 "slack/settings"
+                              {:slack-app-token "fake-token"})))
+
+    (testing "The Slack files channel setting can be set by an admin, and the leading # is stripped if it is present"
+      (mt/with-temporary-setting-values [slack-files-channel nil]
+        (mt/user-http-request :crowberto :put 200 "slack/settings"
+                              {:slack-files-channel "fake-channel"})
+        (is (= "fake-channel" (slack/slack-files-channel)))
+
+        (mt/user-http-request :crowberto :put 200 "slack/settings"
+                              {:slack-files-channel "#fake-channel"})
+        (is (= "fake-channel" (slack/slack-files-channel)))))
+
+    (testing "The Slack app token or files channel settings are cleared if no value is sent in the request"
+      (mt/with-temporary-setting-values [slack-app-token "fake-token"
+                                         slack-files-channel "fake-channel"]
+        (mt/user-http-request :crowberto :put 200 "slack/settings" {})
+        (is (= nil (slack/slack-app-token)))
+        ;; The files channel is reset to its default value
+        (is (= "metabase_files" (slack/slack-files-channel)))))
+
+    (testing "A non-admin cannot modify the Slack app token or files channel settings"
+      (mt/user-http-request :rasta :put 403 "slack/settings"
+                            {:slack-app-token "fake-token"})
+      (mt/user-http-request :rasta :put 403 "slack/settings"
+                            {:slack-files-channel "fake-channel"}))))
+
+(deftest manifest-test
+  (testing "GET /api/slack/manifest"
+    (testing "The Slack manifest can be fetched via an API call"
+      (is (str/starts-with?
+           (mt/user-http-request :crowberto :get 200 "slack/manifest")
+           "_metadata:\n")))
+
+    (testing "A non-admin cannot fetch the Slack manifest"
+      (mt/user-http-request :rasta :get 403 "slack/manifest"))))

test/metabase/integrations/slack_test.clj

@@ -5,7 +5,9 @@
             [clojure.java.io :as io]
             [clojure.test :refer :all]
             [medley.core :as m]
+            [metabase.email.messages :as messages]
             [metabase.integrations.slack :as slack]
+            [metabase.test :as mt]
             [metabase.test.util :as tu]
             [schema.core :as s])
   (:import java.nio.charset.Charset
@@ -141,7 +143,7 @@
                  (slack/users-list))))))))
 
 (defn- mock-files-channel []
-  (let [channel-name @#'slack/files-channel-name]
+  (let [channel-name (slack/slack-files-channel)]
     (-> (mock-conversations)
         first
         (assoc
@@ -208,3 +210,38 @@
                                            slack-token nil]
           (is (schema= expected-schema
                        (slack/post-chat-message! "C94712B6X" ":wow:"))))))))
+
+(deftest slack-token-error-test
+  (with-redefs [messages/all-admin-recipients (constantly ["crowberto@metabase.com"])]
+    (tu/with-temporary-setting-values [slack-app-token "test-token"
+                                       slack-token-valid? true]
+      (mt/with-fake-inbox
+        (http-fake/with-fake-routes {#"^https://slack.com/api/chat\.postMessage.*"
+                                     (fn [_] (mock-200-response {:ok false, :error "account_inactive"}))}
+          (testing "If a slack token is revoked, an email should be sent to admins, and the `slack-token-valid?` setting
+             should be set to false"
+            (try
+              (slack/post-chat-message! "C94712B6X" ":wow:")
+              (catch Throwable e
+                (is (= "Invalid token" (ex-message e)))
+                (is (= (mt/email-to :crowberto {:subject "Your Slack connection stopped working"
+                                                :to #{"crowberto@metabase.com"}
+                                                :body [{"Your Slack connection stopped working." true}]})
+                       (mt/summarize-multipart-email #"Your Slack connection stopped working.")))
+                (is (false? (slack/slack-token-valid?))))))
+
+          (testing "If `slack-token-valid?` is already false, no email should be sent"
+            (mt/reset-inbox!)
+            (try
+              (slack/post-chat-message! "C94712B6X" ":wow:")
+              (catch Throwable e
+                (is (= "Invalid token" (ex-message e)))
+                (is (= {} (mt/summarize-multipart-email #"Your Slack connection stopped working.")))))))
+
+        (testing "No email is sent during token validation checks, even if `slack-token-valid?` is currently true"
+          (tu/with-temporary-setting-values [slack-token-valid? true]
+            (http-fake/with-fake-routes {conversations-endpoint (fn [_] (mock-200-response {:ok false, :error "account_inactive"}))}
+              (mt/reset-inbox!)
+              (is (= false (slack/valid-token? "abc")))
+              (is (= {} (mt/summarize-multipart-email #"Your Slack connection stopped working.")))
+              (is (slack/slack-token-valid?)))))))))