From 6b2640633114f63ea15517fa5bb0ae4b46ad79e8 Mon Sep 17 00:00:00 2001 From: Jared Snider <116684653+JaredSnider-Bitwarden@users.noreply.github.com> Date: Thu, 4 May 2023 14:57:11 -0400 Subject: [PATCH] Defect/PM-1196 - SSO with Email 2FA Flow - Email Required error fixed (#5280) * PM-1196- First draft of solution for solving SSO login with email 2FA not working; this is a working solution but we need to leverage it to build a better solution with a different server generated token vs a OTP. * PM-1196 - Swap from OTP to SSO Email 2FA session token. Working now, but going to revisit whether or not email should come down from the server. Need to clean up the commented out items if we decide email stays encrypted in the session token. * PM-1196 - Email needs to come down from server after SSO in order to flow through to the 2FA comp and be sent to the server * PM-1196 - For email 2FA, if the email is no longer available due to the auth service 2 min expiration clearing the auth state, then we need to show a message explaining that (same message as when a OTP is submitted after expiration) vs actually sending the request without an email and getting a validation error from the server * PM-1196 - (1) Make optional properties optional (2) Update tests to pass (3) Add new test for Email 2FA having additional auth result information * PM-1196 - Remove unnecessary optional chaining operator b/c I go my wires crossed on how it works and the login strategy is not going to be null or undefined... --- .../auth/components/two-factor.component.ts | 10 ++++++ .../src/auth/abstractions/auth.service.ts | 1 + .../login-strategies/login.strategy.spec.ts | 36 +++++++++++++++++++ .../auth/login-strategies/login.strategy.ts | 2 ++ .../login-strategies/sso-login.strategy.ts | 14 ++++++-- .../src/auth/models/domain/auth-result.ts | 2 ++ .../request/two-factor-email.request.ts | 1 + .../response/identity-two-factor.response.ts | 7 +++- libs/common/src/auth/services/auth.service.ts | 9 ++++- 9 files changed, 78 insertions(+), 4 deletions(-) diff --git a/libs/angular/src/auth/components/two-factor.component.ts b/libs/angular/src/auth/components/two-factor.component.ts index 255fb8aa2d3..c7bf5075cd0 100644 --- a/libs/angular/src/auth/components/two-factor.component.ts +++ b/libs/angular/src/auth/components/two-factor.component.ts @@ -232,10 +232,20 @@ export class TwoFactorComponent extends CaptchaProtectedComponent implements OnI return; } + if (this.authService.email == null) { + this.platformUtilsService.showToast( + "error", + this.i18nService.t("errorOccurred"), + this.i18nService.t("sessionTimeout") + ); + return; + } + try { const request = new TwoFactorEmailRequest(); request.email = this.authService.email; request.masterPasswordHash = this.authService.masterPasswordHash; + request.ssoEmail2FaSessionToken = this.authService.ssoEmail2FaSessionToken; request.deviceIdentifier = await this.appIdService.getAppId(); request.authRequestAccessCode = this.authService.accessCode; request.authRequestId = this.authService.authRequestId; diff --git a/libs/common/src/auth/abstractions/auth.service.ts b/libs/common/src/auth/abstractions/auth.service.ts index 2fec2fcd11d..4a59f0ac4ee 100644 --- a/libs/common/src/auth/abstractions/auth.service.ts +++ b/libs/common/src/auth/abstractions/auth.service.ts @@ -18,6 +18,7 @@ export abstract class AuthService { email: string; accessCode: string; authRequestId: string; + ssoEmail2FaSessionToken: string; logIn: ( credentials: diff --git a/libs/common/src/auth/login-strategies/login.strategy.spec.ts b/libs/common/src/auth/login-strategies/login.strategy.spec.ts index 7c7b1ad8fae..3f7e8fb8c13 100644 --- a/libs/common/src/auth/login-strategies/login.strategy.spec.ts +++ b/libs/common/src/auth/login-strategies/login.strategy.spec.ts @@ -223,6 +223,9 @@ describe("LogInStrategy", () => { TwoFactorProviders2: { 0: null }, error: "invalid_grant", error_description: "Two factor required.", + // only sent for emailed 2FA + email: undefined, + ssoEmail2faSessionToken: undefined, }); apiService.postIdentityToken.mockResolvedValue(tokenResponse); @@ -238,6 +241,39 @@ describe("LogInStrategy", () => { expect(result).toEqual(expected); }); + it("rejects login if 2FA via email is required + maps required information", async () => { + // Sample response where Email 2FA required + + const userEmail = "kyle@bitwarden.com"; + const ssoEmail2FaSessionToken = + "BwSsoEmail2FaSessionToken_CfDJ8AMrVzKqBFpKqzzsahUx8ubIi9AhHm6aLHDLpCUYc3QV3qC14iuSVkNg57Q7-kGQUn1z87bGY1WP58jFMNJ6ndaurIgQWNfPNN4DG-dBhvzarOAZ0RKY5oKT5futWm6_k9NMMGd8PcGGHg5Pq1_koOIwRtiXO3IpD-bemB7m8oEvbj__JTQP3Mcz-UediFlCbYBKU3wyIiBL_tF8hW5D4RAUa5ZzXIuauJiiCdDS7QOzBcqcusVAPGFfKjfIdAwFfKSOYd5KmYrhK7Y7ymjweP_igPYKB5aMfcVaYr5ux-fdffeJTGqtJorwNjLUYNv7KA"; + + const tokenResponse = new IdentityTwoFactorResponse({ + TwoFactorProviders: ["1"], + TwoFactorProviders2: { "1": { Email: "k***@bitwarden.com" } }, + error: "invalid_grant", + error_description: "Two factor required.", + // only sent for emailed 2FA + email: userEmail, + ssoEmail2faSessionToken: ssoEmail2FaSessionToken, + }); + + apiService.postIdentityToken.mockResolvedValue(tokenResponse); + + const result = await passwordLogInStrategy.logIn(credentials); + + expect(stateService.addAccount).not.toHaveBeenCalled(); + expect(messagingService.send).not.toHaveBeenCalled(); + + const expected = new AuthResult(); + expected.twoFactorProviders = new Map(); + expected.twoFactorProviders.set(1, { Email: "k***@bitwarden.com" }); + expected.email = userEmail; + expected.ssoEmail2FaSessionToken = ssoEmail2FaSessionToken; + + expect(result).toEqual(expected); + }); + it("sends stored 2FA token to server", async () => { tokenService.getTwoFactorToken.mockResolvedValue(twoFactorToken); apiService.postIdentityToken.mockResolvedValue(identityTokenResponseFactory()); diff --git a/libs/common/src/auth/login-strategies/login.strategy.ts b/libs/common/src/auth/login-strategies/login.strategy.ts index 286e90a26cf..2bcbee88e00 100644 --- a/libs/common/src/auth/login-strategies/login.strategy.ts +++ b/libs/common/src/auth/login-strategies/login.strategy.ts @@ -163,6 +163,8 @@ export abstract class LogInStrategy { this.twoFactorService.setProviders(response); this.captchaBypassToken = response.captchaToken ?? null; + result.ssoEmail2FaSessionToken = response.ssoEmail2faSessionToken; + result.email = response.email; return result; } diff --git a/libs/common/src/auth/login-strategies/sso-login.strategy.ts b/libs/common/src/auth/login-strategies/sso-login.strategy.ts index 7d3f9e519ef..2285d1b6b4e 100644 --- a/libs/common/src/auth/login-strategies/sso-login.strategy.ts +++ b/libs/common/src/auth/login-strategies/sso-login.strategy.ts @@ -18,6 +18,12 @@ export class SsoLogInStrategy extends LogInStrategy { tokenRequest: SsoTokenRequest; orgId: string; + // A session token server side to serve as an authentication factor for the user + // in order to send email OTPs to the user's configured 2FA email address + // as we don't have a master password hash or other verifiable secret when using SSO. + ssoEmail2FaSessionToken?: string; + email?: string; // email not preserved through SSO process so get from server + constructor( cryptoService: CryptoService, apiService: ApiService, @@ -65,7 +71,11 @@ export class SsoLogInStrategy extends LogInStrategy { await this.buildDeviceRequest() ); - const [authResult] = await this.startLogIn(); - return authResult; + const [ssoAuthResult] = await this.startLogIn(); + + this.email = ssoAuthResult.email; + this.ssoEmail2FaSessionToken = ssoAuthResult.ssoEmail2FaSessionToken; + + return ssoAuthResult; } } diff --git a/libs/common/src/auth/models/domain/auth-result.ts b/libs/common/src/auth/models/domain/auth-result.ts index bfc6236d650..6ce1e568e1e 100644 --- a/libs/common/src/auth/models/domain/auth-result.ts +++ b/libs/common/src/auth/models/domain/auth-result.ts @@ -8,6 +8,8 @@ export class AuthResult { resetMasterPassword = false; forcePasswordReset: ForceResetPasswordReason = ForceResetPasswordReason.None; twoFactorProviders: Map = null; + ssoEmail2FaSessionToken?: string; + email: string; get requiresCaptcha() { return !Utils.isNullOrWhitespace(this.captchaSiteKey); diff --git a/libs/common/src/auth/models/request/two-factor-email.request.ts b/libs/common/src/auth/models/request/two-factor-email.request.ts index 22cb638e9d0..769f6171c64 100644 --- a/libs/common/src/auth/models/request/two-factor-email.request.ts +++ b/libs/common/src/auth/models/request/two-factor-email.request.ts @@ -4,4 +4,5 @@ export class TwoFactorEmailRequest extends SecretVerificationRequest { email: string; deviceIdentifier: string; authRequestId: string; + ssoEmail2FaSessionToken?: string; } diff --git a/libs/common/src/auth/models/response/identity-two-factor.response.ts b/libs/common/src/auth/models/response/identity-two-factor.response.ts index ec0cfa0f31a..e2848bd3361 100644 --- a/libs/common/src/auth/models/response/identity-two-factor.response.ts +++ b/libs/common/src/auth/models/response/identity-two-factor.response.ts @@ -7,7 +7,9 @@ export class IdentityTwoFactorResponse extends BaseResponse { twoFactorProviders: TwoFactorProviderType[]; twoFactorProviders2 = new Map(); captchaToken: string; - masterPasswordPolicy: MasterPasswordPolicyResponse; + ssoEmail2faSessionToken: string; + email?: string; + masterPasswordPolicy?: MasterPasswordPolicyResponse; constructor(response: any) { super(response); @@ -25,5 +27,8 @@ export class IdentityTwoFactorResponse extends BaseResponse { this.masterPasswordPolicy = new MasterPasswordPolicyResponse( this.getResponseProperty("MasterPasswordPolicy") ); + + this.ssoEmail2faSessionToken = this.getResponseProperty("SsoEmail2faSessionToken"); + this.email = this.getResponseProperty("Email"); } } diff --git a/libs/common/src/auth/services/auth.service.ts b/libs/common/src/auth/services/auth.service.ts index f5a19bf6367..f8028c6944a 100644 --- a/libs/common/src/auth/services/auth.service.ts +++ b/libs/common/src/auth/services/auth.service.ts @@ -46,7 +46,8 @@ export class AuthService implements AuthServiceAbstraction { get email(): string { if ( this.logInStrategy instanceof PasswordLogInStrategy || - this.logInStrategy instanceof PasswordlessLogInStrategy + this.logInStrategy instanceof PasswordlessLogInStrategy || + this.logInStrategy instanceof SsoLogInStrategy ) { return this.logInStrategy.email; } @@ -72,6 +73,12 @@ export class AuthService implements AuthServiceAbstraction { : null; } + get ssoEmail2FaSessionToken(): string { + return this.logInStrategy instanceof SsoLogInStrategy + ? this.logInStrategy.ssoEmail2FaSessionToken + : null; + } + private logInStrategy: | UserApiLogInStrategy | PasswordLogInStrategy