diff --git a/CHANGES.md b/CHANGES.md index 135151c4b..b0a3fdee6 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -15,14 +15,15 @@ ### :bug: Bugs fixed +- Add more improvements to french translation strings [#591](https://github.com/penpot/penpot/pull/591) - Add some missing database indexes (mainly improves performance on large databases on file-update rpc method, and some background tasks). +- Fix corner cases on invitation/signup flows. - Fix problem width handoff code generation [Taiga #1204](https://tree.taiga.io/project/penpot/issue/1204) - Fix problem with indices refreshing on page changes [#646](https://github.com/penpot/penpot/issues/646) - Have language change notification written in the new language [Taiga #1205](https://tree.taiga.io/project/penpot/issue/1205) - Properly handle errors on github, gitlab and ldap auth backends. - Properly mark profile auth backend (on first register/ auth with 3rd party auth provider). -- Fix corner cases on invitation/signup flows. -- Add more improvements to french translation strings [#591](https://github.com/penpot/penpot/pull/591) +- Refactor LDAP auth backend. ### :heart: Community contributions by (Thank you!) diff --git a/backend/src/app/config.clj b/backend/src/app/config.clj index 03822fa15..ac76d9de3 100644 --- a/backend/src/app/config.clj +++ b/backend/src/app/config.clj @@ -70,20 +70,11 @@ :telemetry-enabled false :telemetry-uri "https://telemetry.penpot.app/" - ;; LDAP auth disabled by default. Set ldap-auth-host to enable - ;:ldap-auth-host "ldap.mysupercompany.com" - ;:ldap-auth-port 389 - ;:ldap-bind-dn "cn=admin,dc=ldap,dc=mysupercompany,dc=com" - ;:ldap-bind-password "verysecure" - ;:ldap-auth-ssl false - ;:ldap-auth-starttls false - ;:ldap-auth-base-dn "ou=People,dc=ldap,dc=mysupercompany,dc=com" - - :ldap-auth-user-query "(|(uid=$username)(mail=$username))" - :ldap-auth-username-attribute "uid" - :ldap-auth-email-attribute "mail" - :ldap-auth-fullname-attribute "displayName" - :ldap-auth-avatar-attribute "jpegPhoto" + :ldap-user-query "(|(uid=$username)(mail=$username))" + :ldap-attrs-username "uid" + :ldap-attrs-email "mail" + :ldap-attrs-fullname "cn" + :ldap-attrs-photo "jpegPhoto" ;; :initial-data-file "resources/initial-data.json" ;; :initial-data-project-name "Penpot Oboarding" @@ -152,18 +143,18 @@ (s/def ::github-client-id ::us/string) (s/def ::github-client-secret ::us/string) -(s/def ::ldap-auth-host ::us/string) -(s/def ::ldap-auth-port ::us/integer) +(s/def ::ldap-host ::us/string) +(s/def ::ldap-port ::us/integer) (s/def ::ldap-bind-dn ::us/string) (s/def ::ldap-bind-password ::us/string) -(s/def ::ldap-auth-ssl ::us/boolean) -(s/def ::ldap-auth-starttls ::us/boolean) -(s/def ::ldap-auth-base-dn ::us/string) -(s/def ::ldap-auth-user-query ::us/string) -(s/def ::ldap-auth-username-attribute ::us/string) -(s/def ::ldap-auth-email-attribute ::us/string) -(s/def ::ldap-auth-fullname-attribute ::us/string) -(s/def ::ldap-auth-avatar-attribute ::us/string) +(s/def ::ldap-ssl ::us/boolean) +(s/def ::ldap-starttls ::us/boolean) +(s/def ::ldap-base-dn ::us/string) +(s/def ::ldap-user-query ::us/string) +(s/def ::ldap-attrs-username ::us/string) +(s/def ::ldap-attrs-email ::us/string) +(s/def ::ldap-attrs-fullname ::us/string) +(s/def ::ldap-attrs-photo ::us/string) (s/def ::telemetry-enabled ::us/boolean) (s/def ::telemetry-with-taiga ::us/boolean) @@ -195,18 +186,18 @@ ::google-client-secret ::http-server-port ::host - ::ldap-auth-avatar-attribute - ::ldap-auth-base-dn - ::ldap-auth-email-attribute - ::ldap-auth-fullname-attribute - ::ldap-auth-host - ::ldap-auth-port - ::ldap-auth-ssl - ::ldap-auth-starttls - ::ldap-auth-user-query - ::ldap-auth-username-attribute + ::ldap-attrs-username + ::ldap-attrs-email + ::ldap-attrs-fullname + ::ldap-attrs-photo ::ldap-bind-dn ::ldap-bind-password + ::ldap-base-dn + ::ldap-host + ::ldap-port + ::ldap-ssl + ::ldap-starttls + ::ldap-user-query ::public-uri ::profile-complaint-threshold ::profile-bounce-threshold diff --git a/backend/src/app/http.clj b/backend/src/app/http.clj index 1158f34b9..d6323b370 100644 --- a/backend/src/app/http.clj +++ b/backend/src/app/http.clj @@ -80,14 +80,12 @@ (s/def ::rpc map?) (s/def ::session map?) (s/def ::metrics map?) -(s/def ::google-auth map?) -(s/def ::gitlab-auth map?) -(s/def ::ldap-auth fn?) +(s/def ::oauth map?) (s/def ::storage map?) (s/def ::assets map?) (defmethod ig/pre-init-spec ::router [_] - (s/keys :req-un [::rpc ::session ::metrics ::google-auth ::gitlab-auth ::storage ::assets])) + (s/keys :req-un [::rpc ::session ::metrics ::oauth ::storage ::assets])) (defmethod ig/init-key ::router [_ cfg] @@ -113,7 +111,7 @@ :body "internal server error"}))))))) (defn- create-router - [{:keys [session rpc google-auth gitlab-auth github-auth metrics ldap-auth svgparse assets] :as cfg}] + [{:keys [session rpc oauth metrics svgparse assets] :as cfg}] (rr/router [["/metrics" {:get (:handler metrics)}] @@ -140,16 +138,14 @@ ["/svg" {:post svgparse}] ["/oauth" - ["/google" {:post (:auth-handler google-auth)}] - ["/google/callback" {:get (:callback-handler google-auth)}] + ["/google" {:post (get-in oauth [:google :handler])}] + ["/google/callback" {:get (get-in oauth [:google :callback-handler])}] - ["/gitlab" {:post (:auth-handler gitlab-auth)}] - ["/gitlab/callback" {:get (:callback-handler gitlab-auth)}] + ["/gitlab" {:post (get-in oauth [:gitlab :handler])}] + ["/gitlab/callback" {:get (get-in oauth [:gitlab :callback-handler])}] - ["/github" {:post (:auth-handler github-auth)}] - ["/github/callback" {:get (:callback-handler github-auth)}]] - - ["/login-ldap" {:post ldap-auth}] + ["/github" {:post (get-in oauth [:github :handler])}] + ["/github/callback" {:get (get-in oauth [:github :callback-handler])}]] ["/rpc" {:middleware [(:middleware session)]} ["/query/:type" {:get (:query-handler rpc)}] diff --git a/backend/src/app/http/auth/ldap.clj b/backend/src/app/http/auth/ldap.clj deleted file mode 100644 index 41e260e9f..000000000 --- a/backend/src/app/http/auth/ldap.clj +++ /dev/null @@ -1,127 +0,0 @@ -;; This Source Code Form is subject to the terms of the Mozilla Public -;; License, v. 2.0. If a copy of the MPL was not distributed with this -;; file, You can obtain one at http://mozilla.org/MPL/2.0/. -;; -;; This Source Code Form is "Incompatible With Secondary Licenses", as -;; defined by the Mozilla Public License, v. 2.0. -;; -;; Copyright (c) 2020 UXBOX Labs SL - -(ns app.http.auth.ldap - (:require - [app.common.exceptions :as ex] - [app.config :as cfg] - [clj-ldap.client :as client] - [clojure.set :as set] - [clojure.spec.alpha :as s] - [clojure.string ] - [clojure.tools.logging :as log] - [integrant.core :as ig])) - -(declare authenticate) -(declare create-connection) -(declare replace-several) - - -(s/def ::host ::cfg/ldap-auth-host) -(s/def ::port ::cfg/ldap-auth-port) -(s/def ::ssl ::cfg/ldap-auth-ssl) -(s/def ::starttls ::cfg/ldap-auth-starttls) -(s/def ::user-query ::cfg/ldap-auth-user-query) -(s/def ::base-dn ::cfg/ldap-auth-base-dn) -(s/def ::username-attribute ::cfg/ldap-auth-username-attribute) -(s/def ::email-attribute ::cfg/ldap-auth-email-attribute) -(s/def ::fullname-attribute ::cfg/ldap-auth-fullname-attribute) -(s/def ::avatar-attribute ::cfg/ldap-auth-avatar-attribute) - -(s/def ::rpc map?) -(s/def ::session map?) - -(defmethod ig/pre-init-spec :app.http.auth/ldap - [_] - (s/keys - :req-un [::rpc ::session] - :opt-un [::host - ::port - ::ssl - ::starttls - ::username-attribute - ::base-dn - ::username-attribute - ::email-attribute - ::fullname-attribute - ::avatar-attribute])) - -(defmethod ig/init-key :app.http.auth/ldap - [_ {:keys [session rpc] :as cfg}] - (let [conn (create-connection cfg)] - (with-meta - (fn [request] - (let [data (:body-params request)] - (when-some [info (authenticate (assoc cfg - :conn conn - :username (:email data) - :password (:password data)))] - (let [method-fn (get-in rpc [:methods :mutation :login-or-register]) - profile (method-fn {:email (:email info) - :backend "ldap" - :fullname (:fullname info)}) - - sxf ((:create session) (:id profile)) - rsp {:status 200 :body profile}] - (sxf request rsp))))) - {::conn conn}))) - -(defmethod ig/halt-key! ::client - [_ handler] - (let [{:keys [::conn]} (meta handler)] - (when (realized? conn) - (.close @conn)))) - -(defn- replace-several [s & {:as replacements}] - (reduce-kv clojure.string/replace s replacements)) - -(defn- create-connection - [cfg] - (let [params (merge {:host {:address (:host cfg) - :port (:port cfg)}} - (-> cfg - (select-keys [:ssl - :starttls - :ldap-bind-dn - :ldap-bind-password]) - (set/rename-keys {:ssl :ssl? - :starttls :startTLS? - :ldap-bind-dn :bind-dn - :ldap-bind-password :password})))] - (delay - (try - (client/connect params) - (catch Exception e - (log/errorf e "Cannot connect to LDAP %s:%s" - (:host cfg) (:port cfg))))))) - - -(defn- authenticate - [{:keys [conn username password] :as cfg}] - (when-some [conn (some-> conn deref)] - (let [user-search-query (replace-several (:user-query cfg) "$username" username) - user-attributes (-> cfg - (select-keys [:username-attribute - :email-attribute - :fullname-attribute - :avatar-attribute]) - vals)] - (when-some [user-entry (-> conn - (client/search (:base-dn cfg) - {:filter user-search-query - :sizelimit 1 - :attributes user-attributes}) - (first))] - (when-not (client/bind? conn (:dn user-entry) password) - (ex/raise :type :authentication - :code :wrong-credentials)) - (set/rename-keys user-entry {(keyword (:avatar-attribute cfg)) :photo - (keyword (:fullname-attribute cfg)) :fullname - (keyword (:email-attribute cfg)) :email}))))) - diff --git a/backend/src/app/http/errors.clj b/backend/src/app/http/errors.clj index 3758942f5..020ebc921 100644 --- a/backend/src/app/http/errors.clj +++ b/backend/src/app/http/errors.clj @@ -46,6 +46,11 @@ [err _] {:status 401 :body (ex-data err)}) + +(defmethod handle-exception :restriction + [err _] + {:status 400 :body (ex-data err)}) + (defmethod handle-exception :validation [err req] (let [header (get-in req [:headers "accept"]) diff --git a/backend/src/app/http/auth/github.clj b/backend/src/app/http/oauth/github.clj similarity index 95% rename from backend/src/app/http/auth/github.clj rename to backend/src/app/http/oauth/github.clj index 3d8552efa..bfa4c3c14 100644 --- a/backend/src/app/http/auth/github.clj +++ b/backend/src/app/http/oauth/github.clj @@ -7,12 +7,12 @@ ;; ;; Copyright (c) 2020-2021 UXBOX Labs SL -(ns app.http.auth.github +(ns app.http.oauth.github (:require [app.common.exceptions :as ex] [app.common.spec :as us] [app.config :as cfg] - [app.http.auth.google :as gg] + [app.http.oauth.google :as gg] [app.util.http :as http] [app.util.time :as dt] [clojure.data.json :as json] @@ -137,7 +137,7 @@ (s/def ::session map?) (s/def ::tokens fn?) -(defmethod ig/pre-init-spec :app.http.auth/github [_] +(defmethod ig/pre-init-spec :app.http.oauth/github [_] (s/keys :req-un [::public-uri ::session ::tokens] @@ -148,12 +148,12 @@ [_] (ex/raise :type :not-found)) -(defmethod ig/init-key :app.http.auth/github +(defmethod ig/init-key :app.http.oauth/github [_ cfg] (if (and (:client-id cfg) (:client-secret cfg)) - {:auth-handler #(auth-handler cfg %) + {:handler #(auth-handler cfg %) :callback-handler #(callback-handler cfg %)} - {:auth-handler default-handler + {:handler default-handler :callback-handler default-handler})) diff --git a/backend/src/app/http/auth/gitlab.clj b/backend/src/app/http/oauth/gitlab.clj similarity index 94% rename from backend/src/app/http/auth/gitlab.clj rename to backend/src/app/http/oauth/gitlab.clj index e932dfdba..4a8b0a7c2 100644 --- a/backend/src/app/http/auth/gitlab.clj +++ b/backend/src/app/http/oauth/gitlab.clj @@ -7,12 +7,12 @@ ;; ;; Copyright (c) 2020 UXBOX Labs SL -(ns app.http.auth.gitlab +(ns app.http.oauth.gitlab (:require [app.common.data :as d] [app.common.exceptions :as ex] [app.common.spec :as us] - [app.http.auth.google :as gg] + [app.http.oauth.google :as gg] [app.util.http :as http] [app.util.time :as dt] [clojure.data.json :as json] @@ -140,7 +140,7 @@ (s/def ::session map?) (s/def ::tokens fn?) -(defmethod ig/pre-init-spec :app.http.auth/gitlab [_] +(defmethod ig/pre-init-spec :app.http.oauth/gitlab [_] (s/keys :req-un [::public-uri ::session ::tokens] @@ -148,8 +148,7 @@ ::client-id ::client-secret])) - -(defmethod ig/prep-key :app.http.auth/gitlab +(defmethod ig/prep-key :app.http.oauth/gitlab [_ cfg] (d/merge {:base-uri "https://gitlab.com"} (d/without-nils cfg))) @@ -158,11 +157,11 @@ [_] (ex/raise :type :not-found)) -(defmethod ig/init-key :app.http.auth/gitlab +(defmethod ig/init-key :app.http.oauth/gitlab [_ cfg] (if (and (:client-id cfg) (:client-secret cfg)) - {:auth-handler #(auth-handler cfg %) + {:handler #(auth-handler cfg %) :callback-handler #(callback-handler cfg %)} - {:auth-handler default-handler + {:handler default-handler :callback-handler default-handler})) diff --git a/backend/src/app/http/auth/google.clj b/backend/src/app/http/oauth/google.clj similarity index 96% rename from backend/src/app/http/auth/google.clj rename to backend/src/app/http/oauth/google.clj index 3f082d064..ce9211689 100644 --- a/backend/src/app/http/auth/google.clj +++ b/backend/src/app/http/oauth/google.clj @@ -7,7 +7,7 @@ ;; ;; Copyright (c) 2020-2021 UXBOX Labs SL -(ns app.http.auth.google +(ns app.http.oauth.google (:require [app.common.exceptions :as ex] [app.common.spec :as us] @@ -50,7 +50,6 @@ (when (= 200 (:status res)) (-> (json/read-str (:body res)) (get "access_token")))) - (catch Exception e (log/error e "unexpected error on get-access-token") nil))) @@ -60,8 +59,10 @@ (try (let [req {:uri "https://openidconnect.googleapis.com/v1/userinfo" :headers {"Authorization" (str "Bearer " token)} + :timeout 6000 :method :get} res (http/send! req)] + (when (= 200 (:status res)) (let [data (json/read-str (:body res))] {:email (get data "email") @@ -78,6 +79,8 @@ info (some->> (get-in request [:params :code]) (get-access-token cfg) (get-user-info cfg))] + + (when-not info (ex/raise :type :internal :code :unable-to-auth)) @@ -158,7 +161,7 @@ (s/def ::session map?) (s/def ::tokens fn?) -(defmethod ig/pre-init-spec :app.http.auth/google [_] +(defmethod ig/pre-init-spec :app.http.oauth/google [_] (s/keys :req-un [::public-uri ::session ::tokens] @@ -169,11 +172,11 @@ [_] (ex/raise :type :not-found)) -(defmethod ig/init-key :app.http.auth/google +(defmethod ig/init-key :app.http.oauth/google [_ cfg] (if (and (:client-id cfg) (:client-secret cfg)) - {:auth-handler #(auth-handler cfg %) + {:handler #(auth-handler cfg %) :callback-handler #(callback-handler cfg %)} - {:auth-handler default-handler + {:handler default-handler :callback-handler default-handler})) diff --git a/backend/src/app/loggers/mattermost.clj b/backend/src/app/loggers/mattermost.clj index 7f40a48bd..2b95a0d3d 100644 --- a/backend/src/app/loggers/mattermost.clj +++ b/backend/src/app/loggers/mattermost.clj @@ -95,7 +95,7 @@ (= k :profile-id) (assoc acc k (uuid/uuid v)) (str/blank? v) acc :else (assoc acc k v))) - {} + {:id (uuid/next)} (:context event))) (defn- parse-event diff --git a/backend/src/app/main.clj b/backend/src/app/main.clj index 2d4745719..c9bc23ce0 100644 --- a/backend/src/app/main.clj +++ b/backend/src/app/main.clj @@ -87,10 +87,7 @@ :tokens (ig/ref :app.tokens/tokens) :public-uri (:public-uri config) :metrics (ig/ref :app.metrics/metrics) - :google-auth (ig/ref :app.http.auth/google) - :gitlab-auth (ig/ref :app.http.auth/gitlab) - :github-auth (ig/ref :app.http.auth/github) - :ldap-auth (ig/ref :app.http.auth/ldap) + :oauth (ig/ref :app.http.oauth/all) :assets (ig/ref :app.http.assets/handlers) :svgparse (ig/ref :app.svgparse/handler) :storage (ig/ref :app.storage/storage) @@ -104,7 +101,12 @@ :cache-max-age (dt/duration {:hours 24}) :signature-max-age (dt/duration {:hours 24 :minutes 5})} - :app.http.auth/google + :app.http.oauth/all + {:google (ig/ref :app.http.oauth/google) + :gitlab (ig/ref :app.http.oauth/gitlab) + :github (ig/ref :app.http.oauth/github)} + + :app.http.oauth/google {:rpc (ig/ref :app.rpc/rpc) :session (ig/ref :app.http.session/session) :tokens (ig/ref :app.tokens/tokens) @@ -112,7 +114,7 @@ :client-id (:google-client-id config) :client-secret (:google-client-secret config)} - :app.http.auth/github + :app.http.oauth/github {:rpc (ig/ref :app.rpc/rpc) :session (ig/ref :app.http.session/session) :tokens (ig/ref :app.tokens/tokens) @@ -120,7 +122,7 @@ :client-id (:github-client-id config) :client-secret (:github-client-secret config)} - :app.http.auth/gitlab + :app.http.oauth/gitlab {:rpc (ig/ref :app.rpc/rpc) :session (ig/ref :app.http.session/session) :tokens (ig/ref :app.tokens/tokens) @@ -129,20 +131,6 @@ :client-id (:gitlab-client-id config) :client-secret (:gitlab-client-secret config)} - :app.http.auth/ldap - {:host (:ldap-auth-host config) - :port (:ldap-auth-port config) - :ssl (:ldap-auth-ssl config) - :starttls (:ldap-auth-starttls config) - :user-query (:ldap-auth-user-query config) - :username-attribute (:ldap-auth-username-attribute config) - :email-attribute (:ldap-auth-email-attribute config) - :fullname-attribute (:ldap-auth-fullname-attribute config) - :avatar-attribute (:ldap-auth-avatar-attribute config) - :base-dn (:ldap-auth-base-dn config) - :session (ig/ref :app.http.session/session) - :rpc (ig/ref :app.rpc/rpc)} - :app.svgparse/svgc {:metrics (ig/ref :app.metrics/metrics)} diff --git a/backend/src/app/rpc.clj b/backend/src/app/rpc.clj index 28f9e2f9e..297d6008f 100644 --- a/backend/src/app/rpc.clj +++ b/backend/src/app/rpc.clj @@ -127,6 +127,7 @@ 'app.rpc.mutations.viewer 'app.rpc.mutations.teams 'app.rpc.mutations.feedback + 'app.rpc.mutations.ldap 'app.rpc.mutations.verify-token) (map (partial process-method cfg)) (into {})))) diff --git a/backend/src/app/rpc/mutations/files.clj b/backend/src/app/rpc/mutations/files.clj index 990273881..dce05f643 100644 --- a/backend/src/app/rpc/mutations/files.clj +++ b/backend/src/app/rpc/mutations/files.clj @@ -265,7 +265,7 @@ (assoc params :file file))))) (defn- update-file - [{:keys [conn] :as cfg} {:keys [file changes session-id] :as params}] + [{:keys [conn] :as cfg} {:keys [file changes session-id profile-id] :as params}] (when (> (:revn params) (:revn file)) (ex/raise :type :validation @@ -287,6 +287,7 @@ (db/insert! conn :file-change {:id (uuid/next) :session-id session-id + :profile-id profile-id :file-id (:id file) :revn (:revn file) :data (:data file) diff --git a/backend/src/app/rpc/mutations/ldap.clj b/backend/src/app/rpc/mutations/ldap.clj new file mode 100644 index 000000000..e8edfc879 --- /dev/null +++ b/backend/src/app/rpc/mutations/ldap.clj @@ -0,0 +1,105 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; This Source Code Form is "Incompatible With Secondary Licenses", as +;; defined by the Mozilla Public License, v. 2.0. +;; +;; Copyright (c) 2020-2021 UXBOX Labs SL + +(ns app.rpc.mutations.ldap + (:require + [app.common.exceptions :as ex] + [app.common.spec :as us] + [app.config :as cfg] + [app.rpc.mutations.profile :refer [login-or-register]] + [app.util.services :as sv] + [clj-ldap.client :as ldap] + [clojure.spec.alpha :as s] + [clojure.string] + [clojure.tools.logging :as log])) + +(def cpool + (delay + (let [params {:ssl? (cfg/get :ldap-ssl) + :startTLS? (cfg/get :ldap-starttls) + :bind-dn (cfg/get :ldap-bind-dn) + :password (cfg/get :ldap-bind-password) + :host {:address (cfg/get :ldap-host) + :port (cfg/get :ldap-port)}}] + (try + (ldap/connect params) + (catch Exception e + (log/errorf e "Cannot connect to LDAP %s:%s" + (get-in params [:host :address]) + (get-in params [:host :port]))))))) + +;; --- Mutation: login-with-ldap + +(declare authenticate) + +(s/def ::email ::us/email) +(s/def ::password ::us/string) +(s/def ::invitation-token ::us/string) + +(s/def ::login-with-ldap + (s/keys :req-un [::email ::password] + :opt-un [::invitation-token])) + +(sv/defmethod ::login-with-ldap {:auth false :rlimit :password} + [{:keys [pool session tokens] :as cfg} {:keys [email password invitation-token] :as params}] + (when-not @cpool + (ex/raise :type :restriction + :code :ldap-disabled + :hint "ldap disabled or unable to connect")) + + (let [info (authenticate @cpool params) + cfg (assoc cfg :conn pool)] + (when-not info + (ex/raise :type :validation + :code :wrong-credentials)) + (let [profile (login-or-register cfg {:email (:email info) + :backend (:backend info) + :fullname (:fullname info)})] + (if-let [token (:invitation-token params)] + ;; If invitation token comes in params, this is because the + ;; user comes from team-invitation process; in this case, + ;; regenerate token and send back to the user a new invitation + ;; token (and mark current session as logged). + (let [claims (tokens :verify {:token token :iss :team-invitation}) + claims (assoc claims + :member-id (:id profile) + :member-email (:email profile)) + token (tokens :generate claims)] + (with-meta + {:invitation-token token} + {:transform-response ((:create session) (:id profile))})) + + (with-meta profile + {:transform-response ((:create session) (:id profile))}))))) + +(defn- replace-several [s & {:as replacements}] + (reduce-kv clojure.string/replace s replacements)) + +(defn- get-ldap-user + [cpool {:keys [email] :as params}] + (let [query (-> (cfg/get :ldap-user-query) + (replace-several "$username" email)) + + attrs [(cfg/get :ldap-attrs-username) + (cfg/get :ldap-attrs-email) + (cfg/get :ldap-attrs-photo) + (cfg/get :ldap-attrs-fullname)] + + base-dn (cfg/get :ldap-base-dn) + params {:filter query :sizelimit 1 :attributes attrs}] + (first (ldap/search cpool base-dn params)))) + +(defn- authenticate + [cpool {:keys [password] :as params}] + (when-let [{:keys [dn] :as luser} (get-ldap-user cpool params)] + (when (ldap/bind? cpool dn password) + {:photo (get luser (keyword (cfg/get :ldap-attrs-photo))) + :fullname (get luser (keyword (cfg/get :ldap-attrs-fullname))) + :email (get luser (keyword (cfg/get :ldap-attrs-email))) + :backend "ldap"}))) diff --git a/backend/src/app/rpc/mutations/profile.clj b/backend/src/app/rpc/mutations/profile.clj index 4ea7d0c8b..339738d57 100644 --- a/backend/src/app/rpc/mutations/profile.clj +++ b/backend/src/app/rpc/mutations/profile.clj @@ -204,10 +204,10 @@ (s/def ::login (s/keys :req-un [::email ::password] - :opt-un [::scope])) + :opt-un [::scope ::invitation-token])) (sv/defmethod ::login {:auth false :rlimit :password} - [{:keys [pool session] :as cfg} {:keys [email password scope] :as params}] + [{:keys [pool session tokens] :as cfg} {:keys [email password scope] :as params}] (letfn [(check-password [profile password] (when (= (:password profile) "!") (ex/raise :type :validation @@ -227,14 +227,27 @@ profile)] (db/with-atomic [conn pool] - (let [prof (-> (profile/retrieve-profile-data-by-email conn email) - (validate-profile) - (profile/strip-private-attrs)) - addt (profile/retrieve-additional-data conn (:id prof)) - prof (merge prof addt)] - (with-meta prof - {:transform-response ((:create session) (:id prof))}))))) + (let [profile (-> (profile/retrieve-profile-data-by-email conn email) + (validate-profile) + (profile/strip-private-attrs)) + profile (merge profile (profile/retrieve-additional-data conn (:id profile)))] + (if-let [token (:invitation-token params)] + ;; If the request comes with an invitation token, this means + ;; that user wants to accept it with different user. A very + ;; strange case but still can happen. In this case, we + ;; proceed in the same way as in register: regenerate the + ;; invitation token and return it to the user for proper + ;; invitation acceptation. + (let [claims (tokens :verify {:token token :iss :team-invitation}) + claims (assoc claims + :member-id (:id profile) + :member-email (:email profile)) + token (tokens :generate claims)] + (with-meta {:invitation-token token} + {:transform-response ((:create session) (:id profile))})) + (with-meta profile + {:transform-response ((:create session) (:id profile))})))))) ;; --- Mutation: Logout @@ -249,12 +262,20 @@ ;; --- Mutation: Register if not exists +(declare login-or-register) + (s/def ::backend ::us/string) (s/def ::login-or-register (s/keys :req-un [::email ::fullname ::backend])) (sv/defmethod ::login-or-register {:auth false} - [{:keys [pool] :as cfg} {:keys [email backend fullname] :as params}] + [{:keys [pool] :as cfg} params] + (db/with-atomic [conn pool] + (-> (assoc cfg :conn conn) + (login-or-register params)))) + +(defn login-or-register + [{:keys [conn] :as cfg} {:keys [email backend] :as params}] (letfn [(populate-additional-data [conn profile] (let [data (profile/retrieve-additional-data conn (:id profile))] (merge profile data))) @@ -275,12 +296,11 @@ (create-profile-initial-data conn profile) profile))] - (db/with-atomic [conn pool] - (let [profile (profile/retrieve-profile-data-by-email conn email) - profile (if profile - (populate-additional-data conn profile) - (register-profile conn params))] - (profile/strip-private-attrs profile))))) + (let [profile (profile/retrieve-profile-data-by-email conn email) + profile (if profile + (populate-additional-data conn profile) + (register-profile conn params))] + (profile/strip-private-attrs profile)))) ;; --- Mutation: Update Profile (own) diff --git a/backend/src/app/rpc/mutations/teams.clj b/backend/src/app/rpc/mutations/teams.clj index c5e37d001..d845775f1 100644 --- a/backend/src/app/rpc/mutations/teams.clj +++ b/backend/src/app/rpc/mutations/teams.clj @@ -303,7 +303,7 @@ team (db/get-by-id conn :team team-id) itoken (tokens :generate {:iss :team-invitation - :exp (dt/in-future "24h") + :exp (dt/in-future "6h") :profile-id (:id profile) :role role :team-id team-id diff --git a/backend/src/app/rpc/mutations/verify_token.clj b/backend/src/app/rpc/mutations/verify_token.clj index dbeadaf7a..6a32bb7d4 100644 --- a/backend/src/app/rpc/mutations/verify_token.clj +++ b/backend/src/app/rpc/mutations/verify_token.clj @@ -115,7 +115,7 @@ ;; If the current session is already matches the invited ;; member, then just return the token and leave the frontend ;; app redirect to correct team. - (assoc claims :status :created) + (assoc claims :state :created) ;; If the session does not matches the invited member, replace ;; the session with a new one matching the invited member. @@ -123,7 +123,7 @@ ;; user clicking the link he already has access to the email ;; account. (with-meta - (assoc claims :status :created) + (assoc claims :state :created) {:transform-response ((:create session) member-id)}))) ;; This happens when member-id is not filled in the invitation but diff --git a/backend/src/app/util/services.clj b/backend/src/app/util/services.clj index e652182be..edc8c1074 100644 --- a/backend/src/app/util/services.clj +++ b/backend/src/app/util/services.clj @@ -21,7 +21,7 @@ ::spec sname ::name (name sname)) - sym (symbol (str "service-method-" (name sname)))] + sym (symbol (str "sm$" (name sname)))] `(do (def ~sym (fn ~args ~@body)) (reset-meta! (var ~sym) ~mdata)))) diff --git a/docker/devenv/docker-compose.yaml b/docker/devenv/docker-compose.yaml index 0c105f87f..789ee0cc2 100644 --- a/docker/devenv/docker-compose.yaml +++ b/docker/devenv/docker-compose.yaml @@ -53,6 +53,7 @@ services: - PENPOT_SMTP_PASSWORD= - PENPOT_SMTP_SSL=false - PENPOT_SMTP_TLS=false + # LDAP setup - PENPOT_LDAP_HOST=ldap - PENPOT_LDAP_PORT=10389 @@ -61,10 +62,10 @@ services: - PENPOT_LDAP_BASE_DN=ou=people,dc=planetexpress,dc=com - PENPOT_LDAP_BIND_DN=cn=admin,dc=planetexpress,dc=com - PENPOT_LDAP_BIND_PASSWORD=GoodNewsEveryone - - PENPOT_LDAP_USERNAME_ATTRIBUTE=uid - - PENPOT_LDAP_EMAIL_ATTRIBUTE=mail - - PENPOT_LDAP_FULLNAME_ATTRIBUTE=displayName - - PENPOT_LDAP_AVATAR_ATTRIBUTE=jpegPhoto + - PENPOT_LDAP_ATTRS_USERNAME=uid + - PENPOT_LDAP_ATTRS_EMAIL=mail + - PENPOT_LDAP_ATTRS_FULLNAME=cn + - PENPOT_LDAP_ATTRS_PHOTO=jpegPhoto postgres: image: postgres:13 diff --git a/frontend/resources/locales.json b/frontend/resources/locales.json index e3c5debcc..00c1fcbde 100644 --- a/frontend/resources/locales.json +++ b/frontend/resources/locales.json @@ -773,7 +773,13 @@ "es" : "Actualizado: %s" } }, - "errors.auth.unauthorized" : { + "errors.ldap-disabled" : { + "translations" : { + "en" : "LDAP authentication is disabled.", + "es" : "La autheticacion via LDAP esta deshabilitada." + } + }, + "errors.wrong-credentials" : { "used-in" : [ "src/app/main/ui/auth/login.cljs" ], "translations" : { "en" : "Username or password seems to be wrong.", diff --git a/frontend/resources/styles/common/framework.scss b/frontend/resources/styles/common/framework.scss index 55089a604..fbd547eb9 100644 --- a/frontend/resources/styles/common/framework.scss +++ b/frontend/resources/styles/common/framework.scss @@ -1160,7 +1160,10 @@ input[type=range]:focus::-ms-fill-upper { .icon { padding: $small; - width: 40px; + width: 48px; + height: 48px; + justify-content: center; + align-items: center; } .content { @@ -1169,6 +1172,7 @@ input[type=range]:focus::-ms-fill-upper { font-size: $fs14; padding: $small; width: 100%; + align-items: center; } } @@ -1227,7 +1231,6 @@ input[type=range]:focus::-ms-fill-upper { &.inline { width: 100%; - margin-bottom: $big; } } diff --git a/frontend/src/app/main/data/auth.cljs b/frontend/src/app/main/data/auth.cljs index 8c4871dca..483f00c30 100644 --- a/frontend/src/app/main/data/auth.cljs +++ b/frontend/src/app/main/data/auth.cljs @@ -79,30 +79,6 @@ (watch [this state s] (rx/of (logged-in profile))))) -(defn login-with-ldap - [{:keys [email password] :as data}] - (us/verify ::login-params data) - (ptk/reify ::login-with-ldap - ptk/UpdateEvent - (update [_ state] - (merge state (dissoc initial-state :route :router))) - - ptk/WatchEvent - (watch [this state s] - (let [{:keys [on-error on-success] - :or {on-error identity - on-success identity}} (meta data) - params {:email email - :password password - :scope "webapp"}] - (->> (rx/timer 100) - (rx/mapcat #(rp/mutation :login-with-ldap params)) - (rx/tap on-success) - (rx/catch (fn [err] - (on-error err) - (rx/empty))) - (rx/map logged-in)))))) - ;; --- Logout (def clear-user-data @@ -131,10 +107,11 @@ ;; --- Register +(s/def ::invitation-token ::us/not-empty-string) + (s/def ::register - (s/keys :req-un [::fullname - ::password - ::email])) + (s/keys :req-un [::fullname ::password ::email] + :opt-un [::invitation-token])) (defn register "Create a register event instance." diff --git a/frontend/src/app/main/repo.cljs b/frontend/src/app/main/repo.cljs index cb914d078..462284eb5 100644 --- a/frontend/src/app/main/repo.cljs +++ b/frontend/src/app/main/repo.cljs @@ -122,11 +122,5 @@ (seq params)) (send-mutation! id form))) -(defmethod mutation :login-with-ldap - [id params] - (let [uri (str cfg/public-uri "/api/login-ldap")] - (->> (http/send! {:method :post :uri uri :body params}) - (rx/mapcat handle-response)))) - (def client-error? http/client-error?) (def server-error? http/server-error?) diff --git a/frontend/src/app/main/ui/auth.cljs b/frontend/src/app/main/ui/auth.cljs index ecb65ec20..29dd7af45 100644 --- a/frontend/src/app/main/ui/auth.cljs +++ b/frontend/src/app/main/ui/auth.cljs @@ -49,7 +49,7 @@ [:& register-success-page {:params params}] :auth-login - [:& login-page {:locale locale :params params}] + [:& login-page {:params params}] :auth-recovery-request [:& recovery-request-page {:locale locale}] diff --git a/frontend/src/app/main/ui/auth/login.cljs b/frontend/src/app/main/ui/auth/login.cljs index e9a4db5cf..15b663531 100644 --- a/frontend/src/app/main/ui/auth/login.cljs +++ b/frontend/src/app/main/ui/auth/login.cljs @@ -55,21 +55,40 @@ (rx/subs (fn [{:keys [redirect-uri] :as rsp}] (.replace js/location redirect-uri))))) +(defn- login-with-ldap + [event params] + (dom/prevent-default event) + (dom/stop-propagation event) + (let [{:keys [on-error]} (meta params)] + (->> (rp/mutation! :login-with-ldap params) + (rx/subs (fn [profile] + (if-let [token (:invitation-token profile)] + (st/emit! (rt/nav :auth-verify-token {} {:token token})) + (st/emit! (da/logged-in profile)))) + (fn [{:keys [type code] :as error}] + (cond + (and (= type :restriction) + (= code :ldap-disabled)) + (st/emit! (dm/error (tr "errors.ldap-disabled"))) + + (fn? on-error) + (on-error error))))))) + (mf/defc login-form - [] - (let [error? (mf/use-state false) - form (fm/use-form :spec ::login-form - :inital {}) + [{:keys [params] :as props}] + (let [error (mf/use-state false) + form (fm/use-form :spec ::login-form + :inital {}) on-error - (fn [form event] - (reset! error? true)) + (fn [_] + (reset! error (tr "errors.wrong-credentials"))) on-submit (mf/use-callback (mf/deps form) (fn [event] - (reset! error? false) + (reset! error nil) (let [params (with-meta (:clean-data @form) {:on-error on-error})] (st/emit! (da/login params))))) @@ -78,17 +97,15 @@ (mf/use-callback (mf/deps form) (fn [event] - (reset! error? false) - (let [params (with-meta (:clean-data @form) - {:on-error on-error})] - (st/emit! (da/login-with-ldap params)))))] + (let [params (merge (:clean-data @form) params)] + (login-with-ldap event (with-meta params {:on-error on-error})))))] [:* - (when @error? + (when-let [message @error] [:& msgs/inline-banner {:type :warning - :content (tr "errors.auth.unauthorized") - :on-close #(reset! error? false)}]) + :content message + :on-close #(reset! error nil)}]) [:& fm/form {:on-submit on-submit :form form} [:div.fields-row @@ -114,13 +131,13 @@ :on-click on-submit-ldap}])]])) (mf/defc login-page - [] + [{:keys [params] :as props}] [:div.generic-form.login-form [:div.form-container [:h1 (tr "auth.login-title")] [:div.subtitle (tr "auth.login-subtitle")] - [:& login-form {}] + [:& login-form {:params params}] [:div.links [:div.link-entry @@ -130,25 +147,25 @@ [:div.link-entry [:span (tr "auth.register") " "] - [:a {:on-click #(st/emit! (rt/nav :auth-register)) + [:a {:on-click #(st/emit! (rt/nav :auth-register {} params)) :tab-index "6"} (tr "auth.register-submit")]]] (when cfg/google-client-id [:a.btn-ocean.btn-large.btn-google-auth - {:on-click login-with-google} + {:on-click #(login-with-google % params)} "Login with Google"]) (when cfg/gitlab-client-id [:a.btn-ocean.btn-large.btn-gitlab-auth - {:on-click login-with-gitlab} + {:on-click #(login-with-gitlab % params)} [:img.logo {:src "/images/icons/brand-gitlab.svg"}] (tr "auth.login-with-gitlab-submit")]) (when cfg/github-client-id [:a.btn-ocean.btn-large.btn-github-auth - {:on-click login-with-github} + {:on-click #(login-with-github % params)} [:img.logo {:src "/images/icons/brand-github.svg"}] (tr "auth.login-with-github-submit")]) diff --git a/frontend/src/app/main/ui/auth/register.cljs b/frontend/src/app/main/ui/auth/register.cljs index ec28f1ce7..63b59b1b9 100644 --- a/frontend/src/app/main/ui/auth/register.cljs +++ b/frontend/src/app/main/ui/auth/register.cljs @@ -43,13 +43,11 @@ (s/def ::fullname ::us/not-empty-string) (s/def ::password ::us/not-empty-string) (s/def ::email ::us/email) -(s/def ::token ::us/not-empty-string) +(s/def ::invitation-token ::us/not-empty-string) (s/def ::register-form - (s/keys :req-un [::password - ::fullname - ::email] - :opt-un [::token])) + (s/keys :req-un [::password ::fullname ::email] + :opt-un [::invitation-token])) (mf/defc register-form [{:keys [params] :as props}] @@ -145,7 +143,7 @@ [:div.links [:div.link-entry [:span (tr "auth.already-have-account") " "] - [:a {:on-click #(st/emit! (rt/nav :auth-login)) + [:a {:on-click #(st/emit! (rt/nav :auth-login {} params)) :tab-index "4"} (tr "auth.login-here")]] diff --git a/frontend/src/app/main/ui/auth/verify_token.cljs b/frontend/src/app/main/ui/auth/verify_token.cljs index 349b451d0..403fd25bd 100644 --- a/frontend/src/app/main/ui/auth/verify_token.cljs +++ b/frontend/src/app/main/ui/auth/verify_token.cljs @@ -52,10 +52,9 @@ [tdata] (case (:state tdata) :created - (let [message (tr "auth.notifications.team-invitation-accepted")] - (st/emit! (du/fetch-profile) - (rt/nav :dashboard-projects {:team-id (:team-id tdata)}) - (dm/success message))) + (st/emit! (dm/success (tr "auth.notifications.team-invitation-accepted")) + (du/fetch-profile) + (rt/nav :dashboard-projects {:team-id (:team-id tdata)})) :pending (let [token (:invitation-token tdata)]