Skip to content
This repository has been archived by the owner on Oct 30, 2023. It is now read-only.

Commit

Permalink
fix: set macOS crypto keychain name earlier (electron#34683)
Browse files Browse the repository at this point in the history
* fix: set macOS crypto keychain name earlier

* spec: ensure arm64 mac tests are cleaned up
  • Loading branch information
MarshallOfSound authored and khalwa committed Feb 22, 2023
1 parent 5387535 commit 85b4425
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 14 deletions.
1 change: 1 addition & 0 deletions .circleci/config/base.yml
Expand Up @@ -216,6 +216,7 @@ step-maybe-cleanup-arm64-mac: &step-maybe-cleanup-arm64-mac
rm -rf ~/Library/Application\ Support/electron*
security delete-generic-password -l "Chromium Safe Storage" || echo "✓ Keychain does not contain password from tests"
security delete-generic-password -l "Electron Test Main Safe Storage" || echo "✓ Keychain does not contain password from tests"
security delete-generic-password -a "electron-test-safe-storage" || echo "✓ Keychain does not contain password from tests"
elif [ "$TARGET_ARCH" == "arm" ] || [ "$TARGET_ARCH" == "arm64" ]; then
XVFB=/usr/bin/Xvfb
/sbin/start-stop-daemon --stop --exec $XVFB || echo "Xvfb not running"
Expand Down
9 changes: 8 additions & 1 deletion shell/browser/electron_browser_main_parts.cc
Expand Up @@ -91,6 +91,7 @@
#endif

#if BUILDFLAG(IS_MAC)
#include "components/os_crypt/keychain_password_mac.h"
#include "services/device/public/cpp/geolocation/geolocation_manager.h"
#include "shell/browser/ui/cocoa/views_delegate_mac.h"
#else
Expand Down Expand Up @@ -490,6 +491,9 @@ void ElectronBrowserMainParts::WillRunMainMessageLoop(
}

void ElectronBrowserMainParts::PostCreateMainMessageLoop() {
#if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_MAC)
std::string app_name = electron::Browser::Get()->GetName();
#endif
#if BUILDFLAG(IS_LINUX)
auto shutdown_cb =
base::BindOnce(base::RunLoop::QuitCurrentWhenIdleClosureDeprecated());
Expand All @@ -500,7 +504,6 @@ void ElectronBrowserMainParts::PostCreateMainMessageLoop() {
// Set up crypt config. This needs to be done before anything starts the
// network service, as the raw encryption key needs to be shared with the
// network service for encrypted cookie storage.
std::string app_name = electron::Browser::Get()->GetName();
const base::CommandLine& command_line =
*base::CommandLine::ForCurrentProcess();
std::unique_ptr<os_crypt::Config> config =
Expand All @@ -517,6 +520,10 @@ void ElectronBrowserMainParts::PostCreateMainMessageLoop() {
base::PathService::Get(DIR_SESSION_DATA, &config->user_data_path);
OSCrypt::SetConfig(std::move(config));
#endif
#if BUILDFLAG(IS_MAC)
KeychainPassword::GetServiceName() = app_name + " Safe Storage";
KeychainPassword::GetAccountName() = app_name;
#endif
#if BUILDFLAG(IS_POSIX)
// Exit in response to SIGINT, SIGTERM, etc.
InstallShutdownSignalHandlers(
Expand Down
10 changes: 0 additions & 10 deletions shell/browser/net/system_network_context_manager.cc
Expand Up @@ -42,10 +42,6 @@
#include "shell/common/options_switches.h"
#include "url/gurl.h"

#if BUILDFLAG(IS_MAC)
#include "components/os_crypt/keychain_password_mac.h"
#endif

#if BUILDFLAG(IS_LINUX)
#include "components/os_crypt/key_storage_config_linux.h"
#endif
Expand Down Expand Up @@ -288,12 +284,6 @@ void SystemNetworkContextManager::OnNetworkServiceCreated(
base::FeatureList::IsEnabled(features::kAsyncDns),
default_secure_dns_mode, doh_config, additional_dns_query_types_enabled);

std::string app_name = electron::Browser::Get()->GetName();
#if BUILDFLAG(IS_MAC)
KeychainPassword::GetServiceName() = app_name + " Safe Storage";
KeychainPassword::GetAccountName() = app_name;
#endif

// The OSCrypt keys are process bound, so if network service is out of
// process, send it the required key.
if (content::IsOutOfProcessNetworkService() &&
Expand Down
6 changes: 3 additions & 3 deletions spec/api-safe-storage-spec.ts
Expand Up @@ -4,7 +4,7 @@ import { safeStorage } from 'electron/main';
import { expect } from 'chai';
import { emittedOnce } from './events-helpers';
import { ifdescribe } from './spec-helpers';
import * as fs from 'fs';
import * as fs from 'fs-extra';

/* isEncryptionAvailable returns false in Linux when running CI due to a mocked dbus. This stops
* Chrome from reaching the system's keyring or libsecret. When running the tests with config.store
Expand Down Expand Up @@ -36,8 +36,8 @@ describe('safeStorage module', () => {
ifdescribe(process.platform !== 'linux')('safeStorage module', () => {
after(async () => {
const pathToEncryptedString = path.resolve(__dirname, 'fixtures', 'api', 'safe-storage', 'encrypted.txt');
if (fs.existsSync(pathToEncryptedString)) {
await fs.unlinkSync(pathToEncryptedString);
if (await fs.pathExists(pathToEncryptedString)) {
await fs.remove(pathToEncryptedString);
}
});

Expand Down

0 comments on commit 85b4425

Please sign in to comment.