Files
team-tryouts/tests/test_authorization.py
T
GGThedandClaude Opus 5 dd8b9671c6 fix(authz): teams.py etait le module que la validation n avait pas atteint
SEC-16, de l'audit anterieur. La vague G a pose un schema a la frontiere de
matches, team_matches, tryouts et evaluations, et le HANDOFF en a tire une
regle : tout champ de formulaire passe par un schema de app/validators.py,
pas par request.form.get. teams.py ne l'avait jamais appliquee.

Deux defauts, pas un.

int(request.form.get('coach_id')) leve sur une valeur non numerique : une
soumission fabriquee etait un 500.

Et l'identifiant obtenu etait ensuite resolu sans verifier le role du compte,
dans deux des trois endroits qui le faisaient. La branche sync_staff
d'edit_team testait isinstance(user, Coach) ; son autre branche non, et
create_team non plus. Le meme fichier en desaccord avec lui-meme, sur
exactement le defaut que la vague G avait corrige dans tryouts.py -- une
soumission fabriquee pouvait nommer un joueur parmi les coachs d'une equipe.
L'identifiant vient d'un <select> rendu par le navigateur : c'est une valeur
que le client choisit.

_staff_member est la reponse unique, et OrgTeamSchema garantit que les
identifiants arrivent en entiers. Les deux tests qui epinglent le role
tombent si la verification saute : verifie par mutation.

Co-Authored-By: Claude Opus 5 <[email protected]>
2026-08-11 19:37:13 -04:00

586 lines
20 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'
class TestTeamDeletionGuard:
"""SEC-AUTHZ-006 — the one team operation guarded differently.
`delete_team` used the global `can_manage_teams()` while the other nine
team operations use `can_manage_this_org_team(team)`. The constat asked
for a straight swap; a straight swap would have handed every coach the
power to delete the team they coach, because `Coach` answers False to
the first and True to the second. It now requires both, which keeps
today's answers and narrows automatically if the per-object rule is ever
tightened.
These tests are what stops the "simplification" from being reapplied.
"""
@pytest.fixture
def org_team(self, app, make_user):
from app.models import OrgTeam
admin_id = make_user('admin')
coach_id = make_user('coach')
with app.app_context():
team = OrgTeam(name='Varsity', created_by=admin_id, coach_id=coach_id)
db.session.add(team)
db.session.commit()
return team.id, coach_id
def test_a_coach_cannot_delete_the_team_they_coach(self, app, client, org_team, login):
"""The regression a straight swap would have introduced."""
from app.models import OrgTeam
team_id, coach_id = org_team
with app.app_context():
username = db.session.get(User, coach_id).username
login(username)
response = client.post(f'/teams/{team_id}/delete', follow_redirects=False)
assert response.status_code < 500
with app.app_context():
assert db.session.get(OrgTeam, team_id) is not None, (
'a coach deleted an organisation team, its notes and its match history'
)
def test_a_manager_still_can(self, app, client, org_team, as_role):
"""The premise. Without it the test above passes against a route
that refuses everyone."""
from app.models import OrgTeam
team_id, _coach_id = org_team
as_role('manager')
client.post(f'/teams/{team_id}/delete', follow_redirects=False)
with app.app_context():
assert db.session.get(OrgTeam, team_id) is None
def test_a_player_cannot(self, app, client, org_team, as_role):
from app.models import OrgTeam
team_id, _coach_id = org_team
as_role('player')
client.post(f'/teams/{team_id}/delete', follow_redirects=False)
with app.app_context():
assert db.session.get(OrgTeam, team_id) is not None
class TestTeamStaffAssignment:
"""SEC-16 — teams.py was the one route module wave G's validation pass
did not reach, and it showed in two ways.
`int(request.form.get('coach_id'))` raised on anything non-numeric, so a
hand-made POST was a 500. And the id it produced was looked up without
checking the account's role in two of the three places that used it:
`edit_team`'s sync_staff branch tested `isinstance(user, Coach)`, its
other branch did not, and `create_team` did not either. The same file
disagreeing with itself — and the same defect wave G fixed in tryouts.py.
"""
def test_a_non_numeric_id_is_refused_not_a_500(self, app, client, as_role):
from app.models import OrgTeam
as_role('manager')
response = client.post(
'/teams/create',
data={'name': 'Varsity', 'coach_id': 'not-a-number'},
follow_redirects=False,
)
assert response.status_code < 500, 'a forged field must not be a server error'
with app.app_context():
assert OrgTeam.query.filter_by(name='Varsity').first() is None
def test_a_player_cannot_be_named_coach_of_a_team(self, app, client, as_role, make_user):
"""The one that matters. The id comes from a <select> the browser
rendered, so it is a value the client chooses."""
from app.models import OrgTeam
player_id = make_user('player')
as_role('manager')
client.post(
'/teams/create',
data={'name': 'Varsity', 'coach_id': str(player_id)},
follow_redirects=False,
)
with app.app_context():
team = OrgTeam.query.filter_by(name='Varsity').first()
assert team is not None, 'the team should still be created, without the bad staff'
assert team.coach_id is None
assert team.coaches.all() == []
def test_a_real_coach_is_still_assigned(self, app, client, as_role, make_user):
"""The premise. Without it the test above passes against a route that
assigns nobody at all."""
from app.models import OrgTeam
coach_id = make_user('coach')
as_role('manager')
client.post(
'/teams/create',
data={'name': 'Varsity', 'coach_id': str(coach_id)},
follow_redirects=False,
)
with app.app_context():
team = OrgTeam.query.filter_by(name='Varsity').first()
assert team.coach_id == coach_id
assert [user.id for user in team.coaches.all()] == [coach_id]
def test_editing_cannot_slip_a_player_in_either(self, app, client, as_role, make_user):
"""The branch that had no role check at all."""
from app.models import OrgTeam
coach_id = make_user('coach')
player_id = make_user('player')
as_role('manager')
client.post('/teams/create', data={'name': 'Varsity', 'coach_id': str(coach_id)})
with app.app_context():
team_id = OrgTeam.query.filter_by(name='Varsity').first().id
client.post(
f'/teams/{team_id}/edit',
data={'name': 'Varsity', 'coach_id': str(player_id)},
follow_redirects=False,
)
with app.app_context():
team = db.session.get(OrgTeam, team_id)
assert player_id not in [user.id for user in team.coaches.all()]
assert team.coach_id != player_id
def test_a_team_still_needs_a_name(self, app, client, as_role):
from app.models import OrgTeam
as_role('manager')
response = client.post('/teams/create', data={'name': ''}, follow_redirects=False)
assert response.status_code < 500
with app.app_context():
assert OrgTeam.query.count() == 0