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

feat(bitwarden): added field for server url during setup for self-hosted servers and others #2246

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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
43 changes: 22 additions & 21 deletions js/autofillSetup.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,30 @@ const setupDialog = require('passwordManager/managerSetup.js')
const settings = require('util/settings/settings.js')
const PasswordManagers = require('passwordManager/passwordManager.js')

const AutofillSetup = {
checkSettings: function () {
const manager = PasswordManagers.getActivePasswordManager()
async function checkSettings () {
const manager = PasswordManagers.getActivePasswordManager()

if (!manager) {
return
}

try {
const configured = await manager.checkIfConfigured()
if (!configured) {
setupDialog.show(manager)
}
} catch (e) {
console.error(e)
}
}

function initialize () {
settings.listen('passwordManager', manager => {
if (!manager) {
return
}

manager.checkIfConfigured().then((configured) => {
if (!configured) {
setupDialog.show(manager)
}
}).catch((err) => {
console.error(err)
})
},
initialize: function () {
settings.listen('passwordManager', function (manager) {
if (manager) {
// Trigger the check on browser launch and after manager is enabled
AutofillSetup.checkSettings()
}
})
}
checkSettings()
})
}

module.exports = AutofillSetup
module.exports = { initialize }
126 changes: 60 additions & 66 deletions js/passwordManager/bitwarden.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
const ProcessSpawner = require('util/process.js')
const path = require('path')

const { join } = require('path')
const fs = require('fs')
var { ipcRenderer } = require('electron')
const { ipcRenderer } = require('electron')

const ProcessSpawner = require('util/process.js')

// Bitwarden password manager. Requires session key to unlock the vault.
class Bitwarden {
Expand All @@ -12,18 +14,17 @@ class Bitwarden {
}

getDownloadLink () {
switch (window.platformType) {
case 'mac':
return 'https://vault.bitwarden.com/download/?app=cli&platform=macos'
case 'windows':
return 'https://vault.bitwarden.com/download/?app=cli&platform=windows'
case 'linux':
return 'https://vault.bitwarden.com/download/?app=cli&platform=linux'
if (window.platformType === 'mac') {
return 'https://vault.bitwarden.com/download/?app=cli&platform=macos'
}
if (window.platformType === 'windows') {
return 'https://vault.bitwarden.com/download/?app=cli&platform=windows'
}
return 'https://vault.bitwarden.com/download/?app=cli&platform=linux'
}

getLocalPath () {
return path.join(window.globalArgs['user-data-path'], 'tools', (platformType === 'windows' ? 'bw.exe' : 'bw'))
return join(window.globalArgs['user-data-path'], 'tools', (platformType === 'windows' ? 'bw.exe' : 'bw'))
}

getSetupMode () {
Expand All @@ -41,7 +42,7 @@ class Bitwarden {
try {
await fs.promises.access(localPath, fs.constants.X_OK)
local = true
} catch (e) { }
} catch { }
if (local) {
return localPath
}
Expand Down Expand Up @@ -71,7 +72,7 @@ class Bitwarden {

// Tries to get a list of credential suggestions for a given domain name.
async getSuggestions (domain) {
if (this.lastCallList[domain] != null) {
if (this.lastCallList[domain]) {
return this.lastCallList[domain]
}

Expand All @@ -84,32 +85,24 @@ class Bitwarden {
throw new Error()
}

this.lastCallList[domain] = this.loadSuggestions(command, domain).then(suggestions => {
this.lastCallList[domain] = null
return suggestions
}).catch(ex => {
this.lastCallList[domain] = null
})

this.lastCallList[domain] = await this.loadSuggestions(command, domain)
return this.lastCallList[domain]
}

// Loads credential suggestions for given domain name.
async loadSuggestions (command, domain) {
try {
const process = new ProcessSpawner(command, ['list', 'items', '--url', this.sanitize(domain), '--session', this.sessionKey])
const data = await process.execute()

const matches = JSON.parse(data)
const credentials = matches.map(match => {
const { login: { username, password } } = match
return { username, password, manager: 'Bitwarden' }
})

return credentials
} catch (ex) {
const { error, data } = ex
console.error('Error accessing Bitwarden CLI. STDOUT: ' + data + '. STDERR: ' + error)
const process = new ProcessSpawner(
command,
['list', 'items', '--url', domain.replace(/[^a-zA-Z0-9.-]/g, ''), '--session', this.sessionKey]
)
const matches = JSON.parse(await process.execute())
return matches.map(
({ login: { username, password } }) =>
({ username, password, manager: 'Bitwarden' })
)
} catch ({ error, data }) {
console.error(`Error accessing Bitwarden CLI. STDOUT: ${data}. STDERR: ${error}`)
return []
}
}
Expand All @@ -118,9 +111,8 @@ class Bitwarden {
try {
const process = new ProcessSpawner(command, ['sync', '--session', this.sessionKey])
await process.execute()
} catch (ex) {
const { error, data } = ex
console.error('Error accessing Bitwarden CLI. STDOUT: ' + data + '. STDERR: ' + error)
} catch ({ error, data }) {
console.error(`Error accessing Bitwarden CLI. STDOUT: ${data}. STDERR: ${error}`)
}
}

Expand All @@ -138,17 +130,17 @@ class Bitwarden {
await this.forceSync(this.path)

return true
} catch (ex) {
const { error, data } = ex
} catch (err) {
const { error, data } = err

console.error('Error accessing Bitwarden CLI. STDOUT: ' + data + '. STDERR: ' + error)
console.error(`Error accessing Bitwarden CLI. STDOUT: ${data}. STDERR: ${error}`)

if (error.includes('not logged in')) {
await this.signInAndSave()
return await this.unlockStore(password)
}

throw ex
throw err
}
}

Expand All @@ -161,41 +153,43 @@ class Bitwarden {
console.warn(e)
}

// show credentials dialog

var signInFields = [
// show ask-for-credential dialog
const signInFields = [
{ placeholder: 'Server URL (Leave blank for the default Bitwarden server)', id: 'url', type: 'text' },
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be a translated string - add it to en-US.json, then do l(stringName) like the examples below.
Actually, maybe "client ID" and "Client Secret" should be translated also, although I think the reason not to translate them was so that it would be clear in Bitwarden what we're referring to.

{ placeholder: 'Client ID', id: 'clientID', type: 'password' },
{ placeholder: 'Client Secret', id: 'clientSecret', type: 'password' }
]

const credentials = ipcRenderer.sendSync('prompt', {
text: l('passwordManagerBitwardenSignIn'),
values: signInFields,
ok: l('dialogConfirmButton'),
cancel: l('dialogSkipButton'),
width: 500,
height: 260
})

for (const key in credentials) {
if (credentials[key] === '') {
throw new Error('no credentials entered')
const credentials = ipcRenderer.sendSync(
'prompt',
{
text: l('passwordManagerBitwardenSignIn'),
values: signInFields,
ok: l('dialogConfirmButton'),
cancel: l('dialogSkipButton'),
width: 500,
height: 260
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dialog is a little bit too short now, at least on macOS:

}
)

if (credentials.clientID === '' || credentials.clientSecret === '') {
throw new Error('no credentials entered')
}

const process = new ProcessSpawner(path, ['login', '--apikey'], {
BW_CLIENTID: credentials.clientID.trim(),
BW_CLIENTSECRET: credentials.clientSecret.trim()
})
credentials.url = credentials.url || 'bitwarden.com'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wasn't clear to me whether I was supposed to type "bitwarden.com" or "vault.bitwarden.com" - do they both work? If not, maybe we should special-case this so if the user enters one we convert to the other.


await process.execute()
const process1 = new ProcessSpawner(path, ['config', 'server', credentials.url.trim()])
await process1.execute()

return true
}

// Basic domain name cleanup. Removes any non-ASCII symbols.
sanitize (domain) {
return domain.replace(/[^a-zA-Z0-9.-]/g, '')
const process2 = new ProcessSpawner(
path,
['login', '--apikey'],
{
BW_CLIENTID: credentials.clientID.trim(),
BW_CLIENTSECRET: credentials.clientSecret.trim()
}
)
await process2.execute()
}
}

Expand Down
51 changes: 29 additions & 22 deletions js/passwordManager/passwordCapture.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ const passwordCapture = {
closeButton: document.getElementById('password-capture-ignore'),
currentDomain: null,
barHeight: 0,
showCaptureBar: function (username, password) {

showCaptureBar (username, password) {
passwordCapture.description.textContent = l('passwordCaptureSavePassword').replace('%s', passwordCapture.currentDomain)
passwordCapture.bar.hidden = false

Expand All @@ -27,15 +28,17 @@ const passwordCapture = {
passwordCapture.barHeight = passwordCapture.bar.getBoundingClientRect().height
webviews.adjustMargin([passwordCapture.barHeight, 0, 0, 0])
},
hideCaptureBar: function () {

hideCaptureBar () {
webviews.adjustMargin([passwordCapture.barHeight * -1, 0, 0, 0])

passwordCapture.bar.hidden = true
passwordCapture.usernameInput.value = ''
passwordCapture.passwordInput.value = ''
passwordCapture.currentDomain = null
},
togglePasswordVisibility: function () {

togglePasswordVisibility () {
if (passwordCapture.passwordInput.type === 'password') {
passwordCapture.passwordInput.type = 'text'
passwordCapture.revealButton.classList.remove('carbon:view')
Expand All @@ -46,8 +49,9 @@ const passwordCapture = {
passwordCapture.revealButton.classList.remove('carbon:view-off')
}
},
handleRecieveCredentials: function (tab, args, frameId) {
var domain = args[0][0]

async handleReceivedCredentials (tab, args, frameId) {
let domain = args[0][0]
if (domain.startsWith('www.')) {
domain = domain.slice(4)
}
Expand All @@ -56,34 +60,37 @@ const passwordCapture = {
return
}

var username = args[0][1] || ''
var password = args[0][2] || ''
try {
const username = args[0][1] || ''
const password = args[0][2] || ''

PasswordManagers.getConfiguredPasswordManager().then(function (manager) {
const manager = await PasswordManagers.getConfiguredPasswordManager()
if (!manager || !manager.saveCredential) {
// the password can't be saved
return
}

// check if this username/password combo is already saved
manager.getSuggestions(domain).then(function (credentials) {
var alreadyExists = credentials.some(cred => cred.username === username && cred.password === password)
if (!alreadyExists) {
if (!passwordCapture.bar.hidden) {
passwordCapture.hideCaptureBar()
}

passwordCapture.currentDomain = domain
passwordCapture.showCaptureBar(username, password)
const credentials = await manager.getSuggestions(domain)
const alreadyExists = credentials.some(cred => cred.username === username && cred.password === password)
if (!alreadyExists) {
if (!passwordCapture.bar.hidden) {
passwordCapture.hideCaptureBar()
}
})
})

passwordCapture.currentDomain = domain
passwordCapture.showCaptureBar(username, password)
}
} catch (e) {
console.error(`Failed to get password suggestions: ${e.message}`)
}
},
initialize: function () {

initialize () {
passwordCapture.usernameInput.placeholder = l('username')
passwordCapture.passwordInput.placeholder = l('password')

webviews.bindIPC('password-form-filled', passwordCapture.handleRecieveCredentials)
webviews.bindIPC('password-form-filled', passwordCapture.handleReceivedCredentials)

passwordCapture.saveButton.addEventListener('click', function () {
if (passwordCapture.usernameInput.checkValidity() && passwordCapture.passwordInput.checkValidity()) {
Expand All @@ -106,7 +113,7 @@ const passwordCapture = {
// the bar can change height when the window is resized, so the webview needs to be resized in response
window.addEventListener('resize', function () {
if (!passwordCapture.bar.hidden) {
var oldHeight = passwordCapture.barHeight
const oldHeight = passwordCapture.barHeight
passwordCapture.barHeight = passwordCapture.bar.getBoundingClientRect().height
webviews.adjustMargin([passwordCapture.barHeight - oldHeight, 0, 0, 0])
}
Expand Down
Loading