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

do not accidentally show password as username #7653

Merged
merged 2 commits into from
Mar 12, 2017
Merged
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
10 changes: 10 additions & 0 deletions app/extensions/brave/content/scripts/passwordManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,16 @@ if (chrome.contentSettings.passwordManager == 'allow') {
// Last resort: find the first text input in the form
username = username || form.querySelector('input[type=text i]')

// If the username turns out to be a password field, just ignore it so
// we don't show the password in plaintext.
if (username) {
let autocomplete = username.getAttribute('autocomplete')
if (username.getAttribute('type') === 'password' ||
(autocomplete && autocomplete.includes('password'))) {
username = null
}
}

// If not a submission, autofill the first password field and ignore the rest
if (!isSubmission || passwords.length === 1) {
return [username instanceof HTMLInputElement ? username : null, passwords[0], null]
Expand Down
64 changes: 48 additions & 16 deletions test/components/notificationBarTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
const Brave = require('../lib/brave')
const {notificationBar, titleBar, urlInput} = require('../lib/selectors')

describe('notificationBar', function () {
describe('notificationBar permissions', function () {
function * setup (client) {
yield client
.waitForBrowserWindow()
Expand All @@ -14,11 +14,6 @@ describe('notificationBar', function () {
Brave.beforeAll(this)
before(function * () {
this.notificationUrl = Brave.server.url('notification.html')
this.loginUrl1 = Brave.server.url('login1.html')
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 @@ -93,22 +88,62 @@ describe('notificationBar', function () {
return this.getText(titleBar).then((val) => val.includes('granted'))
})
})
})

describe('notificationBar passwords', function () {
function * setup (client) {
yield client
.waitForBrowserWindow()
.waitForVisible(urlInput)
}

Brave.beforeAll(this)

beforeEach(function * () {
yield this.app.client
.waitForElementCount('.notificationItem', 0)
})

before(function * () {
this.loginUrl1 = Brave.server.url('login1.html')
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')
this.loginUrl6 = Brave.server.url('login6.html')
yield setup(this.app.client)
})

it('shows notification for login form', function * () {
yield this.app.client
.tabByIndex(0)
.loadUrl(this.loginUrl1)
.url(this.loginUrl1)
.windowByUrl(Brave.browserWindowUrl)
.waitForExist(notificationBar)
.waitUntil(function () {
return this.getText(notificationBar).then((val) => val.includes('localhost') && val.includes('brave_user'))
return this.getText(notificationBar).then((val) => {
return val.includes('localhost') && val.includes('brave_user')
})
}).click('button=No')
})

it('does not include a password in the notification bar', function * () {
yield this.app.client
.tabByIndex(0)
.url(this.loginUrl6)
.windowByUrl(Brave.browserWindowUrl)
.waitForExist(notificationBar)
.waitUntil(function () {
return this.getText(notificationBar).then((val) => {
return val.includes('your password') && !val.includes('secret')
})
}).click('button=No')
})

it('autofills remembered password on login form', function * () {
yield this.app.client
.tabByIndex(0)
.loadUrl(this.loginUrl1)
.url(this.loginUrl1)
.windowByUrl(Brave.browserWindowUrl)
.waitForExist(notificationBar)
.waitUntil(function () {
Expand All @@ -119,7 +154,7 @@ describe('notificationBar', function () {
.waitForExist('[data-test-id="passwordItem"]')
.windowByUrl(Brave.browserWindowUrl)
.tabByIndex(0)
.loadUrl(this.loginUrl4)
.url(this.loginUrl4)
.waitUntil(function () {
return this.getValue('#user').then((val) => val === 'brave_user') &&
this.getValue('#password').then((val) => val === 'testing') &&
Expand All @@ -131,10 +166,7 @@ describe('notificationBar', function () {
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)
.url(this.loginUrl5)
.waitUntil(function () {
return this.getValue('#user').then((val) => val === 'brave_user') &&
this.getValue('#password').then((val) => val === 'testing') &&
Expand All @@ -146,15 +178,15 @@ describe('notificationBar', function () {
it('does not show login form notification if user turns it off for the site', function * () {
yield this.app.client
.tabByIndex(0)
.loadUrl(this.loginUrl3)
.url(this.loginUrl3)
.windowByUrl(Brave.browserWindowUrl)
.waitForExist(notificationBar)
.waitUntil(function () {
return this.getText(notificationBar).then((val) => val.includes('localhost') && val.includes('brave_user'))
})
.click('button=Never for this site')
.tabByIndex(0)
.loadUrl(this.loginUrl2)
.url(this.loginUrl2)
.windowByUrl(Brave.browserWindowUrl)
.isExisting(notificationBar).should.eventually.be.false
})
Expand Down
4 changes: 2 additions & 2 deletions test/fixtures/login1.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@
document.querySelector('#password').value = 'testing'
document.querySelector('#submit').click()
}
}, 1000)
}, 200)
</script>
</head>
<body>
<div id="content" class="login">
<h1>Login</h1>
<form method="post" id="acctmgr_loginform" action="/"><div><input type="hidden" name="__FORM_TOKEN" value="foo" /></div>
<form method="post" id="acctmgr_loginform" action="login4.html"><div><input type="hidden" name="__FORM_TOKEN" value="foo" /></div>
<div>
<input type="hidden" name="referer" value="bar" />
</div>
Expand Down
4 changes: 2 additions & 2 deletions test/fixtures/login2.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@
document.querySelector('#password').value = 'testing'
document.querySelector('#submit').click()
}
}, 1000)
}, 200)
</script>
</head>
<body>
<div id="content" class="login">
<h1>Login</h1>
<form method="post" id="acctmgr_loginform" action=""><div><input type="hidden" name="__FORM_TOKEN" value="foo" /></div>
<form method="post" id="acctmgr_loginform" action="login4.html"><div><input type="hidden" name="__FORM_TOKEN" value="foo" /></div>
<div>
<input type="hidden" name="referer" value="bar" />
</div>
Expand Down
4 changes: 2 additions & 2 deletions test/fixtures/login3.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@
document.querySelector('#password').value = 'testing'
document.querySelector('#submit').click()
}
}, 1000)
}, 200)
</script>
</head>
<body>
<div id="content" class="login">
<h1>Login</h1>
<form method="post" id="acctmgr_loginform" action=""><div><input type="hidden" name="__FORM_TOKEN" value="foo" /></div>
<form method="post" id="acctmgr_loginform" action="login5.html"><div><input type="hidden" name="__FORM_TOKEN" value="foo" /></div>
<div>
<input type="hidden" name="referer" value="bar" />
</div>
Expand Down
4 changes: 2 additions & 2 deletions test/fixtures/login4.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<body>
<div id="content" class="login">
<h1>Login</h1>
<form method="post" id="acctmgr_loginform" action="/"><div><input type="hidden" name="__FORM_TOKEN" value="foo" /></div>
<form method="post" id="acctmgr_loginform" action="login4.html"><div><input type="hidden" name="__FORM_TOKEN" value="foo" /></div>
<div>
<input type="hidden" name="referer" value="bar" />
</div>
Expand All @@ -25,7 +25,7 @@ <h1>Login</h1>
</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>
<form method="post" action="https://brave.com/"><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" />
Expand Down
4 changes: 2 additions & 2 deletions test/fixtures/login5.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<body>
<div id="content1" class="login">
<h1>Login 1</h1>
<form method="post" action="/"><div><input type="hidden" name="__FORM_TOKEN" value="foo" /></div>
<form method="post" action="login4.html"><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" />
Expand All @@ -22,7 +22,7 @@ <h1>Login 1</h1>
</div>
<div id="content2" class="login">
<h1>Login 2</h1>
<form method="post" action="/"><div><input type="hidden" name="__FORM_TOKEN" value="foo" /></div>
<form method="post" action="login4.html"><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" />
Expand Down
21 changes: 21 additions & 0 deletions test/fixtures/login6.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<body>
<script>
window.onload = window.setTimeout(function () {
if (!document.querySelector('#password').value) {
document.querySelector('#password').value = 'secret'
document.querySelector('#old-password').value = 'secret'
document.querySelector('#new-password').value = 'secret2'
document.querySelector('#submit').click()
}
}, 200)
</script>
<form action="/login4.html" name="ChangePassForm" id="ChangePassForm">
<input type="hidden" id="countryId" name="countryId" value="1" />
<input type="hidden" name="MfcISAPICommand" value="HandleNewPassword">
<input type="hidden" name="srt" value="01"><div class="rclSir"></div>
<input id='password' type="password" name="user" />
<input id='old-password' type="password" autocomplete="current-password" />
<input id='new-password' type="password" autocomplete="new-password" />
<input id="submit" type="submit" value="Change password" />
</form>
</body>