mirror of
https://github.com/penpot/penpot.git
synced 2026-04-26 11:48:28 +00:00
9ebd17f31f
Closes #9108. The `case` expression in `get-info` (`backend/src/app/auth/oidc.clj`) dispatched on `:token` and `:userinfo` keywords, but the provider map's `:user-info-source` value is a string — both from config (the malli schema in `app.config` pins it to one of `"token"`, `"userinfo"`, `"auto"`) and from the hard-coded Google / GitHub provider maps (which already write `"userinfo"`). Strings never equal keywords in Clojure `case`, so every call fell through to the auto-fallback that prefers ID-token claims and only hits the UserInfo endpoint when claims are empty. The net effect: setting `PENPOT_OIDC_USER_INFO_SOURCE=userinfo` did nothing, and OIDC flows whose IdP requires the UserInfo endpoint (so claims come back empty/partial) failed with "incomplete user info". - Extract a pure helper `select-user-info-source` that maps the raw config string to a dispatch keyword (`:token`, `:userinfo`, `:auto`), falling back to `:auto` for unknown / missing / accidentally-keyword values - Rewrite `get-info`'s `case` to dispatch on the helper's output so the arms unambiguously match the normalised keyword - Add vitest-style deftests in `auth_oidc_test.clj` pinning the three valid strings, the nil / "auto" / unknown fallback, and the reverse regression (a keyword input must not slip through as if it were the matching string) - Add a CHANGES.md entry under the 2.17.0 Unreleased `🐛 Bugs fixed` section linking back to #9108 Signed-off-by: Andrey Antukh <niwi@niwi.nz> Co-authored-by: Andrey Antukh <niwi@niwi.nz>
56 lines
2.5 KiB
Clojure
56 lines
2.5 KiB
Clojure
;; This Source Code Form is subject to the terms of the Mozilla Public
|
|
;; License, v. 2.0. If a copy of the MPL was not distributed with this
|
|
;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
|
|
;;
|
|
;; Copyright (c) KALEIDOS INC
|
|
|
|
(ns backend-tests.auth-oidc-test
|
|
(:require
|
|
[app.auth.oidc :as oidc]
|
|
[clojure.test :as t]))
|
|
|
|
(def ^:private oidc-provider
|
|
{:id "oidc"
|
|
:type "oidc"})
|
|
|
|
(t/deftest parse-attr-path-supports-dot-and-double-underscore
|
|
(t/is
|
|
(= [:oidc/resource-access :penpot_roles :roles]
|
|
(#'oidc/parse-attr-path oidc-provider "resource_access__penpot_roles__roles")))
|
|
(t/is
|
|
(= [:oidc/ocs :data :email]
|
|
(#'oidc/parse-attr-path oidc-provider "ocs.data.email"))))
|
|
|
|
(t/deftest process-user-info-supports-dot-notation-nested-attrs
|
|
(let [provider (assoc oidc-provider
|
|
:email-attr "ocs.data.email"
|
|
:name-attr "ocs.data.display-name")
|
|
info (#'oidc/process-user-info provider
|
|
{}
|
|
{:email_verified true
|
|
:ocs {:data {:email "nextcloud@example.com"
|
|
:display-name "Nextcloud User"}}})]
|
|
(t/is (= "nextcloud@example.com" (:email info)))
|
|
(t/is (= "Nextcloud User" (:fullname info)))
|
|
(t/is (true? (:email-verified info)))))
|
|
|
|
;; The provider's `:user-info-source` value arrives as a string (enforced by
|
|
;; the malli schema in `app.config` and used as-is by the hard-coded Google /
|
|
;; GitHub provider maps), so the dispatch must interpret strings — not
|
|
;; keywords — to actually honour `PENPOT_OIDC_USER_INFO_SOURCE=userinfo`.
|
|
(t/deftest select-user-info-source-interprets-config-strings
|
|
(t/testing "explicit string values map to keyword dispatch tokens"
|
|
(t/is (= :token (#'oidc/select-user-info-source "token")))
|
|
(t/is (= :userinfo (#'oidc/select-user-info-source "userinfo"))))
|
|
|
|
(t/testing "missing or explicit \"auto\" falls back to auto dispatch"
|
|
(t/is (= :auto (#'oidc/select-user-info-source "auto")))
|
|
(t/is (= :auto (#'oidc/select-user-info-source nil))))
|
|
|
|
(t/testing "unknown values fall back to auto dispatch safely"
|
|
(t/is (= :auto (#'oidc/select-user-info-source "unknown")))
|
|
;; Guards against the reverse regression — a stray keyword value must
|
|
;; not silently slip through as if it were the matching string.
|
|
(t/is (= :auto (#'oidc/select-user-info-source :token)))
|
|
(t/is (= :auto (#'oidc/select-user-info-source :userinfo)))))
|