* FIX#32909 Restrict salary/HR field edit to authorized users
On the user card, a user with only "modify own info" (user->self->write)
could edit thm, tjm, salary, salaryextra and weeklyhours, even though the
display of these fields is gated on salaries->readall / hrm->employee->read.
Guard the write of these fields with the same permission used for display,
so self-service users can no longer change compensation data they are not
allowed to see.
Signed-off-by: Sanjiv Kumar Bhagat <reach.sanjiv.bhagat@gmail.com>
* Update card.php
* Update salary edit permissions logic
---------
Signed-off-by: Sanjiv Kumar Bhagat <reach.sanjiv.bhagat@gmail.com>
Co-authored-by: Laurent Destailleur <eldy@destailleur.fr>
Cache key was using legacy fk_contact field instead of the current
contact id ($cid), causing all contacts after the first to resolve
to the same cached entry in show_actions_messaging().
# SEC: Sanitize SQL query parameters
Sanitize SQL query parameters.
Replace urlencode with explicit type casting to int for fk_statut and signed_status parameters.
On the third party discount tabs (htdocs/comm/remx.php), the multicurrency amount columns
(MulticurrencyAmountHT / MulticurrencyAmountTTC) and their colspan adjustments were guarded by
isModEnabled('multicompany') instead of isModEnabled('multicurrency'). As a result the foreign
currency amounts of available and consumed discounts/down payments were never shown when the
Multicurrency module is enabled (and the Multi-company module disabled).
The wrong test was introduced in commit 3bb1a2387b (2022, 'FIX php8 compatibility') which converted
!empty($conf->multicurrency->enabled) into isModEnabled('multicompany'). Restore the correct module
test. Present from v17 to develop; fixed here on the oldest maintained LTS branch (18.0).
`getOnlineSignatureUrl()` ends each public signature URL with
`&securekey=` followed by the output of `dol_hash(..., '0')`. With the
default `MAIN_SECURITY_HASH_ALGO = password_hash`, that output is a
bcrypt string whose alphabet is `[./A-Za-z0-9]`, so a non-trivial share
of generated URLs legitimately end with `.` or `/`.
Gmail and several mobile mail clients autolink plain-text URLs but
drop trailing punctuation, so the recipient clicks on a URL with the
last character missing and gets a securekey mismatch error
(`Bad value for securitykey`). The user-facing symptom is exactly the
one in #31464.
When multicompany is enabled, the URL already ends with `&entity=N`,
which sidesteps the issue. For installations without multicompany,
append a stable trailing `&_=1` so the URL never ends with the hash.
The receiver in `public/onlinesign/newonlinesign.php` does not read
`_`, only `source`, `ref`, `securekey` and (when multicompany) `entity`,
so adding the parameter has no behavior impact and existing links keep
verifying as before. Only the real URL (mode 0) is affected, not the
mode 1 preview shown in the admin setup.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
The minimum-price guard on the customer invoice, customer order and
customer proposal forms compares the entered unit price to the
product's price_min. When the document is in a foreign currency the
unit price is provided in the foreign currency ($pu_ht_devise), so
it must first be converted back to the base (company) currency before
the comparison.
card.php was multiplying by $currency_tx:
$pu_equivalent = (float) $pu_ht_devise * $currency_tx;
but multicurrency_tx is defined as 'units of foreign currency per
1 unit of base', exactly as price.lib.php uses it
(calcul_price_total at line 181):
$pu = $pu_devise / $multicurrency_tx;
With the reporter's example (1 MXN = 0.055 USD, so tx = 0.055,
product min price = 1000 MXN, entered 55 USD), the wrong direction
gives 55 * 0.055 = 3.025 MXN < 1000 MXN and the form errors out with
'CantBeLessThanMinPrice', even though 55 USD is exactly 1000 MXN.
Dividing yields 55 / 0.055 = 1000 MXN and the check passes.
Mirror calcul_price_total's direction at all four invoice sites
(htdocs/compta/facture/card.php lines 2503/2506 and 2684/2687) plus
the corresponding sites in htdocs/commande/card.php and
htdocs/comm/propal/card.php. Documents in the base currency
($pu_ht_devise empty) are not affected because the conversion
branch is gated on $pu_ht_devise being non-empty.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
The per-card Send by email button on Propal, Commande and Facture
already honours the hidden constants
`PROPOSAL_SENDBYEMAIL_FOR_ALL_STATUS`,
`COMMANDE_SENDBYEMAIL_FOR_ALL_STATUS` and
`FACTURE_SENDBYEMAIL_FOR_ALL_STATUS` to allow sending drafts. The
mass Send by email action in the corresponding lists silently dropped
draft objects regardless of those constants, so an administrator who
intentionally enabled the override saw inconsistent behaviour between
the card and the list.
Apply the same constant check on the three draft-skip guards in
`htdocs/core/actions_massactions.inc.php` so the mass action behaves
the same way as the single-object button. Default behaviour is
unchanged: with none of the constants set, drafts are still skipped
with the existing 'OnlyXxxNotDraftCanBeSentInMassAction' error
message.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
* Fix#38491 Name the field in the ticket too-long error message
When a ticket message exceeds 65000 characters, Ticket::verify() and
Ticket::update() appended the bare string "ErrorFieldTooLong" to
$this->errors. setEventMessages forwards that string and
dol_htmloutput_mesg applies $langs->trans() to it, which yields the
raw template "Field %s is too long." with the %s placeholder still
visible because no argument was passed.
Replace the bare key with a pre-translated message that names the
field ("Initial message"), the same pattern already used in
htdocs/barcode/printsheet.php for the same lang key. Two identical
sites in the file are updated (one in verify, called by create, and
one in update). The "errors" and "ticket" lang files are loaded
defensively before translation because both methods can run before
the page-level loadLangs.
Reported by @ocemag, expected field name confirmed by @aspangaro.
Signed-off-by: Dolicraft <contact@dolicraft.com>
* Fix phan: declare $langs as global in ticket too-long branches
phan level 10 raised PhanUndeclaredVariable and PhanNonClassMethodCall
on the two branches that translate the new error message. The enclosing
Ticket::verify() and Ticket::update() do not declare global $langs;
add it where needed instead of opening the method scope further.
Signed-off-by: Dolicraft <contact@dolicraft.com>
---------
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Alexandre SPANGARO <alexandre.spangaro@gmail.com>
ExtraFields::showInputField normalises the legacy "(field=123)"
filter syntax of "List from a table" extrafields into the modern
USF "(field:=:123)" form. The compatibility regex
"/^\(?([a-z0-9]+)([=<>]+)(\d+)\)?$/i" only accepts alphanumeric
field names, so a filter like "fk_typent=2" does not match, the
filter is left as is, and forgeSQLFromUniversalSearchCriteria later
fails with "Filter error" because the raw expression is not valid
universal search syntax.
Accept the underscore character in the field name class, the rest
of the predicate is untouched. The two identical occurrences in
the file (one per branch of the dispatch) are updated.
Reported and root cause pinpointed by @stephanebourdon-cmyk.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
Updating a product selling price via /product/price.php triggered a
fatal "Call to undefined method stdClass::isEmpty()" at
Product::update line 1457 of 21.0. In the update_price flow,
$this->oldcopy is a stdClass (not a Product instance), so isEmpty()
does not exist on it. The page returned HTTP 500 and the open
transaction was forcibly closed by DoliDBMysqli::close.
Replace the isEmpty() call with empty($this->oldcopy->id), which is
the form already shipped in 22.0, 23.0 and develop after upstream
commit ef94452f77. The accompanying comment from that commit is
also brought in.
Same root cause as issue 37792 (closed, fixed in 23.0.3 and
forward); the fix was never backported to 21.0 even though the bug
is still triggered on that branch.
Reported by @YOYOKMC.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
