From 08dce3bcdcb8570169c876c30746057d1e1ce462 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 25 May 2021 21:19:13 +0200 Subject: [PATCH 1/2] :bug: Fix possible bug in domain whitelisting checking. --- backend/src/app/config.clj | 3 +- backend/src/app/rpc/mutations/profile.clj | 22 +++++----- .../tests/app/tests/test_services_profile.clj | 4 +- common/app/common/spec.cljc | 43 +++++++++++-------- 4 files changed, 39 insertions(+), 33 deletions(-) diff --git a/backend/src/app/config.clj b/backend/src/app/config.clj index 819b3da75..30ca83d38 100644 --- a/backend/src/app/config.clj +++ b/backend/src/app/config.clj @@ -84,7 +84,6 @@ :allow-demo-users true :registration-enabled true - :registration-domain-whitelist "" :telemetry-enabled false :telemetry-uri "https://telemetry.penpot.app/" @@ -161,7 +160,7 @@ (s/def ::profile-complaint-threshold ::us/integer) (s/def ::public-uri ::us/string) (s/def ::redis-uri ::us/string) -(s/def ::registration-domain-whitelist ::us/string) +(s/def ::registration-domain-whitelist ::us/set-of-str) (s/def ::registration-enabled ::us/boolean) (s/def ::rlimits-image ::us/integer) (s/def ::rlimits-password ::us/integer) diff --git a/backend/src/app/rpc/mutations/profile.clj b/backend/src/app/rpc/mutations/profile.clj index 738c3f550..71c2a0d90 100644 --- a/backend/src/app/rpc/mutations/profile.clj +++ b/backend/src/app/rpc/mutations/profile.clj @@ -60,9 +60,10 @@ (ex/raise :type :restriction :code :registration-disabled)) - (when-not (email-domain-in-whitelist? (cfg/get :registration-domain-whitelist) (:email params)) - (ex/raise :type :validation - :code :email-domain-is-not-allowed)) + (when-let [domains (cfg/get :registration-domain-whitelist)] + (when-not (email-domain-in-whitelist? domains (:email params)) + (ex/raise :type :validation + :code :email-domain-is-not-allowed))) (when-not (:terms-privacy params) (ex/raise :type :validation @@ -137,14 +138,15 @@ ::audit/profile-id (:id profile)}))))) (defn email-domain-in-whitelist? - "Returns true if email's domain is in the given whitelist or if given - whitelist is an empty string." - [whitelist email] - (if (str/empty-or-nil? whitelist) + "Returns true if email's domain is in the given whitelist or if + given whitelist is an empty string." + [domains email] + (if (or (empty? domains) + (nil? domains)) true - (let [domains (str/split whitelist #",\s*") - domain (second (str/split email #"@" 2))] - (contains? (set domains) domain)))) + (let [[_ candidate] (-> (str/lower email) + (str/split #"@" 2))] + (contains? domains candidate)))) (def ^:private sql:profile-existence "select exists (select * from profile diff --git a/backend/tests/app/tests/test_services_profile.clj b/backend/tests/app/tests/test_services_profile.clj index 7ac20c849..c3daf5c14 100644 --- a/backend/tests/app/tests/test_services_profile.clj +++ b/backend/tests/app/tests/test_services_profile.clj @@ -179,10 +179,10 @@ )) (t/deftest registration-domain-whitelist - (let [whitelist "gmail.com, hey.com, ya.ru"] + (let [whitelist #{"gmail.com" "hey.com" "ya.ru"}] (t/testing "allowed email domain" (t/is (true? (profile/email-domain-in-whitelist? whitelist "username@ya.ru"))) - (t/is (true? (profile/email-domain-in-whitelist? "" "username@somedomain.com")))) + (t/is (true? (profile/email-domain-in-whitelist? #{} "username@somedomain.com")))) (t/testing "not allowed email domain" (t/is (false? (profile/email-domain-in-whitelist? whitelist "username@somedomain.com")))))) diff --git a/common/app/common/spec.cljc b/common/app/common/spec.cljc index 00aebc82a..61c651136 100644 --- a/common/app/common/spec.cljc +++ b/common/app/common/spec.cljc @@ -137,29 +137,34 @@ ;; --- SPEC: email +(def email-re #"[a-zA-Z0-9_.+-]+@[a-zA-Z0-9-]+\.[a-zA-Z0-9-.]+") -(let [re #"[a-zA-Z0-9_.+-]+@[a-zA-Z0-9-]+\.[a-zA-Z0-9-.]+" - cfn (fn [v] - (if (string? v) - (if-let [matches (re-seq re v)] - (first matches) - (do ::s/invalid)) - ::s/invalid))] - (s/def ::email (s/conformer cfn str))) - +(s/def ::email + (s/conformer + (fn [v] + (if (string? v) + (if-let [matches (re-seq email-re v)] + (first matches) + (do ::s/invalid)) + ::s/invalid)) + str)) ;; --- SPEC: set-of-str -(letfn [(conformer [s] - (cond - (string? s) (into #{} (str/split s #"\s*,\s*")) - (set? s) (if (every? string? s) - s - ::s/invalid) - :else ::s/invalid)) - (unformer [s] - (str/join "," s))] - (s/def ::set-of-str (s/conformer conformer unformer))) +(s/def ::set-of-str + (s/conformer + (fn [s] + (let [xform (comp + (filter string?) + (remove str/empty?) + (remove str/blank?))] + (cond + (string? s) (->> (str/split s #"\s*,\s*") + (into #{} xform)) + (set? s) (into #{} xform s) + :else ::s/invalid))) + (fn [s] + (str/join "," s)))) ;; --- Macros From 2ac790693acdad54842d559c2c858a11c728e43a Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 25 May 2021 23:24:19 +0200 Subject: [PATCH 2/2] :bug: Fix CSRNG usage on webworker context. --- common/app/common/uuid_impl.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/common/app/common/uuid_impl.js b/common/app/common/uuid_impl.js index d276ce516..791dd58ea 100644 --- a/common/app/common/uuid_impl.js +++ b/common/app/common/uuid_impl.js @@ -12,17 +12,13 @@ goog.provide("app.common.uuid_impl"); goog.scope(function() { const core = cljs.core; + const global = goog.global; const self = app.common.uuid_impl; const fill = (() => { - if (typeof window === "object" && typeof window.crypto !== "undefined") { + if (typeof global.crypto !== "undefined") { return (buf) => { - window.crypto.getRandomValues(buf); - return buf; - }; - } else if (typeof self === "object" && typeof self.crypto !== "undefined") { - return (buf) => { - self.crypto.getRandomValues(buf); + global.crypto.getRandomValues(buf); return buf; }; } else if (typeof require === "function") { @@ -34,7 +30,7 @@ goog.scope(function() { }; } else { // FALLBACK - console.warn("No high quality RNG available, switching back to Math.random."); + console.warn("No SRNG available, switching back to Math.random."); return (buf) => { for (let i = 0, r; i < buf.length; i++) {