Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PM-11502] Support Remember Email option when using SSO authentication #13391

Draft
wants to merge 7 commits into
base: auth/pm-17751/remember-sso-email
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 7 additions & 13 deletions apps/browser/src/auth/popup/home.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,10 @@
async ngOnInit(): Promise<void> {
this.listenForUnauthUiRefreshFlagChanges();

const email = await firstValueFrom(this.loginEmailService.loginEmail$);
const rememberEmail = this.loginEmailService.getRememberEmail();
const storedEmail = await firstValueFrom(this.loginEmailService.rememberedEmail$);

Check warning on line 48 in apps/browser/src/auth/popup/home.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/auth/popup/home.component.ts#L48

Added line #L48 was not covered by tests

if (email != null) {
this.formGroup.patchValue({ email, rememberEmail });
} else {
const storedEmail = await firstValueFrom(this.loginEmailService.storedEmail$);

if (storedEmail != null) {
this.formGroup.patchValue({ email: storedEmail, rememberEmail: true });
}
if (storedEmail != null) {
this.formGroup.patchValue({ email: storedEmail, rememberEmail: true });

Check warning on line 51 in apps/browser/src/auth/popup/home.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/auth/popup/home.component.ts#L51

Added line #L51 was not covered by tests
}

this.environmentSelector.onOpenSelfHostedSettings
Expand Down Expand Up @@ -121,8 +114,9 @@

async setLoginEmailValues() {
// Note: Browser saves email settings here instead of the login component
this.loginEmailService.setRememberEmail(this.formGroup.controls.rememberEmail.value);
await this.loginEmailService.setLoginEmail(this.formGroup.controls.email.value);
await this.loginEmailService.saveEmailSettings();
this.loginEmailService.setRememberedEmailChoice(

Check warning on line 117 in apps/browser/src/auth/popup/home.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/auth/popup/home.component.ts#L117

Added line #L117 was not covered by tests
this.formGroup.controls.email.value,
this.formGroup.controls.rememberEmail.value,
);
}
}
15 changes: 8 additions & 7 deletions apps/web/src/app/auth/login/login-v1.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,16 @@
<div class="tw-mb-3 tw-flex tw-flex-col tw-items-center tw-justify-center">
<p class="tw-mb-3">{{ "or" | i18n }}</p>

<a
bitLink
<button
type="button"
bitButton
block
linkType="primary"
routerLink="/login-with-passkey"
(mousedown)="$event.preventDefault()"
buttonType="primary"
(click)="handleLoginWithPasskeyClick()"
>
<span><i class="bwi bwi-passkey"></i> {{ "logInWithPasskey" | i18n }}</span>
</a>
<i class="bwi bwi-passkey tw-mr-1"></i>
{{ "logInWithPasskey" | i18n }}
</button>
</div>

<hr />
Expand Down
16 changes: 14 additions & 2 deletions apps/web/src/app/auth/login/login-v1.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,12 +162,11 @@
}
}

this.loginEmailService.clearValues();
this.loginEmailService.clearLoginEmail();

Check warning on line 165 in apps/web/src/app/auth/login/login-v1.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/auth/login/login-v1.component.ts#L165

Added line #L165 was not covered by tests
await this.router.navigate([this.successRoute]);
}

async goToHint() {
await this.saveEmailSettings();
await this.router.navigateByUrl("/hint");
}

Expand All @@ -182,6 +181,19 @@
await this.router.navigate(["/signup"]);
}

/**
* Handle the Login with Passkey button click.
* We need a handler here in order to persist the remember email selection to state before routing.
* @param event - The event object.
*/
async handleLoginWithPasskeyClick() {
const email = this.formGroup.value.email;

Check warning on line 190 in apps/web/src/app/auth/login/login-v1.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/auth/login/login-v1.component.ts#L190

Added line #L190 was not covered by tests
if (email) {
await this.saveEmailSettings();

Check warning on line 192 in apps/web/src/app/auth/login/login-v1.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/auth/login/login-v1.component.ts#L192

Added line #L192 was not covered by tests
}
await this.router.navigate(["/login-with-passkey"]);

Check warning on line 194 in apps/web/src/app/auth/login/login-v1.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/auth/login/login-v1.component.ts#L194

Added line #L194 was not covered by tests
}

