From b5cd9855b9cd232e765213f30344bfd419fbcd93 Mon Sep 17 00:00:00 2001 From: _ <50262751+hunzlahmalik@users.noreply.github.com> Date: Mon, 18 May 2026 14:06:10 +0500 Subject: [PATCH 1/2] fix(auth): derive SSO display name from email local-part MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit oauth2-proxy was putting the Cognito sub UUID into x-auth-request-user instead of a human-readable username, so newly-provisioned SSO users landed with a UUID as their profile fullname. Drop the header read in the ForwardAuth middleware and use the email local-part instead — the same value both apps already fell back to when the header was absent; we're promoting that fallback to the only source. Existing profiles with UUID fullnames are not auto-corrected: the middleware does not re-sync :fullname on subsequent logins. Co-Authored-By: Claude Opus 4.7 --- backend/src/app/http/auth_request.clj | 4 +--- backend/test/backend_tests/http_middleware_test.clj | 8 +++----- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/backend/src/app/http/auth_request.clj b/backend/src/app/http/auth_request.clj index 867b9c3c4bb..48398909a86 100644 --- a/backend/src/app/http/auth_request.clj +++ b/backend/src/app/http/auth_request.clj @@ -152,10 +152,8 @@ :else (let [local-part (first (str/split email-claim #"@")) email (resolve-email email-claim) - fullname (or (not-empty (yreq/get-header request "x-auth-request-user")) - local-part) profile (try - (get-or-register-profile cfg email fullname) + (get-or-register-profile cfg email local-part) (catch Throwable cause (l/err :hint "x-auth-request: error resolving profile" :email email diff --git a/backend/test/backend_tests/http_middleware_test.clj b/backend/test/backend_tests/http_middleware_test.clj index 133fbe37ad5..86d7b53b77c 100644 --- a/backend/test/backend_tests/http_middleware_test.clj +++ b/backend/test/backend_tests/http_middleware_test.clj @@ -379,14 +379,12 @@ (t/deftest x-auth-request-auto-register-creates-active-profile (binding [cf/flags (conj cf/flags :x-auth-request-auto-register)] (let [email "newuser@example.com" - fullname "New User" captured (volatile! nil) cfg (make-xauth-cfg) handler (#'app.http.auth-request/wrap-authz (fn [req] (vreset! captured req) {::yres/status 200}) cfg) - response (handler (->DummyRequest {"x-auth-request-email" email - "x-auth-request-user" fullname} {}))] + response (handler (->DummyRequest {"x-auth-request-email" email} {}))] ;; Profile must be injected into the downstream request (t/is (uuid? (::session/profile-id @captured))) ;; A session cookie must be set so the browser is authenticated @@ -397,6 +395,7 @@ (profile/get-profile-by-email conn email)))] (t/is (some? profile)) (t/is (true? (:is-active profile))) + (t/is (= "newuser" (:fullname profile))) (t/is (= (::session/profile-id @captured) (:id profile))))))) (t/deftest x-auth-request-auto-register-joins-named-smb-team @@ -418,8 +417,7 @@ handler (#'app.http.auth-request/wrap-authz (fn [req] (vreset! captured req) {::yres/status 200}) cfg) - _ (handler (->DummyRequest {"x-auth-request-email" email - "x-auth-request-user" "Shared Team Join"} {})) + _ (handler (->DummyRequest {"x-auth-request-email" email} {})) profile (db/tx-run! cfg (fn [{:keys [::db/conn]}] (profile/get-profile-by-email conn email))) From 3a7adafc50ebe3894e03b6b1f82c3c161148ac96 Mon Sep 17 00:00:00 2001 From: _ <50262751+hunzlahmalik@users.noreply.github.com> Date: Wed, 3 Jun 2026 14:57:08 +0500 Subject: [PATCH 2/2] test(auth): guard x-auth-request-user UUID is ignored; single-source name Address review on #23: - Middleware: pass nil instead of a pre-split local-part into get-or-register-profile, so that function is the only place deriving the display name (from the resolved email). Removes the duplicate split and the drift risk between the two derivation paths. - Test: re-add x-auth-request-user to the auto-register request with a Cognito-sub-style UUID and keep asserting :fullname is the email local-part. This is the actual regression guard for the reported bug; the prior test dropped the header entirely, so it no longer proved the middleware ignores it. Co-Authored-By: Claude Opus 4.8 --- backend/src/app/http/auth_request.clj | 21 +++++++++++-------- .../backend_tests/http_middleware_test.clj | 8 ++++++- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/backend/src/app/http/auth_request.clj b/backend/src/app/http/auth_request.clj index 48398909a86..7e3dd1e10ac 100644 --- a/backend/src/app/http/auth_request.clj +++ b/backend/src/app/http/auth_request.clj @@ -150,15 +150,18 @@ (handler request) :else - (let [local-part (first (str/split email-claim #"@")) - email (resolve-email email-claim) - profile (try - (get-or-register-profile cfg email local-part) - (catch Throwable cause - (l/err :hint "x-auth-request: error resolving profile" - :email email - :cause cause) - nil))] + ;; Pass nil for the display name: get-or-register-profile is the single + ;; place that derives it from the resolved email local-part. We never + ;; trust an upstream-supplied name (oauth2-proxy put the Cognito sub + ;; UUID in x-auth-request-user), so there is nothing to forward here. + (let [email (resolve-email email-claim) + profile (try + (get-or-register-profile cfg email nil) + (catch Throwable cause + (l/err :hint "x-auth-request: error resolving profile" + :email email + :cause cause) + nil))] (cond (nil? profile) ;; Header email doesn't resolve to a profile (and auto-register diff --git a/backend/test/backend_tests/http_middleware_test.clj b/backend/test/backend_tests/http_middleware_test.clj index d175801c810..76177635553 100644 --- a/backend/test/backend_tests/http_middleware_test.clj +++ b/backend/test/backend_tests/http_middleware_test.clj @@ -384,7 +384,12 @@ handler (#'app.http.auth-request/wrap-authz (fn [req] (vreset! captured req) {::yres/status 200}) cfg) - response (handler (->DummyRequest {"x-auth-request-email" email} {}))] + ;; Regression guard: oauth2-proxy set x-auth-request-user to the + ;; Cognito sub UUID. The middleware must IGNORE that header and derive + ;; :fullname from the email local-part — sending it here proves it is + ;; not trusted for the display name. + response (handler (->DummyRequest {"x-auth-request-email" email + "x-auth-request-user" "892ae5ac-0021-7000-8000-000000000000"} {}))] ;; Profile must be injected into the downstream request (t/is (uuid? (::session/profile-id @captured))) ;; A session cookie must be set so the browser is authenticated @@ -395,6 +400,7 @@ (profile/get-profile-by-email conn email)))] (t/is (some? profile)) (t/is (true? (:is-active profile))) + ;; Derived from the email local-part, NOT the x-auth-request-user UUID. (t/is (= "newuser" (:fullname profile))) (t/is (= (::session/profile-id @captured) (:id profile)))))))