🐛 Fix several issues related to path edition (#8187)

* ✨ Improve save-path-content event consistency

Mainly removing possible race conditions from the event
implementation.

* ✨ Ensure path content snapshot on start-path-edit event

* ✨ Reuse already available shape-id on split-segments

.github/workflows/plugins-deploy-packages.yml

@@ -70,7 +70,7 @@ jobs:
 
   colors-to-tokens-plugin:
     needs: detect-changes
-    if: needs.detect-changes.outputs.colors_to_tokens == 'true'
+    if: github.event_name == 'workflow_dispatch' || needs.detect-changes.outputs.colors_to_tokens == 'true'
     uses: ./.github/workflows/plugins-deploy-package.yml
     secrets: inherit
     with:
@@ -79,7 +79,7 @@ jobs:
 
   contrast-plugin:
     needs: detect-changes
-    if: needs.detect-changes.outputs.contrast == 'true'
+    if: github.event_name == 'workflow_dispatch' || needs.detect-changes.outputs.contrast == 'true'
     uses: ./.github/workflows/plugins-deploy-package.yml
     secrets: inherit
     with:
@@ -88,7 +88,7 @@ jobs:
 
   create-palette-plugin:
     needs: detect-changes
-    if: needs.detect-changes.outputs.create_palette == 'true'
+    if: github.event_name == 'workflow_dispatch' || needs.detect-changes.outputs.create_palette == 'true'
     uses: ./.github/workflows/plugins-deploy-package.yml
     secrets: inherit
     with:
@@ -97,7 +97,7 @@ jobs:
 
   icons-plugin:
     needs: detect-changes
-    if: needs.detect-changes.outputs.icons == 'true'
+    if: github.event_name == 'workflow_dispatch' || needs.detect-changes.outputs.icons == 'true'
     uses: ./.github/workflows/plugins-deploy-package.yml
     secrets: inherit
     with:
@@ -106,7 +106,7 @@ jobs:
 
   lorem-ipsum-plugin:
     needs: detect-changes
-    if: needs.detect-changes.outputs.lorem_ipsum == 'true'
+    if: github.event_name == 'workflow_dispatch' || needs.detect-changes.outputs.lorem_ipsum == 'true'
     uses: ./.github/workflows/plugins-deploy-package.yml
     secrets: inherit
     with:
@@ -115,7 +115,7 @@ jobs:
 
   rename-layers-plugin:
     needs: detect-changes
-    if: needs.detect-changes.outputs.rename_layers == 'true'
+    if: github.event_name == 'workflow_dispatch' || needs.detect-changes.outputs.rename_layers == 'true'
     uses: ./.github/workflows/plugins-deploy-package.yml
     secrets: inherit
     with:
@@ -124,7 +124,7 @@ jobs:
 
   table-plugin:
     needs: detect-changes
-    if: needs.detect-changes.outputs.table == 'true'
+    if: github.event_name == 'workflow_dispatch' || needs.detect-changes.outputs.table == 'true'
     uses: ./.github/workflows/plugins-deploy-package.yml
     secrets: inherit
     with:
@@ -135,7 +135,7 @@ jobs:
   #   Add more jobs for other plugins below, following the same pattern
   #     another-plugin:
   #       needs: detect-changes
-  #       if: needs.detect-changes.outputs.another_plugin == 'true'
+  #       if: github.event_name == 'workflow_dispatch' || needs.detect-changes.outputs.another_plugin == 'true'
   #       uses: ./.github/workflows/plugins-deploy-package.yml
   #       secrets: inherit
   #       with:

backend/scripts/_env

@@ -1,12 +1,7 @@
 #!/usr/bin/env bash
 
-export PENPOT_NITRATE_SHARED_KEY=super-secret-nitrate-api-key
-export PENPOT_EXPORTER_SHARED_KEY=super-secret-exporter-api-key
-export PENPOT_SECRET_KEY=super-secret-devenv-key
-
-# DEPRECATED: only used for subscriptions
 export PENPOT_MANAGEMENT_API_KEY=super-secret-management-api-key
-
+export PENPOT_SECRET_KEY=super-secret-devenv-key
 export PENPOT_HOST=devenv
 export PENPOT_PUBLIC_URI=https://localhost:3449
 

backend/src/app/config.clj

@@ -102,8 +102,6 @@
     [:http-server-io-threads {:optional true} ::sm/int]
     [:http-server-max-worker-threads {:optional true} ::sm/int]
 
-    [:exporter-shared-key {:optional true} :string]
-    [:nitrate-shared-key {:optional true} :string]
     [:management-api-key {:optional true} :string]
 
     [:telemetry-uri {:optional true} :string]

backend/src/app/http/management.clj

@@ -13,13 +13,13 @@
    [app.common.time :as ct]
    [app.config :as cf]
    [app.db :as db]
+   [app.http.middleware :as mw]
    [app.main :as-alias main]
    [app.rpc.commands.profile :as cmd.profile]
    [app.setup :as-alias setup]
    [app.tokens :as tokens]
    [app.worker :as-alias wrk]
    [integrant.core :as ig]
-   [yetti.request :as yreq]
    [yetti.response :as-alias yres]))
 
 ;; ---- ROUTES
