https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39601
--- Comment #20 from Paul Derscheid <[email protected]> --- Thanks for another thorough review, Katrin. I've rewritten the entire patch set from scratch incorporating all your feedback plus additional improvements found during a full coding guidelines review. 1) QA test failures — All resolved in the rewrite: - Forbidden patterns (http:// URLs, DateTime->now, current_timestamp syntax), all fixed - POD coverage — added for all public subs - Tidiness — all files run through misc/devel/tidy.pl - Template filters — [% USE raw %] added where needed - File permissions — exec flags set on .t files and atomic update - Test::NoWarnings — added to WebauthnCredentials.t 2) Code review a) Spelling: Consistent "Register passkey" throughout. b) Translatability: Fixed. The space is now between </i> and <span>, not inside the translatable string: <i class="fa fa-key"></i> <span class="d-none d-sm-inline">Passkey</span> c) "Username:" in JS: Completely removed. Patron context now comes from hidden inputs populated by template variables. 3) Testing: "Registration error: {msg}" This was a JS i18n bug. I incorrectly used __().format() which only supports %s placeholders, but the strings had {name} placeholders. Changed to __x() which uses Koha.i18n.expand() for named placeholders. Error messages should now render correctly. 4) Answers to your questions a) Testing without hardware: - Chrome/Chromium: DevTools → More tools → WebAuthn. Create a virtual authenticator (check "Supports resident keys" and "Supports user verification"). This simulates a passkey without any hardware. - Firefox: Set security.webauth.webauthn_enable_softtoken = true in about:config. - Password managers like Bitwarden or 1Password can also act as passkey providers. b) "Call authenticate_challenge for a patron without credentials": This means: send POST /api/v1/webauthn/authenticate/challenge with a patron_id for a patron who has zero rows in webauthn_credentials, and verify you get HTTP 404 with {"error": "No credentials registered"}. The test suite already covers this (subtest 2 in webauthn.t). Additional improvements in this rewrite beyond QA fixes: - Extracted business logic to Koha::Auth::WebAuthn (thin controller pattern) - Added typed exceptions (Koha::Exceptions::WebAuthn) - Challenge TTL (10 min), single-use consumption, fail-closed patron matching - account_locked check before issuing session - Explicit credential pubkey lookup check before assertion validation - eval/$@ replaced with try/catch throughout - PK renamed to webauthn_credential_id (SQL7), column COMMENTs added (SQL11) - Date columns follow SQL14: created_date (timestamp), last_used_date (datetime) - COLLATE=utf8mb4_unicode_ci, ON UPDATE CASCADE - Table added to kohastructure.sql - Authenticate endpoints made public (no x-koha-authorization — they're login endpoints) - aria-label on passkey button for accessibility - Licence headers on test files -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
