Skip to content

Commit

Permalink
fix(clerk-js,types): Disable fields for SAML only if there is an acti…
Browse files Browse the repository at this point in the history
…ve connection
  • Loading branch information
Mark Pitsilos authored and yourtallness committed Jun 16, 2023
1 parent 5494bac commit 9651658
Show file tree
Hide file tree
Showing 8 changed files with 152 additions and 22 deletions.
6 changes: 6 additions & 0 deletions .changeset/cyan-garlics-act.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@clerk/clerk-js': patch
'@clerk/types': patch
---

Password, first name & last name fields will be disabled if there are active SAML accounts.
4 changes: 4 additions & 0 deletions packages/clerk-js/src/core/resources/SamlAccount.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import { Verification } from './Verification';
export class SamlAccount extends BaseResource implements SamlAccountResource {
id!: string;
provider: SamlIdpSlug = 'saml_custom';
providerUserId: string | null = null;
active = false;
emailAddress = '';
firstName = '';
lastName = '';
Expand All @@ -28,6 +30,8 @@ export class SamlAccount extends BaseResource implements SamlAccountResource {

this.id = data.id;
this.provider = data.provider;
this.providerUserId = data.provider_user_id;
this.active = data.active;
this.emailAddress = data.email_address;
this.firstName = data.first_name;
this.lastName = data.last_name;
Expand Down
18 changes: 9 additions & 9 deletions packages/clerk-js/src/ui/components/UserProfile/PasswordPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export const PasswordPage = withCardStateProvider(() => {
const wizard = useWizard();
const { navigateToFlowStart } = useNavigateToFlowStart();

const canEditPassword = user.samlAccounts.length == 0;
const passwordEditDisabled = user.samlAccounts.some(sa => sa.active);

// Ensure that messages will not use the updated state of User after a password has been set or changed
const successPagePropsRef = useRef<Parameters<typeof SuccessPage>[0]>({
Expand Down Expand Up @@ -127,7 +127,7 @@ export const PasswordPage = withCardStateProvider(() => {
headerTitle={title}
Breadcrumbs={UserProfileBreadcrumbs}
>
{!canEditPassword && <InformationBox message={localizationKeys('userProfile.passwordPage.readonly')} />}
{passwordEditDisabled && <InformationBox message={localizationKeys('userProfile.passwordPage.readonly')} />}

<Form.Root
onSubmit={updatePassword}
Expand All @@ -148,7 +148,7 @@ export const PasswordPage = withCardStateProvider(() => {
minLength={6}
required
autoFocus
isDisabled={!canEditPassword}
isDisabled={passwordEditDisabled}
/>
</Form.ControlRow>
)}
Expand All @@ -158,7 +158,7 @@ export const PasswordPage = withCardStateProvider(() => {
minLength={6}
required
autoFocus={!user.passwordEnabled}
isDisabled={!canEditPassword}
isDisabled={passwordEditDisabled}
/>
</Form.ControlRow>
<Form.ControlRow elementId={confirmField.id}>
Expand All @@ -168,25 +168,25 @@ export const PasswordPage = withCardStateProvider(() => {
displayConfirmPasswordFeedback(e.target.value);
return confirmField.props.onChange(e);
}}
isDisabled={!canEditPassword}
isDisabled={passwordEditDisabled}
/>
</Form.ControlRow>
<Form.ControlRow elementId={sessionsField.id}>
<Form.Control
{...sessionsField.props}
isDisabled={!canEditPassword}
isDisabled={passwordEditDisabled}
/>
</Form.ControlRow>
{canEditPassword ? (
<FormButtons isDisabled={!canSubmit} />
) : (
{passwordEditDisabled ? (
<FormButtonContainer>
<Form.ResetButton
localizationKey={localizationKeys('userProfile.formButtonReset')}
block={false}
onClick={navigateToFlowStart}
/>
</FormButtonContainer>
) : (
<FormButtons isDisabled={!canSubmit} />
)}
</Form.Root>
</ContentPage>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export const ProfilePage = withCardStateProvider(() => {
const requiredFieldsFilled =
hasRequiredFields && !!lastNameField.value && !!firstNameField.value && optionalFieldsChanged;

const canEditName = user.samlAccounts.length == 0;
const nameEditDisabled = user.samlAccounts.some(sa => sa.active);

const onSubmit = async (e: React.FormEvent) => {
e.preventDefault();
Expand Down Expand Up @@ -93,7 +93,7 @@ export const ProfilePage = withCardStateProvider(() => {
headerTitle={title}
Breadcrumbs={UserProfileBreadcrumbs}
>
{!canEditName && <InformationBox message={localizationKeys('userProfile.profilePage.readonly')} />}
{nameEditDisabled && <InformationBox message={localizationKeys('userProfile.profilePage.readonly')} />}

<Form.Root onSubmit={onSubmit}>
<UserProfileAvatarUploader
Expand All @@ -107,7 +107,7 @@ export const ProfilePage = withCardStateProvider(() => {
autoFocus
{...firstNameField.props}
required={first_name.required}
isDisabled={!canEditName}
isDisabled={nameEditDisabled}
/>
</Form.ControlRow>
)}
Expand All @@ -116,7 +116,7 @@ export const ProfilePage = withCardStateProvider(() => {
<Form.Control
{...lastNameField.props}
required={last_name.required}
isDisabled={!canEditName}
isDisabled={nameEditDisabled}
/>
</Form.ControlRow>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ describe('PasswordPage', () => {
});

describe('with SAML', () => {
it('prevents adding a password if user has enterprise connections', async () => {
it('prevents adding a password if user has active enterprise connections', async () => {
const emailAddress = 'george@jungle.com';

const config = createFixtures.config(f => {
Expand All @@ -64,6 +64,7 @@ describe('PasswordPage', () => {
{
id: 'samlacc_foo',
provider: 'saml_okta',
active: true,
email_address: emailAddress,
},
],
Expand All @@ -78,12 +79,48 @@ describe('PasswordPage', () => {
expect(screen.getByLabelText(/confirm password/i)).toBeDisabled();
expect(screen.getByRole('checkbox', { name: 'Sign out of all other devices' })).toBeDisabled();

screen.getByText(
'Your password can currently not be edited because you can sign in only via the enterprise connection.',
);
expect(
screen.getByText(
'Your password can currently not be edited because you can sign in only via the enterprise connection.',
),
).toBeInTheDocument();
});

it('prevents changing a password if user has enterprise connections', async () => {
it('does not prevent adding a password if user has no active enterprise connections', async () => {
const emailAddress = 'george@jungle.com';

const config = createFixtures.config(f => {
f.withEmailAddress();
f.withSaml();
f.withUser({
email_addresses: [emailAddress],
saml_accounts: [
{
id: 'samlacc_foo',
provider: 'saml_okta',
active: false,
email_address: emailAddress,
},
],
});
});

const { wrapper } = await createFixtures(config);

render(<PasswordPage />, { wrapper });

expect(screen.getByLabelText(/new password/i)).not.toBeDisabled();
expect(screen.getByLabelText(/confirm password/i)).not.toBeDisabled();
expect(screen.getByRole('checkbox', { name: 'Sign out of all other devices' })).not.toBeDisabled();

expect(
screen.queryByText(
'Your password can currently not be edited because you can sign in only via the enterprise connection.',
),
).not.toBeInTheDocument();
});

it('prevents changing a password if user has active enterprise connections', async () => {
const emailAddress = 'george@jungle.com';

const config = createFixtures.config(f => {
Expand All @@ -96,6 +133,7 @@ describe('PasswordPage', () => {
{
id: 'samlacc_foo',
provider: 'saml_okta',
active: true,
email_address: emailAddress,
},
],
Expand All @@ -111,9 +149,47 @@ describe('PasswordPage', () => {
expect(screen.getByLabelText(/confirm password/i)).toBeDisabled();
expect(screen.getByRole('checkbox', { name: 'Sign out of all other devices' })).toBeDisabled();

screen.getByText(
'Your password can currently not be edited because you can sign in only via the enterprise connection.',
);
expect(
screen.getByText(
'Your password can currently not be edited because you can sign in only via the enterprise connection.',
),
).toBeInTheDocument();
});

it('does not prevent changing a password if user has no active enterprise connections', async () => {
const emailAddress = 'george@jungle.com';

const config = createFixtures.config(f => {
f.withEmailAddress();
f.withSaml();
f.withUser({
password_enabled: true,
email_addresses: [emailAddress],
saml_accounts: [
{
id: 'samlacc_foo',
provider: 'saml_okta',
active: false,
email_address: emailAddress,
},
],
});
});

const { wrapper } = await createFixtures(config);

render(<PasswordPage />, { wrapper });

expect(screen.getByLabelText(/current password/i)).not.toBeDisabled();
expect(screen.getByLabelText(/new password/i)).not.toBeDisabled();
expect(screen.getByLabelText(/confirm password/i)).not.toBeDisabled();
expect(screen.getByRole('checkbox', { name: 'Sign out of all other devices' })).not.toBeDisabled();

expect(
screen.queryByText(
'Your password can currently not be edited because you can sign in only via the enterprise connection.',
),
).not.toBeInTheDocument();
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe('ProfilePage', () => {
});

describe('with SAML', () => {
it('disables the first & last name inputs if user has enterprise connections', async () => {
it('disables the first & last name inputs if user has active enterprise connections', async () => {
const emailAddress = 'george@jungle.com';
const firstName = 'George';
const lastName = 'Clerk';
Expand All @@ -60,6 +60,7 @@ describe('ProfilePage', () => {
{
id: 'samlacc_foo',
provider: 'saml_okta',
active: true,
email_address: emailAddress,
},
],
Expand All @@ -72,8 +73,47 @@ describe('ProfilePage', () => {

expect(screen.getByRole('textbox', { name: 'First name' })).toBeDisabled();
expect(screen.getByRole('textbox', { name: 'Last name' })).toBeDisabled();

screen.getByText('Your profile information has been provided by the enterprise connection and cannot be edited.');
});

it('does not disable the first & last name inputs if user has no active enterprise connections', async () => {
const emailAddress = 'george@jungle.com';
const firstName = 'George';
const lastName = 'Clerk';

const config = createFixtures.config(f => {
f.withEmailAddress();
f.withSaml();
f.withName();
f.withUser({
first_name: firstName,
last_name: lastName,
email_addresses: [emailAddress],
saml_accounts: [
{
id: 'samlacc_foo',
provider: 'saml_okta',
active: false,
email_address: emailAddress,
},
],
});
});

const { wrapper } = await createFixtures(config);

render(<ProfilePage />, { wrapper });

expect(screen.getByRole('textbox', { name: 'First name' })).not.toBeDisabled();
expect(screen.getByRole('textbox', { name: 'Last name' })).not.toBeDisabled();

expect(
screen.queryByText(
'Your profile information has been provided by the enterprise connection and cannot be edited.',
),
).not.toBeInTheDocument();
});
});

describe('Profile image', () => {
Expand Down
2 changes: 2 additions & 0 deletions packages/types/src/json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ export interface ExternalAccountJSON extends ClerkResourceJSON {
export interface SamlAccountJSON extends ClerkResourceJSON {
object: 'saml_account';
provider: SamlIdpSlug;
provider_user_id: string | null;
active: boolean;
email_address: string;
first_name: string;
last_name: string;
Expand Down
2 changes: 2 additions & 0 deletions packages/types/src/samlAccount.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import type { VerificationResource } from './verification';
*/
export interface SamlAccountResource extends ClerkResource {
provider: SamlIdpSlug;
providerUserId: string | null;
active: boolean;
emailAddress: string;
firstName: string;
lastName: string;
Expand Down

0 comments on commit 9651658

Please sign in to comment.