From f8f506a8beb287f62182c12f522b3217cf0bf8bd Mon Sep 17 00:00:00 2001 From: "alonso.torres" Date: Thu, 27 May 2021 10:33:23 +0200 Subject: [PATCH 1/8] :bug: Fix some problems with paths --- .../app/main/data/workspace/path/changes.cljs | 8 ++--- .../app/main/data/workspace/path/edition.cljs | 29 ++++++++++--------- .../app/main/data/workspace/path/streams.cljs | 6 +++- .../app/main/data/workspace/path/tools.cljs | 2 +- 4 files changed, 25 insertions(+), 20 deletions(-) diff --git a/frontend/src/app/main/data/workspace/path/changes.cljs b/frontend/src/app/main/data/workspace/path/changes.cljs index 8461b2e68a..ba409228fa 100644 --- a/frontend/src/app/main/data/workspace/path/changes.cljs +++ b/frontend/src/app/main/data/workspace/path/changes.cljs @@ -88,10 +88,10 @@ (let [objects (wsh/lookup-page-objects state) page-id (:current-page-id state) id (get-in state [:workspace-local :edition]) - old-content (get-in state [:workspace-local :edit-path id :old-content])] - (if (some? old-content) - (let [shape (get-in state (st/get-path state)) - [rch uch] (generate-path-changes objects page-id shape old-content (:content shape))] + old-content (get-in state [:workspace-local :edit-path id :old-content]) + shape (get-in state (st/get-path state))] + (if (and (some? old-content) (some? shape)) + (let [[rch uch] (generate-path-changes objects page-id shape old-content (:content shape))] (rx/of (dch/commit-changes {:redo-changes rch :undo-changes uch :origin it}))) diff --git a/frontend/src/app/main/data/workspace/path/edition.cljs b/frontend/src/app/main/data/workspace/path/edition.cljs index 9f5b3b6398..4a93e57dd3 100644 --- a/frontend/src/app/main/data/workspace/path/edition.cljs +++ b/frontend/src/app/main/data/workspace/path/edition.cljs @@ -60,20 +60,20 @@ old-points (->> content upg/content->points) new-points (->> new-content upg/content->points) - point-change (->> (map hash-map old-points new-points) (reduce merge)) + point-change (->> (map hash-map old-points new-points) (reduce merge))] - [rch uch] (changes/generate-path-changes objects page-id shape (:content shape) new-content)] - - (if (empty? new-content) - (rx/of (dch/commit-changes {:redo-changes rch - :undo-changes uch - :origin it}) - dwc/clear-edition-mode) - (rx/of (dch/commit-changes {:redo-changes rch - :undo-changes uch - :origin it}) - (selection/update-selection point-change) - (fn [state] (update-in state [:workspace-local :edit-path id] dissoc :content-modifiers :moving-nodes :moving-handler)))))))) + (when (and (some? new-content) (some? shape)) + (let [[rch uch] (changes/generate-path-changes objects page-id shape (:content shape) new-content)] + (if (empty? new-content) + (rx/of (dch/commit-changes {:redo-changes rch + :undo-changes uch + :origin it}) + dwc/clear-edition-mode) + (rx/of (dch/commit-changes {:redo-changes rch + :undo-changes uch + :origin it}) + (selection/update-selection point-change) + (fn [state] (update-in state [:workspace-local :edit-path id] dissoc :content-modifiers :moving-nodes :moving-handler)))))))))) (defn modify-content-point [content {dx :x dy :y} modifiers point] @@ -263,7 +263,8 @@ (streams/drag-stream (rx/concat (->> (streams/move-handler-stream snap-toggled start-point point handler opposite points) - (rx/take-until (->> stream (rx/filter ms/mouse-up?))) + (rx/take-until (->> stream (rx/filter #(or (ms/mouse-up? %) + (streams/finish-edition? %))))) (rx/map (fn [{:keys [x y alt? shift?]}] (let [pos (cond-> (gpt/point x y) diff --git a/frontend/src/app/main/data/workspace/path/streams.cljs b/frontend/src/app/main/data/workspace/path/streams.cljs index 3bf10d31e8..f8059c483a 100644 --- a/frontend/src/app/main/data/workspace/path/streams.cljs +++ b/frontend/src/app/main/data/workspace/path/streams.cljs @@ -24,6 +24,9 @@ (fn [current] (>= (gpt/distance start current) (/ drag-threshold zoom)))) +(defn finish-edition? [event] + (= (ptk/type event) :app.main.data.workspace.common/clear-edition-mode)) + (defn drag-stream ([to-stream] (drag-stream to-stream (rx/empty))) @@ -31,7 +34,8 @@ ([to-stream not-drag-stream] (let [start @ms/mouse-position zoom (get-in @st/state [:workspace-local :zoom] 1) - mouse-up (->> st/stream (rx/filter #(ms/mouse-up? %))) + mouse-up (->> st/stream (rx/filter #(or (finish-edition? %) + (ms/mouse-up? %)))) position-stream (->> ms/mouse-position diff --git a/frontend/src/app/main/data/workspace/path/tools.cljs b/frontend/src/app/main/data/workspace/path/tools.cljs index ddd2ed3f2c..610897cd23 100644 --- a/frontend/src/app/main/data/workspace/path/tools.cljs +++ b/frontend/src/app/main/data/workspace/path/tools.cljs @@ -33,7 +33,7 @@ selected-points (get-in state [:workspace-local :edit-path id :selected-points] #{}) points (or points selected-points)] - (when-not (empty? points) + (when (and (not (empty? points)) (some? shape)) (let [new-content (-> (tool-fn (:content shape) points) (ups/close-subpaths)) [rch uch] (changes/generate-path-changes objects page-id shape (:content shape) new-content)] From e8919ee340baf2c4ea772ac5cfb8a87205d85528 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 27 May 2021 11:37:31 +0200 Subject: [PATCH 2/8] :bug: Add missing `email` scope to OIDC backend. And additionaly emit a warn log message about the error. --- backend/src/app/http/oauth.clj | 18 ++++++++++++++++-- docker/images/config.env | 2 +- docker/images/files/nginx-entrypoint.sh | 2 +- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/backend/src/app/http/oauth.clj b/backend/src/app/http/oauth.clj index 41a40ab188..bfd5d16217 100644 --- a/backend/src/app/http/oauth.clj +++ b/backend/src/app/http/oauth.clj @@ -109,6 +109,17 @@ :cause e) nil))) +(s/def ::backend ::us/not-empty-string) +(s/def ::email ::us/not-empty-string) +(s/def ::fullname ::us/not-empty-string) +(s/def ::props (s/map-of ::us/keyword any?)) + +(s/def ::info + (s/keys :req-un [::backend + ::email + ::fullname + ::props])) + (defn retrieve-info [{:keys [tokens provider] :as cfg} request] (let [state (get-in request [:params :state]) @@ -116,7 +127,10 @@ info (some->> (get-in request [:params :code]) (retrieve-access-token cfg) (retrieve-user-info cfg))] - (when-not info + + (when-not (s/valid? ::info info) + (l/warn :hint "received incomplete profile info object (please set correct scopes)" + :info (pr-str info)) (ex/raise :type :internal :code :unable-to-auth :hint "no user info")) @@ -236,7 +250,7 @@ :token-uri (cf/get :oidc-token-uri) :auth-uri (cf/get :oidc-auth-uri) :user-uri (cf/get :oidc-user-uri) - :scopes (cf/get :oidc-scopes #{"openid" "profile"}) + :scopes (cf/get :oidc-scopes #{"openid" "profile" "email"}) :roles-attr (cf/get :oidc-roles-attr) :roles (cf/get :oidc-roles) :name "oidc"}] diff --git a/docker/images/config.env b/docker/images/config.env index ba5dabeafa..79f1472eef 100644 --- a/docker/images/config.env +++ b/docker/images/config.env @@ -42,7 +42,7 @@ PENPOT_REGISTRATION_ENABLED=true # Comma separated list of allowed domains to register. Empty for allow # all. -PENPOT_REGISTRATION_DOMAIN_WHITELIST="" +# PENPOT_REGISTRATION_DOMAIN_WHITELIST="" # Penpot comes with the facility to create quick demo users that are # automatically deleted after some time. This settings enables or diff --git a/docker/images/files/nginx-entrypoint.sh b/docker/images/files/nginx-entrypoint.sh index 51a118c5a9..7543ab0e1b 100644 --- a/docker/images/files/nginx-entrypoint.sh +++ b/docker/images/files/nginx-entrypoint.sh @@ -97,7 +97,7 @@ update_registration_enabled() { fi } -update_registration_enabled() { +update_analytics_enabled() { if [ -n "$PENPOT_ANALYTICS_ENABLED" ]; then sed -i \ -e "s|^//var penpotAnalyticsEnabled = .*;|var penpotAnalyticsEnabled = $PENPOT_ANALYTICS_ENABLED;|g" \ From 6e8a5015c9e29b46129c2833af93999a067565b3 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 27 May 2021 11:51:44 +0200 Subject: [PATCH 3/8] :sparkles: Add better auth module logging. --- backend/src/app/http/oauth.clj | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/backend/src/app/http/oauth.clj b/backend/src/app/http/oauth.clj index bfd5d16217..ea8f10f4bf 100644 --- a/backend/src/app/http/oauth.clj +++ b/backend/src/app/http/oauth.clj @@ -242,6 +242,13 @@ :auth-uri (get data "authorization_endpoint") :user-uri (get data "userinfo_endpoint")))))) +(defn- obfuscate-string + [s] + (if (< (count s) 10) + (apply str (take (count s) (repeat "*"))) + (str (subs s 0 5) + (apply str (take (- (count s) 5) (repeat "*")))))) + (defn- initialize-oidc-provider [cfg] (let [opts {:base-uri (cf/get :oidc-base-uri) @@ -261,10 +268,12 @@ (string? (:user-uri opts)) (string? (:auth-uri opts))) (do - (l/info :action "initialize" :provider "oid" :method "static") + (l/info :action "initialize" :provider "oidc" :method "static" + :opts (pr-str (update opts :client-secret obfuscate-string))) (assoc-in cfg [:providers "oidc"] opts)) (let [opts (discover-oidc-config opts)] - (l/info :action "initialize" :provider "oid" :method "discover") + (l/info :action "initialize" :provider "oidc" :method "discover" + :opts (pr-str (update opts :client-secret obfuscate-string))) (assoc-in cfg [:providers "oidc"] opts))) cfg))) @@ -280,7 +289,8 @@ (if (and (string? (:client-id opts)) (string? (:client-secret opts))) (do - (l/info :action "initialize" :provider "google") + (l/info :action "initialize" :provider "google" + :opts (pr-str (update opts :client-secret obfuscate-string))) (assoc-in cfg [:providers "google"] opts)) cfg))) @@ -296,7 +306,8 @@ (if (and (string? (:client-id opts)) (string? (:client-secret opts))) (do - (l/info :action "initialize" :provider "github") + (l/info :action "initialize" :provider "github" + :opts (pr-str (update opts :client-secret obfuscate-string))) (assoc-in cfg [:providers "github"] opts)) cfg))) @@ -315,7 +326,8 @@ (if (and (string? (:client-id opts)) (string? (:client-secret opts))) (do - (l/info :action "initialize" :provider "gitlab") + (l/info :action "initialize" :provider "gitlab" + :opts (pr-str (update opts :client-secret obfuscate-string))) (assoc-in cfg [:providers "gitlab"] opts)) cfg))) From 8847047fd10c420b87a7b547d1e69692130dee37 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 27 May 2021 12:21:40 +0200 Subject: [PATCH 4/8] :bug: Fix unexpected exception when user leaves typography name empty. --- frontend/src/app/main/data/workspace.cljs | 1 - .../app/main/data/workspace/libraries.cljs | 2 +- .../sidebar/options/menus/typography.cljs | 36 +++++++++---------- 3 files changed, 19 insertions(+), 20 deletions(-) diff --git a/frontend/src/app/main/data/workspace.cljs b/frontend/src/app/main/data/workspace.cljs index 5b70d9ce93..39f452e49f 100644 --- a/frontend/src/app/main/data/workspace.cljs +++ b/frontend/src/app/main/data/workspace.cljs @@ -1238,7 +1238,6 @@ qparams {:page-id page-id}] (rx/of (rt/nav :workspace pparams qparams)))))) - (defn go-to-viewer ([] (go-to-viewer {})) ([{:keys [file-id page-id]}] diff --git a/frontend/src/app/main/data/workspace/libraries.cljs b/frontend/src/app/main/data/workspace/libraries.cljs index 50e29227eb..7609759fa2 100644 --- a/frontend/src/app/main/data/workspace/libraries.cljs +++ b/frontend/src/app/main/data/workspace/libraries.cljs @@ -228,7 +228,7 @@ ptk/WatchEvent (watch [it state stream] (let [[path name] (cp/parse-path-name (:name typography)) - typography (assoc typography :path path :name name) + typography (assoc typography :path path :name name) prev (get-in state [:workspace-data :typographies (:id typography)]) rchg {:type :mod-typography :typography typography} diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/menus/typography.cljs b/frontend/src/app/main/ui/workspace/sidebar/options/menus/typography.cljs index e0649d7b6d..5fa6300e64 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/menus/typography.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options/menus/typography.cljs @@ -428,26 +428,27 @@ [:> text-transform-options opts]]])) +;; TODO: this need to be refactored, right now combines too much logic +;; and has a dropdown that behaves like a modal but is not a modal. +;; In summary, this need to a good UX/UI/IMPL rework. + (mf/defc typography-entry [{:keys [typography read-only? selected? on-click on-change on-detach on-context-menu editting? focus-name? file]}] - (let [open? (mf/use-state editting?) - hover-detach (mf/use-state false) - name-input-ref (mf/use-ref nil) - value (mf/use-state (cp/merge-path-item (:path typography) (:name typography))) + (let [open? (mf/use-state editting?) + hover-detach (mf/use-state false) + name-input-ref (mf/use-ref) - #_(rt/resolve router :workspace - {:project-id (:project-id file) - :file-id (:id file)} - {:page-id (get-in file [:data :pages 0])}) - - handle-change + on-name-blur (fn [event] - (reset! value (dom/get-target-val event))) + (let [content (dom/get-target-val event)] + (when-not (str/blank? content) + (on-change {:name content})))) handle-go-to-edit - (fn [] (st/emit! (rt/nav :workspace {:project-id (:project-id file) - :file-id (:id file)} - {:page-id (get-in file [:data :pages 0])})))] + (fn [] + (let [pparams {:project-id (:project-id file) + :file-id (:id file)}] + (st/emit! (rt/nav :workspace pparams))))] (mf/use-effect (mf/deps editting?) @@ -459,7 +460,7 @@ (mf/deps focus-name?) (fn [] (when focus-name? - (ts/schedule 100 + (ts/schedule #(when-let [node (mf/ref-val name-input-ref)] (dom/focus! node) (dom/select-text! node)))))) @@ -530,8 +531,7 @@ [:input.element-name.adv-typography-name {:type "text" :ref name-input-ref - :value @value - :on-change handle-change - :on-blur #(on-change {:name @value})}]]] + :default-value (cp/merge-path-item (:path typography) (:name typography)) + :on-blur on-name-blur}]]] [:& typography-options {:values typography :on-change on-change}]])]])) From 4d19b87fffd17265474bdfb63c9ebe165f985f04 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 27 May 2021 12:40:38 +0200 Subject: [PATCH 5/8] :sparkles: Improve error report on uploading invalid image to library. --- backend/src/app/media.clj | 9 ++++----- backend/src/app/rpc/queries/svg.clj | 1 + 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/backend/src/app/media.clj b/backend/src/app/media.clj index 2ca686f500..10de5266e3 100644 --- a/backend/src/app/media.clj +++ b/backend/src/app/media.clj @@ -66,8 +66,7 @@ (defmethod process-error :default [error] - (ex/raise :type :internal - :cause error)) + (throw error)) (defn run [{:keys [rlimits] :as cfg} {:keys [rlimit] :or {rlimit :image} :as params}] @@ -184,11 +183,10 @@ (us/assert ::input input) (let [{:keys [path mtype]} input] (if (= mtype "image/svg+xml") - (let [data (svg/parse (slurp path)) - info (get-basic-info-from-svg data)] + (let [info (some-> path slurp svg/pre-process svg/parse get-basic-info-from-svg)] (when-not info (ex/raise :type :validation - :code :unable-to-retrieve-dimensions + :code :invalid-svg-file :hint "uploaded svg does not provides dimensions")) (assoc info :mtype mtype)) @@ -211,6 +209,7 @@ [error] (ex/raise :type :validation :code :invalid-image + :hint "invalid image" :cause error)) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/backend/src/app/rpc/queries/svg.clj b/backend/src/app/rpc/queries/svg.clj index cd244aafdb..f8e978534f 100644 --- a/backend/src/app/rpc/queries/svg.clj +++ b/backend/src/app/rpc/queries/svg.clj @@ -36,6 +36,7 @@ :message (ex-message e)) (ex/raise :type :validation :code :invalid-svg-file + :hint "invalid svg file" :cause e)))) (declare pre-process) From 69ea8229caa89b1f97b8d7c97924bed881a25dd6 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 27 May 2021 12:59:42 +0200 Subject: [PATCH 6/8] :spakles: Minor improvements on svg uploading on libraries. Mainly reject svgs that have doctype declaration for security reasons. --- backend/src/app/media.clj | 2 +- backend/src/app/rpc/queries/svg.clj | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/src/app/media.clj b/backend/src/app/media.clj index 10de5266e3..b2aeefa184 100644 --- a/backend/src/app/media.clj +++ b/backend/src/app/media.clj @@ -183,7 +183,7 @@ (us/assert ::input input) (let [{:keys [path mtype]} input] (if (= mtype "image/svg+xml") - (let [info (some-> path slurp svg/pre-process svg/parse get-basic-info-from-svg)] + (let [info (some-> path slurp svg/parse get-basic-info-from-svg)] (when-not info (ex/raise :type :validation :code :invalid-svg-file diff --git a/backend/src/app/rpc/queries/svg.clj b/backend/src/app/rpc/queries/svg.clj index f8e978534f..63c0b8aeb2 100644 --- a/backend/src/app/rpc/queries/svg.clj +++ b/backend/src/app/rpc/queries/svg.clj @@ -54,6 +54,6 @@ [data] (cond-> data (str/includes? data "]+>" ""))) + (str/replace #"<\!DOCTYPE[^>]*>" ""))) (def pre-process strip-doctype) From ca5c374ecde15aa1181cbb6c9cd257e75ede622b Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 27 May 2021 13:21:37 +0200 Subject: [PATCH 7/8] :bug: Fix empty font-family handling on custom fonts page. --- frontend/src/app/main/data/fonts.cljs | 2 +- frontend/src/app/main/ui/dashboard/fonts.cljs | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/frontend/src/app/main/data/fonts.cljs b/frontend/src/app/main/data/fonts.cljs index 8992856540..e6cbfe8150 100644 --- a/frontend/src/app/main/data/fonts.cljs +++ b/frontend/src/app/main/data/fonts.cljs @@ -89,7 +89,7 @@ {:content {:data (js/Uint8Array. data) :name name :type type} - :font-family family + :font-family (or family "") :font-weight (cm/parse-font-weight variant) :font-style (cm/parse-font-style variant)})) diff --git a/frontend/src/app/main/ui/dashboard/fonts.cljs b/frontend/src/app/main/ui/dashboard/fonts.cljs index 241f7c2b24..bda48caadf 100644 --- a/frontend/src/app/main/ui/dashboard/fonts.cljs +++ b/frontend/src/app/main/ui/dashboard/fonts.cljs @@ -192,9 +192,10 @@ on-save (fn [event] (let [font-family @state] - (st/emit! (df/update-font - {:id font-id - :name font-family})) + (when-not (str/blank? font-family) + (st/emit! (df/update-font + {:id font-id + :name font-family}))) (reset! edit? false))) on-key-down From d4bf3ef6fdde4452de23ef47a8ead692fa9e7293 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 27 May 2021 13:29:29 +0200 Subject: [PATCH 8/8] :paperclip: Remove mattermost mention-all workds from error report. --- backend/src/app/loggers/mattermost.clj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/app/loggers/mattermost.clj b/backend/src/app/loggers/mattermost.clj index e51c3d73bf..ffc7305f28 100644 --- a/backend/src/app/loggers/mattermost.clj +++ b/backend/src/app/loggers/mattermost.clj @@ -61,7 +61,7 @@ [cfg {:keys [host version id] :as cdata}] (try (let [uri (:uri cfg) - text (str "Unhandled exception (@channel):\n" + text (str "Unhandled exception:\n" "- detail: " (cfg/get :public-uri) "/dbg/error-by-id/" id "\n" "- profile-id: `" (:profile-id cdata) "`\n" "- host: `" host "`\n"