653307 - Adjust reCAPTCHA NoScript hack in Sync

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Richard Newman [:rnewman]]

Sync currently adds recaptcha.net to the NoScript whitelist during setup. That URL is no longer valid (they're using google.com now), and NoScript has appropriate entries in its own whitelist.

Furthermore, we already have a fallback for disabled JS, so our workaround code is unnecessary. KILL THE HEATHEN CODE!

Comment 1

[Richard Newman [:rnewman]]

Attachment: Screenshot.

Let's just say that the user experience with NoScript is less than ideal.

Comment 2

[Richard Newman [:rnewman]]

Attachment: With exception added.

Taking a look at this scrollbar annoyance.

Comment 3

[Richard Newman [:rnewman]]

Attachment: The best I can do...

Have to force a minwidth of 700 on the wizard as whole. Let's hope this looks OK on Windows and Linux…

Comment 4

[Richard Newman [:rnewman]]

Attachment: Alter URL for whitelist. v1

This temporarily adds google.com to the whitelist.

Comment 5

[Richard Newman [:rnewman]]

STR:

* New profile.
* Install NoScript: https://addons.mozilla.org/en-US/firefox/addon/noscript/
* Restart as required.
* Set up Sync. Captcha page in the wizard should look like Attachment 528915 [details], not Attachment 528899 [details].

Comment 6

[Philipp von Weitershausen [:philikon]]

Comment on attachment 528945 [details] [diff] [review]
Alter URL for whitelist. v1

>+// Broader than we'd like, but after this changed from api-secure.recaptcha.net
>+// we had no choice. At least we only do this for the duration of setup.
>+const RECAPTCHA_DOMAIN = "https://www.google.com";
>+

This makes me sad, but apparently NoScript does indeed not allow us to restrict it further as Giorgio writes in bug 508112 comment 14:

  Yes it does. A "site" for NoScript must be either a prePath (in nsIURI terms)
  or a host.

>-  _remoteSites: [Weave.Service.serverURL, "https://api-secure.recaptcha.net"],
>+  _remoteSites: [Weave.Service.serverURL, RECAPTCHA_DOMAIN],

Interesting (pre-existing) bug here: We will always whitelist only auth.s.m.c. If the user changes 'serverURL' by typing in a custom server URL and that custom server also requires a captcha (the minimal server doesn't, but conceivably others could), we never whitelist it. Fix would be to make this a dynamic getter always:

  get _remoteSites(): [Weave.Service.serverURL, RECAPTCHA_DOMAIN],

r=me with that. Please request approval for aurora and 2.0 for the updated patch.

Comment 7

[Richard Newman [:rnewman]]

Pushed to services:

  https://hg.mozilla.org/services/services-central/rev/c01dba4456f9

Comment 8

[Richard Newman [:rnewman]]

Comment on attachment 528945 [details] [diff] [review]
Alter URL for whitelist. v1

Requesting approval for Aurora and 4.0.2 for this patch:

  https://hg.mozilla.org/services/services-central/rev/c01dba4456f9

Justification: without this updated URL, NoScript users will have a degraded Sync signup experience, as shown in Attachment 528899 [details].

Comment 9

[Tracy Walker [:tracy]]

Verified with s-c builds of 20110502 and NoScript 2.1.0.3 installed

Comment 10

[Philipp von Weitershausen [:philikon]]

http://hg.mozilla.org/mozilla-central/rev/c01dba4456f9

Comment 11

[Richard Newman [:rnewman]]

Pushed to Aurora:

  http://hg.mozilla.org/mozilla-aurora/rev/2b6445bfa3e9

Comment 12

[Daniel Veditz [:dveditz]]

Comment on attachment 528945 [details] [diff] [review]
Alter URL for whitelist. v1

Not planning any more 2.0 releases, no point in landing this there.