Notify endpoint should 404 with incorrect table name/id (#25627) (#25703)

src/metabase/api/notify.clj

@@ -32,9 +32,9 @@
                           (thunk)))]
     (api/let-404 [database (db/select-one Database :id id)]
       (cond
-        table_id   (when-let [table (db/select-one Table :db_id id, :id (int table_id))]
+        table_id   (api/let-404 [table (db/select-one Table :db_id id, :id (int table_id))]
                      (execute! #(table-sync-fn table)))
-        table_name (when-let [table (db/select-one Table :db_id id, :name table_name)]
+        table_name (api/let-404 [table (db/select-one Table :db_id id, :name table_name)]
                      (execute! #(table-sync-fn table)))
         :else      (execute! #(db-sync-fn database)))))
   {:success true})

test/metabase/api/notify_test.clj

@@ -19,17 +19,39 @@
       (is (= (get mw.util/response-forbidden :body)
              (client/client :post 403 "notify/db/100"))))))
 
+(def api-headers {:headers {"X-METABASE-APIKEY" "test-api-key"
+                            "Content-Type"      "application/json"}})
+
 (deftest not-found-test
-  (testing "POST /api/notify/db/:id"
-    (testing "database must exist or we get a 404"
-      (is (= {:status 404
-              :body   "Not found."}
-             (try (http/post (client/build-url (format "notify/db/%d" Integer/MAX_VALUE) {})
-                                    {:accept  :json
-                                     :headers {"X-METABASE-APIKEY" "test-api-key"
-                                               "Content-Type"      "application/json"}})
-                  (catch clojure.lang.ExceptionInfo e
-                    (select-keys (ex-data e) [:status :body]))))))))
+  (mt/with-temporary-setting-values [api-key "test-api-key"]
+    (testing "POST /api/notify/db/:id"
+      (testing "database must exist or we get a 404"
+        (is (= {:status 404
+                :body   "Not found."}
+               (try (http/post (client/build-url (format "notify/db/%d" Integer/MAX_VALUE) {})
+                               (merge {:accept :json} api-headers))
+                    (catch clojure.lang.ExceptionInfo e
+                      (select-keys (ex-data e) [:status :body]))))))
+      (testing "table ID must exist or we get a 404"
+        (is (= {:status 404
+                :body   "Not found."}
+               (try (http/post (client/build-url (format "notify/db/%d" (:id (mt/db))) {})
+                               (merge {:accept       :json
+                                       :content-type :json
+                                       :form-params  {:table_id Integer/MAX_VALUE}}
+                                      api-headers))
+                    (catch clojure.lang.ExceptionInfo e
+                      (select-keys (ex-data e) [:status :body]))))))
+      (testing "table name must exist or we get a 404"
+        (is (= {:status 404
+                :body   "Not found."}
+               (try (http/post (client/build-url (format "notify/db/%d" (:id (mt/db))) {})
+                               (merge {:accept       :json
+                                       :content-type :json
+                                       :form-params  {:table_name "IncorrectToucanFact"}}
+                                      api-headers))
+                    (catch clojure.lang.ExceptionInfo e
+                      (select-keys (ex-data e) [:status :body])))))))))
 
 (deftest post-db-id-test
   (binding [api.notify/*execute-asynchronously* false]
@@ -38,11 +60,10 @@
             post       (fn post-api
                          ([payload] (post-api payload 200))
                          ([payload expected-code]
-                          (mt/client :post expected-code (format "notify/db/%d" (u/the-id (mt/db)))
-                                     {:request-options
-                                      {:headers {"X-METABASE-APIKEY" "test-api-key"
-                                                 "Content-Type"      "application/json"}}}
-                                     payload)))]
+                          (mt/with-temporary-setting-values [api-key "test-api-key"]
+                            (mt/client :post expected-code (format "notify/db/%d" (u/the-id (mt/db)))
+                                       {:request-options api-headers}
+                                       payload))))]
         (testing "sync just table when table is provided"
           (let [long-sync-called? (atom false), short-sync-called? (atom false)]
             (with-redefs [metabase.sync/sync-table!                        (fn [_table] (reset! long-sync-called? true))

test/metabase/server/middleware/auth_test.clj

@@ -91,10 +91,10 @@
 
 (deftest wrap-api-key-test
   (testing "No API key in the request"
-    (is (= nil
-           (:metabase-session-id
-            (wrapped-api-key-handler
-             (ring.mock/request :get "/anyurl"))))))
+    (is (nil?
+         (:metabase-session-id
+          (wrapped-api-key-handler
+           (ring.mock/request :get "/anyurl"))))))
 
   (testing "API Key in header"
     (is (= "foobar"