Skip to content

Commit

Permalink
fix(clerk-js): Take into account enabled attributes and not first fac…
Browse files Browse the repository at this point in the history
…tors for Sign Up form

This commit fixes a bug in our UI Sign Up component. Until now for Sign up inputs, we were taking into account attributes that were used as first factors. This was wrong as first factors applies to Sign in only. For Sign up, we should take into account any enabled attribute or a valid combination between them (username + password)

https://www.notion.so/clerkdev/Fix-PSU-with-required-emails-when-using-Sign-in-with-Metamask-32ef5f2f7261407cb3136b877fb20133
  • Loading branch information
chanioxaris committed Nov 1, 2022
1 parent 2685b27 commit d3fdb1d
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 75 deletions.
101 changes: 28 additions & 73 deletions packages/clerk-js/src/core/resources/UserSettings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,141 +56,96 @@ describe('UserSettings', () => {
expect(sut.instanceIsPasswordBased).toEqual(false);
});

it('returns true if the instance has a valid email address auth factor', function () {
it('returns true if the instance has a valid auth factor', function () {
let sut = new UserSettings({
attributes: {
email_address: {
enabled: true,
required: true,
used_for_first_factor: false,
},
phone_number: {
enabled: true,
required: true,
used_for_first_factor: false,
enabled: false,
required: false,
},
password: {
enabled: true,
required: false,
},
username: {
enabled: false,
required: false,
},
},
} as any as UserSettingsJSON);

expect(sut.hasValidAuthFactor).toEqual(false);
expect(sut.hasValidAuthFactor).toEqual(true);

sut = new UserSettings({
attributes: {
email_address: {
enabled: true,
required: true,
used_for_first_factor: true,
enabled: false,
required: false,
},
phone_number: {
enabled: true,
required: true,
used_for_first_factor: false,
required: false,
},
password: {
enabled: false,
required: false,
},
username: {
enabled: true,
required: false,
},
},
} as any as UserSettingsJSON);
expect(sut.hasValidAuthFactor).toEqual(true);
});

it('returns true if the instance has a valid phone number auth factor', function () {
let sut = new UserSettings({
sut = new UserSettings({
attributes: {
email_address: {
enabled: true,
required: true,
used_for_first_factor: false,
enabled: false,
required: false,
},
phone_number: {
enabled: true,
required: true,
used_for_first_factor: false,
},
password: {
enabled: true,
enabled: false,
required: false,
},
},
} as any as UserSettingsJSON);

expect(sut.hasValidAuthFactor).toEqual(false);

sut = new UserSettings({
attributes: {
email_address: {
password: {
enabled: true,
required: true,
used_for_first_factor: false,
},
phone_number: {
username: {
enabled: true,
required: true,
used_for_first_factor: true,
},
password: {
enabled: true,
required: false,
},
},
} as any as UserSettingsJSON);
expect(sut.hasValidAuthFactor).toEqual(true);
});

it('returns true if the instance has username and password auth factor', function () {
let sut = new UserSettings({
it('returns false if the instance has not a valid auth factor', function () {
const sut = new UserSettings({
attributes: {
email_address: {
enabled: true,
required: true,
used_for_first_factor: false,
enabled: false,
required: false,
},
phone_number: {
enabled: true,
required: true,
used_for_first_factor: false,
enabled: false,
required: false,
},
password: {
enabled: true,
required: false,
},
username: {
enabled: true,
enabled: false,
required: false,
},
},
} as any as UserSettingsJSON);

expect(sut.hasValidAuthFactor).toEqual(false);

sut = new UserSettings({
attributes: {
email_address: {
enabled: true,
required: true,
used_for_first_factor: false,
},
phone_number: {
enabled: true,
required: true,
used_for_first_factor: true,
},
password: {
enabled: true,
required: true,
},
username: {
enabled: true,
required: true,
},
},
} as any as UserSettingsJSON);
expect(sut.hasValidAuthFactor).toEqual(true);
});

it('returns enabled social provider strategies', function () {
Expand Down
4 changes: 2 additions & 2 deletions packages/clerk-js/src/core/resources/UserSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ export class UserSettings extends BaseResource implements UserSettingsResource {

get hasValidAuthFactor() {
return (
this.attributes.email_address.used_for_first_factor ||
this.attributes.phone_number.used_for_first_factor ||
this.attributes.email_address.enabled ||
this.attributes.phone_number.enabled ||
(this.attributes.password.required && this.attributes.username.required)
);
}
Expand Down

0 comments on commit d3fdb1d

Please sign in to comment.