Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Commit

Permalink
Fill multiple password forms that have the same action
Browse files Browse the repository at this point in the history
Address #3773 comments

Auditors: @bridiver

Test Plan: covered by an automated test
  • Loading branch information
diracdeltas committed Sep 7, 2016
1 parent c16aa52 commit 0303926
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 9 deletions.
27 changes: 19 additions & 8 deletions app/extensions/brave/content/scripts/passwordManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

if (chrome.contentSettings.passwordManager == 'allow') {
let credentials = {}
function savePassword(username/*: ?string*/, pw/*: string*/, origin/*: string*/, action/*: string*/) {
function savePassword (username/*: ?string*/, pw/*: string*/, origin/*: string*/, action/*: string*/) {
chrome.ipc.send('save-password', username, pw, origin, action)
}

Expand All @@ -28,12 +28,10 @@ if (chrome.contentSettings.passwordManager == 'allow') {
/**
* Try to autofill a form with credentials, using roughly the heuristic from
* http://mxr.mozilla.org/firefox/source/toolkit/components/passwordmgr/src/nsLoginManager.js
* @param {Object.<string, Array.<Element>>} credentials - map of form action
* to password/username elements with that action
* @param {string} formOrigin - origin of the form
* @param {Element} form - the form node
*/
function tryAutofillForm (credentials, formOrigin, form) {
function tryAutofillForm (formOrigin, form) {
var fields = getFormFields(form, false)
var action = form.action || document.location.href
action = normalizeURL(action)
Expand All @@ -44,11 +42,24 @@ if (chrome.contentSettings.passwordManager == 'allow') {
return
}

if (credentials[action]) {
return
if (Array.isArray(credentials[action])) {
let isDuplicate = true
credentials[action].forEach((elems) => {
if (isDuplicate === false) {
return
} else if (elems[0] !== passwordElem || elems[1] !== usernameElem) {
isDuplicate = false
}
})
if (isDuplicate === false) {
credentials[action].push([passwordElem, usernameElem])
} else {
return
}
} else {
credentials[action] = [[passwordElem, usernameElem]]
}

credentials[action] = [[passwordElem, usernameElem]]
// Fill the password immediately if there's only one or if the username
// is already autofilled
chrome.ipc.send('get-passwords', formOrigin, action)
Expand Down Expand Up @@ -123,7 +134,7 @@ if (chrome.contentSettings.passwordManager == 'allow') {
var formNodes = document.querySelectorAll('form')

Array.from(formNodes).forEach((form) => {
tryAutofillForm(credentials, formOrigin, form)
tryAutofillForm(formOrigin, form)
})

chrome.ipc.on('got-password', (e, username, password, origin, action, isUnique) => {
Expand Down
20 changes: 19 additions & 1 deletion test/components/notificationBarTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ describe('notificationBar', function () {
this.loginUrl2 = Brave.server.url('login2.html')
this.loginUrl3 = Brave.server.url('login3.html')
this.loginUrl4 = Brave.server.url('login4.html')
this.loginUrl5 = Brave.server.url('login5.html')
yield setup(this.app.client)
})

Expand Down Expand Up @@ -124,7 +125,24 @@ describe('notificationBar', function () {
.loadUrl(this.loginUrl4)
.waitUntil(function () {
return this.getValue('#user').then((val) => val === 'brave_user') &&
this.getValue('#password').then((val) => val === 'testing')
this.getValue('#password').then((val) => val === 'testing') &&
this.getValue('#user2').then((val) => val === '') &&
this.getValue('#password2').then((val) => val === '')
})
})

it('autofills remembered password on login page with multiple forms', function * () {
yield this.app.client
.tabByIndex(0)
.loadUrl(this.loginUrl1)
.windowByUrl(Brave.browserWindowUrl)
.tabByIndex(0)
.loadUrl(this.loginUrl5)
.waitUntil(function () {
return this.getValue('#user').then((val) => val === 'brave_user') &&
this.getValue('#password').then((val) => val === 'testing') &&
this.getValue('#user2').then((val) => val === 'brave_user') &&
this.getValue('#password2').then((val) => val === 'testing')
})
})

Expand Down
16 changes: 16 additions & 0 deletions test/fixtures/login4.html
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,21 @@ <h1>Login</h1>
</div>
</form>
</div>
<div id="content2" class="login">
<h1>Login 2</h1>
<form method="post" action="/blah"><div><input type="hidden" name="__FORM_TOKEN" value="foo" /></div>
<div class="textbox">
<label for="user">Username:</label><br />
<input type="text" name="user" id='user2' class="textwidget" />
</div>
<div class="textbox">
<label for="password">Password:</label><br />
<input type="password" id='password2' name="password" class="textwidget" />
</div>
<div class="buttons central nav">
<input type="submit" value="Login" />
</div>
</form>
</div>
</body>
</html>
40 changes: 40 additions & 0 deletions test/fixtures/login5.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<html>
<head>
<meta charset="utf-8">
<title>Login</title>
</head>
<body>
<div id="content1" class="login">
<h1>Login 1</h1>
<form method="post" action="/"><div><input type="hidden" name="__FORM_TOKEN" value="foo" /></div>
<div class="textbox">
<label for="user">Username:</label><br />
<input type="text" id="user" name="user" class="textwidget" />
</div>
<div class="textbox">
<label for="password">Password:</label><br />
<input type="password" id="password" name="password" class="textwidget" />
</div>
<div id="login_options" class="buttons central nav">
<input id="submit" type="submit" value="Login" />
</div>
</form>
</div>
<div id="content2" class="login">
<h1>Login 2</h1>
<form method="post" action="/"><div><input type="hidden" name="__FORM_TOKEN" value="foo" /></div>
<div class="textbox">
<label for="user">Username:</label><br />
<input type="text" name="user" id='user2' class="textwidget" />
</div>
<div class="textbox">
<label for="password">Password:</label><br />
<input type="password" id='password2' name="password" class="textwidget" />
</div>
<div class="buttons central nav">
<input type="submit" value="Login" />
</div>
</form>
</div>
</body>
</html>

0 comments on commit 0303926

Please sign in to comment.