diff --git a/app/app.py b/app/app.py index a8fab15..8a603f2 100644 --- a/app/app.py +++ b/app/app.py @@ -7,7 +7,7 @@ the Flask application instance with comprehensive security hardening. import os from flask import Flask, request, redirect, jsonify, render_template, url_for 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 werkzeug.exceptions import HTTPException import markupsafe @@ -32,9 +32,15 @@ def nl2br(value): return '' -def create_app(): +def create_app(config=None): """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: - Secret key for session security - Database configuration @@ -54,16 +60,35 @@ def create_app(): Flask: Configured Flask application instance. """ app = Flask(__name__) + + # --- defaults from the environment ------------------------------------ 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']: 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']: raise RuntimeError( '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) 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 # 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()] - + if allowed_origins: CORS( app, @@ -183,10 +208,9 @@ def create_app(): Respects the X-Forwarded-Proto header from reverse proxies. 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 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 @@ -356,14 +380,20 @@ def create_app(): # ========================================================================= with app.app_context(): 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 - try: - from app.discord_bot import start_bot - start_bot(flask_app=app) - except Exception as e: - app.logger.warning('Could not start Discord bot: %s', e) + if app.config['ENABLE_DISCORD_BOT']: + try: + from app.discord_bot import start_bot + start_bot(flask_app=app) + except Exception as e: + app.logger.warning('Could not start Discord bot: %s', e) return app diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..11d0acd --- /dev/null +++ b/pyproject.toml @@ -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" diff --git a/requirements-dev.txt b/requirements-dev.txt new file mode 100644 index 0000000..73f32a2 --- /dev/null +++ b/requirements-dev.txt @@ -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 diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..cfa021b --- /dev/null +++ b/tests/conftest.py @@ -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 diff --git a/tests/test_auth_session.py b/tests/test_auth_session.py new file mode 100644 index 0000000..8a2a8aa --- /dev/null +++ b/tests/test_auth_session.py @@ -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) diff --git a/tests/test_authorization.py b/tests/test_authorization.py new file mode 100644 index 0000000..bf6d860 --- /dev/null +++ b/tests/test_authorization.py @@ -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 = '' + + client.post('/users/profile/edit', data={ + 'username': payload, + 'full_name': 'Legit Name', + 'email': 'legit@example.test', + }, 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': 'legit@example.test', + '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': 'weak@example.test', + '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': 'admin-self@example.test', + '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 diff --git a/tests/test_security_headers.py b/tests/test_security_headers.py new file mode 100644 index 0000000..477a1a1 --- /dev/null +++ b/tests/test_security_headers.py @@ -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 \nsecond')) + + assert '