diff --git a/CHANGES.md b/CHANGES.md index 1cbc9a5f9b..598d5fd771 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -4,11 +4,46 @@ ### :boom: Breaking changes & Deprecations -- The backend RPC API URLS are changed from `/api/rpc/command/` - to `/api/main/methods/` (the previou PATH is preserved for - backward compatibility; however, if you are a user of this API, it - is strongly recommended that you adapt your code to use the new - PATH. +#### Backend RPC API changes + +The backend RPC API URLS are changed from `/api/rpc/command/` to +`/api/main/methods/` (the previou PATH is preserved for backward +compatibility; however, if you are a user of this API, it is strongly +recommended that you adapt your code to use the new PATH. + + +#### Updated SSO Callback URL + +The OAuth / Single Sign-On (SSO) callback endpoint has changed to +align with the new OpenID Connect (OIDC) implementation. + +Old callback URL: + +``` +https:///api/auth/oauth//callback +``` + +New callback URL: + +``` +https:///api/auth/oidc/callback +``` + +**Action required:** + +If you have SSO/Social-Auth configured on your on-premise instance, +the following actions are required before update: + +Update your OAuth or SSO provider configuration (e.g., Okta, Google, +Azure AD, etc.) to use the new callback URL. Failure to update may +result in authentication failures after upgrading. + +**Reason for change:** + +This update standardizes all authentication flows under the single URL +and makis it more modular, enabling the ability to configure SSO auth +provider dinamically. + ### :rocket: Epics and highlights diff --git a/backend/scripts/_env b/backend/scripts/_env index fb1d8d8e69..2709853db6 100644 --- a/backend/scripts/_env +++ b/backend/scripts/_env @@ -7,12 +7,12 @@ export PENPOT_HOST=devenv export PENPOT_FLAGS="\ $PENPOT_FLAGS \ - enable-login-with-ldap \ enable-login-with-password - enable-login-with-oidc \ - enable-login-with-google \ - enable-login-with-github \ - enable-login-with-gitlab \ + disable-login-with-ldap \ + disable-login-with-oidc \ + disable-login-with-google \ + disable-login-with-github \ + disable-login-with-gitlab \ enable-backend-worker \ enable-backend-asserts \ disable-feature-fdata-pointer-map \ diff --git a/backend/src/app/auth/oidc.clj b/backend/src/app/auth/oidc.clj index 989dbf5c83..93330b4e4d 100644 --- a/backend/src/app/auth/oidc.clj +++ b/backend/src/app/auth/oidc.clj @@ -15,17 +15,15 @@ [app.common.schema :as sm] [app.common.time :as ct] [app.common.uri :as u] + [app.common.uuid :as uuid] [app.config :as cf] [app.db :as db] [app.email.blacklist :as email.blacklist] [app.email.whitelist :as email.whitelist] [app.http.client :as http] [app.http.errors :as errors] - [app.http.middleware :as mw] - [app.http.security :as sec] [app.http.session :as session] [app.loggers.audit :as audit] - [app.rpc :as rpc] [app.rpc.commands.profile :as profile] [app.setup :as-alias setup] [app.tokens :as tokens] @@ -36,7 +34,6 @@ [clojure.set :as set] [cuerdas.core :as str] [integrant.core :as ig] - [yetti.request :as yreq] [yetti.response :as-alias yres])) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -55,61 +52,53 @@ ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (defn- discover-oidc-config - [cfg {:keys [base-uri] :as opts}] - (let [uri (dm/str (u/join base-uri ".well-known/openid-configuration")) - rsp (http/req! cfg {:method :get :uri uri} {:sync? true})] + [cfg {:keys [base-uri] :as provider}] + (let [uri (u/join base-uri ".well-known/openid-configuration") + rsp (http/req! cfg {:method :get :uri (dm/str uri)})] + (if (= 200 (:status rsp)) - (let [data (-> rsp :body json/decode) - token-uri (get data :token_endpoint) - auth-uri (get data :authorization_endpoint) - user-uri (get data :userinfo_endpoint) - jwks-uri (get data :jwks_uri)] + (let [data (-> rsp :body json/decode) + token-uri (get data :token_endpoint) + auth-uri (get data :authorization_endpoint) + user-uri (get data :userinfo_endpoint) + jwks-uri (get data :jwks_uri) + logout-uri (get data :end_session_endpoint)] - (l/debug :hint "oidc uris discovered" - :token-uri token-uri - :auth-uri auth-uri - :user-uri user-uri - :jwks-uri jwks-uri) + (-> provider + (assoc :token-uri token-uri) + (assoc :auth-uri auth-uri) + (assoc :user-uri user-uri) + (assoc :jwks-uri jwks-uri) + (assoc :logout-uri logout-uri))) - {:token-uri token-uri - :auth-uri auth-uri - :user-uri user-uri - :jwks-uri jwks-uri}) - (do - (l/warn :hint "unable to discover OIDC configuration" + (ex/raise :type ::internal + :code :invalid-sso-config + :hint "unable to discover OIDC configuration" :discover-uri uri - :http-status (:status rsp)) - nil)))) + :response-status-code (:status rsp))))) -(defn- prepare-oidc-opts - [cfg] - (let [opts {:base-uri (cf/get :oidc-base-uri) - :client-id (cf/get :oidc-client-id) - :client-secret (cf/get :oidc-client-secret) - :token-uri (cf/get :oidc-token-uri) - :auth-uri (cf/get :oidc-auth-uri) - :user-uri (cf/get :oidc-user-uri) - :jwks-uri (cf/get :oidc-jwks-uri) - :scopes (cf/get :oidc-scopes #{"openid" "profile" "email"}) - :roles-attr (cf/get :oidc-roles-attr) - :roles (cf/get :oidc-roles) - :name "oidc"} +(def ^:private default-oidc-scopes + #{"openid" "profile" "email"}) - opts (d/without-nils opts)] - - (when (and (string? (:base-uri opts)) - (string? (:client-id opts)) - (string? (:client-secret opts))) - (if (and (string? (:token-uri opts)) - (string? (:user-uri opts)) - (string? (:auth-uri opts))) - opts - (try - (-> (discover-oidc-config cfg opts) - (merge opts {:discover? true})) - (catch Throwable cause - (l/warn :hint "unable to discover OIDC configuration" - :cause cause))))))) +(defn- get-oidc-config + "Get the OIDC config params from global config" + [] + (d/without-nils + {:base-uri (cf/get :oidc-base-uri) + :client-id (cf/get :oidc-client-id) + :client-secret (cf/get :oidc-client-secret) + :token-uri (cf/get :oidc-token-uri) + :auth-uri (cf/get :oidc-auth-uri) + :user-uri (cf/get :oidc-user-uri) + :jwks-uri (cf/get :oidc-jwks-uri) + :scopes (cf/get :oidc-scopes default-oidc-scopes) + :roles (cf/get :oidc-roles) + :user-info-source (cf/get :oidc-user-info-source) + :roles-attr (cf/get :oidc-roles-attr) + :email-attr (cf/get :oidc-email-attr "email") + :name-attr (cf/get :oidc-name-attr "name") + :type "oidc" + :id "oidc"})) (defn- process-oidc-jwks [keys] @@ -126,20 +115,54 @@ keys)) (defn- fetch-oidc-jwks - [cfg {:keys [jwks-uri]}] - (when jwks-uri - (try - (let [{:keys [status body]} (http/req! cfg {:method :get :uri jwks-uri} {:sync? true})] - (if (= 200 status) - (-> body json/decode :keys process-oidc-jwks) - (do - (l/warn :hint "unable to retrieve JWKs (unexpected response status code)" - :response-status status - :response-body body) - nil))) - (catch Throwable cause - (l/warn :hint "unable to retrieve JWKs (unexpected exception)" - :cause cause))))) + [cfg jwks-uri] + (let [{:keys [status body]} (http/req! cfg {:method :get :uri jwks-uri})] + (if (= 200 status) + (-> body json/decode :keys process-oidc-jwks) + (ex/raise :type ::internal + :code :unable-to-fetch-sso-jwks + :hint "unable to retrieve JWKs (unexpected response status code)" + :response-status-code status)))) + +(defn- populate-jwks + "Fetch and Add (if possible) JWK's to the OIDC provider" + [cfg provider] + (try + (if-let [jwks (some->> (:jwks-uri provider) (fetch-oidc-jwks cfg))] + (assoc provider :jwks jwks) + provider) + (catch Throwable cause + (l/warn :hint "unable to fetch JWKs for the OIDC provider" + :provider (str (:id provider)) + :cause cause) + provider))) + +(defn- prepare-oidc-provider + [cfg params] + (when-not (and (string? (:base-uri params)) + (string? (:client-id params)) + (string? (:client-secret params))) + (ex/raise :type ::internal + :code :invalid-sso-config + :hint "missing params for provider initialization" + :provider (:id params))) + + (try + (if (and (string? (:token-uri params)) + (string? (:user-uri params)) + (string? (:auth-uri params))) + (populate-jwks cfg params) + (let [provider (->> params + (discover-oidc-config cfg) + (populate-jwks cfg))] + (with-meta provider {::discovered true}))) + + (catch Throwable cause + (ex/raise :type ::internal + :type :invalid-sso-config + :hint "unexpected exception on configuring provider" + :provider (:id params) + :cause cause)))) (defmethod ig/assert-key ::providers/generic [_ params] @@ -148,52 +171,64 @@ (defmethod ig/init-key ::providers/generic [_ cfg] (when (contains? cf/flags :login-with-oidc) - (if-let [opts (prepare-oidc-opts cfg)] - (let [jwks (fetch-oidc-jwks cfg opts)] + (try + (let [provider (->> (get-oidc-config) + (prepare-oidc-provider cfg))] (l/inf :hint "provider initialized" - :provider "oidc" - :method (if (:discover? opts) "discover" "manual") - :client-id (:client-id opts) - :client-secret (obfuscate-string (:client-secret opts)) - :scopes (str/join "," (:scopes opts)) - :auth-uri (:auth-uri opts) - :user-uri (:user-uri opts) - :token-uri (:token-uri opts) - :roles-attr (:roles-attr opts) - :roles (:roles opts) - :keys (str/join "," (map str (keys jwks)))) - (assoc opts :jwks jwks)) - (do - (l/warn :hint "unable to initialize auth provider, missing configuration" :provider "oidc") - nil)))) + :provider (:id provider) + :client-id (:client-id provider) + :client-secret (obfuscate-string (:client-secret provider))) + provider) + + (catch Throwable cause + (l/warn :hint "unable to initialize auth provider" + :provider "oidc" + :cause cause))))) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; GOOGLE AUTH PROVIDER ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +(defn- get-google-config + [] + (d/without-nils + {:client-id (cf/get :google-client-id) + :client-secret (cf/get :google-client-secret) + :scopes #{"openid" "email" "profile"} + :auth-uri "https://accounts.google.com/o/oauth2/v2/auth" + :token-uri "https://oauth2.googleapis.com/token" + :user-uri "https://openidconnect.googleapis.com/v1/userinfo" + :user-info-source "userinfo" + :type "google" + :id "google"})) + +(defn- prepare-google-provider + [params] + (when-not (and (string? (:client-id params)) + (string? (:client-secret params))) + (ex/raise :type ::internal + :code :invalid-sso-config + :hint "missing params for provider initialization" + :provider (:id params))) + + params) + (defmethod ig/init-key ::providers/google [_ _] - (let [opts {:client-id (cf/get :google-client-id) - :client-secret (cf/get :google-client-secret) - :scopes #{"openid" "email" "profile"} - :auth-uri "https://accounts.google.com/o/oauth2/v2/auth" - :token-uri "https://oauth2.googleapis.com/token" - :user-uri "https://openidconnect.googleapis.com/v1/userinfo" - :name "google"}] + (when (contains? cf/flags :login-with-google) + (try + (let [provider (->> (get-google-config) + (prepare-google-provider))] + (l/inf :hint "provider initialized" + :provider (:id provider) + :client-id (:client-id provider) + :client-secret (obfuscate-string (:client-secret provider))) + provider) - (when (contains? cf/flags :login-with-google) - (if (and (string? (:client-id opts)) - (string? (:client-secret opts))) - (do - (l/inf :hint "provider initialized" - :provider "google" - :client-id (:client-id opts) - :client-secret (obfuscate-string (:client-secret opts))) - opts) - - (do - (l/warn :hint "unable to initialize auth provider, missing configuration" :provider "google") - nil))))) + (catch Throwable cause + (l/warn :hint "unable to initialize auth provider" + :provider "google" + :cause cause))))) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; GITHUB AUTH PROVIDER @@ -203,7 +238,7 @@ [val start end] (and (<= start val) (< val end))) -(defn- retrieve-github-email +(defn- lookup-github-email [cfg tdata props] (or (some-> props :github/email) (let [params {:uri "https://api.github.com/user/emails" @@ -211,7 +246,7 @@ :timeout 6000 :method :get} - {:keys [status body]} (http/req! cfg params {:sync? true})] + {:keys [status body]} (http/req! cfg params)] (when-not (int-in-range? status 200 300) (ex/raise :type :internal @@ -223,44 +258,61 @@ (->> body json/decode (filter :primary) first :email)))) +(defn- get-github-config + [cfg] + (d/without-nils + {:client-id (cf/get :github-client-id) + :client-secret (cf/get :github-client-secret) + :scopes #{"read:user" "user:email"} + :auth-uri "https://github.com/login/oauth/authorize" + :token-uri "https://github.com/login/oauth/access_token" + :user-uri "https://api.github.com/user" + :type "github" + :id "github" + :user-info-source "userinfo" + + ;; Additional hooks for provider specific way of + ;; retrieve emails. + ::get-email-fn (partial lookup-github-email cfg)})) + +(defn- prepare-github-provider + [params] + (when-not (and (string? (:client-id params)) + (string? (:client-secret params))) + (ex/raise :type ::internal + :code :invalid-sso-config + :hint "several required params for configuring GITHUB SSO are missing" + :provider (:id params))) + + params) + (defmethod ig/assert-key ::providers/github [_ params] (assert (http/client? (::http/client params)) "expected a valid http client")) (defmethod ig/init-key ::providers/github [_ cfg] - (let [opts {:client-id (cf/get :github-client-id) - :client-secret (cf/get :github-client-secret) - :scopes #{"read:user" "user:email"} - :auth-uri "https://github.com/login/oauth/authorize" - :token-uri "https://github.com/login/oauth/access_token" - :user-uri "https://api.github.com/user" - :name "github" + (when (contains? cf/flags :login-with-github) + (try + (let [provider (->> (get-github-config cfg) + (prepare-github-provider))] + (l/inf :hint "provider initialized" + :provider (:id provider) + :client-id (:client-id provider) + :client-secret (obfuscate-string (:client-secret provider))) + provider) - ;; Additional hooks for provider specific way of - ;; retrieve emails. - :get-email-fn (partial retrieve-github-email cfg)}] - - (when (contains? cf/flags :login-with-github) - (if (and (string? (:client-id opts)) - (string? (:client-secret opts))) - (do - (l/inf :hint "provider initialized" - :provider "github" - :client-id (:client-id opts) - :client-secret (obfuscate-string (:client-secret opts))) - opts) - - (do - (l/warn :hint "unable to initialize auth provider, missing configuration" :provider "github") - nil))))) + (catch Throwable cause + (l/warn :hint "unable to initialize auth provider" + :provider "github" + :cause cause))))) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; GITLAB AUTH PROVIDER ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -(defmethod ig/init-key ::providers/gitlab - [_ cfg] +(defn- get-gitlab-config + [] (let [base (cf/get :gitlab-base-uri "https://gitlab.com") opts {:base-uri base :client-id (cf/get :gitlab-client-id) @@ -270,21 +322,89 @@ :token-uri (str base "/oauth/token") :user-uri (str base "/oauth/userinfo") :jwks-uri (str base "/oauth/discovery/keys") - :name "gitlab"}] - (when (contains? cf/flags :login-with-gitlab) - (if (and (string? (:client-id opts)) - (string? (:client-secret opts))) - (let [jwks (fetch-oidc-jwks cfg opts)] - (l/inf :hint "provider initialized" - :provider "gitlab" - :base-uri base - :client-id (:client-id opts) - :client-secret (obfuscate-string (:client-secret opts))) - (assoc opts :jwks jwks)) + :type "gitlab" + :id "gitlab"}] + (d/without-nils opts))) - (do - (l/warn :hint "unable to initialize auth provider, missing configuration" :provider "gitlab") - nil))))) +(defn- prepare-gitlab-provider + [cfg params] + (when-not (and (string? (:client-id params)) + (string? (:client-secret params))) + (ex/raise :type ::internal + :code :invalid-sso-config + :hint "missing params for provider initialization" + :provider (:id params))) + + (try + (let [provider (populate-jwks cfg params)] + (l/inf :hint "provider initialized" + :provider "gitlab" + :base-uri (:base-uri provider) + :client-id (:client-id provider) + :client-secret (obfuscate-string (:client-secret provider))) + provider) + (catch Throwable cause + (ex/raise :type ::internal + :type :invalid-sso-config + :hint "unexpected exception on configuring provider" + :provider (:id params) + :cause cause)))) + +(defmethod ig/init-key ::providers/gitlab + [_ cfg] + + (when (contains? cf/flags :login-with-gitlab) + (try + (let [provider (->> (get-gitlab-config) + (prepare-gitlab-provider cfg))] + + (l/inf :hint "provider initialized" + :provider (:id provider) + :client-id (:client-id provider) + :client-secret (obfuscate-string (:client-secret provider))) + provider) + + (catch Throwable cause + (l/warn :hint "unable to initialize auth provider" + :provider "gitlab" + :cause cause))))) + +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; PROVIDERS COLLECTOR +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(def ^:private schema:provider + [:map {:title "Provider"} + [:type ::sm/text] + [:client-id ::sm/text] + [:client-secret ::sm/text] + + [:id [:or :string ::sm/uuid]] + [:base-uri {:optional true} ::sm/text] + [:token-uri {:optional true} ::sm/text] + [:auth-uri {:optional true} ::sm/text] + [:user-uri {:optional true} ::sm/text] + [:scopes {:optional true} + [::sm/set ::sm/text]] + [:roles {:optional true} + [::sm/set ::sm/text]] + [:roles-attr {:optional true} ::sm/text] + [:email-attr {:optional true} ::sm/text] + [:name-attr {:optional true} ::sm/text]]) + +(def ^:private schema:providers + [:map-of :string schema:provider]) + +(defmethod ig/assert-key ::providers + [_ providers] + (let [check-provider (sm/check-fn schema:provider)] + (assert (every? check-provider (filter identity providers))))) + +(defmethod ig/init-key ::providers + [_ providers] + (->> providers + (filter identity) + (d/index-by :id))) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; HANDLERS @@ -293,19 +413,19 @@ (defn- parse-attr-path [provider path] (let [[fitem & items] (str/split path "__")] - (into [(keyword (:name provider) fitem)] (map keyword) items))) + (into [(keyword (:type provider) fitem)] (map keyword) items))) (defn- build-redirect-uri - [{:keys [::provider] :as cfg}] + [] (let [public (u/uri (cf/get :public-uri))] - (str (assoc public :path (str "/api/auth/oauth/" (:name provider) "/callback"))))) + (str (assoc public :path (str "/api/auth/oidc/callback"))))) -(defn- build-auth-uri - [{:keys [::provider] :as cfg} state] +(defn- build-auth-redirect-uri + [provider token] (let [params {:client_id (:client-id provider) - :redirect_uri (build-redirect-uri cfg) + :redirect_uri (build-redirect-uri) :response_type "code" - :state state + :state token :scope (str/join " " (:scopes provider []))} query (u/map->query-string params)] (-> (u/uri (:auth-uri provider)) @@ -314,7 +434,7 @@ (defn- qualify-prop-key [provider k] - (keyword (:name provider) (name k))) + (keyword (:type provider) (name k))) (defn- qualify-props [provider props] @@ -324,12 +444,12 @@ props)) (defn- fetch-access-token - [{:keys [::provider] :as cfg} code] + [cfg provider code] (let [params {:client_id (:client-id provider) :client_secret (:client-secret provider) :code code :grant_type "authorization_code" - :redirect_uri (build-redirect-uri cfg)} + :redirect_uri (build-redirect-uri)} req {:method :post :headers {"content-type" "application/x-www-form-urlencoded" "accept" "application/json"} @@ -337,14 +457,13 @@ :body (u/map->query-string params)}] (l/trc :hint "fetch access token" - :provider (:name provider) + :provider (:id provider) :client-id (:client-id provider) :client-secret (obfuscate-string (:client-secret provider)) :grant-type (:grant_type params) :redirect-uri (:redirect_uri params)) - (let [{:keys [status body]} (http/req! cfg req {:sync? true})] - (l/trc :hint "access token fetched" :status status :body body) + (let [{:keys [status body]} (http/req! cfg req)] (if (= status 200) (let [data (json/decode body) data {:token/access (get data :access_token) @@ -367,26 +486,30 @@ (letfn [(get-email [props] ;; Allow providers hook into this for custom email ;; retrieval method. - (if-let [get-email-fn (:get-email-fn provider)] + (if-let [get-email-fn (::get-email-fn provider)] (get-email-fn tdata props) - (let [attr-kw (cf/get :oidc-email-attr "email") + (let [attr-kw (get provider :email-attr "email") attr-ph (parse-attr-path provider attr-kw)] (get-in props attr-ph)))) (get-name [props] - (let [attr-kw (cf/get :oidc-name-attr "name") - attr-ph (parse-attr-path provider attr-kw)] - (get-in props attr-ph)))] + (or (let [attr-kw (get provider :name-attr "name") + attr-ph (parse-attr-path provider attr-kw)] + (get-in props attr-ph)) + (let [attr-ph (parse-attr-path provider "nickname")] + (get-in props attr-ph))))] - (let [props (qualify-props provider info) + (let [info (assoc info :provider-id (str (:id provider))) + props (qualify-props provider info) email (get-email props)] - {:backend (:name provider) + {:backend (:type provider) :fullname (or (get-name props) email) - :email email - :props props}))) + :email email + :email-verified (get info :email_verified false) + :props props}))) (defn- fetch-user-info - [{:keys [::provider] :as cfg} tdata] + [cfg provider tdata] (l/trc :hint "fetch user info" :uri (:user-uri provider) :token (obfuscate-string (:token/access tdata))) @@ -395,7 +518,7 @@ :headers {"Authorization" (str (:token/type tdata) " " (:token/access tdata))} :timeout 6000 :method :get} - response (http/req! cfg params {:sync? true})] + response (http/req! cfg params)] (l/trc :hint "user info response" :status (:status response) @@ -410,17 +533,16 @@ (-> response :body json/decode))) -(defn- get-user-info - [{:keys [::provider]} tdata] +(defn- get-id-token-claims + [provider tdata] (try (when (:token/id tdata) - (let [{:keys [kid alg] :as theader} (jwt/decode-header (:token/id tdata))] + (let [{:keys [kid alg]} (jwt/decode-header (:token/id tdata))] (when-let [key (if (str/starts-with? (name alg) "hs") (:client-secret provider) (get-in provider [:jwks kid]))] - (let [claims (jwt/unsign (:token/id tdata) key {:alg alg})] - (dissoc claims :exp :iss :iat :sid :aud :sub))))) + (jwt/unsign (:token/id tdata) key {:alg alg})))) (catch Throwable cause (l/warn :hint "unable to get user info from JWT token (unexpected exception)" :cause cause)))) @@ -430,41 +552,41 @@ [:backend ::sm/text] [:email ::sm/email] [:fullname ::sm/text] - [:props [:map-of :keyword :any]]]) + [:email-verified :boolean] + [:props [:map-of :keyword ::sm/any]]]) (def ^:private valid-info? (sm/validator schema:info)) (defn- get-info - [{:keys [::provider] :as cfg} {:keys [params] :as request}] - (let [state (get params :state) - code (get params :code) - state (tokens/verify cfg {:token state :iss :oauth}) - tdata (fetch-access-token cfg code) - info (case (cf/get :oidc-user-info-source) - :token (get-user-info cfg tdata) - :userinfo (fetch-user-info cfg tdata) - (or (get-user-info cfg tdata) - (fetch-user-info cfg tdata))) + [cfg provider state code] + (let [tdata (fetch-access-token cfg provider code) + claims (get-id-token-claims provider tdata) + + info (case (get provider :user-info-source) + :token (dissoc claims :exp :iss :iat :aud :sub :sid) + :userinfo (fetch-user-info cfg provider tdata) + (or (some-> claims (dissoc :exp :iss :iat :aud :sub :sid)) + (fetch-user-info cfg provider tdata))) info (process-user-info provider tdata info)] - (l/trc :hint "user info" :info info) - - (when-not (valid-info? info) - (l/warn :hint "received incomplete profile info object (please set correct scopes)" :info info) - (ex/raise :type :internal - :code :incomplete-user-info - :hint "inconmplete user info" - :info info)) + (if (valid-info? info) + (l/trc :hint "received valid user info object" :info info) + (do + (l/warn :hint "received incomplete user info object (please set correct scopes)" :info info) + (ex/raise :type :internal + :code :incomplete-user-info + :hint "inconmplete user info" + :info info))) ;; If the provider is OIDC, we can proceed to check ;; roles if they are defined. - (when (and (= "oidc" (:name provider)) + (when (and (= "oidc" (:type provider)) (seq (:roles provider))) (let [expected-roles (into #{} (:roles provider)) - current-roles (let [roles-kw (cf/get :oidc-roles-attr "roles") + current-roles (let [roles-kw (get provider :roles-attr "roles") roles-ph (parse-attr-path provider roles-kw) roles (get-in (:props info) roles-ph)] (cond @@ -479,6 +601,12 @@ :hint "not enough permissions")))) (cond-> info + (some? (:sid claims)) + (assoc :sso-session-id (:sid claims)) + + (uuid? (:id provider)) + (assoc :sso-provider-id (:id provider)) + (some? (:invitation-token state)) (assoc :invitation-token (:invitation-token state)) @@ -491,9 +619,9 @@ (update :props merge (:props state))))) (defn- get-profile - [cfg info] + [cfg email] (db/run! cfg (fn [{:keys [::db/conn]}] - (some->> (:email info) + (some->> email (profile/clean-email) (profile/get-profile-by-email conn))))) @@ -513,13 +641,13 @@ (redirect-response uri)))) (defn- redirect-to-register - [cfg info request] + [cfg info provider] (let [info (assoc info :iss :prepared-register :exp (ct/in-future {:hours 48})) params {:token (tokens/generate cfg info) - :provider (:provider (:path-params request)) + :provider (:provider (:id provider)) :fullname (:fullname info)} params (d/without-nils params)] @@ -538,144 +666,169 @@ (redirect-response uri))) (defn- provider-has-email-verified? - [{:keys [::provider] :as cfg} {:keys [props] :as info}] + [provider {:keys [props] :as info}] (let [prop (qualify-prop-key provider :email_verified)] (true? (get props prop)))) (defn- profile-has-provider-props? - [{:keys [::provider] :as cfg} profile] - (let [prop (qualify-prop-key provider :email)] - (contains? (:props profile) prop))) + [provider {:keys [props] :as profile}] + (if (and (uuid? (:id provider)) + (= "oidc" (:type provider))) + (= (str (:id provider)) + (get props :oidc/provider-id)) + (let [prop (qualify-prop-key provider :email)] + (contains? props prop)))) -(defn- provider-matches-profile? - [{:keys [::provider] :as cfg} profile info] - (or (= (:auth-backend profile) (:name provider)) - (profile-has-provider-props? cfg profile) - (provider-has-email-verified? cfg info))) +(defn- decode-row + [{:keys [roles scopes] :as row}] + (cond-> row + (nil? scopes) + (assoc :scopes default-oidc-scopes) + + (db/pgarray? scopes) + (assoc :scopes (db/decode-pgarray scopes #{})) + + (db/pgarray? roles) + (assoc :roles (db/decode-pgarray roles #{})))) + +;; TODO: add cache layer for avoid build an discover each time + +(defn get-provider + [cfg id] + (try + (when-let [params (some->> (db/get* cfg :sso-provider {:id id :is-enabled true}) + (decode-row))] + (case (:type params) + "oidc" (prepare-oidc-provider cfg params))) + (catch Throwable cause + (l/err :hint "unable to configure custom SSO provider" + :provider (str id) + :cause cause)))) + +(defn- resolve-provider + [{:keys [::providers] :as cfg} params] + (let [provider (get params :provider) + provider (if (uuid? provider) + provider + (or (uuid/parse* provider) provider))] -(defn- process-callback - [cfg request info profile] - (cond - (some? profile) (cond - (:is-blocked profile) - (redirect-with-error "profile-blocked") + (uuid? provider) + (or (get-provider cfg provider) + (ex/raise :type :restriction + :code :sso-provider-not-configured + :hint "provider not configured" + :provider provider)) - (not (provider-matches-profile? cfg profile info)) - (redirect-with-error "auth-provider-not-allowed") - - (not (:is-active profile)) - (let [info (assoc info :profile-id (:id profile))] - (redirect-to-register cfg info request)) + (string? provider) + (or (get providers provider) + (ex/raise :type :restriction + :code :sso-provider-not-configured + :hint "provider not configured" + :provider provider)) :else - (let [sxf (session/create-fn cfg (:id profile)) - token (or (:invitation-token info) - (tokens/generate cfg - {:iss :auth - :exp (ct/in-future "15m") - :profile-id (:id profile)})) - props (audit/profile->props profile) - context (d/without-nils {:external-session-id (:external-session-id info)})] + (throw (IllegalArgumentException. "invalid data for provider"))))) - (audit/submit! cfg {::audit/type "action" - ::audit/name "login-with-oidc" - ::audit/profile-id (:id profile) - ::audit/ip-addr (inet/parse-request request) - ::audit/props props - ::audit/context context}) - - (->> (redirect-to-verify-token token) - (sxf request)))) - - (and (email.blacklist/enabled? cfg) - (email.blacklist/contains? cfg (:email info))) - (redirect-with-error "email-domain-not-allowed") - - (and (email.whitelist/enabled? cfg) - (not (email.whitelist/contains? cfg (:email info)))) - (redirect-with-error "email-domain-not-allowed") - - :else - (let [info (assoc info :is-active (provider-has-email-verified? cfg info))] - (if (or (contains? cf/flags :registration) - (contains? cf/flags :oidc-registration)) - (redirect-to-register cfg info request) - (redirect-with-error "registration-disabled"))))) - -(defn- get-external-session-id - [request] - (let [session-id (yreq/get-header request "x-external-session-id")] - (when (string? session-id) - (if (or (> (count session-id) 256) - (= session-id "null") - (str/blank? session-id)) - nil - session-id)))) +(defn- update-profile-with-info + [cfg {:keys [id props] :as profile} info] + (let [props' (merge props (:props info))] + (if (not= props props') + (do + (db/update! cfg :profile + {:props (db/tjson props')} + {:id id} + {::db/return-keys false}) + (assoc profile :props props')) + profile))) (defn- auth-handler [cfg {:keys [params] :as request}] - (let [props (audit/extract-utm-params params) - esid (rpc/get-external-session-id request) - params {:iss :oauth - :invitation-token (:invitation-token params) - :external-session-id esid - :props props - :exp (ct/in-future "4h")} + (let [provider (resolve-provider cfg params) + props (audit/extract-utm-params params) + esid (audit/get-external-session-id request) + params {:iss "oidc" + :provider (:id provider) + :invitation-token (:invitation-token params) + :external-session-id esid + :props props + :exp (ct/in-future "4h")} state (tokens/generate cfg (d/without-nils params)) - uri (build-auth-uri cfg state)] + uri (build-auth-redirect-uri provider state)] + {::yres/status 200 ::yres/body {:redirect-uri uri}})) (defn- callback-handler - [{:keys [::provider] :as cfg} request] - (try - (if-let [error (dm/get-in request [:params :error])] - (redirect-with-error "unable-to-auth" error) - (let [info (get-info cfg request) - profile (get-profile cfg info)] - (process-callback cfg request info profile))) - (catch Throwable cause - (binding [l/*context* (-> (errors/request->context request) - (assoc :auth/provider (:name provider)))] - (let [edata (ex-data cause)] + [cfg {:keys [params] :as request}] + (if-let [error (get params :error)] + (redirect-with-error "unable-to-auth" error) + (try + (let [code (get params :code) + state (get params :state) + state (tokens/verify cfg {:token state :iss "oidc"}) + + provider (resolve-provider cfg state) + info (get-info cfg provider state code) + profile (get-profile cfg (:email info))] + + (cond + (not profile) (cond - (= :validation (:type edata)) - (l/wrn :hint "invalid token received" :cause cause) + (and (email.blacklist/enabled? cfg) + (email.blacklist/contains? cfg (:email info))) + (redirect-with-error "email-domain-not-allowed") + + (and (email.whitelist/enabled? cfg) + (not (email.whitelist/contains? cfg (:email info)))) + (redirect-with-error "email-domain-not-allowed") :else - (l/err :hint "error on oauth process" :cause cause)))) + (if (or (contains? cf/flags :registration) + (contains? cf/flags :oidc-registration)) + (redirect-to-register cfg info provider) + (redirect-with-error "registration-disabled"))) - (redirect-with-error "unable-to-auth" (ex-message cause))))) + (:is-blocked profile) + (redirect-with-error "profile-blocked") -(def provider-lookup - {:compile - (fn [& _] - (fn [handler {:keys [::providers] :as cfg}] - (fn [request] - (let [provider (some-> request :path-params :provider keyword)] - (if-let [provider (get providers provider)] - (handler (assoc cfg ::provider provider) request) - (ex/raise :type :restriction - :code :provider-not-configured - :provider provider - :hint "provider not configured"))))))}) + (not (or (= (:auth-backend profile) (:type provider)) + (profile-has-provider-props? provider profile) + (provider-has-email-verified? provider info))) + (redirect-with-error "auth-provider-not-allowed") -(def ^:private schema:provider - [:map {:title "provider"} - [:client-id ::sm/text] - [:client-secret ::sm/text] - [:base-uri {:optional true} ::sm/text] - [:token-uri {:optional true} ::sm/text] - [:auth-uri {:optional true} ::sm/text] - [:user-uri {:optional true} ::sm/text] - [:scopes {:optional true} - [::sm/set ::sm/text]] - [:roles {:optional true} - [::sm/set ::sm/text]] - [:roles-attr {:optional true} ::sm/text] - [:email-attr {:optional true} ::sm/text] - [:name-attr {:optional true} ::sm/text]]) + (not (:is-active profile)) + (let [info (assoc info :profile-id (:id profile))] + (redirect-to-register cfg info provider)) + + :else + (let [sxf (session/create-fn cfg profile info) + token (or (:invitation-token info) + (tokens/generate cfg + {:iss :auth + :exp (ct/in-future "15m") + :profile-id (:id profile)})) + + ;; If proceed, update profile on the database + profile (update-profile-with-info cfg profile info) + + props (audit/profile->props profile) + context (d/without-nils {:external-session-id (:external-session-id info)})] + + (audit/submit! cfg {::audit/type "action" + ::audit/name "login-with-oidc" + ::audit/profile-id (:id profile) + ::audit/ip-addr (inet/parse-request request) + ::audit/props props + ::audit/context context}) + + (->> (redirect-to-verify-token token) + (sxf request))))) + + (catch Throwable cause + (binding [l/*context* (errors/request->context request)] + (l/err :hint "error on process oidc callback" :cause cause) + (redirect-with-error "unable-to-auth" (ex-message cause))))))) (def ^:private schema:routes-params [:map @@ -683,7 +836,7 @@ ::http/client ::setup/props ::db/pool - [::providers [:map-of :keyword [:maybe schema:provider]]]]) + [::providers schema:providers]]) (defmethod ig/assert-key ::routes [_ params] @@ -691,14 +844,11 @@ (defmethod ig/init-key ::routes [_ cfg] - (let [cfg (update cfg :providers d/without-nils)] - ["/api" {:middleware [[mw/cors] - [sec/client-header-check] - [provider-lookup cfg]]} - ["/auth/oauth" - ["/:provider" - {:handler auth-handler - :allowed-methods #{:post}}] - ["/:provider/callback" - {:handler callback-handler - :allowed-methods #{:get}}]]])) + (let [cfg (update cfg ::providers d/without-nils)] + ["/api/auth/oidc" {:middleware [[session/authz cfg]]} + ["" + {:handler (partial auth-handler cfg) + :allowed-methods #{:post}}] + ["/callback" + {:handler (partial callback-handler cfg) + :allowed-methods #{:get}}]])) diff --git a/backend/src/app/config.clj b/backend/src/app/config.clj index 2ff18a5974..1fc3b0539d 100644 --- a/backend/src/app/config.clj +++ b/backend/src/app/config.clj @@ -168,7 +168,7 @@ [:google-client-id {:optional true} :string] [:google-client-secret {:optional true} :string] [:oidc-client-id {:optional true} :string] - [:oidc-user-info-source {:optional true} :keyword] + [:oidc-user-info-source {:optional true} [:enum "auto" "userinfo" "token"]] [:oidc-client-secret {:optional true} :string] [:oidc-base-uri {:optional true} :string] [:oidc-token-uri {:optional true} :string] diff --git a/backend/src/app/http/access_token.clj b/backend/src/app/http/access_token.clj index 70af751488..c90fc7d3df 100644 --- a/backend/src/app/http/access_token.clj +++ b/backend/src/app/http/access_token.clj @@ -9,7 +9,7 @@ [app.common.logging :as l] [app.config :as cf] [app.db :as db] - [app.http.auth :as-alias http.auth] + [app.http :as-alias http] [app.main :as-alias main] [app.setup :as-alias setup] [app.tokens :as tokens])) @@ -33,25 +33,26 @@ (defn- get-token-data [pool claims] (when-not (db/read-only? pool) - (when-let [token-id (-> (deref claims) (get :tid))] + (when-let [token-id (get claims :tid)] (some-> (db/exec-one! pool [sql:get-token-data token-id]) (update :perms db/decode-pgarray #{}))))) (defn- wrap-authz [handler {:keys [::db/pool]}] - (fn [{:keys [::http.auth/token-type] :as request}] - (if (= :token token-type) - (let [{:keys [perms profile-id expires-at]} (some->> (get request ::http.auth/claims) - (get-token-data pool))] - (handler (cond-> request - (some? perms) - (assoc ::perms perms) - (some? profile-id) - (assoc ::profile-id profile-id) - (some? expires-at) - (assoc ::expires-at expires-at)))) + (fn [request] + (let [{:keys [type claims]} (get request ::http/auth-data)] + (if (= :token type) + (let [{:keys [perms profile-id expires-at]} (some->> claims (get-token-data pool))] + ;; FIXME: revisit this, this data looks unused + (handler (cond-> request + (some? perms) + (assoc ::perms perms) + (some? profile-id) + (assoc ::profile-id profile-id) + (some? expires-at) + (assoc ::expires-at expires-at)))) - (handler request)))) + (handler request))))) (def authz {:name ::authz diff --git a/backend/src/app/http/client.clj b/backend/src/app/http/client.clj index 456d66ae1e..7fcbd3ff00 100644 --- a/backend/src/app/http/client.clj +++ b/backend/src/app/http/client.clj @@ -9,8 +9,7 @@ (:require [app.common.schema :as sm] [integrant.core :as ig] - [java-http-clj.core :as http] - [promesa.core :as p]) + [java-http-clj.core :as http]) (:import java.net.http.HttpClient)) @@ -29,14 +28,9 @@ (defn send! ([client req] (send! client req {})) - ([client req {:keys [response-type sync?] :or {response-type :string sync? false}}] + ([client req {:keys [response-type] :or {response-type :string}}] (assert (client? client) "expected valid http client") - (if sync? - (http/send req {:client client :as response-type}) - (try - (http/send-async req {:client client :as response-type}) - (catch Throwable cause - (p/rejected cause)))))) + (http/send req {:client client :as response-type}))) (defn- resolve-client [params] @@ -56,8 +50,8 @@ ([cfg-or-client request] (let [client (resolve-client cfg-or-client) request (update request :uri str)] - (send! client request {:sync? true}))) + (send! client request {}))) ([cfg-or-client request options] (let [client (resolve-client cfg-or-client) request (update request :uri str)] - (send! client request (merge {:sync? true} options))))) + (send! client request options)))) diff --git a/backend/src/app/http/errors.clj b/backend/src/app/http/errors.clj index 8cc87dba1c..31bda4a616 100644 --- a/backend/src/app/http/errors.clj +++ b/backend/src/app/http/errors.clj @@ -23,7 +23,7 @@ (defn request->context "Extracts error report relevant context data from request." [request] - (let [claims (some-> (get request ::auth/claims) deref)] + (let [{:keys [claims] :as auth} (get request ::http/auth-data)] (-> (cf/logging-context) (assoc :request/path (:path request)) (assoc :request/method (:method request)) @@ -31,6 +31,7 @@ (assoc :request/user-agent (yreq/get-header request "user-agent")) (assoc :request/ip-addr (inet/parse-request request)) (assoc :request/profile-id (get claims :uid)) + (assoc :request/auth-data auth) (assoc :version/frontend (or (yreq/get-header request "x-frontend-version") "unknown"))))) (defmulti handle-error @@ -59,7 +60,6 @@ ::yres/body data} (binding [l/*context* (request->context request)] - (l/wrn :hint "restriction error" :cause err) {::yres/status 400 ::yres/body data})))) diff --git a/backend/src/app/http/middleware.clj b/backend/src/app/http/middleware.clj index 683e541aa5..88f3958289 100644 --- a/backend/src/app/http/middleware.clj +++ b/backend/src/app/http/middleware.clj @@ -12,7 +12,7 @@ [app.common.schema :as-alias sm] [app.common.transit :as t] [app.config :as cf] - [app.http.auth :as-alias auth] + [app.http :as-alias http] [app.http.errors :as errors] [app.util.pointer-map :as pmap] [cuerdas.core :as str] @@ -242,6 +242,7 @@ (handler request) {::yres/status 405}))))))}) + (defn- wrap-auth [handler decoders] (let [token-re @@ -252,30 +253,28 @@ (when-let [[_ token-type token] (some->> (yreq/get-header request "authorization") (re-matches token-re))] (if (= "token" (str/lower token-type)) - [:token token] - [:bearer token]))) + {:type :token + :token token} + {:type :bearer + :token token}))) get-token-from-cookie (fn [request] (let [cname (cf/get :auth-token-cookie-name) token (some-> (yreq/get-cookie request cname) :value)] (when-not (str/empty? token) - [:cookie token]))) + {:type :cookie + :token token}))) get-token (some-fn get-token-from-cookie get-token-from-authorization) process-request (fn [request] - (if-let [[token-type token] (get-token request)] - (let [request (-> request - (assoc ::auth/token token) - (assoc ::auth/token-type token-type)) - decoder (get decoders token-type)] - - (if (fn? decoder) - (assoc request ::auth/claims (delay (decoder token))) - request)) + (if-let [{:keys [type token] :as auth} (get-token request)] + (if-let [decode-fn (get decoders type)] + (assoc request ::http/auth-data (assoc auth :claims (decode-fn token))) + (assoc request ::http/auth-data auth)) request))] (fn [request] diff --git a/backend/src/app/http/session.clj b/backend/src/app/http/session.clj index 362e5a559a..afe92c7945 100644 --- a/backend/src/app/http/session.clj +++ b/backend/src/app/http/session.clj @@ -11,17 +11,19 @@ [app.common.logging :as l] [app.common.schema :as sm] [app.common.time :as ct] + [app.common.uuid :as uuid] [app.config :as cf] [app.db :as db] [app.db.sql :as sql] + [app.http :as-alias http] [app.http.auth :as-alias http.auth] [app.http.session.tasks :as-alias tasks] [app.main :as-alias main] [app.setup :as-alias setup] [app.tokens :as tokens] - [cuerdas.core :as str] [integrant.core :as ig] - [yetti.request :as yreq])) + [yetti.request :as yreq] + [yetti.response :as yres])) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; DEFAULTS @@ -38,10 +40,10 @@ ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (defprotocol ISessionManager - (read [_ key]) - (write! [_ key data]) - (update! [_ data]) - (delete! [_ key])) + (read-session [_ id]) + (create-session [_ params]) + (update-session [_ session]) + (delete-session [_ id])) (defn manager? [o] @@ -56,71 +58,82 @@ ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (def ^:private schema:params - [:map {:title "session-params"} - [:user-agent ::sm/text] + [:map {:title "SessionParams" :closed true} [:profile-id ::sm/uuid] - [:created-at ::ct/inst]]) + [:user-agent {:optional true} ::sm/text] + [:sso-provider-id {:optional true} ::sm/uuid] + [:sso-session-id {:optional true} :string]]) (def ^:private valid-params? (sm/validator schema:params)) -(defn- prepare-session-params - [params key] - (assert (string? key) "expected key to be a string") - (assert (not (str/blank? key)) "expected key to be not empty") - (assert (valid-params? params) "expected valid params") - - {:user-agent (:user-agent params) - :profile-id (:profile-id params) - :created-at (:created-at params) - :updated-at (:created-at params) - :id key}) - (defn- database-manager [pool] (reify ISessionManager - (read [_ token] - (db/exec-one! pool (sql/select :http-session {:id token}))) + (read-session [_ id] + (if (string? id) + ;; Backward compatibility + (let [session (db/exec-one! pool (sql/select :http-session {:id id}))] + (-> session + (assoc :modified-at (:updated-at session)) + (dissoc :updated-at))) + (db/exec-one! pool (sql/select :http-session-v2 {:id id})))) - (write! [_ key params] - (let [params (-> params - (assoc :created-at (ct/now)) - (prepare-session-params key))] - (db/insert! pool :http-session params) - params)) + (create-session [_ params] + (assert (valid-params? params) "expect valid session params") - (update! [_ params] - (let [updated-at (ct/now)] - (db/update! pool :http-session - {:updated-at updated-at} - {:id (:id params)}) - (assoc params :updated-at updated-at))) + (let [now (ct/now) + params (-> params + (assoc :id (uuid/next)) + (assoc :created-at now) + (assoc :modified-at now))] + (db/insert! pool :http-session-v2 params + {::db/return-keys true}))) - (delete! [_ token] - (db/delete! pool :http-session {:id token}) + (update-session [_ session] + (let [modified-at (ct/now)] + (if (string? (:id session)) + (let [params (-> session + (assoc :id (uuid/next)) + (assoc :created-at modified-at) + (assoc :modified-at modified-at))] + (db/insert! pool :http-session-v2 params)) + + (db/update! pool :http-session-v2 + {:modified-at modified-at} + {:id (:id session)})))) + + (delete-session [_ id] + (if (string? id) + (db/delete! pool :http-session {:id id} {::db/return-keys false}) + (db/delete! pool :http-session-v2 {:id id} {::db/return-keys false})) nil))) (defn inmemory-manager [] (let [cache (atom {})] (reify ISessionManager - (read [_ token] - (get @cache token)) + (read-session [_ id] + (get @cache id)) - (write! [_ key params] - (let [params (-> params - (assoc :created-at (ct/now)) - (prepare-session-params key))] - (swap! cache assoc key params) - params)) + (create-session [_ params] + (assert (valid-params? params) "expect valid session params") - (update! [_ params] - (let [updated-at (ct/now)] - (swap! cache update (:id params) assoc :updated-at updated-at) - (assoc params :updated-at updated-at))) + (let [now (ct/now) + session (-> params + (assoc :id (uuid/next)) + (assoc :created-at now) + (assoc :modified-at now))] + (swap! cache assoc (:id session) session) + session)) - (delete! [_ token] - (swap! cache dissoc token) + (update-session [_ session] + (let [modified-at (ct/now)] + (swap! cache update (:id session) assoc :modified-at modified-at) + (assoc session :modified-at modified-at))) + + (delete-session [_ id] + (swap! cache dissoc id) nil)))) (defmethod ig/assert-key ::manager @@ -140,43 +153,48 @@ ;; MANAGER IMPL ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -(declare ^:private assign-auth-token-cookie) -(declare ^:private clear-auth-token-cookie) -(declare ^:private gen-token) +(declare ^:private assign-session-cookie) +(declare ^:private clear-session-cookie) + +(defn- assign-token + [cfg session] + (let [token (tokens/generate cfg + {:iss "authentication" + :aud "penpot" + :sid (:id session) + :iat (:modified-at session) + :uid (:profile-id session) + :sso-provider-id (:sso-provider-id session) + :sso-session-id (:sso-session-id session)})] + (assoc session :token token))) (defn create-fn - [{:keys [::manager] :as cfg} profile-id] + [{:keys [::manager] :as cfg} {profile-id :id :as profile} + & {:keys [sso-provider-id sso-session-id]}] + (assert (manager? manager) "expected valid session manager") (assert (uuid? profile-id) "expected valid uuid for profile-id") (fn [request response] (let [uagent (yreq/get-header request "user-agent") - params {:profile-id profile-id - :user-agent uagent} - token (gen-token cfg params) - session (write! manager token params)] - (l/trc :hint "create" :profile-id (str profile-id)) - (-> response - (assign-auth-token-cookie session))))) + session (->> {:user-agent uagent + :profile-id profile-id + :sso-provider-id sso-provider-id + :sso-session-id sso-session-id} + (d/without-nils) + (create-session manager) + (assign-token cfg))] + + (l/trc :hint "create" :id (str (:id session)) :profile-id (str profile-id)) + (assign-session-cookie response session)))) (defn delete-fn [{:keys [::manager]}] (assert (manager? manager) "expected valid session manager") (fn [request response] - (let [cname (cf/get :auth-token-cookie-name) - cookie (yreq/get-cookie request cname)] - (l/trc :hint "delete" :profile-id (:profile-id request)) - (some->> (:value cookie) (delete! manager)) - (-> response - (assoc :status 204) - (assoc :body nil) - (clear-auth-token-cookie))))) + (some->> (get request ::id) (delete-session manager)) + (clear-session-cookie response))) -(defn- gen-token - [cfg {:keys [profile-id created-at]}] - (tokens/generate cfg {:iss "authentication" - :iat created-at - :uid profile-id})) (defn decode-token [cfg token] (try @@ -186,44 +204,63 @@ :token token :cause cause)))) +(defn get-session + [request] + (get request ::session)) + +(defn invalidate-others + [cfg session] + (let [sql "delete from http_session_v2 where profile_id = ? and id != ?"] + (-> (db/exec-one! cfg [sql (:profile-id session) (:id session)]) + (db/get-update-count)))) + (defn- renew-session? - [{:keys [updated-at] :as session}] - (and (ct/inst? updated-at) - (let [elapsed (ct/diff updated-at (ct/now))] - (neg? (compare default-renewal-max-age elapsed))))) + [{:keys [id modified-at] :as session}] + (or (string? id) + (and (ct/inst? modified-at) + (let [elapsed (ct/diff modified-at (ct/now))] + (neg? (compare default-renewal-max-age elapsed)))))) (defn- wrap-authz [handler {:keys [::manager] :as cfg}] (assert (manager? manager) "expected valid session manager") - (fn [{:keys [::http.auth/token-type] :as request}] - (cond - (= token-type :cookie) - (let [session (some->> (get request ::http.auth/token) - (read manager)) - request (cond-> request - (some? session) - (-> (assoc ::profile-id (:profile-id session)) - (assoc ::id (:id session)))) + (fn [request] + (let [{:keys [type token claims]} (get request ::http/auth-data)] + (cond + (= type :cookie) + (let [session (if-let [sid (:sid claims)] + (read-session manager sid) + ;; BACKWARD COMPATIBILITY WITH OLD TOKENS + (read-session manager token)) - response (handler request)] + request (cond-> request + (some? session) + (-> (assoc ::profile-id (:profile-id session)) + (assoc ::session session))) - (if (renew-session? session) - (let [session (update! manager session)] - (-> response - (assign-auth-token-cookie session))) - response)) + response (handler request)] - (= token-type :bearer) - (let [session (some->> (get request ::http.auth/token) - (read manager)) - request (cond-> request - (some? session) - (-> (assoc ::profile-id (:profile-id session)) - (assoc ::id (:id session))))] - (handler request)) + (if (renew-session? session) + (let [session (->> session + (update-session manager) + (assign-token cfg))] + (assign-session-cookie response session)) + response)) - :else - (handler request)))) + (= type :bearer) + (let [session (if-let [sid (:sid claims)] + (read-session manager sid) + ;; BACKWARD COMPATIBILITY WITH OLD TOKENS + (read-session manager token)) + + request (cond-> request + (some? session) + (-> (assoc ::profile-id (:profile-id session)) + (assoc ::session session)))] + (handler request)) + + :else + (handler request))))) (def authz {:name ::authz @@ -231,10 +268,10 @@ ;; --- IMPL -(defn- assign-auth-token-cookie - [response {token :id updated-at :updated-at}] +(defn- assign-session-cookie + [response {token :token modified-at :modified-at}] (let [max-age (cf/get :auth-token-cookie-max-age default-cookie-max-age) - created-at updated-at + created-at modified-at renewal (ct/plus created-at default-renewal-max-age) expires (ct/plus created-at max-age) secure? (contains? cf/flags :secure-session-cookies) @@ -249,12 +286,12 @@ :comment comment :same-site (if cors? :none (if strict? :strict :lax)) :secure secure?}] - (update response :cookies assoc name cookie))) + (update response ::yres/cookies assoc name cookie))) -(defn- clear-auth-token-cookie +(defn- clear-session-cookie [response] (let [cname (cf/get :auth-token-cookie-name)] - (update response :cookies assoc cname {:path "/" :value "" :max-age 0}))) + (update response ::yres/cookies assoc cname {:path "/" :value "" :max-age 0}))) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; TASK: SESSION GC diff --git a/backend/src/app/loggers/audit.clj b/backend/src/app/loggers/audit.clj index 46e50662c8..9471f5dad4 100644 --- a/backend/src/app/loggers/audit.clj +++ b/backend/src/app/loggers/audit.clj @@ -25,7 +25,8 @@ [app.util.inet :as inet] [app.util.services :as-alias sv] [app.worker :as wrk] - [cuerdas.core :as str])) + [cuerdas.core :as str] + [yetti.request :as yreq])) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; HELPERS @@ -90,6 +91,22 @@ ::ip-addr (::rpc/ip-addr params) ::context (d/without-nils context)})) +(defn get-external-session-id + [request] + (when-let [session-id (yreq/get-header request "x-external-session-id")] + (when-not (or (> (count session-id) 256) + (= session-id "null") + (str/blank? session-id)) + session-id))) + +(defn- get-external-event-origin + [request] + (when-let [origin (yreq/get-header request "x-event-origin")] + (when-not (or (> (count origin) 256) + (= origin "null") + (str/blank? origin)) + origin))) + ;; --- SPECS ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -126,8 +143,6 @@ (::rpc/profile-id params) uuid/zero) - session-id (get params ::rpc/external-session-id) - event-origin (get params ::rpc/external-event-origin) props (-> (or (::replace-props resultm) (-> params (merge (::props resultm)) @@ -138,8 +153,10 @@ token-id (::actoken/id request) context (-> (::context resultm) - (assoc :external-session-id session-id) - (assoc :external-event-origin event-origin) + (assoc :external-session-id + (get-external-session-id request)) + (assoc :external-event-origin + (get-external-event-origin request)) (assoc :access-token-id (some-> token-id str)) (d/without-nils)) diff --git a/backend/src/app/main.clj b/backend/src/app/main.clj index 678ef5cfc6..1fa26fabe1 100644 --- a/backend/src/app/main.clj +++ b/backend/src/app/main.clj @@ -259,14 +259,17 @@ ::oidc.providers/generic {::http.client/client (ig/ref ::http.client/client)} + ::oidc/providers + [(ig/ref ::oidc.providers/google) + (ig/ref ::oidc.providers/github) + (ig/ref ::oidc.providers/gitlab) + (ig/ref ::oidc.providers/generic)] + ::oidc/routes {::http.client/client (ig/ref ::http.client/client) ::db/pool (ig/ref ::db/pool) ::setup/props (ig/ref ::setup/props) - ::oidc/providers {:google (ig/ref ::oidc.providers/google) - :github (ig/ref ::oidc.providers/github) - :gitlab (ig/ref ::oidc.providers/gitlab) - :oidc (ig/ref ::oidc.providers/generic)} + ::oidc/providers (ig/ref ::oidc/providers) ::session/manager (ig/ref ::session/manager) ::email/blacklist (ig/ref ::email/blacklist) ::email/whitelist (ig/ref ::email/whitelist)} @@ -298,6 +301,7 @@ {::db/pool (ig/ref ::db/pool) ::mtx/metrics (ig/ref ::mtx/metrics) ::mbus/msgbus (ig/ref ::mbus/msgbus) + ::setup/props (ig/ref ::setup/props) ::session/manager (ig/ref ::session/manager)} :app.http.assets/routes diff --git a/backend/src/app/media.clj b/backend/src/app/media.clj index 350bd1bdc5..de1a90fe20 100644 --- a/backend/src/app/media.clj +++ b/backend/src/app/media.clj @@ -17,6 +17,7 @@ [app.common.time :as ct] [app.config :as cf] [app.db :as-alias db] + [app.http.client :as http] [app.storage :as-alias sto] [app.storage.tmp :as tmp] [buddy.core.bytes :as bb] @@ -37,6 +38,9 @@ org.im4java.core.IMOperation org.im4java.core.Info)) +(def default-max-file-size + (* 1024 1024 10)) ; 10 MiB + (def schema:upload [:map {:title "Upload"} [:filename :string] @@ -241,7 +245,7 @@ (ex/raise :type :validation :code :invalid-svg-file :hint "uploaded svg does not provides dimensions")) - (merge input info {:ts (ct/now)})) + (merge input info {:ts (ct/now) :size (fs/size path)})) (let [instance (Info. (str path)) mtype' (.getProperty instance "Mime type")] @@ -261,6 +265,7 @@ (assoc input :width width :height height + :size (fs/size path) :ts (ct/now))))))) (defmethod process-error org.im4java.core.InfoException @@ -270,6 +275,54 @@ :hint "invalid image" :cause error)) +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; IMAGE HELPERS +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(defn download-image + "Download an image from the provided URI and return the media input object" + [{:keys [::http/client]} uri] + (letfn [(parse-and-validate [{:keys [headers] :as response}] + (let [size (some-> (get headers "content-length") d/parse-integer) + mtype (get headers "content-type") + format (cm/mtype->format mtype) + max-size (cf/get :media-max-file-size default-max-file-size)] + + (when-not size + (ex/raise :type :validation + :code :unknown-size + :hint "seems like the url points to resource with unknown size")) + + (when (> size max-size) + (ex/raise :type :validation + :code :file-too-large + :hint (str/ffmt "the file size % is greater than the maximum %" + size + default-max-file-size))) + + (when (nil? format) + (ex/raise :type :validation + :code :media-type-not-allowed + :hint "seems like the url points to an invalid media object")) + + {:size size :mtype mtype :format format}))] + + (let [{:keys [body] :as response} (http/req! client + {:method :get :uri uri} + {:response-type :input-stream}) + {:keys [size mtype]} (parse-and-validate response) + path (tmp/tempfile :prefix "penpot.media.download.") + written (io/write* path body :size size)] + + (when (not= written size) + (ex/raise :type :internal + :code :mismatch-write-size + :hint "unexpected state: unable to write to file")) + + {;; :size size + :path path + :mtype mtype}))) + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; FONTS ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/backend/src/app/migrations.clj b/backend/src/app/migrations.clj index d9b3be6824..3976ba41c1 100644 --- a/backend/src/app/migrations.clj +++ b/backend/src/app/migrations.clj @@ -450,7 +450,13 @@ :fn (mg/resource "app/migrations/sql/0141-add-idx-to-file-library-rel.sql")} {:name "0141-add-file-data-table.sql" - :fn (mg/resource "app/migrations/sql/0141-add-file-data-table.sql")}]) + :fn (mg/resource "app/migrations/sql/0141-add-file-data-table.sql")} + + {:name "0142-add-sso-provider-table" + :fn (mg/resource "app/migrations/sql/0142-add-sso-provider-table.sql")} + + {:name "0143-http-session-v2-table" + :fn (mg/resource "app/migrations/sql/0143-add-http-session-v2-table.sql")}]) (defn apply-migrations! [pool name migrations] diff --git a/backend/src/app/migrations/sql/0142-add-sso-provider-table.sql b/backend/src/app/migrations/sql/0142-add-sso-provider-table.sql new file mode 100644 index 0000000000..4099cc4d26 --- /dev/null +++ b/backend/src/app/migrations/sql/0142-add-sso-provider-table.sql @@ -0,0 +1,33 @@ +CREATE TABLE sso_provider ( + id uuid PRIMARY KEY, + + created_at timestamptz NOT NULL DEFAULT now(), + modified_at timestamptz NOT NULL DEFAULT now(), + + is_enabled boolean NOT NULL DEFAULT true, + + type text NOT NULL CHECK (type IN ('oidc')), + domain text NOT NULL, + + client_id text NOT NULL, + client_secret text NOT NULL, + + base_uri text NOT NULL, + token_uri text NULL, + auth_uri text NULL, + user_uri text NULL, + jwks_uri text NULL, + logout_uri text NULL, + + roles_attr text NULL, + email_attr text NULL, + name_attr text NULL, + user_info_source text NOT NULL DEFAULT 'token' + CHECK (user_info_source IN ('token', 'userinfo', 'auto')), + + scopes text[] NULL, + roles text[] NULL +); + +CREATE UNIQUE INDEX sso_provider__domain__idx + ON sso_provider(domain); diff --git a/backend/src/app/migrations/sql/0143-add-http-session-v2-table.sql b/backend/src/app/migrations/sql/0143-add-http-session-v2-table.sql new file mode 100644 index 0000000000..0dda845ce7 --- /dev/null +++ b/backend/src/app/migrations/sql/0143-add-http-session-v2-table.sql @@ -0,0 +1,23 @@ +CREATE TABLE http_session_v2 ( + id uuid PRIMARY KEY, + + created_at timestamptz NOT NULL DEFAULT now(), + modified_at timestamptz NOT NULL DEFAULT now(), + + profile_id uuid REFERENCES profile(id) ON DELETE CASCADE, + user_agent text NULL, + + sso_provider_id uuid NULL REFERENCES sso_provider(id) ON DELETE CASCADE, + sso_session_id text NULL +); + +CREATE INDEX http_session_v2__profile_id__idx + ON http_session_v2(profile_id); + +CREATE INDEX http_session_v2__sso_provider_id__idx + ON http_session_v2(sso_provider_id) + WHERE sso_provider_id IS NOT NULL; + +CREATE INDEX http_session_v2__sso_session_id__idx + ON http_session_v2(sso_session_id) + WHERE sso_session_id IS NOT NULL; diff --git a/backend/src/app/rpc.clj b/backend/src/app/rpc.clj index fbd5d4b4d9..5b7d4a0edd 100644 --- a/backend/src/app/rpc.clj +++ b/backend/src/app/rpc.clj @@ -68,33 +68,21 @@ response (if (fn? result) (result request) (let [result (rph/unwrap result) - status (::http/status mdata 200) + status (or (::http/status mdata) + (if (nil? result) + 204 + 200)) headers (cond-> (::http/headers mdata {}) (yres/stream-body? result) (assoc "content-type" "application/octet-stream"))] {::yres/status status ::yres/headers headers ::yres/body result}))] + (-> response (handle-response-transformation request mdata) (handle-before-comple-hook mdata)))) -(defn get-external-session-id - [request] - (when-let [session-id (yreq/get-header request "x-external-session-id")] - (when-not (or (> (count session-id) 256) - (= session-id "null") - (str/blank? session-id)) - session-id))) - -(defn- get-external-event-origin - [request] - (when-let [origin (yreq/get-header request "x-event-origin")] - (when-not (or (> (count origin) 256) - (= origin "null") - (str/blank? origin)) - origin))) - (defn- make-rpc-handler "Ring handler that dispatches cmd requests and convert between internal async flow into ring async flow." @@ -105,23 +93,19 @@ etag (yreq/get-header request "if-none-match") profile-id (or (::session/profile-id request) (::actoken/profile-id request)) - ip-addr (inet/parse-request request) - session-id (get-external-session-id request) - event-origin (get-external-event-origin request) data (-> params (assoc ::handler-name handler-name) (assoc ::ip-addr ip-addr) (assoc ::request-at (ct/now)) - (assoc ::external-session-id session-id) - (assoc ::external-event-origin event-origin) - (assoc ::session/id (::session/id request)) (assoc ::cond/key etag) (cond-> (uuid? profile-id) (assoc ::profile-id profile-id))) - data (vary-meta data assoc ::http/request request) + data (with-meta data + {::http/request request}) + handler-fn (get methods (keyword handler-name) default-handler)] (when (and (or (= method :get) @@ -367,7 +351,6 @@ (let [public-uri (cf/get :public-uri)] ["/api" - ["/management" ["/methods/:type" {:middleware [[mw/shared-key-auth (cf/get :management-api-shared-key)] diff --git a/backend/src/app/rpc/commands/auth.clj b/backend/src/app/rpc/commands/auth.clj index b223cadb2f..742077e875 100644 --- a/backend/src/app/rpc/commands/auth.clj +++ b/backend/src/app/rpc/commands/auth.clj @@ -7,21 +7,24 @@ (ns app.rpc.commands.auth (:require [app.auth :as auth] + [app.auth.oidc :as oidc] [app.common.data :as d] - [app.common.data.macros :as dm] [app.common.exceptions :as ex] [app.common.features :as cfeat] [app.common.logging :as l] [app.common.schema :as sm] [app.common.time :as ct] + [app.common.uri :as u] [app.common.uuid :as uuid] [app.config :as cf] [app.db :as db] [app.email :as eml] [app.email.blacklist :as email.blacklist] [app.email.whitelist :as email.whitelist] + [app.http :as-alias http] [app.http.session :as session] [app.loggers.audit :as audit] + [app.media :as media] [app.rpc :as-alias rpc] [app.rpc.climit :as-alias climit] [app.rpc.commands.profile :as profile] @@ -30,6 +33,7 @@ [app.rpc.helpers :as rph] [app.setup :as-alias setup] [app.setup.welcome-file :refer [create-welcome-file]] + [app.storage :as sto] [app.tokens :as tokens] [app.util.services :as sv] [app.worker :as wrk] @@ -109,7 +113,7 @@ (assoc profile :is-admin (let [admins (cf/get :admins)] (contains? admins (:email profile)))))] (-> response - (rph/with-transform (session/create-fn cfg (:id profile))) + (rph/with-transform (session/create-fn cfg profile)) (rph/with-meta {::audit/props (audit/profile->props profile) ::audit/profile-id (:id profile)}))))] @@ -145,7 +149,24 @@ [cfg params] (if (= (:profile-id params) (::rpc/profile-id params)) - (rph/with-transform {} (session/delete-fn cfg)) + (let [{:keys [claims]} + (rph/get-auth-data params) + + provider + (some->> (get claims :sso-provider-id) + (oidc/get-provider cfg)) + + response + (if (and provider (:logout-uri provider)) + (let [params {"logout_hint" (get claims :sso-session-id) + "client_id" (get provider :client-id) + "post_logout_redirect_uri" (str (cf/get :public-uri))} + uri (-> (u/uri (:logout-uri provider)) + (assoc :query (u/map->query-string params)))] + {:redirect-uri uri}) + {})] + + (rph/with-transform response (session/delete-fn cfg))) {})) ;; ---- COMMAND: Recover Profile @@ -271,11 +292,29 @@ ;; ---- COMMAND: Register Profile -(defn create-profile! +(defn import-profile-picture + [cfg uri] + (try + (let [storage (sto/resolve cfg) + input (media/download-image cfg uri) + input (media/run {:cmd :info :input input}) + hash (sto/calculate-hash (:path input)) + content (-> (sto/content (:path input) (:size input)) + (sto/wrap-with-hash hash)) + sobject (sto/put-object! storage {::sto/content content + ::sto/deduplicate? true + :bucket "profile" + :content-type (:mtype input)})] + (:id sobject)) + (catch Throwable cause + (l/err :hint "unable to import profile picture" + :cause cause) + nil))) + +(defn create-profile "Create the profile entry on the database with limited set of input attrs (all the other attrs are filled with default values)." - [conn {:keys [email] :as params}] - (dm/assert! ::sm/email email) + [{:keys [::db/conn] :as cfg} {:keys [email] :as params}] (let [id (or (:id params) (uuid/next)) props (-> (audit/extract-utm-params params) (merge (:props params)) @@ -283,8 +322,7 @@ :viewed-walkthrough? false :nudge {:big 10 :small 1} :v2-info-shown true - :release-notes-viewed (:main cf/version)}) - (db/tjson)) + :release-notes-viewed (:main cf/version)})) password (or (:password params) "!") @@ -299,6 +337,12 @@ theme (:theme params nil) email (str/lower email) + photo-id (some->> (or (:oidc/picture props) + (:google/picture props) + (:github/picture props) + (:gitlab/picture props)) + (import-profile-picture cfg)) + params {:id id :fullname (:fullname params) :email email @@ -306,11 +350,13 @@ :lang locale :password password :deleted-at (:deleted-at params) - :props props + :props (db/tjson props) :theme theme + :photo-id photo-id :is-active is-active :is-muted is-muted :is-demo is-demo}] + (try (-> (db/insert! conn :profile params) (profile/decode-row)) @@ -323,7 +369,7 @@ (throw cause)))))) -(defn create-profile-rels! +(defn create-profile-rels [conn {:keys [id] :as profile}] (let [features (cfeat/get-enabled-features cf/flags) team (teams/create-team conn @@ -373,12 +419,13 @@ ;; to detect if the profile is already registered (or (profile/get-profile-by-email conn (:email claims)) (let [is-active (or (boolean (:is-active claims)) + (boolean (:email-verified claims)) (not (contains? cf/flags :email-verification))) params (-> params (assoc :is-active is-active) (update :password auth/derive-password)) - profile (->> (create-profile! conn params) - (create-profile-rels! conn))] + profile (->> (create-profile cfg params) + (create-profile-rels conn))] (vary-meta profile assoc :created true)))) created? (-> profile meta :created true?) @@ -416,10 +463,10 @@ (and (some? invitation) (= (:email profile) (:member-email invitation))) - (let [claims (assoc invitation :member-id (:id profile)) - token (tokens/generate cfg claims)] + (let [invitation (assoc invitation :member-id (:id profile)) + token (tokens/generate cfg invitation)] (-> {:invitation-token token} - (rph/with-transform (session/create-fn cfg (:id profile))) + (rph/with-transform (session/create-fn cfg profile claims)) (rph/with-meta {::audit/replace-props props ::audit/context {:action "accept-invitation"} ::audit/profile-id (:id profile)}))) @@ -430,7 +477,7 @@ created? (if (:is-active profile) (-> (profile/strip-private-attrs profile) - (rph/with-transform (session/create-fn cfg (:id profile))) + (rph/with-transform (session/create-fn cfg profile claims)) (rph/with-defer create-welcome-file-when-needed) (rph/with-meta {::audit/replace-props props @@ -559,4 +606,32 @@ [cfg params] (db/tx-run! cfg request-profile-recovery params)) +;; --- COMMAND: get-sso-config +(defn- extract-domain + "Extract the domain part from email" + [email] + (let [at (str/last-index-of email "@")] + (when (and (>= at 0) + (< at (dec (count email)))) + (-> (subs email (inc at)) + (str/trim) + (str/lower))))) + +(def ^:private schema:get-sso-provider + [:map {:title "get-sso-config"} + [:email ::sm/email]]) + +(def ^:private schema:get-sso-provider-result + [:map {:title "SSOProvider"} + [:id ::sm/uuid]]) + +(sv/defmethod ::get-sso-provider + {::rpc/auth false + ::doc/added "2.12" + ::sm/params schema:get-sso-provider + ::sm/result schema:get-sso-provider-result} + [cfg {:keys [email]}] + (when-let [domain (extract-domain email)] + (when-let [config (db/get* cfg :sso-provider {:domain domain})] + (select-keys config [:id])))) diff --git a/backend/src/app/rpc/commands/demo.clj b/backend/src/app/rpc/commands/demo.clj index d6825a3f68..e609d7ec79 100644 --- a/backend/src/app/rpc/commands/demo.clj +++ b/backend/src/app/rpc/commands/demo.clj @@ -49,9 +49,9 @@ :deleted-at (ct/in-future (cf/get-deletion-delay)) :password (derive-password password) :props {}} - profile (db/tx-run! cfg (fn [{:keys [::db/conn]}] - (->> (auth/create-profile! conn params) - (auth/create-profile-rels! conn))))] + profile (db/tx-run! cfg (fn [{:keys [::db/conn] :as cfg}] + (->> (auth/create-profile cfg params) + (auth/create-profile-rels conn))))] (with-meta {:email email :password password} {::audit/profile-id (:id profile)}))) diff --git a/backend/src/app/rpc/commands/ldap.clj b/backend/src/app/rpc/commands/ldap.clj index 26524f208f..5aa2a21935 100644 --- a/backend/src/app/rpc/commands/ldap.clj +++ b/backend/src/app/rpc/commands/ldap.clj @@ -66,12 +66,12 @@ :member-email (:email profile)) token (tokens/generate cfg claims)] (-> {:invitation-token token} - (rph/with-transform (session/create-fn cfg (:id profile))) + (rph/with-transform (session/create-fn cfg profile)) (rph/with-meta {::audit/props (:props profile) ::audit/profile-id (:id profile)}))) (-> (profile/strip-private-attrs profile) - (rph/with-transform (session/create-fn cfg (:id profile))) + (rph/with-transform (session/create-fn cfg profile)) (rph/with-meta {::audit/props (:props profile) ::audit/profile-id (:id profile)})))))) @@ -83,6 +83,6 @@ (profile/clean-email) (profile/get-profile-by-email conn)) (->> (assoc info :is-active true :is-demo false) - (auth/create-profile! conn) - (auth/create-profile-rels! conn) + (auth/create-profile cfg) + (auth/create-profile-rels conn) (profile/strip-private-attrs)))))) diff --git a/backend/src/app/rpc/commands/media.clj b/backend/src/app/rpc/commands/media.clj index 63793742a0..80e49b6366 100644 --- a/backend/src/app/rpc/commands/media.clj +++ b/backend/src/app/rpc/commands/media.clj @@ -7,14 +7,10 @@ (ns app.rpc.commands.media (:require [app.common.data :as d] - [app.common.exceptions :as ex] - [app.common.media :as cm] [app.common.schema :as sm] [app.common.time :as ct] [app.common.uuid :as uuid] - [app.config :as cf] [app.db :as db] - [app.http.client :as http] [app.loggers.audit :as-alias audit] [app.media :as media] [app.rpc :as-alias rpc] @@ -22,13 +18,7 @@ [app.rpc.commands.files :as files] [app.rpc.doc :as-alias doc] [app.storage :as sto] - [app.storage.tmp :as tmp] - [app.util.services :as sv] - [cuerdas.core :as str] - [datoteka.io :as io])) - -(def default-max-file-size - (* 1024 1024 10)) ; 10 MiB + [app.util.services :as sv])) (def thumbnail-options {:width 100 @@ -197,56 +187,12 @@ mobj)) -(defn download-image - [{:keys [::http/client]} uri] - (letfn [(parse-and-validate [{:keys [headers] :as response}] - (let [size (some-> (get headers "content-length") d/parse-integer) - mtype (get headers "content-type") - format (cm/mtype->format mtype) - max-size (cf/get :media-max-file-size default-max-file-size)] - - (when-not size - (ex/raise :type :validation - :code :unknown-size - :hint "seems like the url points to resource with unknown size")) - - (when (> size max-size) - (ex/raise :type :validation - :code :file-too-large - :hint (str/ffmt "the file size % is greater than the maximum %" - size - default-max-file-size))) - - (when (nil? format) - (ex/raise :type :validation - :code :media-type-not-allowed - :hint "seems like the url points to an invalid media object")) - - {:size size :mtype mtype :format format}))] - - (let [{:keys [body] :as response} (http/req! client - {:method :get :uri uri} - {:response-type :input-stream :sync? true}) - {:keys [size mtype]} (parse-and-validate response) - path (tmp/tempfile :prefix "penpot.media.download.") - written (io/write* path body :size size)] - - (when (not= written size) - (ex/raise :type :internal - :code :mismatch-write-size - :hint "unexpected state: unable to write to file")) - - {:filename "tempfile" - :size size - :path path - :mtype mtype}))) - (defn- create-file-media-object-from-url [cfg {:keys [url name] :as params}] - (let [content (download-image cfg url) + (let [content (media/download-image cfg url) params (-> params (assoc :content content) - (assoc :name (or name (:filename content))))] + (assoc :name (d/nilv name "unknown")))] ;; NOTE: we use the climit here in a dynamic invocation because we ;; don't want saturate the process-image limit with IO (download diff --git a/backend/src/app/rpc/commands/profile.clj b/backend/src/app/rpc/commands/profile.clj index 4207db2d22..631f84af58 100644 --- a/backend/src/app/rpc/commands/profile.clj +++ b/backend/src/app/rpc/commands/profile.clj @@ -154,7 +154,6 @@ (declare validate-password!) (declare update-profile-password!) -(declare invalidate-profile-session!) (def ^:private schema:update-profile-password @@ -169,8 +168,7 @@ ::climit/id :auth/global ::db/transaction true} [cfg {:keys [::rpc/profile-id password] :as params}] - (let [profile (validate-password! cfg (assoc params :profile-id profile-id)) - session-id (::session/id params)] + (let [profile (validate-password! cfg (assoc params :profile-id profile-id))] (when (= (:email profile) (str/lower (:password params))) (ex/raise :type :validation @@ -178,14 +176,12 @@ :hint "you can't use your email as password")) (update-profile-password! cfg (assoc profile :password password)) - (invalidate-profile-session! cfg profile-id session-id) - nil)) -(defn- invalidate-profile-session! - "Removes all sessions except the current one." - [{:keys [::db/conn]} profile-id session-id] - (let [sql "delete from http_session where profile_id = ? and id != ?"] - (:next.jdbc/update-count (db/exec-one! conn [sql profile-id session-id])))) + (->> (rph/get-request params) + (session/get-session) + (session/invalidate-others cfg)) + + nil)) (defn- validate-password! [{:keys [::db/conn] :as cfg} {:keys [profile-id old-password] :as params}] @@ -284,9 +280,9 @@ :file-path (str (:path file)) :file-mtype (:mtype file)}})))) -(defn- generate-thumbnail! - [_ file] - (let [input (media/run {:cmd :info :input file}) +(defn- generate-thumbnail + [_ input] + (let [input (media/run {:cmd :info :input input}) thumb (media/run {:cmd :profile-thumbnail :format :jpeg :quality 85 @@ -307,7 +303,7 @@ (assoc ::climit/id [[:process-image/by-profile (:profile-id params)] [:process-image/global]]) (assoc ::climit/label "upload-photo") - (climit/invoke! generate-thumbnail! file))] + (climit/invoke! generate-thumbnail file))] (sto/put-object! storage params))) ;; --- MUTATION: Request Email Change diff --git a/backend/src/app/rpc/commands/verify_token.clj b/backend/src/app/rpc/commands/verify_token.clj index b650e6a8b9..a3454f7135 100644 --- a/backend/src/app/rpc/commands/verify_token.clj +++ b/backend/src/app/rpc/commands/verify_token.clj @@ -73,7 +73,7 @@ {:id (:id profile)})) (-> claims - (rph/with-transform (session/create-fn cfg profile-id)) + (rph/with-transform (session/create-fn cfg profile)) (rph/with-meta {::audit/name "verify-profile-email" ::audit/props (audit/profile->props profile) ::audit/profile-id (:id profile)})))) diff --git a/backend/src/app/rpc/helpers.clj b/backend/src/app/rpc/helpers.clj index 5b117b82fd..f014410ab4 100644 --- a/backend/src/app/rpc/helpers.clj +++ b/backend/src/app/rpc/helpers.clj @@ -83,3 +83,16 @@ "A convenience allias for yetti.response/stream-body" [f] (yres/stream-body f)) + +(defn get-request + "Get http request from RPC params" + [params] + (assert (contains? params ::rpc/request-at) "rpc params required") + (-> (meta params) + (get ::http/request))) + +(defn get-auth-data + "Get http auth-data from RPC params" + [params] + (-> (get-request params) + (get ::http/auth-data))) diff --git a/backend/src/app/srepl/cli.clj b/backend/src/app/srepl/cli.clj index 4df356752d..519df65b6e 100644 --- a/backend/src/app/srepl/cli.clj +++ b/backend/src/app/srepl/cli.clj @@ -61,8 +61,8 @@ :is-active is-active :password password :props {}}] - (->> (cmd.auth/create-profile! conn params) - (cmd.auth/create-profile-rels! conn))))))) + (->> (cmd.auth/create-profile system params) + (cmd.auth/create-profile-rels conn))))))) (defmethod exec-command "update-profile" [{:keys [fullname email password is-active]}] diff --git a/backend/src/app/srepl/main.clj b/backend/src/app/srepl/main.clj index 51832cefac..30c8b403dc 100644 --- a/backend/src/app/srepl/main.clj +++ b/backend/src/app/srepl/main.clj @@ -25,6 +25,7 @@ [app.db.sql :as-alias sql] [app.features.fdata :as fdata] [app.features.file-snapshots :as fsnap] + [app.http.session :as session] [app.loggers.audit :as audit] [app.main :as main] [app.msgbus :as mbus] @@ -843,10 +844,33 @@ :deleted-at deleted-at :id id}))))))) +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; SSO +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(defn add-sso-config + [& {:keys [base-uri client-id client-secret domain]}] + + (assert (and (string? base-uri) (str/starts-with? base-uri "http")) "expected a valid base-uri") + (assert (string? client-id) "expected a valid client-id") + (assert (string? client-secret) "expected a valid client-secret") + (assert (string? domain) "expected a valid domain") + (db/insert! main/system :sso-provider + {:id (uuid/next) + :type "oidc" + :client-id client-id + :client-secret client-secret + :domain domain + :base-uri base-uri})) + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; MISC ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +(defn decode-session-token + [token] + (session/decode-token main/system token)) + (defn instrument-var [var] (alter-var-root var (fn [f] diff --git a/backend/test/backend_tests/helpers.clj b/backend/test/backend_tests/helpers.clj index 20f83ce455..4bacb15613 100644 --- a/backend/test/backend_tests/helpers.clj +++ b/backend/test/backend_tests/helpers.clj @@ -104,13 +104,8 @@ (assoc-in [:app.rpc/methods :app.setup/templates] templates) (dissoc :app.srepl/server :app.http/server - :app.http/router - :app.auth.oidc.providers/google - :app.auth.oidc.providers/gitlab - :app.auth.oidc.providers/github - :app.auth.oidc.providers/generic + :app.http/route :app.setup/templates - :app.auth.oidc/routes :app.http.oauth/handler :app.notifications/handler :app.loggers.mattermost/reporter @@ -182,10 +177,10 @@ :is-demo false} params)] (db/run! system - (fn [{:keys [::db/conn]}] + (fn [{:keys [::db/conn] :as cfg}] (->> params - (cmd.auth/create-profile! conn) - (cmd.auth/create-profile-rels! conn))))))) + (cmd.auth/create-profile cfg) + (cmd.auth/create-profile-rels conn))))))) (defn create-project* ([i params] (create-project* *system* i params)) diff --git a/backend/test/backend_tests/http_management_test.clj b/backend/test/backend_tests/http_management_test.clj index 9e8f554017..21201f62ff 100644 --- a/backend/test/backend_tests/http_management_test.clj +++ b/backend/test/backend_tests/http_management_test.clj @@ -22,17 +22,6 @@ (t/use-fixtures :once th/state-init) (t/use-fixtures :each th/database-reset) - -(t/deftest authenticate-method - (let [profile (th/create-profile* 1) - token (#'sess/gen-token th/*system* {:profile-id (:id profile)}) - request {:params {:token token}} - response (#'mgmt/authenticate th/*system* request)] - - (t/is (= 200 (::yres/status response))) - (t/is (= "authentication" (-> response ::yres/body :iss))) - (t/is (= (:id profile) (-> response ::yres/body :uid))))) - (t/deftest get-customer-method (let [profile (th/create-profile* 1) request {:params {:id (:id profile)}} @@ -89,7 +78,3 @@ (let [subs' (-> response ::yres/body :subscription)] (t/is (= subs' subs)))))) - - - - diff --git a/backend/test/backend_tests/http_middleware_test.clj b/backend/test/backend_tests/http_middleware_test.clj index c91ba0ef20..2d4b8ac029 100644 --- a/backend/test/backend_tests/http_middleware_test.clj +++ b/backend/test/backend_tests/http_middleware_test.clj @@ -8,8 +8,8 @@ (:require [app.common.time :as ct] [app.db :as db] + [app.http :as-alias http] [app.http.access-token] - [app.http.auth :as-alias auth] [app.http.middleware :as mw] [app.http.session :as session] [app.main :as-alias main] @@ -42,13 +42,14 @@ (handler (->DummyRequest {} {})) - (t/is (nil? (::auth/token-type @request))) - (t/is (nil? (::auth/token @request))) + (t/is (nil? (::http/auth-data @request))) (handler (->DummyRequest {"authorization" "Token aaaa"} {})) - (t/is (= :token (::auth/token-type @request))) - (t/is (= "aaaa" (::auth/token @request))))) + (let [{:keys [token claims] token-type :type} (get @request ::http/auth-data)] + (t/is (= :token token-type)) + (t/is (= "aaaa" token)) + (t/is (nil? claims))))) (t/deftest auth-middleware-2 (let [request (volatile! nil) @@ -57,16 +58,14 @@ {})] (handler (->DummyRequest {} {})) - - (t/is (nil? (::auth/token-type @request))) - (t/is (nil? (::auth/token @request))) - (t/is (nil? (::auth/claims @request))) + (t/is (nil? (::http/auth-data @request))) (handler (->DummyRequest {"authorization" "Bearer aaaa"} {})) - (t/is (= :bearer (::auth/token-type @request))) - (t/is (= "aaaa" (::auth/token @request))) - (t/is (nil? (::auth/claims @request))))) + (let [{:keys [token claims] token-type :type} (get @request ::http/auth-data)] + (t/is (= :bearer token-type)) + (t/is (= "aaaa" token)) + (t/is (nil? claims))))) (t/deftest auth-middleware-3 (let [request (volatile! nil) @@ -75,35 +74,14 @@ {})] (handler (->DummyRequest {} {})) - - (t/is (nil? (::auth/token-type @request))) - (t/is (nil? (::auth/token @request))) - (t/is (nil? (::auth/claims @request))) + (t/is (nil? (::http/auth-data @request))) (handler (->DummyRequest {} {"auth-token" "foobar"})) - (t/is (= :cookie (::auth/token-type @request))) - (t/is (= "foobar" (::auth/token @request))) - (t/is (nil? (::auth/claims @request))))) - -(t/deftest auth-middleware-4 - (let [request (volatile! nil) - handler (#'app.http.middleware/wrap-auth - (fn [req] (vreset! request req)) - {:cookie (fn [_] "foobaz")})] - - (handler (->DummyRequest {} {})) - - (t/is (nil? (::auth/token-type @request))) - (t/is (nil? (::auth/token @request))) - (t/is (nil? (::auth/claims @request))) - - (handler (->DummyRequest {} {"auth-token" "foobar"})) - - (t/is (= :cookie (::auth/token-type @request))) - (t/is (= "foobar" (::auth/token @request))) - (t/is (delay? (::auth/claims @request))) - (t/is (= "foobaz" (-> @request ::auth/claims deref))))) + (let [{:keys [token claims] token-type :type} (get @request ::http/auth-data)] + (t/is (= :cookie token-type)) + (t/is (= "foobar" token)) + (t/is (nil? claims))))) (t/deftest shared-key-auth (let [handler (#'app.http.middleware/wrap-shared-key-auth @@ -122,40 +100,36 @@ (t/deftest access-token-authz (let [profile (th/create-profile* 1) token (db/tx-run! th/*system* app.rpc.commands.access-token/create-access-token (:id profile) "test" nil) - request (volatile! {}) + handler (#'app.http.access-token/wrap-authz identity th/*system*)] - handler (#'app.http.access-token/wrap-authz - (fn [req] (vreset! request req)) - th/*system*)] + (let [response (handler nil)] + (t/is (nil? response))) - (handler nil) - (t/is (nil? @request)) - - (handler {::auth/claims (delay {:tid (:id token)}) - ::auth/token-type :token}) - - (t/is (= #{} (:app.http.access-token/perms @request))) - (t/is (= (:id profile) (:app.http.access-token/profile-id @request))))) + (let [response (handler {::http/auth-data {:type :token :token "foobar" :claims {:tid (:id token)}}})] + (t/is (= #{} (:app.http.access-token/perms response))) + (t/is (= (:id profile) (:app.http.access-token/profile-id response)))))) (t/deftest session-authz - (let [manager (session/inmemory-manager) - profile (th/create-profile* 1) - handler (-> (fn [req] req) - (#'session/wrap-authz {::session/manager manager}) - (#'mw/wrap-auth {}))] + (let [cfg th/*system* + manager (session/inmemory-manager) + profile (th/create-profile* 1) + handler (-> (fn [req] req) + (#'session/wrap-authz {::session/manager manager}) + (#'mw/wrap-auth {:bearer (partial session/decode-token cfg) + :cookie (partial session/decode-token cfg)})) + session (->> (session/create-session manager {:profile-id (:id profile) + :user-agent "user agent"}) + (#'session/assign-token cfg)) - (let [response (handler (->DummyRequest {} {"auth-token" "foobar"}))] - (t/is (= :cookie (::auth/token-type response))) - (t/is (= "foobar" (::auth/token response)))) + response (handler (->DummyRequest {} {"auth-token" (:token session)})) + {:keys [token claims] token-type :type} + (get response ::http/auth-data)] - (session/write! manager "foobar" {:profile-id (:id profile) - :user-agent "user agent" - :created-at (ct/now)}) - - (let [response (handler (->DummyRequest {} {"auth-token" "foobar"}))] - (t/is (= :cookie (::auth/token-type response))) - (t/is (= "foobar" (::auth/token response))) - (t/is (= (:id profile) (::session/profile-id response))) - (t/is (= "foobar" (::session/id response)))))) + (t/is (= :cookie token-type)) + (t/is (= (:token session) token)) + (t/is (= "authentication" (:iss claims))) + (t/is (= "penpot" (:aud claims))) + (t/is (= (:id session) (:sid claims))) + (t/is (= (:id profile) (:uid claims))))) diff --git a/common/src/app/common/flags.cljc b/common/src/app/common/flags.cljc index b0faa1629a..4177de8855 100644 --- a/common/src/app/common/flags.cljc +++ b/common/src/app/common/flags.cljc @@ -33,7 +33,9 @@ :login-with-ldap ;; Uses any generic authentication provider that implements OIDC protocol as credentials. :login-with-oidc - ;; Allows registration with Open ID + ;; Enables custom SSO flow + :login-with-custom-sso + ;; Allows registration with OIDC (takes effect only when general `registration` is disabled) :oidc-registration ;; This logs to console the invitation tokens. It's useful in case the SMTP is not configured. :log-invitation-tokens}) diff --git a/frontend/src/app/main/data/auth.cljs b/frontend/src/app/main/data/auth.cljs index 93225c506a..cfa11e4263 100644 --- a/frontend/src/app/main/data/auth.cljs +++ b/frontend/src/app/main/data/auth.cljs @@ -8,7 +8,6 @@ "Auth related data events" (:require [app.common.data :as d] - [app.common.data.macros :as dm] [app.common.exceptions :as ex] [app.common.schema :as sm] [app.common.uuid :as uuid] @@ -148,9 +147,7 @@ (defn login-with-ldap [params] - (dm/assert! - "expected valid params" - (sm/check schema:login-with-ldap params)) + (assert (sm/check schema:login-with-ldap params)) (ptk/reify ::login-with-ldap ptk/WatchEvent @@ -166,6 +163,32 @@ (logged-in)))) (rx/catch on-error)))))) +(def ^:private schema:login-with-sso + [:map {:title "login-with-sso"} + [:provider [:or :string ::sm/uuid]]]) + +(defn login-with-sso + "Start the SSO flow" + [params] + (assert (sm/check schema:login-with-sso params)) + (ptk/reify ::login-with-sso + ptk/WatchEvent + (watch [_ _ _] + (->> (rp/cmd! :login-with-oidc params) + (rx/map (fn [{:keys [redirect-uri] :as rsp}] + (if redirect-uri + (rt/nav-raw :uri redirect-uri) + (ex/raise :type :internal + :code :unexpected-response + :hint "unexpected response from OIDC method" + :resp (pr-str rsp))))) + (rx/catch (fn [cause] + (let [{:keys [type code] :as error} (ex-data cause)] + (if (and (= type :restriction) + (= code :provider-not-configured)) + (rx/of (ntf/error (tr "errors.auth-provider-not-configured"))) + (rx/throw cause))))))))) + (defn login-from-token "Used mainly as flow continuation after token validation." [{:keys [profile] :as tdata}] @@ -201,7 +224,7 @@ ;; --- EVENT: logout (defn logged-out - [] + [{:keys [redirect-uri]}] (ptk/reify ::logged-out ptk/UpdateEvent (update [_ state] @@ -209,12 +232,16 @@ ptk/WatchEvent (watch [_ _ _] - (rx/merge - ;; NOTE: We need the `effect` of the current event to be - ;; executed before the redirect. - (->> (rx/of (rt/nav :auth-login)) - (rx/observe-on :async)) - (rx/of (ws/finalize)))) + (if redirect-uri + (->> (rx/of (rt/nav-raw :uri (str redirect-uri))) + (rx/observe-on :async)) + + (rx/merge + ;; NOTE: We need the `effect` of the current event to be + ;; executed before the redirect. + (->> (rx/of (rt/nav :auth-login)) + (rx/observe-on :async)) + (rx/of (ws/finalize))))) ptk/EffectEvent (effect [_ _ _] @@ -235,7 +262,7 @@ (rx/mapcat (fn [_] (->> (rp/cmd! :logout {:profile-id profile-id}) (rx/delay-at-least 300) - (rx/catch (constantly (rx/of 1)))))) + (rx/catch (constantly (rx/of nil)))))) (rx/map logged-out)))))) ;; --- Update Profile @@ -248,9 +275,7 @@ (defn request-profile-recovery [data] - (dm/assert! - "expected valid parameters" - (sm/check schema:request-profile-recovery data)) + (assert (sm/check schema:request-profile-recovery data)) (ptk/reify ::request-profile-recovery ptk/WatchEvent @@ -273,9 +298,7 @@ (defn recover-profile [data] - (dm/assert! - "expected valid arguments" - (sm/check schema:recover-profile data)) + (assert (sm/check schema:recover-profile data)) (ptk/reify ::recover-profile ptk/WatchEvent diff --git a/frontend/src/app/main/repo.cljs b/frontend/src/app/main/repo.cljs index 8db5c58367..adb1d5deea 100644 --- a/frontend/src/app/main/repo.cljs +++ b/frontend/src/app/main/repo.cljs @@ -171,9 +171,8 @@ (send! id params nil)) (defmethod cmd! :login-with-oidc - [_ {:keys [provider] :as params}] - (let [uri (u/join cf/public-uri "api/auth/oauth/" (d/name provider)) - params (dissoc params :provider)] + [_ params] + (let [uri (u/join cf/public-uri "api/auth/oidc")] (->> (http/send! {:method :post :uri uri :credentials "include" diff --git a/frontend/src/app/main/ui/auth/login.cljs b/frontend/src/app/main/ui/auth/login.cljs index 5f8888da3b..7273a18205 100644 --- a/frontend/src/app/main/ui/auth/login.cljs +++ b/frontend/src/app/main/ui/auth/login.cljs @@ -23,12 +23,11 @@ [app.main.ui.notifications.context-notification :refer [context-notification]] [app.util.dom :as dom] [app.util.i18n :refer [tr]] - [app.util.keyboard :as k] [app.util.storage :as s] [beicon.v2.core :as rx] [rumext.v2 :as mf])) -(def show-alt-login-buttons? +(def ^:const show-sso-login-buttons? (some (partial contains? cf/flags) [:login-with-google :login-with-github @@ -47,53 +46,36 @@ (st/emit! (da/create-demo-profile))) (defn- store-login-redirect - [save-login-redirect] + [] (binding [s/*sync* true] - (if (some? save-login-redirect) - ;; Save the current login raw uri for later redirect user back to - ;; the same page, we need it to be synchronous because the user is - ;; going to be redirected instantly to the oidc provider uri - (swap! s/session assoc :login-redirect (rt/get-current-href)) - ;; Clean the login redirect - (swap! s/session dissoc :login-redirect)))) + ;; Save the current login raw uri for later redirect user back to + ;; the same page, we need it to be synchronous because the user is + ;; going to be redirected instantly to the oidc provider uri + (swap! s/session assoc :login-redirect (rt/get-current-href)))) -(defn- login-with-oidc - [event provider params] - (dom/prevent-default event) +(defn- clear-login-redirect + [] + (binding [s/*sync* true] + (swap! s/session dissoc :login-redirect))) - (store-login-redirect (:save-login-redirect params)) - - ;; FIXME: this code should be probably moved outside of the UI - (->> (rp/cmd! :login-with-oidc (assoc params :provider provider)) - (rx/subs! (fn [{:keys [redirect-uri] :as rsp}] - (if redirect-uri - (st/emit! (rt/nav-raw :uri redirect-uri)) - (log/error :hint "unexpected response from OIDC method" - :resp (pr-str rsp)))) - (fn [cause] - (let [{:keys [type code] :as error} (ex-data cause)] - (cond - (and (= type :restriction) - (= code :provider-not-configured)) - (st/emit! (ntf/error (tr "errors.auth-provider-not-configured"))) - - :else - (st/emit! (ntf/error (tr "errors.generic"))))))))) +(defn- login-with-sso + [provider params] + (let [params (assoc params :provider provider)] + (st/emit! (da/login-with-sso params)))) (def ^:private schema:login-form [:map {:title "LoginForm"} [:email [::sm/email {:error/code "errors.invalid-email"}]] - [:password [:string {:min 1}]] + [:password {:optional true} [:string {:min 1}]] [:invitation-token {:optional true} [:string {:min 1}]]]) -(mf/defc login-form - [{:keys [params on-success-callback on-recovery-request origin] :as props}] +(mf/defc login-form* + [{:keys [params handle-redirect on-success-callback on-recovery-request origin] :as props}] (let [initial (mf/with-memo [params] params) error (mf/use-state false) form (fm/use-form :schema schema:login-form :initial initial) - on-error (fn [cause] (let [cause (ex-data cause)] @@ -121,20 +103,41 @@ :else (reset! error (tr "errors.generic"))))) + show-password-field* + (mf/use-state #(not (contains? cf/flags :login-with-custom-sso))) + + show-password-field? + (deref show-password-field*) + on-success (fn [data] (when (fn? on-success-callback) (on-success-callback data))) on-submit - (mf/use-callback + (mf/use-fn + (mf/deps show-password-field? params) (fn [form _event] - (store-login-redirect (:save-login-redirect params)) (reset! error nil) - (let [params (with-meta (:clean-data @form) - {:on-error on-error - :on-success on-success})] - (st/emit! (da/login params))))) + + (let [data (:clean-data @form)] + (if show-password-field? + (let [params (-> (merge params data) + (with-meta {:on-error on-error + :on-success on-success}))] + (st/emit! (da/login params))) + + (let [params (merge params data)] + (->> (rp/cmd! :get-sso-provider {:email (:email params)}) + (rx/map :id) + (rx/catch (fn [cause] + (log/error :hint "error on retrieving sso provider" :cause cause) + (rx/of nil))) + (rx/subs! (fn [sso-provider-id] + (if sso-provider-id + (let [params {:provider sso-provider-id}] + (st/emit! (da/login-with-sso params))) + (reset! show-password-field* true)))))))))) on-submit-ldap (mf/use-callback @@ -150,12 +153,15 @@ :on-success on-success})] (st/emit! (da/login-with-ldap params))))) - default-recovery-req - (mf/use-fn - #(st/emit! (rt/nav :auth-recovery-request))) + on-recovery-request + (or on-recovery-request + #(st/emit! (rt/nav :auth-recovery-request)))] - on-recovery-request (or on-recovery-request - default-recovery-req)] + + (mf/with-effect [handle-redirect] + (if handle-redirect + (store-login-redirect) + (clear-login-redirect))) [:* (when-let [message @error] @@ -165,6 +171,7 @@ [:& fm/form {:on-submit on-submit :class (stl/css :login-form) :form form} + [:div {:class (stl/css :fields-row)} [:& fm/input {:name :email @@ -172,12 +179,14 @@ :label (tr "auth.work-email") :class (stl/css :form-field)}]] - [:div {:class (stl/css :fields-row)} - [:& fm/input - {:type "password" - :name :password - :label (tr "auth.password") - :class (stl/css :form-field)}]] + (when show-password-field? + [:div {:class (stl/css :fields-row)} + [:& fm/input + {:type "password" + :name :password + :auto-focus? true + :label (tr "auth.password") + :class (stl/css :form-field)}]]) (when (and (not= origin :viewer) (or (contains? cf/flags :login) @@ -202,12 +211,12 @@ :class (stl/css :login-ldap-button) :on-click on-submit-ldap}])]]])) -(mf/defc login-buttons +(mf/defc login-sso-buttons* [{:keys [params] :as props}] - (let [login-with-google (mf/use-fn (mf/deps params) #(login-with-oidc % :google params)) - login-with-github (mf/use-fn (mf/deps params) #(login-with-oidc % :github params)) - login-with-gitlab (mf/use-fn (mf/deps params) #(login-with-oidc % :gitlab params)) - login-with-oidc (mf/use-fn (mf/deps params) #(login-with-oidc % :oidc params))] + (let [login-with-google (mf/use-fn (mf/deps params) #(login-with-sso "google" params)) + login-with-github (mf/use-fn (mf/deps params) #(login-with-sso "github" params)) + login-with-gitlab (mf/use-fn (mf/deps params) #(login-with-sso "gitlab" params)) + login-with-oidc (mf/use-fn (mf/deps params) #(login-with-sso "oidc" params))] [:div {:class (stl/css :auth-buttons)} (when (contains? cf/flags :login-with-google) @@ -234,32 +243,12 @@ :label (tr "auth.login-with-oidc-submit") :class (stl/css :login-btn :btn-oidc-auth)}])])) -(mf/defc login-button-oidc +(mf/defc login-dialog* [{:keys [params] :as props}] - (let [login-oidc - (mf/use-fn - (mf/deps params) - (fn [event] - (login-with-oidc event :oidc params))) - - handle-key-down - (mf/use-fn - (fn [event] - (when (k/enter? event) - (login-oidc event))))] - (when (contains? cf/flags :login-with-oidc) - [:button {:tab-index "0" - :class (stl/css :link-entry :link-oidc) - :on-key-down handle-key-down - :on-click login-oidc} - (tr "auth.login-with-oidc-submit")]))) - -(mf/defc login-methods - [{:keys [params on-success-callback on-recovery-request origin] :as props}] [:* - (when show-alt-login-buttons? + (when show-sso-login-buttons? [:* - [:& login-buttons {:params params}] + [:> login-sso-buttons* {:params params}] (when (or (contains? cf/flags :login) (contains? cf/flags :login-with-password) @@ -269,7 +258,7 @@ (when (or (contains? cf/flags :login) (contains? cf/flags :login-with-password) (contains? cf/flags :login-with-ldap)) - [:& login-form {:params params :on-success-callback on-success-callback :on-recovery-request on-recovery-request :origin origin}])]) + [:> login-form* props])]) (mf/defc login-page [{:keys [params] :as props}] @@ -287,7 +276,7 @@ (when (contains? cf/flags :demo-warning) [:& demo-warning]) - [:& login-methods {:params params}] + [:> login-dialog* {:params params}] [:hr {:class (stl/css :separator)}] diff --git a/frontend/src/app/main/ui/auth/register.cljs b/frontend/src/app/main/ui/auth/register.cljs index 570d62d106..45ec87dd2e 100644 --- a/frontend/src/app/main/ui/auth/register.cljs +++ b/frontend/src/app/main/ui/auth/register.cljs @@ -191,9 +191,9 @@ {::mf/props :obj} [{:keys [params hide-separator on-success-callback]}] [:* - (when login/show-alt-login-buttons? - [:& login/login-buttons {:params params}]) - (when (or login/show-alt-login-buttons? (false? hide-separator)) + (when login/show-sso-login-buttons? + [:> login/login-sso-buttons* {:params params}]) + (when (or login/show-sso-login-buttons? (false? hide-separator)) [:hr {:class (stl/css :separator)}]) (when (contains? cf/flags :login-with-password) [:& register-form {:params params :on-success-callback on-success-callback}])]) diff --git a/frontend/src/app/main/ui/static.cljs b/frontend/src/app/main/ui/static.cljs index a943f01c91..1140ee319b 100644 --- a/frontend/src/app/main/ui/static.cljs +++ b/frontend/src/app/main/ui/static.cljs @@ -21,7 +21,7 @@ [app.main.repo :as rp] [app.main.router :as rt] [app.main.store :as st] - [app.main.ui.auth.login :refer [login-methods]] + [app.main.ui.auth.login :refer [login-dialog*]] [app.main.ui.auth.recovery-request :refer [recovery-request-page recovery-sent-page]] [app.main.ui.auth.register :as register] [app.main.ui.dashboard.sidebar :refer [sidebar*]] @@ -75,7 +75,8 @@ [:div {:class (stl/css :main-message)} (tr "errors.invite-invalid")] [:div {:class (stl/css :desc-message)} (tr "errors.invite-invalid.info")]]) -(mf/defc login-dialog* +(mf/defc login-modal* + {::mf/private true} [] (let [current-section (mf/use-state :login) user-email (mf/use-state "") @@ -136,9 +137,9 @@ [:* [:div {:class (stl/css :logo-title)} (tr "labels.login")] [:div {:class (stl/css :logo-subtitle)} (tr "not-found.login.free")] - [:& login-methods {:on-recovery-request set-section-recovery + [:> login-dialog* {:on-recovery-request set-section-recovery :on-success-callback success-login - :params {:save-login-redirect true}}] + :handle-redirect true}] [:hr {:class (stl/css :separator)}] [:div {:class (stl/css :change-section)} (tr "auth.register") @@ -559,7 +560,7 @@ :is-dashboard dashboard? :is-viewer view? :profile profile} - [:> login-dialog* {}]] + [:> login-modal* {}]] (when (get info :loaded false) (if request-access? [:> context-wrapper* {:is-workspace workspace? diff --git a/frontend/src/app/main/ui/viewer/login.cljs b/frontend/src/app/main/ui/viewer/login.cljs index a23a8245d4..30c002a3be 100644 --- a/frontend/src/app/main/ui/viewer/login.cljs +++ b/frontend/src/app/main/ui/viewer/login.cljs @@ -10,7 +10,7 @@ [app.common.logging :as log] [app.main.data.modal :as modal] [app.main.store :as st] - [app.main.ui.auth.login :refer [login-methods]] + [app.main.ui.auth.login :refer [login-dialog*]] [app.main.ui.auth.recovery-request :refer [recovery-request-page]] [app.main.ui.auth.register :refer [register-methods register-success-page terms-register register-validate-form]] [app.main.ui.icons :as deprecated-icon] @@ -75,7 +75,9 @@ (case current-section :login [:div {:class (stl/css :form-container)} - [:& login-methods {:on-success-callback success-login :origin :viewer}] + [:> login-dialog* + {:on-success-callback success-login + :origin :viewer}] [:div {:class (stl/css :links)} [:div {:class (stl/css :recovery-request)} [:a {:on-click set-section