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

Auth - PM-7392 & PM-7436 - Token Service - Desktop - Add disk fallback for secure storage failures #8913

Merged
Show file tree
Hide file tree
Changes from 96 commits
Commits
Show all changes
106 commits
Select commit Hold shift + click to select a range
093f9cd
PM-7392 - EncryptSvc - add new method for detecting if a simple strin…
JaredSnider-Bitwarden Apr 25, 2024
aba9dee
PM-7392 - TokenSvc - add checks when setting and retrieving the acces…
JaredSnider-Bitwarden Apr 25, 2024
814cf07
PM-7392 - (1) Clean up token svc (2) export access token key type for…
JaredSnider-Bitwarden Apr 25, 2024
e05e839
PM-7392 - Get token svc tests passing; WIP more tests to come for new…
JaredSnider-Bitwarden Apr 25, 2024
51f38d0
Merge branch 'main' into auth/pm-7392/token-service-add-secure-storag…
JaredSnider-Bitwarden May 1, 2024
911613a
PM-7392 - Access token secure storage to disk fallback WIP but mostly…
JaredSnider-Bitwarden May 1, 2024
3c3b529
PM-7392 - Clean up unnecessary comment
JaredSnider-Bitwarden May 2, 2024
0188a51
Merge branch 'main' into auth/pm-7392/token-service-add-secure-storag…
JaredSnider-Bitwarden May 2, 2024
180e658
PM-7392 - TokenSvc - refresh token disk storage fallback
JaredSnider-Bitwarden May 2, 2024
7ac1c19
PM-7392 - Fix token service tests in prep for adding tests for new sc…
JaredSnider-Bitwarden May 2, 2024
cd76803
PM-7392 - TokenSvc tests - Test new setRefreshToken scenarios
JaredSnider-Bitwarden May 2, 2024
3e9f904
PM-7392 - TokenSvc - getRefreshToken should return null or a value - …
JaredSnider-Bitwarden May 2, 2024
6b2a032
PM-7392 - Fix test name.
JaredSnider-Bitwarden May 2, 2024
e4d31d5
PM-7392 - TokenSvc tests - clean up test names that reference removed…
JaredSnider-Bitwarden May 2, 2024
ffb8065
PM-7392 - getRefreshToken tests done.
JaredSnider-Bitwarden May 2, 2024
e6beb0c
PM-7392 - Fix error quote
JaredSnider-Bitwarden May 2, 2024
e0a77c6
PM-7392 - TokenSvc tests - setAccessToken new scenarios tested.
JaredSnider-Bitwarden May 2, 2024
e6e017c
PM-7392 - TokenSvc - getAccessToken - if secure storage errors add er…
JaredSnider-Bitwarden May 3, 2024
87c8520
PM-7392 - TokenSvc tests - getAccessToken - all new scenarios tested
JaredSnider-Bitwarden May 3, 2024
1db1728
PM-7392 - EncryptSvc - test new stringIsEncString method
JaredSnider-Bitwarden May 3, 2024
dbbf100
PM-7392 - Main.ts - fix circ dep issue.
JaredSnider-Bitwarden May 3, 2024
7416fec
Merge branch 'main' into auth/pm-7392/token-service-add-secure-storag…
JaredSnider-Bitwarden May 3, 2024
6456670
PM-7392 - Main.ts - remove comment.
JaredSnider-Bitwarden May 3, 2024
e9d2f3f
PM-7392 - Don't re-invent the wheel and simply use existing isSeriali…
JaredSnider-Bitwarden May 3, 2024
8c8c444
PM-7392 - Enc String - (1) Add handling for Nan in parseEncryptedStri…
JaredSnider-Bitwarden May 3, 2024
ee6b1a3
PM-7392 - Remove encrypt service method
JaredSnider-Bitwarden May 3, 2024
3885608
PM-7392 - Actually fix circ dep issues with Justin. Ty!
JaredSnider-Bitwarden May 3, 2024
c2295e2
PM-7392 - TokenSvc - update to use EncString instead of EncryptSvc + …
JaredSnider-Bitwarden May 3, 2024
457e7a4
Merge branch 'main' into auth/pm-7392/token-service-add-secure-storag…
JaredSnider-Bitwarden May 3, 2024
1b372b4
PM-7392 - TokenSvc - (1) Remove test code (2) Refactor decryptAccessT…
JaredSnider-Bitwarden May 3, 2024
ec01624
Merge remote-tracking branch 'origin/main' into auth/pm-7392/token-se…
JaredSnider-Bitwarden May 6, 2024
f7a0892
PM-7392 - Per PR feedback and discussion, do not log the user out if …
JaredSnider-Bitwarden May 6, 2024
11f7024
PM-7392 - API service - update doAuthRefresh error to clarify which t…
JaredSnider-Bitwarden May 6, 2024
843e045
PM-7392 - Fix SetRefreshToken case where a null input would incorrect…
JaredSnider-Bitwarden May 7, 2024
1acb5d0
PM-7392 - If the access token cannot be refreshed due to a missing re…
JaredSnider-Bitwarden May 7, 2024
c2bea80
PM-7392 - Fix CLI build errors
JaredSnider-Bitwarden May 7, 2024
5e2cb48
PM-7392 - Per PR feedback, add missing tests (thank you Jake for writ…
JaredSnider-Bitwarden May 7, 2024
b5508ec
PM-7392 - Per PR feedback, update incorrect comment from 3 releases t…
JaredSnider-Bitwarden May 7, 2024
bee849d
PM-7392 - Per PR feedback, remove links.
JaredSnider-Bitwarden May 7, 2024
20bc6d7
PM-7392 - Per PR feedback, move tests to existing describe.
JaredSnider-Bitwarden May 7, 2024
923c6cf
PM-7392 - Per PR feedback, adjust all test names to match naming conv…
JaredSnider-Bitwarden May 7, 2024
935c97f
PM-7392 - ApiService - refreshIdentityToken - log error before swallo…
JaredSnider-Bitwarden May 7, 2024
c5b8071
PM-7392 - Fix copy for errorRefreshingAccessToken
JaredSnider-Bitwarden May 7, 2024
28d2a87
PM-7392 - Per PR feedback, move error handling toast responsibility t…
JaredSnider-Bitwarden May 7, 2024
26eb132
PM-7392 - Swap logout reason from enum to type.
JaredSnider-Bitwarden May 7, 2024
ed6f2e4
PM-7392 - ApiService - Stop using messaging to trigger toast to let u…
JaredSnider-Bitwarden May 7, 2024
4d8d43b
PM-7392 - Per PR feedback, adjust enc string changes and tests.
JaredSnider-Bitwarden May 7, 2024
0ef0bd3
PM-7392 - Rename file to be type from enum
JaredSnider-Bitwarden May 7, 2024
2f414d6
PM-7392 - ToastService - we need to await the activeToast.onHidden ob…
JaredSnider-Bitwarden May 7, 2024
00cac8e
PM-7392 - Desktop AppComp - cleanup messaging
JaredSnider-Bitwarden May 7, 2024
ec8f336
PM-7392 - Move Logout reason custom type to auth/common
JaredSnider-Bitwarden May 7, 2024
d078255
Merge branch 'main' into auth/pm-7392/token-service-add-secure-storag…
JaredSnider-Bitwarden May 7, 2024
7069350
PM-7392 - WIP - Enhancing logout callback to consider the logout reas…
JaredSnider-Bitwarden May 8, 2024
18d84bd
PM-7392 - Logout callback should simply pass along the LogoutReason i…
JaredSnider-Bitwarden May 8, 2024
7aae92e
PM-7392 - More replacements of expired with logoutReason
JaredSnider-Bitwarden May 8, 2024
6f5439b
PM-7392 - More expired to logoutReason replacements
JaredSnider-Bitwarden May 8, 2024
5d85de8
PM-7392 - Build new handlers for displaying the logout reason for des…
JaredSnider-Bitwarden May 8, 2024
d88766a
PM-7392 - Revert ToastService changes
JaredSnider-Bitwarden May 8, 2024
bcf9099
PM-7392 - TokenSvc - Replace messageSender with logout callback per P…
JaredSnider-Bitwarden May 8, 2024
f731e9b
PM-7392 - Desktop App comp - replace toast usage with simple dialog t…
JaredSnider-Bitwarden May 8, 2024
24e31fe
PM-7392 - Web app comp - fix issue
JaredSnider-Bitwarden May 8, 2024
e75d602
PM-7392 - Desktop App comp - don't show cancel btn on simple dialogs.
JaredSnider-Bitwarden May 8, 2024
628190b
PM-7392 - Desktop App comp - Don't open n simple dialogs.
JaredSnider-Bitwarden May 9, 2024
0788c35
PM-7392 - Fix browser build
JaredSnider-Bitwarden May 9, 2024
c700e05
PM-7392 - Remove logout reason from CLI as each logout call handles m…
JaredSnider-Bitwarden May 9, 2024
5791be9
Merge branch 'main' into auth/pm-7392/token-service-add-secure-storag…
JaredSnider-Bitwarden May 9, 2024
1e6f187
PM-7392 - Previously, if a security stamp was invalid, the session wa…
JaredSnider-Bitwarden May 9, 2024
d088ec6
Merge branch 'auth/pm-7392/token-service-add-secure-storage-fallback'…
JaredSnider-Bitwarden May 9, 2024
52b83fe
PM-7392 - Update sync service logoutCallback to include optional user…
JaredSnider-Bitwarden May 9, 2024
4451947
Merge branch 'main' into auth/pm-7392/token-service-add-secure-storag…
JaredSnider-Bitwarden May 9, 2024
dceeedd
PM-7392 - Clean up web app comp
JaredSnider-Bitwarden May 9, 2024
775598f
PM-7392 - Web - app comp - only handle actually possible web logout s…
JaredSnider-Bitwarden May 9, 2024
89d2d5e
PM-7392 - Browser Popup app comp - restore done logging out message f…
JaredSnider-Bitwarden May 9, 2024
279ac75
PM-7392 - Add optional user id to logout callbacks.
JaredSnider-Bitwarden May 9, 2024
ff3b0dd
PM-7392 - Main.background.ts - add clarifying comment.
JaredSnider-Bitwarden May 9, 2024
0d385c6
PM-7392 - Per feedback, use danger simple dialog type for error.
JaredSnider-Bitwarden May 9, 2024
870e04b
PM-7392 - Browser Popup - add comment clarifying expectation of seein…
JaredSnider-Bitwarden May 9, 2024
d79faad
PM-7392 - Consolidate invalidSecurityStamp error handling
JaredSnider-Bitwarden May 13, 2024
4ded9e8
PM-7392 - Per PR feedback, REFRESH_ACCESS_TOKEN_ERROR_CALLBACK can be…
JaredSnider-Bitwarden May 13, 2024
dd6d301
PM-7392 - Per PR feedback, use a named callback for refreshAccessToke…
JaredSnider-Bitwarden May 13, 2024
e4981d6
PM-7392 - Add TODO
JaredSnider-Bitwarden May 13, 2024
48fabc1
Merge remote-tracking branch 'origin/main' into auth/pm-7392/token-se…
JaredSnider-Bitwarden May 13, 2024
e473456
Merge branch 'main' into auth/pm-7392/token-service-add-secure-storag…
JaredSnider-Bitwarden May 13, 2024
eaec61d
Merge branch 'main' into auth/pm-7392/token-service-add-secure-storag…
JaredSnider-Bitwarden May 14, 2024
dfbb615
Merge remote-tracking branch 'origin/main' into auth/pm-7392/token-se…
JaredSnider-Bitwarden May 15, 2024
37b0c22
PM-7392 - Re-apply bw.ts changes to new service-container.
JaredSnider-Bitwarden May 15, 2024
6335cb5
Merge branch 'main' into auth/pm-7392/token-service-add-secure-storag…
JaredSnider-Bitwarden May 15, 2024
60ebf71
Merge branch 'main' into auth/pm-7392/token-service-add-secure-storag…
JaredSnider-Bitwarden May 16, 2024
fa5db1a
Merge branch 'main' into auth/pm-7392/token-service-add-secure-storag…
JaredSnider-Bitwarden May 17, 2024
99dc59f
Merge remote-tracking branch 'origin/main' into auth/pm-7392/token-se…
JaredSnider-Bitwarden May 20, 2024
f91c2ae
Merge branch 'main' into auth/pm-7392/token-service-add-secure-storag…
JaredSnider-Bitwarden May 21, 2024
1178bff
Merge branch 'main' into auth/pm-7392/token-service-add-secure-storag…
JaredSnider-Bitwarden May 22, 2024
01e417c
Merge branch 'main' into auth/pm-7392/token-service-add-secure-storag…
JaredSnider-Bitwarden May 22, 2024
c07750e
PM-7392 - TokenSvc - tweak error message.
JaredSnider-Bitwarden May 28, 2024
225fcd3
PM-7392 - Fix test
JaredSnider-Bitwarden May 28, 2024
81e919c
Merge branch 'main' into auth/pm-7392/token-service-add-secure-storag…
JaredSnider-Bitwarden May 28, 2024
9fe18c5
Merge remote-tracking branch 'origin/main' into auth/pm-7392/token-se…
JaredSnider-Bitwarden May 29, 2024
b749da2
PM-7392 - Clean up merge conflict where I duplicated dependencies.
JaredSnider-Bitwarden May 29, 2024
03388ae
PM-7392 - Per discussion with product, change default logout toast to…
JaredSnider-Bitwarden May 30, 2024
931cdaf
Merge branch 'main' into auth/pm-7392/token-service-add-secure-storag…
JaredSnider-Bitwarden May 30, 2024
2f9f595
Merge branch 'auth/pm-7392/token-service-add-secure-storage-fallback'…
JaredSnider-Bitwarden May 30, 2024
eed6555
PM-7392 - After merge, add new logout reason to sync service.
JaredSnider-Bitwarden May 30, 2024
6489742
Merge branch 'main' into auth/pm-7392/token-service-add-secure-storag…
JaredSnider-Bitwarden May 30, 2024
d614360
Merge branch 'main' into auth/pm-7392/token-service-add-secure-storag…
JaredSnider-Bitwarden May 31, 2024
e76dad4
PM-7392 - Remove default logout message per discussion with product s…
JaredSnider-Bitwarden May 31, 2024
895c825
PM-7392 - address PR feedback.
JaredSnider-Bitwarden Jun 3, 2024
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
9 changes: 9 additions & 0 deletions apps/browser/src/_locales/en/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -599,6 +599,9 @@
"loggedOut": {
"message": "Logged out"
},
"loggedOutDesc": {
"message": "You have been logged out of your account."
},
"loginExpired": {
"message": "Your login session has expired."
},
Expand Down Expand Up @@ -1714,6 +1717,12 @@
"ok": {
"message": "Ok"
},
"errorRefreshingAccessToken":{
"message": "Access Token Refresh Error"
},
"errorRefreshingAccessTokenDesc":{
"message": "No refresh token or API keys found. Please try logging out and logging back in."
},
"desktopSyncVerificationTitle": {
"message": "Desktop sync verification"
},
Expand Down
24 changes: 19 additions & 5 deletions apps/browser/src/background/main.background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
AuthRequestService,
LoginEmailServiceAbstraction,
LoginEmailService,
LogoutReason,
} from "@bitwarden/auth/common";
import { ApiService as ApiServiceAbstraction } from "@bitwarden/common/abstractions/api.service";
import { AuditService as AuditServiceAbstraction } from "@bitwarden/common/abstractions/audit.service";
Expand Down Expand Up @@ -375,8 +376,17 @@
}
};

