CAPTCHA doesn't load when certain extensions are installed

VERIFIED FIXED in 1.2

Status

P3
normal
VERIFIED FIXED
9 years ago
8 years ago

People

(Reporter: hello, Assigned: mconnor)

Tracking

unspecified
Points:
---
Dependency tree / graph
Bug Flags:
blocking-weave1.0 -
blocking-weave1.2 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

9 years ago
The CAPTCHA doesn't load when some extensions (e.g. NoScript, Adblock Plus, etc) are installed.

Weave should figure out how to bypass or add itself to the whitelist so we can side-step this problem.

As a temporary measure, we should add a note to the wizard.
(Reporter)

Updated

9 years ago
Flags: blocking-weave1.0+
(Reporter)

Updated

9 years ago
Assignee: nobody → anant
Created attachment 393723 [details] [diff] [review]
Fallback to <noscript> for Captcha

The patch makes captcha display as-is in an iframe, instead of loading it in a seperate XUL <browser> and extracting the image out of it. This has two effects:
- We now comply with ReCaptcha's terms of service which states that the ReCaptcha wordmark must be present in all deployments.
- We fallback to a plain HTML page in case Javascript is disabled (using the <noscript> tag), which asks the user to perform additional steps to validate.

I looked at how we can circumvent NoScript while the captcha is loading but this proves to be difficult, because:
- Adding recaptcha.net to the whitelist does not help.
- Temporarily disabling NoScript via the preferences requires a browser restart to take effect, and I couldn't find any other way to do it.

In it's current form, the patch will allow users to register even when they have NoScript installed, though the process is a bit cumbersome. AdBlockPlus did not have any effect on the captcha in all my tests.
Attachment #393723 - Flags: review?(thunder)

Updated

9 years ago
Priority: -- → P1
(Reporter)

Updated

9 years ago
Attachment #393723 - Flags: review?(thunder) → review+
(Reporter)

Comment 2

9 years ago
Comment on attachment 393723 [details] [diff] [review]
Fallback to <noscript> for Captcha

I'm OK with this patch, but we should sync up tomorrow.  The switch to about:weave makes this patch sorta obsolete.

