mirror of
https://github.com/penpot/penpot.git
synced 2026-08-01 10:56:20 +00:00
🐛 Fix PENPOT_OIDC_USER_INFO_SOURCE flag being silently ignored (#9114)
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>
This commit is contained in:
parent
4061673528
commit
9ebd17f31f
@ -50,6 +50,7 @@
|
|||||||
|
|
||||||
### :bug: Bugs fixed
|
### :bug: Bugs fixed
|
||||||
|
|
||||||
|
- Fix `PENPOT_OIDC_USER_INFO_SOURCE` flag being silently ignored (`userinfo` / `token`) in the OIDC callback, causing "incomplete user info" failures during registration [Github #9108](https://github.com/penpot/penpot/issues/9108)
|
||||||
- Fix `get-view-only-bundle` crashing when a share-link viewer encounters a team member whose email lacks `@` (NullPointerException in `obfuscate-email`) or whose domain has no `.` (previously produced a dangling-dot `****@****.`); now the viewer-side obfuscation is nil-safe and omits the trailing dot when the domain has no TLD
|
- Fix `get-view-only-bundle` crashing when a share-link viewer encounters a team member whose email lacks `@` (NullPointerException in `obfuscate-email`) or whose domain has no `.` (previously produced a dangling-dot `****@****.`); now the viewer-side obfuscation is nil-safe and omits the trailing dot when the domain has no TLD
|
||||||
- Remove `corepack` from the MCP local launcher so it runs on Node.js 25+, where corepack is no longer bundled [Github #8877](https://github.com/penpot/penpot/issues/8877)
|
- Remove `corepack` from the MCP local launcher so it runs on Node.js 25+, where corepack is no longer bundled [Github #8877](https://github.com/penpot/penpot/issues/8877)
|
||||||
- Fix Copy as SVG: emit a single valid SVG document when multiple shapes are selected, and publish `image/svg+xml` to the clipboard so the paste target works in Inkscape and other SVG-native tools [Github #838](https://github.com/penpot/penpot/issues/838)
|
- Fix Copy as SVG: emit a single valid SVG document when multiple shapes are selected, and publish `image/svg+xml` to the clipboard so the paste target works in Inkscape and other SVG-native tools [Github #838](https://github.com/penpot/penpot/issues/838)
|
||||||
|
|||||||
@ -548,16 +548,29 @@
|
|||||||
(def ^:private valid-info?
|
(def ^:private valid-info?
|
||||||
(sm/validator schema:info))
|
(sm/validator schema:info))
|
||||||
|
|
||||||
|
(defn- select-user-info-source
|
||||||
|
"Normalise the provider's configured user-info source into a keyword the
|
||||||
|
dispatch below can match. The raw value comes from config as a string
|
||||||
|
per the malli schema in `app.config` (`\"token\"`, `\"userinfo\"`, or
|
||||||
|
`\"auto\"`) and from hard-coded per-provider maps as strings as well;
|
||||||
|
any unrecognised or missing value falls back to `:auto` (prefer claims,
|
||||||
|
use userinfo as fallback)."
|
||||||
|
[source]
|
||||||
|
(case source
|
||||||
|
"token" :token
|
||||||
|
"userinfo" :userinfo
|
||||||
|
:auto))
|
||||||
|
|
||||||
(defn- get-info
|
(defn- get-info
|
||||||
[cfg provider state code]
|
[cfg provider state code]
|
||||||
(let [tdata (fetch-access-token cfg provider code)
|
(let [tdata (fetch-access-token cfg provider code)
|
||||||
claims (get-id-token-claims provider tdata)
|
claims (get-id-token-claims provider tdata)
|
||||||
|
|
||||||
info (case (get provider :user-info-source)
|
info (case (select-user-info-source (get provider :user-info-source))
|
||||||
:token (dissoc claims :exp :iss :iat :aud :sid)
|
:token (dissoc claims :exp :iss :iat :aud :sid)
|
||||||
:userinfo (fetch-user-info cfg provider tdata)
|
:userinfo (fetch-user-info cfg provider tdata)
|
||||||
(or (some-> claims (dissoc :exp :iss :iat :aud :sid))
|
:auto (or (some-> claims (dissoc :exp :iss :iat :aud :sid))
|
||||||
(fetch-user-info cfg provider tdata)))
|
(fetch-user-info cfg provider tdata)))
|
||||||
|
|
||||||
info (process-user-info provider tdata info)]
|
info (process-user-info provider tdata info)]
|
||||||
|
|
||||||
|
|||||||
@ -33,3 +33,23 @@
|
|||||||
(t/is (= "nextcloud@example.com" (:email info)))
|
(t/is (= "nextcloud@example.com" (:email info)))
|
||||||
(t/is (= "Nextcloud User" (:fullname info)))
|
(t/is (= "Nextcloud User" (:fullname info)))
|
||||||
(t/is (true? (:email-verified 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)))))
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user