Aug 25 - Fixed inspection access denied by inspector
This commit is contained in:
@@ -906,6 +906,10 @@ Without the `is_(None)` on the second arm the original inspector keeps seeing it
|
||||
|
||||
**The generic "inspectors see only their own inspections" filter has to be deferred** when `follow_up_required=true` is requested — an assigned follow-up lives on an inspection somebody *else* performed, so applying authorship first hides exactly the rows the assignee needs.
|
||||
|
||||
**Read access follows the LIST, not authorship (Aug 2026).** `index()` scopes an inspector by FACILITY (rule 58 — their scope covers all data in their contracted facilities), but `view()` and `export_pdf()` scoped by authorship, so the list offered rows that answered "Access denied" on click. It also blocked the assignee from opening the parent inspection they had just been asked to re-inspect — the button they needed was on a page they could not reach. Both reads now use `_inspector_may_read()` (facility scope). **Writes stay owner-only**: `execute`, `save_draft_ajax`, `upload_photo_ajax` and `flag_issue` keep the authorship check.
|
||||
|
||||
**A live follow-up has exactly one owner.** `reinspect()` lets the follow-up's owner start it; when a follow-up is assigned, even the original inspector is refused ("assigned to someone else") — that is the point of assigning it. With no follow-up outstanding, an inspector may re-inspect their own work and nobody else's. The Re-inspect buttons render only for `is_own_inspection or owns_follow_up`, so the page never shows a control that fails on click.
|
||||
|
||||
**The picker is contract-scoped** (`_followup_assignees_for()`), for the same reason the flag-issue list is (rule 93): a Customer Director must never see, or assign work to, another client's inspector. Only the two INSPECTOR roles are offered — directors/PMs/auditors hold no `InspectorAssignment`, so they could not open the re-inspection anyway. The POST re-validates against that list, and a facility with no contract offers nobody (fail-closed, follow-up stays with the original inspector). `clear_followup` (single and bulk) clears the assignment too.
|
||||
|
||||
### Per-Account Overrides for Customer Roles (Phase 51)
|
||||
|
||||
@@ -772,7 +772,11 @@ def view(inspection_id):
|
||||
if inspection is None:
|
||||
abort(404)
|
||||
|
||||
if current_user.is_inspector and inspection.inspector_id != current_user.id:
|
||||
# Read access matches the LIST (rule 58) — an inspector may open anything
|
||||
# at their contracted facilities, not only what they performed. Editing
|
||||
# someone else's inspection is still refused (execute / save-draft /
|
||||
# upload-photo / flag-issue keep the authorship check).
|
||||
if current_user.is_inspector and not _inspector_may_read(inspection, current_user):
|
||||
flash('Access denied.', 'danger')
|
||||
return redirect(url_for('inspections.index'))
|
||||
if current_user.role == 'customer':
|
||||
@@ -970,8 +974,22 @@ def view(inspection_id):
|
||||
}
|
||||
|
||||
followup_assignees = _followup_assignees_for(inspection, current_user)
|
||||
# An inspector viewing SOMEBODY ELSE's inspection gets a read-only page.
|
||||
# Without this the buttons would all render and then fail on click — the
|
||||
# same list-says-yes / page-says-no mismatch this change removes.
|
||||
is_own_inspection = (not current_user.is_inspector
|
||||
or inspection.inspector_id == current_user.id)
|
||||
# Whoever is expected to carry out the follow-up (phase53) may start the
|
||||
# re-inspection even though the original inspection is not theirs.
|
||||
owns_follow_up = bool(
|
||||
inspection.follow_up_required
|
||||
and inspection.follow_up_owner
|
||||
and inspection.follow_up_owner.id == current_user.id
|
||||
)
|
||||
return render_template('inspections/view.html',
|
||||
followup_assignees=followup_assignees,
|
||||
is_own_inspection=is_own_inspection,
|
||||
owns_follow_up=owns_follow_up,
|
||||
inspection=inspection,
|
||||
form_fields=form_fields,
|
||||
form_data=form_data,
|
||||
@@ -1311,7 +1329,11 @@ def export_pdf(inspection_id):
|
||||
if inspection is None:
|
||||
abort(404)
|
||||
|
||||
if current_user.is_inspector and inspection.inspector_id != current_user.id:
|
||||
# Read access matches the LIST (rule 58) — an inspector may open anything
|
||||
# at their contracted facilities, not only what they performed. Editing
|
||||
# someone else's inspection is still refused (execute / save-draft /
|
||||
# upload-photo / flag-issue keep the authorship check).
|
||||
if current_user.is_inspector and not _inspector_may_read(inspection, current_user):
|
||||
flash('Access denied.', 'danger')
|
||||
return redirect(url_for('inspections.index'))
|
||||
if current_user.role == 'customer':
|
||||
@@ -1396,6 +1418,28 @@ def _view_url(inspection_id):
|
||||
return url_for('inspections.view', inspection_id=inspection_id)
|
||||
|
||||
|
||||
def _inspector_may_read(inspection, user):
|
||||
"""May this inspector OPEN someone else's inspection?
|
||||
|
||||
Yes, when it happened at a facility on one of their contracts — the same
|
||||
scope `index()` uses (rule 58: an inspector's scope covers all data in their
|
||||
contracted facilities, not just their own work).
|
||||
|
||||
This used to test authorship instead, and the two disagreed: the list
|
||||
showed every inspection at the inspector's facilities, then clicking one
|
||||
said "Access denied". It also blocked the phase53 follow-up assignee from
|
||||
opening the parent inspection they had just been asked to re-inspect —
|
||||
the button they needed was on a page they could not reach.
|
||||
|
||||
READ only. Editing someone else's inspection is still refused: execute,
|
||||
save-draft, upload-photo and flag-issue all keep the authorship check.
|
||||
"""
|
||||
fids = get_inspector_scope(user)
|
||||
if fids is None: # not an inspector — no scoping applies here
|
||||
return True
|
||||
return bool(fids) and inspection.facility_id in fids
|
||||
|
||||
|
||||
def _followup_assignees_for(inspection, actor):
|
||||
"""Inspectors who may be handed this inspection's follow-up.
|
||||
|
||||
@@ -1826,6 +1870,26 @@ def reinspect(inspection_id):
|
||||
flash('Access denied.', 'danger')
|
||||
return redirect(url_for('inspections.index'))
|
||||
|
||||
# An inspector may re-inspect their OWN work, or work they have been
|
||||
# handed the follow-up for (phase53). Anything else at a contracted
|
||||
# facility is readable but not theirs to redo — starting a re-inspection
|
||||
# of a colleague's inspection uninvited only creates confusion about who
|
||||
# is doing it.
|
||||
if current_user.is_inspector:
|
||||
if parent.follow_up_required and parent.follow_up_owner:
|
||||
# A live follow-up has exactly ONE owner (phase53). Even the
|
||||
# original inspector does not start it once it has been handed to
|
||||
# someone else — that is the whole point of assigning it, and two
|
||||
# people turning up is the failure being designed out.
|
||||
may = parent.follow_up_owner.id == current_user.id
|
||||
else:
|
||||
# No follow-up outstanding: re-inspecting your own work is fine,
|
||||
# someone else's is not yours to redo uninvited.
|
||||
may = parent.inspector_id == current_user.id
|
||||
if not may:
|
||||
flash('That re-inspection has been assigned to someone else.', 'warning')
|
||||
return redirect(_view_url(inspection_id))
|
||||
|
||||
session['reinspect_parent_id'] = parent.id
|
||||
session['reinspect_template_id'] = parent.template_id
|
||||
session['reinspect_facility_id'] = parent.facility_id
|
||||
|
||||
@@ -353,7 +353,12 @@
|
||||
<button onclick="window.print()" class="btn btn-sm btn-outline-secondary">
|
||||
<i class="bi bi-printer"></i> Print
|
||||
</button>
|
||||
{% if current_user.role not in ['customer'] %}
|
||||
{# Offered to managers, to the inspector who did this inspection, and to
|
||||
whoever the follow-up was assigned to (phase53). Not to any other
|
||||
inspector who can merely SEE it: reinspect() refuses them, and showing
|
||||
a button that fails on click is the mismatch this page just fixed. #}
|
||||
{% if current_user.role not in ['customer']
|
||||
and (is_own_inspection or owns_follow_up) %}
|
||||
<a href="{{ url_for('inspections.reinspect', inspection_id=inspection.id) }}"
|
||||
class="btn btn-sm btn-outline-primary"
|
||||
title="Start a follow-up re-inspection with the same template and facility">
|
||||
@@ -427,8 +432,10 @@
|
||||
</div>
|
||||
{% endif %}
|
||||
{% if inspection.follow_up_note %}<br><span class="small">{{ inspection.follow_up_note }}</span>{% endif %}
|
||||
{# Re-inspection is staff work — reinspect() already refuses customers. #}
|
||||
{% if current_user.role != 'customer' %}
|
||||
{# Re-inspection is staff work — reinspect() already refuses customers —
|
||||
and among inspectors it belongs to the follow-up's OWNER. #}
|
||||
{% if current_user.role != 'customer'
|
||||
and (is_own_inspection or owns_follow_up) %}
|
||||
<div class="mt-2">
|
||||
<a href="{{ url_for('inspections.reinspect', inspection_id=inspection.id) }}"
|
||||
class="btn btn-sm btn-warning">
|
||||
|
||||
Reference in New Issue
Block a user