Skip to content

Commit

Permalink
Fleet UI: Handle SSO user auth when SSO is globally disabled (#24705)
Browse files Browse the repository at this point in the history
  • Loading branch information
RachelElysia authored Dec 12, 2024
1 parent 95ae7c3 commit 73f6381
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ const AddUserModal = ({
onCancel={onCancel}
onSubmit={onSubmit}
availableTeams={availableTeams || []}
submitText="Add"
isPremiumTier={isPremiumTier}
smtpConfigured={smtpConfigured}
sesConfigured={sesConfigured}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ const EditUserModal = ({
onCancel={onCancel}
onSubmit={onSubmit}
availableTeams={availableTeams}
submitText="Save"
isPremiumTier={isPremiumTier}
smtpConfigured={smtpConfigured}
sesConfigured={sesConfigured}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ describe("UserForm - component", () => {
availableTeams: [],
onCancel: noop,
onSubmit: noop,
submitText: "Add",
isModifiedByGlobalAdmin: true,
isPremiumTier: true,
smtpConfigured: true,
Expand Down Expand Up @@ -42,7 +41,7 @@ describe("UserForm - component", () => {
it("renders SSO option when canUseSso is true", () => {
render(<UserForm {...defaultProps} canUseSso />);

expect(screen.getByLabelText("Enable single sign-on")).toBeInTheDocument();
expect(screen.getByLabelText("Single sign-on")).toBeInTheDocument();
});

it("disables invite user option when SMTP and SES are not configured", () => {
Expand Down Expand Up @@ -72,20 +71,51 @@ describe("UserForm - component", () => {
// Verify that non-premium elements are still present
expect(screen.getByLabelText("Full name")).toBeInTheDocument();
expect(screen.getByLabelText("Email")).toBeInTheDocument();
expect(screen.getByLabelText("Password")).toBeInTheDocument();
expect(
screen.queryByRole("radio", { name: "Password" })
).toBeInTheDocument();
});

it("does not render password and 2FA sections when SSO is enabled", () => {
it("does not render password and 2FA sections when SSO is selected", () => {
render(<UserForm {...defaultProps} canUseSso />);

// Enable SSO
const ssoRadio = screen.getByLabelText("Enable single sign-on");
const ssoRadio = screen.getByLabelText("Single sign-on");
ssoRadio.click();

// Check that password and 2FA sections are not present
expect(screen.queryByText("Password")).not.toBeInTheDocument();
// Check that the password radio is present
const passwordRadio = screen.getByRole("radio", { name: "Password" });
expect(passwordRadio).not.toBeDisabled();

// Check that password input field and 2FA sections are not present
expect(
screen.queryByRole("input", { name: "Password" })
).not.toBeInTheDocument();
expect(
screen.queryByText("Enable two-factor authentication")
).not.toBeInTheDocument();
});

it("displays disabled SSO option when SSO is globally disabled but was previously enabled for the user", async () => {
const props = {
...defaultProps,
defaultName: "User 1",
defaultEmail: "user@example.com",
currentUserId: 1,
canUseSso: false,
isSsoEnabled: true,
isNewUser: false,
};

const { user } = renderWithSetup(<UserForm {...props} />);

// Check that the SSO radio is disabled
const ssoRadio = screen.getByLabelText("Single sign-on");
expect(ssoRadio).toBeDisabled();

await user.click(screen.getByText("Save"));
expect(
screen.getByText(/Password field must be completed/i)
).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import SelectedTeamsForm from "../SelectedTeamsForm/SelectedTeamsForm";
import SelectRoleForm from "../SelectRoleForm/SelectRoleForm";
import { roleOptions } from "../../helpers/userManagementHelpers";

const baseClass = "add-user-form";
const baseClass = "user-form";

export enum NewUserType {
AdminInvited = "ADMIN_INVITED",
Expand Down Expand Up @@ -63,7 +63,6 @@ interface IUserFormProps {
availableTeams: ITeam[];
onCancel: () => void;
onSubmit: (formData: IUserFormData) => void;
submitText: string;
defaultName?: string;
defaultEmail?: string;
currentUserId?: number;
Expand All @@ -90,7 +89,6 @@ const UserForm = ({
availableTeams,
onCancel,
onSubmit,
submitText,
defaultName,
defaultEmail,
currentUserId,
Expand Down Expand Up @@ -140,6 +138,14 @@ const UserForm = ({

const [isGlobalUser, setIsGlobalUser] = useState(!!defaultGlobalRole);

useEffect(() => {
// If SSO is globally disabled but user previously signed in via SSO,
// require password is automatically selected on first render
if (!canUseSso && !isNewUser && isSsoEnabled) {
setFormData({ ...formData, sso_enabled: false });
}
}, []);

// For scrollable modal (re-rerun when formData changes)
useEffect(() => {
checkScroll();
Expand Down Expand Up @@ -248,12 +254,16 @@ const UserForm = ({
} else if (!validEmail(formData.email)) {
newErrors.email = `${formData.email} is not a valid email`;
}
// Only test password on new created user (not invited user) or if sso not enabled
if (

// Password auth required for new "create user" (not new "invite user") with SSO disabled
const isNewAdminCreatedUserWithoutSSO =
isNewUser &&
formData.newUserType === NewUserType.AdminCreated &&
!formData.sso_enabled
) {
!formData.sso_enabled;
// Force switch existing user to password auth if SSO is disabled globally but was enabled
const isExistingUserForcedToPasswordAuth = !canUseSso && isSsoEnabled;

if (isNewAdminCreatedUserWithoutSSO || isExistingUserForcedToPasswordAuth) {
if (formData.password !== null && !validPassword(formData.password)) {
newErrors.password = "Password must meet the criteria below";
}
Expand Down Expand Up @@ -470,50 +480,39 @@ const UserForm = ({
<div className="form-field__label">Authentication</div>
<Radio
className={`${baseClass}__radio-input`}
label="Enable single sign-on"
id="enable-single-sign-on"
label={
canUseSso ? (
"Single sign-on"
) : (
<TooltipWrapper
tipContent={
<>
SSO is not enabled in organization settings.
<br />
User must sign in with a password.
</>
}
>
Single sign-on
</TooltipWrapper>
)
}
id="single-sign-on-authentication"
checked={!!formData.sso_enabled}
value="true"
name="authentication-type"
onChange={() => onSsoChange(true)}
disabled={!canUseSso}
/>
<Radio
className={`${baseClass}__radio-input`}
label="Require password"
id="require-password"
label="Password"
id="password-authentication"
disabled={!(smtpConfigured || sesConfigured)}
checked={!formData.sso_enabled}
value="false"
name="authentication-type"
onChange={() => onSsoChange(false)}
/** Note: This was a checkbox that is now a radio button, waiting on
* product to confirm if we're adding help text to radio buttons as
* it's not in Figma design system, the Figma here, or in the Radio
* component, but the helpText already existed on the checkbox version
*/
// helpText={
// canUseSso ? (
// <p className={`${baseClass}__sso-input sublabel`}>
// Password authentication will be disabled for this user.
// </p>
// ) : (
// <span className={`${baseClass}__sso-input sublabel-nosso`}>
// This user previously signed in via SSO, which has been
// globally disabled.{" "}
// <button
// className="button--text-link"
// onClick={onSsoDisable}
// >
// Add password instead
// <Icon
// name="chevron-right"
// color="core-fleet-blue"
// size="small"
// />
// </button>
// </span>
// )
// }
/>
</div>
);
Expand Down Expand Up @@ -622,8 +621,7 @@ const UserForm = ({
<form autoComplete="off">
{isNewUser && renderAccountSection()}
{renderNameAndEmailSection()}
{((!isNewUser && formData.sso_enabled) || canUseSso) &&
renderAuthenticationSection()}
{renderAuthenticationSection()}
{((isNewUser && formData.newUserType !== NewUserType.AdminInvited) ||
(!isNewUser && !isInvitePending && isModifiedByGlobalAdmin)) &&
!formData.sso_enabled &&
Expand All @@ -650,11 +648,11 @@ const UserForm = ({
type="submit"
variant="brand"
onClick={onFormSubmit}
className={`${submitText === "Add" ? "add" : "save"}-loading
className={`${isNewUser ? "add" : "save"}-loading
`}
isLoading={isUpdatingUsers}
>
{submitText}
{isNewUser ? "Add" : "Save"}
</Button>
</>
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
.add-user-form {
.user-form {
max-height: 581px;
overflow-y: auto;

Expand Down

0 comments on commit 73f6381

Please sign in to comment.