const logoutCallback = async (expired: boolean, userId?: UserId) =>
await this.logout(expired, userId);
const logoutCallback = async (logoutReason: LogoutReason, userId?: UserId) =>
await this.logout(logoutReason, userId);

Check warning on line 380 in apps/browser/src/background/main.background.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/background/main.background.ts#L379-L380

Added lines #L379 - L380 were not covered by tests

const refreshAccessTokenErrorCallback = () => {

Check warning on line 382 in apps/browser/src/background/main.background.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/background/main.background.ts#L382

Added line #L382 was not covered by tests
// Send toast to popup
this.messagingService.send("showToast", {

Check warning on line 384 in apps/browser/src/background/main.background.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/background/main.background.ts#L384

Added line #L384 was not covered by tests
type: "error",
title: this.i18nService.t("errorRefreshingAccessToken"),
message: this.i18nService.t("errorRefreshingAccessTokenDesc"),
});
};

const isDev = process.env.ENV === "development";
this.logService = new ConsoleLogService(isDev);
Expand Down Expand Up @@ -523,6 +533,7 @@
this.keyGenerationService,
this.encryptService,
this.logService,
logoutCallback,
);

const migrationRunner = new MigrationRunner(
Expand Down Expand Up @@ -608,9 +619,12 @@
this.platformUtilsService,
this.environmentService,
this.appIdService,
refreshAccessTokenErrorCallback,
this.logService,
(logoutReason: LogoutReason, userId?: UserId) => this.logout(logoutReason, userId),

Check warning on line 624 in apps/browser/src/background/main.background.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/background/main.background.ts#L624

Added line #L624 was not covered by tests
this.vaultTimeoutSettingsService,
(expired: boolean) => this.logout(expired),
);

