mirror of
https://github.com/bitwarden/browser
synced 2025-12-18 01:03:35 +00:00
[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 <mail@quexten.com>
This commit is contained in:
@@ -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";
|
||||
}
|
||||
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -3,7 +3,9 @@ import { MockProxy, mock } from "jest-mock-extended";
|
||||
import { of } 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 {
|
||||
Environment,
|
||||
@@ -41,8 +43,7 @@ describe("DesktopLoginComponentService", () => {
|
||||
let ssoLoginService: MockProxy<SsoLoginServiceAbstraction>;
|
||||
let i18nService: MockProxy<I18nService>;
|
||||
let toastService: MockProxy<ToastService>;
|
||||
|
||||
let superLaunchSsoBrowserWindowSpy: jest.SpyInstance;
|
||||
let ssoUrlService: MockProxy<SsoUrlService>;
|
||||
|
||||
beforeEach(() => {
|
||||
cryptoFunctionService = mock<CryptoFunctionService>();
|
||||
@@ -60,6 +61,8 @@ describe("DesktopLoginComponentService", () => {
|
||||
ssoLoginService = mock<SsoLoginServiceAbstraction>();
|
||||
i18nService = mock<I18nService>();
|
||||
toastService = mock<ToastService>();
|
||||
platformUtilsService.getClientType.mockReturnValue(ClientType.Desktop);
|
||||
ssoUrlService = mock<SsoUrlService>();
|
||||
|
||||
TestBed.configureTestingModule({
|
||||
providers: [
|
||||
@@ -74,6 +77,7 @@ describe("DesktopLoginComponentService", () => {
|
||||
ssoLoginService,
|
||||
i18nService,
|
||||
toastService,
|
||||
ssoUrlService,
|
||||
),
|
||||
},
|
||||
{ provide: DefaultLoginComponentService, useExisting: DesktopLoginComponentService },
|
||||
@@ -84,15 +88,11 @@ describe("DesktopLoginComponentService", () => {
|
||||
{ provide: SsoLoginServiceAbstraction, useValue: ssoLoginService },
|
||||
{ provide: I18nService, useValue: i18nService },
|
||||
{ provide: ToastService, useValue: toastService },
|
||||
{ provide: SsoUrlService, useValue: ssoUrlService },
|
||||
],
|
||||
});
|
||||
|
||||
service = TestBed.inject(DesktopLoginComponentService);
|
||||
|
||||
superLaunchSsoBrowserWindowSpy = jest.spyOn(
|
||||
DefaultLoginComponentService.prototype,
|
||||
"launchSsoBrowserWindow",
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -106,7 +106,7 @@ describe("DesktopLoginComponentService", () => {
|
||||
expect(service).toBeTruthy();
|
||||
});
|
||||
|
||||
describe("launchSsoBrowserWindow", () => {
|
||||
describe("redirectToSso", () => {
|
||||
// Array of all permutations of isAppImage, isSnapStore, and isDev
|
||||
const permutations = [
|
||||
[true, false, false], // Case 1: isAppImage true
|
||||
@@ -125,36 +125,27 @@ describe("DesktopLoginComponentService", () => {
|
||||
(global as any).ipc.platform.isSnapStore = isSnapStore;
|
||||
(global as any).ipc.platform.isDev = isDev;
|
||||
|
||||
const email = "user@example.com";
|
||||
const clientId = "desktop";
|
||||
const codeChallenge = "testCodeChallenge";
|
||||
const codeVerifier = "testCodeVerifier";
|
||||
const email = "test@bitwarden.com";
|
||||
const state = "testState";
|
||||
const codeVerifierHash = new Uint8Array(64);
|
||||
const codeVerifier = "testCodeVerifier";
|
||||
const codeChallenge = "testCodeChallenge";
|
||||
|
||||
passwordGenerationService.generatePassword.mockResolvedValueOnce(state);
|
||||
passwordGenerationService.generatePassword.mockResolvedValueOnce(codeVerifier);
|
||||
cryptoFunctionService.hash.mockResolvedValueOnce(codeVerifierHash);
|
||||
jest.spyOn(Utils, "fromBufferToUrlB64").mockReturnValue(codeChallenge);
|
||||
|
||||
await service.launchSsoBrowserWindow(email, clientId);
|
||||
await service.redirectToSsoLogin(email);
|
||||
|
||||
if (isAppImage || isSnapStore || isDev) {
|
||||
expect(superLaunchSsoBrowserWindowSpy).not.toHaveBeenCalled();
|
||||
|
||||
// Assert that the standard logic is executed
|
||||
expect(ssoLoginService.setSsoEmail).toHaveBeenCalledWith(email);
|
||||
expect(passwordGenerationService.generatePassword).toHaveBeenCalledTimes(2);
|
||||
expect(cryptoFunctionService.hash).toHaveBeenCalledWith(codeVerifier, "sha256");
|
||||
expect(ssoLoginService.setSsoState).toHaveBeenCalledWith(state);
|
||||
expect(ssoLoginService.setCodeVerifier).toHaveBeenCalledWith(codeVerifier);
|
||||
expect(ipc.platform.localhostCallbackService.openSsoPrompt).toHaveBeenCalledWith(
|
||||
codeChallenge,
|
||||
state,
|
||||
email,
|
||||
);
|
||||
} else {
|
||||
// If all values are false, expect the super method to be called
|
||||
expect(superLaunchSsoBrowserWindowSpy).toHaveBeenCalledWith(email, clientId);
|
||||
expect(ssoLoginService.setSsoState).toHaveBeenCalledWith(state);
|
||||
expect(ssoLoginService.setCodeVerifier).toHaveBeenCalledWith(codeVerifier);
|
||||
expect(platformUtilsService.launchUri).toHaveBeenCalled();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,14 +1,15 @@
|
||||
// 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 { DESKTOP_SSO_CALLBACK, 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";
|
||||
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
|
||||
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
|
||||
import { Utils } from "@bitwarden/common/platform/misc/utils";
|
||||
import { ToastService } from "@bitwarden/components";
|
||||
import { PasswordGenerationServiceAbstraction } from "@bitwarden/generator-legacy";
|
||||
|
||||
@@ -25,6 +26,7 @@ export class DesktopLoginComponentService
|
||||
protected ssoLoginService: SsoLoginServiceAbstraction,
|
||||
protected i18nService: I18nService,
|
||||
protected toastService: ToastService,
|
||||
protected ssoUrlService: SsoUrlService,
|
||||
) {
|
||||
super(
|
||||
cryptoFunctionService,
|
||||
@@ -33,38 +35,50 @@ export class DesktopLoginComponentService
|
||||
platformUtilsService,
|
||||
ssoLoginService,
|
||||
);
|
||||
this.clientType = this.platformUtilsService.getClientType();
|
||||
}
|
||||
|
||||
override async launchSsoBrowserWindow(email: string, clientId: "desktop"): Promise<void | null> {
|
||||
if (!ipc.platform.isAppImage && !ipc.platform.isSnapStore && !ipc.platform.isDev) {
|
||||
return super.launchSsoBrowserWindow(email, clientId);
|
||||
/**
|
||||
* On the desktop, 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> {
|
||||
// For platforms that cannot support a protocol-based (e.g. bitwarden://) callback, we use a localhost callback
|
||||
// Otherwise, we launch the SSO component in a browser window and wait for the callback
|
||||
if (ipc.platform.isAppImage || ipc.platform.isSnapStore || ipc.platform.isDev) {
|
||||
await this.initiateSsoThroughLocalhostCallback(email, state, codeChallenge);
|
||||
} else {
|
||||
const env = await firstValueFrom(this.environmentService.environment$);
|
||||
const webVaultUrl = env.getWebVaultUrl();
|
||||
|
||||
const redirectUri = DESKTOP_SSO_CALLBACK;
|
||||
|
||||
const ssoWebAppUrl = this.ssoUrlService.buildSsoUrl(
|
||||
webVaultUrl,
|
||||
this.clientType,
|
||||
redirectUri,
|
||||
state,
|
||||
codeChallenge,
|
||||
email,
|
||||
);
|
||||
|
||||
this.platformUtilsService.launchUri(ssoWebAppUrl);
|
||||
}
|
||||
}
|
||||
|
||||
// Save email for SSO
|
||||
await this.ssoLoginService.setSsoEmail(email);
|
||||
|
||||
// Generate SSO params
|
||||
const passwordOptions: any = {
|
||||
type: "password",
|
||||
length: 64,
|
||||
uppercase: true,
|
||||
lowercase: true,
|
||||
numbers: true,
|
||||
special: false,
|
||||
};
|
||||
|
||||
const state = await this.passwordGenerationService.generatePassword(passwordOptions);
|
||||
const codeVerifier = await this.passwordGenerationService.generatePassword(passwordOptions);
|
||||
const codeVerifierHash = await this.cryptoFunctionService.hash(codeVerifier, "sha256");
|
||||
const codeChallenge = Utils.fromBufferToUrlB64(codeVerifierHash);
|
||||
|
||||
// Save SSO params
|
||||
await this.ssoLoginService.setSsoState(state);
|
||||
await this.ssoLoginService.setCodeVerifier(codeVerifier);
|
||||
|
||||
private async initiateSsoThroughLocalhostCallback(
|
||||
email: string,
|
||||
state: string,
|
||||
challenge: string,
|
||||
): Promise<void> {
|
||||
try {
|
||||
await ipc.platform.localhostCallbackService.openSsoPrompt(codeChallenge, state);
|
||||
await ipc.platform.localhostCallbackService.openSsoPrompt(challenge, state, email);
|
||||
// FIXME: Remove when updating file. Eslint update
|
||||
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||
} catch (err) {
|
||||
|
||||
@@ -220,9 +220,10 @@ export class LoginComponentV1 extends BaseLoginComponent implements OnInit, OnDe
|
||||
if (!ipc.platform.isAppImage && !ipc.platform.isSnapStore && !ipc.platform.isDev) {
|
||||
return super.launchSsoBrowser(clientId, ssoRedirectUri);
|
||||
}
|
||||
const email = this.formGroup.controls.email.value;
|
||||
|
||||
// Save off email for SSO
|
||||
await this.ssoLoginService.setSsoEmail(this.formGroup.controls.email.value);
|
||||
await this.ssoLoginService.setSsoEmail(email);
|
||||
|
||||
// Generate necessary sso params
|
||||
const passwordOptions: any = {
|
||||
@@ -243,7 +244,7 @@ export class LoginComponentV1 extends BaseLoginComponent implements OnInit, OnDe
|
||||
await this.ssoLoginService.setCodeVerifier(ssoCodeVerifier);
|
||||
|
||||
try {
|
||||
await ipc.platform.localhostCallbackService.openSsoPrompt(codeChallenge, state);
|
||||
await ipc.platform.localhostCallbackService.openSsoPrompt(codeChallenge, state, email);
|
||||
// FIXME: Remove when updating file. Eslint update
|
||||
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||
} catch (err) {
|
||||
|
||||
@@ -5,6 +5,7 @@ import * as path from "path";
|
||||
import { app } from "electron";
|
||||
import { Subject, firstValueFrom } from "rxjs";
|
||||
|
||||
import { SsoUrlService } from "@bitwarden/auth/common";
|
||||
import { AccountServiceImplementation } from "@bitwarden/common/auth/services/account.service";
|
||||
import { ClientType } from "@bitwarden/common/enums";
|
||||
import { RegionConfig } from "@bitwarden/common/platform/abstractions/environment.service";
|
||||
@@ -66,6 +67,7 @@ export class Main {
|
||||
desktopSettingsService: DesktopSettingsService;
|
||||
mainCryptoFunctionService: MainCryptoFunctionService;
|
||||
migrationRunner: MigrationRunner;
|
||||
ssoUrlService: SsoUrlService;
|
||||
|
||||
windowMain: WindowMain;
|
||||
messagingMain: MessagingMain;
|
||||
@@ -261,7 +263,13 @@ export class Main {
|
||||
this.sshAgentService = new MainSshAgentService(this.logService, this.messagingService);
|
||||
|
||||
new EphemeralValueStorageService();
|
||||
new SSOLocalhostCallbackService(this.environmentService, this.messagingService);
|
||||
|
||||
this.ssoUrlService = new SsoUrlService();
|
||||
new SSOLocalhostCallbackService(
|
||||
this.environmentService,
|
||||
this.messagingService,
|
||||
this.ssoUrlService,
|
||||
);
|
||||
|
||||
this.nativeAutofillMain = new NativeAutofillMain(this.logService, this.windowMain);
|
||||
void this.nativeAutofillMain.init();
|
||||
|
||||
@@ -127,8 +127,8 @@ const ephemeralStore = {
|
||||
};
|
||||
|
||||
const localhostCallbackService = {
|
||||
openSsoPrompt: (codeChallenge: string, state: string): Promise<void> => {
|
||||
return ipcRenderer.invoke("openSsoPrompt", { codeChallenge, state });
|
||||
openSsoPrompt: (codeChallenge: string, state: string, email: string): Promise<void> => {
|
||||
return ipcRenderer.invoke("openSsoPrompt", { codeChallenge, state, email });
|
||||
},
|
||||
};
|
||||
|
||||
|
||||
@@ -5,6 +5,8 @@ import * as http from "http";
|
||||
import { ipcMain } from "electron";
|
||||
import { firstValueFrom } from "rxjs";
|
||||
|
||||
import { SsoUrlService } from "@bitwarden/auth/common";
|
||||
import { ClientType } from "@bitwarden/common/enums";
|
||||
import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
|
||||
import { MessageSender } from "@bitwarden/common/platform/messaging";
|
||||
|
||||
@@ -18,9 +20,10 @@ export class SSOLocalhostCallbackService {
|
||||
constructor(
|
||||
private environmentService: EnvironmentService,
|
||||
private messagingService: MessageSender,
|
||||
private ssoUrlService: SsoUrlService,
|
||||
) {
|
||||
ipcMain.handle("openSsoPrompt", async (event, { codeChallenge, state }) => {
|
||||
const { ssoCode, recvState } = await this.openSsoPrompt(codeChallenge, state);
|
||||
ipcMain.handle("openSsoPrompt", async (event, { codeChallenge, state, email }) => {
|
||||
const { ssoCode, recvState } = await this.openSsoPrompt(codeChallenge, state, email);
|
||||
this.messagingService.send("ssoCallback", {
|
||||
code: ssoCode,
|
||||
state: recvState,
|
||||
@@ -32,6 +35,7 @@ export class SSOLocalhostCallbackService {
|
||||
private async openSsoPrompt(
|
||||
codeChallenge: string,
|
||||
state: string,
|
||||
email: string,
|
||||
): Promise<{ ssoCode: string; recvState: string }> {
|
||||
const env = await firstValueFrom(this.environmentService.environment$);
|
||||
|
||||
@@ -78,18 +82,17 @@ export class SSOLocalhostCallbackService {
|
||||
for (let port = 8065; port <= 8070; port++) {
|
||||
try {
|
||||
this.ssoRedirectUri = "http://localhost:" + port;
|
||||
const ssoUrl = this.ssoUrlService.buildSsoUrl(
|
||||
webUrl,
|
||||
ClientType.Desktop,
|
||||
this.ssoRedirectUri,
|
||||
state,
|
||||
codeChallenge,
|
||||
email,
|
||||
);
|
||||
callbackServer.listen(port, () => {
|
||||
this.messagingService.send("launchUri", {
|
||||
url:
|
||||
webUrl +
|
||||
"/#/sso?clientId=" +
|
||||
"desktop" +
|
||||
"&redirectUri=" +
|
||||
encodeURIComponent(this.ssoRedirectUri) +
|
||||
"&state=" +
|
||||
state +
|
||||
"&codeChallenge=" +
|
||||
codeChallenge,
|
||||
url: ssoUrl,
|
||||
});
|
||||
});
|
||||
foundPort = true;
|
||||
@@ -112,15 +115,6 @@ export class SSOLocalhostCallbackService {
|
||||
});
|
||||
}
|
||||
|
||||
private getOrgIdentifierFromState(state: string): string {
|
||||
if (state === null || state === undefined) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const stateSplit = state.split("_identifier=");
|
||||
return stateSplit.length > 1 ? stateSplit[1] : null;
|
||||
}
|
||||
|
||||
private checkState(state: string, checkState: string): boolean {
|
||||
if (state === null || state === undefined) {
|
||||
return false;
|
||||
|
||||
Reference in New Issue
Block a user