From 2828ccda7f96ae87419f8f9a273fc5e3487ed0cf Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Fri, 16 Apr 2021 13:07:41 +0200 Subject: [PATCH] :sparkles: Add the ability to check roles to openid integration. --- backend/src/app/config.clj | 6 +++ backend/src/app/http/oauth.clj | 52 ++++++++++++++++------- backend/src/app/rpc/mutations/profile.clj | 29 +++++++++++-- common/app/common/spec.cljc | 14 ++++++ frontend/src/app/util/i18n.cljs | 19 ++++++--- 5 files changed, 93 insertions(+), 27 deletions(-) diff --git a/backend/src/app/config.clj b/backend/src/app/config.clj index 3518d601e..260e00c5f 100644 --- a/backend/src/app/config.clj +++ b/backend/src/app/config.clj @@ -111,6 +111,9 @@ (s/def ::oidc-token-uri ::us/string) (s/def ::oidc-auth-uri ::us/string) (s/def ::oidc-user-uri ::us/string) +(s/def ::oidc-scopes ::us/set-of-str) +(s/def ::oidc-roles ::us/set-of-str) +(s/def ::oidc-roles-attr ::us/keyword) (s/def ::host ::us/string) (s/def ::http-server-port ::us/integer) (s/def ::http-session-cookie-name ::us/string) @@ -190,6 +193,9 @@ ::oidc-token-uri ::oidc-auth-uri ::oidc-user-uri + ::oidc-scopes + ::oidc-roles-attr + ::oidc-roles ::host ::http-server-port ::http-session-idle-max-age diff --git a/backend/src/app/http/oauth.clj b/backend/src/app/http/oauth.clj index 20866091b..fd1b3241e 100644 --- a/backend/src/app/http/oauth.clj +++ b/backend/src/app/http/oauth.clj @@ -13,7 +13,9 @@ [app.util.logging :as l] [app.util.time :as dt] [clojure.data.json :as json] + [clojure.set :as set] [clojure.spec.alpha :as s] + [cuerdas.core :as str] [integrant.core :as ig] [lambdaisland.uri :as u])) @@ -32,9 +34,7 @@ (defn register-profile [{:keys [rpc] :as cfg} info] (let [method-fn (get-in rpc [:methods :mutation :login-or-register]) - profile (method-fn {:email (:email info) - :backend (:backend info) - :fullname (:fullname info)})] + profile (method-fn info)] (cond-> profile (some? (:invitation-token info)) (assoc :invitation-token (:invitation-token info))))) @@ -60,7 +60,7 @@ :redirect_uri (build-redirect-uri cfg) :response_type "code" :state state - :scope (:scope provider)} + :scope (str/join " " (:scopes provider []))} query (u/map->query-string params)] (-> (u/uri (:auth-uri provider)) (assoc :query query) @@ -98,10 +98,10 @@ res (http/send! req)] (when (= 200 (:status res)) - (let [data (json/read-str (:body res))] - {:email (get data "email") - :backend (:name provider) - :fullname (get data "name")}))) + (let [{:keys [name] :as data} (json/read-str (:body res) :key-fn keyword)] + (-> data + (assoc :backend (:name provider)) + (assoc :fullname name))))) (catch Exception e (l/error :hint "unexpected exception on retrieve-user-info" @@ -109,7 +109,7 @@ nil))) (defn retrieve-info - [{:keys [tokens] :as cfg} request] + [{:keys [tokens provider] :as cfg} request] (let [state (get-in request [:params :state]) state (tokens :verify {:token state :iss :oauth}) info (some->> (get-in request [:params :code]) @@ -119,6 +119,23 @@ (ex/raise :type :internal :code :unable-to-auth)) + ;; If the provider is OIDC, we can proceed to check + ;; roles if they are defined. + (when (and (= "oidc" (:name provider)) + (seq (:roles provider))) + (let [provider-roles (into #{} (:roles provider)) + profile-roles (let [attr (cf/get :oidc-roles-attr :roles) + roles (get info attr)] + (cond + (string? roles) (into #{} (str/words roles)) + (vector? roles) (into #{} roles) + :else #{}))] + ;; check if profile has a configured set of roles + (when-not (set/subset? provider-roles profile-roles) + (ex/raise :type :internal + :code :unable-to-auth + :hint "not enought permissions")))) + (cond-> info (some? (:invitation-token state)) (assoc :invitation-token (:invitation-token state))))) @@ -198,7 +215,10 @@ :token-uri (cf/get :oidc-token-uri) :auth-uri (cf/get :oidc-auth-uri) :user-uri (cf/get :oidc-user-uri) - :scope "openid profile email name" + :scopes (into #{"openid" "profile" "email" "name"} + (cf/get :oidc-scopes #{})) + :roles-attr (cf/get :oidc-roles-attr) + :roles (cf/get :oidc-roles) :name "oidc"}] (if (and (string? (:base-uri opts)) (string? (:client-id opts)) @@ -218,10 +238,9 @@ [cfg] (let [opts {:client-id (cf/get :google-client-id) :client-secret (cf/get :google-client-secret) - :scope (str "email profile " - "https://www.googleapis.com/auth/userinfo.email " - "https://www.googleapis.com/auth/userinfo.profile " - "openid") + :scopes #{"email" "profile" "openid" + "https://www.googleapis.com/auth/userinfo.email" + "https://www.googleapis.com/auth/userinfo.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" @@ -237,7 +256,8 @@ [cfg] (let [opts {:client-id (cf/get :github-client-id) :client-secret (cf/get :github-client-secret) - :scope "user:email" + :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" @@ -256,7 +276,7 @@ opts {:base-uri base :client-id (cf/get :gitlab-client-id) :client-secret (cf/get :gitlab-client-secret) - :scope "read_user" + :scopes #{"read_user"} :auth-uri (str base "/oauth/authorize") :token-uri (str base "/oauth/token") :user-uri (str base "/api/v4/user") diff --git a/backend/src/app/rpc/mutations/profile.clj b/backend/src/app/rpc/mutations/profile.clj index e056ef840..8095b3133 100644 --- a/backend/src/app/rpc/mutations/profile.clj +++ b/backend/src/app/rpc/mutations/profile.clj @@ -6,6 +6,7 @@ (ns app.rpc.mutations.profile (:require + [app.common.data :as d] [app.common.exceptions :as ex] [app.common.spec :as us] [app.common.uuid :as uuid] @@ -303,16 +304,35 @@ (defn login-or-register [{:keys [conn] :as cfg} {:keys [email backend] :as params}] - (letfn [(create-profile [conn {:keys [fullname email]}] + (letfn [(info->props [info] + (dissoc info :name :fullname :email :backend)) + + (info->lang [{:keys [locale] :as info}] + (when (and (string? locale) + (not (str/blank? locale))) + locale)) + + (create-profile [conn {:keys [email] :as info}] (db/insert! conn :profile {:id (uuid/next) - :fullname fullname + :fullname (:fullname info) :email (str/lower email) + :lang (info->lang info) :auth-backend backend :is-active true :password "!" + :props (db/tjson (info->props info)) :is-demo false})) + (update-profile [conn info profile] + (let [props (d/merge (:props profile) + (info->props info))] + (db/update! conn :profile + {:props (db/tjson props) + :modified-at (dt/now)} + {:id (:id profile)}) + (assoc profile :props props))) + (register-profile [conn params] (let [profile (->> (create-profile conn params) (create-profile-relations conn))] @@ -321,7 +341,9 @@ (let [profile (profile/retrieve-profile-data-by-email conn email) profile (if profile - (profile/populate-additional-data conn profile) + (->> profile + (update-profile conn params) + (profile/populate-additional-data conn)) (register-profile conn params))] (profile/strip-private-attrs profile)))) @@ -346,7 +368,6 @@ (update-profile conn params) nil)) - ;; --- Mutation: Update Password (declare validate-password!) diff --git a/common/app/common/spec.cljc b/common/app/common/spec.cljc index 4e968b6c1..eabce91ff 100644 --- a/common/app/common/spec.cljc +++ b/common/app/common/spec.cljc @@ -133,6 +133,20 @@ ::s/invalid))] (s/def ::email (s/conformer cfn str))) + +;; --- SPEC: set-of-str +(letfn [(conformer [s] + (cond + (string? s) (into #{} (str/split s #"\s*,\s*")) + (set? s) (if (every? string? s) + s + ::s/invalid) + :else ::s/invalid)) + + (unformer [s] + (str/join "," s))] + (s/def ::set-of-str (s/conformer conformer unformer))) + ;; --- Macros (defn spec-assert* diff --git a/frontend/src/app/util/i18n.cljs b/frontend/src/app/util/i18n.cljs index 9c3e06939..ac66402b0 100644 --- a/frontend/src/app/util/i18n.cljs +++ b/frontend/src/app/util/i18n.cljs @@ -29,8 +29,7 @@ (defn- parse-locale [locale] - (let [locale (-> (.-language globals/navigator) - (str/lower) + (let [locale (-> (str/lower locale) (str/replace "-" "_"))] (cond-> [locale] (str/includes? locale "_") @@ -66,11 +65,17 @@ (set! translations data)) (defn set-locale! - [lang] - (if lang - (do - (swap! storage assoc ::locale lang) - (reset! locale lang)) + [lname] + (if lname + (let [supported (into #{} (map :value supported-locales)) + lname (loop [locales (seq (parse-locale lname))] + (if-let [locale (first locales)] + (if (contains? supported locale) + locale + (recur (rest locales))) + cfg/default-language))] + (swap! storage assoc ::locale lname) + (reset! locale lname)) (do (swap! storage dissoc ::locale) (reset! locale (autodetect)))))