1621054 - Please add UITour permission for monitor.firefox.com

Status: RESOLVED FIXED

(Firefox :: Tours, task, P1)

Description

[Lesley Norton [:lnorton]]

Apologies as I'm not sure if this would be more appropriately filed somewhere else but please add Firefox Monitor (monitor.firefox.com) to the list of allowed origins for UI Tour. We have multiple Lockwise entrypoints and would like to be able to show desktop Firefox users how to find and open about:logins.

Comment 1

[Matthew N. [:MattN]]

I discussed this with Luke on Slack many months ago and I'm hesitant to add Monitor to the list since it does a lot more than what Monitor needs, OTOH I see that https://screenshots.firefox.com is on the list.

Dan, do you know our current stance on whether UITour origins should be on extensions.webextensions.restrictedDomains? It seems like https://screenshots.firefox.com isn't currently. Do you have opinions on whether Monitor should be given UITour permission?

There is tension about whether extensions.webextensions.restrictedDomains should be allowed to use trackers such as Google Analytics but I don't think I ever saw a final decision on that (bug 1578909).

Comment 2

[Daniel Veditz [:dveditz]]

My personal feeling is that we shouldn't have privileged sites at all (it's not fair, it adds attack surface) but currently restrictedDomains is more about the install and remote-troubleshooting permissions, as well as sync/FxA. UITour is fairly benign -- or at least it has been. We keep adding capabilities to it and I don't know what kind of review those go through.

I guess my concern would be more about what new things were added to UITour for Monitor (that any hacked UITour site could also use) than whether Monitor itself should be granted that permission. But also, if Monitor is going to be one of the privileged sites what are the access controls for the site, who can change its content, and what kind of review process those changes have? I assume monitor has had a RRA. Have things changed since then such that we should do a new RRA or does the old one still cover things?

Having a web page be able to open about:logins gives me a nagging worry. I don't consciously know what that's about yet, but I've learned to pay attention to that feeling. Clickjacking the "Copy" button? Making sure someone's entered their master password before doing some kind of auto-password-fill attack (then using XSS on the victim sites)?

Comment 3

[Matthew N. [:MattN]]

(In reply to Daniel Veditz [:dveditz] from comment #2)

My personal feeling is that we shouldn't have privileged sites at all (it's not fair, it adds attack surface) but currently restrictedDomains is more about the install and remote-troubleshooting permissions, as well as sync/FxA. UITour is fairly benign -- or at least it has been. We keep adding capabilities to it and I don't know what kind of review those go through.

I watch the changes to the directory and usually review the changes but occasionally I don't get flagged e.g. Mozilla.UITour.showProtectionReport

I guess my concern would be more about what new things were added to UITour for Monitor (that any hacked UITour site could also use) than whether Monitor itself should be granted that permission.

I don't think we will add anything new for Monitor.

But also, if Monitor is going to be one of the privileged sites what are the access controls for the site, who can change its content, and what kind of review process those changes have? I assume monitor has had a RRA. Have things changed since then such that we should do a new RRA or does the old one still cover things?

I'll let Lesley answer this.

Having a web page be able to open about:logins gives me a nagging worry. I don't consciously know what that's about yet, but I've learned to pay attention to that feeling. Clickjacking the "Copy" button? Making sure someone's entered their master password before doing some kind of auto-password-fill attack (then using XSS on the victim sites)?

UITour doesn't currently allow opening about:logins directly, I pushed back against that when it was wanted for https://www.mozilla.org/firefox/lockwise/ as I was also concerned about clickjacking issues. Instead it can open the menu with the Logins and Passwords menu item highlighted (click "Open in Firefox" on that page). Since it requires a click before opening about:logins, do you think it's fine to grant the uitour permission if the access control to the Monitor repo is locked down?

I had suggested to Luke that an alternative would be using a WebChannel specific to https://monitor.firefox.com with only the ability to highlight the menu though that may be more complicated since UITour assumes it was invoked from the DOM API.

Comment 4

[Lesley Norton [:lnorton]]

Hey sorry for the latent response here- I was out most of last week and am still getting caught up. Everything Matt said is correct, we're not looking to add any new features to UITour, just mimic what happens on https://www.mozilla.org/en-US/firefox/lockwise/ when you click the "Open in Firefox" button.

But also, if Monitor is going to be one of the privileged sites what are the access controls for the site, who can change its content, and what kind of review process those changes have?

Code and content changes from non-admins must be reviewed/approved before they can be merged to master and there are only a handful of people (all Mozilla employees) who contribute patches to Monitor in any case since we can't give out our HIBP API key.

I assume monitor has had a RRA. Have things changed since then such that we should do a new RRA or does the old one still cover things?

Monitor had an RRA and it should still cover things but we'd of course be happy to do another one if necessary.

Comment 5

[Matthew N. [:MattN]]

So Dan, which would you prefer? Giving Monitor access to the existing UITour API surface or making a new custom API service via WebChannel (same thing used by accounts.firefox.com) that is only granted to the Monitor origin? Neither will allow directly opening about:logins, both will simply highlight the Logins and Passwords item in the main menu.

The WebChannel listener would run the following code:

let loginsTarget = await UITour.getTarget(window, "logins");
await UITour.showHighlight(
            window,
            loginsTarget,
            "none",
            { autohide: true }
);

Comment 6

[Daniel Veditz [:dveditz]]

These limitations sound reasonable to let Monitor use the UITour permission. Glad it's not opening about:logins directly.

Comment 7

[Daniel Veditz [:dveditz]]

Oh, and the second part of the original question was about whether UITour sites need to be on RestrictedDomains for WebExtensions. So far we've kept UITour abilities fairly benign so I don't think we need to.

Comment 8

[Matthew N. [:MattN]]

Thanks Dan. Lesley, do you want to make the addition since you're in m-c now anyways? browser/app/permissions is file to add the UITour line.

During development on the Monitor website you should use the browser.uitour.testingOrigins preference to add non-prod domains (they must be https: unless you also toggle browser.uitour.requireSecure). You can retrieve the latest version of the UITour library from https://hg.mozilla.org/mozilla-central/raw-file/tip/browser/components/uitour/UITour-lib.js

Comment 9

[Matthew N. [:MattN]]

Attachment: Bug 1621054 - Add UITour permission for monitor.firefox.com. r=jaws

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3b6d27247515
Add UITour permission for monitor.firefox.com. r=jaws

Comment 11

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/3b6d27247515