GGThedandClaude Opus 5
7cec18c139
QUA-002, premiere moitie. **Ce commit ne fait que reformater** : aucun changement de comportement, aucune ligne de logique touchee. 72 fichiers, 4 restaient deja conformes. Il est isole exprès, pour que `git log -p` sur les commits voisins reste lisible. `quote-style = "preserve"` etait deja pose dans pyproject.toml, ce qui evite le brassage guillemets simples / doubles : le diff porte sur les retours a la ligne, l indentation des appels longs et les virgules finales, pas sur le style de chaine. Verification : 263 tests passent avant et apres, ruff check propre. L activation en CI arrive dans le commit suivant, separement, pour que ce diff-ci ne contienne rien d autre. Co-Authored-By: Claude Opus 5 <[email protected]>
314 lines
11 KiB
Python
314 lines
11 KiB
Python
"""Session lifecycle and account state.
|
|
|
|
These lock in the two fixes from wave 0: sessions now actually expire, and
|
|
deactivating an account now closes the sessions it already holds.
|
|
"""
|
|
|
|
import re
|
|
|
|
import pytest
|
|
|
|
from app.extensions import db
|
|
from app.models import User
|
|
|
|
#: Matches the flash markup emitted by layouts/base.html.
|
|
_FLASH_PATTERN = re.compile(
|
|
r'<div class="alert alert-\w+ alert-dismissible">\s*<span>(.*?)</span>',
|
|
re.S,
|
|
)
|
|
|
|
|
|
def _flash_messages(body):
|
|
"""Flash texts rendered in a response, independent of the locale."""
|
|
return [m.strip() for m in _FLASH_PATTERN.findall(body)]
|
|
|
|
|
|
def _set_active(app, user_id, active):
|
|
with app.app_context():
|
|
user = db.session.get(User, user_id)
|
|
user.is_active_account = active
|
|
db.session.commit()
|
|
|
|
|
|
class TestSessionExpiry:
|
|
def test_login_issues_an_expiring_session_cookie(self, app, client, as_role):
|
|
"""PERMANENT_SESSION_LIFETIME only applies to permanent sessions.
|
|
|
|
Before the fix, session.permanent was never set anywhere in app/,
|
|
so Flask emitted a browser-session cookie with no Expires attribute
|
|
and the configured one-hour lifetime was silently ignored.
|
|
"""
|
|
as_role('player')
|
|
|
|
cookie = client.get_cookie('session')
|
|
assert cookie is not None, 'no session cookie was issued at login'
|
|
assert cookie.expires is not None, (
|
|
'session cookie has no expiry: session.permanent was not set, '
|
|
'so PERMANENT_SESSION_LIFETIME has no effect'
|
|
)
|
|
|
|
def test_session_lifetime_matches_configuration(self, app):
|
|
from datetime import timedelta
|
|
|
|
assert app.permanent_session_lifetime == timedelta(seconds=3600)
|
|
|
|
|
|
class TestAccountDeactivation:
|
|
def test_deactivated_account_cannot_log_in(self, app, client, make_user, login):
|
|
user_id = make_user('player')
|
|
_set_active(app, user_id, False)
|
|
|
|
with app.app_context():
|
|
username = db.session.get(User, user_id).username
|
|
|
|
login(username)
|
|
response = client.get('/users/profile', follow_redirects=False)
|
|
assert response.status_code in (301, 302)
|
|
assert '/auth/login' in response.headers.get('Location', '')
|
|
|
|
def test_deactivated_account_loses_its_existing_session(self, app, client, as_role):
|
|
"""The fix that matters: revocation has to reach live sessions.
|
|
|
|
is_active_account used to be consulted only at login. User did not
|
|
override UserMixin.is_active, so Flask-Login treated every account
|
|
as active, and disabling someone merely stopped them reconnecting —
|
|
their open session kept working.
|
|
"""
|
|
user_id = as_role('player')
|
|
|
|
assert client.get('/users/profile').status_code == 200
|
|
|
|
_set_active(app, user_id, False)
|
|
|
|
response = client.get('/users/profile', follow_redirects=False)
|
|
assert response.status_code in (301, 302), (
|
|
'a deactivated account kept access with its existing session'
|
|
)
|
|
assert '/auth/login' in response.headers.get('Location', '')
|
|
|
|
def test_is_active_property_tracks_the_column(self, app, make_user):
|
|
user_id = make_user('coach')
|
|
with app.app_context():
|
|
user = db.session.get(User, user_id)
|
|
assert user.is_active is True
|
|
user.is_active_account = False
|
|
assert user.is_active is False
|
|
|
|
|
|
class TestLogout:
|
|
def test_logout_ends_the_session(self, client, as_role):
|
|
as_role('player')
|
|
assert client.get('/users/profile').status_code == 200
|
|
|
|
client.post('/auth/logout')
|
|
|
|
response = client.get('/users/profile', follow_redirects=False)
|
|
assert response.status_code in (301, 302)
|
|
assert '/auth/login' in response.headers.get('Location', '')
|
|
|
|
def test_a_get_no_longer_logs_anyone_out(self, client, as_role):
|
|
"""SEC-019 — a GET route carries no CSRF token, so any page could
|
|
sign the user out with <img src=".../auth/logout">."""
|
|
as_role('player')
|
|
|
|
assert client.get('/auth/logout').status_code == 405
|
|
assert client.get('/users/profile').status_code == 200
|
|
|
|
def test_the_logout_control_carries_a_csrf_token(self, app_with_csrf, make_user, login):
|
|
"""The nav entry is a form now; without the token it would 400 on
|
|
every user, and only in production where CSRF is on."""
|
|
client = app_with_csrf.test_client()
|
|
with app_with_csrf.app_context():
|
|
from app.extensions import hash_password
|
|
from app.models import Player
|
|
|
|
user = Player(
|
|
username='navtest',
|
|
password_hash=hash_password('Password123'),
|
|
role='player',
|
|
full_name='Nav Test',
|
|
email='[email protected]',
|
|
)
|
|
db.session.add(user)
|
|
db.session.commit()
|
|
|
|
page = client.get('/auth/login').get_data(as_text=True)
|
|
token = re.search(r'name="csrf_token" value="([^"]+)"', page).group(1)
|
|
client.post(
|
|
'/auth/login',
|
|
data={'username': 'navtest', 'password': 'Password123', 'csrf_token': token},
|
|
)
|
|
|
|
body = client.get('/users/profile').get_data(as_text=True)
|
|
form = re.search(r'<form method="POST" action="/auth/logout".*?</form>', body, re.S)
|
|
assert form is not None, 'no logout form in the navigation'
|
|
assert 'name="csrf_token"' in form.group(0)
|
|
|
|
|
|
class TestLoginRejection:
|
|
def test_wrong_password_is_refused(self, client, make_user, login, app):
|
|
user_id = make_user('player')
|
|
with app.app_context():
|
|
username = db.session.get(User, user_id).username
|
|
|
|
login(username, password='WrongPassword1')
|
|
|
|
response = client.get('/users/profile', follow_redirects=False)
|
|
assert response.status_code in (301, 302)
|
|
|
|
def test_login_failure_message_does_not_reveal_account_existence(self, client, make_user, app):
|
|
"""Compares the rendered flash messages rather than looking for a
|
|
known substring: the site is served in French by default, so an
|
|
English marker would silently match nothing on both sides and make
|
|
the test pass while the flaw is still there."""
|
|
user_id = make_user('player')
|
|
with app.app_context():
|
|
username = db.session.get(User, user_id).username
|
|
|
|
def failure_message(name):
|
|
body = client.post(
|
|
'/auth/login',
|
|
data={'username': name, 'password': 'WrongPassword1'},
|
|
follow_redirects=True,
|
|
).get_data(as_text=True)
|
|
return _flash_messages(body)
|
|
|
|
assert failure_message(username) == failure_message('no-such-account')
|
|
|
|
def test_the_message_stays_the_same_past_the_attempt_threshold(self, client, make_user, app):
|
|
"""The tally used to be counted out loud — "3 attempt(s) remaining"
|
|
— which is the same disclosure, spread over five requests."""
|
|
user_id = make_user('player')
|
|
with app.app_context():
|
|
username = db.session.get(User, user_id).username
|
|
|
|
seen = set()
|
|
for _ in range(7):
|
|
body = client.post(
|
|
'/auth/login',
|
|
data={'username': username, 'password': 'WrongPassword1'},
|
|
follow_redirects=True,
|
|
).get_data(as_text=True)
|
|
seen.update(_flash_messages(body))
|
|
|
|
assert len(seen) == 1, f'the message varies with the tally: {seen}'
|
|
|
|
|
|
class TestFailedAttemptThrottle:
|
|
"""SEC-018 — five wrong guesses used to take a known account out of
|
|
service for fifteen minutes, repeatably. On a president's account that
|
|
is the whole administration, and nothing in the product could undo it."""
|
|
|
|
@staticmethod
|
|
def _exhaust(client, username, times=6):
|
|
for _ in range(times):
|
|
client.post(
|
|
'/auth/login',
|
|
data={'username': username, 'password': 'WrongPassword1'},
|
|
follow_redirects=True,
|
|
)
|
|
|
|
def test_the_owner_still_gets_in_after_the_threshold(self, app, client, make_user, login):
|
|
user_id = make_user('player')
|
|
with app.app_context():
|
|
username = db.session.get(User, user_id).username
|
|
|
|
self._exhaust(client, username)
|
|
login(username)
|
|
|
|
assert client.get('/users/profile').status_code == 200
|
|
|
|
def test_a_successful_login_clears_the_tally(self, app, client, make_user, login):
|
|
user_id = make_user('player')
|
|
with app.app_context():
|
|
username = db.session.get(User, user_id).username
|
|
|
|
self._exhaust(client, username)
|
|
login(username)
|
|
|
|
with app.app_context():
|
|
user = db.session.get(User, user_id)
|
|
assert user.failed_login_attempts == 0
|
|
assert user.locked_until is None
|
|
|
|
def test_the_window_is_still_recorded(self, app, client, make_user):
|
|
"""It no longer gates anything, but it is the trace an administrator
|
|
reads when an account is being hammered."""
|
|
user_id = make_user('player')
|
|
with app.app_context():
|
|
username = db.session.get(User, user_id).username
|
|
|
|
self._exhaust(client, username)
|
|
|
|
with app.app_context():
|
|
user = db.session.get(User, user_id)
|
|
assert user.failed_login_attempts >= 5
|
|
assert user.locked_until is not None
|
|
|
|
def test_the_window_grows_with_repetition(self):
|
|
from app.routes.auth import MAX_LOCKOUT_MINUTES, cooloff_minutes
|
|
|
|
assert cooloff_minutes(5) == 15
|
|
assert cooloff_minutes(10) == 30
|
|
assert cooloff_minutes(15) == 60
|
|
assert cooloff_minutes(1000) == MAX_LOCKOUT_MINUTES
|
|
|
|
|
|
class TestRedirectValidation:
|
|
"""SEC-020 — is_safe_url() asked urlparse for a netloc, and urlparse
|
|
reads '/\evil.com' as a path. Several browsers normalise the backslash
|
|
to a slash first, which makes it the protocol-relative '//evil.com'."""
|
|
|
|
REFUSED = [
|
|
'//evil.example',
|
|
'/\evil.example',
|
|
'\\evil.example',
|
|
'https://evil.example/phish',
|
|
'javascript:alert(1)',
|
|
'dashboard',
|
|
'/dash\nboard',
|
|
'',
|
|
None,
|
|
]
|
|
|
|
ACCEPTED = [
|
|
'/dashboard',
|
|
'/users/profile?tab=games',
|
|
'/teams#roster',
|
|
]
|
|
|
|
@pytest.mark.parametrize('target', REFUSED)
|
|
def test_a_hostile_target_is_refused(self, app, target):
|
|
from app.routes.auth import is_safe_url
|
|
|
|
with app.test_request_context('/auth/login'):
|
|
assert not is_safe_url(target)
|
|
|
|
@pytest.mark.parametrize('target', ACCEPTED)
|
|
def test_an_internal_path_is_accepted(self, app, target):
|
|
from app.routes.auth import is_safe_url
|
|
|
|
with app.test_request_context('/auth/login'):
|
|
assert is_safe_url(target)
|
|
|
|
def test_the_same_host_spelled_out_is_accepted(self, app):
|
|
from app.routes.auth import is_safe_url
|
|
|
|
with app.test_request_context('/auth/login'):
|
|
from flask import request
|
|
|
|
assert is_safe_url(f'http://{request.host}/dashboard')
|
|
|
|
def test_the_login_redirect_refuses_to_leave_the_site(self, client, make_user, app):
|
|
user_id = make_user('player')
|
|
with app.app_context():
|
|
username = db.session.get(User, user_id).username
|
|
|
|
response = client.post(
|
|
'/auth/login?next=/%5Cevil.example',
|
|
data={'username': username, 'password': 'Password123'},
|
|
follow_redirects=False,
|
|
)
|
|
|
|
assert 'evil.example' not in response.headers.get('Location', '')
|