diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4fb8cd92a..fc3d2b7fd 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -31,8 +31,8 @@ in the changelog.** ## Pull requests ## If you want propose a change or bug fix with the Pull-Request system -firstly you should carefully read the **Contributor License Aggreement** -section and format your commits accordingly. +firstly you should carefully read the **DCO** section and format your +commits accordingly. If you intend to fix a bug it's fine to submit a pull request right away but we still recommend to file an issue detailing what you're diff --git a/backend/src/app/config.clj b/backend/src/app/config.clj index 71de67169..3a313d4a9 100644 --- a/backend/src/app/config.clj +++ b/backend/src/app/config.clj @@ -57,7 +57,6 @@ :registration-domain-whitelist "" :telemetry-enabled false - :telemetry-with-taiga true :telemetry-uri "https://telemetry.penpot.app/" ;; LDAP auth disabled by default. Set ldap-auth-host to enable diff --git a/backend/src/app/db.clj b/backend/src/app/db.clj index 12bbbcf0d..1fcb18e47 100644 --- a/backend/src/app/db.clj +++ b/backend/src/app/db.clj @@ -72,7 +72,7 @@ ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (def initsql - (str "SET statement_timeout = 10000;\n" + (str "SET statement_timeout = 60000;\n" "SET idle_in_transaction_session_timeout = 120000;")) (defn- create-datasource-config diff --git a/backend/src/app/http/auth/google.clj b/backend/src/app/http/auth/google.clj index aaa4c1a2b..a615ddd80 100644 --- a/backend/src/app/http/auth/google.clj +++ b/backend/src/app/http/auth/google.clj @@ -35,51 +35,40 @@ (defn- get-access-token [cfg code] - (let [params {:code code - :client_id (:client-id cfg) - :client_secret (:client-secret cfg) - :redirect_uri (build-redirect-url cfg) - :grant_type "authorization_code"} - req {:method :post - :headers {"content-type" "application/x-www-form-urlencoded"} - :uri "https://oauth2.googleapis.com/token" - :body (uri/map->query-string params)} - res (http/send! req)] + (try + (let [params {:code code + :client_id (:client-id cfg) + :client_secret (:client-secret cfg) + :redirect_uri (build-redirect-url cfg) + :grant_type "authorization_code"} + req {:method :post + :headers {"content-type" "application/x-www-form-urlencoded"} + :uri "https://oauth2.googleapis.com/token" + :body (uri/map->query-string params)} + res (http/send! req)] - (when (not= 200 (:status res)) - (ex/raise :type :internal - :code :invalid-response-from-google - :context {:status (:status res) - :body (:body res)})) + (when (= 200 (:status res)) + (-> (json/read-str (:body res)) + (get "access_token")))) - (try - (let [data (json/read-str (:body res))] - (get data "access_token")) - (catch Throwable e - (log/error "unexpected error on parsing response body from google access token request" e) - nil)))) + (catch Exception e + (log/error e "unexpected error on get-access-token") + nil))) (defn- get-user-info [token] - (let [req {:uri "https://openidconnect.googleapis.com/v1/userinfo" - :headers {"Authorization" (str "Bearer " token)} - :method :get} - res (http/send! req)] - - (when (not= 200 (:status res)) - (ex/raise :type :internal - :code :invalid-response-from-google - :context {:status (:status res) - :body (:body res)})) - - (try - (let [data (json/read-str (:body res))] - ;; (clojure.pprint/pprint data) - {:email (get data "email") - :fullname (get data "name")}) - (catch Throwable e - (log/error "unexpected error on parsing response body from google access token request" e) - nil)))) + (try + (let [req {:uri "https://openidconnect.googleapis.com/v1/userinfo" + :headers {"Authorization" (str "Bearer " token)} + :method :get} + res (http/send! req)] + (when (= 200 (:status res)) + (let [data (json/read-str (:body res))] + {:email (get data "email") + :fullname (get data "name")}))) + (catch Exception e + (log/error e "unexpected exception on get-user-info") + nil))) (defn- auth [{:keys [tokens] :as cfg} _req] @@ -99,33 +88,39 @@ (defn- callback [{:keys [tokens rpc session] :as cfg} request] - (let [token (get-in request [:params :state]) - _ (tokens :verify {:token token :iss :google-oauth}) - info (some->> (get-in request [:params :code]) - (get-access-token cfg) - (get-user-info))] - - (when-not info - (ex/raise :type :authentication - :code :unable-to-authenticate-with-google)) - - (let [method-fn (get-in rpc [:methods :mutation :login-or-register]) + (try + (let [token (get-in request [:params :state]) + _ (tokens :verify {:token token :iss :google-oauth}) + info (some->> (get-in request [:params :code]) + (get-access-token cfg) + (get-user-info)) + _ (when-not info + (ex/raise :type :internal + :code :unable-to-auth)) + method-fn (get-in rpc [:methods :mutation :login-or-register]) profile (method-fn {:email (:email info) :fullname (:fullname info)}) uagent (get-in request [:headers "user-agent"]) token (tokens :generate {:iss :auth :exp (dt/in-future "15m") :profile-id (:id profile)}) - uri (-> (uri/uri (:public-uri cfg)) (assoc :path "/#/auth/verify-token") (assoc :query (uri/map->query-string {:token token}))) + sid (session/create! session {:profile-id (:id profile) :user-agent uagent})] {:status 302 :headers {"location" (str uri)} :cookies (session/cookies session {:value sid}) - :body ""}))) + :body ""}) + (catch Exception _e + (let [uri (-> (uri/uri (:public-uri cfg)) + (assoc :path "/#/auth/login") + (assoc :query (uri/map->query-string {:error "unable-to-auth"})))] + {:status 302 + :headers {"location" (str uri)} + :body ""})))) (s/def ::client-id ::us/not-empty-string) (s/def ::client-secret ::us/not-empty-string) diff --git a/backend/src/app/main.clj b/backend/src/app/main.clj index 02cc651b4..e26d84a92 100644 --- a/backend/src/app/main.clj +++ b/backend/src/app/main.clj @@ -192,19 +192,19 @@ :fn (ig/ref :app.tasks.file-media-gc/handler)} {:id "file-xlog-gc" - :cron #app/cron "0 0 */6 * * ?" ;; every 2 hours + :cron #app/cron "0 0 */1 * * ?" ;; hourly :fn (ig/ref :app.tasks.file-xlog-gc/handler)} {:id "storage-deleted-gc" - :cron #app/cron "0 0 */6 * * ?" ;; every 6 hours + :cron #app/cron "0 0 1 */1 * ?" ;; daily (1 hour shift) :fn (ig/ref :app.storage/gc-deleted-task)} {:id "storage-touched-gc" - :cron #app/cron "0 30 */6 * * ?" ;; every 6 hours + :cron #app/cron "0 0 2 */1 * ?" ;; daily (2 hour shift) :fn (ig/ref :app.storage/gc-touched-task)} {:id "storage-recheck" - :cron #app/cron "0 0 */6 * * ?" ;; every 6 hours + :cron #app/cron "0 0 */1 * * ?" ;; hourly :fn (ig/ref :app.storage/recheck-task)} {:id "tasks-gc" @@ -260,7 +260,7 @@ :app.tasks.file-xlog-gc/handler {:pool (ig/ref :app.db/pool) :metrics (ig/ref :app.metrics/metrics) - :max-age (dt/duration {:hours 24})} + :max-age (dt/duration {:hours 48})} :app.tasks.telemetry/handler {:pool (ig/ref :app.db/pool) diff --git a/backend/src/app/media.clj b/backend/src/app/media.clj index b775c4852..14a4191e0 100644 --- a/backend/src/app/media.clj +++ b/backend/src/app/media.clj @@ -175,7 +175,12 @@ (ex/raise :type :internal :code :rlimit-not-configured :hint ":image rlimit not configured")) - (rlm/execute rlimit (process params)))) + (try + (rlm/execute rlimit (process params)) + (catch org.im4java.core.InfoException e + (ex/raise :type :validation + :code :invalid-image + :cause e))))) ;; --- Utility functions diff --git a/backend/src/app/migrations.clj b/backend/src/app/migrations.clj index 74e191701..2d247e11c 100644 --- a/backend/src/app/migrations.clj +++ b/backend/src/app/migrations.clj @@ -145,6 +145,9 @@ {:name "0044-add-storage-refcount" :fn (mg/resource "app/migrations/sql/0044-add-storage-refcount.sql")} + + {:name "0045-add-index-to-file-change-table" + :fn (mg/resource "app/migrations/sql/0045-add-index-to-file-change-table.sql")} ]) diff --git a/backend/src/app/migrations/sql/0045-add-index-to-file-change-table.sql b/backend/src/app/migrations/sql/0045-add-index-to-file-change-table.sql new file mode 100644 index 000000000..1537bcd4c --- /dev/null +++ b/backend/src/app/migrations/sql/0045-add-index-to-file-change-table.sql @@ -0,0 +1,2 @@ +CREATE INDEX file_change__created_at_idx + ON file_change (created_at); diff --git a/backend/src/app/rpc/mutations/media.clj b/backend/src/app/rpc/mutations/media.clj index b6b115d7c..e9d4d9bab 100644 --- a/backend/src/app/rpc/mutations/media.clj +++ b/backend/src/app/rpc/mutations/media.clj @@ -38,7 +38,7 @@ ;; --- Create File Media object (upload) (declare create-file-media-object) -(declare select-file-for-update) +(declare select-file) (s/def ::content ::media/upload) (s/def ::is-local ::us/boolean) @@ -50,7 +50,7 @@ (sv/defmethod ::upload-file-media-object [{:keys [pool] :as cfg} {:keys [profile-id file-id] :as params}] (db/with-atomic [conn pool] - (let [file (select-file-for-update conn file-id)] + (let [file (select-file conn file-id)] (teams/check-edition-permissions! conn profile-id (:team-id file)) (-> (assoc cfg :conn conn) (create-file-media-object params))))) @@ -66,9 +66,18 @@ [info] (= (:mtype info) "image/svg+xml")) +(defn- fetch-url + [url] + (try + (http/get! url {:as :byte-array}) + (catch Exception e + (ex/raise :type :validation + :code :unable-to-access-to-url + :cause e)))) + (defn- download-media [{:keys [storage] :as cfg} url] - (let [result (http/get! url {:as :byte-array}) + (let [result (fetch-url url) data (:body result) mtype (get (:headers result) "content-type") format (cm/mtype->format mtype)] @@ -129,7 +138,7 @@ (sv/defmethod ::create-file-media-object-from-url [{:keys [pool storage] :as cfg} {:keys [profile-id file-id url name] :as params}] (db/with-atomic [conn pool] - (let [file (select-file-for-update conn file-id)] + (let [file (select-file conn file-id)] (teams/check-edition-permissions! conn profile-id (:team-id file)) (let [mobj (download-media cfg url) content {:filename "tempfile" @@ -152,7 +161,7 @@ (sv/defmethod ::clone-file-media-object [{:keys [pool] :as cfg} {:keys [profile-id file-id] :as params}] (db/with-atomic [conn pool] - (let [file (select-file-for-update conn file-id)] + (let [file (select-file conn file-id)] (teams/check-edition-permissions! conn profile-id (:team-id file)) (-> (assoc cfg :conn conn) @@ -175,17 +184,17 @@ ;; --- HELPERS -(def ^:private sql:select-file-for-update +(def ^:private + sql:select-file "select file.*, project.team_id as team_id from file inner join project on (project.id = file.project_id) - where file.id = ? - for update of file") + where file.id = ?") -(defn- select-file-for-update +(defn- select-file [conn id] - (let [row (db/exec-one! conn [sql:select-file-for-update id])] + (let [row (db/exec-one! conn [sql:select-file id])] (when-not row (ex/raise :type :not-found)) row)) diff --git a/backend/src/app/srepl/main.clj b/backend/src/app/srepl/main.clj index 417410be2..c626a6def 100644 --- a/backend/src/app/srepl/main.clj +++ b/backend/src/app/srepl/main.clj @@ -72,3 +72,17 @@ (let [profile (prof/retrieve-profile-data-by-email conn user-email) profile (merge profile (prof/retrieve-additional-data conn (:id profile)))] (pid/create-profile-initial-data conn file profile))))) + + +;; Migrate + +(defn update-file-data-blob-format + [system] + (db/with-atomic [conn (:app.db/pool system)] + (doseq [id (->> (db/exec! conn ["select id from file;"]) (map :id))] + (let [{:keys [data]} (db/get-by-id conn :file id {:columns [:id :data]})] + (prn "Updating file:" id) + (db/update! conn :file + {:data (-> (blob/decode data) + (blob/encode {:version 2}))} + {:id id}))))) diff --git a/backend/src/app/svgparse.clj b/backend/src/app/svgparse.clj index f7a7c0ce7..f9d658663 100644 --- a/backend/src/app/svgparse.clj +++ b/backend/src/app/svgparse.clj @@ -121,11 +121,16 @@ (defn parse [data] - (with-open [istream (IOUtils/toInputStream data "UTF-8")] - (xml/parse istream))) + (try + (with-open [istream (IOUtils/toInputStream data "UTF-8")] + (xml/parse istream)) + (catch org.xml.sax.SAXParseException _e + (ex/raise :type :validation + :code :invalid-svg-file)))) (defn process-request [{:keys [svgc] :as cfg} body] (let [data (slurp body) data (svgc data)] (parse data))) + diff --git a/backend/src/app/tasks/telemetry.clj b/backend/src/app/tasks/telemetry.clj index bd5f5a3b4..64ca03c8e 100644 --- a/backend/src/app/tasks/telemetry.clj +++ b/backend/src/app/tasks/telemetry.clj @@ -63,6 +63,7 @@ :uri (:uri cfg) :headers {"content-type" "application/json"} :body (json/encode-str data)})] + (when (not= 200 (:status response)) (ex/raise :type :internal :code :invalid-response-from-google @@ -129,7 +130,7 @@ [{:keys [conn version]}] (merge {:version version - :with-taiga (:telemetry-with-taiga cfg/config) + :with-taiga (:telemetry-with-taiga cfg/config false) :total-teams (retrieve-num-teams conn) :total-projects (retrieve-num-projects conn) :total-files (retrieve-num-files conn)} diff --git a/backend/src/app/telemetry.clj b/backend/src/app/telemetry.clj index b97c79360..a8e5edae7 100644 --- a/backend/src/app/telemetry.clj +++ b/backend/src/app/telemetry.clj @@ -12,6 +12,7 @@ [app.common.spec :as us] [app.db :as db] [app.http.middleware :refer [wrap-parse-request-body]] + [clojure.pprint :refer [pprint]] [clojure.spec.alpha :as s] [clojure.tools.logging :as log] [integrant.core :as ig] @@ -87,7 +88,12 @@ (catch Exception e ;; We don't want notify user of a error, just log it for posible ;; future investigation. - (log/warnf e "Unexpected error on telemetry."))) + (log/warn e (str "Unexpected error on telemetry:\n" + (when-let [edata (ex-data e)] + (str "ex-data: \n" + (with-out-str (pprint edata)))) + (str "params: \n" + (with-out-str (pprint params))))))) {:status 200 :body "OK\n"}) diff --git a/frontend/resources/locales.json b/frontend/resources/locales.json index 712a139a9..898a25a64 100644 --- a/frontend/resources/locales.json +++ b/frontend/resources/locales.json @@ -830,6 +830,12 @@ "es" : "Actualizado: %s" } }, + "errors.google-auth-not-enabled" : { + "translations" : { + "en" : "Authentication with google disabled on backend", + "es" : "Autenticación con google esta dehabilitada en el servidor" + } + }, "errors.auth.unauthorized" : { "used-in" : [ "src/app/main/ui/auth/login.cljs:89" ], "translations" : { diff --git a/frontend/src/app/main/data/viewer.cljs b/frontend/src/app/main/data/viewer.cljs index 96d3f1bdd..550b5ddb9 100644 --- a/frontend/src/app/main/data/viewer.cljs +++ b/frontend/src/app/main/data/viewer.cljs @@ -137,7 +137,8 @@ (d/index-by :id) (assoc state :comment-threads))) (on-error [{:keys [type] :as err}] - (if (= :authentication type) + (if (or (= :authentication type) + (= :not-found type)) (rx/empty) (rx/throw err)))] diff --git a/frontend/src/app/main/data/workspace/persistence.cljs b/frontend/src/app/main/data/workspace/persistence.cljs index 4b4f5fea2..c8b260b46 100644 --- a/frontend/src/app/main/data/workspace/persistence.cljs +++ b/frontend/src/app/main/data/workspace/persistence.cljs @@ -417,13 +417,22 @@ (defn- handle-upload-error [on-error stream] (->> stream (rx/catch - (fn on-error [error] + (fn on-error* [error] (if (ex/ex-info? error) - (on-error (ex-data error)) + (on-error* (ex-data error)) (cond + (= (:code error) :invalid-svg-file) + (rx/of (dm/error (tr "errors.media-type-not-allowed"))) + (= (:code error) :media-type-not-allowed) (rx/of (dm/error (tr "errors.media-type-not-allowed"))) + (= (:code error) :ubable-to-access-to-url) + (rx/of (dm/error (tr "errors.media-type-not-allowed"))) + + (= (:code error) :invalid-image) + (rx/of (dm/error (tr "errors.media-type-not-allowed"))) + (= (:code error) :media-too-large) (rx/of (dm/error (tr "errors.media-too-large"))) diff --git a/frontend/src/app/main/ui/auth/login.cljs b/frontend/src/app/main/ui/auth/login.cljs index f85e5acde..bbdc805e7 100644 --- a/frontend/src/app/main/ui/auth/login.cljs +++ b/frontend/src/app/main/ui/auth/login.cljs @@ -37,7 +37,9 @@ (dom/prevent-default event) (->> (rp/mutation! :login-with-google {}) (rx/subs (fn [{:keys [redirect-uri] :as rsp}] - (.replace js/location redirect-uri))))) + (.replace js/location redirect-uri)) + (fn [{:keys [type] :as error}] + (st/emit! (dm/error (tr "errors.google-auth-not-enabled"))))))) (defn- login-with-gitlab [event]