SEC-AUTH-008. L addition a deux operandes entre 1 et 10 avait dix-neuf reponses possibles et se resolvait en lisant la question comme une chaine. Elle n arretait aucune inscription automatisee. Elle ajoutait en revanche une etape a chaque personne, lecteur d ecran compris, contre une apparence de protection — ce qui est pire que rien, puisque ca se compte comme une protection. L autre branche proposee par l audit etait un vrai service de CAPTCHA : un tiers, une cle d API, une requete a chaque affichage, et un script etranger remis dans script-src, defaisant le travail qui a ferme SEC-WEB-001. Disproportionne pour le site d un club. A la place, deux verifications invisibles pour un visiteur : un champ piege, cache par la feuille de style et hors du parcours clavier, qu un robot remplisseur complete et qu une personne ne voit jamais ; et un delai minimal entre la remise du formulaire et son retour, l horodatage etant dans la session signee et non dans un champ. Le plafond est dit dans le code plutot que sous-entendu : ceci arrete le pourriel de masse, pas quelqu un qui lit la page. Ce qui filtrerait vraiment les inscriptions serait l activation des comptes par le staff — is_active_account vaut True par defaut. C est une decision de produit. L angle « session forgeable » du constat tombe : avec SECRET_KEY compromise (SEC-001), on forge une session connectee sur n importe quel compte et on n a aucune raison de s inscrire. Au passage, ARCH-005 en partie : le bloc « regenerer, purger les mots de passe, re-rendre » etait recopie quatre fois. Un seul helper, et la purge des mots de passe ne peut plus etre oubliee dans la cinquieme copie. Les refus sont journalises avec leur motif — c est le seul endroit ou un abus du formulaire devient visible — mais restent indistinguables pour l expediteur : nommer la regle indique comment la contourner. 429 tests.
218 lines
7.6 KiB
Python
218 lines
7.6 KiB
Python
"""What now stands between a robot and a new account (SEC-AUTH-008).
|
|
|
|
The arithmetic CAPTCHA it replaces had nineteen possible answers and could
|
|
be solved by reading the question as a string. It stopped nothing, cost every
|
|
human a step, and — being counted as a protection — was worse than nothing.
|
|
|
|
Two checks took its place: a honeypot input, and a floor on how fast the
|
|
form can come back. Both are invisible to a person filling in the form. Both
|
|
are honest about their ceiling: they stop commodity spam, not somebody who
|
|
reads the page.
|
|
|
|
The tests that matter most here are the ones asserting a *legitimate*
|
|
sign-up still works. A screening rule that quietly refuses real people is a
|
|
worse outcome than the CAPTCHA was.
|
|
"""
|
|
|
|
import time
|
|
|
|
from app.routes.auth import (
|
|
MIN_REGISTRATION_SECONDS,
|
|
REGISTRATION_HONEYPOT_FIELD,
|
|
REGISTRATION_ISSUED_KEY,
|
|
)
|
|
|
|
FORM = {
|
|
'username': 'brandnew',
|
|
'email': '[email protected]',
|
|
'password': 'Password123',
|
|
'confirm_password': 'Password123',
|
|
'full_name': 'Brand New',
|
|
}
|
|
|
|
|
|
def _issued_long_ago(client, seconds_ago=None):
|
|
"""Pretend the form was handed out a while back.
|
|
|
|
Backdated rather than slept through: waiting three real seconds per test
|
|
would add a minute to the suite for nothing.
|
|
"""
|
|
if seconds_ago is None:
|
|
seconds_ago = MIN_REGISTRATION_SECONDS + 1
|
|
with client.session_transaction() as session:
|
|
session[REGISTRATION_ISSUED_KEY] = time.time() - seconds_ago
|
|
|
|
|
|
def _account_exists(app, username='brandnew'):
|
|
from app.models import User
|
|
|
|
with app.app_context():
|
|
return User.query.filter_by(username=username).first() is not None
|
|
|
|
|
|
class TestLegitimateSignUp:
|
|
def test_a_person_filling_the_form_gets_an_account(self, app, client):
|
|
client.get('/auth/register')
|
|
_issued_long_ago(client)
|
|
|
|
client.post('/auth/register', data=dict(FORM), follow_redirects=True)
|
|
|
|
assert _account_exists(app)
|
|
|
|
def test_an_empty_honeypot_is_not_a_refusal(self, app, client):
|
|
"""A browser submits the hidden input as an empty string, not absent."""
|
|
_issued_long_ago(client)
|
|
|
|
client.post(
|
|
'/auth/register',
|
|
data=dict(FORM, **{REGISTRATION_HONEYPOT_FIELD: ''}),
|
|
follow_redirects=True,
|
|
)
|
|
|
|
assert _account_exists(app)
|
|
|
|
def test_correcting_a_typo_does_not_restart_the_clock(self, app, client):
|
|
"""The dwell timer must survive a failed attempt.
|
|
|
|
Reissuing it on every re-render would refuse the second submission of
|
|
anyone who fixes a mistake quickly — a rule that fires on real people
|
|
and not on robots, which is the wrong way round.
|
|
"""
|
|
client.get('/auth/register')
|
|
_issued_long_ago(client)
|
|
|
|
# First attempt fails validation: passwords do not match.
|
|
client.post(
|
|
'/auth/register',
|
|
data=dict(FORM, confirm_password='Different123'),
|
|
follow_redirects=True,
|
|
)
|
|
# Corrected and sent straight back, well inside the dwell floor.
|
|
client.post('/auth/register', data=dict(FORM), follow_redirects=True)
|
|
|
|
assert _account_exists(app)
|
|
|
|
|
|
class TestScreening:
|
|
def test_a_filled_honeypot_is_refused(self, app, client):
|
|
_issued_long_ago(client)
|
|
|
|
client.post(
|
|
'/auth/register',
|
|
data=dict(FORM, **{REGISTRATION_HONEYPOT_FIELD: 'http://spam.example'}),
|
|
follow_redirects=True,
|
|
)
|
|
|
|
assert not _account_exists(app)
|
|
|
|
def test_a_form_returned_instantly_is_refused(self, app, client):
|
|
client.get('/auth/register')
|
|
|
|
client.post('/auth/register', data=dict(FORM), follow_redirects=True)
|
|
|
|
assert not _account_exists(app)
|
|
|
|
def test_a_post_that_never_fetched_the_form_is_refused(self, app, client):
|
|
"""The strongest signal available: no form was ever issued."""
|
|
client.post('/auth/register', data=dict(FORM), follow_redirects=True)
|
|
|
|
assert not _account_exists(app)
|
|
|
|
def test_the_two_rules_are_indistinguishable_to_the_sender(self, app, client):
|
|
"""Naming the rule tells whoever tripped it how to avoid it.
|
|
|
|
Both refusals must read identically from outside. The log, which the
|
|
sender cannot see, is where they are told apart.
|
|
|
|
One client per scenario, on purpose: the dwell stamp lives in the
|
|
session and is deliberately kept across a refusal, so reusing a single
|
|
client would carry the first scenario's backdated stamp into the
|
|
second and the "too fast" case would never fire.
|
|
"""
|
|
_issued_long_ago(client)
|
|
tripped_honeypot = client.post(
|
|
'/auth/register',
|
|
data=dict(FORM, **{REGISTRATION_HONEYPOT_FIELD: 'spam'}),
|
|
follow_redirects=True,
|
|
).get_data(as_text=True)
|
|
|
|
fresh = app.test_client()
|
|
fresh.get('/auth/register')
|
|
too_fast = fresh.post('/auth/register', data=dict(FORM), follow_redirects=True).get_data(
|
|
as_text=True
|
|
)
|
|
|
|
def flashes(page):
|
|
return [line for line in page.splitlines() if 'alert-danger' in line]
|
|
|
|
assert flashes(tripped_honeypot) == flashes(too_fast)
|
|
assert flashes(tripped_honeypot), 'no message at all is not the same as a uniform one'
|
|
|
|
|
|
class TestTheFormItself:
|
|
def test_no_arithmetic_question_is_asked(self, client):
|
|
page = client.get('/auth/register').get_data(as_text=True)
|
|
|
|
assert 'captcha' not in page.lower()
|
|
|
|
def test_the_honeypot_is_hidden_from_assistive_technology(self, client):
|
|
"""An input a screen reader announces is a trap for a person.
|
|
|
|
aria-hidden, tabindex=-1 and display:none all have to hold. The CSS
|
|
rule lives in the stylesheet rather than in a style attribute so that
|
|
it survives a future tightening of style-src.
|
|
"""
|
|
page = client.get('/auth/register').get_data(as_text=True)
|
|
|
|
assert 'class="honeypot" aria-hidden="true"' in page
|
|
assert f'name="{REGISTRATION_HONEYPOT_FIELD}" tabindex="-1"' in page
|
|
|
|
def test_the_stylesheet_actually_hides_it(self, app):
|
|
import os
|
|
|
|
with open(os.path.join(app.static_folder, 'css', 'style.css'), encoding='utf-8') as handle:
|
|
css = handle.read()
|
|
|
|
block = css.split('.honeypot')[-1]
|
|
assert 'display: none' in block.split('}')[0]
|
|
|
|
|
|
class TestRefusalIsVisible:
|
|
def test_a_refusal_is_written_to_the_audit_log(self, app, client, monkeypatch):
|
|
"""Sign-up abuse leaves a trace or it is not happening, as far as
|
|
anyone can tell."""
|
|
recorded = []
|
|
from app.routes import auth as auth_module
|
|
|
|
monkeypatch.setattr(
|
|
auth_module,
|
|
'log_auth_event',
|
|
lambda event, **fields: recorded.append((event, fields)),
|
|
)
|
|
_issued_long_ago(client)
|
|
|
|
client.post(
|
|
'/auth/register',
|
|
data=dict(FORM, **{REGISTRATION_HONEYPOT_FIELD: 'spam'}),
|
|
follow_redirects=True,
|
|
)
|
|
|
|
assert recorded, 'the double was never called — the test would pass on nothing'
|
|
assert recorded[0][0] == 'account.registration_refused'
|
|
assert recorded[0][1]['reason'] == 'honeypot'
|
|
|
|
def test_the_reason_distinguishes_the_two_rules(self, app, client, monkeypatch):
|
|
recorded = []
|
|
from app.routes import auth as auth_module
|
|
|
|
monkeypatch.setattr(
|
|
auth_module,
|
|
'log_auth_event',
|
|
lambda event, **fields: recorded.append((event, fields)),
|
|
)
|
|
client.get('/auth/register')
|
|
|
|
client.post('/auth/register', data=dict(FORM), follow_redirects=True)
|
|
|
|
assert recorded and recorded[0][1]['reason'] == 'too-fast'
|