protected override async handleMigrateEncryptionKey(result: AuthResult): Promise<boolean> {
if (!result.requiresEncryptionKeyMigration) {
return false;
Expand Down
2 changes: 1 addition & 1 deletion apps/web/src/app/auth/two-factor.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@
}

goAfterLogIn = async () => {
this.loginEmailService.clearValues();
this.loginEmailService.clearLoginEmail();

Check warning on line 132 in apps/web/src/app/auth/two-factor.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/auth/two-factor.component.ts#L132

Added line #L132 was not covered by tests
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.router.navigate([this.successRoute], {
Expand Down
35 changes: 13 additions & 22 deletions libs/angular/src/auth/components/login-v1.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,6 @@
this.formPromise = this.loginStrategyService.logIn(credentials);
const response = await this.formPromise;

await this.saveEmailSettings();

if (this.handleCaptchaRequired(response)) {
return;
} else if (await this.handleMigrateEncryptionKey(response)) {
Expand All @@ -193,7 +191,7 @@
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.onSuccessfulLoginForceResetNavigate();
} else {
this.loginEmailService.clearValues();
this.loginEmailService.clearLoginEmail();

Check warning on line 194 in libs/angular/src/auth/components/login-v1.component.ts

View check run for this annotation

Codecov / codecov/patch

libs/angular/src/auth/components/login-v1.component.ts#L194

Added line #L194 was not covered by tests
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.router.navigate([this.forcePasswordResetRoute]);
Expand All @@ -210,7 +208,7 @@
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.onSuccessfulLoginNavigate(response.userId);
} else {
this.loginEmailService.clearValues();
this.loginEmailService.clearLoginEmail();

Check warning on line 211 in libs/angular/src/auth/components/login-v1.component.ts

View check run for this annotation

Codecov / codecov/patch

libs/angular/src/auth/components/login-v1.component.ts#L211

Added line #L211 was not covered by tests
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.router.navigate([this.successRoute]);
Expand Down Expand Up @@ -240,7 +238,6 @@
return;
}

await this.saveEmailSettings();
await this.router.navigate(["/login-with-device"]);
}

Expand Down Expand Up @@ -291,6 +288,7 @@
const emailValid = this.formGroup.get("email").valid;

if (emailValid) {
await this.saveEmailSettings();

Check warning on line 291 in libs/angular/src/auth/components/login-v1.component.ts

View check run for this annotation

Codecov / codecov/patch

libs/angular/src/auth/components/login-v1.component.ts#L291

Added line #L291 was not covered by tests
this.toggleValidateEmail(true);
await this.getLoginWithDevice(this.loggedEmail);
}
Expand Down Expand Up @@ -320,31 +318,24 @@
}

private async loadEmailSettings() {
// Try to load from memory first
const email = await firstValueFrom(this.loginEmailService.loginEmail$);
const rememberEmail = this.loginEmailService.getRememberEmail();

if (email) {
this.formGroup.controls.email.setValue(email);
this.formGroup.controls.rememberEmail.setValue(rememberEmail);
const rememberedEmail = await firstValueFrom(this.loginEmailService.rememberedEmail$);

Check warning on line 321 in libs/angular/src/auth/components/login-v1.component.ts

View check run for this annotation

Codecov / codecov/patch

libs/angular/src/auth/components/login-v1.component.ts#L321

Added line #L321 was not covered by tests
if (rememberedEmail) {
this.formGroup.controls.email.setValue(rememberedEmail);
this.formGroup.controls.rememberEmail.setValue(true);

Check warning on line 324 in libs/angular/src/auth/components/login-v1.component.ts

View check run for this annotation

Codecov / codecov/patch

libs/angular/src/auth/components/login-v1.component.ts#L323-L324

Added lines #L323 - L324 were not covered by tests
} else {
// If not in memory, check email on disk
const storedEmail = await firstValueFrom(this.loginEmailService.storedEmail$);
if (storedEmail) {
// If we have a stored email, rememberEmail should default to true
this.formGroup.controls.email.setValue(storedEmail);
this.formGroup.controls.rememberEmail.setValue(true);
}
this.formGroup.controls.rememberEmail.setValue(false);

Check warning on line 326 in libs/angular/src/auth/components/login-v1.component.ts

View check run for this annotation

Codecov / codecov/patch

libs/angular/src/auth/components/login-v1.component.ts#L326

Added line #L326 was not covered by tests
}
}

protected async saveEmailSettings() {
// Save off email for SSO
await this.ssoLoginService.setSsoEmail(this.formGroup.value.email);

this.loginEmailService.setLoginEmail(this.formGroup.value.email);
this.loginEmailService.setRememberEmail(this.formGroup.value.rememberEmail);
await this.loginEmailService.saveEmailSettings();
await this.loginEmailService.setLoginEmail(this.formGroup.value.email);
await this.loginEmailService.setRememberedEmailChoice(

Check warning on line 335 in libs/angular/src/auth/components/login-v1.component.ts

View check run for this annotation

Codecov / codecov/patch

libs/angular/src/auth/components/login-v1.component.ts#L334-L335

Added lines #L334 - L335 were not covered by tests
this.formGroup.value.email,
this.formGroup.value.rememberEmail,
);
}

// Legacy accounts used the master key to encrypt data. Migration is required but only performed on web
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -515,11 +515,6 @@ export class LoginViaAuthRequestComponentV1
}

private async handleSuccessfulLoginNavigation() {
if (this.state === State.StandardAuthRequest) {
// Only need to set remembered email on standard login with auth req flow
await this.loginEmailService.saveEmailSettings();
}

if (this.onSuccessfulLogin != null) {
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -306,17 +306,17 @@ describe("TwoFactorComponent", () => {
expect(component.onSuccessfulLogin).toHaveBeenCalled();
});

it("calls loginEmailService.clearValues() when login is successful", async () => {
it("calls loginEmailService.clearLoginEmail() when login is successful", async () => {
// Arrange
mockLoginStrategyService.logInTwoFactor.mockResolvedValue(new AuthResult());
// spy on loginEmailService.clearValues
const clearValuesSpy = jest.spyOn(mockLoginEmailService, "clearValues");
// spy on loginEmailService.clearLoginEmail
const clearEmailSpy = jest.spyOn(mockLoginEmailService, "clearLoginEmail");

// Act
await component.submit();

// Assert
expect(clearValuesSpy).toHaveBeenCalled();
expect(clearEmailSpy).toHaveBeenCalled();
});

describe("Set Master Password scenarios", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@
await this.submit();
};
goAfterLogIn = async () => {
this.loginEmailService.clearValues();
this.loginEmailService.clearLoginEmail();

Check warning on line 113 in libs/angular/src/auth/components/two-factor-auth/two-factor-auth.component.ts

View check run for this annotation

Codecov / codecov/patch

libs/angular/src/auth/components/two-factor-auth/two-factor-auth.component.ts#L113

Added line #L113 was not covered by tests
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.router.navigate([this.successRoute], {
Expand Down Expand Up @@ -264,7 +264,7 @@
// - Browser SSO on extension open
const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id;
await this.ssoLoginService.setActiveUserOrganizationSsoIdentifier(this.orgIdentifier, userId);
this.loginEmailService.clearValues();
this.loginEmailService.clearLoginEmail();

// note: this flow affects both TDE & standard users
if (this.isForcePasswordResetRequired(authResult)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,17 +301,17 @@ describe("TwoFactorComponent", () => {
expect(component.onSuccessfulLogin).toHaveBeenCalled();
});

it("calls loginEmailService.clearValues() when login is successful", async () => {
it("calls loginEmailService.clearLoginEmail() when login is successful", async () => {
// Arrange
mockLoginStrategyService.logInTwoFactor.mockResolvedValue(new AuthResult());
// spy on loginEmailService.clearValues
const clearValuesSpy = jest.spyOn(mockLoginEmailService, "clearValues");
// spy on loginEmailService.clearLoginEmail()
const clearLoginEmailSpy = jest.spyOn(mockLoginEmailService, "clearLoginEmail");

// Act
await component.doSubmit();

// Assert
expect(clearValuesSpy).toHaveBeenCalled();
expect(clearLoginEmailSpy).toHaveBeenCalled();
});

describe("Set Master Password scenarios", () => {
Expand Down
2 changes: 1 addition & 1 deletion libs/angular/src/auth/components/two-factor.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ export class TwoFactorComponent extends CaptchaProtectedComponent implements OnI
// - Browser SSO on extension open
const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id;
await this.ssoLoginService.setActiveUserOrganizationSsoIdentifier(this.orgIdentifier, userId);
this.loginEmailService.clearValues();
this.loginEmailService.clearLoginEmail();

// note: this flow affects both TDE & standard users
if (this.isForcePasswordResetRequired(authResult)) {
Expand Down
2 changes: 1 addition & 1 deletion libs/angular/src/services/jslib-services.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1430,7 +1430,7 @@ const safeProviders: SafeProvider[] = [
safeProvider({
provide: LoginSuccessHandlerService,
useClass: DefaultLoginSuccessHandlerService,
deps: [SyncService, UserAsymmetricKeysRegenerationService],
deps: [SyncService, UserAsymmetricKeysRegenerationService, LoginEmailService],
}),
safeProvider({
provide: PasswordLoginStrategy,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -570,11 +570,6 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy {
}

private async handleSuccessfulLoginNavigation(userId: UserId) {
if (this.flow === Flow.StandardAuthRequest) {
// Only need to set remembered email on standard login with auth req flow
await this.loginEmailService.saveEmailSettings();
}

await this.loginSuccessHandlerService.run(userId);
await this.router.navigate(["vault"]);
}
Expand Down
19 changes: 12 additions & 7 deletions libs/auth/src/angular/login/login.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,12 @@

<!-- Remember Email input -->
<bit-form-control>
<input type="checkbox" formControlName="rememberEmail" bitCheckbox />
<input
type="checkbox"
formControlName="rememberEmail"
(input)="onRememberEmailInput($event)"
bitCheckbox
/>
<bit-label>{{ "rememberEmail" | i18n }}</bit-label>
</bit-form-control>

Expand All @@ -39,18 +44,18 @@

<div class="tw-text-center">{{ "or" | i18n }}</div>

<!-- Link to Login with Passkey page -->
<!-- Button to Login with Passkey -->
<ng-container *ngIf="isLoginWithPasskeySupported()">
<a
<button
type="button"
bitButton
block
linkType="primary"
routerLink="/login-with-passkey"
(mousedown)="$event.preventDefault()"
buttonType="secondary"
(click)="handleLoginWithPasskeyClick()"
>
<i class="bwi bwi-passkey tw-mr-1"></i>
{{ "logInWithPasskey" | i18n }}
</a>
</button>
</ng-container>

<!-- Button to Login with SSO -->
Expand Down
Loading
Loading