Fix bcrypt password verification and enforce bcrypt for all new password storage

[Copilot]

verifyPassword() routed to password_verify() only when $salt === "bcrypt", but several write paths stored bcrypt hashes while leaving salt as an MD5 string — causing login to silently fall through to SHA1 comparison and always fail.

Changes

app/helper/security.php

• Added isBcryptHash() to detect bcrypt hashes by prefix ($2y$, $2b$, $2a$)
• verifyPassword() now uses bcrypt path when the hash is bcrypt, regardless of salt value

// Before: only bcrypt-verified when salt === "bcrypt"
if ($salt === "bcrypt") {
    return password_verify($password, $hash);
}
return hash_equals($this->hash($password, $salt), $hash); // always ran for MD5 salt, even against $2y$ hashes

// After: auto-detects bcrypt hashes by prefix
if ($salt === "bcrypt" || $this->isBcryptHash($hash)) {
    return password_verify($password, $hash);
}

app/controller/user.php, app/controller/admin.php, app/controller/index.php

• All password write paths (user password change, admin user create/edit, password reset, forced reset) were calling hash($password, $salt) with a non-null salt, hitting the SHA1 path instead of bcrypt
• Fixed to call hash($password) (no salt) and unpack the returned ["salt" => "bcrypt", "hash" => "..."] array
• Temporary passwords in admin keep salt = null to preserve forced-reset-on-login behaviour; the bcrypt hash is still correctly verified via prefix detection

install.php

• Initial admin password created during fresh installation was also using the legacy SHA1 path; now uses hash() without a salt so new installs store a bcrypt hash from the start

tests/stringTest.php

• Added testVerifyBcryptPasswordWithMismatchedSalt() covering the exact failure case: bcrypt hash stored alongside an MD5 salt

[Citydampf]

looks good

[Copilot]

Pull request overview

This PR updates password verification and storage paths so bcrypt hashes are verified even when legacy salt metadata is inconsistent, and several password update flows now generate bcrypt hashes directly.

Changes:

• Added bcrypt hash prefix detection in Helper\Security::verifyPassword().
• Updated account, admin, password reset, and forced-reset password write paths to use bcrypt hash output.
• Added a regression test for bcrypt verification when the stored salt is legacy-looking.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
app/helper/security.php	Detects bcrypt hashes by prefix during password verification.
app/controller/user.php	Stores bcrypt output for user-initiated password changes.
app/controller/index.php	Stores bcrypt output for reset and forced-reset flows.
app/controller/admin.php	Stores bcrypt output for admin-set temporary/permanent passwords.
tests/stringTest.php	Adds regression coverage for mismatched bcrypt hash and salt metadata.

Comments suppressed due to low confidence (1)

app/controller/index.php:300

• Mandatory: this forced-reset path also writes the full bcrypt hash into user.password, which is still declared as char(40) in the database schemas. The stored hash can be truncated or rejected, leaving users unable to complete the forced reset/login flow until the schema is updated.

            $hashResult = $security->hash($f3->get("POST.password1"));
            $user->salt = $hashResult["salt"];
            $user->password = $hashResult["hash"];

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.