Closed Bug 653307 Opened 10 years ago Closed 10 years ago
CAPTCHA No Script hack in Sync
82.10 KB, image/png
65.09 KB, image/png
63.50 KB, image/png
1.62 KB, patch
|Details | Diff | Splinter Review|
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!
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
Summary: Remove redundant NoScript hack from Sync → Remove redundant reCAPTCHA NoScript hack from Sync
Let's just say that the user experience with NoScript is less than ideal.
Taking a look at this scrollbar annoyance.
Summary: Remove redundant reCAPTCHA NoScript hack from Sync → Adjust reCAPTCHA NoScript hack in Sync
Have to force a minwidth of 700 on the wizard as whole. Let's hope this looks OK on Windows and Linux…
This temporarily adds google.com to the whitelist.
Attachment #528945 - Flags: review?(philipp)
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 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.
Attachment #528945 - Flags: review?(philipp) → review+
Pushed to services: https://hg.mozilla.org/services/services-central/rev/c01dba4456f9
Whiteboard: [fixed in services]
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].
Verified with s-c builds of 20110502 and NoScript 22.214.171.124 installed
Whiteboard: [fixed in services] → [verified in services]
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
Attachment #528945 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Pushed to Aurora: http://hg.mozilla.org/mozilla-aurora/rev/2b6445bfa3e9
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.
Attachment #528945 - Flags: approval2.0? → approval2.0-
Component: Firefox Sync: Backend → Sync
Product: Cloud Services → Firefox
You need to log in before you can comment on or make changes to this bug.