From c31eb2df4288d5971a5337d1ecc62cdc0e994327 Mon Sep 17 00:00:00 2001
From: Andrey Antukh <niwi@niwi.nz>
Date: Mon, 24 Apr 2023 15:13:24 +0200
Subject: [PATCH] :bug: Fix OICD auth provider roles checking mechanism

---
 CHANGES.md                    |  3 ++-
 backend/src/app/auth/oidc.clj | 17 +++++++++++------
 backend/src/app/config.clj    |  2 +-
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/CHANGES.md b/CHANGES.md
index 78fc19eaf6..ea4ee017e7 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -1,12 +1,13 @@
 # CHANGELOG
 
-## 1.18.3
+## 1.18.3 (Unreleased)
 
 ### :bug: Bugs fixed
 
 - Fix problem with rulers not placing correctly [Taiga #5093](https://tree.taiga.io/project/penpot/issue/5093)
 - Fix page context menu [Taiga #5145](https://tree.taiga.io/project/penpot/issue/5145)
 - Fix project file count [Taiga #5148](https://tree.taiga.io/project/penpot/issue/5148)
+- Fix OIDC roles checking mechanism [GH #3152](https://github.com/penpot/penpot/issues/3152)
 
 ### :arrow_up: Deps updates
 
diff --git a/backend/src/app/auth/oidc.clj b/backend/src/app/auth/oidc.clj
index fa857d5d43..fca5d10586 100644
--- a/backend/src/app/auth/oidc.clj
+++ b/backend/src/app/auth/oidc.clj
@@ -196,7 +196,7 @@
 
               ;; Additional hooks for provider specific way of
               ;; retrieve emails.
-              :get-email-fn           (partial retrieve-github-email cfg)}]
+              :get-email-fn  (partial retrieve-github-email cfg)}]
 
     (when (contains? cf/flags :login-with-github)
       (if (and (string? (:client-id opts))
@@ -377,21 +377,26 @@
 
 (defn get-info
   [{:keys [provider] :as cfg} {:keys [params] :as request}]
-  (letfn [(validate-oidc [info]
+  (letfn [(parse-oidc-attrs-path [path]
+            (let [[fitem & items] (str/split path "__")]
+              (into [(keyword "oidc" fitem)] (map keyword) items)))
+
+          (validate-oidc [info]
             ;; 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)]
+              (let [expected-roles (into #{} (:roles provider))
+                    current-roles  (let [roles (->> (cf/get :oidc-roles-attr "roles")
+                                                    (parse-oidc-attrs-path)
+                                                    (get-in info))]
                                      (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)
+                (when-not (set/subset? expected-roles current-roles)
                   (ex/raise :type :internal
                             :code :unable-to-auth
                             :hint "not enough permissions"))))
diff --git a/backend/src/app/config.clj b/backend/src/app/config.clj
index b66d607967..8314ae1e52 100644
--- a/backend/src/app/config.clj
+++ b/backend/src/app/config.clj
@@ -153,7 +153,7 @@
 (s/def ::oidc-user-uri ::us/string)
 (s/def ::oidc-scopes ::us/set-of-strings)
 (s/def ::oidc-roles ::us/set-of-strings)
-(s/def ::oidc-roles-attr ::us/keyword)
+(s/def ::oidc-roles-attr ::us/string)
 (s/def ::oidc-email-attr ::us/keyword)
 (s/def ::oidc-name-attr ::us/keyword)
 (s/def ::host ::us/string)