this.domainSettingsService = new DefaultDomainSettingsService(this.stateProvider);
this.fileUploadService = new FileUploadService(this.logService);
this.cipherFileUploadService = new CipherFileUploadService(
Expand Down Expand Up @@ -1277,7 +1291,7 @@
}
}

async logout(expired: boolean, userId?: UserId) {
async logout(logoutReason: LogoutReason, userId?: UserId) {
const activeUserId = await firstValueFrom(
this.accountService.activeAccount$.pipe(
map((a) => a?.id),
Expand Down Expand Up @@ -1343,7 +1357,7 @@
await logoutPromise;

this.messagingService.send("doneLoggingOut", {
expired: expired,
logoutReason: logoutReason,
userId: userBeingLoggedOut,
});

Expand Down
44 changes: 37 additions & 7 deletions apps/browser/src/popup/app.component.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { ChangeDetectorRef, Component, NgZone, OnDestroy, OnInit } from "@angular/core";

Check notice on line 1 in apps/browser/src/popup/app.component.ts

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

✅ Getting better: Overall Code Complexity

The mean cyclomatic complexity decreases from 5.00 to 4.78, threshold = 4. This file has many conditional statements (e.g. if, for, while) across its implementation, leading to lower code health. Avoid adding more conditionals.
import { NavigationEnd, Router, RouterOutlet } from "@angular/router";
import { Subject, takeUntil, firstValueFrom, concatMap, filter, tap } from "rxjs";

import { LogoutReason } from "@bitwarden/auth/common";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
Expand All @@ -10,7 +11,12 @@
import { MessageListener } from "@bitwarden/common/platform/messaging";
import { UserId } from "@bitwarden/common/types/guid";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { DialogService, SimpleDialogOptions, ToastService } from "@bitwarden/components";
import {

Check warning on line 14 in apps/browser/src/popup/app.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/popup/app.component.ts#L14

Added line #L14 was not covered by tests
DialogService,
SimpleDialogOptions,
ToastOptions,
ToastService,
} from "@bitwarden/components";

import { BrowserApi } from "../platform/browser/browser-api";
import { BrowserStateService } from "../platform/services/abstractions/browser-state.service";
Expand Down Expand Up @@ -83,13 +89,10 @@
.pipe(
tap((msg: any) => {
if (msg.command === "doneLoggingOut") {
// TODO: why do we call logout in the popup after receiving the doneLoggingOut message? Hasn't this already completeted logout?
this.authService.logOut(async () => {
if (msg.expired) {
this.toastService.showToast({
variant: "warning",
title: this.i18nService.t("loggedOut"),
message: this.i18nService.t("loginExpired"),
});
if (msg.logoutReason) {
await this.displayLogoutReason(msg.logoutReason);

Check warning on line 95 in apps/browser/src/popup/app.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/popup/app.component.ts#L95

Added line #L95 was not covered by tests
}
});
this.changeDetectorRef.detectChanges();
Expand Down Expand Up @@ -233,4 +236,31 @@
this.browserSendStateService.setBrowserSendTypeComponentState(null),
]);
}

// Displaying toasts isn't super useful on the popup due to the reloads we do.
// However, it is visible for a moment on the FF sidebar logout.
private async displayLogoutReason(logoutReason: LogoutReason) {
let toastOptions: ToastOptions;
switch (logoutReason) {
case "invalidSecurityStamp":
case "sessionExpired": {
toastOptions = {

Check warning on line 247 in apps/browser/src/popup/app.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/popup/app.component.ts#L247

Added line #L247 was not covered by tests
variant: "warning",
title: this.i18nService.t("loggedOut"),
message: this.i18nService.t("loginExpired"),
};
break;

Check warning on line 252 in apps/browser/src/popup/app.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/popup/app.component.ts#L252

Added line #L252 was not covered by tests
}
default: {
toastOptions = {

Check warning on line 255 in apps/browser/src/popup/app.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/popup/app.component.ts#L255

Added line #L255 was not covered by tests
variant: "warning",
title: this.i18nService.t("loggedOut"),
message: this.i18nService.t("loggedOutDesc"),
};
break;

Check warning on line 260 in apps/browser/src/popup/app.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/popup/app.component.ts#L260

Added line #L260 was not covered by tests
}
}

this.toastService.showToast(toastOptions);

Check warning on line 264 in apps/browser/src/popup/app.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/popup/app.component.ts#L264

Added line #L264 was not covered by tests
}
}
9 changes: 7 additions & 2 deletions apps/cli/src/platform/services/node-api.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import { TokenService } from "@bitwarden/common/auth/abstractions/token.service";
import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service";
import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
import { ApiService } from "@bitwarden/common/services/api.service";

Expand All @@ -21,17 +22,21 @@
platformUtilsService: PlatformUtilsService,
environmentService: EnvironmentService,
appIdService: AppIdService,
refreshAccessTokenErrorCallback: () => Promise<void>,
logService: LogService,
logoutCallback: () => Promise<void>,
vaultTimeoutSettingsService: VaultTimeoutSettingsService,
logoutCallback: (expired: boolean) => Promise<void>,
customUserAgent: string = null,
) {
super(
tokenService,
platformUtilsService,
environmentService,
appIdService,
vaultTimeoutSettingsService,
refreshAccessTokenErrorCallback,
logService,
logoutCallback,
vaultTimeoutSettingsService,

Check notice on line 39 in apps/cli/src/platform/services/node-api.service.ts

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

ℹ Getting worse: Excess Number of Function Arguments

NodeApiService.constructor increases from 7 to 9 arguments, threshold = 4. This function has too many arguments, indicating a lack of encapsulation. Avoid adding more arguments.
customUserAgent,
);
}
Expand Down
15 changes: 12 additions & 3 deletions apps/cli/src/service-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,179 +255,188 @@
p = path.join(process.env.HOME, ".config/Bitwarden CLI");
}

const logoutCallback = async () => await this.logout();

this.platformUtilsService = new CliPlatformUtilsService(ClientType.Cli, packageJson);
this.logService = new ConsoleLogService(
this.platformUtilsService.isDev(),
(level) => process.env.BITWARDENCLI_DEBUG !== "true" && level <= LogLevelType.Info,
);
this.cryptoFunctionService = new NodeCryptoFunctionService();
this.encryptService = new EncryptServiceImplementation(
this.cryptoFunctionService,
this.logService,
true,
);
this.storageService = new LowdbStorageService(this.logService, null, p, false, true);
this.secureStorageService = new NodeEnvSecureStorageService(
this.storageService,
this.logService,
this.encryptService,
);

this.memoryStorageService = new MemoryStorageService();
this.memoryStorageForStateProviders = new MemoryStorageServiceForStateProviders();

const storageServiceProvider = new StorageServiceProvider(
this.storageService,
this.memoryStorageForStateProviders,
);

this.globalStateProvider = new DefaultGlobalStateProvider(storageServiceProvider);

const stateEventRegistrarService = new StateEventRegistrarService(
this.globalStateProvider,
storageServiceProvider,
);

this.stateEventRunnerService = new StateEventRunnerService(
this.globalStateProvider,
storageServiceProvider,
);

this.i18nService = new I18nService("en", "./locales", this.globalStateProvider);

this.singleUserStateProvider = new DefaultSingleUserStateProvider(
storageServiceProvider,
stateEventRegistrarService,
);

this.messagingService = MessageSender.EMPTY;

this.accountService = new AccountServiceImplementation(
this.messagingService,
this.logService,
this.globalStateProvider,
);

this.activeUserStateProvider = new DefaultActiveUserStateProvider(
this.accountService,
this.singleUserStateProvider,
);

this.derivedStateProvider = new DefaultDerivedStateProvider();

this.stateProvider = new DefaultStateProvider(
this.activeUserStateProvider,
this.singleUserStateProvider,
this.globalStateProvider,
this.derivedStateProvider,
);

this.environmentService = new DefaultEnvironmentService(
this.stateProvider,
this.accountService,
);

this.keyGenerationService = new KeyGenerationService(this.cryptoFunctionService);

this.tokenService = new TokenService(
this.singleUserStateProvider,
this.globalStateProvider,
this.platformUtilsService.supportsSecureStorage(),
this.secureStorageService,
this.keyGenerationService,
this.encryptService,
this.logService,
logoutCallback,
);

const migrationRunner = new MigrationRunner(
this.storageService,
this.logService,
new MigrationBuilderService(),
ClientType.Cli,
);

this.stateService = new StateService(
this.storageService,
this.secureStorageService,
this.memoryStorageService,
this.logService,
new StateFactory(GlobalState, Account),
this.accountService,
this.environmentService,
this.tokenService,
migrationRunner,
);

this.masterPasswordService = new MasterPasswordService(
this.stateProvider,
this.stateService,
this.keyGenerationService,
this.encryptService,
);

this.kdfConfigService = new KdfConfigService(this.stateProvider);

this.pinService = new PinService(
this.accountService,
this.cryptoFunctionService,
this.encryptService,
this.kdfConfigService,
this.keyGenerationService,
this.logService,
this.masterPasswordService,
this.stateProvider,
this.stateService,
);

this.cryptoService = new CryptoService(
this.pinService,
this.masterPasswordService,
this.keyGenerationService,
this.cryptoFunctionService,
this.encryptService,
this.platformUtilsService,
this.logService,
this.stateService,
this.accountService,
this.stateProvider,
this.kdfConfigService,
);

this.appIdService = new AppIdService(this.globalStateProvider);

const customUserAgent =
"Bitwarden_CLI/" +
this.platformUtilsService.getApplicationVersionSync() +
" (" +
this.platformUtilsService.getDeviceString().toUpperCase() +
")";

this.biometricStateService = new DefaultBiometricStateService(this.stateProvider);
this.userDecryptionOptionsService = new UserDecryptionOptionsService(this.stateProvider);

this.organizationService = new OrganizationService(this.stateProvider);
this.policyService = new PolicyService(this.stateProvider, this.organizationService);

this.vaultTimeoutSettingsService = new VaultTimeoutSettingsService(
this.accountService,
this.pinService,
this.userDecryptionOptionsService,
this.cryptoService,
this.tokenService,
this.policyService,
this.biometricStateService,
this.stateProvider,
this.logService,
VaultTimeoutStringType.Never, // default vault timeout
);

const refreshAccessTokenErrorCallback = () => {
throw new Error("Refresh Access token error");

Check warning on line 428 in apps/cli/src/service-container.ts

View check run for this annotation

Codecov / codecov/patch

apps/cli/src/service-container.ts#L428

Added line #L428 was not covered by tests
};

this.apiService = new NodeApiService(
this.tokenService,
this.platformUtilsService,
this.environmentService,
this.appIdService,
refreshAccessTokenErrorCallback,
this.logService,
logoutCallback,
this.vaultTimeoutSettingsService,
async (expired: boolean) => await this.logout(),
customUserAgent,
);

Expand Down Expand Up @@ -485,7 +494,7 @@
this.logService,
this.organizationService,
this.keyGenerationService,
async (expired: boolean) => await this.logout(),
logoutCallback,
this.stateProvider,
);

Expand Down Expand Up @@ -660,7 +669,7 @@
this.sendApiService,
this.userDecryptionOptionsService,
this.avatarService,
async (expired: boolean) => await this.logout(),
logoutCallback,

Check warning on line 672 in apps/cli/src/service-container.ts

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ Getting worse: Large Method

ServiceContainer.constructor increases from 416 to 423 lines of code, threshold = 70. Large functions with many lines of code are generally harder to understand and lower the code health. Avoid adding more lines to this function.
this.billingAccountProfileStateService,
this.tokenService,
this.authService,
Expand Down
93 changes: 80 additions & 13 deletions apps/desktop/src/app/app.component.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { DialogRef } from "@angular/cdk/dialog";

Check notice on line 1 in apps/desktop/src/app/app.component.ts

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

ℹ Getting worse: Overall Code Complexity

The mean cyclomatic complexity increases from 6.63 to 6.65, threshold = 4. This file has many conditional statements (e.g. if, for, while) across its implementation, leading to lower code health. Avoid adding more conditionals.
import {
Component,
NgZone,
Expand All @@ -13,6 +14,7 @@
import { ModalRef } from "@bitwarden/angular/components/modal/modal.ref";
import { ModalService } from "@bitwarden/angular/services/modal.service";
import { FingerprintDialogComponent } from "@bitwarden/auth/angular";
import { LogoutReason } from "@bitwarden/auth/common";
import { EventUploadService } from "@bitwarden/common/abstractions/event/event-upload.service";
import { NotificationsService } from "@bitwarden/common/abstractions/notifications.service";
import { SearchService } from "@bitwarden/common/abstractions/search.service";
Expand Down Expand Up @@ -48,7 +50,7 @@
import { InternalFolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction";
import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction";
import { CipherType } from "@bitwarden/common/vault/enums";
import { DialogService, ToastService } from "@bitwarden/components";
import { DialogService, ToastOptions, ToastService } from "@bitwarden/components";

Check warning on line 53 in apps/desktop/src/app/app.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/app.component.ts#L53

Added line #L53 was not covered by tests

import { DeleteAccountComponent } from "../auth/delete-account.component";
import { LoginApprovalComponent } from "../auth/login/login-approval.component";
Expand Down Expand Up @@ -108,6 +110,7 @@
private idleTimer: number = null;
private isIdle = false;
private activeUserId: UserId = null;
private activeSimpleDialog: DialogRef<boolean> = null;

Check warning on line 113 in apps/desktop/src/app/app.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/app.component.ts#L113

Added line #L113 was not covered by tests

private destroy$ = new Subject<void>();

Expand Down Expand Up @@ -207,7 +210,7 @@
break;
case "logout":
this.loading = message.userId == null || message.userId === this.activeUserId;
await this.logOut(!!message.expired, message.userId);
await this.logOut(message.logoutReason, message.userId);

Check warning on line 213 in apps/desktop/src/app/app.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/app.component.ts#L213

Added line #L213 was not covered by tests
this.loading = false;
break;
case "lockVault":
Expand Down Expand Up @@ -565,9 +568,81 @@
this.messagingService.send("updateAppMenu", { updateRequest: updateRequest });
}

private async displayLogoutReason(logoutReason: LogoutReason) {
let toastOptions: ToastOptions;

switch (logoutReason) {
case "invalidSecurityStamp":
case "sessionExpired": {
toastOptions = {

Check warning on line 577 in apps/desktop/src/app/app.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/app.component.ts#L577

Added line #L577 was not covered by tests
variant: "warning",
title: this.i18nService.t("loggedOut"),
message: this.i18nService.t("loginExpired"),
};
break;

Check warning on line 582 in apps/desktop/src/app/app.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/app.component.ts#L582

Added line #L582 was not covered by tests
}
// We don't expect these scenarios to be common, but we want the user to
// understand why they are being logged out before a process reload.
case "accessTokenUnableToBeDecrypted": {
// Don't create multiple dialogs if this fires multiple times
if (this.activeSimpleDialog) {
jlf0dev marked this conversation as resolved.
Show resolved Hide resolved
// Let the caller of this function listen for the dialog to close
return firstValueFrom(this.activeSimpleDialog.closed);

Check warning on line 590 in apps/desktop/src/app/app.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/app.component.ts#L590

Added line #L590 was not covered by tests
}

this.activeSimpleDialog = this.dialogService.openSimpleDialogRef({

Check warning on line 593 in apps/desktop/src/app/app.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/app.component.ts#L593

Added line #L593 was not covered by tests
title: { key: "loggedOut" },
content: { key: "accessTokenUnableToBeDecrypted" },
acceptButtonText: { key: "ok" },
cancelButtonText: null,
type: "danger",
});

await firstValueFrom(this.activeSimpleDialog.closed);
this.activeSimpleDialog = null;

Check warning on line 602 in apps/desktop/src/app/app.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/app.component.ts#L601-L602

Added lines #L601 - L602 were not covered by tests

break;

Check warning on line 604 in apps/desktop/src/app/app.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/app.component.ts#L604

Added line #L604 was not covered by tests
}
case "refreshTokenSecureStorageRetrievalFailure": {
// Don't create multiple dialogs if this fires multiple times
if (this.activeSimpleDialog) {
// Let the caller of this function listen for the dialog to close
return firstValueFrom(this.activeSimpleDialog.closed);

Check warning on line 610 in apps/desktop/src/app/app.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/app.component.ts#L610

Added line #L610 was not covered by tests
}

this.activeSimpleDialog = this.dialogService.openSimpleDialogRef({

Check warning on line 613 in apps/desktop/src/app/app.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/app.component.ts#L613

Added line #L613 was not covered by tests
title: { key: "loggedOut" },
content: { key: "refreshTokenSecureStorageRetrievalFailure" },
acceptButtonText: { key: "ok" },
cancelButtonText: null,
type: "danger",
});

await firstValueFrom(this.activeSimpleDialog.closed);
this.activeSimpleDialog = null;

Check warning on line 622 in apps/desktop/src/app/app.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/app.component.ts#L621-L622

Added lines #L621 - L622 were not covered by tests

break;

Check warning on line 624 in apps/desktop/src/app/app.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/app.component.ts#L624

Added line #L624 was not covered by tests
}
default: {
toastOptions = {

Check warning on line 627 in apps/desktop/src/app/app.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/app.component.ts#L627

Added line #L627 was not covered by tests
variant: "warning",
title: this.i18nService.t("loggedOut"),
message: this.i18nService.t("loggedOutDesc"),
};
break;

Check warning on line 632 in apps/desktop/src/app/app.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/app.component.ts#L632

Added line #L632 was not covered by tests
}
}

if (toastOptions) {
this.toastService.showToast(toastOptions);

Check warning on line 637 in apps/desktop/src/app/app.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/app.component.ts#L637

Added line #L637 was not covered by tests
}
}

// Even though the userId parameter is no longer optional doesn't mean a message couldn't be
// passing null-ish values to us.
private async logOut(expired: boolean, userId: UserId) {
private async logOut(logoutReason: LogoutReason, userId: UserId) {
await this.displayLogoutReason(logoutReason);

Check warning on line 644 in apps/desktop/src/app/app.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/app.component.ts#L644

Added line #L644 was not covered by tests

const activeUserId = await firstValueFrom(
this.accountService.activeAccount$.pipe(map((a) => a?.id)),
);
Expand Down Expand Up @@ -640,15 +715,7 @@
// This must come last otherwise the logout will prematurely trigger
// a process reload before all the state service user data can be cleaned up
if (userBeingLoggedOut === activeUserId) {
this.authService.logOut(async () => {
if (expired) {
this.platformUtilsService.showToast(
"warning",
this.i18nService.t("loggedOut"),
this.i18nService.t("loginExpired"),
);
}
});
this.authService.logOut(async () => {});

Check notice on line 718 in apps/desktop/src/app/app.component.ts

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

✅ No longer an issue: Bumpy Road Ahead

AppComponent.logOut is no longer above the threshold for logical blocks with deeply nested code

Check warning on line 718 in apps/desktop/src/app/app.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/app.component.ts#L718

Added line #L718 was not covered by tests
}
}

Expand Down Expand Up @@ -730,7 +797,7 @@
// 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
options[1] === "logOut"
? this.logOut(false, userId as UserId)
? this.logOut("vaultTimeout", userId as UserId)
: await this.vaultTimeoutService.lock(userId);
}
}
Expand Down
Loading
Loading