🐛 Add proper validation of registration domain whitelist on oidc

Fixes #3348

CHANGES.md

@@ -7,7 +7,7 @@
 ### :sparkles: New features
 
 - Default naming of text layers [Taiga #2836](https://tree.taiga.io/project/penpot/us/2836)
-- Create typography style from a selected text layer[Taiga #3041](https://tree.taiga.io/project/penpot/us/3041)
+- Create typography style from a selected text layer [Taiga #3041](https://tree.taiga.io/project/penpot/us/3041)
 - Board as ruler origin [Taiga #4833](https://tree.taiga.io/project/penpot/us/4833)
 - Access tokens support [Taiga #4460](https://tree.taiga.io/project/penpot/us/4460)
 - Show interactions setting at the view mode [Taiga #1330](https://tree.taiga.io/project/penpot/issue/1330)
@@ -15,6 +15,7 @@
   rendered as bitmap images.
 - Add the ability to disable google fonts provider with the `disable-google-fonts-provider` flag
 - Add the ability to disable dashboard templates section with the `disable-dashboard-templates-section` flag
+- Add the ability to use the registration whitelist with OICD [Github #3348](https://github.com/penpot/penpot/issues/3348)
 
 ### :bug: Bugs fixed
 

backend/src/app/auth.clj

@@ -6,7 +6,9 @@
 
 (ns app.auth
   (:require
+   [app.config :as cf]
    [buddy.hashers :as hashers]
+   [cuerdas.core :as str]
    [promesa.exec :as px]))
 
 (def default-params
@@ -27,3 +29,16 @@
       {:update false
        :valid false})))
 
+(defn email-domain-in-whitelist?
+  "Returns true if email's domain is in the given whitelist or if
+  given whitelist is an empty string."
+  ([email]
+   (let [domains (cf/get :registration-domain-whitelist)]
+     (email-domain-in-whitelist? domains email)))
+  ([domains email]
+   (if (or (nil? domains) (empty? domains))
+     true
+     (let [[_ candidate] (-> (str/lower email)
+                             (str/split #"@" 2))]
+       (contains? domains candidate)))))
+

backend/src/app/auth/oidc.clj

@@ -7,6 +7,7 @@
 (ns app.auth.oidc
   "OIDC client implementation."
   (:require
+   [app.auth :as auth]
    [app.auth.oidc.providers :as-alias providers]
    [app.common.data :as d]
    [app.common.data.macros :as dm]
@@ -430,10 +431,24 @@
    ::yrs/headers {"location" (str uri)}})
 
 (defn- generate-error-redirect
-  [_ error]
+  [_ cause]
-  (let [uri (-> (u/uri (cf/get :public-uri))
+  (let [data   (if (ex/error? cause) (ex-data cause) nil)
+        code   (or (:code data) :unexpected)
+        type   (or (:type data) :internal)
+        hint   (or (:hint data)
+                   (if (ex/exception? cause)
+                     (ex-message cause)
+                     (str cause)))
+
+        params {:error "unable-to-auth"
+                :hint hint
+                :type type
+                :code code}
+
+        uri    (-> (u/uri (cf/get :public-uri))
                    (assoc :path "/#/auth/login")
-                (assoc :query (u/map->query-string {:error "unable-to-auth" :hint (ex-message error)})))]
+                   (assoc :query (u/map->query-string params)))]
+
     (redirect-response uri)))
 
 (defn- generate-redirect
@@ -463,6 +478,8 @@
       (->> (redirect-response uri)
            (sxf request)))
 
+
+    (if (auth/email-domain-in-whitelist? (:email info))
       (let [info   (assoc info
                           :iss :prepared-register
                           :is-active true
@@ -475,7 +492,9 @@
                        (assoc :path "/#/auth/register/validate")
                        (assoc :query (u/map->query-string params)))]
 
-      (redirect-response uri))))
+        (redirect-response uri))
+      (generate-error-redirect cfg "email-domain-not-allowed"))))
+
 
 (defn- auth-handler
   [cfg {:keys [params] :as request}]

backend/src/app/rpc/commands/auth.clj

@@ -6,6 +6,7 @@
 
 (ns app.rpc.commands.auth
   (:require
+   [app.auth :as auth]
    [app.common.data :as d]
    [app.common.exceptions :as ex]
    [app.common.logging :as l]
@@ -38,19 +39,6 @@
 (s/def ::invitation-token ::us/not-empty-string)
 (s/def ::token ::us/not-empty-string)
 
-;; ---- HELPERS
-
-(defn email-domain-in-whitelist?
-  "Returns true if email's domain is in the given whitelist or if
-  given whitelist is an empty string."
-  [domains email]
-  (if (or (empty? domains)
-          (nil? domains))
-    true
-    (let [[_ candidate] (-> (str/lower email)
-                            (str/split #"@" 2))]
-      (contains? domains candidate))))
-
 ;; ---- COMMAND: login with password
 
 (defn login-with-password
@@ -180,10 +168,9 @@
                   :code :email-does-not-match-invitation
                   :hint "email should match the invitation"))))
 
-  (when-let [domains (cf/get :registration-domain-whitelist)]
+  (when-not (auth/email-domain-in-whitelist? (:email params))
-    (when-not (email-domain-in-whitelist? domains (:email params))
     (ex/raise :type :validation
-                :code :email-domain-is-not-allowed)))
+              :code :email-domain-is-not-allowed))
 
   ;; Don't allow proceed in preparing registration if the profile is
   ;; already reported as spammer.

backend/test/backend_tests/rpc_profile_test.clj

@@ -10,7 +10,7 @@
    [app.config :as cf]
    [app.db :as db]
    [app.rpc :as-alias rpc]
-   [app.rpc.commands.auth :as cauth]
+   [app.auth :as auth]
    [app.tokens :as tokens]
    [app.util.time :as dt]
    [backend-tests.helpers :as th]
@@ -226,11 +226,11 @@
 (t/deftest registration-domain-whitelist
   (let [whitelist #{"gmail.com" "hey.com" "ya.ru"}]
     (t/testing "allowed email domain"
-      (t/is (true? (cauth/email-domain-in-whitelist? whitelist "username@ya.ru")))
+      (t/is (true? (auth/email-domain-in-whitelist? whitelist "username@ya.ru")))
-      (t/is (true? (cauth/email-domain-in-whitelist? #{} "username@somedomain.com"))))
+      (t/is (true? (auth/email-domain-in-whitelist? #{} "username@somedomain.com"))))
 
     (t/testing "not allowed email domain"
-      (t/is (false? (cauth/email-domain-in-whitelist? whitelist "username@somedomain.com"))))))
+      (t/is (false? (auth/email-domain-in-whitelist? whitelist "username@somedomain.com"))))))
 
 (t/deftest prepare-register-and-register-profile-1
   (let [data  {::th/type :prepare-register-profile