mirror of
https://github.com/bitwarden/browser
synced 2025-12-06 00:13:28 +00:00
rework error handling & presentation
This commit is contained in:
@@ -15,6 +15,17 @@ pub mod sandbox {
|
||||
use anyhow::{anyhow, Result};
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
// Before requesting access outside of sandbox, determine if browser is actually installed
|
||||
const BROWSER_BUNDLE_IDS: &[(&str, &str)] = &[
|
||||
("Chrome", "com.google.Chrome"),
|
||||
("Chromium", "org.chromium.Chromium"),
|
||||
("Microsoft Edge", "com.microsoft.edgemac"),
|
||||
("Brave", "com.brave.Browser"),
|
||||
("Arc", "company.thebrowser.Browser"),
|
||||
("Opera", "com.operasoftware.Opera"),
|
||||
("Vivaldi", "com.vivaldi.Vivaldi"),
|
||||
];
|
||||
|
||||
#[derive(Debug, Deserialize)]
|
||||
#[serde(tag = "type")]
|
||||
enum CommandResult<T> {
|
||||
@@ -61,6 +72,13 @@ pub mod sandbox {
|
||||
.find(|b| b.name == browser_name)
|
||||
.ok_or_else(|| anyhow!("Unsupported browser: {}", browser_name))?;
|
||||
|
||||
if !is_browser_installed(browser_name).await? {
|
||||
return Err(anyhow!(
|
||||
"chromiumImporterBrowserNotInstalled:{}",
|
||||
browser_name
|
||||
));
|
||||
}
|
||||
|
||||
// For macOS, data_dir is always a single-element array
|
||||
let relative_path = config
|
||||
.data_dir
|
||||
@@ -163,6 +181,44 @@ pub mod sandbox {
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
async fn is_browser_installed(browser_name: &str) -> Result<bool> {
|
||||
let bundle_id = BROWSER_BUNDLE_IDS
|
||||
.iter()
|
||||
.find(|(name, _)| *name == browser_name)
|
||||
.map(|(_, id)| *id);
|
||||
|
||||
let bundle_id = match bundle_id {
|
||||
Some(id) => id,
|
||||
None => return Ok(true),
|
||||
};
|
||||
|
||||
let input = CommandInput {
|
||||
namespace: "chromium_importer".to_string(),
|
||||
command: "check_browser_installed".to_string(),
|
||||
params: serde_json::json!({
|
||||
"bundleId": bundle_id,
|
||||
}),
|
||||
};
|
||||
|
||||
let output = desktop_objc::run_command(serde_json::to_string(&input)?)
|
||||
.await
|
||||
.map_err(|e| anyhow!("Failed to call ObjC command: {}", e))?;
|
||||
|
||||
let result: CommandResult<CheckBrowserInstalledResponse> = serde_json::from_str(&output)
|
||||
.map_err(|e| anyhow!("Failed to parse ObjC response: {}", e))?;
|
||||
|
||||
match result {
|
||||
CommandResult::Success { value } => Ok(value.is_installed),
|
||||
CommandResult::Error { error } => Err(anyhow!("{}", error)),
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize)]
|
||||
struct CheckBrowserInstalledResponse {
|
||||
#[serde(rename = "isInstalled")]
|
||||
is_installed: bool,
|
||||
}
|
||||
}
|
||||
|
||||
//
|
||||
|
||||
@@ -14,7 +14,7 @@
|
||||
}
|
||||
|
||||
- (NSString *)requestAccessToBrowserDir:(NSString *)browserName relativePath:(NSString *)relativePath {
|
||||
|
||||
|
||||
if (!relativePath) {
|
||||
return nil;
|
||||
}
|
||||
@@ -53,14 +53,7 @@
|
||||
|
||||
NSURL *localStatePath = [selectedURL URLByAppendingPathComponent:@"Local State"];
|
||||
if (![[NSFileManager defaultManager] fileExistsAtPath:localStatePath.path]) {
|
||||
NSAlert *alert = [[NSAlert alloc] init];
|
||||
alert.messageText = @"Invalid Folder";
|
||||
alert.informativeText = [NSString stringWithFormat:
|
||||
@"The selected folder doesn't appear to be a valid %@ data directory. Please select the correct folder.",
|
||||
browserName];
|
||||
alert.alertStyle = NSAlertStyleWarning;
|
||||
[alert runModal];
|
||||
|
||||
// Invalid directory selected caller will handle
|
||||
return nil;
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,8 @@
|
||||
#ifndef CHECK_BROWSER_INSTALLED_H
|
||||
#define CHECK_BROWSER_INSTALLED_H
|
||||
|
||||
#import <Foundation/Foundation.h>
|
||||
|
||||
void checkBrowserInstalledCommand(void *context, NSDictionary *params);
|
||||
|
||||
#endif
|
||||
@@ -0,0 +1,29 @@
|
||||
#import <Foundation/Foundation.h>
|
||||
#import <CoreServices/CoreServices.h>
|
||||
#import "../../interop.h"
|
||||
#import "check_browser_installed.h"
|
||||
|
||||
void checkBrowserInstalledCommand(void* context, NSDictionary *params) {
|
||||
NSString *bundleId = params[@"bundleId"];
|
||||
|
||||
if (!bundleId) {
|
||||
return _return(context, _error(@"Missing required parameter: bundleId"));
|
||||
}
|
||||
|
||||
CFURLRef appURL = NULL;
|
||||
OSStatus status = LSFindApplicationForInfo(
|
||||
kLSUnknownCreator,
|
||||
(__bridge CFStringRef)bundleId,
|
||||
NULL,
|
||||
NULL,
|
||||
&appURL
|
||||
);
|
||||
|
||||
BOOL isInstalled = (status == noErr && appURL != NULL);
|
||||
|
||||
if (appURL != NULL) {
|
||||
CFRelease(appURL);
|
||||
}
|
||||
|
||||
_return(context, _success(@{@"isInstalled": @(isInstalled)}));
|
||||
}
|
||||
@@ -15,7 +15,7 @@ void requestAccessCommand(void* context, NSDictionary *params) {
|
||||
NSString *bookmarkData = [manager requestAccessToBrowserDir:browserName relativePath:relativePath];
|
||||
|
||||
if (bookmarkData == nil) {
|
||||
return _return(context, _error(@"User denied access or selected an invalid browser directory"));
|
||||
return _return(context, _error(@"browserAccessDenied"));
|
||||
}
|
||||
|
||||
_return(context, _success(@{@"bookmark": bookmarkData}));
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
#import "commands/has_stored_access.h"
|
||||
#import "commands/start_access.h"
|
||||
#import "commands/stop_access.h"
|
||||
#import "commands/check_browser_installed.h"
|
||||
#import "../interop.h"
|
||||
#import "../utils.h"
|
||||
#import "run_chromium_command.h"
|
||||
@@ -19,6 +20,8 @@ void runChromiumCommand(void* context, NSDictionary *input) {
|
||||
return startAccessCommand(context, params);
|
||||
} else if ([command isEqual:@"stop_access"]) {
|
||||
return stopAccessCommand(context, params);
|
||||
} else if ([command isEqual:@"check_browser_installed"]) {
|
||||
return checkBrowserInstalledCommand(context, params);
|
||||
}
|
||||
|
||||
_return(context, _error([NSString stringWithFormat:@"Unknown command: %@", command]));
|
||||
|
||||
@@ -61,8 +61,26 @@ export class ImportDesktopComponent {
|
||||
try {
|
||||
// Request browser access (required for sandboxed builds, no-op otherwise)
|
||||
await ipc.tools.chromiumImporter.requestBrowserAccess(browser);
|
||||
} catch {
|
||||
throw new Error(this.i18nService.t("browserAccessDenied"));
|
||||
} catch (error) {
|
||||
const rawMessage = error instanceof Error ? error.message : "";
|
||||
|
||||
// Check verbose error chain for specific i18n key indicating browser not installed
|
||||
const browserNotInstalledMatch = rawMessage.match(
|
||||
/chromiumImporterBrowserNotInstalled:([^:]+)/,
|
||||
);
|
||||
let message: string;
|
||||
|
||||
if (browserNotInstalledMatch) {
|
||||
message = this.i18nService.t(
|
||||
"chromiumImporterBrowserNotInstalled",
|
||||
browserNotInstalledMatch[1],
|
||||
);
|
||||
} else {
|
||||
// Invalid folder, explicit permission denial, or system error
|
||||
message = this.i18nService.t("browserAccessDenied");
|
||||
}
|
||||
|
||||
throw new Error(message);
|
||||
}
|
||||
return ipc.tools.chromiumImporter.getAvailableProfiles(browser);
|
||||
}
|
||||
|
||||
@@ -4299,5 +4299,14 @@
|
||||
},
|
||||
"sessionTimeoutSettingsSetUnlockMethodToChangeTimeoutAction": {
|
||||
"message": "Set an unlock method to change your timeout action"
|
||||
},
|
||||
"chromiumImporterBrowserNotInstalled": {
|
||||
"message": "$BROWSER$ is not installed. Please install $BROWSER$ before importing passwords from it.",
|
||||
"placeholders": {
|
||||
"browser": {
|
||||
"content": "$1",
|
||||
"example": "Chrome"
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -124,10 +124,8 @@ export class ImportChromeComponent implements OnInit, OnDestroy {
|
||||
);
|
||||
} catch (error) {
|
||||
this.logService.error("Error loading profiles from browser:", error);
|
||||
const keyOrMessage = this.getValidationErrorI18nKey(error);
|
||||
this.error.emit(
|
||||
keyOrMessage === "errorOccurred" ? this.i18nService.t("errorOccurred") : keyOrMessage,
|
||||
);
|
||||
const translatedMessage = this.translateValidationError(error);
|
||||
this.error.emit(translatedMessage);
|
||||
}
|
||||
}
|
||||
});
|
||||
@@ -185,20 +183,40 @@ export class ImportChromeComponent implements OnInit, OnDestroy {
|
||||
return null;
|
||||
} catch (error) {
|
||||
this.logService.error(`Chromium importer error: ${error}`);
|
||||
const keyOrMessage = this.getValidationErrorI18nKey(error);
|
||||
const translatedMessage = this.translateValidationError(error);
|
||||
return {
|
||||
errors: {
|
||||
message:
|
||||
keyOrMessage === "errorOccurred" ? this.i18nService.t("errorOccurred") : keyOrMessage,
|
||||
message: translatedMessage,
|
||||
},
|
||||
};
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
private getValidationErrorI18nKey(error: any): string {
|
||||
private translateValidationError(error: any): string {
|
||||
const message = typeof error === "string" ? error : error?.message;
|
||||
return message || "errorOccurred";
|
||||
if (!message) {
|
||||
return this.i18nService.t("errorOccurred");
|
||||
}
|
||||
|
||||
// Check for specific browser not installed error
|
||||
const browserNotInstalledMatch = message.match(/chromiumImporterBrowserNotInstalled:([^:]+)/);
|
||||
if (browserNotInstalledMatch) {
|
||||
return this.i18nService.t("chromiumImporterBrowserNotInstalled", browserNotInstalledMatch[1]);
|
||||
}
|
||||
|
||||
// Generic IPC error
|
||||
if (message.includes("Error invoking remote method")) {
|
||||
return this.i18nService.t("errorOccurred");
|
||||
}
|
||||
|
||||
// Check if it's a known i18n key
|
||||
if (message === "browserAccessDenied") {
|
||||
return this.i18nService.t("browserAccessDenied");
|
||||
}
|
||||
|
||||
// Return raw message as fallback
|
||||
return message;
|
||||
}
|
||||
|
||||
private getBrowserName(format: ImportType): string {
|
||||
|
||||
@@ -463,6 +463,7 @@
|
||||
[onLoadProfilesFromBrowser]="this.onLoadProfilesFromBrowser"
|
||||
[format]="this.format"
|
||||
(csvDataLoaded)="this.formGroup.controls.fileContents.setValue($event)"
|
||||
(error)="this.handleChromeImportError($event)"
|
||||
></import-chrome>
|
||||
} @else {
|
||||
<bit-form-field>
|
||||
|
||||
@@ -513,6 +513,14 @@ export class ImportComponent implements OnInit, OnDestroy, AfterViewInit {
|
||||
return null;
|
||||
}
|
||||
|
||||
protected handleChromeImportError(error: string) {
|
||||
this.toastService.showToast({
|
||||
variant: "error",
|
||||
title: this.i18nService.t("errorOccurred"),
|
||||
message: error,
|
||||
});
|
||||
}
|
||||
|
||||
protected setImportOptions() {
|
||||
this.featuredImportOptions = [...this.importService.featuredImportOptions];
|
||||
|
||||
|
||||
Reference in New Issue
Block a user