@@ -49,40 +49,28 @@
          (fn [cfg request]
            (db/tx-run! cfg handler request)))))})
 
-(def ^:private shared-key-auth
-  {:name ::shared-key-auth
-   :compile
-   (fn [_ _]
-     (fn [handler key]
-       (if key
-         (fn [request]
-           (if-let [key' (yreq/get-header request "x-shared-key")]
-             (if (= key key')
-               (handler request)
-               {::yres/status 403})
-             {::yres/status 403}))
-         (fn [_ _]
-           {::yres/status 403}))))})
-
 (defmethod ig/init-key ::routes
-  [_ cfg]
+  [_ {:keys [::setup/props] :as cfg}]
 
-  ["" {:middleware [[shared-key-auth (cf/get :management-api-key)]
-                    [default-system cfg]
-                    [transaction]]}
-   ["/authenticate"
-    {:handler authenticate
-     :allowed-methods #{:post}}]
+  (let [management-key (or (cf/get :management-api-key)
+                           (get props :management-key))]
 
-   ["/get-customer"
-    {:handler get-customer
-     :transaction true
-     :allowed-methods #{:post}}]
+    ["" {:middleware [[mw/shared-key-auth management-key]
+                      [default-system cfg]
+                      [transaction]]}
+     ["/authenticate"
+      {:handler authenticate
+       :allowed-methods #{:post}}]
 
-   ["/update-customer"
-    {:handler update-customer
-     :allowed-methods #{:post}
-     :transaction true}]])
+     ["/get-customer"
+      {:handler get-customer
+       :transaction true
+       :allowed-methods #{:post}}]
+
+     ["/update-customer"
+      {:handler update-customer
+       :allowed-methods #{:post}
+       :transaction true}]]))
 
 ;; ---- HELPERS
 

backend/src/app/http/middleware.clj

@@ -16,6 +16,7 @@
    [app.http.errors :as errors]
    [app.tokens :as tokens]
    [app.util.pointer-map :as pmap]
+   [buddy.core.codecs :as bc]
    [cuerdas.core :as str]
    [yetti.adapter :as yt]
    [yetti.middleware :as ymw]
@@ -300,20 +301,16 @@
    :compile (constantly wrap-auth)})
 
 (defn- wrap-shared-key-auth
-  [handler keys]
-  (if (seq keys)
-    (fn [request]
-      (if-let [[key-id key] (some-> (yreq/get-header request "x-shared-key")
-                                    (str/split #"\s+" 2))]
-        (let [key-id (str/lower key-id)]
-          (if (and (string? key)
-                   (contains? keys key-id)
-                   (= key (get keys key-id)))
-            (-> request
-                (assoc ::http/auth-key-id key-id)
-                (handler))
-            {::yres/status 403}))
-        {::yres/status 403}))
+  [handler shared-key]
+  (if shared-key
+    (let [shared-key (if (string? shared-key)
+                       shared-key
+                       (bc/bytes->b64-str shared-key true))]
+      (fn [request]
+        (let [key (yreq/get-header request "x-shared-key")]
+          (if (= key shared-key)
+            (handler (assoc request ::http/auth-with-shared-key true))
+            {::yres/status 403}))))
     (fn [_ _]
       {::yres/status 403})))
 

backend/src/app/loggers/audit.clj

@@ -140,14 +140,10 @@
         client-version      (get-client-version request)
         client-user-agent   (get-client-user-agent request)
         session-id          (get-external-session-id request)
-        key-id              (::http/auth-key-id request)
-        token-id            (::actoken/id request)
-        token-type          (::actoken/type request)]
+        token-id       (::actoken/id request)]
     (d/without-nils
      {:external-session-id session-id
-      :initiator (or key-id "app")
       :access-token-id (some-> token-id str)
-      :access-token-type (some-> token-type str)
       :client-event-origin client-event-origin
       :client-user-agent client-user-agent
       :client-version client-version

backend/src/app/main.clj

@@ -275,7 +275,8 @@
     ::email/whitelist    (ig/ref ::email/whitelist)}
 
    ::mgmt/routes
-   {::db/pool (ig/ref ::db/pool)}
+   {::db/pool            (ig/ref ::db/pool)
+    ::setup/props        (ig/ref ::setup/props)}
 
    :app.http/router
    {::session/manager    (ig/ref ::session/manager)
@@ -351,13 +352,13 @@
     ::setup/props        (ig/ref ::setup/props)}
 
    ::rpc/routes
-   {::rpc/methods            (ig/ref :app.rpc/methods)
+   {::rpc/methods        (ig/ref :app.rpc/methods)
     ::rpc/management-methods (ig/ref :app.rpc/management-methods)
 
     ;; FIXME: revisit if db/pool is necessary here
     ::db/pool                (ig/ref ::db/pool)
     ::session/manager        (ig/ref ::session/manager)
-    ::setup/shared-keys      (ig/ref ::setup/shared-keys)}
+    ::setup/props            (ig/ref ::setup/props)}
 
    ::wrk/registry
    {::mtx/metrics (ig/ref ::mtx/metrics)
@@ -445,11 +446,6 @@
     ;; module requires the migrations to run before initialize.
     ::migrations (ig/ref :app.migrations/migrations)}
 
-   ::setup/shared-keys
-   {::setup/props (ig/ref ::setup/props)
-    :nitrate (cf/get :nitrate-shared-key)
-    :exporter (cf/get :exporter-shared-key)}
-
    ::setup/clock
    {}
 

backend/src/app/rpc.clj

@@ -92,11 +92,11 @@
     (fn [{:keys [params path-params method] :as request}]
       (let [handler-name (:type path-params)
             etag         (yreq/get-header request "if-none-match")
-
-            key-id       (get request ::http/auth-key-id)
             profile-id   (or (::session/profile-id request)
                              (::actoken/profile-id request)
-                             (if key-id uuid/zero nil))
+                             (if (::http/auth-with-shared-key request)
+                               uuid/zero
+                               nil))
 
             ip-addr      (inet/parse-request request)
 
@@ -345,20 +345,23 @@
 
 (defmethod ig/assert-key ::routes
   [_ params]
-  (assert (map? (::setup/shared-keys params)))
   (assert (db/pool? (::db/pool params)) "expect valid database pool")
+  (assert (some? (::setup/props params)))
   (assert (session/manager? (::session/manager params)) "expect valid session manager")
   (assert (valid-methods? (::methods params)) "expect valid methods map")
   (assert (valid-methods? (::management-methods params)) "expect valid methods map"))
 
 (defmethod ig/init-key ::routes
-  [_ {:keys [::methods ::management-methods ::setup/shared-keys] :as cfg}]
+  [_ {:keys [::methods ::management-methods ::setup/props] :as cfg}]
+
+  (let [public-uri     (cf/get :public-uri)
+        management-key (or (cf/get :management-api-key)
+                           (get props :management-key))]
 
-  (let [public-uri (cf/get :public-uri)]
     ["/api"
      ["/management"
       ["/methods/:type"
-       {:middleware [[mw/shared-key-auth shared-keys]
+       {:middleware [[mw/shared-key-auth management-key]
                      [session/authz cfg]]
         :handler (make-rpc-handler management-methods)}]
 

backend/src/app/setup.clj

@@ -17,7 +17,6 @@
    [app.setup.templates]
    [buddy.core.codecs :as bc]
    [buddy.core.nonce :as bn]
-   [cuerdas.core :as str]
    [integrant.core :as ig]))
 
 (defn- generate-random-key
@@ -89,38 +88,7 @@
                       (-> (get-all-props conn)
                           (assoc :secret-key secret)
                           (assoc :tokens-key (keys/derive secret :salt "tokens"))
+                          (assoc :management-key (keys/derive secret :salt "management"))
                           (update :instance-id handle-instance-id conn (db/read-only? pool)))))))
 
 (sm/register! ::props [:map-of :keyword ::sm/any])
-
-
-(defmethod ig/init-key ::shared-keys
-  [_ {:keys [::props] :as cfg}]
-  (let [secret (get props :secret-key)]
-    (d/without-nils
-     {"exporter"
-      (let [key (or (get cfg :exporter)
-                    (-> (keys/derive secret :salt "exporter")
-                        (bc/bytes->b64-str true)))]
-        (if (or (str/empty? key)
-                (str/blank? key))
-          (do
-            (l/wrn :hint "exporter key is disabled because empty string found")
-            nil)
-          (do
-            (l/inf :hint "exporter key initialized" :key (d/obfuscate-string key))
-            key)))
-
-      "nitrate"
-      (let [key (or (get cfg :nitrate)
-                    (-> (keys/derive secret :salt "nitrate")
-                        (bc/bytes->b64-str true)))]
-        (if (or (str/empty? key)
-                (str/blank? key))
-          (do
-            (l/wrn :hint "nitrate key is disabled because empty string found")
-            nil)
-          (do
-            (l/inf :hint "nitrate key initialized" :key (d/obfuscate-string key))
-            key)))})))
-

backend/test/backend_tests/http_middleware_test.clj

@@ -86,7 +86,7 @@
 (t/deftest shared-key-auth
   (let [handler (#'app.http.middleware/wrap-shared-key-auth
                  (fn [req] {::yres/status 200})
-                 {"test1" "secret-key"})]
+                 "secret-key")]
 
     (let [response (handler (->DummyRequest {} {}))]
       (t/is (= 403 (::yres/status response))))
@@ -95,9 +95,6 @@
       (t/is (= 403 (::yres/status response))))
 
     (let [response (handler (->DummyRequest {"x-shared-key" "secret-key"} {}))]
-      (t/is (= 403 (::yres/status response))))
-
-    (let [response (handler (->DummyRequest {"x-shared-key" "test1 secret-key"} {}))]
       (t/is (= 200 (::yres/status response))))))
 
 (t/deftest access-token-authz

exporter/src/app/config.cljs

@@ -12,14 +12,11 @@
    ["node:process" :as process]
    [app.common.data :as d]
    [app.common.flags :as flags]
-   [app.common.logging :as l]
    [app.common.schema :as sm]
    [app.common.version :as v]
    [cljs.core :as c]
    [cuerdas.core :as str]))
 
-(l/set-level! :info)
-
 (def ^:private defaults
   {:public-uri "http://localhost:3449"
    :tenant "default"
@@ -33,7 +30,7 @@
   [:map {:title "config"}
    [:secret-key :string]
    [:public-uri {:optional true} ::sm/uri]
-   [:exporter-shared-key {:optional true} :string]
+   [:management-api-key {:optional true} :string]
    [:host {:optional true} :string]
    [:tenant {:optional true} :string]
    [:flags {:optional true} [::sm/set :keyword]]
@@ -101,10 +98,8 @@
    (c/get config key default)))
 
 (def management-key
-  (let [key (or (c/get config :exporter-shared-key)
-                (let [secret-key  (c/get config :secret-key)
-                      derived-key (crypto/hkdfSync "blake2b512" secret-key, "exporter" "" 32)]
-                  (-> (.from buffer/Buffer derived-key)
-                      (.toString "base64url"))))]
-    (l/inf :hint "exporter key initialized" :key (d/obfuscate-string key))
-    key))
+  (or (c/get config :management-api-key)
+      (let [secret-key  (c/get config :secret-key)
+            derived-key (crypto/hkdfSync "blake2b512" secret-key, "management" "" 32)]
+        (-> (.from buffer/Buffer derived-key)
+            (.toString "base64url")))))

exporter/src/app/handlers/resources.cljs

@@ -73,7 +73,7 @@
        (p/mcat (fn [blob]
                  (let [fdata  (new http/FormData)
                        agent  (new http/Agent #js {:connect #js {:rejectUnauthorized false}})
-                       headers #js {"X-Shared-Key" (str "exporter " cf/management-key)
+                       headers #js {"X-Shared-Key" cf/management-key
                                     "Authorization" (str "Bearer " auth-token)}
 
                        request #js {:headers headers

frontend/src/app/main/data/workspace/path/changes.cljs

@@ -70,20 +70,22 @@
                               (= (-> content last :command) :move-to))
                        (into [] (take (dec (count content)) content))
                        content)]
-         (-> state
-             (st/set-content content))))
+         (st/set-content state content)))
 
      ptk/WatchEvent
      (watch [it state _]
        (let [page-id     (:current-page-id state)
-             objects     (dsh/lookup-page-objects state page-id)
-             id          (dm/get-in state [:workspace-local :edition])
-             old-content (dm/get-in state [:workspace-local :edit-path id :old-content])
-             shape       (st/get-path state)]
+             local       (get state :workspace-local)
+             id          (get local :edition)
+             objects     (dsh/lookup-page-objects state page-id)]
 
-         (if (and (some? old-content) (some? (:id shape)))
-           (let [changes (generate-path-changes it objects page-id shape old-content (:content shape))]
-             (rx/of (dch/commit-changes changes)))
-           (rx/empty)))))))
+         ;; NOTE: we proceed only if the shape is present on the
+         ;; objects, if shape is a ephimeral drawing shape, we should
+         ;; do nothing
+         (when-let [shape (get objects id)]
+           (when-let [old-content (dm/get-in local [:edit-path id :old-content])]
+             (let [new-content (get shape :content)
+                   changes     (generate-path-changes it objects page-id shape old-content new-content)]
+               (rx/of (dch/commit-changes changes))))))))))
 
 

