FEATURE: improve honeypot and challenge logic

FEATURE: improve honeypot and challenge logic

This feature amends it so instead of using one challenge and honeypot statically per site we have a rotating honeypot and challenge value which changes every hour.

This means you must grab a fresh copy of honeypot and challenge value once an hour or account registration will be rejected.

We also now cycle the value of the challenge when after successful account registration forcing an extra call to hp.json between account registrations

Client has been made aware of these changes.

Additionally this contains a JavaScript workaround for: Issue 987293 - chromium - An open-source project to help move the web forward. - Monorail

This is client side code that is specific to Chrome user agent and swaps a PASSWORD type honeypot with a TEXT type honeypot.

diff --git a/app/assets/javascripts/discourse/controllers/create-account.js.es6 b/app/assets/javascripts/discourse/controllers/create-account.js.es6
index 1c8ecb254a..0db34a4e94 100644
--- a/app/assets/javascripts/discourse/controllers/create-account.js.es6
+++ b/app/assets/javascripts/discourse/controllers/create-account.js.es6
@@ -24,7 +24,6 @@ export default Ember.Controller.extend(
     login: Ember.inject.controller(),
 
     complete: false,
-    accountPasswordConfirm: 0,
     accountChallenge: 0,
     formSubmitted: false,
     rejectedEmails: Ember.A([]),
@@ -191,8 +190,36 @@ export default Ember.Controller.extend(
     @on("init")
     fetchConfirmationValue() {
       return ajax(userPath("hp.json")).then(json => {
+        this._challengeDate = new Date();
+        // remove 30 seconds for jitter, make sure this works for at least
+        // 30 seconds so we don't have hard loops
+        this._challengeExpiry = parseInt(json.expires_in, 10) - 30;
+        if (this._challengeExpiry < 30) {
+          this._challengeExpiry = 30;
+        }
+
+        const confirmation = document.getElementById(
+          "new-account-confirmation"
+        );
+        if (confirmation) {
+          confirmation.value = json.value;
+        }
+
+        // Chrome autocomplete is buggy per:
+        // https://bugs.chromium.org/p/chromium/issues/detail?id=987293
+        // work around issue while leaving a semi useable honeypot for
+        // bots that are running full Chrome
+        if (confirmation && navigator.userAgent.indexOf("Chrome") > 0) {
+          const newConfirmation = document.createElement("input");
+
+          newConfirmation.type = "text";
+          newConfirmation.id = "new-account-confirmation";
+          newConfirmation.value = json.value;
+
+          confirmation.parentNode.replaceChild(newConfirmation, confirmation);
+        }
+
         this.setProperties({
-          accountPasswordConfirm: json.value,
           accountChallenge: json.challenge
             .split("")
             .reverse()
@@ -201,85 +228,102 @@ export default Ember.Controller.extend(
       });
     },
 
-    actions: {
-      externalLogin(provider) {
-        this.login.send("externalLogin", provider);
-      },
+    performAccountCreation() {
+      const attrs = this.getProperties(
+        "accountName",
+        "accountEmail",
+        "accountPassword",
+        "accountUsername",
+        "accountChallenge"
+      );
 
-      createAccount() {
-        const attrs = this.getProperties(
-          "accountName",
-          "accountEmail",
-          "accountPassword",
-          "accountUsername",
-          "accountPasswordConfirm",
-          "accountChallenge"
-        );
-        const userFields = this.userFields;
-        const destinationUrl = this.get("authOptions.destination_url");
+      attrs["accountPasswordConfirm"] = document.getElementById(
+        "new-account-confirmation"
+      ).value;
 
-        if (!Ember.isEmpty(destinationUrl)) {
-          $.cookie("destination_url", destinationUrl, { path: "/" });
-        }
+      const userFields = this.userFields;
+      const destinationUrl = this.get("authOptions.destination_url");
 
-        // Add the userfields to the data
-        if (!Ember.isEmpty(userFields)) {
-          attrs.userFields = {};
-          userFields.forEach(
-            f => (attrs.userFields[f.get("field.id")] = f.get("value"))
-          );
-        }
+      if (!Ember.isEmpty(destinationUrl)) {
+        $.cookie("destination_url", destinationUrl, { path: "/" });
+      }
 
-        this.set("formSubmitted", true);
-        return Discourse.User.createAccount(attrs).then(
-          result => {
-            this.set("isDeveloper", false);
-            if (result.success) {
-              // Trigger the browser's password manager using the hidden static login form:
-              const $hidden_login_form = $("#hidden-login-form");
-              $hidden_login_form
-                .find("input[name=username]")
-                .val(attrs.accountUsername);
-              $hidden_login_form
-                .find("input[name=password]")
-                .val(attrs.accountPassword);
-              $hidden_login_form
-                .find("input[name=redirect]")
-                .val(userPath("account-created"));
-              $hidden_login_form.submit();
-            } else {
-              this.flash(
-                result.message || I18n.t("create_account.failed"),
-                "error"
-              );
-              if (result.is_developer) {
-                this.set("isDeveloper", true);
-              }
-              if (
-                result.errors &&
-                result.errors.email &&
-                result.errors.email.length > 0 &&
-                result.values
-              ) {
-                this.rejectedEmails.pushObject(result.values.email);
-              }
-              if (
-                result.errors &&
-                result.errors.password &&
-                result.errors.password.length > 0
-              ) {
-                this.rejectedPasswords.pushObject(attrs.accountPassword);
-              }
-              this.set("formSubmitted", false);
-              $.removeCookie("destination_url");
+      // Add the userfields to the data
+      if (!Ember.isEmpty(userFields)) {
+        attrs.userFields = {};
+        userFields.forEach(
+          f => (attrs.userFields[f.get("field.id")] = f.get("value"))
+        );
+      }
+
+      this.set("formSubmitted", true);
+      return Discourse.User.createAccount(attrs).then(
+        result => {
+          this.set("isDeveloper", false);
+          if (result.success) {
+            // invalidate honeypot
+            this._challengeExpiry = 1;
+
+            // Trigger the browser's password manager using the hidden static login form:
+            const $hidden_login_form = $("#hidden-login-form");
+            $hidden_login_form
+              .find("input[name=username]")
+              .val(attrs.accountUsername);
+            $hidden_login_form
+              .find("input[name=password]")
+              .val(attrs.accountPassword);
+            $hidden_login_form
+              .find("input[name=redirect]")
+              .val(userPath("account-created"));
+            $hidden_login_form.submit();
+          } else {
+            this.flash(
+              result.message || I18n.t("create_account.failed"),
+              "error"
+            );
+            if (result.is_developer) {
+              this.set("isDeveloper", true);
+            }
+            if (
+              result.errors &&
+              result.errors.email &&
+              result.errors.email.length > 0 &&
+              result.values
+            ) {
+              this.rejectedEmails.pushObject(result.values.email);
+            }
+            if (
+              result.errors &&
+              result.errors.password &&
+              result.errors.password.length > 0
+            ) {
+              this.rejectedPasswords.pushObject(attrs.accountPassword);
             }
-          },
-          () => {
             this.set("formSubmitted", false);
             $.removeCookie("destination_url");
-            return this.flash(I18n.t("create_account.failed"), "error");
           }
-        );
+        },
+        () => {
+          this.set("formSubmitted", false);
+          $.removeCookie("destination_url");
+          return this.flash(I18n.t("create_account.failed"), "error");
+        }
+      );
+    },
+
+    actions: {
+      externalLogin(provider) {
+        this.login.send("externalLogin", provider);
+      },
+
+      createAccount() {
+        if (new Date() - this._challengeDate > 1000 * this._challengeExpiry) {
+          this.fetchConfirmationValue().then(() =>
+            this.performAccountCreation()
+          );
+        } else {
+          this.performAccountCreation();
+        }
       }
     }
   }

[... diff too long, it was truncated ...]

GitHub sha: d5d8db7f

2 Likes

This commit has been mentioned on Discourse Meta. There might be relevant details there:

A controller shouldn’t access the DOM. This needs to happen in a component.

Also I am curious why the linter didn’t complain about attrs["accountPasswordConfirm"] since it’s not a dynamic lookup it is preferable to do attrs.accountPasswordConfirm

1 Like

Another access of the DOM in a controller. This definitely needs to be in a component instead.

More DOM access here that needs to be removed.

DEV: avoid making direct HTML changes in controllers

OK shifted this into a dedicated component. ^^^

3 Likes

Thanks, the honeypot thing is fixed.

The jQuery hidden form code is all still there though.

Missed that, was unrelated to my change I think @techAPJ did this years ago, can you follow it up with guidance from Robin, Arpit?

1 Like

I see, that is indeed old code that was moved around. @techAPJ can you add to your list to move that DOM manipulation to a component? Low priority.

2 Likes