From 64582c38555eb26cb7364a98235910c15ab9fb98 Mon Sep 17 00:00:00 2001 From: Robert Roland <rob@metabase.com> Date: Fri, 11 Sep 2020 12:27:52 -0700 Subject: [PATCH] Add an error msg for disabled accounts via Google Auth (#13155) Previously, if your Metabase account for a Google login was disabled, you would be returned to the login box with no indication as to what was wrong. Now we return to the page with a red error that says "Your account is disabled." Resolves #3245 --- .../metabase/auth/components/GoogleButton.jsx | 29 +++++++++++++++++-- src/metabase/api/session.clj | 16 ++++++++-- test/metabase/api/session_test.clj | 29 ++++++++++++++++++- 3 files changed, 67 insertions(+), 7 deletions(-) diff --git a/frontend/src/metabase/auth/components/GoogleButton.jsx b/frontend/src/metabase/auth/components/GoogleButton.jsx index 091a76d73a4..4acff8e30db 100644 --- a/frontend/src/metabase/auth/components/GoogleButton.jsx +++ b/frontend/src/metabase/auth/components/GoogleButton.jsx @@ -40,15 +40,38 @@ export default class GoogleButton extends Component { auth2.attachClickHandler( element, {}, - googleUser => { + async googleUser => { this.setState({ errorMessage: null }); - loginGoogle(googleUser, location.query.redirect); + const result = await loginGoogle( + googleUser, + location.query.redirect, + ); + + if ( + result.payload["status"] && + result.payload["status"] === 400 && + result.payload.data && + result.payload.data.errors + ) { + let errorMessage = ""; + for (const [, value] of Object.entries( + result.payload.data.errors, + )) { + if (errorMessage !== "") { + errorMessage = errorMessage + "<br/>"; + } + errorMessage = errorMessage + value; + } + this.setState({ + errorMessage: errorMessage, + }); + } }, error => { this.setState({ errorMessage: GOOGLE_AUTH_ERRORS[error.error] || - t`There was an issue signing in with Google. Pleast contact an administrator.`, + t`There was an issue signing in with Google. Please contact an administrator.`, }); }, ); diff --git a/src/metabase/api/session.clj b/src/metabase/api/session.clj index 24bcbdbdf67..a24e2c9e540 100644 --- a/src/metabase/api/session.clj +++ b/src/metabase/api/session.clj @@ -66,6 +66,9 @@ (def ^:private password-fail-message (deferred-tru "Password did not match stored password.")) (def ^:private password-fail-snippet (deferred-tru "did not match stored password")) +(def ^:private disabled-account-message (deferred-tru "Your account is disabled. Please contact your administrator.")) +(def ^:private disabled-account-snippet (deferred-tru "Your account is disabled.")) + (s/defn ^:private ldap-login :- (s/maybe UUID) "If LDAP is enabled and a matching user exists return a new Session for them, or `nil` if they couldn't be authenticated." @@ -314,13 +317,20 @@ :email email}))] (create-session! :sso user))) -(defn- do-google-auth [{{:keys [token]} :body :as request}] +(defn do-google-auth + "Call to Google to perform an authentication" + [{{:keys [token]} :body :as request}] (let [token-info-response (http/post (format google-auth-token-info-url token)) {:keys [given_name family_name email]} (google-auth-token-info token-info-response)] (log/info (trs "Successfully authenticated Google Auth token for: {0} {1}" given_name family_name)) (let [session-id (api/check-500 (google-auth-fetch-or-create-user! given_name family_name email)) - response {:id session-id}] - (mw.session/set-session-cookie request response session-id)))) + response {:id session-id} + user (db/select-one [User :id :is_active], :email email)] + (if (and user (:is_active user)) + (mw.session/set-session-cookie request response session-id) + (throw (ex-info (str disabled-account-message) + {:status-code 400 + :errors {:account disabled-account-snippet}})))))) (api/defendpoint POST "/google_auth" "Login with Google Auth." diff --git a/test/metabase/api/session_test.clj b/test/metabase/api/session_test.clj index f154207bd21..4ea956c38c0 100644 --- a/test/metabase/api/session_test.clj +++ b/test/metabase/api/session_test.clj @@ -20,7 +20,8 @@ [metabase.test.integrations.ldap :as ldap.test] [schema.core :as s] [toucan.db :as db]) - (:import java.util.UUID)) + (:import clojure.lang.ExceptionInfo + java.util.UUID)) ;; one of the tests below compares the way properties for the H2 driver are translated, so we need to make sure it's ;; loaded @@ -406,6 +407,32 @@ token-2)} token-1))))))) +(deftest google-auth-tests + (mt/with-temporary-setting-values [google-auth-client-id "PRETEND-GOOD-GOOGLE-CLIENT-ID"] + (testing "with an active account" + (mt/with-temp User [user {:email "test@metabase.com" :is_active true}] + (with-redefs [http/post (fn [url] {:status 200 + :body (str "{\"aud\":\"PRETEND-GOOD-GOOGLE-CLIENT-ID\"," + "\"email_verified\":\"true\"," + "\"first_name\":\"test\"," + "\"last_name\":\"user\"," + "\"email\":\"test@metabase.com\"}")})] + + (let [result (session-api/do-google-auth {:body {:token "foo"}})] + (is (= 200 (:status result))))))) + (testing "with a disabled account" + (mt/with-temp User [user {:email "test@metabase.com" :is_active false}] + (with-redefs [http/post (fn [url] {:status 200 + :body (str "{\"aud\":\"PRETEND-GOOD-GOOGLE-CLIENT-ID\"," + "\"email_verified\":\"true\"," + "\"first_name\":\"test\"," + "\"last_name\":\"user\"," + "\"email\":\"test@metabase.com\"}")})] + (is (thrown-with-msg? + ExceptionInfo + #"Your account is disabled. Please contact your administrator." + (session-api/do-google-auth {:body {:token "foo"}})))))))) + ;;; --------------------------------------- google-auth-fetch-or-create-user! ---------------------------------------- (deftest google-auth-fetch-or-create-user!-test -- GitLab