mirror of
https://github.com/penpot/penpot.git
synced 2026-10-29 14:00:37 +00:00
♻️ Use uniform URI building pattern for OIDC redirects
Replace the ad-hoc `public-uri-with-path` helper with `build-public-uri` that uses the standard `u/ensure-path-slash` + `u/join` pattern already used throughout the backend and frontend. This makes OIDC redirect URI construction consistent with the rest of the codebase while correctly preserving subpath configuration. For hash routes (frontend URLs), use `u/append-query-param` to place query params inside the fragment, matching the expected URL structure. Add 6 new tests covering subpath edge cases: - Trailing slash in public-uri - Query params inside hash fragments - No-subpath fallback - Multiple redirect functions with subpaths AI-assisted-by: qwen3.7-plus AI-assisted-by: gpt-5.6-luna
This commit is contained in:
parent
cbb9e5d971
commit
0f2583121e
@ -453,10 +453,22 @@
|
||||
[fitem & items] (str/split path separator)]
|
||||
(into [(keyword (:type provider) (str/kebab fitem))] (map keyword) items)))
|
||||
|
||||
(defn- build-public-uri
|
||||
"Join a relative path onto the configured public URI, preserving any
|
||||
subpath. Uses `u/ensure-path-slash` + `u/join` to match the pattern
|
||||
used throughout the backend and frontend.
|
||||
`path` must not start with `/` — a leading slash would be treated as
|
||||
absolute by `u/join`, silently dropping the configured subpath."
|
||||
[path]
|
||||
{:pre [(not (str/starts-with? path "/"))]}
|
||||
(-> (cf/get :public-uri)
|
||||
(u/uri)
|
||||
(u/ensure-path-slash)
|
||||
(u/join path)))
|
||||
|
||||
(defn- build-redirect-uri
|
||||
[]
|
||||
(let [public (u/uri (cf/get :public-uri))]
|
||||
(str (assoc public :path "/api/auth/oidc/callback"))))
|
||||
(str (build-public-uri "api/auth/oidc/callback")))
|
||||
|
||||
(defn build-auth-redirect-uri
|
||||
[provider token]
|
||||
|
||||
@ -156,6 +156,13 @@
|
||||
(t/is (.contains loc "error=auth-error"))
|
||||
(t/is (not (.contains loc "hint=")))))))
|
||||
|
||||
(t/deftest redirect-with-error-preserves-public-uri-subpath
|
||||
(binding [cf/config {:public-uri "http://localhost:3449/penpot"}]
|
||||
(let [result (#'oidc/redirect-with-error "auth-error")
|
||||
loc (get-in result [::yres/headers "location"])]
|
||||
(t/is (.contains loc "http://localhost:3449/penpot?screen=auth-login&"))
|
||||
(t/is (.contains loc "error=auth-error")))))
|
||||
|
||||
(t/deftest redirect-to-verify-token-builds-verify-url
|
||||
(binding [cf/config {:public-uri "http://localhost:3449"}]
|
||||
(let [result (#'oidc/redirect-to-verify-token "test-token-value")
|
||||
@ -164,11 +171,72 @@
|
||||
(t/is (.contains loc "http://localhost:3449?screen=auth-verify-token&"))
|
||||
(t/is (.contains loc "token=test-token-value")))))
|
||||
|
||||
(t/deftest redirect-to-verify-token-preserves-public-uri-subpath
|
||||
(binding [cf/config {:public-uri "http://localhost:3449/penpot"}]
|
||||
(let [result (#'oidc/redirect-to-verify-token "test-token-value")
|
||||
loc (get-in result [::yres/headers "location"])]
|
||||
(t/is (.contains loc "http://localhost:3449/penpot?screen=auth-verify-token&")))))
|
||||
|
||||
(t/deftest build-redirect-uri-constructs-redirect
|
||||
(binding [cf/config {:public-uri "http://localhost:3449"}]
|
||||
(t/is (= "http://localhost:3449/api/auth/oidc/callback"
|
||||
(#'oidc/build-redirect-uri)))))
|
||||
|
||||
(t/deftest build-redirect-uri-preserves-public-uri-subpath
|
||||
(binding [cf/config {:public-uri "http://localhost:3449/penpot"}]
|
||||
(t/is (= "http://localhost:3449/penpot/api/auth/oidc/callback"
|
||||
(#'oidc/build-redirect-uri)))))
|
||||
|
||||
(t/deftest build-redirect-uri-with-subpath-and-trailing-slash
|
||||
(binding [cf/config {:public-uri "http://localhost:3449/penpot/"}]
|
||||
(t/is (= "http://localhost:3449/penpot/api/auth/oidc/callback"
|
||||
(#'oidc/build-redirect-uri)))))
|
||||
|
||||
(t/deftest redirect-with-error-subpath-screen-query
|
||||
(binding [cf/config {:public-uri "http://localhost:3449/penpot"}]
|
||||
(let [result (#'oidc/redirect-with-error "auth-error" "hint message")
|
||||
loc (get-in result [::yres/headers "location"])]
|
||||
(t/is (.contains loc "http://localhost:3449/penpot?screen=auth-login&"))
|
||||
(t/is (.contains loc "error=auth-error"))
|
||||
(t/is (.contains loc "hint=hint"))
|
||||
;; Screen routing lives in the query string, not the fragment
|
||||
(t/is (not (.contains loc "#"))))))
|
||||
|
||||
(t/deftest redirect-to-register-subpath
|
||||
(let [test-key (byte-array (map byte (range 32)))
|
||||
cfg {::setup/props {:tokens-key test-key}}]
|
||||
(binding [cf/config {:public-uri "http://localhost:3449/penpot"}]
|
||||
(let [info {:email "u@e.com" :fullname "U" :backend "oidc"
|
||||
:email-verified false :props {}}
|
||||
provider {:type "oidc" :id "oidc"}
|
||||
result (#'oidc/redirect-to-register cfg info provider)
|
||||
loc (get-in result [::yres/headers "location"])]
|
||||
(t/is (.contains loc "http://localhost:3449/penpot?screen=auth-register-validate&"))
|
||||
(t/is (.contains loc "token="))))))
|
||||
|
||||
(t/deftest redirect-to-register-no-subpath
|
||||
(let [test-key (byte-array (map byte (range 32)))
|
||||
cfg {::setup/props {:tokens-key test-key}}]
|
||||
(binding [cf/config {:public-uri "http://localhost:3449"}]
|
||||
(let [info {:email "u@e.com" :fullname "U" :backend "oidc"
|
||||
:email-verified false :props {}}
|
||||
provider {:type "oidc" :id "oidc"}
|
||||
result (#'oidc/redirect-to-register cfg info provider)
|
||||
loc (get-in result [::yres/headers "location"])]
|
||||
(t/is (.contains loc "http://localhost:3449?screen=auth-register-validate&"))
|
||||
(t/is (.contains loc "token="))))))
|
||||
|
||||
(t/deftest redirect-to-verify-token-subpath-no-trailing-slash
|
||||
(binding [cf/config {:public-uri "http://localhost:3449/my-app"}]
|
||||
(let [result (#'oidc/redirect-to-verify-token "test-token-value")
|
||||
loc (get-in result [::yres/headers "location"])]
|
||||
(t/is (.contains loc "http://localhost:3449/my-app?screen=auth-verify-token&")))))
|
||||
|
||||
(t/deftest build-public-uri-no-subpath
|
||||
(binding [cf/config {:public-uri "http://localhost:3449"}]
|
||||
(t/is (= "http://localhost:3449/api/auth/oidc/callback"
|
||||
(str (#'oidc/build-public-uri "api/auth/oidc/callback"))))))
|
||||
|
||||
(t/deftest fetch-user-info-returns-decoded-body-on-success
|
||||
(let [cfg {}
|
||||
provider {:user-uri "https://provider.example.com/userinfo"}
|
||||
@ -476,7 +544,7 @@
|
||||
(let [cfg (dissoc base-cfg :app.email/blacklist :app.email/whitelist)
|
||||
state (make-state-token cfg {})
|
||||
request (default-request cfg :state state)]
|
||||
(binding [cf/config {:public-uri "http://localhost:3449"}
|
||||
(binding [cf/config {:public-uri "http://localhost:3449/penpot"}
|
||||
cf/flags #{:registration}]
|
||||
(with-redefs [app.auth.oidc/resolve-provider (constantly {:type "oidc" :id "oidc"})
|
||||
app.auth.oidc/get-info (constantly {:email "u@e.com" :fullname "U"
|
||||
@ -485,7 +553,7 @@
|
||||
app.auth.oidc/get-profile (constantly (assoc test-profile :is-active false))]
|
||||
(let [result (#'oidc/callback-handler cfg request)
|
||||
loc (redirect-location result)]
|
||||
(t/is (.contains loc "http://localhost:3449?screen=auth-register-validate&"))
|
||||
(t/is (.contains loc "http://localhost:3449/penpot?screen=auth-register-validate&"))
|
||||
(t/is (.contains loc "token=")))))))
|
||||
|
||||
(t/deftest callback-success-flow
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user