From 7f246b7e5976f1b14dd55541ff5eb96d504dd5e5 Mon Sep 17 00:00:00 2001
From: "Mahatthana (Kelvin) Nomsawadi" <mahatthana.n@gmail.com>
Date: Thu, 23 Jun 2022 20:31:57 +0700
Subject: [PATCH] Hide password + user form fields when logged in via JWT and
SAML (#23476)
* Hide password + user form fields when logged in via JWT and SAML
* ngoc - update the maybe-add-sso_soruce
* Address review: removing deprecated Cypress functions
Co-authored-by: Ngoc Khuat <qn.khuat@gmail.com>
---
frontend/src/metabase/plugins/builtin.js | 2 +
.../src/metabase/plugins/builtin/auth/jwt.js | 3 +
.../src/metabase/plugins/builtin/auth/saml.js | 3 +
.../onboarding/setup/user_settings.cy.spec.js | 62 ++++++++++++++++---
src/metabase/api/user.clj | 10 ++-
5 files changed, 71 insertions(+), 9 deletions(-)
create mode 100644 frontend/src/metabase/plugins/builtin/auth/jwt.js
create mode 100644 frontend/src/metabase/plugins/builtin/auth/saml.js
diff --git a/frontend/src/metabase/plugins/builtin.js b/frontend/src/metabase/plugins/builtin.js
index 4024ac3b9c6..31893852ed9 100644
--- a/frontend/src/metabase/plugins/builtin.js
+++ b/frontend/src/metabase/plugins/builtin.js
@@ -1,4 +1,6 @@
import "metabase/plugins/builtin/auth/password";
import "metabase/plugins/builtin/auth/google";
import "metabase/plugins/builtin/auth/ldap";
+import "metabase/plugins/builtin/auth/jwt";
+import "metabase/plugins/builtin/auth/saml";
import "metabase/plugins/builtin/settings/hosted";
diff --git a/frontend/src/metabase/plugins/builtin/auth/jwt.js b/frontend/src/metabase/plugins/builtin/auth/jwt.js
new file mode 100644
index 00000000000..0082cf4e747
--- /dev/null
+++ b/frontend/src/metabase/plugins/builtin/auth/jwt.js
@@ -0,0 +1,3 @@
+import { PLUGIN_IS_PASSWORD_USER } from "metabase/plugins";
+
+PLUGIN_IS_PASSWORD_USER.push(user => user.sso_source !== "jwt");
diff --git a/frontend/src/metabase/plugins/builtin/auth/saml.js b/frontend/src/metabase/plugins/builtin/auth/saml.js
new file mode 100644
index 00000000000..a4c08d2fbae
--- /dev/null
+++ b/frontend/src/metabase/plugins/builtin/auth/saml.js
@@ -0,0 +1,3 @@
+import { PLUGIN_IS_PASSWORD_USER } from "metabase/plugins";
+
+PLUGIN_IS_PASSWORD_USER.push(user => user.sso_source !== "saml");
diff --git a/frontend/test/metabase/scenarios/onboarding/setup/user_settings.cy.spec.js b/frontend/test/metabase/scenarios/onboarding/setup/user_settings.cy.spec.js
index e4bc318f280..a092aed27eb 100644
--- a/frontend/test/metabase/scenarios/onboarding/setup/user_settings.cy.spec.js
+++ b/frontend/test/metabase/scenarios/onboarding/setup/user_settings.cy.spec.js
@@ -64,8 +64,7 @@ describe("user > settings", () => {
});
it("should update the user without fetching memberships", () => {
- cy.server();
- cy.route("GET", "/api/permissions/membership").as("membership");
+ cy.intercept("GET", "/api/permissions/membership").as("membership");
cy.visit("/account/profile");
cy.findByDisplayValue(first_name)
.click()
@@ -82,8 +81,7 @@ describe("user > settings", () => {
});
it("should have a change password tab", () => {
- cy.server();
- cy.route("GET", "/api/user/current").as("getUser");
+ cy.intercept("GET", "/api/user/current").as("getUser");
cy.visit("/account/profile");
cy.wait("@getUser");
@@ -171,8 +169,7 @@ describe("user > settings", () => {
describe("when user is authenticated via ldap", () => {
beforeEach(() => {
- cy.server();
- cy.route(
+ cy.intercept(
"GET",
"/api/user/current",
Object.assign({}, CURRENT_USER, {
@@ -191,8 +188,7 @@ describe("user > settings", () => {
describe("when user is authenticated via google", () => {
beforeEach(() => {
- cy.server();
- cy.route(
+ cy.intercept(
"GET",
"/api/user/current",
Object.assign({}, CURRENT_USER, {
@@ -214,4 +210,54 @@ describe("user > settings", () => {
cy.findByLabelText("Email").should("not.exist");
});
});
+
+ describe("when user is authenticated via JWT", () => {
+ beforeEach(() => {
+ cy.intercept(
+ "GET",
+ "/api/user/current",
+ Object.assign({}, CURRENT_USER, {
+ sso_source: "jwt",
+ }),
+ ).as("getUser");
+
+ cy.visit("/account/profile");
+ cy.wait("@getUser");
+ });
+
+ it("should hide change password tab", () => {
+ cy.findByText("Password").should("not.exist");
+ });
+
+ it("should hide first name, last name, and email input (metabase#23298)", () => {
+ cy.findByLabelText("First name").should("not.exist");
+ cy.findByLabelText("Last name").should("not.exist");
+ cy.findByLabelText("Email").should("not.exist");
+ });
+ });
+
+ describe("when user is authenticated via SAML", () => {
+ beforeEach(() => {
+ cy.intercept(
+ "GET",
+ "/api/user/current",
+ Object.assign({}, CURRENT_USER, {
+ sso_source: "saml",
+ }),
+ ).as("getUser");
+
+ cy.visit("/account/profile");
+ cy.wait("@getUser");
+ });
+
+ it("should hide change password tab", () => {
+ cy.findByText("Password").should("not.exist");
+ });
+
+ it("should hide first name, last name, and email input (metabase#23298)", () => {
+ cy.findByLabelText("First name").should("not.exist");
+ cy.findByLabelText("Last name").should("not.exist");
+ cy.findByLabelText("Email").should("not.exist");
+ });
+ });
});
diff --git a/src/metabase/api/user.clj b/src/metabase/api/user.clj
index 5209411675c..7c9a73fa8e9 100644
--- a/src/metabase/api/user.clj
+++ b/src/metabase/api/user.clj
@@ -184,6 +184,13 @@
(with-advanced-permissions user)
user))
+(defn- maybe-add-sso-source
+ "Adds `sso_source` key to the `User`, so FE could determine if the user is logged in via SSO."
+ [{:keys [id] :as user}]
+ (if (premium-features/enable-sso?)
+ (assoc user :sso_source (db/select-one-field :sso_source User :id id))
+ user))
+
(defn- add-has-question-and-dashboard
"True when the user has permissions for at least one un-archived question and one un-archived dashboard."
[user]
@@ -212,7 +219,8 @@
(hydrate :personal_collection_id :group_ids :is_installer :has_invited_second_user)
add-has-question-and-dashboard
add-first-login
- maybe-add-advanced-permissions))
+ maybe-add-advanced-permissions
+ maybe-add-sso-source))
(api/defendpoint GET "/:id"
"Fetch a `User`. You must be fetching yourself *or* be a superuser *or* a Group Manager."
--
GitLab