CFR Addon Recommendations call remote AMO API before clicking "Install"

VERIFIED FIXED in Firefox 65

Status

()

P1
major
VERIFIED FIXED
2 months ago
21 days ago

People

(Reporter: decoder, Assigned: rrosario)

Tracking

(Blocks: 1 bug, {github-merged, privacy, regression})

Trunk
Firefox 67
github-merged, privacy, regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(relnote-firefox 65+, firefox-esr60 unaffected, firefox65+ verified, firefox66+ verified, firefox67+ verified)

Details

Attachments

(2 attachments)

(Reporter)

Description

2 months ago

It was reported that the contextual addon recommendation immediately makes a request to https://services.addons.mozilla.org/api/v3/addons/addon/<ID>/ when showing the doorhanger, even before the user clicks "Install".

:dholbert confirmed this behavior locally and :tspurway confirmed that this is a bug and the request should only happen when the user clicks "Install". The fact that the request contains the addon ID indirectly leaks what site the user has just visited without any user interaction (only affects the handful of sites hardcoded in the CFR code). Fixing this by putting the request behind the "Install" click should be sufficient from a privacy aspect.

:dholbert also figured out that this is probably a regression from bug 1494275, marking as such.

Severity: normal → major
Iteration: --- → 67.2 - Feb 11 - 24
status-firefox66: --- → affected
Priority: -- → P1
Blocks: 1471328
status-firefox65: --- → affected
tracking-firefox66: --- → ?

Did bug #1494275 introduce the regression? :rrosario, do you want to take a look at this one?

Flags: needinfo?(rrosario)

Specifically, this probably happens right now via the following chain of function calls:

  • addRecommendation() gets called to create the recommendation
  • ...and it calls _maybeAddAddonInstallURL()
  • ...which calls _fetchLatestAddonVersion()
  • ...which pings the addons API with the ID of the addon that we're recommending

Source, for reference:
https://searchfox.org/mozilla-central/rev/03ebbdab952409640c6857d835d3040bf6f9e2db/browser/components/newtab/lib/CFRPageActions.jsm#512

(In reply to Tim Spurway [:tspurway] from comment #1)

Did bug #1494275 introduce the regression?

Based on searchfox hover-attribution, yes. That's where we added the call to _maybeAddAddonInstallURL which is what's (indirectly) triggering this request, per comment 2.

(Assignee)

Updated

2 months ago
Assignee: nobody → rrosario
Flags: needinfo?(rrosario)
status-firefox-esr60: --- → unaffected
tracking-firefox65: --- → +
tracking-firefox66: ? → +
tracking-firefox67: --- → +
Summary: CFR Addon Recommendations call remote API before clicking "Install" → CFR Addon Recommendations call remote AMO API before clicking "Install"
(Assignee)

Updated

2 months ago
Keywords: github-merged

Comment on attachment 9042653 [details]
Bug 1526387 - CFR Addon Recommendations call remote API before clicking "Install"

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

https://bugzilla.mozilla.org/show_bug.cgi?id=1471328

User impact if declined

There is a privacy issue with CFR campaigns that suggest AMO addons leaking the site the user is getting the recommendation from

Is this code covered by automated tests?

Yes

Has the fix been verified in Nightly?

No

Needs manual test from QE?

Yes

If yes, steps to reproduce

  • pref on asrouter dev tools browser.newtabpage.activity-stream.asrouter.devtoolsEnabled
  • go to about:newtab#asrouter
  • open a browser console (Ctrl+Shift+J) and tell it to show XHR
  • using about:newtab#asrouter, force a CFR recommendation to show

Expected Result:

You should not see an XHR for https://services.addons.mozilla.org/api

ALSO:

Please ensure existing CFR actions and functionality as described in the Test Suite: https://goo.gl/jM45bW;

List of other uplifts needed

None

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

CFR is a new feature that doesn't affect all users. If it is broken, the worst that can happen is a missing recommendation. The patch is reasonably small.

String changes made/needed

none

Attachment #9042653 - Flags: approval-mozilla-release?
Attachment #9042653 - Flags: approval-mozilla-beta?

Comment 7

2 months ago
Pushed by rrosario@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/25cbdc59246f
CFR Addon Recommendations call remote API before clicking "Install" r=k88hudson

Comment 8

a month ago
bugherder
Status: NEW → RESOLVED
Last Resolved: a month ago
status-firefox67: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67

Comment on attachment 9042653 [details]
Bug 1526387 - CFR Addon Recommendations call remote API before clicking "Install"

[Triage Comment]
Fixes a CFR privacy issue, approved for 66.0b7 and 65.0.1.

Attachment #9042653 - Flags: approval-mozilla-release?
Attachment #9042653 - Flags: approval-mozilla-release+
Attachment #9042653 - Flags: approval-mozilla-beta?
Attachment #9042653 - Flags: approval-mozilla-beta+
Flags: needinfo?(ciprian.muresan)
relnote-firefox: --- → 65+

Comment 10

a month ago
bugherderuplift
status-firefox65: affected → fixed
Flags: in-testsuite+
Flags: qe-verify+

Comment 11

a month ago
bugherderuplift
status-firefox66: affected → fixed
Whiteboard: [qa-triaged]

I have verified that the issue no longer reproducible on the latest Nightly 67.0a1 (Build ID 20190211215545), latest Beta 66.0b7 (Build ID 20190211185957), and Release 65.0.1 (Build ID 20190211233335) on Windows 10 x64, Mac 10.14, and Arch Linux 4.14.3.
There is no XHR call when the recommendation appears or after it is clicked; the XHR request is made only when the "Add Now" button is clicked.

Status: RESOLVED → VERIFIED
status-firefox65: fixed → verified
status-firefox66: fixed → verified
status-firefox67: fixed → verified
Flags: qe-verify+
Flags: needinfo?(ciprian.muresan)

Updated

a month ago
Blocks: 1527504

Updated

a month ago
No longer blocks: 1527504

Updated

a month ago
Blocks: 1528119

Updated

a month ago
No longer blocks: 1528119
QA Whiteboard: [qa-triaged]
Whiteboard: [qa-triaged]
You need to log in before you can comment on or make changes to this bug.