frontend/src/app/main/data/workspace/path/edition.cljs

@@ -8,7 +8,6 @@
   (:require
    [app.common.data :as d]
    [app.common.data.macros :as dm]
-   [app.common.files.helpers :as cfh]
    [app.common.geom.point :as gpt]
    [app.common.types.path :as path]
    [app.common.types.path.helpers :as path.helpers]
@@ -289,34 +288,34 @@
 
 (declare stop-path-edit)
 
+
 (defn start-path-edit
   [id]
   (ptk/reify ::start-path-edit
     ptk/UpdateEvent
     (update [_ state]
       (let [objects   (dsh/lookup-page-objects state)
-            edit-path (dm/get-in state [:workspace-local :edit-path id])
-            content   (st/get-path state :content)
-            state     (cond-> state
-                        (cfh/path-shape? objects id)
-                        (st/set-content (path/close-subpaths content)))]
+            shape     (get objects id)]
 
-        (cond-> state
-          (or (not edit-path)
-              (= :draw (:edit-mode edit-path)))
-          (assoc-in [:workspace-local :edit-path id] {:edit-mode :move
-                                                      :selected #{}
-                                                      :snap-toggled false})
-          (and (some? edit-path)
-               (= :move (:edit-mode edit-path)))
-          (assoc-in [:workspace-local :edit-path id :edit-mode] :draw))))
+        (-> state
+            (st/set-content (path/close-subpaths (:content shape)))
+            (update-in [:workspace-local :edit-path id]
+                       (fn [state]
+                         (let [state (if state
+                                       (if (= :move (:edit-mode state))
+                                         (assoc state :edit-mode :draw)
+                                         state)
+                                       {:edit-mode :move
+                                        :selected #{}
+                                        :snap-toggled false})]
+                           (assoc state :old-content (:content shape))))))))
 
     ptk/WatchEvent
     (watch [_ _ stream]
-      (let [stopper (->> stream
-                         (rx/filter #(let [type (ptk/type %)]
-                                       (= type ::dwe/clear-edition-mode)
-                                       (= type ::start-path-edit))))]
+      (let [stopper (rx/filter #(let [type (ptk/type %)]
+                                  (= type ::dwe/clear-edition-mode)
+                                  (= type ::start-path-edit))
+                               stream)]
         (rx/concat
          (rx/of (undo/start-path-undo))
          (->> stream
@@ -325,7 +324,8 @@
               (rx/map #(stop-path-edit id))
               (rx/take-until stopper)))))))
 
