Skip to content

Commit 20ddf3b

Browse files
authored
[PM-26649] Prevent log-out when changing KDF settings (except old clients) (#16775)
* Prevent log-out when changing KDF settings (except old clients) * test coverage * logout reason enum
1 parent a15d686 commit 20ddf3b

File tree

6 files changed

+96
-6
lines changed

6 files changed

+96
-6
lines changed

libs/common/src/enums/feature-flag.enum.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ export enum FeatureFlag {
3737
PM25174_DisableType0Decryption = "pm-25174-disable-type-0-decryption",
3838
WindowsBiometricsV2 = "pm-25373-windows-biometrics-v2",
3939
UnlockWithMasterPasswordUnlockData = "pm-23246-unlock-with-master-password-unlock-data",
40+
NoLogoutOnKdfChange = "pm-23995-no-logout-on-kdf-change",
4041

4142
/* Tools */
4243
DesktopSendUIRefresh = "desktop-send-ui-refresh",
@@ -120,6 +121,7 @@ export const DefaultFeatureFlagValue = {
120121
[FeatureFlag.PM25174_DisableType0Decryption]: FALSE,
121122
[FeatureFlag.WindowsBiometricsV2]: FALSE,
122123
[FeatureFlag.UnlockWithMasterPasswordUnlockData]: FALSE,
124+
[FeatureFlag.NoLogoutOnKdfChange]: FALSE,
123125

124126
/* Platform */
125127
[FeatureFlag.IpcChannelFramework]: FALSE,

libs/common/src/enums/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,4 @@ export * from "./http-status-code.enum";
66
export * from "./integration-type.enum";
77
export * from "./native-messaging-version.enum";
88
export * from "./notification-type.enum";
9+
export * from "./push-notification-logout-reason.enum";
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
export const PushNotificationLogOutReasonType = Object.freeze({
2+
KdfChange: 0,
3+
} as const);
4+
5+
export type PushNotificationLogOutReasonType =
6+
(typeof PushNotificationLogOutReasonType)[keyof typeof PushNotificationLogOutReasonType];

libs/common/src/models/response/notification.response.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { NotificationViewResponse as EndUserNotificationResponse } from "@bitwarden/common/vault/notifications/models";
22

3-
import { NotificationType } from "../../enums";
3+
import { NotificationType, PushNotificationLogOutReasonType } from "../../enums";
44

55
import { BaseResponse } from "./base.response";
66

@@ -41,9 +41,11 @@ export class NotificationResponse extends BaseResponse {
4141
case NotificationType.SyncOrganizations:
4242
case NotificationType.SyncOrgKeys:
4343
case NotificationType.SyncSettings:
44-
case NotificationType.LogOut:
4544
this.payload = new UserNotification(payload);
4645
break;
46+
case NotificationType.LogOut:
47+
this.payload = new LogOutNotification(payload);
48+
break;
4749
case NotificationType.SyncSendCreate:
4850
case NotificationType.SyncSendUpdate:
4951
case NotificationType.SyncSendDelete:
@@ -184,3 +186,14 @@ export class ProviderBankAccountVerifiedPushNotification extends BaseResponse {
184186
this.adminId = this.getResponseProperty("AdminId");
185187
}
186188
}
189+
190+
export class LogOutNotification extends BaseResponse {
191+
userId: string;
192+
reason?: PushNotificationLogOutReasonType;
193+
194+
constructor(response: any) {
195+
super(response);
196+
this.userId = this.getResponseProperty("UserId");
197+
this.reason = this.getResponseProperty("Reason");
198+
}
199+
}

libs/common/src/platform/server-notifications/internal/default-server-notifications.service.spec.ts

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { Matrix } from "../../../../spec/matrix";
1111
import { AccountService } from "../../../auth/abstractions/account.service";
1212
import { AuthService } from "../../../auth/abstractions/auth.service";
1313
import { AuthenticationStatus } from "../../../auth/enums/authentication-status";
14-
import { NotificationType } from "../../../enums";
14+
import { NotificationType, PushNotificationLogOutReasonType } from "../../../enums";
1515
import { NotificationResponse } from "../../../models/response/notification.response";
1616
import { UserId } from "../../../types/guid";
1717
import { AppIdService } from "../../abstractions/app-id.service";
@@ -340,4 +340,56 @@ describe("NotificationsService", () => {
340340
expect(webPushNotificationConnectionService.supportStatus$).toHaveBeenCalledTimes(1);
341341
subscription.unsubscribe();
342342
});
343+
344+
describe("processNotification", () => {
345+
beforeEach(async () => {
346+
appIdService.getAppId.mockResolvedValue("test-app-id");
347+
activeAccount.next({ id: mockUser1, email: "email", name: "Test Name", emailVerified: true });
348+
});
349+
350+
describe("NotificationType.LogOut", () => {
351+
it.each([
352+
{ featureFlagEnabled: false, reason: undefined },
353+
{ featureFlagEnabled: true, reason: undefined },
354+
{ featureFlagEnabled: false, reason: PushNotificationLogOutReasonType.KdfChange },
355+
])(
356+
"should call logout callback when featureFlag=$featureFlagEnabled and reason=$reason",
357+
async ({ featureFlagEnabled, reason }) => {
358+
configService.getFeatureFlag$.mockReturnValue(of(featureFlagEnabled));
359+
360+
const payload: { UserId: UserId; Reason?: PushNotificationLogOutReasonType } = {
361+
UserId: mockUser1,
362+
Reason: undefined,
363+
};
364+
if (reason != null) {
365+
payload.Reason = reason;
366+
}
367+
368+
const notification = new NotificationResponse({
369+
type: NotificationType.LogOut,
370+
payload,
371+
contextId: "different-app-id",
372+
});
373+
374+
await sut["processNotification"](notification, mockUser1);
375+
376+
expect(logoutCallback).toHaveBeenCalledWith("logoutNotification", mockUser1);
377+
},
378+
);
379+
380+
it("should skip logout when receiving KDF change reason with feature flag enabled", async () => {
381+
configService.getFeatureFlag$.mockReturnValue(of(true));
382+
383+
const notification = new NotificationResponse({
384+
type: NotificationType.LogOut,
385+
payload: { UserId: mockUser1, Reason: PushNotificationLogOutReasonType.KdfChange },
386+
contextId: "different-app-id",
387+
});
388+
389+
await sut["processNotification"](notification, mockUser1);
390+
391+
expect(logoutCallback).not.toHaveBeenCalled();
392+
});
393+
});
394+
});
343395
});

libs/common/src/platform/server-notifications/internal/default-server-notifications.service.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@ import { trackedMerge } from "@bitwarden/common/platform/misc";
2222
import { AccountInfo, AccountService } from "../../../auth/abstractions/account.service";
2323
import { AuthService } from "../../../auth/abstractions/auth.service";
2424
import { AuthenticationStatus } from "../../../auth/enums/authentication-status";
25-
import { NotificationType } from "../../../enums";
25+
import { NotificationType, PushNotificationLogOutReasonType } from "../../../enums";
2626
import {
27+
LogOutNotification,
2728
NotificationResponse,
2829
SyncCipherNotification,
2930
SyncFolderNotification,
@@ -263,10 +264,25 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer
263264
this.activitySubject.next("inactive"); // Force a disconnect
264265
this.activitySubject.next("active"); // Allow a reconnect
265266
break;
266-
case NotificationType.LogOut:
267+
case NotificationType.LogOut: {
267268
this.logService.info("[Notifications Service] Received logout notification");
268-
await this.logoutCallback("logoutNotification", userId);
269+
270+
const logOutNotification = notification.payload as LogOutNotification;
271+
const noLogoutOnKdfChange = await firstValueFrom(
272+
this.configService.getFeatureFlag$(FeatureFlag.NoLogoutOnKdfChange),
273+
);
274+
if (
275+
noLogoutOnKdfChange &&
276+
logOutNotification.reason === PushNotificationLogOutReasonType.KdfChange
277+
) {
278+
this.logService.info(
279+
"[Notifications Service] Skipping logout due to no logout KDF change",
280+
);
281+
} else {
282+
await this.logoutCallback("logoutNotification", userId);
283+
}
269284
break;
285+
}
270286
case NotificationType.SyncSendCreate:
271287
case NotificationType.SyncSendUpdate:
272288
await this.syncService.syncUpsertSend(

0 commit comments

Comments
 (0)