diff --git a/backend/src/app/http/auth_request.clj b/backend/src/app/http/auth_request.clj index 867b9c3c4bb..7e3dd1e10ac 100644 --- a/backend/src/app/http/auth_request.clj +++ b/backend/src/app/http/auth_request.clj @@ -150,17 +150,18 @@ (handler request) :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) - (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 9b498c7437f..76177635553 100644 --- a/backend/test/backend_tests/http_middleware_test.clj +++ b/backend/test/backend_tests/http_middleware_test.clj @@ -379,14 +379,17 @@ (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) + ;; 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" fullname} {}))] + "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 @@ -397,6 +400,8 @@ (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))))))) (t/deftest x-auth-request-auto-register-joins-named-smb-team @@ -418,8 +423,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)))