Adapt to metabase 50

- adaptations for toucan 2 - remove backend code linter git hook - configure stratio modules in clj-kondo ns-module-checker - fix clj-kondo linter warnings - fix in front to make it show our "editing user first namd is forbidden" message - small adaptations in stratio dockerfile
grios-stratio · Aug 22, 2024 · ab79bb0 · ab79bb0
1 parent 37a3a7c
commit ab79bb0
Show file tree

Hide file tree

Showing 9 changed files with 115 additions and 99 deletions.
diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn
@@ -236,6 +236,8 @@
                                      metabase.shared.util.internal.time-common
                                      metabase.shared.util.namespaces
                                      metabase.shared.util.time}                                     ; TODO -- consolidate these into a real API namespace.
+    metabase.stratio               #{metbase.stratio
+                                     metabase.stratio.middleware}
     metabase.sync                  #{metabase.sync
                                      metabase.sync.analyze
                                      metabase.sync.concurrent
@@ -306,6 +308,7 @@
     metabase.server                :any
     metabase.setup                 :any
     metabase.shared                :any
+    metabase.stratio               :any
     metabase.sync                  :any
     metabase.task                  :any
     metabase.troubleshooting       :any

diff --git a/frontend/src/metabase/redux/user.ts b/frontend/src/metabase/redux/user.ts
@@ -43,7 +43,11 @@ export const currentUser = createReducer<User | null>(null, builder => {
       return state;
     })
     .addCase(Users.actionTypes.UPDATE, (state, { payload }) => {
-      const isCurrentUserUpdated = state?.id === payload.user.id;
+      // < STRATIO - forbid editing username, wihtout this change our error message does not show,
+      // instead, a "cannot read properties of undefined" error appears
+      // const isCurrentUserUpdated = state?.id === payload.user.id;
+      const isCurrentUserUpdated = state?.id === (payload.user?.id || -1);
+      // STRATIO >
       if (isCurrentUserUpdated) {
         return {
           ...state,

diff --git a/package.json b/package.json
@@ -419,10 +419,6 @@
     ],
     "e2e/test/scenarios/*/{*.(js|ts),!(helpers|shared)/*.(js|ts)}": [
       "node e2e/validate-e2e-test-files.js"
-    ],
-    "**/*.{clj,cljc,cljs,bb}": [
-      "clj-kondo --config ./.clj-kondo/config.edn --config-dir ./.clj-kondo --parallel --lint",
-      "./bin/whitespace_lint_staged.sh"
     ]
   },
   "browserslist": [

diff --git a/src/metabase/stratio/auth.clj b/src/metabase/stratio/auth.clj
@@ -1,32 +1,27 @@
 (ns metabase.stratio.auth
-  (:require [clojure.set :as set]
-            [clojure.string :as str]
-            [clojure.tools.logging :as log]
-            [metabase.api.session :as session]
-            [metabase.integrations.common :as integrations]
-            [metabase.models
-             [permissions-group :as group :refer [PermissionsGroup]]
-             [user :as user :refer [User]]]
-            [metabase.stratio
-             [config :as st.config]
-             [header-user-info :refer [http-headers->user-info]]
-             [util :as st.util]]
-            [metabase.server.request.util :as request.u]
-            [metabase.util :as u]
-            [toucan.db :as db]
-            [toucan.hydrate :refer [hydrate]])
-  (:import java.util.UUID))
+  (:require
+   [clojure.set :as set]
+   [metabase.api.session :as api.session]
+   [metabase.integrations.common :as integrations]
+   [metabase.models.permissions-group :as perms-group]
+   [metabase.server.request.util :as req.util]
+   [metabase.stratio.config :as st.config]
+   [metabase.stratio.header-user-info :refer [http-headers->user-info]]
+   [metabase.stratio.util :as st.util]
+   [metabase.util :as u]
+   [metabase.util.log :as log]
+   [toucan2.core :as t2]))
 
-(def dummy-email-domain      (st.config/config-str :dummy-email-domain))
-(def create-and-sync-groups? (st.config/config-bool :create-and-sync-groups))
-(def admin-group             (st.config/config-str :admin-group))
-(def whitelist               (-> :allowed-groups
-                                 st.config/config-vector
-                                 (conj admin-group)
-                                 ((partial remove empty?))
-                                 set))
-(def whitelist-enabled?      (st.config/config-bool :use-group-whitelist))
-(def whitelist-disabled?     (not whitelist-enabled?))
+(def ^:private dummy-email-domain      (st.config/config-str :dummy-email-domain))
+(def ^:private create-and-sync-groups? (st.config/config-bool :create-and-sync-groups))
+(def ^:private admin-group             (st.config/config-str :admin-group))
+(def ^:private whitelist               (-> :allowed-groups
+                                           st.config/config-vector
+                                           (conj admin-group)
+                                           ((partial remove empty?))
+                                           set))
+(def ^:private whitelist-enabled?      (st.config/config-bool :use-group-whitelist))
+(def ^:private whitelist-disabled?     (not whitelist-enabled?))
 
 (defn- allowed?
   [groups]
@@ -42,8 +37,8 @@
   [groups superuser?]
   (cond-> (set groups)
     whitelist-enabled? (set/intersection whitelist)
-    true               (disj group/admin-group-name)  ;; prevent a SSO "Administrators" group to trigger admin status
-    superuser?         (conj group/admin-group-name)))
+    true               (disj perms-group/admin-group-name)  ;; prevent a SSO "Administrators" group to trigger admin status
+    superuser?         (conj perms-group/admin-group-name)))
 
 (defn- allowed-user
   [{:keys [user groups error]}]
@@ -60,13 +55,15 @@
 (defn- insert-new-user!
   "Creates a new user, defaulting the password when not provided"
   [new-user]
-  (db/insert! User (update new-user :password #(or % (str (UUID/randomUUID))))))
+  (t2/insert-returning-instance! :model/User (update new-user :password #(or % (str (random-uuid))))))
 
 (defn- group-name->group-id []
-  (db/select-field->id :name PermissionsGroup))
+  (t2/select-fn->pk :name :model/PermissionsGroup))
+
+(map (fn [x] {:a x}) [1 2 3])
 
 (defn- insert-groups! [group-names]
-  (db/insert-many! PermissionsGroup (map (fn [name] {:name name}) group-names)))
+  (t2/insert-returning-pks! :model/PermissionsGroup (map (fn [name] {:name name}) group-names)))
 
 (defn- create-and-sync-groups!
   [user-id group-names]
@@ -78,38 +75,39 @@
                                     (map group-name->group-id)
                                     (filter some?))
           user-group-ids       (concat existing-group-ids created-group-ids)
-          all-metabase-groups  (db/select-ids PermissionsGroup)]
+          all-metabase-groups  (t2/select-pks-set :model/PermissionsGroup)]
       (integrations/sync-group-memberships! user-id user-group-ids all-metabase-groups))
     (catch Exception e
       (log/error "Could not create and sync groups. Error:" (st.util/stack-trace e)))))
 
 (defn- fetch-or-create-user!
   [{first_name :first_name {groups :groups} :login_attributes superuser? :is_superuser, :as allowed-user}]
-  (or (if-let [user-in-db (db/select-one [User :id :last_login :is_superuser] :first_name first_name)]
-        (do
-          ;; Check if superuser status has changed and update if necessary
-          (if (or (apply not= (map :is_superuser     [user-in-db allowed-user]))
+  (or (when-let [user-in-db (t2/select-one :model/User :first_name first_name)]
+        ;; Check if superuser status has changed and update if necessary
+        (when (or (apply not= (map :is_superuser [user-in-db allowed-user]))
                   (apply not= (map :login_attributes [user-in-db allowed-user])))
-            (db/update! User (:id user-in-db)
-                        :is_superuser     superuser?
-                        :login_attributes (:login_attributes allowed-user)))
-          (if create-and-sync-groups?
-            (create-and-sync-groups! (:id user-in-db) (effective-groups groups superuser?)))
-          user-in-db))
+          (t2/update! :model/User (:id user-in-db) {:is_superuser superuser?
+                                                    :login_attributes (:login_attributes allowed-user)}))
+        (when create-and-sync-groups?
+          (create-and-sync-groups! (:id user-in-db) (effective-groups groups superuser?)))
+        user-in-db)
       (let [user-inserted (insert-new-user! allowed-user)]
-        (if create-and-sync-groups?
+        (when create-and-sync-groups?
           (create-and-sync-groups! (:id user-inserted) (effective-groups groups superuser?)))
         user-inserted)))
 
 (defn create-session-from-headers!
+  "Reads the SSO user info in the request (either as jwt or as plain headers) and returs a 'user' (a map with some
+  user-related keys, including a valid Metbase session in :session. If the user does not exists in the Metabse DB,
+  it is created, and optionally, their groups are also created and synced."
   [{headers :headers, :as request}]
   (let [user-info    (http-headers->user-info headers)
         allowed-user (allowed-user user-info)]
     (log/debug "received user info " user-info)
     (if (:error allowed-user)
       allowed-user
       (try
-        (let [session (session/create-session! :sso (fetch-or-create-user! allowed-user) (request.u/device-info request))]
+        (let [session (api.session/create-session! :sso (fetch-or-create-user! allowed-user) (req.util/device-info request))]
           (assoc allowed-user :session session))
         (catch Exception e
           {:error (st.util/stack-trace e)})))))

diff --git a/src/metabase/stratio/config.clj b/src/metabase/stratio/config.clj
@@ -1,8 +1,10 @@
 (ns metabase.stratio.config
-  (:require [clojure.string :as str]
-            [metabase.config :as config]
-            [metabase.models.setting :refer [defsetting]]
-            [metabase.stratio.util :as st.util]))
+  (:require
+   [metabase.config :as config]
+   [metabase.models.setting :refer [defsetting]]
+   [metabase.stratio.util :as st.util]))
+
+(set! *warn-on-reflection* true)
 
 (def ^:private stratio-defaults
   {:authenticator "gosec-sso"
@@ -25,7 +27,6 @@
    :admin-group ""
    :create-and-sync-groups "true"})
 
-
 (defn config-str  [k] (or (config/config-str k) ((keyword k) stratio-defaults)))
 (defn config-int  [k] (some-> k config-str Integer/parseInt))
 (defn config-bool [k] (some-> k config-str Boolean/parseBoolean))
@@ -44,4 +45,5 @@
   :type        :boolean
   :default     gosec-sso?
   :visibility  :public
-  :setter      :none)
+  :setter      :none
+  :export?     false)
diff --git a/src/metabase/stratio/header_user_info.clj b/src/metabase/stratio/header_user_info.clj
@@ -1,17 +1,17 @@
 (ns metabase.stratio.header-user-info
-  (:require [buddy.sign.jwt :as jwt]
-            [buddy.core.keys :as keys]
-            [buddy.core.codecs :as codecs]
-            [buddy.core.codecs.base64 :as b64]
-            [clj-http.client :as http]
-            [clj-time.core :as time]
-            [clojure.string :as str]
-            [cheshire.core :as json]
-            [metabase.config :as config]
-            [metabase.stratio
-             [config :as st.config]
-             [util :as st.util]]
-            [clojure.tools.logging :as log]))
+  (:require
+   [buddy.core.codecs :as codecs]
+   [buddy.core.codecs.base64 :as b64]
+   [buddy.core.keys :as keys]
+   [buddy.sign.jwt :as jwt]
+   [cheshire.core :as json]
+   [clj-http.client :as http]
+   [clojure.string :as str]
+   [metabase.config :as config]
+   [metabase.stratio.config :as st.config]
+   [metabase.stratio.util :as st.util]
+   [metabase.util :as u]
+   [metabase.util.log :as log]))
 
 (defn- split-token
   [token]
@@ -32,8 +32,8 @@
           header       (parse-data header-b64)
           alg          (:alg header)]
       (cond-> header
-        alg (assoc :alg (keyword (str/lower-case alg)))))
-    (catch com.fasterxml.jackson.core.JsonParseException e
+        alg (assoc :alg (keyword (u/lower-case-en alg)))))
+    (catch com.fasterxml.jackson.core.JsonParseException _
       (throw (ex-info "Message seems corrupt or manipulated."
                {:type :validation :cause :header})))))
 
