fix: enforce student and doctype permissions in education API

[tuwafula]

• Add ownership checks for student-scoped endpoints (programs, invoices, attendance), guard student group and assessment result access by doctype permission, and remove whitelist decorators from internal helpers (get_student_guardians, get_result).

[greptile-apps]

Confidence Score: 3/5

Not safe to merge as-is — the new check_permission guard can silently deny all non-Administrator staff and will break entirely for students without a linked portal user.

The ownership-only gate in check_permission closes a real gap but overshoots: it denies teachers, system managers, and any user who is not literally Administrator from accessing student programs, invoices, and attendance. When Student.user is None, every call from any non-Administrator user is rejected. The Assessment Result Tool permission check is also uncertain.

education/education/api.py — specifically the new check_permission function and the get_assessment_students permission guard.

Important Files Changed

Filename	Overview
education/education/api.py	Adds ownership and doctype permission guards to student-scoped endpoints and removes @frappe.whitelist() from internal helpers; the new check_permission helper has a logic bug that blocks legitimate staff access and breaks entirely when Student.user is unset.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Whitelisted API Call] --> B{Endpoint type?}
    B -->|get_student_programs\nget_student_invoices\nget_student_attendance| C[check_permission\nstudent, resource_type]
    B -->|get_student_group_students| D[frappe.has_permission\nStudent Group, read, doc]
    B -->|get_assessment_students| E[frappe.has_permission\nAssessment Result Tool, read]
    C --> F{user == Administrator?}
    F -->|Yes| G[Allow]
    F -->|No| H[get_value Student.user]
    H --> I{student_user == user?}
    I -->|Yes| G
    I -->|No or None| J[PermissionError]
    D -->|False| K[PermissionError]
    D -->|True| L[Return students]
    E -->|False| M[PermissionError]
    E -->|True| N[call get_student_group_students]
    N --> O[Return assessment list]
    style J fill:#f88,stroke:#c00
    style K fill:#f88,stroke:#c00
    style M fill:#f88,stroke:#c00

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[Whitelisted API Call] --> B{Endpoint type?}
    B -->|get_student_programs\nget_student_invoices\nget_student_attendance| C[check_permission\nstudent, resource_type]
    B -->|get_student_group_students| D[frappe.has_permission\nStudent Group, read, doc]
    B -->|get_assessment_students| E[frappe.has_permission\nAssessment Result Tool, read]
    C --> F{user == Administrator?}
    F -->|Yes| G[Allow]
    F -->|No| H[get_value Student.user]
    H --> I{student_user == user?}
    I -->|Yes| G
    I -->|No or None| J[PermissionError]
    D -->|False| K[PermissionError]
    D -->|True| L[Return students]
    E -->|False| M[PermissionError]
    E -->|True| N[call get_student_group_students]
    N --> O[Return assessment list]
    style J fill:#f88,stroke:#c00
    style K fill:#f88,stroke:#c00
    style M fill:#f88,stroke:#c00

Loading

Prompt To Fix All With AI

Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
education/education/api.py:800-809
**Ownership-only guard blocks all non-Administrator staff and fails silently when `Student.user` is unset**

`check_permission` only exempts the literal `"Administrator"` username; any other privileged user (System Manager, Instructor, Education Manager) calling `get_student_programs`, `get_student_invoices`, or `get_student_attendance` on behalf of a student will get a `PermissionError`. More critically, `frappe.db.get_value("Student", student, "user")` returns `None` if the Student record has no linked portal user — `None != user` is always `True`, so every non-Administrator call for such students is denied, silently locking out legitimate access. A role-based fallback (e.g., `frappe.has_permission("Student", "read", student)`) should be checked before rejecting the request.

### Issue 2 of 3
education/education/api.py:317-318
**`Assessment Result Tool` may not have a standard doctype permission entry**

`frappe.has_permission("Assessment Result Tool", "read")` relies on this DocType having a permissions table configured in the system. `Assessment Result Tool` is a page/tool form in Frappe Education — if it has no `DocType` record with role permissions defined, `frappe.has_permission` will either raise or return `False` for everyone, effectively blocking all access to `get_assessment_students`. Consider checking against a data doctype that is always configured (e.g., `"Assessment Result"`) or explicitly guarding with `frappe.db.exists("DocType", "Assessment Result Tool")` before relying on this check.

### Issue 3 of 3
education/education/api.py:807-808
**User-visible error strings are not wrapped with `_()`**

