[PM-3533] Support onboarding Key Connector users with existing master passwords (#6082)

* Added checks for new KeyConnector URL in all references to the legacy one.

* Updated KeyConnector logoutCallback to be a Promise

* Removed extra dependencies from KeyConnectorService

* Made the logout callback async.

* Adjusted logic to handle having a master password.

* Updated not to return error if master key is not found.

* Undid change to callback to reduce scope of this change.

* Cleaned up functions.

* Updated tests.

* Updated comments.

* Updated comments.

* Updated to use getKeyConnectorUrl helper.
This commit is contained in:
Todd Martin 2023-08-24 21:30:52 -04:00 committed by GitHub
parent 26c330ff60
commit e215828e85
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 61 additions and 20 deletions

View File

@ -572,8 +572,6 @@ import { AbstractThemingService } from "./theming/theming.service.abstraction";
LogService,
OrganizationServiceAbstraction,
CryptoFunctionServiceAbstraction,
SyncNotifierServiceAbstraction,
MessagingServiceAbstraction,
LOGOUT_CALLBACK,
],
},

View File

@ -266,11 +266,11 @@ describe("SsoLogInStrategy", () => {
describe("Key Connector", () => {
let tokenResponse: IdentityTokenResponse;
beforeEach(() => {
tokenResponse = identityTokenResponseFactory();
tokenResponse = identityTokenResponseFactory(null, { HasMasterPassword: false });
tokenResponse.keyConnectorUrl = keyConnectorUrl;
});
it("gets and sets the master key if Key Connector is enabled", async () => {
it("gets and sets the master key if Key Connector is enabled and the user doesn't have a master password", async () => {
const masterKey = new SymmetricCryptoKey(
new Uint8Array(64).buffer as CsprngArray
) as MasterKey;
@ -283,7 +283,7 @@ describe("SsoLogInStrategy", () => {
expect(keyConnectorService.setMasterKeyFromUrl).toHaveBeenCalledWith(keyConnectorUrl);
});
it("converts new SSO user to Key Connector on first login", async () => {
it("converts new SSO user with no master password to Key Connector on first login", async () => {
tokenResponse.key = null;
apiService.postIdentityToken.mockResolvedValue(tokenResponse);
@ -296,7 +296,7 @@ describe("SsoLogInStrategy", () => {
);
});
it("decrypts and sets the user key if Key Connector is enabled", async () => {
it("decrypts and sets the user key if Key Connector is enabled and the user doesn't have a master password", async () => {
const userKey = new SymmetricCryptoKey(new Uint8Array(64).buffer as CsprngArray) as UserKey;
const masterKey = new SymmetricCryptoKey(
new Uint8Array(64).buffer as CsprngArray

View File

@ -77,19 +77,50 @@ export class SsoLogInStrategy extends LogInStrategy {
}
protected override async setMasterKey(tokenResponse: IdentityTokenResponse) {
// TODO: discuss how this is no longer true with TDE
// eventually well need to support migration of existing TDE users to Key Connector
const newSsoUser = tokenResponse.key == null;
if (tokenResponse.keyConnectorUrl != null) {
if (!newSsoUser) {
await this.keyConnectorService.setMasterKeyFromUrl(tokenResponse.keyConnectorUrl);
} else {
// The only way we can be setting a master key at this point is if we are using Key Connector.
// First, check to make sure that we should do so based on the token response.
if (this.shouldSetMasterKeyFromKeyConnector(tokenResponse)) {
// If we're here, we know that the user should use Key Connector (they have a KeyConnectorUrl) and does not have a master password.
// We can now check the key on the token response to see whether they are a brand new user or an existing user.
// The presence of a masterKeyEncryptedUserKey indicates that the user has already been provisioned in Key Connector.
const newSsoUser = tokenResponse.key == null;
if (newSsoUser) {
await this.keyConnectorService.convertNewSsoUserToKeyConnector(tokenResponse, this.orgId);
} else {
const keyConnectorUrl = this.getKeyConnectorUrl(tokenResponse);
await this.keyConnectorService.setMasterKeyFromUrl(keyConnectorUrl);
}
}
}
/**
* Determines if it is possible set the `masterKey` from Key Connector.
* @param tokenResponse
* @returns `true` if the master key can be set from Key Connector, `false` otherwise
*/
private shouldSetMasterKeyFromKeyConnector(tokenResponse: IdentityTokenResponse): boolean {
const userDecryptionOptions = tokenResponse?.userDecryptionOptions;
// If the user has a master password, this means that they need to migrate to Key Connector, so we won't set the key here.
// We default to false here because old server versions won't have hasMasterPassword and in that case we want to rely solely on the keyConnectorUrl.
// TODO: remove null default after 2023.10 release (https://bitwarden.atlassian.net/browse/PM-3537)
const userHasMasterPassword = userDecryptionOptions?.hasMasterPassword ?? false;
const keyConnectorUrl = this.getKeyConnectorUrl(tokenResponse);
// In order for us to set the master key from Key Connector, we need to have a Key Connector URL
// and the user must not have a master password.
return keyConnectorUrl != null && !userHasMasterPassword;
}
private getKeyConnectorUrl(tokenResponse: IdentityTokenResponse): string {
// TODO: remove tokenResponse.keyConnectorUrl reference after 2023.10 release (https://bitwarden.atlassian.net/browse/PM-3537)
const userDecryptionOptions = tokenResponse?.userDecryptionOptions;
return (
tokenResponse.keyConnectorUrl ?? userDecryptionOptions?.keyConnectorOption?.keyConnectorUrl
);
}
// TODO: future passkey login strategy will need to support setting user key (decrypting via TDE or admin approval request)
// so might be worth moving this logic to a common place (base login strategy or a separate service?)
protected override async setUserKey(tokenResponse: IdentityTokenResponse): Promise<void> {
@ -117,9 +148,8 @@ export class SsoLogInStrategy extends LogInStrategy {
await this.trySetUserKeyWithDeviceKey(tokenResponse);
}
} else if (
// TODO: remove tokenResponse.keyConnectorUrl when it's deprecated
masterKeyEncryptedUserKey != null &&
(tokenResponse.keyConnectorUrl || userDecryptionOptions?.keyConnectorOption?.keyConnectorUrl)
this.getKeyConnectorUrl(tokenResponse) != null
) {
// Key connector enabled for user
await this.trySetUserKeyWithMasterKey();
@ -208,12 +238,15 @@ export class SsoLogInStrategy extends LogInStrategy {
private async trySetUserKeyWithMasterKey(): Promise<void> {
const masterKey = await this.cryptoService.getMasterKey();
// There is a scenario in which the master key is not set here. That will occur if the user
// has a master password and is using Key Connector. In that case, we cannot set the master key
// because the user hasn't entered their master password yet.
// Instead, we'll return here and let the migration to Key Connector handle setting the master key.
if (!masterKey) {
throw new Error("Master key not found");
return;
}
const userKey = await this.cryptoService.decryptUserKeyWithMasterKey(masterKey);
await this.cryptoService.setUserKey(userKey);
}

View File

@ -24,7 +24,7 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction {
private logService: LogService,
private organizationService: OrganizationService,
private cryptoFunctionService: CryptoFunctionService,
private logoutCallback: (expired: boolean, userId?: string) => void
private logoutCallback: (expired: boolean, userId?: string) => Promise<void>
) {}
setUsesKeyConnector(usesKeyConnector: boolean) {
@ -84,7 +84,15 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction {
}
async convertNewSsoUserToKeyConnector(tokenResponse: IdentityTokenResponse, orgId: string) {
const { kdf, kdfIterations, kdfMemory, kdfParallelism, keyConnectorUrl } = tokenResponse;
// TODO: Remove after tokenResponse.keyConnectorUrl is deprecated in 2023.10 release (https://bitwarden.atlassian.net/browse/PM-3537)
const {
kdf,
kdfIterations,
kdfMemory,
kdfParallelism,
keyConnectorUrl: legacyKeyConnectorUrl,
userDecryptionOptions,
} = tokenResponse;
const password = await this.cryptoFunctionService.randomBytes(64);
const kdfConfig = new KdfConfig(kdfIterations, kdfMemory, kdfParallelism);
@ -104,6 +112,8 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction {
const [pubKey, privKey] = await this.cryptoService.makeKeyPair();
try {
const keyConnectorUrl =
legacyKeyConnectorUrl ?? userDecryptionOptions?.keyConnectorOption?.keyConnectorUrl;
await this.apiService.postUserKeyToKeyConnector(keyConnectorUrl, keyConnectorRequest);
} catch (e) {
this.handleKeyConnectorError(e);