To test:
prove -v /kohadevbox/koha/xt/author/pod_coverage.t
The changes to Schema files are fixes that were overlooked originally.
The 'die' line in C4/SIP/Sip/Configuration.pm is to force the script to die if SIP configuration does not exist, which only happens when running pod_coverage.t
The original code (prior to 37893) did not consider the possibiliy of a config_file being invalid or undef, parser->XMLin(config_file) died silently, but because pod_coverage.t skips over files that die, it didn't fail. See bug 41271.
Now, with the changes from bug 37893, if a config_file doesnt exist the code falls back to whats in the cache, so it doesn't try to XMLin load an invalid config_file anymore, and fails verbosely.
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Remove login_password column from the DBIx::Class schema result class
to reflect the database structure changes.
This updates the auto-generated schema to match the removal of the
login_password column from the sip_accounts table.
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This follow-up removes the security vulnerability of storing plaintext
passwords in the sip_accounts table. The SIP2 system now relies entirely
on Koha's secure password authentication.
Changes:
- Remove plaintext password comparison from login_core() and telnet_transport()
- Remove login_password column from sip_accounts table
- Update database schema files and atomic update
- Remove password field from API definitions and Vue form
- Update all tests to remove login_password references
Authentication now works in two stages:
1. Access control: Check if userid exists in sip_accounts (allowlist)
2. Authentication: Validate password against hash in borrowers table
This maintains SIP access control while eliminating plaintext password
storage. Staff users authenticate with their Koha password (securely hashed).
Test plan:
1. Run updated tests to verify they pass
2. Verify SIP2 authentication works with staff user credentials
3. Confirm no plaintext passwords are stored
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
To test:
1) Ensure a port is already in use in the server
2) Try creating a new listener using that same port.
3) Notice the error 'Something went wrong: Error: Can't listen on port 127.0.0.1:8023/tcp/IPv4. Port already in use.' is shown on the UI
Co-authored-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Auto generated files resulted from running 'dbic'
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch simply skips undefined records - this prevents the error
though it doesn't explain why we get undef records here and a Z39 search
from standard cataloging just returns nothing.
This prevents a breakage of the search.
To test:
1 - Have LOC defined as a Z39 Source (standard in KTD)
2 - Open the advanced editor
3 - Search for ISBN 9780520401129
4 - Search explodes, logs like:
AH01215: stderr from /kohadevbox/koha/svc/cataloguing/metasearch: Can't call method "raw" on an undefined value at /kohadevbox/koha/Koha/MetaSearcher.pm line 267., referer: http://localhost:8081/cgi-bin/koha/cataloguing/editor.pl
AH01215: stderr from /kohadevbox/koha/svc/cataloguing/metasearch: Magic number checking on storable file failed at /usr/lib/x86_64-linux-gnu/perl/5.36/Storable.pm line 443, at /kohadevbox/koha/Koha/MetaSearcher.pm line 144., referer: http://localhost:8081/cgi-bin/koha/cataloguing/editor.pl
5 - Must refresh the page to clear the bad error message
6 - Apply patch
7 - Repeat search - no results, LOC says 'Permanent system error'
8 - You can close the results and do a new search
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
In order to prevent permission errors to user without the 'parameters'
permission we are introducing a separate endpoint for the ERM module to
retrieve additional fields for this specific module.
* Adds a new endpoint /erm/extended_attribute_types
* Adjusts Cypress and REST API tests
* Adds REST API swagger specs
I wanted to keep the code as simple as possible and decided to create a
wrapper for the additional fields API client. The magic is in
koha-tmpl/intranet-tmpl/prog/js/fetch/additional-fields-api-client.js
Ideally we should provide a test for this but I failed to write them.
If required by QA I would suggest to deal with them on a separate bug to
not delay this bugfix. The code has been kept designed to be used in
other contexts but the api client is not used outside of Vue and
outside of the ERM module.
Test plan:
Confirm that user without the 'parameters' permission can use the ERM
module with additional fields.
Sponsored-by: Karlsruhe Institute of Technology (KIT)
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
prove t/db_dependent/api/v1/patrons_hold_groups.t
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch adds sorting to lists of lists in the OPAC.
The mechanism used here for op=list is similar to the one
used in op=view, so it's stylistically and functionally
consistent with the rest of the Lists module in the OPAC.
Test plan:
0. Apply the patch and restart Plack (ie koha-plack --restart kohadev)
1. Create 20+ public lists and 20+ private lists for a particular user
you control (try to use names that range throughout the alphabet)
2. Using SQL, update the Modification date for at least 1 public
list and 1 private list
3. As an anonymous (ie not logged in user), go to
/cgi-bin/koha/opac-shelves.pl?op=list&public=0
4. Note you cannot see any lists
5. Go to http://localhost:8080/cgi-bin/koha/opac-shelves.pl?op=list&public=1
6, Note you can see public lists with a toolbar with the button "Sort"
below the alert "Log in to create a new list"
7. Using the "Sort" option, try out all the different sort options
for default, "List name", and "Modification date"
8. After sorting, try paging through the list of lists. You should
note that the sorting is retained through the paging.
9. prove t/db_dependent/Virtualshelves.t
10. Celebrate!
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The AuthClient template plugin was causing 500 errors during database
upgrades when the identity_providers schema was out of sync. The plugin
tried to query the new 'auto_register_opac' column before the database
schema was updated, creating a chicken-egg problem.
This patch wraps the database query in Try::Tiny to handle schema
mismatches gracefully during upgrades, allowing the upgrade process
to continue without crashing the authentication interface.
Test plan:
1. On main, apply all bug 37711 patches BUT THIS ONE
2. Simulate an upgrade by:
* Edit Koha.pm so it points to the next version
* Move the atomicupdate file to the right number in db_revs
3. Restart everything:
$ ktd --shell
k$ restart_all
4. Access http://kohadev-intra.localhost
=> FAIL: You get a 500! With a stack trace!
5. Apply this patch
6. Repeat 3 and 4
=> SUCCESS: No 500 error, redirects to installer
7. Proceed with the upgrade:
=> SUCCESS: It works as it should
8. Run tests:
k$ prove -v t/db_dependent/Template/Plugin/AuthClient.t
=> SUCCESS: All tests pass
9. Sign off :-D
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Before this patches, only OPAC auto-registration at login using
OAuth2/OIDC IdPs was allowed.
This patch makes the staff interface to be able to auto-register new
users.
On doing it, I had to track down the possible values for the `interface`
parameter in the case of the staff interface.
Koha::Template::Plugin::AuthClient uses 'staff' internally so I sticked
with it. The whole area needs more param validation and exceptions to
avoid mistakes for not knowing the valid values.
I added some safeguards on the Koha::REST::Plugin::Auth::IdP helpers.
They get tested on the added tests.
To test:
1. Apply this patch
2. Run:
$ prove t/db_dependent/Koha/REST/Plugin/Auth/IdP.t
=> SUCCESS: Tests pass!
3. Have a valid OAuth...
4. Enable Staff auto-register
5. Login with an external IdP on the staff interface
=> SUCCESS: It gets registered!
Note: it will probably lack permissions to access the staff interface,
but you can check on the DB it got registered.
6. Sign off :-D
Sponsored-by: ByWater Solutions
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
We should not log/leak session_id!
Failed test 'Context set from userenv + interface'
at t/db_dependent/Koha/BackgroundJob.t line 132.
Structures begin differing at:
$got->{session_id} = undef
$expected->{session_id} = Does not exist
Looks like you failed 1 test of 10.
Failed test 'enqueue() tests'
at t/db_dependent/Koha/BackgroundJob.t line 135.
Failed test 'Userenv set from job context on process'
at t/db_dependent/Koha/BackgroundJob.t line 240.
Structures begin differing at:
$got->{session_id} = undef
$expected->{session_id} = Does not exist
Looks like you failed 1 test of 5.
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
In bug 40587 we renamed a variable from $b to $basketno. After rebasing,
the auto-orders blocking code had a bug where it referenced an undefined
variable $b instead of $basketno when checking for PO number conflicts.
This caused warnings: "Use of uninitialized value $Koha::EDI::b in exists"
This patch:
- Renames $b to $basketno throughout the conflict checking code
- Updates test plan from 7 to 8 to match actual subtest count
Test plan:
1. Run prove t/db_dependent/Koha/EDI.t
2. Verify all tests pass without warnings
3. Confirm auto-orders blocking works correctly for duplicate PO numbers
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
When po_is_basketname is enabled for a vendor, duplicate purchase order
numbers should prevent both automatic ordering (auto_orders) and manual
UI-triggered ordering to avoid conflicts with existing baskets.
This patch implements blocking functionality that:
1. Auto-orders blocking:
- Tracks baskets with PO number conflicts during quote processing
- Skips auto-ordering for baskets with duplicate PO numbers
- Logs clear warnings about blocked baskets
2. UI basket ordering blocking:
- Adds duplicate checking in create_edi_order function
- Returns detailed error information when conflicts detected
- Prevents EDI order creation for conflicting baskets
3. User-friendly error handling:
- Updates basket.pl to handle duplicate PO number errors
- Stays on basket page to display error messages
- Provides clear messaging about conflicts and resolution steps
4. Template messaging:
- Adds clear error message explaining duplicate PO conflicts
- Shows which existing basket uses the same PO number
- Guides users on next steps to resolve the issue
This ensures data integrity and prevents order processing conflicts
while providing clear feedback to users about duplicate PO number
issues.
Test plan:
1. Set up a vendor with EDI account and enable "po_is_basketname"
2. Create a basket with a specific name (e.g., "PO12345")
3. Process an EDIFACT quote that contains the same PO number:
- If auto_orders is enabled: verify the new basket is created but NOT automatically ordered
- Check the logs show "Auto-order blocked" message
4. For UI testing, create another basket manually with the same name
5. Try to generate EDI order via "Generate EDI order" button
6. Verify: Error message appears stating duplicate PO number conflict
7. Verify: Basket remains open (not closed)
8. Verify: Error message shows which existing basket uses the PO number
9. Test with po_is_basketname disabled: verify normal operation continues
10. Test with unique PO numbers: verify orders process normally
Signed-off-by: Hannah Dunne-Howrie <hdunne-howrie@westminster.gov.uk>
Sponsored-by: Westminster City Council <https://www.westminster.gov.uk/>
Sponsored-by: Royal Borough of Kensington and Chelsea <https://www.rbkc.gov.uk/>
Sponsored-by: OpenFifth <https://openfifth.co.uk/>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Pedro Amorim
Lucas Gass
Martin Renvoize
Pedro Amorim
Nick Clemens
Jonathan Druart
Paul Derscheid
Martin Renvoize
David Cook
Tomas Cohen Arazi