From 56537e51ae0db05683c8e5ac983d323853888816 Mon Sep 17 00:00:00 2001 From: yan Date: Sat, 11 Mar 2017 00:29:01 +0000 Subject: [PATCH 1/2] do not accidentally show password as username fix https://github.com/brave/browser-laptop/issues/7649 Test Plan: 1. automated tests for notificationBar should pass 2. changing the password on https://reg.ebay.com/reg/ChangePwd and clicking 'submit' should not show the current password in the notification bar --- .../brave/content/scripts/passwordManager.js | 10 +++++++++ test/components/notificationBarTest.js | 14 +++++++++++++ test/fixtures/login6.html | 21 +++++++++++++++++++ 3 files changed, 45 insertions(+) create mode 100644 test/fixtures/login6.html diff --git a/app/extensions/brave/content/scripts/passwordManager.js b/app/extensions/brave/content/scripts/passwordManager.js index fd24f2a388b..a33d6db8142 100644 --- a/app/extensions/brave/content/scripts/passwordManager.js +++ b/app/extensions/brave/content/scripts/passwordManager.js @@ -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] diff --git a/test/components/notificationBarTest.js b/test/components/notificationBarTest.js index da217957e6a..14410122607 100644 --- a/test/components/notificationBarTest.js +++ b/test/components/notificationBarTest.js @@ -19,6 +19,7 @@ describe('notificationBar', function () { 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) }) @@ -105,6 +106,19 @@ describe('notificationBar', function () { }).click('button=No') }) + it('does not include a password in the notification bar', function * () { + yield this.app.client + .tabByIndex(0) + .loadUrl(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) diff --git a/test/fixtures/login6.html b/test/fixtures/login6.html new file mode 100644 index 00000000000..7e97f9d72c8 --- /dev/null +++ b/test/fixtures/login6.html @@ -0,0 +1,21 @@ + + +
+ + +
+ + + + +
+ From 51d2d585c5d4be4055786cbaebc7c171632c870c Mon Sep 17 00:00:00 2001 From: yan Date: Sat, 11 Mar 2017 03:28:34 +0000 Subject: [PATCH 2/2] make notification bar password tests run faster --- test/components/notificationBarTest.js | 54 +++++++++++++++++--------- test/fixtures/login1.html | 4 +- test/fixtures/login2.html | 4 +- test/fixtures/login3.html | 4 +- test/fixtures/login4.html | 4 +- test/fixtures/login5.html | 4 +- test/fixtures/login6.html | 2 +- 7 files changed, 47 insertions(+), 29 deletions(-) diff --git a/test/components/notificationBarTest.js b/test/components/notificationBarTest.js index 14410122607..8fb330d9f00 100644 --- a/test/components/notificationBarTest.js +++ b/test/components/notificationBarTest.js @@ -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() @@ -14,12 +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') - this.loginUrl6 = Brave.server.url('login6.html') yield setup(this.app.client) }) @@ -94,22 +88,49 @@ 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) - .loadUrl(this.loginUrl6) + .url(this.loginUrl6) .windowByUrl(Brave.browserWindowUrl) .waitForExist(notificationBar) .waitUntil(function () { @@ -122,7 +143,7 @@ describe('notificationBar', function () { 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 () { @@ -133,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') && @@ -145,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') && @@ -160,7 +178,7 @@ 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 () { @@ -168,7 +186,7 @@ describe('notificationBar', function () { }) .click('button=Never for this site') .tabByIndex(0) - .loadUrl(this.loginUrl2) + .url(this.loginUrl2) .windowByUrl(Brave.browserWindowUrl) .isExisting(notificationBar).should.eventually.be.false }) diff --git a/test/fixtures/login1.html b/test/fixtures/login1.html index b7ae1dab716..373da46f8e5 100644 --- a/test/fixtures/login1.html +++ b/test/fixtures/login1.html @@ -9,13 +9,13 @@ document.querySelector('#password').value = 'testing' document.querySelector('#submit').click() } - }, 1000) + }, 200)