test: socle de tests executables et fabrique d'application parametrable
Il n'existait aucun test, et le code n'offrait aucune prise pour en ecrire :
create_app() exigeait SECRET_KEY et DATABASE_URL dans l'environnement,
creait les tables et demarrait un bot Discord. C'etait la cause, pas le
symptome.
create_app(config=None)
Les valeurs par defaut viennent toujours de l'environnement, les
surcharges de l'appelant sont appliquees ensuite, et la validation
vient en dernier pour qu'un test puisse fournir les siennes. Deux
effets de bord passent sous drapeau, actifs par defaut pour que la
production et le developpement se comportent a l'identique :
AUTO_CREATE_TABLES controle db.create_all()
ENABLE_DISCORD_BOT controle start_bot()
FORCE_HTTPS passe egalement en configuration : lu via os.getenv a
chaque requete, il renvoyait un 301 sur tout appel de test.
Suite de tests : 47 tests, 3 xfail, 32 % de couverture.
tests/conftest.py fabriques par role, connexion par le vrai
formulaire, base SQLite temporaire
test_auth_session.py expiration de session, desactivation de
compte, deconnexion
test_security_headers.py en-tetes, non-divulgation sur /health,
echappement de nl2br
test_authorization.py acces anonyme, vertical, horizontal,
validation des entrees, CSRF
Les tests marques xfail(strict=True) decrivent des constats non encore
corriges. Ils echouent par construction ; le mode strict transforme une
reussite inattendue en echec, ce qui signale qu'il faut retirer le
marqueur. Trois subsistent : enumeration de comptes (SEC-AUTH-006), CSP
unsafe-inline (SEC-WEB-001), auto-retrogradation du dernier administrateur
(SEC-AUTHZ-007).
pyproject.toml
Configuration pytest et ruff. Ruff n'avait aucune configuration : la CI
l'executait avec le jeu de regles par defaut. Les 33 F401 de
app/models/__init__.py sont ignores par fichier, c'est une facade de
re-export intentionnelle.
requirements-dev.txt separe l'outillage de test des dependances de
production.
Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
+46
-16
@@ -7,7 +7,7 @@ the Flask application instance with comprehensive security hardening.
|
|||||||
import os
|
import os
|
||||||
from flask import Flask, request, redirect, jsonify, render_template, url_for
|
from flask import Flask, request, redirect, jsonify, render_template, url_for
|
||||||
from flask_cors import CORS
|
from flask_cors import CORS
|
||||||
from app.extensions import db, login_manager, csrf, hash_password, check_password, limiter
|
from app.extensions import db, login_manager, csrf, limiter
|
||||||
from sqlalchemy import text
|
from sqlalchemy import text
|
||||||
from werkzeug.exceptions import HTTPException
|
from werkzeug.exceptions import HTTPException
|
||||||
import markupsafe
|
import markupsafe
|
||||||
@@ -32,9 +32,15 @@ def nl2br(value):
|
|||||||
return ''
|
return ''
|
||||||
|
|
||||||
|
|
||||||
def create_app():
|
def create_app(config=None):
|
||||||
"""Create and configure the Flask application.
|
"""Create and configure the Flask application.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
config: Optional mapping of configuration overrides, applied after the
|
||||||
|
environment defaults and before validation. This is what makes the
|
||||||
|
factory usable from tests: pass a throwaway database URI, a dummy
|
||||||
|
secret, and turn off the Discord bot, without touching os.environ.
|
||||||
|
|
||||||
Initializes Flask with:
|
Initializes Flask with:
|
||||||
- Secret key for session security
|
- Secret key for session security
|
||||||
- Database configuration
|
- Database configuration
|
||||||
@@ -54,16 +60,35 @@ def create_app():
|
|||||||
Flask: Configured Flask application instance.
|
Flask: Configured Flask application instance.
|
||||||
"""
|
"""
|
||||||
app = Flask(__name__)
|
app = Flask(__name__)
|
||||||
|
|
||||||
|
# --- defaults from the environment ------------------------------------
|
||||||
app.config['SECRET_KEY'] = os.getenv('SECRET_KEY')
|
app.config['SECRET_KEY'] = os.getenv('SECRET_KEY')
|
||||||
|
app.config['SQLALCHEMY_DATABASE_URI'] = os.getenv('DATABASE_URL')
|
||||||
|
app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = False
|
||||||
|
app.config['WTF_CSRF_ENABLED'] = True
|
||||||
|
app.config['CORS_ALLOWED_ORIGINS'] = os.getenv('CORS_ALLOWED_ORIGINS', '')
|
||||||
|
app.config['FORCE_HTTPS'] = os.getenv('FORCE_HTTPS', 'true').lower() == 'true'
|
||||||
|
|
||||||
|
# Side effects of create_app(), both on by default so that production and
|
||||||
|
# development behave exactly as before. Tests turn them off.
|
||||||
|
app.config['AUTO_CREATE_TABLES'] = (
|
||||||
|
os.getenv('AUTO_CREATE_TABLES', 'true').lower() == 'true'
|
||||||
|
)
|
||||||
|
app.config['ENABLE_DISCORD_BOT'] = (
|
||||||
|
os.getenv('ENABLE_DISCORD_BOT', 'true').lower() == 'true'
|
||||||
|
)
|
||||||
|
|
||||||
|
# --- caller overrides win ---------------------------------------------
|
||||||
|
if config:
|
||||||
|
app.config.update(config)
|
||||||
|
|
||||||
|
# --- validation, after overrides so tests can supply their own ---------
|
||||||
if not app.config['SECRET_KEY']:
|
if not app.config['SECRET_KEY']:
|
||||||
raise RuntimeError('SECRET_KEY environment variable must be set for security')
|
raise RuntimeError('SECRET_KEY environment variable must be set for security')
|
||||||
app.config['SQLALCHEMY_DATABASE_URI'] = os.getenv('DATABASE_URL')
|
|
||||||
if not app.config['SQLALCHEMY_DATABASE_URI']:
|
if not app.config['SQLALCHEMY_DATABASE_URI']:
|
||||||
raise RuntimeError(
|
raise RuntimeError(
|
||||||
'DATABASE_URL environment variable must be set to a PostgreSQL connection string'
|
'DATABASE_URL environment variable must be set to a PostgreSQL connection string'
|
||||||
)
|
)
|
||||||
app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = False
|
|
||||||
app.config['WTF_CSRF_ENABLED'] = True
|
|
||||||
|
|
||||||
# File upload size limit (16 MB)
|
# File upload size limit (16 MB)
|
||||||
app.config['MAX_CONTENT_LENGTH'] = 16 * 1024 * 1024
|
app.config['MAX_CONTENT_LENGTH'] = 16 * 1024 * 1024
|
||||||
@@ -75,9 +100,9 @@ def create_app():
|
|||||||
app.config['PERMANENT_SESSION_LIFETIME'] = 3600 # 1 hour session timeout
|
app.config['PERMANENT_SESSION_LIFETIME'] = 3600 # 1 hour session timeout
|
||||||
|
|
||||||
# Configure CORS - restrict to specific origins in production
|
# Configure CORS - restrict to specific origins in production
|
||||||
allowed_origins = os.getenv('CORS_ALLOWED_ORIGINS', '').split(',')
|
allowed_origins = str(app.config.get('CORS_ALLOWED_ORIGINS') or '').split(',')
|
||||||
allowed_origins = [origin.strip() for origin in allowed_origins if origin.strip()]
|
allowed_origins = [origin.strip() for origin in allowed_origins if origin.strip()]
|
||||||
|
|
||||||
if allowed_origins:
|
if allowed_origins:
|
||||||
CORS(
|
CORS(
|
||||||
app,
|
app,
|
||||||
@@ -183,10 +208,9 @@ def create_app():
|
|||||||
Respects the X-Forwarded-Proto header from reverse proxies.
|
Respects the X-Forwarded-Proto header from reverse proxies.
|
||||||
Can be disabled via FORCE_HTTPS environment variable.
|
Can be disabled via FORCE_HTTPS environment variable.
|
||||||
"""
|
"""
|
||||||
if not app.debug:
|
if not app.debug and app.config['FORCE_HTTPS']:
|
||||||
if not request.is_secure and request.headers.get('X-Forwarded-Proto') != 'https':
|
if not request.is_secure and request.headers.get('X-Forwarded-Proto') != 'https':
|
||||||
if os.getenv('FORCE_HTTPS', 'true').lower() == 'true':
|
return redirect(request.url.replace('http://', 'https://'), code=301)
|
||||||
return redirect(request.url.replace('http://', 'https://'), code=301)
|
|
||||||
|
|
||||||
# =========================================================================
|
# =========================================================================
|
||||||
# Health Check Endpoint
|
# Health Check Endpoint
|
||||||
@@ -356,14 +380,20 @@ def create_app():
|
|||||||
# =========================================================================
|
# =========================================================================
|
||||||
with app.app_context():
|
with app.app_context():
|
||||||
import app.models as models # noqa: F401 — registers all models with SQLAlchemy
|
import app.models as models # noqa: F401 — registers all models with SQLAlchemy
|
||||||
db.create_all()
|
# NOTE: create_all() only ever creates missing tables. It never adds a
|
||||||
|
# column to an existing one, so a model change is silently absent from
|
||||||
|
# any database that already has the table. Replacing this with Alembic
|
||||||
|
# is tracked as DB-002/DB-004; until then the behaviour is preserved.
|
||||||
|
if app.config['AUTO_CREATE_TABLES']:
|
||||||
|
db.create_all()
|
||||||
|
|
||||||
# Start the Discord bot for notifications
|
# Start the Discord bot for notifications
|
||||||
try:
|
if app.config['ENABLE_DISCORD_BOT']:
|
||||||
from app.discord_bot import start_bot
|
try:
|
||||||
start_bot(flask_app=app)
|
from app.discord_bot import start_bot
|
||||||
except Exception as e:
|
start_bot(flask_app=app)
|
||||||
app.logger.warning('Could not start Discord bot: %s', e)
|
except Exception as e:
|
||||||
|
app.logger.warning('Could not start Discord bot: %s', e)
|
||||||
|
|
||||||
return app
|
return app
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,42 @@
|
|||||||
|
# Tooling configuration.
|
||||||
|
#
|
||||||
|
# Deliberately limited to tool settings: the project is run from wsgi.py,
|
||||||
|
# not installed as a distribution, so there is no [project] table yet.
|
||||||
|
# Consolidating requirements.txt / requirements-dev.txt into dependency
|
||||||
|
# groups here is tracked as QUA-001.
|
||||||
|
|
||||||
|
[tool.pytest.ini_options]
|
||||||
|
testpaths = ["tests"]
|
||||||
|
pythonpath = ["."]
|
||||||
|
addopts = "-q --strict-markers --strict-config"
|
||||||
|
filterwarnings = [
|
||||||
|
"default",
|
||||||
|
# discord.py imports audioop, removed from the stdlib in 3.13.
|
||||||
|
"ignore:'audioop' is deprecated:DeprecationWarning",
|
||||||
|
# Every model uses datetime.utcnow as a column default. Tracked as
|
||||||
|
# DB-009; the warning would otherwise drown the run.
|
||||||
|
"ignore:datetime.datetime.utcnow:DeprecationWarning",
|
||||||
|
]
|
||||||
|
|
||||||
|
[tool.ruff]
|
||||||
|
line-length = 100
|
||||||
|
target-version = "py312"
|
||||||
|
exclude = [".venv", "venv", "migrations", "docs"]
|
||||||
|
|
||||||
|
[tool.ruff.lint]
|
||||||
|
# Starting from ruff's default rule set (pyflakes + a slice of pycodestyle).
|
||||||
|
# Widening it — bugbear, isort, pyupgrade — is deliberately deferred until
|
||||||
|
# the codebase has been formatted once, so that the first enforcement is
|
||||||
|
# about real defects rather than churn. Tracked as QUA-002.
|
||||||
|
select = ["E4", "E7", "E9", "F"]
|
||||||
|
|
||||||
|
[tool.ruff.lint.per-file-ignores]
|
||||||
|
# Intentional re-export facade: `from app.models import User, Tryout, ...`
|
||||||
|
# is the documented entry point, and importing the modules is what
|
||||||
|
# registers every model with SQLAlchemy.
|
||||||
|
"app/models/__init__.py" = ["F401"]
|
||||||
|
"app/models/user_model/__init__.py" = ["F401"]
|
||||||
|
"tests/conftest.py" = ["E402"]
|
||||||
|
|
||||||
|
[tool.ruff.format]
|
||||||
|
quote-style = "preserve"
|
||||||
@@ -0,0 +1,13 @@
|
|||||||
|
# Development and test dependencies.
|
||||||
|
# Install with: pip install -r requirements.txt -r requirements-dev.txt
|
||||||
|
#
|
||||||
|
# Kept separate from requirements.txt so that a production install stays
|
||||||
|
# free of test tooling. Consolidating both into a pyproject.toml with
|
||||||
|
# dependency groups is tracked as QUA-001.
|
||||||
|
|
||||||
|
-r requirements.txt
|
||||||
|
|
||||||
|
pytest==8.4.2
|
||||||
|
pytest-cov==7.0.0
|
||||||
|
ruff==0.14.4
|
||||||
|
pip-audit==2.9.0
|
||||||
@@ -0,0 +1,173 @@
|
|||||||
|
"""Shared pytest fixtures for the Team Tryouts test suite.
|
||||||
|
|
||||||
|
The application factory is driven entirely through the ``config`` argument
|
||||||
|
here: no environment variable is required to run the suite, no database
|
||||||
|
server is needed, and the Discord bot never starts.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import os
|
||||||
|
import sys
|
||||||
|
import tempfile
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
# Make the project root importable as the 'app' package.
|
||||||
|
sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
|
||||||
|
|
||||||
|
from app.app import create_app # noqa: E402
|
||||||
|
from app.extensions import db as _db # noqa: E402
|
||||||
|
from app.models import Admin, Coach, Manager, Player, Scout # noqa: E402
|
||||||
|
|
||||||
|
|
||||||
|
ROLE_CLASSES = {
|
||||||
|
'admin': Admin,
|
||||||
|
'manager': Manager,
|
||||||
|
'coach': Coach,
|
||||||
|
'player': Player,
|
||||||
|
'scout': Scout,
|
||||||
|
}
|
||||||
|
|
||||||
|
#: Satisfies the documented policy (8+ chars, upper, lower, digit).
|
||||||
|
VALID_PASSWORD = 'Password123'
|
||||||
|
|
||||||
|
|
||||||
|
def _base_config(db_path, csrf=False):
|
||||||
|
return {
|
||||||
|
'SECRET_KEY': 'test-secret-not-used-anywhere-real',
|
||||||
|
'SQLALCHEMY_DATABASE_URI': f'sqlite:///{db_path}',
|
||||||
|
'TESTING': True,
|
||||||
|
'WTF_CSRF_ENABLED': csrf,
|
||||||
|
# Without these three the suite would 301 every request, start a
|
||||||
|
# Discord bot, and refuse to issue cookies over the test client.
|
||||||
|
'FORCE_HTTPS': False,
|
||||||
|
'SESSION_COOKIE_SECURE': False,
|
||||||
|
'ENABLE_DISCORD_BOT': False,
|
||||||
|
# The schema still comes from create_all() until Alembic lands (DB-002).
|
||||||
|
'AUTO_CREATE_TABLES': True,
|
||||||
|
'CORS_ALLOWED_ORIGINS': '',
|
||||||
|
'RATELIMIT_ENABLED': False,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def app():
|
||||||
|
"""A fully configured application backed by a throwaway SQLite file.
|
||||||
|
|
||||||
|
A file rather than :memory: because Flask-SQLAlchemy hands out a
|
||||||
|
connection per thread, and an in-memory database is not shared between
|
||||||
|
them — tables created on one connection would be invisible to the next.
|
||||||
|
"""
|
||||||
|
fd, db_path = tempfile.mkstemp(suffix='.sqlite')
|
||||||
|
os.close(fd)
|
||||||
|
|
||||||
|
application = create_app(_base_config(db_path))
|
||||||
|
|
||||||
|
yield application
|
||||||
|
|
||||||
|
with application.app_context():
|
||||||
|
_db.session.remove()
|
||||||
|
_db.engine.dispose()
|
||||||
|
try:
|
||||||
|
os.unlink(db_path)
|
||||||
|
except OSError:
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def app_with_csrf():
|
||||||
|
"""Same application, with CSRF protection left switched on."""
|
||||||
|
fd, db_path = tempfile.mkstemp(suffix='.sqlite')
|
||||||
|
os.close(fd)
|
||||||
|
|
||||||
|
application = create_app(_base_config(db_path, csrf=True))
|
||||||
|
|
||||||
|
yield application
|
||||||
|
|
||||||
|
with application.app_context():
|
||||||
|
_db.session.remove()
|
||||||
|
_db.engine.dispose()
|
||||||
|
try:
|
||||||
|
os.unlink(db_path)
|
||||||
|
except OSError:
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def client(app):
|
||||||
|
return app.test_client()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def db(app):
|
||||||
|
"""Database handle bound to an active application context."""
|
||||||
|
with app.app_context():
|
||||||
|
yield _db
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def make_user(app):
|
||||||
|
"""Factory creating a user of a given role and returning its id.
|
||||||
|
|
||||||
|
Returns the primary key rather than the instance: the object would be
|
||||||
|
detached once the fixture's application context is popped, and every
|
||||||
|
caller wants to look it up inside its own context anyway.
|
||||||
|
"""
|
||||||
|
counter = {'n': 0}
|
||||||
|
|
||||||
|
def _make(role='player', password=VALID_PASSWORD, **kwargs):
|
||||||
|
from app.extensions import hash_password
|
||||||
|
|
||||||
|
counter['n'] += 1
|
||||||
|
n = counter['n']
|
||||||
|
cls = ROLE_CLASSES[role]
|
||||||
|
with app.app_context():
|
||||||
|
user = cls(
|
||||||
|
username=kwargs.pop('username', f'{role}{n}'),
|
||||||
|
password_hash=hash_password(password),
|
||||||
|
role=role,
|
||||||
|
full_name=kwargs.pop('full_name', f'{role.title()} {n}'),
|
||||||
|
email=kwargs.pop('email', f'{role}{n}@example.test'),
|
||||||
|
**kwargs,
|
||||||
|
)
|
||||||
|
_db.session.add(user)
|
||||||
|
_db.session.commit()
|
||||||
|
return user.id
|
||||||
|
|
||||||
|
return _make
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def login(client):
|
||||||
|
"""Log a user in through the real login form.
|
||||||
|
|
||||||
|
Deliberately exercises the actual authentication path rather than
|
||||||
|
poking flask_login's session key, so that session handling itself
|
||||||
|
stays under test.
|
||||||
|
"""
|
||||||
|
|
||||||
|
def _login(username, password=VALID_PASSWORD):
|
||||||
|
return client.post(
|
||||||
|
'/auth/login',
|
||||||
|
data={'username': username, 'password': password},
|
||||||
|
follow_redirects=False,
|
||||||
|
)
|
||||||
|
|
||||||
|
return _login
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def as_role(app, client, make_user, login):
|
||||||
|
"""Create a user of the given role, log in, and return its id."""
|
||||||
|
|
||||||
|
def _as(role='player', **kwargs):
|
||||||
|
user_id = make_user(role, **kwargs)
|
||||||
|
with app.app_context():
|
||||||
|
from app.models import User
|
||||||
|
username = _db.session.get(User, user_id).username
|
||||||
|
response = login(username)
|
||||||
|
assert response.status_code in (301, 302), (
|
||||||
|
f'login for {username} did not redirect: {response.status_code}'
|
||||||
|
)
|
||||||
|
return user_id
|
||||||
|
|
||||||
|
return _as
|
||||||
@@ -0,0 +1,132 @@
|
|||||||
|
"""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 pytest
|
||||||
|
|
||||||
|
from app.extensions import db
|
||||||
|
from app.models import User
|
||||||
|
|
||||||
|
|
||||||
|
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.get('/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', '')
|
||||||
|
|
||||||
|
|
||||||
|
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)
|
||||||
|
|
||||||
|
@pytest.mark.xfail(
|
||||||
|
strict=True,
|
||||||
|
reason='SEC-AUTH-006: the two branches emit different messages, '
|
||||||
|
'which lets an unauthenticated caller enumerate accounts',
|
||||||
|
)
|
||||||
|
def test_login_failure_message_does_not_reveal_account_existence(
|
||||||
|
self, client, make_user, app
|
||||||
|
):
|
||||||
|
user_id = make_user('player')
|
||||||
|
with app.app_context():
|
||||||
|
username = db.session.get(User, user_id).username
|
||||||
|
|
||||||
|
existing = client.post(
|
||||||
|
'/auth/login',
|
||||||
|
data={'username': username, 'password': 'WrongPassword1'},
|
||||||
|
follow_redirects=True,
|
||||||
|
).get_data(as_text=True)
|
||||||
|
|
||||||
|
unknown = client.post(
|
||||||
|
'/auth/login',
|
||||||
|
data={'username': 'no-such-account', 'password': 'WrongPassword1'},
|
||||||
|
follow_redirects=True,
|
||||||
|
).get_data(as_text=True)
|
||||||
|
|
||||||
|
assert ('attempt(s) remaining' in existing) == ('attempt(s) remaining' in unknown)
|
||||||
@@ -0,0 +1,333 @@
|
|||||||
|
"""Access control regression tests.
|
||||||
|
|
||||||
|
Two kinds of test live here.
|
||||||
|
|
||||||
|
Passing tests pin down behaviour that is currently correct, so that the
|
||||||
|
architecture work in wave D — unifying the two coach/team models — cannot
|
||||||
|
quietly break it.
|
||||||
|
|
||||||
|
Tests marked xfail(strict=True) describe behaviour the audit found missing.
|
||||||
|
They fail today by design and will start passing when the matching finding
|
||||||
|
is fixed; strict mode then turns the unexpected pass into a failure, which
|
||||||
|
is the signal to remove the marker. They are executable documentation of
|
||||||
|
the gap, not a wish list.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from app.extensions import db
|
||||||
|
from app.models import User
|
||||||
|
|
||||||
|
#: Routes that must never answer to an unauthenticated caller.
|
||||||
|
PROTECTED_ROUTES = [
|
||||||
|
'/users',
|
||||||
|
'/users/create',
|
||||||
|
'/users/profile',
|
||||||
|
'/users/contracts',
|
||||||
|
'/tryouts',
|
||||||
|
'/teams',
|
||||||
|
'/evaluations',
|
||||||
|
'/matches/calendar',
|
||||||
|
'/team-matches',
|
||||||
|
]
|
||||||
|
|
||||||
|
#: Admin-only user management surface.
|
||||||
|
ADMIN_ONLY_ROUTES = [
|
||||||
|
'/users',
|
||||||
|
'/users/create',
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
|
def _redirected(response):
|
||||||
|
return response.status_code in (301, 302)
|
||||||
|
|
||||||
|
|
||||||
|
def _username(app, user_id):
|
||||||
|
with app.app_context():
|
||||||
|
return db.session.get(User, user_id).username
|
||||||
|
|
||||||
|
|
||||||
|
class TestAnonymousAccess:
|
||||||
|
@pytest.mark.parametrize('route', PROTECTED_ROUTES)
|
||||||
|
def test_anonymous_is_sent_to_login(self, client, route):
|
||||||
|
response = client.get(route, follow_redirects=False)
|
||||||
|
assert _redirected(response), f'{route} answered an anonymous caller'
|
||||||
|
assert '/auth/login' in response.headers.get('Location', '')
|
||||||
|
|
||||||
|
|
||||||
|
class TestVerticalAccess:
|
||||||
|
@pytest.mark.parametrize('route', ADMIN_ONLY_ROUTES)
|
||||||
|
@pytest.mark.parametrize('role', ['player', 'coach', 'manager', 'scout'])
|
||||||
|
def test_only_admin_reaches_user_management(self, client, as_role, role, route):
|
||||||
|
as_role(role)
|
||||||
|
response = client.get(route, follow_redirects=False)
|
||||||
|
assert _redirected(response), (
|
||||||
|
f'{role} reached {route}, which is meant to be admin-only'
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_admin_reaches_user_management(self, client, as_role):
|
||||||
|
as_role('admin')
|
||||||
|
assert client.get('/users').status_code == 200
|
||||||
|
|
||||||
|
def test_player_cannot_list_evaluations(self, client, as_role):
|
||||||
|
as_role('player')
|
||||||
|
assert _redirected(client.get('/evaluations', follow_redirects=False))
|
||||||
|
|
||||||
|
def test_scout_cannot_list_teams(self, client, as_role):
|
||||||
|
as_role('scout')
|
||||||
|
assert _redirected(client.get('/teams', follow_redirects=False))
|
||||||
|
|
||||||
|
def test_non_player_cannot_request_one_on_one(self, client, as_role):
|
||||||
|
as_role('coach')
|
||||||
|
assert _redirected(client.get('/users/one-on-one', follow_redirects=False))
|
||||||
|
|
||||||
|
def test_non_coach_cannot_reach_notes_dashboard(self, client, as_role):
|
||||||
|
as_role('manager')
|
||||||
|
assert _redirected(client.get('/users/notes-dashboard', follow_redirects=False))
|
||||||
|
|
||||||
|
def test_player_cannot_delete_another_user(self, app, client, as_role, make_user):
|
||||||
|
victim_id = make_user('player')
|
||||||
|
as_role('player')
|
||||||
|
|
||||||
|
response = client.post(f'/users/{victim_id}/delete', follow_redirects=False)
|
||||||
|
assert _redirected(response)
|
||||||
|
|
||||||
|
with app.app_context():
|
||||||
|
assert db.session.get(User, victim_id) is not None, 'the user was deleted'
|
||||||
|
|
||||||
|
|
||||||
|
class TestHorizontalAccess:
|
||||||
|
def test_player_cannot_delete_another_players_availability(
|
||||||
|
self, app, client, as_role, make_user
|
||||||
|
):
|
||||||
|
from app.models import PlayerDisponibility
|
||||||
|
from datetime import time
|
||||||
|
|
||||||
|
owner_id = make_user('player')
|
||||||
|
with app.app_context():
|
||||||
|
slot = PlayerDisponibility(
|
||||||
|
player_id=owner_id, day_of_week=1,
|
||||||
|
start_time=time(10, 0), end_time=time(10, 30),
|
||||||
|
)
|
||||||
|
db.session.add(slot)
|
||||||
|
db.session.commit()
|
||||||
|
slot_id = slot.id
|
||||||
|
|
||||||
|
as_role('player')
|
||||||
|
response = client.post(f'/users/disponibilities/{slot_id}/delete')
|
||||||
|
|
||||||
|
assert response.status_code == 403
|
||||||
|
with app.app_context():
|
||||||
|
assert db.session.get(PlayerDisponibility, slot_id) is not None
|
||||||
|
|
||||||
|
|
||||||
|
class TestNestedResourceOwnership:
|
||||||
|
"""SEC-AUTHZ-002 — routes taking both a parent and a child id have to
|
||||||
|
check that the child actually belongs to the parent. Authorising only
|
||||||
|
the parent lets a manager of tryout A reach a team of tryout B."""
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _make_tryout_with_team(app, owner_id, title):
|
||||||
|
from datetime import date
|
||||||
|
|
||||||
|
from app.models import Team, Tryout
|
||||||
|
|
||||||
|
with app.app_context():
|
||||||
|
tryout = Tryout(
|
||||||
|
title=title, game='Valorant', date=date(2030, 1, 1),
|
||||||
|
created_by=owner_id, status='upcoming',
|
||||||
|
)
|
||||||
|
db.session.add(tryout)
|
||||||
|
db.session.flush()
|
||||||
|
team = Team(tryout_id=tryout.id, name=f'{title} squad', created_by=owner_id)
|
||||||
|
db.session.add(team)
|
||||||
|
db.session.commit()
|
||||||
|
return tryout.id, team.id
|
||||||
|
|
||||||
|
def test_cannot_add_a_player_to_a_team_of_another_tryout(
|
||||||
|
self, app, client, as_role, make_user
|
||||||
|
):
|
||||||
|
from app.models import TeamMember, TryoutRegistration
|
||||||
|
|
||||||
|
other_admin = make_user('admin')
|
||||||
|
_, foreign_team_id = self._make_tryout_with_team(app, other_admin, 'Foreign')
|
||||||
|
|
||||||
|
manager_id = as_role('manager')
|
||||||
|
own_tryout_id, _ = self._make_tryout_with_team(app, manager_id, 'Mine')
|
||||||
|
|
||||||
|
player_id = make_user('player')
|
||||||
|
with app.app_context():
|
||||||
|
db.session.add(TryoutRegistration(
|
||||||
|
tryout_id=own_tryout_id, player_id=player_id))
|
||||||
|
db.session.commit()
|
||||||
|
|
||||||
|
response = client.post(
|
||||||
|
f'/tryouts/{own_tryout_id}/team/{foreign_team_id}/add',
|
||||||
|
data={'player_id': player_id},
|
||||||
|
follow_redirects=False,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert response.status_code == 404, (
|
||||||
|
'a team belonging to another tryout was accepted'
|
||||||
|
)
|
||||||
|
with app.app_context():
|
||||||
|
assert TeamMember.query.filter_by(team_id=foreign_team_id).count() == 0
|
||||||
|
|
||||||
|
def test_cannot_add_a_player_who_is_not_registered(
|
||||||
|
self, app, client, as_role, make_user
|
||||||
|
):
|
||||||
|
from app.models import TeamMember
|
||||||
|
|
||||||
|
manager_id = as_role('manager')
|
||||||
|
tryout_id, team_id = self._make_tryout_with_team(app, manager_id, 'Mine')
|
||||||
|
outsider_id = make_user('player')
|
||||||
|
|
||||||
|
client.post(
|
||||||
|
f'/tryouts/{tryout_id}/team/{team_id}/add',
|
||||||
|
data={'player_id': outsider_id},
|
||||||
|
follow_redirects=True,
|
||||||
|
)
|
||||||
|
|
||||||
|
with app.app_context():
|
||||||
|
assert TeamMember.query.filter_by(team_id=team_id).count() == 0
|
||||||
|
|
||||||
|
def test_a_registered_player_can_still_be_added(
|
||||||
|
self, app, client, as_role, make_user
|
||||||
|
):
|
||||||
|
"""Guard against over-correcting: the normal path must keep working."""
|
||||||
|
from app.models import TeamMember, TryoutRegistration
|
||||||
|
|
||||||
|
manager_id = as_role('manager')
|
||||||
|
tryout_id, team_id = self._make_tryout_with_team(app, manager_id, 'Mine')
|
||||||
|
player_id = make_user('player')
|
||||||
|
|
||||||
|
with app.app_context():
|
||||||
|
db.session.add(TryoutRegistration(tryout_id=tryout_id, player_id=player_id))
|
||||||
|
db.session.commit()
|
||||||
|
|
||||||
|
client.post(
|
||||||
|
f'/tryouts/{tryout_id}/team/{team_id}/add',
|
||||||
|
data={'player_id': player_id, 'position': 'Duelist'},
|
||||||
|
follow_redirects=True,
|
||||||
|
)
|
||||||
|
|
||||||
|
with app.app_context():
|
||||||
|
member = TeamMember.query.filter_by(team_id=team_id).one()
|
||||||
|
assert member.player_id == player_id
|
||||||
|
assert member.position == 'Duelist'
|
||||||
|
|
||||||
|
def test_a_non_numeric_player_id_does_not_crash(self, app, client, as_role):
|
||||||
|
"""int(player_id) on raw form input used to raise, i.e. a 500."""
|
||||||
|
manager_id = as_role('manager')
|
||||||
|
tryout_id, team_id = self._make_tryout_with_team(app, manager_id, 'Mine')
|
||||||
|
|
||||||
|
response = client.post(
|
||||||
|
f'/tryouts/{tryout_id}/team/{team_id}/add',
|
||||||
|
data={'player_id': 'not-a-number'},
|
||||||
|
follow_redirects=False,
|
||||||
|
)
|
||||||
|
assert response.status_code < 500
|
||||||
|
|
||||||
|
|
||||||
|
class TestInputValidation:
|
||||||
|
"""SEC-AUTHZ-001 — regression guard.
|
||||||
|
|
||||||
|
CreateUserSchema, EditUserSchema and EditProfileSchema used to be
|
||||||
|
imported at users.py:23-26 and never called: each name appeared exactly
|
||||||
|
once in the file, on its import line. The three routes read request.form
|
||||||
|
directly, so no password policy and no username format rule applied
|
||||||
|
anywhere in user management. These tests fail if that ever comes back.
|
||||||
|
"""
|
||||||
|
|
||||||
|
def test_edit_profile_rejects_html_in_username(self, app, client, as_role):
|
||||||
|
user_id = as_role('player')
|
||||||
|
payload = '<img src=x onerror=alert(1)>'
|
||||||
|
|
||||||
|
client.post('/users/profile/edit', data={
|
||||||
|
'username': payload,
|
||||||
|
'full_name': 'Legit Name',
|
||||||
|
'email': '[email protected]',
|
||||||
|
}, follow_redirects=True)
|
||||||
|
|
||||||
|
with app.app_context():
|
||||||
|
assert db.session.get(User, user_id).username != payload
|
||||||
|
|
||||||
|
def test_edit_profile_enforces_the_password_policy(self, app, client, as_role):
|
||||||
|
user_id = as_role('player')
|
||||||
|
with app.app_context():
|
||||||
|
before = db.session.get(User, user_id).password_hash
|
||||||
|
|
||||||
|
client.post('/users/profile/edit', data={
|
||||||
|
'username': _username(app, user_id),
|
||||||
|
'full_name': 'Legit Name',
|
||||||
|
'email': '[email protected]',
|
||||||
|
'password': 'a',
|
||||||
|
}, follow_redirects=True)
|
||||||
|
|
||||||
|
with app.app_context():
|
||||||
|
assert db.session.get(User, user_id).password_hash == before, (
|
||||||
|
'a one-character password was accepted'
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_create_user_enforces_the_password_policy(self, app, client, as_role):
|
||||||
|
as_role('admin')
|
||||||
|
|
||||||
|
client.post('/users/create', data={
|
||||||
|
'username': 'weakling',
|
||||||
|
'email': '[email protected]',
|
||||||
|
'password': 'a',
|
||||||
|
'full_name': 'Weak Account',
|
||||||
|
'role': 'admin',
|
||||||
|
}, follow_redirects=True)
|
||||||
|
|
||||||
|
with app.app_context():
|
||||||
|
created = User.query.filter_by(username='weakling').first()
|
||||||
|
assert created is None, 'an admin account was created with password "a"'
|
||||||
|
|
||||||
|
def test_edit_user_rejects_a_duplicate_email(self, app, client, as_role, make_user):
|
||||||
|
other_id = make_user('player')
|
||||||
|
target_id = make_user('player')
|
||||||
|
as_role('admin')
|
||||||
|
|
||||||
|
with app.app_context():
|
||||||
|
taken = db.session.get(User, other_id).email
|
||||||
|
|
||||||
|
response = client.post(f'/users/{target_id}/edit', data={
|
||||||
|
'full_name': 'Target',
|
||||||
|
'email': taken,
|
||||||
|
'role': 'player',
|
||||||
|
}, follow_redirects=False)
|
||||||
|
|
||||||
|
assert response.status_code < 500, 'duplicate email produced a server error'
|
||||||
|
|
||||||
|
|
||||||
|
class TestAdminSafety:
|
||||||
|
@pytest.mark.xfail(
|
||||||
|
strict=True,
|
||||||
|
reason='SEC-AUTHZ-007: the role change excludes neither the current '
|
||||||
|
'user nor the last remaining admin',
|
||||||
|
)
|
||||||
|
def test_the_last_admin_cannot_demote_itself(self, app, client, as_role):
|
||||||
|
admin_id = as_role('admin')
|
||||||
|
|
||||||
|
client.post(f'/users/{admin_id}/edit', data={
|
||||||
|
'full_name': 'Admin',
|
||||||
|
'email': '[email protected]',
|
||||||
|
'role': 'player',
|
||||||
|
}, follow_redirects=True)
|
||||||
|
|
||||||
|
with app.app_context():
|
||||||
|
assert db.session.get(User, admin_id).role == 'admin', (
|
||||||
|
'the only administrator demoted itself; no interface can undo this'
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
class TestCsrf:
|
||||||
|
def test_state_changing_post_without_a_token_is_rejected(self, app_with_csrf):
|
||||||
|
"""CSRFProtect is global. This pins that down so a future
|
||||||
|
@csrf.exempt cannot slip in unnoticed."""
|
||||||
|
client = app_with_csrf.test_client()
|
||||||
|
response = client.post('/auth/login', data={
|
||||||
|
'username': 'someone', 'password': 'Password123',
|
||||||
|
})
|
||||||
|
assert response.status_code == 400
|
||||||
@@ -0,0 +1,82 @@
|
|||||||
|
"""HTTP hardening, error disclosure, and template escaping."""
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from app.app import nl2br
|
||||||
|
|
||||||
|
|
||||||
|
class TestSecurityHeaders:
|
||||||
|
def test_core_headers_are_present(self, client):
|
||||||
|
headers = client.get('/auth/login').headers
|
||||||
|
|
||||||
|
assert headers['X-Content-Type-Options'] == 'nosniff'
|
||||||
|
assert headers['X-Frame-Options'] == 'DENY'
|
||||||
|
assert headers['Referrer-Policy'] == 'strict-origin-when-cross-origin'
|
||||||
|
assert 'frame-ancestors' in headers['Content-Security-Policy']
|
||||||
|
|
||||||
|
def test_deprecated_xss_auditor_header_is_not_sent(self, client):
|
||||||
|
"""X-XSS-Protection was removed: deprecated, and harmful in its
|
||||||
|
last implementations."""
|
||||||
|
assert 'X-XSS-Protection' not in client.get('/auth/login').headers
|
||||||
|
|
||||||
|
@pytest.mark.xfail(
|
||||||
|
strict=True,
|
||||||
|
reason="SEC-WEB-001: 15 inline <script> blocks still require "
|
||||||
|
"'unsafe-inline'; lifting it is tracked as OPS-010",
|
||||||
|
)
|
||||||
|
def test_csp_does_not_allow_inline_script(self, client):
|
||||||
|
csp = client.get('/auth/login').headers['Content-Security-Policy']
|
||||||
|
script_src = [d for d in csp.split(';') if d.strip().startswith('script-src')][0]
|
||||||
|
assert "'unsafe-inline'" not in script_src
|
||||||
|
|
||||||
|
|
||||||
|
class TestErrorDisclosure:
|
||||||
|
def test_health_reports_status_without_the_driver_message(self, app, client):
|
||||||
|
"""/health is unauthenticated. Driver exceptions routinely carry the
|
||||||
|
host, database name and user of the connection string."""
|
||||||
|
from app.extensions import db
|
||||||
|
|
||||||
|
def boom(*args, **kwargs):
|
||||||
|
raise RuntimeError(
|
||||||
|
'FATAL: password authentication failed for user "app" '
|
||||||
|
'host=db.internal port=5432'
|
||||||
|
)
|
||||||
|
|
||||||
|
original = db.session.execute
|
||||||
|
db.session.execute = boom
|
||||||
|
try:
|
||||||
|
response = client.get('/health')
|
||||||
|
finally:
|
||||||
|
db.session.execute = original
|
||||||
|
|
||||||
|
assert response.status_code == 503
|
||||||
|
body = response.get_data(as_text=True)
|
||||||
|
assert response.get_json()['database'] == 'error'
|
||||||
|
for leaked in ('password', 'db.internal', '5432', 'FATAL'):
|
||||||
|
assert leaked not in body, f'{leaked!r} leaked through /health'
|
||||||
|
|
||||||
|
def test_health_reports_healthy_when_the_database_answers(self, client):
|
||||||
|
response = client.get('/health')
|
||||||
|
assert response.status_code == 200
|
||||||
|
assert response.get_json()['database'] == 'connected'
|
||||||
|
|
||||||
|
|
||||||
|
class TestTemplateEscaping:
|
||||||
|
def test_nl2br_escapes_markup(self, app):
|
||||||
|
"""Markup('<br>'.join(...)) marked attacker text as safe.
|
||||||
|
Markup('<br>').join(...) escapes each segment first."""
|
||||||
|
with app.app_context():
|
||||||
|
rendered = str(nl2br('<script>alert(1)</script>\nsecond'))
|
||||||
|
|
||||||
|
assert '<script>' not in rendered
|
||||||
|
assert '<script>' in rendered
|
||||||
|
assert '<br>' in rendered, 'the line break itself must stay markup'
|
||||||
|
|
||||||
|
def test_nl2br_still_joins_lines(self, app):
|
||||||
|
with app.app_context():
|
||||||
|
assert str(nl2br('a\nb')) == 'a<br>b'
|
||||||
|
|
||||||
|
def test_nl2br_handles_empty_input(self, app):
|
||||||
|
with app.app_context():
|
||||||
|
assert nl2br('') == ''
|
||||||
|
assert nl2br(None) == ''
|
||||||
Reference in New Issue
Block a user