bitwarden/browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser synced 2026-02-13 23:13:36 +00:00
Code Issues Releases Wiki Activity

address PR feedback 'Race condition with concurrent auth requests'

Browse Source
This commit is contained in:
rr-bw
2025-11-11 15:31:39 -08:00
parent 3683c213b2
commit b9d9e30801
2 changed files with 106 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files

89
apps/browser/src/popup/app.component.ts
View File

@@ -78,7 +78,8 @@ export class AppComponent implements OnInit, OnDestroy {
private lastActivity: Date;
private activeUserId: UserId;
private routerAnimations = false;
private processingPendingAuth = false;
private processingPendingAuthRequests = false;
private shouldRerunAuthRequestProcessing = false;
private destroy$ = new Subject<void>();
@@ -197,44 +198,25 @@ export class AppComponent implements OnInit, OnDestroy {
await this.router.navigate(["lock"]);
} else if (msg.command === "openLoginApproval") {
if (this.processingPendingAuth) {
if (this.processingPendingAuthRequests) {
// If an "openLoginApproval" message is received while we are currently processing other
// auth requests, then set a flag so we remember to process that new auth request
this.shouldRerunAuthRequestProcessing = true;
return;
}
this.processingPendingAuth = true;
try {
// Always query server for all pending requests and open a dialog for each
const pendingList = await firstValueFrom(
this.authRequestService.getPendingAuthRequests$(),
);
if (Array.isArray(pendingList) && pendingList.length > 0) {
const respondedIds = new Set<string>();
for (const req of pendingList) {
if (req?.id == null) {
continue;
}
const dialogRef = LoginApprovalDialogComponent.open(this.dialogService, {
notificationId: req.id,
});
const result = await firstValueFrom(dialogRef.closed);
if (result !== undefined && typeof result === "boolean") {
respondedIds.add(req.id);
if (respondedIds.size === pendingList.length && this.activeUserId != null) {
await this.pendingAuthRequestsState.clear(this.activeUserId);
}
}
}
}
} finally {
this.processingPendingAuth = false;
}
/**
* This do/while loop allows us to:
* - a) call processPendingAuthRequests() once on "openLoginApproval"
* - b) remember to re-call processPendingAuthRequests() if another "openLoginApproval" was
* received while we were processing the original auth requests
*/
do {
this.shouldRerunAuthRequestProcessing = false;
await this.processPendingAuthRequests();
// If an "openLoginApproval" message was received while processPendingAuthRequests() was running, then
// shouldRerunAuthRequestProcessing will have been set to true
} while (this.shouldRerunAuthRequestProcessing);
} else if (msg.command === "showDialog") {
// 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
@@ -372,4 +354,39 @@ export class AppComponent implements OnInit, OnDestroy {
this.toastService.showToast(toastOptions);
}
private async processPendingAuthRequests() {
this.processingPendingAuth = true;
try {
// Always query server for all pending requests and open a dialog for each
const pendingList = await firstValueFrom(this.authRequestService.getPendingAuthRequests$());
if (Array.isArray(pendingList) && pendingList.length > 0) {
const respondedIds = new Set<string>();
for (const req of pendingList) {
if (req?.id == null) {
continue;
}
const dialogRef = LoginApprovalDialogComponent.open(this.dialogService, {
notificationId: req.id,
});
const result = await firstValueFrom(dialogRef.closed);
if (result !== undefined && typeof result === "boolean") {
respondedIds.add(req.id);
if (respondedIds.size === pendingList.length && this.activeUserId != null) {
await this.pendingAuthRequestsState.clear(this.activeUserId);
}
}
}
}
} finally {
this.processingPendingAuth = false;
}
}
}

88
apps/desktop/src/app/app.component.ts
View File

@@ -152,7 +152,8 @@ export class AppComponent implements OnInit, OnDestroy {
private isIdle = false;
private activeUserId: UserId = null;
private activeSimpleDialog: DialogRef<boolean> = null;
private processingPendingAuth = false;
private processingPendingAuthRequests = false;
private shouldRerunAuthRequestProcessing = false;
private destroy$ = new Subject<void>();
@@ -505,44 +506,26 @@ export class AppComponent implements OnInit, OnDestroy {
await this.checkForSystemTimeout(VaultTimeoutStringType.OnIdle);
break;
case "openLoginApproval":
if (this.processingPendingAuth) {
if (this.processingPendingAuthRequests) {
// If an "openLoginApproval" message is received while we are currently processing other
// auth requests, then set a flag so we remember to process that new auth request
this.shouldRerunAuthRequestProcessing = true;
return;
}
this.processingPendingAuth = true;
/**
* This do/while loop allows us to:
* - a) call processPendingAuthRequests() once on "openLoginApproval"
* - b) remember to re-call processPendingAuthRequests() if another "openLoginApproval" was
* received while we were processing the original auth requests
*/
do {
this.shouldRerunAuthRequestProcessing = false;
await this.processPendingAuthRequests();
// If an "openLoginApproval" message was received while processPendingAuthRequests() was running, then
// shouldRerunAuthRequestProcessing will have been set to true
} while (this.shouldRerunAuthRequestProcessing);
try {
// Always query server for all pending requests and open a dialog for each
const pendingList = await firstValueFrom(
this.authRequestService.getPendingAuthRequests$(),
);
if (Array.isArray(pendingList) && pendingList.length > 0) {
const respondedIds = new Set<string>();
for (const req of pendingList) {
if (req?.id == null) {
continue;
}
const dialogRef = LoginApprovalDialogComponent.open(this.dialogService, {
notificationId: req.id,
});
const result = await firstValueFrom(dialogRef.closed);
if (result !== undefined && typeof result === "boolean") {
respondedIds.add(req.id);
if (respondedIds.size === pendingList.length && this.activeUserId != null) {
await this.pendingAuthRequestsState.clear(this.activeUserId);
}
}
}
}
} finally {
this.processingPendingAuth = false;
}
break;
case "redrawMenu":
await this.updateAppMenu();
@@ -924,4 +907,39 @@ export class AppComponent implements OnInit, OnDestroy {
DeleteAccountComponent.open(this.dialogService);
}
private async processPendingAuthRequests() {
this.processingPendingAuthRequests = true;
try {
// Always query server for all pending requests and open a dialog for each
const pendingList = await firstValueFrom(this.authRequestService.getPendingAuthRequests$());
if (Array.isArray(pendingList) && pendingList.length > 0) {
const respondedIds = new Set<string>();
for (const req of pendingList) {
if (req?.id == null) {
continue;
}
const dialogRef = LoginApprovalDialogComponent.open(this.dialogService, {
notificationId: req.id,
});
const result = await firstValueFrom(dialogRef.closed);
if (result !== undefined && typeof result === "boolean") {
respondedIds.add(req.id);
if (respondedIds.size === pendingList.length && this.activeUserId != null) {
await this.pendingAuthRequestsState.clear(this.activeUserId);
}
}
}
}
} finally {
this.processingPendingAuthRequests = false;
}
}
}