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

Chore/credential migration #2562

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions js/default.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ require('autofillSetup.js').initialize()
require('passwordManager/passwordManager.js').initialize()
require('passwordManager/passwordCapture.js').initialize()
require('passwordManager/passwordViewer.js').initialize()
require('passwordManager/passwordMigrator.js').initialize()
require('util/theme.js').initialize()
require('userscripts.js').initialize()
require('statistics.js').initialize()
Expand Down
2 changes: 1 addition & 1 deletion js/passwordManager/keychain.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class Keychain {
const domainWithProtocol = includesProtocol ? credential.url : `https://${credential.url}`

return {
domain: new URL(domainWithProtocol).hostname.replace(/^www\./g, ''),
domain: new URL(domainWithProtocol).origin,
username: credential.username,
password: credential.password
}
Expand Down
7 changes: 2 additions & 5 deletions js/passwordManager/onePassword.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,11 +148,8 @@ class OnePassword {

const credentials = matches.filter((match) => {
try {
var matchHost = new URL(match.urls.find(url => url.primary).href).hostname
if (matchHost.startsWith('www.')) {
matchHost = matchHost.slice(4)
}
return matchHost === domain
var matchHost = new URL(match.urls.find(url => url.primary).href).origin
return matchHost.replace('www.', '') === domain || matchHost === domain
} catch (e) {
return false
}
Expand Down
8 changes: 4 additions & 4 deletions js/passwordManager/passwordCapture.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,11 @@ const passwordCapture = {
},
handleRecieveCredentials: function (tab, args, frameId) {
var domain = args[0][0]
if (domain.startsWith('www.')) {
domain = domain.slice(4)
}

if (settings.get('passwordsNeverSaveDomains') && settings.get('passwordsNeverSaveDomains').includes(domain)) {
if (settings.get('passwordsNeverSaveDomains') && (
settings.get('passwordsNeverSaveDomains').includes(domain.replace('www.', '')) ||
settings.get('passwordsNeverSaveDomains').includes(domain)
)) {
return
}

Expand Down
11 changes: 3 additions & 8 deletions js/passwordManager/passwordManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ const PasswordManagers = {
webviews.bindIPC('password-autofill', function (tab, args, frameId, frameURL) {
// it's important to use frameURL here and not the tab URL, because the domain of the
// requesting iframe may not match the domain of the top-level page
const hostname = new URL(frameURL).hostname
const origin = new URL(frameURL).origin

PasswordManagers.getConfiguredPasswordManager().then(async (manager) => {
if (!manager) {
Expand All @@ -97,16 +97,11 @@ const PasswordManagers = {
await PasswordManagers.unlock(manager)
}

var formattedHostname = hostname
if (formattedHostname.startsWith('www.')) {
formattedHostname = formattedHostname.slice(4)
}

manager.getSuggestions(formattedHostname).then(credentials => {
manager.getSuggestions(origin).then(credentials => {
if (credentials != null) {
webviews.callAsync(tab, 'sendToFrame', [frameId, 'password-autofill-match', {
credentials,
hostname
origin
}])
}
}).catch(e => {
Expand Down
94 changes: 94 additions & 0 deletions js/passwordManager/passwordMigrator.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
const { ipcRenderer } = require('electron')
const PasswordManagers = require('passwordManager/passwordManager.js')
const places = require('places/places.js')
const settings = require('util/settings/settings.js')

class PasswordMigrator {
#currentVersion = 2;

constructor() {
this.startMigration()
}

async _isOutdated(version) {
return version < this.#currentVersion
}

async _getInUseCredentialVersion() {
const version = await ipcRenderer.invoke('credentialStoreGetVersion')
return version
}

async startMigration() {
const inUseVersion = await this._getInUseCredentialVersion()
const isOutdated = await this._isOutdated(inUseVersion)
if (!isOutdated) return

try {
if (inUseVersion === 1 && this.#currentVersion === 2) {
await this.migrateVersion1to2()
console.log('[PasswordMigrator]: Migration complete.')
return
}
} catch (error) {
console.error('Error during password migration:', error)
}
}

async migrateVersion1to2() {
console.log('[PasswordMigrator]: Migrating keychain data to version', this.#currentVersion)

const passwordManager = await PasswordManagers.getConfiguredPasswordManager()
if (!passwordManager || !passwordManager.getAllCredentials) {
throw new Error('Incompatible password manager')
}

const historyData = await places.getAllItems()
const currentCredentials = await passwordManager.getAllCredentials()
console.log('[PasswordMigrator]: Found', historyData.length, 'history entries', historyData)
console.log('[PasswordMigrator]: Found', currentCredentials.length, 'credentials in the current password manager', currentCredentials)

function createNewCredential(credential) {
// 1) check if the saved url has been visited, if so use that url
const historyEntry = historyData.find(entry => new URL(entry.url).host.replace(/^(https?:\/\/)?(www\.)?/, '') === credential.domain.replace(/^(https?:\/\/)?(www\.)?/, ''))
if (historyEntry) {
return {
username: credential.username,
password: credential.password,
url: historyEntry.url
}
}

// 2) check if domain has protocol, if not, add 'https://'
if (!newUrl.startsWith('http://') && !newUrl.startsWith('https://')) {
newUrl = `https://${newUrl}`
}

return {
username: credential.username,
password: credential.password,
url: newUrl
};
}

const migratedCredentials = currentCredentials.map(createNewCredential)
console.log('[PasswordMigrator]: Migrated', migratedCredentials.length, 'credentials', migratedCredentials);

const neverSavedCredentials = settings.get('passwordsNeverSaveDomains') || []
console.log('[PasswordMigrator]: Found', neverSavedCredentials.length, 'never-saved credentials', neverSavedCredentials)
const migratedNeverSavedCredentials = neverSavedCredentials.map(createNewCredential)
settings.set('passwordsNeverSaveDomains', migratedNeverSavedCredentials)
console.log('[PasswordMigrator]: Migrated', migratedNeverSavedCredentials.length, 'never-saved credentials', migratedNeverSavedCredentials)

await ipcRenderer.invoke('credentialStoreSetPasswordBulk', migratedCredentials)

// finally upate the version
await ipcRenderer.invoke('credentialStoreSetVersion', this.#currentVersion)
}
}

function initialize() {
new PasswordMigrator()
}

module.exports = { initialize }
6 changes: 3 additions & 3 deletions js/preload/passwordFill.js
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ function handleBlur (event) {
// Handle credentials fetched from the backend. Credentials are expected to be
// an array of { username, password, manager } objects.
ipc.on('password-autofill-match', (event, data) => {
if (data.hostname !== window.location.hostname) {
if (data.origin.replace('www.', '') !== window.location.origin || data.origin !== window.location.origin) {
throw new Error('password origin must match current page origin')
}

Expand Down Expand Up @@ -344,7 +344,7 @@ function handleFormSubmit () {
var passwordValue = getBestPasswordField()?.value

if ((usernameValue && usernameValue.length > 0) && (passwordValue && passwordValue.length > 0)) {
ipc.send('password-form-filled', [window.location.hostname, usernameValue, passwordValue])
ipc.send('password-form-filled', [window.location.origin, usernameValue, passwordValue])
}
}

Expand Down Expand Up @@ -428,7 +428,7 @@ ipc.on('generate-password', function (location) {
setTimeout(function () {
if (input.value === generatedPassword) {
var usernameValue = getBestUsernameField()?.value
ipc.send('password-form-filled', [window.location.hostname, usernameValue, generatedPassword])
ipc.send('password-form-filled', [window.location.origin, usernameValue, generatedPassword])
}
}, 0)
}
Expand Down
10 changes: 10 additions & 0 deletions main/keychainService.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,13 @@ ipc.handle('credentialStoreDeletePassword', async function (event, account) {
ipc.handle('credentialStoreGetCredentials', async function () {
return readSavedPasswordFile().credentials
})

ipc.handle('credentialStoreGetVersion', async function () {
return readSavedPasswordFile().version
})

ipc.handl('credentialStoreSetVersion', async function (event, version) {
const fileContent = readSavedPasswordFile()
fileContent.version = version
return writeSavedPasswordFile(fileContent)
})