@@ -46,7 +46,7 @@
 (defn- http-header->jwt-token
   [headers]
   (let [header-name       (st.config/config-str :jwt-header-name)
-        header-name-lower (str/lower-case header-name)]
+        header-name-lower (u/lower-case-en header-name)]
     (cond
       (contains? headers header-name)       (get headers header-name)
       (contains? headers header-name-lower) (get headers header-name-lower)
@@ -88,7 +88,7 @@
         pkey (let [info (-> token
                             (verify-token pkey)
                             (select-keys [username-claim groups-claim])
-                            (update-in [groups-claim] st.util/make-vector))
+                            (update groups-claim st.util/make-vector))
                    user-name (username-claim info)]
                (if (empty? user-name)
                  {:error "No username claim found in token"}

diff --git a/src/metabase/stratio/middleware.clj b/src/metabase/stratio/middleware.clj
@@ -1,27 +1,30 @@
 (ns metabase.stratio.middleware
   (:require
    [clojure.string :as str]
-   [clojure.tools.logging :as log]
+   [java-time.api :as t]
    [metabase.api.common :as api]
-   [metabase.models.user :refer [User]]
    [metabase.server.middleware.session :as mw.session]
-   [metabase.stratio
-    [auth :as st.auth]
-    [config :as st.config]]
-   [toucan.db :as db]))
+   [metabase.stratio.auth :as st.auth]
+   [metabase.stratio.config :as st.config]
+   [metabase.util :as u]
+   [metabase.util.log :as log]
+   [toucan2.core :as t2]))
 
+(set! *warn-on-reflection* true)
 
-(def ^:dynamic ^Boolean *is-sync-request?* false)
+(def ^:dynamic ^Boolean *is-sync-request?*
+  "Dynamic variable set by the sync endpoint to let us know if we are handling a sync database request"
+  false)
 
-(def public-api-endpoints
+(def ^:private public-api-endpoints
   ["/api/embed" "/api/geojson" "/api/public" "/api/setup" "/api/util" "/api/session/properties" "/api/health"])
 
 (defn- email-login-request?
-  [{:keys [:request-method :uri]}]
+  [{:keys [request-method uri]}]
   (and (= uri "/api/session") (= request-method :post)))
 
 (defn- sync-db-request?
-  [{:keys [:request-method :uri]}]
+  [{:keys [request-method uri]}]
   (boolean
    (and
     (= request-method :post)
@@ -30,18 +33,18 @@
 
 (defn- editing-user-name?
   "The username of an existing user should never be edited"
-  [{:keys [:uri :request-method :body]}]
+  [{:keys [uri request-method body]}]
   (when (and (re-matches #"/api/user/[0-9]+/?" uri) (= request-method :put))
     (let [user-id (Integer/parseInt (last (str/split uri #"/")))
-          old-username (db/select-one-field :first_name User :id user-id)
+          old-username (t2/select-one-fn :first_name :model/User :id user-id)
           new-username (:first_name body)]
       (and old-username (not= old-username new-username)))))
 
 (defn- add-session-to-request-and-response
   [handler session]
   (fn [request respond raise]
-    (handler (assoc request :metabase-session-id (-> session :id str))
-             #(respond (mw.session/set-session-cookie request % session))
+    (handler (assoc request :metabase-session-id (-> session :id str) :metabase-session-type :normal)
+             #(respond (mw.session/set-session-cookies request % session (t/zoned-date-time (t/zone-id "GMT"))))
              raise)))
 
 (defn- forbid-email-login
@@ -78,7 +81,7 @@
                                     (add-session-to-request-and-response session))]
             (log/info "User" first_name "auto-logged-in through headers")
             (log/debug "Request triggering the auto-login:"
-                       (str/upper-case (name (:request-method request)))
+                       (u/upper-case-en (name (:request-method request)))
                        (:uri request))
             (wrapped-handler request respond raise)))))))
 
@@ -113,6 +116,7 @@
       forbid-email-login))
 
 (def stratio-middleware
+  "Packs all needed middleware depending on login strategy"
   (if st.config/should-auto-login?
     (comp auto-login-middleware default-middleware)
     default-middleware))

diff --git a/src/metabase/stratio/util.clj b/src/metabase/stratio/util.clj
@@ -1,16 +1,19 @@
 (ns metabase.stratio.util
-  (:require [clojure.string :as str]))
-
+  (:require
+   [clojure.stacktrace :refer [print-stack-trace]]
+   [clojure.string :as str]))
 
 (defn make-vector
+  "takes a string containing a coma-seperated list of values an returns a vetor containing those values"
   [comma-separated-values]
   (if (empty? comma-separated-values)
     []
     (->> comma-separated-values
          (#(str/split % #","))
          (mapv str/trim)
-         (filterv #(not (empty? %))))))
-
+         (filterv seq))))
 
-(defn stack-trace [e]
-  (with-out-str (clojure.stacktrace/print-stack-trace e)))
+(defn stack-trace
+  "returns the stack trace of the exception as a string"
+  [e]
+  (with-out-str (print-stack-trace e)))