-(defn stop-path-edit [id]
+(defn stop-path-edit
+  [id]
   (ptk/reify ::stop-path-edit
     ptk/UpdateEvent
     (update [_ state]
@@ -335,13 +335,12 @@
     (watch [_ _ _]
       (rx/of (ptk/data-event :layout/update {:ids [id]})))))
 
-(defn split-segments
-  [{:keys [from-p to-p t]}]
+(defn- split-segments
+  [id {:keys [from-p to-p t]}]
   (ptk/reify ::split-segments
     ptk/UpdateEvent
     (update [_ state]
-      (let [id (st/get-path-id state)
-            content (st/get-path state :content)]
+      (let [content (st/get-path state :content)]
         (-> state
             (assoc-in [:workspace-local :edit-path id :old-content] content)
             (st/set-content (-> content
@@ -353,10 +352,10 @@
       (rx/of (changes/save-path-content {:preserve-move-to true})))))
 
 (defn create-node-at-position
-  [event]
+  [params]
   (ptk/reify ::create-node-at-position
     ptk/WatchEvent
     (watch [_ state _]
       (let [id (st/get-path-id state)]
         (rx/of (dwsh/update-shapes [id] path/convert-to-path)
-               (split-segments event))))))
+               (split-segments id params))))))

plugins/apps/colors-to-tokens-plugin/wrangler.toml

@@ -1,7 +1,7 @@
 name = "color-to-tokens-plugin"
 compatibility_date = "2025-01-01"
 
-assets = { directory = "../../dist/apps/color-to-tokens-plugin/browser" }
+assets = { directory = "../../dist/apps/colors-to-tokens-plugin/browser" }
 
 [[routes]]
 pattern = "WORKER_URI"