[PM-17751] Store SSO email in state on web client (#13295)

* Moved saving of SSO email outside of browser/desktop code * Clarified comments. * Tests * Refactored login component services to manage state * Fixed input on login component * Fixed tests * Linting * Moved web setting in state into web override * updated tests * Fixed typing. * Fixed type safety issues. * Added comments and renamed for clarity. * Removed method parameters that weren't used * Added clarifying comments * Added more comments. * Removed test that is not necessary on base * Test cleanup * More comments. * Linting * Fixed test. * Fixed base URL * Fixed typechecking. * Type checking * Moved setting of email state to default service * Added comments. * Consolidated SSO URL formatting * Updated comment * Fixed reference. * Fixed missing parameter. * Initialized service. * Added comments * Added initialization of new service * Made email optional due to CLI. * Fixed comment on handleSsoClick. * Added SSO email persistence to v1 component. --------- Co-authored-by: Bernd Schoolmann <[email protected]>
bitwarden · Feb 21, 2025 · 077e0f8 · 077e0f8
1 parent 9dd2033
commit 077e0f8
Show file tree

Hide file tree

Showing 26 changed files with 532 additions and 243 deletions.
diff --git a/apps/browser/src/auth/popup/login/extension-login-component.service.spec.ts b/apps/browser/src/auth/popup/login/extension-login-component.service.spec.ts
@@ -1,11 +1,18 @@
 import { TestBed } from "@angular/core/testing";
 import { MockProxy, mock } from "jest-mock-extended";
+import { BehaviorSubject } from "rxjs";
 
 import { DefaultLoginComponentService } from "@bitwarden/auth/angular";
+import { SsoUrlService } from "@bitwarden/auth/common";
 import { SsoLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/sso-login.service.abstraction";
+import { ClientType } from "@bitwarden/common/enums";
 import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service";
-import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
+import {
+  Environment,
+  EnvironmentService,
+} from "@bitwarden/common/platform/abstractions/environment.service";
 import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
+import { Utils } from "@bitwarden/common/platform/misc/utils";
 import { PasswordGenerationServiceAbstraction } from "@bitwarden/generator-legacy";
 
 import { BrowserPlatformUtilsService } from "../../../platform/services/platform-utils/browser-platform-utils.service";
@@ -18,20 +25,28 @@ jest.mock("../../../platform/flags", () => ({
 }));
 
 describe("ExtensionLoginComponentService", () => {
+  const baseUrl = "https://webvault.bitwarden.com";
   let service: ExtensionLoginComponentService;
   let cryptoFunctionService: MockProxy<CryptoFunctionService>;
   let environmentService: MockProxy<EnvironmentService>;
   let passwordGenerationService: MockProxy<PasswordGenerationServiceAbstraction>;
   let platformUtilsService: MockProxy<BrowserPlatformUtilsService>;
   let ssoLoginService: MockProxy<SsoLoginServiceAbstraction>;
   let extensionAnonLayoutWrapperDataService: MockProxy<ExtensionAnonLayoutWrapperDataService>;
+  let ssoUrlService: MockProxy<SsoUrlService>;
   beforeEach(() => {
     cryptoFunctionService = mock<CryptoFunctionService>();
     environmentService = mock<EnvironmentService>();
     passwordGenerationService = mock<PasswordGenerationServiceAbstraction>();
     platformUtilsService = mock<BrowserPlatformUtilsService>();
     ssoLoginService = mock<SsoLoginServiceAbstraction>();
+    ssoUrlService = mock<SsoUrlService>();
     extensionAnonLayoutWrapperDataService = mock<ExtensionAnonLayoutWrapperDataService>();
+    environmentService.environment$ = new BehaviorSubject<Environment>({
+      getWebVaultUrl: () => baseUrl,
+    } as Environment);
+    platformUtilsService.getClientType.mockReturnValue(ClientType.Browser);
+
     TestBed.configureTestingModule({
       providers: [
         {
@@ -44,6 +59,7 @@ describe("ExtensionLoginComponentService", () => {
               platformUtilsService,
               ssoLoginService,
               extensionAnonLayoutWrapperDataService,
+              ssoUrlService,
             ),
         },
         { provide: DefaultLoginComponentService, useExisting: ExtensionLoginComponentService },
@@ -52,6 +68,11 @@ describe("ExtensionLoginComponentService", () => {
         { provide: PasswordGenerationServiceAbstraction, useValue: passwordGenerationService },
         { provide: PlatformUtilsService, useValue: platformUtilsService },
         { provide: SsoLoginServiceAbstraction, useValue: ssoLoginService },
+        {
+          provide: ExtensionAnonLayoutWrapperDataService,
+          useValue: extensionAnonLayoutWrapperDataService,
+        },
+        { provide: SsoUrlService, useValue: ssoUrlService },
       ],
     });
     service = TestBed.inject(ExtensionLoginComponentService);
@@ -61,6 +82,26 @@ describe("ExtensionLoginComponentService", () => {
     expect(service).toBeTruthy();
   });
 
+  describe("redirectToSso", () => {
+    it("launches SSO browser window", async () => {
+      const email = "[email protected]";
+      const state = "testState";
+      const expectedState = "testState:clientId=browser";
+      const codeVerifier = "testCodeVerifier";
+      const codeChallenge = "testCodeChallenge";
+
+      passwordGenerationService.generatePassword.mockResolvedValueOnce(state);
+      passwordGenerationService.generatePassword.mockResolvedValueOnce(codeVerifier);
+      jest.spyOn(Utils, "fromBufferToUrlB64").mockReturnValue(codeChallenge);
+
+      await service.redirectToSsoLogin(email);
+
+      expect(ssoLoginService.setSsoState).toHaveBeenCalledWith(expectedState);
+      expect(ssoLoginService.setCodeVerifier).toHaveBeenCalledWith(codeVerifier);
+      expect(platformUtilsService.launchUri).toHaveBeenCalled();
+    });
+  });
+
   describe("showBackButton", () => {
     it("sets showBackButton in extensionAnonLayoutWrapperDataService", () => {
       service.showBackButton(true);

diff --git a/apps/browser/src/auth/popup/login/extension-login-component.service.ts b/apps/browser/src/auth/popup/login/extension-login-component.service.ts
@@ -1,8 +1,10 @@
 // FIXME: Update this file to be type safe and remove this and next line
 // @ts-strict-ignore
 import { Injectable } from "@angular/core";
+import { firstValueFrom } from "rxjs";
 
 import { DefaultLoginComponentService, LoginComponentService } from "@bitwarden/auth/angular";
+import { SsoUrlService } from "@bitwarden/auth/common";
 import { SsoLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/sso-login.service.abstraction";
 import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service";
 import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
@@ -23,6 +25,7 @@ export class ExtensionLoginComponentService
     platformUtilsService: PlatformUtilsService,
     ssoLoginService: SsoLoginServiceAbstraction,
     private extensionAnonLayoutWrapperDataService: ExtensionAnonLayoutWrapperDataService,
+    private ssoUrlService: SsoUrlService,
   ) {
     super(
       cryptoFunctionService,
@@ -31,7 +34,35 @@ export class ExtensionLoginComponentService
       platformUtilsService,
       ssoLoginService,
     );
-    this.clientType = this.platformUtilsService.getClientType();
+  }
+
+  /**
+   * On the extension, redirecting to the SSO login page is done via a new browser window, opened
+   * to the SSO component on the web client.
+   * @param email the email of the user trying to log in, used to look up the org SSO identifier
+   * @param state the state that will be used to verify the SSO login, which needs to be passed to the IdP
+   * @param codeChallenge the challenge that will be verified after the code is returned from the IdP, which needs to be passed to the IdP
+   */
+  protected override async redirectToSso(
+    email: string,
+    state: string,
+    codeChallenge: string,
+  ): Promise<void> {
+    const env = await firstValueFrom(this.environmentService.environment$);
+    const webVaultUrl = env.getWebVaultUrl();
+
+    const redirectUri = webVaultUrl + "/sso-connector.html";
+
+    const webAppSsoUrl = this.ssoUrlService.buildSsoUrl(
+      webVaultUrl,
+      this.clientType,
+      redirectUri,
+      state,
+      codeChallenge,
+      email,
+    );
+
+    this.platformUtilsService.launchUri(webAppSsoUrl);
   }
 
   showBackButton(showBackButton: boolean): void {

diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts
@@ -27,7 +27,12 @@ import {
   LoginDecryptionOptionsService,
   SsoComponentService,
 } from "@bitwarden/auth/angular";
-import { LockService, LoginEmailService, PinServiceAbstraction } from "@bitwarden/auth/common";
+import {
+  LockService,
+  LoginEmailService,
+  PinServiceAbstraction,
+  SsoUrlService,
+} from "@bitwarden/auth/common";
 import { ApiService } from "@bitwarden/common/abstractions/api.service";
 import { EventCollectionService as EventCollectionServiceAbstraction } from "@bitwarden/common/abstractions/event/event-collection.service";
 import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout.service";
@@ -550,6 +555,11 @@ const safeProviders: SafeProvider[] = [
     useExisting: ExtensionAnonLayoutWrapperDataService,
     deps: [],
   }),
+  safeProvider({
+    provide: SsoUrlService,
+    useClass: SsoUrlService,
+    deps: [],
+  }),
   safeProvider({
     provide: LoginComponentService,
     useClass: ExtensionLoginComponentService,
@@ -560,6 +570,7 @@ const safeProviders: SafeProvider[] = [
       PlatformUtilsService,
       SsoLoginServiceAbstraction,
       ExtensionAnonLayoutWrapperDataService,
+      SsoUrlService,
     ],
   }),
   safeProvider({

diff --git a/apps/cli/src/auth/commands/login.command.ts b/apps/cli/src/auth/commands/login.command.ts
@@ -11,6 +11,7 @@ import {
   LoginStrategyServiceAbstraction,
   PasswordLoginCredentials,
   SsoLoginCredentials,
+  SsoUrlService,
   UserApiLoginCredentials,
 } from "@bitwarden/auth/common";
 import { ApiService } from "@bitwarden/common/abstractions/api.service";
@@ -28,6 +29,7 @@ import { TokenTwoFactorRequest } from "@bitwarden/common/auth/models/request/ide
 import { PasswordRequest } from "@bitwarden/common/auth/models/request/password.request";
 import { TwoFactorEmailRequest } from "@bitwarden/common/auth/models/request/two-factor-email.request";
 import { UpdateTempPasswordRequest } from "@bitwarden/common/auth/models/request/update-temp-password.request";
+import { ClientType } from "@bitwarden/common/enums";
 import { ErrorResponse } from "@bitwarden/common/models/response/error.response";
 import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service";
 import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
@@ -71,6 +73,7 @@ export class LoginCommand {
     protected orgService: OrganizationService,
     protected logoutCallback: () => Promise<void>,
     protected kdfConfigService: KdfConfigService,
+    protected ssoUrlService: SsoUrlService,
   ) {}
 
   async run(email: string, password: string, options: OptionValues) {
@@ -738,17 +741,14 @@ export class LoginCommand {
         try {
           this.ssoRedirectUri = "http://localhost:" + port;
           callbackServer.listen(port, () => {
-            this.platformUtilsService.launchUri(
-              webUrl +
-                "/#/sso?clientId=" +
-                "cli" +
-                "&redirectUri=" +
-                encodeURIComponent(this.ssoRedirectUri) +
-                "&state=" +
-                state +
-                "&codeChallenge=" +
-                codeChallenge,
+            const webAppSsoUrl = this.ssoUrlService.buildSsoUrl(
+              webUrl,
+              ClientType.Cli,
+              this.ssoRedirectUri,
+              state,
+              codeChallenge,
             );
+            this.platformUtilsService.launchUri(webAppSsoUrl);
           });
           foundPort = true;
           break;

diff --git a/apps/cli/src/program.ts b/apps/cli/src/program.ts
@@ -170,6 +170,7 @@ export class Program extends BaseProgram {
             this.serviceContainer.organizationService,
             async () => await this.serviceContainer.logout(),
             this.serviceContainer.kdfConfigService,
+            this.serviceContainer.ssoUrlService,
           );
           const response = await command.run(email, password, options);
           this.processResponse(response, true);

diff --git a/apps/cli/src/service-container/service-container.ts b/apps/cli/src/service-container/service-container.ts
@@ -19,6 +19,7 @@ import {
   PinService,
   PinServiceAbstraction,
   UserDecryptionOptionsService,
+  SsoUrlService,
 } from "@bitwarden/auth/common";
 import { EventCollectionService as EventCollectionServiceAbstraction } from "@bitwarden/common/abstractions/event/event-collection.service";
 import { EventUploadService as EventUploadServiceAbstraction } from "@bitwarden/common/abstractions/event/event-upload.service";
@@ -274,6 +275,7 @@ export class ServiceContainer {
   sdkService: SdkService;
   sdkLoadService: SdkLoadService;
   cipherAuthorizationService: CipherAuthorizationService;
+  ssoUrlService: SsoUrlService;
 
   constructor() {
     let p = null;
@@ -457,6 +459,7 @@ export class ServiceContainer {
 
     this.biometricStateService = new DefaultBiometricStateService(this.stateProvider);
     this.userDecryptionOptionsService = new UserDecryptionOptionsService(this.stateProvider);
+    this.ssoUrlService = new SsoUrlService();
 
     this.organizationService = new DefaultOrganizationService(this.stateProvider);
     this.policyService = new PolicyService(this.stateProvider, this.organizationService);

diff --git a/apps/desktop/src/app/app.component.ts b/apps/desktop/src/app/app.component.ts
@@ -17,7 +17,7 @@ import { CollectionService } from "@bitwarden/admin-console/common";
 import { ModalRef } from "@bitwarden/angular/components/modal/modal.ref";
 import { ModalService } from "@bitwarden/angular/services/modal.service";
 import { FingerprintDialogComponent, LoginApprovalComponent } from "@bitwarden/auth/angular";
-import { LogoutReason } from "@bitwarden/auth/common";
+import { DESKTOP_SSO_CALLBACK, LogoutReason } from "@bitwarden/auth/common";
 import { EventUploadService } from "@bitwarden/common/abstractions/event/event-upload.service";
 import { SearchService } from "@bitwarden/common/abstractions/search.service";
 import { VaultTimeoutSettingsService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout-settings.service";
@@ -299,7 +299,7 @@ export class AppComponent implements OnInit, OnDestroy {
             const queryParams = {
               code: message.code,
               state: message.state,
-              redirectUri: message.redirectUri ?? "bitwarden://sso-callback",
+              redirectUri: message.redirectUri ?? DESKTOP_SSO_CALLBACK,
             };
             // 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
@@ -812,7 +812,7 @@ export class AppComponent implements OnInit, OnDestroy {
 
     if (urlString.indexOf("bitwarden://import-callback-lp") === 0) {
       message = "importCallbackLastPass";
-    } else if (urlString.indexOf("bitwarden://sso-callback") === 0) {
+    } else if (urlString.indexOf(DESKTOP_SSO_CALLBACK) === 0) {
       message = "ssoCallback";
     }
 

diff --git a/apps/desktop/src/app/services/services.module.ts b/apps/desktop/src/app/services/services.module.ts
@@ -32,6 +32,7 @@ import {
   LoginApprovalComponentServiceAbstraction,
   LoginEmailService,
   PinServiceAbstraction,
+  SsoUrlService,
 } from "@bitwarden/auth/common";
 import { ApiService } from "@bitwarden/common/abstractions/api.service";
 import { VaultTimeoutSettingsService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout-settings.service";
@@ -378,6 +379,11 @@ const safeProviders: SafeProvider[] = [
       InternalUserDecryptionOptionsServiceAbstraction,
     ],
   }),
+  safeProvider({
+    provide: SsoUrlService,
+    useClass: SsoUrlService,
+    deps: [],
+  }),
   safeProvider({
     provide: LoginComponentService,
     useClass: DesktopLoginComponentService,
@@ -389,6 +395,7 @@ const safeProviders: SafeProvider[] = [
       SsoLoginServiceAbstraction,
       I18nServiceAbstraction,
       ToastService,
+      SsoUrlService,
     ],
   }),
   safeProvider({