Fix/file attachment report auth

[catinhere]

No description provided.

[greptile-apps]

Greptile Summary

This PR fixes two authorization gaps: report file downloads were previously unprotected (noted as a TODO), and attachment authorization was refactored from a broken "require-on-every-linked-object" loop to a proper Bouncer rule. It also fixes a bug in Comparison creation where comparison_criterion objects were never explicitly added to the session.

• Report auth: name_generator now embeds course.uuid in every filename; file_retrieve extracts the UUID via regex and checks that the requesting user is an instructor or TA in that course (admins bypass the check).
• Attachment auth: The per-object require loop is replaced by a new if_can_read_attachment Bouncer predicate that grants access if the user can read any linked assignment/answer, or uploaded the file themselves.
• Comparison bug fix: Inside the ComparisonCriterion creation loop, db.session.add(comparison) (a no-op re-add) is corrected to db.session.add(comparison_criterion).

Confidence Score: 5/5

The PR closes two real authorization gaps and contains a targeted bug fix; all changed code paths are straightforward and well-tested.

The attachment auth refactor moves to a correct Bouncer predicate, the report auth embeds an unforgeable course UUID in the filename and validates role membership server-side, and the comparison-criterion fix corrects an inadvertent no-op add. Tests cover the key allow/deny scenarios for both file types. No incorrect data handling or broken auth paths were found.

No files require special attention.

Important Files Changed

Filename	Overview
compair/api/init.py	Replaces broken per-object require loop with a single bouncer require on the File, and adds report authorization by extracting and validating course UUID embedded in the filename.
compair/authorization.py	Adds the if_can_read_attachment Bouncer predicate granting File READ to users who can read any linked assignment/answer, or uploaded the file; also refactors the require helper to a one-liner.
compair/api/report.py	Appends course.uuid to the generated report filename so the authorization layer can extract and verify it; all UUID characters survive the valid_chars filter.
compair/models/comparison.py	Bug fix: corrects db.session.add(comparison) to db.session.add(comparison_criterion) inside the criterion loop.
compair/tests/api/test_file.py	Adds tests for attachment authorization and report authorization covering student 403, cross-course instructor 403, TA 200, instructor 200, and admin 200 cases.

Sequence Diagram

sequenceDiagram
    participant Client
    participant FileRetrieve as file_retrieve()
    participant Bouncer as Bouncer (authorization.py)
    participant DB as Database

    Client->>FileRetrieve: "GET /app/attachment/<file_name>"
    FileRetrieve->>DB: File.get_by_file_name_or_404(file_name)
    DB-->>FileRetrieve: attachment (File)
    FileRetrieve->>Bouncer: require(READ, attachment)
    Bouncer->>DB: file.assignments.all() / file.answers.all()
    DB-->>Bouncer: "linked assignments & answers"
    Bouncer-->>FileRetrieve: allowed / Forbidden
    FileRetrieve-->>Client: file stream or 403

    Client->>FileRetrieve: "GET /app/report/<file_name>"
    FileRetrieve->>FileRetrieve: "extract UUID via regex(--{22chars}.ext$)"
    FileRetrieve->>DB: "Course.query.filter_by(uuid=course_uuid)"
    DB-->>FileRetrieve: course (or None)
    FileRetrieve->>FileRetrieve: check user.user_courses for instructor/TA role
    FileRetrieve-->>Client: file stream or 403

Loading

_{Reviews (4): Last reviewed commit: "fix report access to verify enrollment i..." | Re-trigger Greptile}

[catinhere]

@greptileai

[catinhere]

@greptileai

[catinhere]

@greptileai