Aug 7 - Update: add external inspector

2026-08-07 16:08:10 -04:00
parent 97c1dec54d
commit 6ca30c0dea
28 changed files with 727 additions and 96 deletions
@@ -0,0 +1,303 @@
+"""
+tests/test_external_inspector.py
+--------------------------------
+Behaviour tests for MT-15 — the 'external_inspector' role.
+
+Runs on the in-memory SQLite app fixture (multi-tenancy inert). Covers:
+
+  * the role predicates: is_inspector covers both roles, is_external_inspector
+    does not, and role_label renders the display name
+  * get_inspector_scope() applies the SAME InspectorAssignment scoping to an
+    external inspector as to an internal one — the regression this phase exists
+    to prevent is an external inspector falling into the privileged branch
+    (scope None = unrestricted) and seeing every contract
+  * an unassigned external inspector sees nothing (strict mode)
+  * facility / inspection / issue list routes stay scoped for the new role
+  * the notification matrix exposes an External Inspector column whose defaults
+    mirror the Inspector column
+  * creating an external inspector sends an invitation instead of setting a
+    password: password_set is False and a set-password token is minted
+  * creating any other role still requires a password
+  * the assign-contracts page accepts an external inspector (it 404'd before)
+
+The regression guard that matters most is
+test_external_inspector_scope_is_not_unrestricted: if a future edit reverts a
+membership test back to `role == 'inspector'`, an external inspector silently
+gains org-wide visibility, which is a cross-customer data leak rather than a
+cosmetic bug.
+"""
+
+import pytest
+
+
+@pytest.fixture
+def client(app):
+    """Fresh schema + test client for each test (isolated in-memory DB)."""
+    with app.app_context():
+        from app import db
+        # get_inspector_scope() imports this model lazily, so the mapper is not
+        # registered at create_all() time and the table is missing when a
+        # logged-in inspector hits the dashboard. Import it up front.
+        from app.models import inspector_assignment  # noqa: F401
+        db.drop_all()
+        db.create_all()
+        yield app.test_client()
+        db.session.remove()
+
+
+def _user(username, role, **kw):
+    from app import db
+    from app.models.user import User
+    u = User(username=username, full_name=username.title(), role=role,
+             email=f'{username}@example.com', active=True, **kw)
+    u.set_password('pw-correct1')
+    db.session.add(u)
+    db.session.commit()
+    return u
+
+
+def _seed():
+    """Two contracts, one facility each, and the three inspector-ish accounts."""
+    from app import db
+    from app.models.facility import Facility
+    from app.models.inspection import InspectionTemplate
+    from app.models.project import Project
+
+    proj_a = Project(name='Contract A', active=True)
+    proj_b = Project(name='Contract B', active=True)
+    tmpl = InspectionTemplate(name='Restroom Check', active=True,
+                              form_schema=[{'id': 'f1', 'type': 'rating_5',
+                                            'label': 'Clean', 'row': 0, 'col': 0,
+                                            'rowSpan': 1, 'colSpan': 1}])
+    db.session.add_all([proj_a, proj_b, tmpl])
+    db.session.commit()
+
+    fac_a = Facility(name='Client A Site', active=True, project_id=proj_a.id)
+    fac_b = Facility(name='Client B Site', active=True, project_id=proj_b.id)
+    db.session.add_all([fac_a, fac_b])
+    db.session.commit()
+
+    internal = _user('ivy', 'inspector')
+    external = _user('xan', 'external_inspector')
+    admin    = _user('ada', 'admin')
+
+    return dict(tmpl=tmpl, proj_a=proj_a, proj_b=proj_b,
+                fac_a=fac_a, fac_b=fac_b,
+                internal=internal, external=external, admin=admin)
+
+
+def _assign(user, project):
+    from app import db
+    from app.models.inspector_assignment import InspectorAssignment
+    db.session.add(InspectorAssignment(user_id=user.id, project_id=project.id))
+    db.session.commit()
+
+
+def _login(client, user):
+    return client.post('/auth/login',
+                       data={'username': user.username, 'password': 'pw-correct1'},
+                       follow_redirects=True)
+
+
+# ── Role predicates ──────────────────────────────────────────────────────────
+
+def test_is_inspector_covers_both_inspector_roles(client):
+    env = _seed()
+    assert env['internal'].is_inspector is True
+    assert env['external'].is_inspector is True
+    assert env['admin'].is_inspector is False
+
+
+def test_is_external_inspector_distinguishes_the_two(client):
+    env = _seed()
+    assert env['external'].is_external_inspector is True
+    assert env['internal'].is_external_inspector is False
+
+
+def test_role_label_renders_display_name(client):
+    env = _seed()
+    assert env['external'].role_label == 'External Inspector'
+    assert env['internal'].role_label == 'Inspector'
+
+
+def test_inspector_roles_tuple_contains_both(client):
+    from app.models.user import User
+    assert set(User.INSPECTOR_ROLES) == {'inspector', 'external_inspector'}
+
+
+# ── Scoping — the security-critical behaviour ────────────────────────────────
+
+def test_external_inspector_scope_is_not_unrestricted(client):
+    """An external inspector must NEVER get scope None (= see everything).
+
+    This is the regression this phase exists to prevent. If a membership test
+    is ever reverted to `role == 'inspector'`, get_inspector_scope() returns
+    None for the external role and every downstream query drops its facility
+    filter — a cross-customer leak, not a cosmetic bug.
+    """
+    from app.utils.scope import get_inspector_scope
+
+    env = _seed()
+    _assign(env['external'], env['proj_a'])
+
+    scope = get_inspector_scope(env['external'])
+    assert scope is not None, 'external inspector fell into the unrestricted branch'
+    assert scope == [env['fac_a'].id]
+    assert env['fac_b'].id not in scope
+
+
+def test_external_and_internal_inspectors_scope_identically(client):
+    from app.utils.scope import get_inspector_scope
+
+    env = _seed()
+    _assign(env['internal'], env['proj_a'])
+    _assign(env['external'], env['proj_a'])
+
+    assert (get_inspector_scope(env['external'])
+            == get_inspector_scope(env['internal'])
+            == [env['fac_a'].id])
+
+
+def test_unassigned_external_inspector_sees_nothing(client):
+    """Strict mode: no assignments means an empty list, not unrestricted."""
+    from app.utils.scope import get_inspector_scope
+
+    env = _seed()
+    assert get_inspector_scope(env['external']) == []
+
+
+def test_non_inspector_roles_still_unrestricted(client):
+    from app.utils.scope import get_inspector_scope
+
+    env = _seed()
+    assert get_inspector_scope(env['admin']) is None
+
+
+# ── Route-level scoping ──────────────────────────────────────────────────────
+
+def test_facility_list_is_scoped_for_external_inspector(client):
+    env = _seed()
+    _assign(env['external'], env['proj_a'])
+    _login(client, env['external'])
+
+    resp = client.get('/facilities/')
+    assert resp.status_code == 200
+    body = resp.get_data(as_text=True)
+    assert 'Client A Site' in body
+    assert 'Client B Site' not in body
+
+
+def test_inspection_and_issue_lists_load_for_external_inspector(client):
+    """The new role must not 403 or 500 on the core scoped list routes."""
+    env = _seed()
+    _assign(env['external'], env['proj_a'])
+    _login(client, env['external'])
+
+    for path in ('/inspections/', '/issues/'):
+        resp = client.get(path)
+        assert resp.status_code == 200, f'{path} returned {resp.status_code}'
+
+
+# ── Notification matrix ──────────────────────────────────────────────────────
+
+def test_matrix_exposes_external_inspector_column(client):
+    from app.models.notification_matrix import MATRIX_ROLES
+    keys = [k for k, _ in MATRIX_ROLES]
+    assert 'external_inspector' in keys
+
+
+def test_matrix_defaults_mirror_the_inspector_column(client):
+    from app.models.notification_matrix import MATRIX_DEFAULTS, MATRIX_EVENTS
+
+    mirrored = 0
+    for event in MATRIX_EVENTS:
+        if (event, 'inspector') in MATRIX_DEFAULTS:
+            assert (event, 'external_inspector') in MATRIX_DEFAULTS, event
+            assert (MATRIX_DEFAULTS[(event, 'external_inspector')]
+                    == MATRIX_DEFAULTS[(event, 'inspector')]), event
+            mirrored += 1
+    assert mirrored > 0, 'no inspector defaults found to mirror'
+
+
+# ── Invitation flow ──────────────────────────────────────────────────────────
+
+def test_creating_external_inspector_invites_instead_of_setting_password(client):
+    from app.models.user import User
+
+    env = _seed()
+    _login(client, env['admin'])
+
+    resp = client.post('/auth/users/new', data={
+        'username': 'newxan',
+        'full_name': 'New Xan',
+        'email': 'newxan@example.com',
+        'role': 'external_inspector',
+        'password': '',
+        'confirm_password': '',
+    }, follow_redirects=True)
+    assert resp.status_code == 200
+
+    created = User.query.filter_by(username='newxan').first()
+    assert created is not None, 'external inspector was not created'
+    assert created.role == 'external_inspector'
+    # Invited, not password-set: login is blocked until they use the link.
+    assert created.password_set is False
+    assert created.set_password_token is not None
+    assert created.set_password_token_expires is not None
+
+
+def test_invited_external_inspector_cannot_log_in_until_setup(client):
+    from app.models.user import User
+
+    env = _seed()
+    _login(client, env['admin'])
+    client.post('/auth/users/new', data={
+        'username': 'newxan',
+        'full_name': 'New Xan',
+        'email': 'newxan@example.com',
+        'role': 'external_inspector',
+        'password': '',
+        'confirm_password': '',
+    }, follow_redirects=True)
+    client.get('/auth/logout', follow_redirects=True)
+
+    created = User.query.filter_by(username='newxan').first()
+    # The placeholder hash is random, so no password can work; assert the
+    # account is in the blocked state rather than guessing a credential.
+    assert created.password_set is False
+
+
+def test_creating_a_normal_role_still_requires_a_password(client):
+    from app.models.user import User
+
+    env = _seed()
+    _login(client, env['admin'])
+
+    resp = client.post('/auth/users/new', data={
+        'username': 'nopw',
+        'full_name': 'No Password',
+        'email': 'nopw@example.com',
+        'role': 'inspector',
+        'password': '',
+        'confirm_password': '',
+    }, follow_redirects=True)
+    assert resp.status_code == 200
+    assert User.query.filter_by(username='nopw').first() is None
+
+
+# ── Assign-contracts page ────────────────────────────────────────────────────
+
+def test_assign_contracts_page_accepts_external_inspector(client):
+    env = _seed()
+    _login(client, env['admin'])
+
+    resp = client.get(f"/auth/users/{env['external'].id}/assign-contracts")
+    assert resp.status_code == 200
+
+
+def test_assign_contracts_page_still_rejects_non_inspectors(client):
+    env = _seed()
+    _login(client, env['admin'])
+
+    resp = client.get(f"/auth/users/{env['admin'].id}/assign-contracts")
+    assert resp.status_code == 404