Aug 27 - Update free-plan gate, MFA is no longer bypassable via the app
This commit is contained in:
+16
-1
@@ -416,11 +416,26 @@ ST's bare `Length(min=6)`. Porting ST's version would be a downgrade.
|
||||
|
||||
### 12.2 Phase log
|
||||
|
||||
Migrations `phase41` → `phase52` in `migrations/versions/` are the parity track:
|
||||
Migrations `phase41` → `phase56` in `migrations/versions/` are the parity track:
|
||||
auditor role, area QR tokens, schedule plan fields, internal handler, frequency
|
||||
ENUM widening, recurrence, end date, parent inspection, follow-up attribution,
|
||||
schedule acknowledgement, and then:
|
||||
|
||||
**Aug 2026 tail — ST parity, ported after MT-17.** ST's chain numbers these
|
||||
differently (its phase51/52/53); MT's was already past those numbers, so match
|
||||
by NAME:
|
||||
|
||||
| MT | ST | What |
|
||||
|---|---|---|
|
||||
| `phase54_user_notif_matrix` | phase51 | Per-account notification overrides for the two customer-side roles. Empty table = everyone inherits the global matrix, so it shipped changing nothing. See CLAUDE.md §27b. |
|
||||
| `phase55_template_contracts` | phase52 | Forms restricted to specific contracts. **No rows = shared**, which is why it needed no backfill. |
|
||||
| `phase56_followup_assignee` | phase53 | `inspections.follow_up_assigned_to` — hand a re-inspection to another inspector. NULL = the inspection's own inspector, as before. See CLAUDE.md §27c. |
|
||||
|
||||
Shipped alongside them, without migrations: customer-role management under
|
||||
`/customers`, bulk actions on the issues/inspections lists, list-filter
|
||||
preservation, contract-scoped flag-issue assignees, Customer Directors planning
|
||||
their own inspections, and the support surface serving both customer roles.
|
||||
|
||||
**MT-15 — External Inspector role. ✅ DONE** (`phase51_external_inspector`)
|
||||
Adds `external_inspector` to the `users.role` ENUM: an inspector employed by the
|
||||
customer or a third party, with identical capabilities to `inspector` and scoped
|
||||
|
||||
@@ -32,6 +32,7 @@ import logging
|
||||
|
||||
from flask import Blueprint, request, g
|
||||
from app import db, limiter
|
||||
from app.tenancy.gates import feature_required
|
||||
from app.models.user import User
|
||||
from app.models.api_token import RefreshToken, DeviceToken
|
||||
from app.api.errors import api_ok, api_error
|
||||
@@ -61,6 +62,13 @@ def _user_payload(user: User) -> dict:
|
||||
|
||||
@bp.route('/auth/login', methods=['POST'])
|
||||
@limiter.limit('10 per minute; 3 per second')
|
||||
# The plan gate belongs HERE, not only on the write endpoints. It used to sit
|
||||
# on POST /inspections and POST /issues alone, so a tenant without mobile API
|
||||
# access could sign in, sync reference data and let an inspector complete a
|
||||
# whole inspection on site — and only then get a 403, with the work already
|
||||
# done and no way to submit it. Refusing at the door is the honest answer.
|
||||
# Inert in single-tenant mode and for any plan that allows the mobile API.
|
||||
@feature_required('mobile_api')
|
||||
def login():
|
||||
"""
|
||||
Authenticate with username + password.
|
||||
@@ -70,10 +78,22 @@ def login():
|
||||
{
|
||||
"username": "john",
|
||||
"password": "secret",
|
||||
"mfa_code": "123456", // required IF the account has 2FA on
|
||||
"device_id": "A1B2C3D4...", // UIDevice.identifierForVendor (optional)
|
||||
"device_name": "John's iPhone" // (optional)
|
||||
}
|
||||
|
||||
Response 401 — second factor needed
|
||||
-----------------------------------
|
||||
{
|
||||
"ok": false,
|
||||
"error": "A verification code is required for this account.",
|
||||
"mfa_required": true
|
||||
}
|
||||
|
||||
The password was correct; the client should prompt for the 6-digit code
|
||||
(or a recovery code) and POST again with `mfa_code`.
|
||||
|
||||
Response 200
|
||||
------------
|
||||
{
|
||||
@@ -106,6 +126,41 @@ def login():
|
||||
if not user.active:
|
||||
return api_error('Account is disabled. Please contact an administrator.', 401)
|
||||
|
||||
# ── Two-factor (phase35 parity) ───────────────────────────────────────
|
||||
# The web login defers identity to /auth/mfa when an account has TOTP
|
||||
# enabled. This endpoint did not, so anyone who turned MFA on could skip it
|
||||
# entirely by signing in through the app — the factor was decorative for
|
||||
# exactly the accounts that chose to enable it.
|
||||
#
|
||||
# Accepts either a TOTP code or a single-use recovery code, the same two
|
||||
# the web challenge accepts. A missing code is answered with
|
||||
# `mfa_required: true` so a client can prompt for it rather than treating
|
||||
# this as a wrong password.
|
||||
if user.mfa_enabled and user.mfa_secret:
|
||||
from app.utils.mfa import verify_totp, check_and_consume_recovery
|
||||
|
||||
code = (data.get('mfa_code') or '').strip()
|
||||
if not code:
|
||||
logger.info('API login | mfa_required | username=%s', user.username)
|
||||
return api_error('A verification code is required for this account.',
|
||||
401, extra={'mfa_required': True})
|
||||
|
||||
if not verify_totp(user.mfa_secret, code):
|
||||
matched, remaining = check_and_consume_recovery(
|
||||
user.mfa_recovery_codes, code)
|
||||
if not matched:
|
||||
logger.warning('API login | mfa_failed | username=%s | ip=%s',
|
||||
user.username, request.remote_addr)
|
||||
return api_error('That verification code is not valid.',
|
||||
401, extra={'mfa_required': True})
|
||||
# Recovery codes are single-use — persist the shortened list before
|
||||
# any token is issued, so a crash cannot hand out a login while
|
||||
# leaving the code usable again.
|
||||
user.mfa_recovery_codes = remaining
|
||||
db.session.commit()
|
||||
logger.warning('API login | recovery_code_used | username=%s | '
|
||||
'remaining=%d', user.username, len(remaining))
|
||||
|
||||
device_id = (data.get('device_id') or '')[:64] or None
|
||||
device_name = (data.get('device_name') or '')[:100] or None
|
||||
|
||||
@@ -154,6 +209,11 @@ def login():
|
||||
|
||||
@bp.route('/auth/refresh', methods=['POST'])
|
||||
@limiter.limit('30 per minute; 5 per second')
|
||||
# Gated too: without it a device that signed in before the plan changed would
|
||||
# keep rotating tokens forever and never notice it had lost access.
|
||||
# logout stays open on purpose — a blocked device must still be able to
|
||||
# surrender its refresh token and clean up.
|
||||
@feature_required('mobile_api')
|
||||
def refresh():
|
||||
"""
|
||||
Exchange a valid refresh token for a new access token.
|
||||
|
||||
+15
-4
@@ -31,13 +31,24 @@ def api_ok(data=None, status=200):
|
||||
}), status
|
||||
|
||||
|
||||
def api_error(message: str, status: int = 400):
|
||||
"""Return an error JSON response."""
|
||||
return jsonify({
|
||||
def api_error(message: str, status: int = 400, extra: dict | None = None):
|
||||
"""Return an error JSON response.
|
||||
|
||||
`extra` merges additional top-level keys into the envelope — for flags a
|
||||
client must branch on rather than parse out of the message, e.g.
|
||||
`mfa_required` on a login that needs a second factor. Reserved keys
|
||||
(ok/data/error) always win, so a caller cannot accidentally rewrite the
|
||||
envelope's shape.
|
||||
"""
|
||||
payload = {
|
||||
'ok': False,
|
||||
'data': None,
|
||||
'error': message,
|
||||
}), status
|
||||
}
|
||||
if extra:
|
||||
for k, v in extra.items():
|
||||
payload.setdefault(k, v)
|
||||
return jsonify(payload), status
|
||||
|
||||
|
||||
# ── Registered error handlers (attached to the api blueprint) ─────────────────
|
||||
|
||||
@@ -1388,6 +1388,20 @@ def export_pdf(issue_id):
|
||||
|
||||
|
||||
# ── Vendor work order dispatch (phase36) ─────────────────────────────────────
|
||||
#
|
||||
# NOT LINKED FROM THE UI (Aug 2026). The "Contractor Work Orders" card was
|
||||
# removed from the issue detail page, so nothing posts here any more. The
|
||||
# endpoint is kept deliberately rather than deleted:
|
||||
#
|
||||
# * it is the ONLY way to create a work order, so removing it would strand
|
||||
# the public contractor pages (/work-orders/<token>), the model, the email
|
||||
# template and the phase36 migration — a whole feature, not dead code;
|
||||
# * tests/test_work_orders.py drives the end-to-end flow through it.
|
||||
#
|
||||
# To bring the feature back, restore the card in templates/issues/view.html —
|
||||
# nothing here needs to change. To retire it for good, remove this route, the
|
||||
# work_orders blueprint, its templates, the model and those tests together,
|
||||
# and only once no tokenized links are still outstanding with contractors.
|
||||
|
||||
@bp.route('/<int:issue_id>/work-order', methods=['POST'])
|
||||
@login_required
|
||||
|
||||
@@ -119,9 +119,9 @@
|
||||
<label class="form-label small mb-1">Handled By</label>
|
||||
<select name="handler_type" class="form-select form-select-sm">
|
||||
<option value="">All Handlers</option>
|
||||
<option value="internal" {{ 'selected' if handler_filter == 'internal' }}>Janitorial Staff</option>
|
||||
<option value="facility" {{ 'selected' if handler_filter == 'facility' }}>Facility Staff</option>
|
||||
<option value="vendor" {{ 'selected' if handler_filter == 'vendor' }}>External Vendor</option>
|
||||
<option value="internal" {{ 'selected' if handler_type_filter == 'internal' }}>Janitorial Staff</option>
|
||||
<option value="facility" {{ 'selected' if handler_type_filter == 'facility' }}>Facility Staff</option>
|
||||
<option value="vendor" {{ 'selected' if handler_type_filter == 'vendor' }}>External Vendor</option>
|
||||
</select>
|
||||
</div>
|
||||
</div>
|
||||
@@ -314,7 +314,7 @@
|
||||
{% if p %}
|
||||
<li class="page-item {{ 'active' if p == issues.page }}">
|
||||
<a class="page-link"
|
||||
href="{{ url_for('issues.index', page=p, issue_id=issue_id_filter, severity=severity_filter, status=status_filter, sla=sla_filter, contract_id=contract_filter, facility_id=facility_filter, date_from=date_from_filter, date_to=date_to_filter, reporter_id=reporter_filter, handler_type=handler_filter, unassigned=unassigned_filter) }}">{{ p }}</a>
|
||||
href="{{ url_for('issues.index', page=p, issue_id=issue_id_filter, severity=severity_filter, status=status_filter, sla=sla_filter, contract_id=contract_filter, facility_id=facility_filter, date_from=date_from_filter, date_to=date_to_filter, reporter_id=reporter_filter, handler_type=handler_type_filter, unassigned=unassigned_filter) }}">{{ p }}</a>
|
||||
</li>
|
||||
{% else %}<li class="page-item disabled"><span class="page-link">…</span></li>{% endif %}
|
||||
{% endfor %}
|
||||
|
||||
@@ -27,7 +27,8 @@ g.tenant_engine — no teardown handler is needed here.
|
||||
|
||||
import logging
|
||||
|
||||
from flask import g, request, current_app, Response, session, redirect, url_for
|
||||
from flask import (g, request, current_app, Response, session, redirect,
|
||||
url_for, jsonify)
|
||||
|
||||
from app.tenancy.resolver import resolve_tenant
|
||||
from app.tenancy.engine_cache import get_tenant_engine
|
||||
@@ -52,6 +53,26 @@ _UNKNOWN_TENANT_PAGE = (
|
||||
)
|
||||
|
||||
|
||||
def _wants_json():
|
||||
"""True when this request is the mobile API (or explicitly asks for JSON).
|
||||
|
||||
Mirrors gates._is_api_request(). The tenancy and billing gates run BEFORE
|
||||
any route, so without this they answer an iPad with a 302 to an HTML page:
|
||||
URLSession follows it, the client decodes the login/billing markup as JSON
|
||||
and reports "the data couldn't be read". The inspector sees a parse error
|
||||
instead of "your subscription has expired", and nothing in the app can tell
|
||||
the two apart.
|
||||
"""
|
||||
return (request.path.startswith('/api/')
|
||||
or request.accept_mimetypes.best == 'application/json')
|
||||
|
||||
|
||||
def _json(payload, status):
|
||||
"""Small local responder — the API error helpers live in a blueprint that
|
||||
is not necessarily importable this early in the request."""
|
||||
return jsonify(payload), status
|
||||
|
||||
|
||||
def _is_exempt(path):
|
||||
if path.startswith('/static/'):
|
||||
return True
|
||||
@@ -158,6 +179,12 @@ def init_tenancy(app):
|
||||
host = (request.host or '').split(':')[0].strip().lower()
|
||||
tenant = resolve_tenant(host)
|
||||
if tenant is None:
|
||||
if _wants_json():
|
||||
# An HTML "Workspace not found" page is unreadable to the iPad
|
||||
# — it decodes as a parse failure, which looks like a bug in
|
||||
# the app rather than a wrong/retired server address.
|
||||
return _json({'ok': False,
|
||||
'error': 'Workspace not found for this address.'}, 404)
|
||||
return Response(_UNKNOWN_TENANT_PAGE, status=404, mimetype='text/html')
|
||||
|
||||
g.tenant = tenant
|
||||
@@ -205,6 +232,18 @@ def init_tenancy(app):
|
||||
# they can still see their plan page and the subscribe button.
|
||||
if not (request.path.startswith('/billing/')
|
||||
or request.path.startswith('/settings/')):
|
||||
if _wants_json():
|
||||
# 402, not a redirect: the caller is a program.
|
||||
# Distinct from 401 on purpose — the iPad retries a
|
||||
# 401 by refreshing its token, which would loop
|
||||
# forever against a billing block.
|
||||
return _json({
|
||||
'ok': False,
|
||||
'error': 'This workspace\'s trial has ended. '
|
||||
'An administrator needs to choose a plan '
|
||||
'before the app can sync again.',
|
||||
'billing_required': True,
|
||||
}, 402)
|
||||
return redirect(url_for('billing.subscribe'))
|
||||
else:
|
||||
days_left = (trial_ends_at - now).days
|
||||
@@ -217,4 +256,11 @@ def init_tenancy(app):
|
||||
return
|
||||
|
||||
# status == 'cancelled' — block and redirect to subscription page.
|
||||
if _wants_json():
|
||||
return _json({
|
||||
'ok': False,
|
||||
'error': 'This workspace is suspended. An administrator needs to '
|
||||
'reactivate the subscription before the app can sync again.',
|
||||
'billing_required': True,
|
||||
}, 402)
|
||||
return redirect(url_for('billing.suspended'))
|
||||
|
||||
@@ -49,6 +49,17 @@ MIGRATABLE_STATUSES = ('provisioning', 'active', 'suspended')
|
||||
# Squashed baseline that builds the full schema (migrations/versions root).
|
||||
BASELINE_REVISION = '0003_add_user_active'
|
||||
|
||||
#: The last revision the squashed baseline actually covers.
|
||||
#:
|
||||
#: Everything AFTER this point is existence-guarded (INFORMATION_SCHEMA checks,
|
||||
#: or an idempotent ENUM MODIFY), so it can be replayed over the baseline
|
||||
#: safely. Everything BEFORE it is not — several pre-phase33 migrations would
|
||||
#: collide with the full-schema baseline, which is why bootstrap stamps past
|
||||
#: them instead of running them.
|
||||
#:
|
||||
#: If the baseline is ever re-squashed to a later point, move this with it.
|
||||
BASELINE_COVERS_THROUGH = 'phase32_device_token_columns'
|
||||
|
||||
TenantRef = namedtuple('TenantRef', ['id', 'slug', 'db_uri'])
|
||||
|
||||
|
||||
@@ -130,14 +141,34 @@ def upgrade_tenant(tenant, script_location=None, version_locations=None, record_
|
||||
|
||||
def bootstrap_tenant(tenant, script_location=None, version_locations=None,
|
||||
baseline_rev=BASELINE_REVISION, record_job=True):
|
||||
"""Build a FRESH tenant database, then mark it current.
|
||||
"""Build a FRESH tenant database, then bring it to head.
|
||||
|
||||
Runs the squashed baseline (full schema) only, then `stamp head` — the
|
||||
historical phase migrations are NOT replayed (several are not idempotent and
|
||||
would conflict with the full-schema baseline). Use this for brand-new tenant
|
||||
databases; use upgrade_tenant() for ongoing incremental migrations.
|
||||
Three steps, and the middle one is the load-bearing part:
|
||||
|
||||
Returns the stamped head revision.
|
||||
1. `upgrade(baseline)` — the squashed baseline builds the bulk of the
|
||||
schema in one go.
|
||||
2. `stamp(BASELINE_COVERS_THROUGH)` — declare the DB to be at the last
|
||||
revision the baseline actually covers, WITHOUT running the pre-phase33
|
||||
migrations. Those are not idempotent and would collide with the
|
||||
baseline; skipping them is the whole reason bootstrap exists.
|
||||
3. `upgrade(head)` — replay the guarded tail (phase33 onward). Every one
|
||||
of those checks INFORMATION_SCHEMA before touching anything (the two
|
||||
ENUM widenings use an idempotent MODIFY), so this adds exactly what
|
||||
the baseline lacks and skips the rest.
|
||||
|
||||
This used to stop after step 1 and `stamp('head')` instead — claiming the
|
||||
database was current when it was missing every column and table added since
|
||||
phase32. Because the baseline was last refreshed around phase33, a tenant
|
||||
provisioned that way lacked `users.mfa_enabled` and `users.ui_theme`, and
|
||||
SQLAlchemy emits every mapped column in its SELECT — so the new workspace
|
||||
could not even log in. It failed at the first query, not at some optional
|
||||
feature, and only for freshly provisioned tenants (tenant-zero was adopted
|
||||
in place with a real schema), which is what kept it hidden.
|
||||
|
||||
Use upgrade_tenant() for ongoing incremental migrations of a tenant that
|
||||
already exists.
|
||||
|
||||
Returns the head revision the database ends up at.
|
||||
"""
|
||||
db_uri = tenant.db_uri
|
||||
cfg = _make_config(db_uri, script_location, version_locations)
|
||||
@@ -153,10 +184,20 @@ def bootstrap_tenant(tenant, script_location=None, version_locations=None,
|
||||
|
||||
try:
|
||||
with contextlib.redirect_stdout(io.StringIO()):
|
||||
command.upgrade(cfg, baseline_rev) # build full schema (baseline only)
|
||||
command.stamp(cfg, 'head') # mark at head without replaying phases
|
||||
command.upgrade(cfg, baseline_rev) # 1. baseline schema
|
||||
command.stamp(cfg, BASELINE_COVERS_THROUGH) # 2. skip the unguarded past
|
||||
command.upgrade(cfg, 'head') # 3. replay the guarded tail
|
||||
applied = current_revision(db_uri)
|
||||
|
||||
# A bootstrap that does not end at head has silently produced a broken
|
||||
# tenant — exactly the failure this sequence exists to prevent. Say so
|
||||
# here rather than letting it surface as a missing column later.
|
||||
expected = chain_head(script_location, version_locations)
|
||||
if applied != expected:
|
||||
raise RuntimeError(
|
||||
f'bootstrap ended at {applied!r}, expected head {expected!r} — '
|
||||
f'the tenant database is incomplete')
|
||||
|
||||
with control_session() as s:
|
||||
t = s.get(Tenant, tenant.id)
|
||||
if t is not None:
|
||||
@@ -166,7 +207,8 @@ def bootstrap_tenant(tenant, script_location=None, version_locations=None,
|
||||
if j is not None:
|
||||
j.status = 'ok'
|
||||
j.finished_at = now_eastern()
|
||||
j.log = f'bootstrapped (baseline {baseline_rev}) + stamped {applied}'
|
||||
j.log = (f'bootstrapped (baseline {baseline_rev}, stamped '
|
||||
f'{BASELINE_COVERS_THROUGH}, upgraded to {applied})')
|
||||
return applied
|
||||
|
||||
except Exception as e:
|
||||
|
||||
@@ -37,14 +37,41 @@ from control.models import Plan, Tenant, ProvisioningJob
|
||||
from control.seed import seed_plans
|
||||
from control.tenant_migrate import bootstrap_tenant, chain_head
|
||||
|
||||
# What a COMPLETE tenant database holds: the baseline tables plus everything
|
||||
# the guarded tail (phase33 onward) adds. This list used to stop at the
|
||||
# baseline, which is why it passed while bootstrap was producing databases that
|
||||
# could not serve a login — the expectation encoded the bug.
|
||||
EXPECTED_TABLES = {
|
||||
# baseline (0003_add_user_active)
|
||||
'api_device_tokens', 'api_refresh_tokens', 'areas', 'audit_logs', 'broadcasts',
|
||||
'checklist_items', 'customer_assignments', 'device_registrations', 'facilities',
|
||||
'facility_score_alerts', 'inspection_results', 'inspection_templates',
|
||||
'inspections', 'inspector_assignments', 'issue_comments', 'issue_followers',
|
||||
'issues', 'notification_matrix', 'notification_preferences', 'notifications',
|
||||
'projects', 'scheduled_reports', 'support_ticket_replies', 'support_tickets',
|
||||
'users',
|
||||
'tenant_settings', 'users',
|
||||
# guarded tail
|
||||
'inspection_schedules', # phase34
|
||||
'issue_work_orders', # phase36
|
||||
'project_notification_recipients', # phase37
|
||||
'support_chat_sessions', # phase40
|
||||
'support_chat_messages', # phase40
|
||||
'support_knowledge', # phase40
|
||||
'user_notification_matrix', # phase54
|
||||
'template_contracts', # phase55
|
||||
}
|
||||
|
||||
# Columns added after the baseline, on tables the baseline already creates.
|
||||
# A missing one here is what broke login: SQLAlchemy SELECTs every mapped
|
||||
# column, so the first User.query fails with "Unknown column".
|
||||
EXPECTED_COLUMNS = {
|
||||
'users': ['mfa_enabled', 'mfa_secret', 'mfa_recovery_codes', 'ui_theme'],
|
||||
'facilities': ['qr_token'],
|
||||
'areas': ['qr_token'],
|
||||
'issues': ['handler_type', 'facility_handler_name'],
|
||||
'inspections': ['inspection_schedule_id', 'follow_up_requested_by',
|
||||
'follow_up_requested_at', 'follow_up_assigned_to'],
|
||||
'support_knowledge': ['sort_order'],
|
||||
}
|
||||
|
||||
|
||||
@@ -148,6 +175,22 @@ def verify(args):
|
||||
if need not in ucols:
|
||||
print(f" !! users.{need} missing"); ok = False
|
||||
print(f" users columns: {len(ucols)} (incl id/username/email/role)")
|
||||
|
||||
# Columns added AFTER the baseline. These are the ones that decide
|
||||
# whether the tenant can serve a request at all: SQLAlchemy SELECTs
|
||||
# every mapped column, so one missing here means the first query
|
||||
# against that table raises "Unknown column" — for users, that is the
|
||||
# login. Checked explicitly because a table-only check passed happily
|
||||
# while every post-baseline column was absent.
|
||||
for table, needed in sorted(EXPECTED_COLUMNS.items()):
|
||||
if table not in built:
|
||||
continue # already reported as a missing table above
|
||||
have = {c['name'] for c in insp.get_columns(table)}
|
||||
gone = [c for c in needed if c not in have]
|
||||
if gone:
|
||||
print(f" !! {table} missing post-baseline columns: {gone}"); ok = False
|
||||
if ok:
|
||||
print(" post-baseline columns: all present")
|
||||
finally:
|
||||
engine.dispose()
|
||||
return ok
|
||||
|
||||
Reference in New Issue
Block a user