diff --git a/cms/sass/components/_alert.scss b/cms/sass/components/_alert.scss index 47f841bdb5..508f379fa8 100644 --- a/cms/sass/components/_alert.scss +++ b/cms/sass/components/_alert.scss @@ -3,7 +3,7 @@ .alert { display: inline-block; padding: $spacing-03; - margin: $spacing-02 0$spacing-03 0; + margin: $spacing-02 0 $spacing-03 0; background-color: $light-grey; border: 1px solid $warm-black; @include typescale-06; diff --git a/cms/sass/components/_honeypotfield.scss b/cms/sass/components/_honeypotfield.scss new file mode 100644 index 0000000000..7dc4df79de --- /dev/null +++ b/cms/sass/components/_honeypotfield.scss @@ -0,0 +1,11 @@ +.hpemail { + position: absolute; + left: -9999px; /* Large negative offset */ + top: -9999px; + height: 1px; + width: 1px; + overflow: hidden; + border: 0; + padding: 0; + margin: 0; +} \ No newline at end of file diff --git a/cms/sass/layout/_page-header.scss b/cms/sass/layout/_page-header.scss index 038f0dcf7d..d9b21d4a4e 100644 --- a/cms/sass/layout/_page-header.scss +++ b/cms/sass/layout/_page-header.scss @@ -7,7 +7,8 @@ min-height: 100px; } - a { + /* do not underline the links, unless in an alert */ + a:not(.alert a) { text-decoration: none; } diff --git a/cms/sass/main.scss b/cms/sass/main.scss index 0f72c8eac8..94636ffcfd 100644 --- a/cms/sass/main.scss +++ b/cms/sass/main.scss @@ -38,6 +38,7 @@ "components/filters", "components/form", "components/hero", + "components/honeypotfield", "components/input-group", "components/label", "components/loading", diff --git a/doajtest/fixtures/registrationForm.py b/doajtest/fixtures/registrationForm.py new file mode 100644 index 0000000000..c8c8ae7672 --- /dev/null +++ b/doajtest/fixtures/registrationForm.py @@ -0,0 +1,74 @@ +from werkzeug.datastructures import ImmutableMultiDict, CombinedMultiDict +from portality.core import app + +def get_longer_time_than_hp_threshold(): + return app.config.get("HONEYPOT_TIMER_THRESHOLD", 5000) + 100 + +def get_shorter_time_than_hp_threshold(): + return app.config.get("HONEYPOT_TIMER_THRESHOLD", 5000) - 100 + +COMMON = ImmutableMultiDict([ + ('next', '/register'), +]) + +VALID_FORM = ImmutableMultiDict([ + ('name', 'Aga'), + ('sender_email', 'aga@example.com'), +]) + +INVALID_FORM = ImmutableMultiDict([ + ('name', 'Aga'), + ('sender_email', ''), +]) + +VALID_HONEYPOT = ImmutableMultiDict([ + ('email', ''), + ('hptimer', get_longer_time_than_hp_threshold()) +]) + +INVALID_HONEYPOT_TIMER_BELOW_THRESHOLD = ImmutableMultiDict([ + ('email', ''), + ('hptimer', get_shorter_time_than_hp_threshold()) +]) + +INVALID_HONEYPOT_EMAIL_NOT_EMPTY = ImmutableMultiDict([ + ('email', 'this_field@should_be.empty'), + ('hptimer', get_shorter_time_than_hp_threshold()) +]) + +INVALID_HONEYPOT_BOTH_FIELDS = ImmutableMultiDict([ + ('email', 'this_field@should_be.empty'), + ('hptimer', get_longer_time_than_hp_threshold()) +]) + +# Method 1: Valid form with valid honeypot +def create_valid_form_with_valid_honeypot(): + return CombinedMultiDict([COMMON, VALID_FORM, VALID_HONEYPOT]) + +# Method 2: Valid form with invalid honeypot (timer exceeds threshold) +def create_valid_form_with_invalid_honeypot_timer_exceeds(): + return CombinedMultiDict([COMMON, VALID_FORM, INVALID_HONEYPOT_TIMER_BELOW_THRESHOLD]) + +# Method 3: Valid form with invalid honeypot (email not empty) +def create_valid_form_with_invalid_honeypot_email_not_empty(): + return CombinedMultiDict([COMMON, VALID_FORM, INVALID_HONEYPOT_EMAIL_NOT_EMPTY]) + +# Method 4: Invalid form with valid honeypot +def create_invalid_form_with_valid_honeypot(): + return CombinedMultiDict([COMMON, INVALID_FORM, VALID_HONEYPOT]) + +# Method 5: Invalid form with invalid honeypot (timer exceeds threshold) +def create_invalid_form_with_invalid_honeypot_timer_exceeds(): + return CombinedMultiDict([COMMON, INVALID_FORM, INVALID_HONEYPOT_TIMER_BELOW_THRESHOLD]) + +# Method 6: Invalid form with invalid honeypot (email not empty) +def create_invalid_form_with_invalid_honeypot_email_not_empty(): + return CombinedMultiDict([COMMON, INVALID_FORM, INVALID_HONEYPOT_EMAIL_NOT_EMPTY]) + +# Method 7: Valid form with invalid honeypot (both fields) +def create_valid_form_with_invalid_honeypot_both_fields(): + return CombinedMultiDict([COMMON, VALID_FORM, INVALID_HONEYPOT_BOTH_FIELDS]) + +# Method 8: Invalid form with invalid honeypot (both fields) +def create_invalid_form_with_invalid_honeypot_both_fields(): + return CombinedMultiDict([COMMON, INVALID_FORM, INVALID_HONEYPOT_BOTH_FIELDS]) diff --git a/doajtest/unit/test_honeypot.py b/doajtest/unit/test_honeypot.py new file mode 100644 index 0000000000..297206e5d3 --- /dev/null +++ b/doajtest/unit/test_honeypot.py @@ -0,0 +1,41 @@ +from doajtest.helpers import DoajTestCase +from doajtest.fixtures import registrationForm +from portality.view.account import RegisterForm +from werkzeug.datastructures import ImmutableMultiDict + +class TestHoneypot(DoajTestCase): + + def setUp(self): + pass + + def test_01_valid_form_with_valid_honeypot(self): + valid_form = RegisterForm(registrationForm.create_valid_form_with_valid_honeypot()) + assert valid_form.is_bot() is False, "Test failed: The form should not be identified as a bot." + + def test_02_valid_form_with_invalid_honeypot_timer_exceeds(self): + valid_form = RegisterForm(registrationForm.create_valid_form_with_invalid_honeypot_timer_exceeds()) + assert valid_form.is_bot() is True, "Test failed: The form should be identified as a bot due to honeypot timer exceeding threshold." + + def test_03_valid_form_with_invalid_honeypot_email_not_empty(self): + valid_form = RegisterForm(registrationForm.create_valid_form_with_invalid_honeypot_email_not_empty()) + assert valid_form.is_bot() is True, "Test failed: The form should be identified as a bot due to honeypot email field not being empty." + + def test_04_invalid_form_with_valid_honeypot(self): + invalid_form = RegisterForm(registrationForm.create_invalid_form_with_valid_honeypot()) + assert invalid_form.is_bot() is False, "Test failed: The form should not be identified as a bot since honeypot is valid." + + def test_05_invalid_form_with_invalid_honeypot_timer_exceeds(self): + invalid_form = RegisterForm(registrationForm.create_invalid_form_with_invalid_honeypot_timer_exceeds()) + assert invalid_form.is_bot() is True, "Test failed: The form should be identified as a bot due to honeypot timer exceeding threshold." + + def test_06_invalid_form_with_invalid_honeypot_email_not_empty(self): + invalid_form = RegisterForm(registrationForm.create_invalid_form_with_invalid_honeypot_email_not_empty()) + assert invalid_form.is_bot() is True, "Test failed: The form should be identified as a bot due to honeypot email field not being empty." + + def test_07_valid_form_with_invalid_honeypot_both_fields(self): + valid_form = RegisterForm(registrationForm.create_valid_form_with_invalid_honeypot_both_fields()) + assert valid_form.is_bot() is True, "Test failed: The form should be identified as a bot due to both honeypot fields being invalid." + + def test_08_invalid_form_with_invalid_honeypot_both_fields(self): + invalid_form = RegisterForm(registrationForm.create_invalid_form_with_invalid_honeypot_both_fields()) + assert invalid_form.is_bot() is True, "Test failed: The form should be identified as a bot due to both honeypot fields being invalid." \ No newline at end of file diff --git a/portality/app.py b/portality/app.py index 0ad6e4f609..3eade6a17b 100644 --- a/portality/app.py +++ b/portality/app.py @@ -413,14 +413,6 @@ def api_directory(): ) return jsonify({'api_versions': vers}) - -# Make the reCAPTCHA key available to the js -# ~~-> ReCAPTCHA:ExternalService~~ -@app.route('/get_recaptcha_site_key') -def get_site_key(): - return app.config.get('RECAPTCHA_SITE_KEY', '') - - @app.errorhandler(400) def page_not_found(e): return render_template(templates.ERROR_400), 400 diff --git a/portality/settings.py b/portality/settings.py index 9d6e313e6b..cfeca1d51d 100644 --- a/portality/settings.py +++ b/portality/settings.py @@ -1342,15 +1342,6 @@ # assume it's a zombie, and ignore it HARVESTER_ZOMBIE_AGE = 604800 -####################################################### -# ReCAPTCHA configuration -# ~~->ReCAPTCHA:ExternalService - -#Recaptcha test keys, should be overridden in dev.cfg by the keys obtained from Google ReCaptcha v2 -RECAPTCHA_ENABLE = True -RECAPTCHA_SITE_KEY = '6LeIxAcTAAAAAJcZVRqyHh71UMIEGNQ_MXjiZKhI' -RECAPTCHA_SECRET_KEY = "6LeIxAcTAAAAAGG-vFI1TnRWxMZNFuojJ4WifJWe" - ####################################################### # Preservation configuration # ~~->Preservation:Feature @@ -1554,3 +1545,7 @@ BGJOB_MANAGE_REDUNDANT_ACTIONS = [ 'read_news', 'journal_csv' ] + +################################################## +# Honeypot bot-trap settings for forms (now: only registration form) +HONEYPOT_TIMER_THRESHOLD = 5000; diff --git a/portality/static/js/honeypot.js b/portality/static/js/honeypot.js new file mode 100644 index 0000000000..bd972abdaf --- /dev/null +++ b/portality/static/js/honeypot.js @@ -0,0 +1,18 @@ +// ~~Honeypot:Feature~~ +doaj.honeypot = {} + +doaj.honeypot.init = function () { + console.log("init"); + doaj.honeypot.startTime = performance.now(); + $("#submitBtn").on("click", (event) => doaj.honeypot.handleRegistration(event)); +} + +doaj.honeypot.handleRegistration = function (event) { + event.preventDefault(); + const endTime = performance.now(); + const elapsedTime = endTime - doaj.honeypot.startTime; + // reset the timer + doaj.honeypot.startTime = performance.now(); + $("#hptimer").val(elapsedTime); + $("#registrationForm").submit(); +} \ No newline at end of file diff --git a/portality/static/js/recaptcha.js b/portality/static/js/recaptcha.js deleted file mode 100644 index 9b64fb388a..0000000000 --- a/portality/static/js/recaptcha.js +++ /dev/null @@ -1,21 +0,0 @@ -var onloadCallback = function() { - - $("#submitBtn").prop("disabled", true); - - var captchaCallback = function(param) { - $('#recaptcha_value').val(param); - $("#submitBtn").prop("disabled", false); - }; - - function ajax1() { - return $.get("/get_recaptcha_site_key"); - } - - $.when(ajax1()).done(function(key) { - grecaptcha.render('recaptcha_div', { - 'sitekey' : key, - 'callback' : captchaCallback, - }); - } - ); -}; \ No newline at end of file diff --git a/portality/templates-v2/_account/includes/_register_form.html b/portality/templates-v2/_account/includes/_register_form.html index 718fee8793..38fd018520 100644 --- a/portality/templates-v2/_account/includes/_register_form.html +++ b/portality/templates-v2/_account/includes/_register_form.html @@ -1,30 +1,31 @@ {% from "includes/_formhelpers.html" import render_field %} -
- + + + {# This input is a bot-bait, it should stay invisible to the users and empty. #} + {# Make sure it's invisible on the screen AND FOR SCREEN READERS/KEYBOARD USERS' #} +
{% if current_user.is_authenticated and current_user.has_role("create_user") %} - {# Admins can specify a user ID #} + {# Admins can specify a user ID #} {{ render_field(form.identifier) }}
{% endif %} {{ render_field(form.name, placeholder="Firstname Lastname") }}
- {{ render_field(form.email, placeholder="user@example.com") }} + {{ render_field(form.sender_email, placeholder="user@example.com") }}
{% if current_user.is_authenticated and current_user.has_role("create_user") %} {# Admins can specify a user ID #}
- {{ render_field(form.roles) }} + {{ render_field(form.roles) }}
{% endif %} - -
- {{ render_field(form.next) }} - {{form.recaptcha_value(id="recaptcha_value")}} - -
- -
-
+ +
+ +
+ diff --git a/portality/templates-v2/management/admin/account/create.html b/portality/templates-v2/management/admin/account/create.html index 105deb99c6..691b30bca3 100644 --- a/portality/templates-v2/management/admin/account/create.html +++ b/portality/templates-v2/management/admin/account/create.html @@ -2,14 +2,7 @@ {% block page_title %}Create User{% endblock %} -{% block admin_stylesheets %} - -{% endblock %} +{% block admin_stylesheets %}{% endblock %} {% block admin_content %}
@@ -24,6 +17,7 @@ {% endblock %} {% block admin_js %} + {% if current_user.is_authenticated and current_user.has_role("create_user") %} - {% endif %} - {% if config.get("RECAPTCHA_ENABLE") %} - - - + {% else %} + {% endif %} {% endblock %} diff --git a/portality/templates-v2/public/account/register.html b/portality/templates-v2/public/account/register.html index 579777747e..de0edf0361 100644 --- a/portality/templates-v2/public/account/register.html +++ b/portality/templates-v2/public/account/register.html @@ -2,14 +2,7 @@ {% block page_title %}Register{% endblock %} -{% block public_stylesheets %} - -{% endblock %} +{% block public_stylesheets %}{% endblock %} {% block public_content %}
@@ -34,6 +27,7 @@

Register

{% endblock %} {% block public_js %} + {% if current_user.is_authenticated and current_user.has_role("create_user") %} - {% endif %} - {% if config.get("RECAPTCHA_ENABLE") %} - - - + {% else %} + {% endif %} {% endblock %} diff --git a/portality/ui/messages.py b/portality/ui/messages.py index 5702b45aa4..f97ebbf3d5 100644 --- a/portality/ui/messages.py +++ b/portality/ui/messages.py @@ -141,6 +141,8 @@ class Messages(object): PRESERVATION_NO_FILE = "No file provided for upload" + ARE_YOU_A_HUMAN = "Are you sure you're a human? If you're having trouble logging in, please contact us." + @classmethod def flash(cls, tup): if isinstance(tup, tuple): diff --git a/portality/util.py b/portality/util.py index 2a4e1f36f3..ff44a6ebb5 100644 --- a/portality/util.py +++ b/portality/util.py @@ -137,17 +137,6 @@ def ipt_prefix(type): return type -def verify_recaptcha(g_recaptcha_response): - """ - ~~ReCAPTCHA:ExternalService~~ - :param g_recaptcha_response: - :return: - """ - with urllib.request.urlopen('https://www.recaptcha.net/recaptcha/api/siteverify?secret=' + app.config.get("RECAPTCHA_SECRET_KEY") + '&response=' + g_recaptcha_response) as url: - data = json.loads(url.read().decode()) - return data - - def url_for(*args, **kwargs): """ This function is a hack to allow us to use url_for where we may nor may not have the diff --git a/portality/view/account.py b/portality/view/account.py index 3b9681c40b..b4c38252b0 100644 --- a/portality/view/account.py +++ b/portality/view/account.py @@ -3,7 +3,7 @@ from flask import Blueprint, request, url_for, flash, redirect, make_response from flask import render_template, abort from flask_login import login_user, logout_user, current_user, login_required -from wtforms import StringField, HiddenField, PasswordField, validators, Form +from wtforms import StringField, HiddenField, PasswordField, DecimalField, validators, Form from portality import util from portality import constants @@ -12,6 +12,7 @@ from portality.models import Account, Event from portality.forms.validate import DataOptional, EmailAvailable, ReservedUsernames, IdAvailable, IgnoreUnchanged from portality.bll import DOAJ +from portality.ui.messages import Messages from portality.ui import templates @@ -308,33 +309,48 @@ def logout(): class RegisterForm(RedirectForm): identifier = StringField('ID', [ReservedUsernames(), IdAvailable()]) name = StringField('Name', [validators.Optional(), validators.Length(min=3, max=64)]) - email = StringField('Email address', [ + sender_email = StringField('Email address', [ validators.DataRequired(), validators.Length(min=3, max=254), validators.Email(message='Must be a valid email address'), EmailAvailable(message="That email address is already in use. Please reset your password. If you still cannot login, contact us.") ]) roles = StringField('Roles') - recaptcha_value = HiddenField() + # These are honeypot (bot-trap) fields + email = StringField('email') + hptimer = DecimalField('hptimer', [validators.Optional()]) + def is_bot(self): + """ + Checks honeypot fields and determines whether the form was submitted by a bot + :return: True, if bot suspected; False, if human + """ + return self.email.data != "" or self.hptimer.data < app.config.get("HONEYPOT_TIMER_THRESHOLD", 5000) @blueprint.route('/register', methods=['GET', 'POST']) @ssl_required @write_required() def register(template=templates.REGISTER): + # ~~-> Honeypot:Feature ~~ # 3rd-party registration only for those with create_user role, only allow public registration when configured if current_user.is_authenticated and not current_user.has_role("create_user") \ or current_user.is_anonymous and app.config.get('PUBLIC_REGISTER', False) is False: abort(401) # todo: we may need a template to explain this since it's linked from the application form form = RegisterForm(request.form, csrf_enabled=False, roles='api,publisher', identifier=Account.new_short_uuid()) - if request.method == 'POST' and form.validate(): - if app.config.get("RECAPTCHA_ENABLE"): - recap_data = util.verify_recaptcha(form.recaptcha_value.data) - else: - recap_data = {"success": True} - if recap_data["success"]: - account = Account.make_account(email=form.email.data, username=form.identifier.data, name=form.name.data, + + if request.method == 'POST': + + if not current_user.is_authenticated and form.is_bot(): + if app.config.get('DEBUG', True): + flash(Messages.ARE_YOU_A_HUMAN, "error") + flash(f"Debug mode - Values submitted: bot trap field = '{form.email.data}'; anti-bot timer: '{form.hptimer.data}' ('{form.hptimer.data/1000:.2f}' sec)") + else: + flash(Messages.ARE_YOU_A_HUMAN, "error") + return render_template(template, form=form) + + if form.validate(): + account = Account.make_account(email=form.sender_email.data, username=form.identifier.data, name=form.name.data, roles=[r.strip() for r in form.roles.data.split(',')]) account.save() @@ -349,17 +365,14 @@ def register(template=templates.REGISTER): util.flash_with_url('Account created for {0}. View Account: {1}'.format(account.email, url_for('.username', username=account.id))) return redirect(url_for('.index')) else: - flash('Thank you, please verify email address ' + form.email.data + ' to set your password and login.', + flash('Thank you, please verify email address ' + form.sender_email.data + ' to set your password and login.', 'success') # We must redirect home because the user now needs to verify their email address. return redirect(url_for('doaj.home')) + else: + flash('Please correct the errors', 'error') - else: # recaptcha fail - util.flash("reCAPTCHA failed, please retry.") - - if request.method == 'POST' and not form.validate(): - flash('Please correct the errors', 'error') return render_template(template, form=form) @blueprint.route('/create/', methods=['GET', 'POST']) diff --git a/portality/view/doaj.py b/portality/view/doaj.py index d4d775e846..4f72b93943 100644 --- a/portality/view/doaj.py +++ b/portality/view/doaj.py @@ -63,7 +63,8 @@ def dismiss_site_note(): else: resp = make_response() # set a cookie that lasts for one year - resp.set_cookie(app.config.get("SITE_NOTE_KEY"), app.config.get("SITE_NOTE_COOKIE_VALUE"), max_age=app.config.get("SITE_NOTE_SLEEP"), samesite=None, secure=True) + resp.set_cookie(app.config.get("SITE_NOTE_KEY"), app.config.get("SITE_NOTE_COOKIE_VALUE"), + max_age=app.config.get("SITE_NOTE_SLEEP"), samesite=None, secure=True) return resp @@ -109,7 +110,7 @@ def search(): def search_post(): """ Redirect a query from the box on the index page to the search page. """ if request.form.get('origin') != 'ui': - abort(400) # bad request - we must receive searches from our own UI + abort(400) # bad request - we must receive searches from our own UI ref = request.form.get("ref") if ref is None: @@ -124,14 +125,14 @@ def search_post(): # lhs for journals, rhs for articles field_map = { - "all" : (None, None), - "title" : ("bibjson.title", "bibjson.title"), - "abstract" : (None, "bibjson.abstract"), - "subject" : ("index.classification", "index.classification"), - "author" : (None, "bibjson.author.name"), - "issn" : ("index.issn.exact", None), - "publisher" : ("bibjson.publisher.name", None), - "country" : ("index.country", None) + "all": (None, None), + "title": ("bibjson.title", "bibjson.title"), + "abstract": (None, "bibjson.abstract"), + "subject": ("index.classification", "index.classification"), + "author": (None, "bibjson.author.name"), + "issn": ("index.issn.exact", None), + "publisher": ("bibjson.publisher.name", None), + "country": ("index.country", None) } default_field_opts = field_map.get(field, None) default_field = None @@ -152,6 +153,7 @@ def search_post(): return redirect(route + '?source=' + urllib.parse.quote(json.dumps(query)) + "&ref=" + urllib.parse.quote(ref)) + ############################################# @blueprint.route("/csv") @@ -222,11 +224,13 @@ def get_from_local_store(container, filename): def autocomplete(doc_type, field_name): prefix = request.args.get('q', '') if not prefix: - return jsonify({'suggestions': [{"id": "", "text": "No results found"}]}) # select2 does not understand 400, which is the correct code here... + return jsonify({'suggestions': [{"id": "", + "text": "No results found"}]}) # select2 does not understand 400, which is the correct code here... m = models.lookup_model(doc_type) if not m: - return jsonify({'suggestions': [{"id": "", "text": "No results found"}]}) # select2 does not understand 404, which is the correct code here... + return jsonify({'suggestions': [{"id": "", + "text": "No results found"}]}) # select2 does not understand 404, which is the correct code here... size = request.args.get('size', 5) @@ -315,6 +319,7 @@ def find_correct_redirect_identifier(identifier, bibjson) -> str: # let it continue loading if we only have the hex UUID for the journal (no ISSNs) # and the user is referring to the toc page via that ID + @blueprint.route("/toc/") def toc(identifier=None): """ Table of Contents page for a journal. identifier may be the journal id or an issn """ @@ -346,7 +351,7 @@ def toc_articles(identifier=None): return render_template(templates.PUBLIC_TOC_ARTICLES, journal=journal, bibjson=bibjson, tab="articles") -#~~->Article:Page~~ +# ~~->Article:Page~~ @blueprint.route("/article/") def article_page(identifier=None): # identifier must be the article id @@ -499,11 +504,13 @@ def faq(): def about(): return render_template(templates.STATIC_PAGE, page_frag="/about/index.html") + @blueprint.route("/at-20/") def at_20(): return render_template(templates.STATIC_PAGE, page_frag="/about/at-20.html") + @blueprint.route("/about/ambassadors/") def ambassadors(): return render_template(templates.STATIC_PAGE, page_frag="/about/ambassadors.html") diff --git a/portality/view/forms.py b/portality/view/forms.py index 832c85e18c..f3f8fe0596 100644 --- a/portality/view/forms.py +++ b/portality/view/forms.py @@ -127,8 +127,6 @@ class MakeContinuation(Form): class ContactUs(Form): - recaptcha_value = HiddenField() - email = StringField('Your Email', [validators.DataRequired(), validators.Email()]) subject = StringField("Subject", [validators.Optional()])