I copied some of it into about:weave, but have other issues with the captcha there (it's so big).

Updated

9 years ago
Target Milestone: 0.6 → 0.7

Comment 3

9 years ago
We should also try talking to the NoScript developers to see if they can whitelist chrome:// or about: URLs.
Priority: P1 → P2
(Reporter)

Updated

9 years ago
Priority: P2 → P3

Updated

9 years ago
Assignee: anant → mconnor
Target Milestone: 0.7 → 0.8
(Assignee)

Comment 4

9 years ago
I'm not sure how this patch ever worked.  The manual challenge stuff doesn't seem to work sanely...
(Assignee)

Updated

9 years ago
Target Milestone: 0.8 → 0.9
Target Milestone: 1.0beta → 1.0
We should contact the NoScript devs to see what they did before and if we can just do that again to avoid it being blocked.
Target Milestone: 1.0 → 1.0 beta2
Target Milestone: 1.0 beta2 → 1.0
(Assignee)

Comment 6

9 years ago
Discussed with rags, we're going to punt on this for 1.0.  We have a warning for users going into the setup, which seems to have deflected most of this.  It's not awesome, but it's good enough to ship.

cc-ing mao, to see if there's something easy we can do to make noscript ignore this <browser> element.
Flags: blocking-weave1.0+ → blocking-weave1.0-
Target Milestone: 1.0 → 1.1

Comment 7

9 years ago
I'm checking what can be done, but if you're importing a 3rd party web script over an insecure connection, I guess most paranoid users won't be happy of this potentially hijackable source being hardcoded in the whitelist, possibly without a mean to remove it.

On the other hand, I know first hand (my blog) that recaptcha features a decent noscript fallback. 
Why doesn't it work for you (per comment 4)?

Comment 8

9 years ago
OK, I checked and I've seen why the scriptless recaptcha is ugly and disfunctional in your wizard.

Good news is that you can easily work around by temporarily whitelisting the following "secure" sources:

https://auth.services.mozilla.com
https://api-secure.recaptcha.net
(Assignee)

Comment 9

9 years ago
Giorgio, is there a simple API we can call to do that in NoScript?  I couldn't find my way through the source to find a way to do that.
Hope this help:

<SNIP>

var ns = Components.classes["@maone.net/noscript-service;1"]
  .getService().wrappedJSObject;

var disabled = [];
["https://auth.services.mozilla.com", "https://api-secure.recaptcha.net"]
  .forEach(function(site) {
  if (!ns.isJSEnabled(site)) {
    disabled.push(site); // save status
    ns.setJSEnabled(site, true); // allow site
  }
});

// do whatever you need to...

//... and finally reset permissions
disabled.forEach(function(site) {
  ns.setJSEnabled(site, false);
});

</SNIP>
(Assignee)

Updated

9 years ago
Duplicate of this bug: 442441
(Assignee)

Updated

9 years ago
Flags: blocking-weave1.2+
Target Milestone: 1.1 → 1.2
(Assignee)

Comment 12

9 years ago
Created attachment 433408 [details] [diff] [review]
fix

adds the selected weave server + api-secure.recaptcha.net to the whitelist, if it's not added already, and cleans up on successful account creation or wizard cancel.
Attachment #393723 - Attachment is obsolete: true
Attachment #433408 - Flags: review?(edilee)
(Assignee)

Updated

9 years ago
Whiteboard: [has patch][needs review Mardak]
Comment on attachment 433408 [details] [diff] [review]
fix

>+++ b/source/chrome/content/preferences/fx-setup.js
>@@ -211,16 +204,53 @@ var gWeaveSetup = {
>+  get _remoteSites() {
>+    let serverURL = Weave.Service.serverURL;
>+    // kinda lame, but NoScript chokes on the trailing slash
>+    if (serverURL[serverURL.length - 1] == "/")
>+      serverURL = serverURL.substr(0, serverURL.length - 1);
serverURL might be a whole path and not just a domain. Does noscript care?

>+  _handleNoScript: function (addExceptions) {
>+    if (addExceptions) {
>+      let self = this;
>+      this._remoteSites.forEach(function(site) {
>+        if (!ns.isJSEnabled(site)) {
>+          self._disabledSites.push(site); // save status
>+          ns.setJSEnabled(site, true); // allow site
>+        }
>+      });
You can pass this as a second argument to forEach and it'll be this inside the function.

>+    }
>+    else {
>+      this._disabledSites.forEach(function(site) {
>+        ns.setJSEnabled(site, false);
>+      });
>+      this._disabledSites = [];
Doesn't matter too much, but why conditionally setJSEnabled(true) and push items into _disabledSites and unconditionally setJSEnabled(false) and set to []?
(In reply to comment #13)
> >+    if (serverURL[serverURL.length - 1] == "/")
> >+      serverURL = serverURL.substr(0, serverURL.length - 1);
> serverURL might be a whole path and not just a domain. Does noscript care?

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

There's an easy way to satisfy this, by removing the serverURL trailing slash "lame" hack and slightly changing the _handleNoScript() code as follows:

    this._remoteSites.forEach(function(site) {
       if (!ns.isJSEnabled(site = ns.getSite(site))) {
(Assignee)

Updated

9 years ago
Whiteboard: [has patch][needs review Mardak] → [needs new patch]
(Assignee)

Updated

9 years ago
Attachment #433408 - Attachment is obsolete: true
Attachment #433408 - Flags: review?(edilee)
(Assignee)

Comment 15

9 years ago
Created attachment 433757 [details] [diff] [review]
fix comments

(In reply to comment #13)
> (From update of attachment 433408 [details] [diff] [review])
> serverURL might be a whole path and not just a domain. Does noscript care?

yes, fixed.

> You can pass this as a second argument to forEach and it'll be this inside the
> function.

right right, fixed

> Doesn't matter too much, but why conditionally setJSEnabled(true) and push
> items into _disabledSites and unconditionally setJSEnabled(false) and set to
> []?

Because if one of the domains is already whitelisted, we don't want to muck with that setting.
Attachment #433757 - Flags: review?(edilee)
(Assignee)

Updated

9 years ago
Whiteboard: [needs new patch] → [has patch][needs review Mardak]
Comment on attachment 433757 [details] [diff] [review]
fix comments

Did you attach the wrong patch? Seems the same as the previous one. ?
(Assignee)

Comment 17

9 years ago
Created attachment 433771 [details] [diff] [review]
fix comments

Bah, yes.
Attachment #433757 - Attachment is obsolete: true
Attachment #433771 - Flags: review?(edilee)
Attachment #433757 - Flags: review?(edilee)
Attachment #433771 - Flags: review?(edilee) → review+
Whiteboard: [has patch][needs review Mardak] → [has patch][has review]
(Assignee)

Comment 18

9 years ago
http://hg.mozilla.org/labs/weave/rev/22d53cec2c35
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Whiteboard: [has patch][has review]
Verified fix on 1.2b5.  noscript and adblock plus works with captcha
Status: RESOLVED → VERIFIED

Updated

8 years ago
Depends on: 639226
You need to log in before you can comment on or make changes to this bug.