Skip to content

Commit

Permalink
[PM-3574] Fix leak of login credentials to foreign origin due to race…
Browse files Browse the repository at this point in the history
… condition during autofill (#6700)

* [PM-3574] Fix leak of login credentials to foreign origin due to race condition during autofill

* [PM-3574] Adding a temporary artificial delay to facilitate QA testing

* [PM-3574] Adding a temporary artificial delay to facilitate QA testing

* [PM-4590] Cached Page Details of Formless Input Fields Breaks Autofill

* [PM-3574] Reworking implementation to take into account the page details url

* [PM-3574] Fixing jest tests

* [PM-3574] Fixing jest tests

* [PM-3574] Removing 5 second delay on autofill
  • Loading branch information
cagonzalezcs authored Nov 15, 2023
1 parent 90bad00 commit 8e047f6
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ type AutofillExtensionMessage = {
tab?: chrome.tabs.Tab;
sender?: string;
fillScript?: AutofillScript;
url?: string;
pageDetailsUrl?: string;
};

type AutofillExtensionMessageHandlers = {
Expand Down
29 changes: 26 additions & 3 deletions apps/browser/src/autofill/content/autofill-init.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,36 @@ describe("AutofillInit", () => {
});

describe("fillForm", () => {
it("will call the InsertAutofillContentService to fill the form", () => {
const fillScript = mock<AutofillScript>();
beforeEach(() => {
jest
.spyOn(bitwardenAutofillInit.insertAutofillContentService, "fillForm")
.mockImplementation();
});

it("skips calling the InsertAutofillContentService and does not fill the form if the url to fill is not equal to the current tab url", () => {
const fillScript = mock<AutofillScript>();
const message = {
command: "fillForm",
fillScript,
pageDetailsUrl: "https://a-different-url.com",
};

bitwardenAutofillInit.fillForm(message);

expect(bitwardenAutofillInit.insertAutofillContentService.fillForm).not.toHaveBeenCalledWith(
fillScript
);
});

it("will call the InsertAutofillContentService to fill the form", () => {
const fillScript = mock<AutofillScript>();
const message = {
command: "fillForm",
fillScript,
pageDetailsUrl: window.location.href,
};

bitwardenAutofillInit.fillForm(fillScript);
bitwardenAutofillInit.fillForm(message);

expect(bitwardenAutofillInit.insertAutofillContentService.fillForm).toHaveBeenCalledWith(
fillScript
Expand Down
13 changes: 8 additions & 5 deletions apps/browser/src/autofill/content/autofill-init.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import AutofillPageDetails from "../models/autofill-page-details";
import AutofillScript from "../models/autofill-script";
import CollectAutofillContentService from "../services/collect-autofill-content.service";
import DomElementVisibilityService from "../services/dom-element-visibility.service";
import InsertAutofillContentService from "../services/insert-autofill-content.service";
Expand All @@ -17,7 +16,7 @@ class AutofillInit implements AutofillInitInterface {
private readonly extensionMessageHandlers: AutofillExtensionMessageHandlers = {
collectPageDetails: ({ message }) => this.collectPageDetails(message),
collectPageDetailsImmediately: ({ message }) => this.collectPageDetails(message, true),
fillForm: ({ message }) => this.fillForm(message.fillScript),
fillForm: ({ message }) => this.fillForm(message),
};

/**
Expand Down Expand Up @@ -76,10 +75,14 @@ class AutofillInit implements AutofillInitInterface {

/**
* Fills the form with the given fill script.
* @param {AutofillScript} fillScript
* @private
*
* @param {AutofillExtensionMessage} message
*/
private fillForm(fillScript: AutofillScript) {
private fillForm({ fillScript, pageDetailsUrl }: AutofillExtensionMessage) {
if ((document.defaultView || window).location.href !== pageDetailsUrl) {
return;
}

this.insertAutofillContentService.fillForm(fillScript);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ describe("AutofillService", () => {
untrustedIframe: false,
},
url: currentAutofillPageDetails.tab.url,
pageDetailsUrl: "url",
},
{
frameId: currentAutofillPageDetails.frameId,
Expand Down
1 change: 1 addition & 0 deletions apps/browser/src/autofill/services/autofill.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ export default class AutofillService implements AutofillServiceInterface {
command: "fillForm",
fillScript: fillScript,
url: tab.url,
pageDetailsUrl: pd.details.url,
},
{ frameId: pd.frameId }
);
Expand Down

0 comments on commit 8e047f6

Please sign in to comment.