AS1024/itflow
1
0
Fork 0
mirror of https://github.com/itflow-org/itflow synced 2026-09-01 12:25:11 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix client portal review findings: PIN wipe, log indexing, statement currency (db 2.7.8)

Browse Source 
Setting a PIN containing < or > silently cleared it: the length check ran before escapeSql(), whose strip_tags() then emptied the value, and the UPDATE stored the blank while flashing success. Length is now checked after sanitising.

Password and PIN changes require the current password. SSO contacts are exempt - no local password to check, and the IdP already did it.

New index on logs(log_user_id, log_client_id) for the portal profile and activity pages, which were scanning the whole table twice per profile view. admin/audit_logs.php's date filter rewritten as a half-open range so KEY log_created_at is usable - DATE(log_created_at) BETWEEN made it non-sargable.

Portal statement page and PDF now render in the client's currency, matching the guest view and the emailed statement.

Quick Send asks for confirmation; confirm-link extended to submit buttons.

Portal audit entries logged an empty name - client/post.php used , which only exists agent-side.
This commit is contained in:
johnnyq
2026-08-28 16:38:42 -04:00
parent 820103460f
commit 156c51224d
12 changed files with 213 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
admin/audit_logs.php
View File

@@ -46,6 +46,14 @@ if (isset($_GET['action']) & !empty($_GET['action'])) {
$action_filter = '';
}
/*
* The date filter is a half-open range rather than DATE(log_created_at) BETWEEN
* ... - wrapping the column in a function makes the comparison non-sargable, so
* KEY log_created_at could never be used and every page view scanned the whole
* table. '< dtt + 1 day' keeps the end date inclusive, which BETWEEN on a bare
* DATETIME would not: '2026-08-28' alone means midnight, and would drop that
* day's rows.
*/
$sql = mysqli_query(
$mysqli,
"SELECT SQL_CALC_FOUND_ROWS client_id, client_name, log_action, log_created_at, log_description, log_entity_id, log_id,
@@ -53,7 +61,7 @@ $sql = mysqli_query(
LEFT JOIN users ON log_user_id = user_id
LEFT JOIN clients ON log_client_id = client_id
WHERE (log_type LIKE '%$q%' OR log_action LIKE '%$q%' OR log_description LIKE '%$q%' OR log_ip LIKE '%$q%' OR log_user_agent LIKE '%$q%' OR user_name LIKE '%$q%' OR client_name LIKE '%$q%')
AND DATE(log_created_at) BETWEEN '$dtf' AND '$dtt'
AND log_created_at >= '$dtf 00:00:00' AND log_created_at < DATE_ADD('$dtt', INTERVAL 1 DAY)
$user_query
$client_query
$log_type_query

38
admin/database_updates/2.7.8.php Normal file
View File

@@ -0,0 +1,38 @@
<?php
/*
* ITFlow - Database update to version 2.7.8 (from 2.7.7)
* Included by admin/database_updates.php - do not access directly
*/
defined('FROM_DB_UPDATER') || die("Direct file access is not allowed");
// The logs table only ever had KEY log_created_at, which is the column the
// one page that reads it does NOT filter on usefully. Two shapes now do:
//
// 1. The client portal. profile.php shows a contact their recent sign-ins
// and recent actions, and activity.php lists the lot - all filtered on
// log_user_id + log_client_id. That is two full scans on every profile
// view of a table that grows forever and is never pruned.
//
// 2. admin/audit_logs.php, which filters a date range. It had an index
// available the whole time and could not use it, because wrapping the
// column in DATE() makes the comparison non-sargable. That query is
// rewritten as a half-open range in the same commit as this migration,
// so KEY log_created_at finally does its job.
//
// log_user_id leads the composite: a single user is a small slice of the
// table, whereas one client can account for most of it on a single-client
// install. Same selectivity rule as the 2.7.5 and 2.7.6 passes.
$itflow_index_exists = mysqli_query($mysqli, "SELECT 1 FROM information_schema.STATISTICS
WHERE TABLE_SCHEMA = DATABASE()
AND TABLE_NAME = 'logs'
AND INDEX_NAME = 'log_user_id'
LIMIT 1");
if (!$itflow_index_exists || mysqli_num_rows($itflow_index_exists) === 0) {
mysqli_query($mysqli, "ALTER TABLE `logs` ADD KEY `log_user_id` (`log_user_id`, `log_client_id`)");
}
unset($itflow_index_exists);

10
agent/invoice.php
View File

@@ -246,7 +246,10 @@ if (isset($_GET['invoice_id'])) {
</button>
<div class="dropdown-menu">
<?php if (!empty($config_smtp_provider) && $emailable_contacts > 0) { ?>
<button type="submit" class="dropdown-item" form="quickSendInvoice"
<button type="submit" class="dropdown-item confirm-link" form="quickSendInvoice"
data-confirm-title="Send this invoice now?"
data-confirm-text="It goes to the default contacts without opening the picker."
data-confirm-button="Send"
name="invoice_id" value="<?= $invoice_id ?>">
<i class="fas fa-fw fa-bolt me-2"></i>Quick Send
</button>
@@ -318,7 +321,10 @@ if (isset($_GET['invoice_id'])) {
<i class="fa fa-fw fa-box-open text-secondary me-2"></i>Packing Slip
</a>
<?php if (!empty($config_smtp_provider) && $emailable_contacts > 0) { ?>
<button type="submit" class="dropdown-item" form="quickSendInvoice"
<button type="submit" class="dropdown-item confirm-link" form="quickSendInvoice"
data-confirm-title="Send this invoice now?"
data-confirm-text="It goes to the default contacts without opening the picker."
data-confirm-button="Send"
name="invoice_id" value="<?= $invoice_id ?>">
<i class="fa fa-fw fa-bolt text-secondary me-2"></i>Quick Send
</button>

5
agent/invoices.php
View File

@@ -441,7 +441,10 @@ $num_rows = mysqli_fetch_row(mysqli_query($mysqli, "SELECT FOUND_ROWS()"));
</a>
<div class="dropdown-divider"></div>
<?php if (!empty($config_smtp_provider)) { ?>
<button type="submit" class="dropdown-item" form="quickSendInvoice"
<button type="submit" class="dropdown-item confirm-link" form="quickSendInvoice"
data-confirm-title="Send this invoice now?"
data-confirm-text="It goes to the default contacts without opening the picker."
data-confirm-button="Send"
name="invoice_id" value="<?= $invoice_id ?>">
<i class="fas fa-fw fa-bolt me-2"></i>Quick Send
</button>

10
agent/quote.php
View File

@@ -168,7 +168,10 @@ if (isset($_GET['quote_id'])) {
</button>
<div class="dropdown-menu">
<?php if (!empty($config_smtp_provider) && $emailable_contacts > 0) { ?>
<button type="submit" class="dropdown-item" form="quickSendQuote"
<button type="submit" class="dropdown-item confirm-link" form="quickSendQuote"
data-confirm-title="Send this quote now?"
data-confirm-text="It goes to the default contacts without opening the picker."
data-confirm-button="Send"
name="quote_id" value="<?= $quote_id ?>">
<i class="fas fa-fw fa-bolt me-2"></i>Quick Send
</button>
@@ -237,7 +240,10 @@ if (isset($_GET['quote_id'])) {
<i class="fa fa-fw fa-download text-secondary me-2"></i>Download PDF
</a>
<?php if (!empty($config_smtp_provider) && $emailable_contacts > 0) { ?>
<button type="submit" class="dropdown-item" form="quickSendQuote"
<button type="submit" class="dropdown-item confirm-link" form="quickSendQuote"
data-confirm-title="Send this quote now?"
data-confirm-text="It goes to the default contacts without opening the picker."
data-confirm-button="Send"
name="quote_id" value="<?= $quote_id ?>">
<i class="fa fa-fw fa-bolt text-secondary me-2"></i>Quick Send
</button>

5
agent/quotes.php
View File

@@ -224,7 +224,10 @@ $num_rows = mysqli_fetch_row(mysqli_query($mysqli, "SELECT FOUND_ROWS()"));
</a>
<?php if (!empty($config_smtp_provider)) { ?>
<div class="dropdown-divider"></div>
<button type="submit" class="dropdown-item" form="quickSendQuote"
<button type="submit" class="dropdown-item confirm-link" form="quickSendQuote"
data-confirm-title="Send this quote now?"
data-confirm-text="It goes to the default contacts without opening the picker."
data-confirm-button="Send"
name="quote_id" value="<?= $quote_id ?>">
<i class="fas fa-fw fa-bolt me-2"></i>Quick Send
</button>

30
client/functions.php
View File

@@ -72,6 +72,36 @@ function enforceContactCan($capability) {
}
}
/*
* Confirms the person at the keyboard is the account holder, before a change
* that would let someone who hijacked a session take the account over or
* defeat phone verification.
*
* Returns true for SSO contacts without checking anything: there is no local
* password to compare against, and the identity provider has already done this
* work. Gating them on a password they do not have would just lock them out.
*/
function portalReauthenticate($current_password) {
global $mysqli, $session_user_id;
if (($_SESSION['login_method'] ?? 'local') !== 'local') {
return true;
}
if (empty($current_password)) {
return false;
}
$sql = mysqli_query($mysqli, "SELECT user_password FROM users WHERE user_id = $session_user_id LIMIT 1");
$row = mysqli_fetch_assoc($sql);
if (!$row || empty($row['user_password'])) {
return false;
}
return password_verify($current_password, $row['user_password']);
}
/*
* A timestamp a person can read, in the company's configured date and time
* format rather than the raw DATETIME the database hands back.

58
client/post.php
View File

@@ -232,21 +232,27 @@ if (isset($_POST['set_contact_pin'])) {
* section, so every signed-in contact manages their own - same as the
* password change above.
*/
$pin = trim($_POST['pin'] ?? '');
if ($pin === '') {
flashAlert("Enter a PIN, or leave the page to keep the current one", 'error');
redirect('profile.php');
}
if (strlen($pin) < 4) {
flashAlert("Your PIN needs to be at least 4 characters", 'error');
// The PIN is what we read back to verify a caller, so changing it is
// re-authenticated for password logins. SSO contacts are exempt: they have
// no local password to check, and the identity provider already did this.
if (!portalReauthenticate($_POST['current_password'] ?? '')) {
flashAlert("That password was not right - your PIN has not been changed", 'error');
redirect('profile.php');
}
// contact_pin is varchar(255) - trim to fit rather than let an over-long
// value error out under strict mode
$pin = escapeSql(substr($pin, 0, 255));
// value error out under strict mode.
//
// escapeSql() runs strip_tags() before escaping, so the length has to be
// re-checked AFTER it: a PIN of "<1234>" passed a check on the raw input,
// came out of strip_tags() as an empty string, and the UPDATE below then
// silently WIPED the contact's PIN while flashing "Phone PIN updated".
$pin = escapeSql(substr(trim($_POST['pin'] ?? ''), 0, 255));
if (strlen($pin) < 4) {
flashAlert("Your PIN needs to be at least 4 characters, and cannot contain < or >", 'error');
redirect('profile.php');
}
mysqli_query($mysqli, "UPDATE contacts SET contact_pin = '$pin' WHERE contact_id = $session_contact_id AND contact_client_id = $session_client_id");
@@ -460,10 +466,17 @@ if (isset($_GET['export_statement_pdf'])) {
*/
$client_id = $session_client_id;
$sql = mysqli_query($mysqli, "SELECT client_name FROM clients WHERE client_id = $client_id LIMIT 1");
$sql = mysqli_query($mysqli, "SELECT client_currency_code, client_name FROM clients WHERE client_id = $client_id LIMIT 1");
$row = mysqli_fetch_assoc($sql);
$client_name = escapeHtml($row['client_name']);
// Match client/statement.php, the guest invoice view and the emailed
// statement - all four render in the client's own currency
$statement_currency_code = escapeHtml($row['client_currency_code']);
if (empty($statement_currency_code)) {
$statement_currency_code = $session_company_currency;
}
$sql = mysqli_query($mysqli, "SELECT company_address, company_city, company_country, company_logo, company_name,
company_phone, company_phone_country_code, company_state, company_website, company_zip
FROM companies WHERE company_id = 1");
@@ -581,15 +594,15 @@ if (isset($_GET['export_statement_pdf'])) {
<td style="font-size:9pt;">' . $invoice_scope . '</td>
<td style="font-size:9pt;">' . $invoice_date . '</td>
<td style="font-size:9pt;"' . $due_style . '>' . $invoice_due . '</td>
<td style="font-size:9pt;" align="right">' . numfmt_format_currency($currency_format, $invoice_amount, $session_company_currency) . '</td>
<td style="font-size:9pt;" align="right">' . numfmt_format_currency($currency_format, $amount_paid, $session_company_currency) . '</td>
<td style="font-size:9pt;" align="right">' . numfmt_format_currency($currency_format, $invoice_balance, $session_company_currency) . '</td>
<td style="font-size:9pt;" align="right">' . numfmt_format_currency($currency_format, $invoice_amount, $statement_currency_code) . '</td>
<td style="font-size:9pt;" align="right">' . numfmt_format_currency($currency_format, $amount_paid, $statement_currency_code) . '</td>
<td style="font-size:9pt;" align="right">' . numfmt_format_currency($currency_format, $invoice_balance, $statement_currency_code) . '</td>
</tr>';
}
$html .= '<tr>
<td colspan="6" align="right" style="font-weight:bold;">Total Balance Due</td>
<td align="right" style="font-weight:bold;">' . numfmt_format_currency($currency_format, $statement_total, $session_company_currency) . '</td>
<td align="right" style="font-weight:bold;">' . numfmt_format_currency($currency_format, $statement_total, $statement_currency_code) . '</td>
</tr>
</table>';
@@ -621,6 +634,13 @@ if (isset($_POST['edit_profile'])) {
$new_password = $_POST['new_password'];
// Without this a hijacked session could set a new password without knowing
// the old one, locking the real contact out of their own portal.
if (!empty($new_password) && !portalReauthenticate($_POST['current_password'] ?? '')) {
flashAlert("That password was not right - your password has not been changed", 'error');
redirect('profile.php');
}
if (!empty($new_password)) {
$password_hash = password_hash($new_password, PASSWORD_DEFAULT);
mysqli_query($mysqli, "UPDATE users SET user_password = '$password_hash' WHERE user_id = $session_user_id");
@@ -913,7 +933,7 @@ if (isset($_GET['add_payment_by_provider'])) {
// Notify/log
appNotify("Invoice Paid", "Invoice $invoice_prefix$invoice_number automatically paid", "/agent/invoice.php?invoice_id=$invoice_id", $client_id);
logAudit("Invoice", "Payment", "$session_name initiated Stripe payment amount of " . numfmt_format_currency($currency_format, $invoice_amount, $invoice_currency_code) . " added to invoice $invoice_prefix$invoice_number - $pi_id $extended_log_desc", $client_id, $invoice_id);
logAudit("Invoice", "Payment", "$session_contact_name initiated Stripe payment amount of " . numfmt_format_currency($currency_format, $invoice_amount, $invoice_currency_code) . " added to invoice $invoice_prefix$invoice_number - $pi_id $extended_log_desc", $client_id, $invoice_id);
triggerCustomAction('invoice_pay', $invoice_id);
flashAlert("The amount " . numfmt_format_currency($currency_format, $invoice_amount, $invoice_currency_code) . " paid Invoice $invoice_prefix$invoice_number");
@@ -1374,14 +1394,14 @@ if (isset($_POST['set_recurring_payment'])) {
// Get Payment ID for reference
$recurring_payment_id = mysqli_insert_id($mysqli);
logAudit("Recurring Invoice", "Auto Payment", "$session_name created Auto Pay for Recurring Invoice $recurring_invoice_prefix$recurring_invoice_number in the amount of " . numfmt_format_currency($currency_format, $recurring_invoice_amount, $recurring_invoice_currency_code), $session_client_id, $recurring_invoice_id);
logAudit("Recurring Invoice", "Auto Payment", "$session_contact_name created Auto Pay for Recurring Invoice $recurring_invoice_prefix$recurring_invoice_number in the amount of " . numfmt_format_currency($currency_format, $recurring_invoice_amount, $recurring_invoice_currency_code), $session_client_id, $recurring_invoice_id);
flashAlert("Automatic Payment $saved_payment_description enabled for Recurring Invoice $recurring_invoice_prefix$recurring_invoice_number");
} else {
// Delete
mysqli_query($mysqli, "DELETE FROM recurring_payments WHERE recurring_payment_recurring_invoice_id = $recurring_invoice_id");
logAudit("Recurring Invoice", "Auto Payment", "$session_name removed Auto Pay for Recurring Invoice $recurring_invoice_prefix$recurring_invoice_number in the amount of " . numfmt_format_currency($currency_format, $recurring_invoice_amount, $recurring_invoice_currency_code), $session_client_id, $recurring_invoice_id);
logAudit("Recurring Invoice", "Auto Payment", "$session_contact_name removed Auto Pay for Recurring Invoice $recurring_invoice_prefix$recurring_invoice_number in the amount of " . numfmt_format_currency($currency_format, $recurring_invoice_amount, $recurring_invoice_currency_code), $session_client_id, $recurring_invoice_id);
flashAlert("Automatic Payment Disabled for Recurring Invoice $recurring_invoice_prefix$recurring_invoice_number");
}

20
client/profile.php
View File

@@ -439,6 +439,17 @@ $sql_actions = mysqli_query(
will remember but that is not easy to guess.
</p>
<?php if ($login_method === 'local') { ?>
<div class="mb-3">
<label for="pinCurrentPassword">Your current password</label>
<div class="input-group">
<span class="input-group-text"><i class="fa fa-fw fa-lock"></i></span>
<input type="password" class="form-control" id="pinCurrentPassword" name="current_password"
autocomplete="current-password" required>
</div>
</div>
<?php } ?>
<div class="mb-3">
<label for="contactPin"><?= empty($contact_pin) ? 'New PIN' : 'Replace with' ?></label>
<div class="input-group">
@@ -476,6 +487,15 @@ $sql_actions = mysqli_query(
</div>
<div class="modal-body">
<div class="mb-3">
<label for="currentPassword">Your current password</label>
<div class="input-group">
<span class="input-group-text"><i class="fa fa-fw fa-lock"></i></span>
<input type="password" class="form-control" id="currentPassword" name="current_password"
autocomplete="current-password" required>
</div>
</div>
<div class="mb-3">
<label for="newPassword">New password</label>
<div class="input-group">

20
client/statement.php
View File

@@ -17,6 +17,18 @@ require_once "includes/inc_all.php";
enforceContactCan('accounting');
/*
* Amounts render in the client's own currency, matching the guest invoice view
* and the statement an agent emails. This page and its PDF used to render in
* the company currency, so the same invoice carried a different symbol
* depending on where you read it.
*/
$row = mysqli_fetch_assoc(mysqli_query($mysqli, "SELECT client_currency_code FROM clients WHERE client_id = $session_client_id LIMIT 1"));
$statement_currency_code = escapeHtml($row['client_currency_code']);
if (empty($statement_currency_code)) {
$statement_currency_code = $session_company_currency;
}
/*
* Payments are summed in a derived table rather than joined directly, or an
* invoice with two payments against it would be counted twice.
@@ -119,9 +131,9 @@ $statement_total = 0;
<td><?= $invoice_scope_display ?></td>
<td><?= $invoice_date ?></td>
<td class="<?= $overdue_color ?>"><?= $invoice_due ?></td>
<td class="text-end"><?= numfmt_format_currency($currency_format, $invoice_amount, $session_company_currency) ?></td>
<td class="text-end"><?= numfmt_format_currency($currency_format, $amount_paid, $session_company_currency) ?></td>
<td class="text-end fw-bold"><?= numfmt_format_currency($currency_format, $invoice_balance, $session_company_currency) ?></td>
<td class="text-end"><?= numfmt_format_currency($currency_format, $invoice_amount, $statement_currency_code) ?></td>
<td class="text-end"><?= numfmt_format_currency($currency_format, $amount_paid, $statement_currency_code) ?></td>
<td class="text-end fw-bold"><?= numfmt_format_currency($currency_format, $invoice_balance, $statement_currency_code) ?></td>
</tr>
<?php
@@ -134,7 +146,7 @@ $statement_total = 0;
<tfoot>
<tr>
<th colspan="6" class="text-end">Total Balance Due</th>
<th class="text-end"><?= numfmt_format_currency($currency_format, $statement_total, $session_company_currency) ?></th>
<th class="text-end"><?= numfmt_format_currency($currency_format, $statement_total, $statement_currency_code) ?></th>
</tr>
</tfoot>
</table>

3
db.sql
View File

@@ -1513,7 +1513,8 @@ CREATE TABLE `logs` (
`log_user_id` int(11) NOT NULL DEFAULT 0,
`log_entity_id` int(11) NOT NULL DEFAULT 0,
PRIMARY KEY (`log_id`),
KEY `log_created_at` (`log_created_at`)
KEY `log_created_at` (`log_created_at`),
KEY `log_user_id` (`log_user_id`,`log_client_id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci;
/*!40101 SET character_set_client = @saved_cs_client */;

40
js/confirm_modal.js
View File

@@ -1,8 +1,13 @@
/**
* Confirmation dialogs, on SweetAlert2.
*
* Delegated on document rather than bound to the links present at page load, so
* that confirm-link also works on markup injected by an ajax modal.
* Delegated on document rather than bound to the elements present at page load,
* so that confirm-link also works on markup injected by an ajax modal.
*
* Works on an <a class="confirm-link"> (navigates on confirm) and on a
* <button class="confirm-link"> that submits a form (submits on confirm). The
* button form exists for actions that must not be a GET - Quick Send mails a
* document to a client, so it posts.
*
* Per-link copy comes from data attributes, all optional:
* data-confirm-title heading (default: "Are you sure?")
@@ -15,12 +20,18 @@
* without touching any of them.
*/
document.addEventListener('click', function (e) {
const link = e.target.closest('a.confirm-link');
const link = e.target.closest('a.confirm-link, button.confirm-link');
if (!link || typeof Swal === 'undefined') {
return;
}
e.preventDefault();
// A submit button contributes its own name/value to the submission, and
// requestSubmit() preserves that where form.submit() would drop it - which
// is exactly how the Quick Send buttons carry their invoice or quote id.
const submitter = link.tagName === 'BUTTON' ? link : null;
const form = submitter ? (submitter.form || submitter.closest('form')) : null;
const destructive = link.classList.contains('text-danger');
Swal.fire({
@@ -42,8 +53,27 @@ document.addEventListener('click', function (e) {
cancelButton: 'btn btn-secondary mx-1'
}
}).then(function (result) {
if (result.isConfirmed) {
window.location.href = link.getAttribute('href');
if (!result.isConfirmed) {
return;
}
if (form) {
if (typeof form.requestSubmit === 'function') {
form.requestSubmit(submitter);
} else {
// Fallback for browsers without requestSubmit: replay the
// button's name/value as a hidden field so the handler still
// sees which record was picked.
if (submitter.name) {
const carried = document.createElement('input');
carried.type = 'hidden';
carried.name = submitter.name;
carried.value = submitter.value;
form.appendChild(carried);
}
form.submit();
}
return;
}
window.location.href = link.getAttribute('href');
});
});