diff --git a/enterprise/backend/src/metabase_enterprise/sso/api/sso.clj b/enterprise/backend/src/metabase_enterprise/sso/api/sso.clj index 00ac09ef1b2f8..c8282f2c81020 100644 --- a/enterprise/backend/src/metabase_enterprise/sso/api/sso.clj +++ b/enterprise/backend/src/metabase_enterprise/sso/api/sso.clj @@ -10,10 +10,12 @@ [metabase-enterprise.sso.integrations.saml] [metabase-enterprise.sso.integrations.sso-settings :as sso-settings] [metabase.api.common :as api] + [metabase.server.middleware.session :as mw.session] [metabase.util :as u] [metabase.util.i18n :refer [trs]] [metabase.util.log :as log] [metabase.util.malli :as mu] + [metabase.util.malli.schema :as ms] [metabase.util.urls :as urls] [saml20-clj.core :as saml] [saml20-clj.encode-decode :as encode-decode] @@ -68,22 +70,27 @@ ;; POST /auth/sso/logout (api/defendpoint POST "/logout" "Logout." - [:as {:keys [metabase-session-id]}] - (api/check-exists? :model/Session metabase-session-id) - (let [{:keys [email sso_source]} - (t2/query-one {:select [:u.email :u.sso_source] - :from [[:core_user :u]] - :join [[:core_session :session] [:= :u.id :session.user_id]] - :where [:= :session.id metabase-session-id]})] - {:saml-logout-url - (when (and (sso-settings/saml-enabled) - (= sso_source "saml")) - (saml/logout-redirect-location - :idp-url (sso-settings/saml-identity-provider-uri) - :issuer (sso-settings/saml-application-name) - :user-email email - :relay-state (encode-decode/str->base64 - (str (urls/site-url) metabase-slo-redirect-url))))})) + [:as {cookies :cookies}] + {cookies [:map [mw.session/metabase-session-cookie [:map [:value ms/NonBlankString]]]]} + (let [metabase-session-id (get-in cookies [mw.session/metabase-session-cookie :value])] + (api/check-exists? :model/Session metabase-session-id) + (let [{:keys [email sso_source]} + (t2/query-one {:select [:u.email :u.sso_source] + :from [[:core_user :u]] + :join [[:core_session :session] [:= :u.id :session.user_id]] + :where [:= :session.id metabase-session-id]})] + ;; If a user doesn't have SLO setup on their IdP, + ;; they will never hit "/handle_slo" so we must delete the session here: + (t2/delete! :model/Session :id metabase-session-id) + {:saml-logout-url + (when (and (sso-settings/saml-enabled) + (= sso_source "saml")) + (saml/logout-redirect-location + :idp-url (sso-settings/saml-identity-provider-uri) + :issuer (sso-settings/saml-application-name) + :user-email email + :relay-state (encode-decode/str->base64 + (str (urls/site-url) metabase-slo-redirect-url))))}))) ;; POST /auth/sso/handle_slo (api/defendpoint POST "/handle_slo" diff --git a/enterprise/backend/test/metabase_enterprise/sso/integrations/saml_test.clj b/enterprise/backend/test/metabase_enterprise/sso/integrations/saml_test.clj index 160dcc299e395..d3007c758592e 100644 --- a/enterprise/backend/test/metabase_enterprise/sso/integrations/saml_test.clj +++ b/enterprise/backend/test/metabase_enterprise/sso/integrations/saml_test.clj @@ -686,3 +686,16 @@ "After a successful log-out, you don't have a session") (is (not (t2/exists? :model/Session :id session-id)) "After a successful log-out, the session is deleted"))))))) + +(deftest logout-should-delete-session-when-idp-slo-conf-missing-test + (testing "Missing SAML SLO config logouts should still delete the user's session." + (let [session-id (str (random-uuid))] + (mt/with-temp [:model/User user {:email "saml_test@metabase.com" :sso_source "saml"} + :model/Session _ {:user_id (:id user) :id session-id}] + (is (t2/exists? :model/Session :id session-id)) + (let [req-options (-> (saml-post-request-options + (saml-test-response) + (saml/str->base64 default-redirect-uri)) + (assoc-in [:request-options :cookies mw.session/metabase-session-cookie :value] session-id))] + (client :post "/auth/sso/logout" req-options) + (is (not (t2/exists? :model/Session :id session-id))))))))