team-tryouts/tests/test_authorization.py

"""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


class TestTeamRosterAssignment:
    """SEC-16, the half wave K missed.

    Wave K put a schema on `create_team` and `edit_team` and left five
    sibling routes reading `int(request.form.get(...))` — `add_coach`,
    `add_manager`, `remove_coach`, `remove_manager` and `add_player`. Each
    was a 500 on a non-numeric id. `add_player` also accepted any `status`
    string into a NOT NULL column, and none of them checked whether the
    account had been deactivated.

    Fixing a pattern in one place and not its neighbours is the mistake this
    project keeps making. Here it was made by the fix for it.
    """

    @pytest.fixture
    def team(self, app, make_user):
        from app.models import OrgTeam

        admin_id = make_user('admin')
        with app.app_context():
            org_team = OrgTeam(name='Varsity', created_by=admin_id)
            db.session.add(org_team)
            db.session.commit()
            return org_team.id

    @pytest.mark.parametrize(
        ('path', 'field'),
        [
            ('add_coach', 'coach_id'),
            ('add_manager', 'manager_id'),
            ('remove_coach', 'coach_id'),
            ('remove_manager', 'manager_id'),
            ('add_player', 'player_id'),
        ],
    )
    def test_a_non_numeric_id_is_not_a_500(self, app, client, team, as_role, path, field):
        as_role('admin')

        response = client.post(f'/teams/{team}/{path}', data={field: 'not-a-number'})

        assert response.status_code < 500

    def test_a_real_coach_is_added(self, app, client, team, as_role, make_user):
        """The premise for the two tests below."""
        from app.models import OrgTeam

        coach_id = make_user('coach')
        as_role('admin')

        client.post(f'/teams/{team}/add_coach', data={'coach_id': str(coach_id)})

        with app.app_context():
            org_team = db.session.get(OrgTeam, team)
            assert [c.id for c in org_team.coaches.all()] == [coach_id]

    def test_a_player_cannot_be_added_as_coach(self, app, client, team, as_role, make_user):
        from app.models import OrgTeam

        player_id = make_user('player')
        as_role('admin')

        client.post(f'/teams/{team}/add_coach', data={'coach_id': str(player_id)})

        with app.app_context():
            assert db.session.get(OrgTeam, team).coaches.all() == []

    def test_a_deactivated_coach_cannot_be_added(self, app, client, team, as_role, make_user):
        """`is_active_account` is what says the person has left the club.
        Putting them back on a roster contradicts it."""
        from app.models import OrgTeam, User

        coach_id = make_user('coach')
        with app.app_context():
            db.session.get(User, coach_id).is_active_account = False
            db.session.commit()
        as_role('admin')

        client.post(f'/teams/{team}/add_coach', data={'coach_id': str(coach_id)})

        with app.app_context():
            assert db.session.get(OrgTeam, team).coaches.all() == []

    def test_a_deactivated_player_cannot_be_added(self, app, client, team, as_role, make_user):
        from app.models import TeamPlayer, User

        player_id = make_user('player')
        with app.app_context():
            db.session.get(User, player_id).is_active_account = False
            db.session.commit()
        as_role('admin')

        client.post(f'/teams/{team}/add_player', data={'player_id': str(player_id)})

        with app.app_context():
            assert TeamPlayer.query.filter_by(org_team_id=team).count() == 0

    def test_a_deactivated_player_is_not_offered(self, app, client, as_role, make_user):
        """The select and the route agreed on nothing: the query had no
        is_active_account filter while the two beside it did."""
        from app.models import User

        player_id = make_user('player', username='ghostplayer')
        with app.app_context():
            db.session.get(User, player_id).is_active_account = False
            db.session.commit()
        as_role('admin')

        body = client.get('/teams').get_data(as_text=True)

        assert 'ghostplayer' not in body

    def test_an_unknown_roster_status_is_refused(self, app, client, team, as_role, make_user):
        """`status` went into a NOT NULL String(20) unchecked, and the toggle
        reads anything that is not 'starter' as substitute — so an unknown
        value silently promoted its holder on the next press."""
        from app.models import TeamPlayer

        player_id = make_user('player')
        as_role('admin')

        client.post(
            f'/teams/{team}/add_player',
            data={'player_id': str(player_id), 'status': 'captain-for-life'},
        )

        with app.app_context():
            rows = TeamPlayer.query.filter_by(org_team_id=team).all()
            assert [r.status for r in rows] != ['captain-for-life']

    def test_a_known_status_is_kept(self, app, client, team, as_role, make_user):
        from app.models import TeamPlayer

        player_id = make_user('player')
        as_role('admin')

        client.post(
            f'/teams/{team}/add_player',
            data={'player_id': str(player_id), 'status': 'substitute'},
        )

        with app.app_context():
            row = TeamPlayer.query.filter_by(org_team_id=team).one()
            assert row.status == 'substitute'