The f-string `f"You are not authorized to access this student's {resource_type}"` cannot be wrapped with `_()` as-is and will not be translated. The same applies to the bare string literals in `get_student_group_students` (line 195) and `get_assessment_students` (line 318). Use `frappe.throw(_("..."), frappe.PermissionError)` with a static translatable string, or at minimum use `_("Not Authorized")` for the simple cases.

_{Reviews (1): Last reviewed commit: "fix: enforce student and doctype permiss..." | Re-trigger Greptile}

[greptile-apps]

Ownership-only guard blocks all non-Administrator staff and fails silently when Student.user is unset

check_permission only exempts the literal "Administrator" username; any other privileged user (System Manager, Instructor, Education Manager) calling get_student_programs, get_student_invoices, or get_student_attendance on behalf of a student will get a PermissionError. More critically, frappe.db.get_value("Student", student, "user") returns None if the Student record has no linked portal user — None != user is always True, so every non-Administrator call for such students is denied, silently locking out legitimate access. A role-based fallback (e.g., frappe.has_permission("Student", "read", student)) should be checked before rejecting the request.

Context Used: This is a Frappe Framework application (Python bac... (source)

Prompt To Fix With AI

This is a comment left during a code review.
Path: education/education/api.py
Line: 800-809

Comment:
**Ownership-only guard blocks all non-Administrator staff and fails silently when `Student.user` is unset**

`check_permission` only exempts the literal `"Administrator"` username; any other privileged user (System Manager, Instructor, Education Manager) calling `get_student_programs`, `get_student_invoices`, or `get_student_attendance` on behalf of a student will get a `PermissionError`. More critically, `frappe.db.get_value("Student", student, "user")` returns `None` if the Student record has no linked portal user — `None != user` is always `True`, so every non-Administrator call for such students is denied, silently locking out legitimate access. A role-based fallback (e.g., `frappe.has_permission("Student", "read", student)`) should be checked before rejecting the request.

**Context Used:** This is a Frappe Framework application (Python bac... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

Assessment Result Tool may not have a standard doctype permission entry

frappe.has_permission("Assessment Result Tool", "read") relies on this DocType having a permissions table configured in the system. Assessment Result Tool is a page/tool form in Frappe Education — if it has no DocType record with role permissions defined, frappe.has_permission will either raise or return False for everyone, effectively blocking all access to get_assessment_students. Consider checking against a data doctype that is always configured (e.g., "Assessment Result") or explicitly guarding with frappe.db.exists("DocType", "Assessment Result Tool") before relying on this check.

Context Used: This is a Frappe Framework application (Python bac... (source)

Prompt To Fix With AI

This is a comment left during a code review.
Path: education/education/api.py
Line: 317-318

Comment:
**`Assessment Result Tool` may not have a standard doctype permission entry**

`frappe.has_permission("Assessment Result Tool", "read")` relies on this DocType having a permissions table configured in the system. `Assessment Result Tool` is a page/tool form in Frappe Education — if it has no `DocType` record with role permissions defined, `frappe.has_permission` will either raise or return `False` for everyone, effectively blocking all access to `get_assessment_students`. Consider checking against a data doctype that is always configured (e.g., `"Assessment Result"`) or explicitly guarding with `frappe.db.exists("DocType", "Assessment Result Tool")` before relying on this check.

**Context Used:** This is a Frappe Framework application (Python bac... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

User-visible error strings are not wrapped with _()

The f-string f"You are not authorized to access this student's {resource_type}" cannot be wrapped with _() as-is and will not be translated. The same applies to the bare string literals in get_student_group_students (line 195) and get_assessment_students (line 318). Use frappe.throw(_("..."), frappe.PermissionError) with a static translatable string, or at minimum use _("Not Authorized") for the simple cases.

Context Used: This is a Frappe Framework application (Python bac... (source)

Prompt To Fix With AI

This is a comment left during a code review.
Path: education/education/api.py
Line: 807-808

Comment:
**User-visible error strings are not wrapped with `_()`**

The f-string `f"You are not authorized to access this student's {resource_type}"` cannot be wrapped with `_()` as-is and will not be translated. The same applies to the bare string literals in `get_student_group_students` (line 195) and `get_assessment_students` (line 318). Use `frappe.throw(_("..."), frappe.PermissionError)` with a static translatable string, or at minimum use `_("Not Authorized")` for the simple cases.

**Context Used:** This is a Frappe Framework application (Python bac... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!