Active la regle isort (I) de ruff. 45 fichiers reordonnes, aucun changement de comportement : la suite passe avant comme apres. app/models/__init__.py en est exclu. Ses imports sont ranges en onze couches commentees qui decrivent le graphe de dependances ; trier par ordre alphabetique laisse chaque titre au-dessus d un import qu il ne decrit pas, et ce fichier n a qu un role, etre lu. Commit isole, comme le formatage : un diff de brassage ne doit pas servir de couverture a un changement de comportement.
417 lines
14 KiB
Python
417 lines
14 KiB
Python
"""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 datetime import time
|
|
|
|
from app.models import PlayerDisponibility
|
|
|
|
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:
|
|
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'
|
|
)
|
|
|
|
def test_an_admin_cannot_change_its_own_role_even_with_others_present(
|
|
self, app, client, as_role, make_user
|
|
):
|
|
"""Self-demotion is refused on its own, not only when last."""
|
|
make_user('admin')
|
|
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'
|
|
|
|
def test_another_admin_can_still_be_demoted(self, app, client, as_role, make_user):
|
|
"""Guard against over-correcting: the legitimate path must work."""
|
|
other_id = make_user('admin')
|
|
as_role('admin')
|
|
|
|
client.post(
|
|
f'/users/{other_id}/edit',
|
|
data={
|
|
'full_name': 'Other',
|
|
'email': '[email protected]',
|
|
'role': 'coach',
|
|
'is_active_account': 'on',
|
|
},
|
|
follow_redirects=True,
|
|
)
|
|
|
|
with app.app_context():
|
|
assert db.session.get(User, other_id).role == 'coach'
|
|
|
|
|
|
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
|
|
|
|
|
|
class TestCorsPolicy:
|
|
"""SEC-WEB-003 — with no origins configured, flask-cors defaulted to '*'
|
|
and, credentials being allowed, echoed back the caller's Origin."""
|
|
|
|
def test_no_cors_headers_without_explicit_configuration(self, client):
|
|
response = client.get('/auth/login', headers={'Origin': 'https://evil.test'})
|
|
assert 'Access-Control-Allow-Origin' not in response.headers
|
|
assert 'Access-Control-Allow-Credentials' not in response.headers
|
|
|
|
def test_configured_origins_are_still_honoured(self, app_with_csrf):
|
|
from app.app import create_app
|
|
|
|
application = create_app(
|
|
{
|
|
'SECRET_KEY': 'test',
|
|
'SQLALCHEMY_DATABASE_URI': 'sqlite:///:memory:',
|
|
'TESTING': True,
|
|
'FORCE_HTTPS': False,
|
|
'ENABLE_DISCORD_BOT': False,
|
|
'AUTO_CREATE_TABLES': False,
|
|
'CORS_ALLOWED_ORIGINS': 'https://trusted.test',
|
|
}
|
|
)
|
|
response = application.test_client().get(
|
|
'/auth/login', headers={'Origin': 'https://trusted.test'}
|
|
)
|
|
|
|
assert response.headers.get('Access-Control-Allow-Origin') == 'https://trusted.test'
|