Resolves the following comments:
I'd prefer to see a generate_csrf method than a CSRF flag.
It'd be better to use instead of require the 2 modules.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Marc <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Move calls to WWW::CSRF to Koha::Token.
Send a safe random string to WWW::CSRF instead of letting CSRF make a
blocking call to Bytes::Random::Secure. If your server has not enough
entropy, opac-memberentry will hang waiting for more characters in
dev/random. Koha::Token uses Bytes::Random::Secure with the NonBlocking
flag.
Test plan:
[1] Do not yet apply this patch.
[2] If your server has not enough entropy, calling opac-memberentry may
take a while. But this not may be the case for you (no worries).
[3] Apply this patch.
[4] Verify that opac-memberentry still works as expected.
[5] Run t/Token.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Yes, my server had entropy trouble (reason for finding the problem).
This patch resolves the delay.
Tested all 3 patches together, works as expected.
Signed-off-by: Marc <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mason James <mtj@kohaaloha.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 42460b8714)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
To recreate:
/cgi-bin/koha/patroncards/edit-template.pl?op=edit&element_id=23%20and%201%3d2+union+all+select+1,user(),@@version+--%20
Look at the Profile dropdown list.
To fix this problem and to make sure it does not appears anywhere else
in the label and patroncards modules, I have refactored the way the
queries are built in C4::Creators::Lib
Now all of the subroutine takes a hashref in parameters with a 'fields'
and 'filters' parameters.
From these 2 parameters the new internal subroutine _build_query will
build the query and use placeholders.
Test plan:
1/ Make sure you do not recreate the vulnerability with this patch
applied.
2/ With decent data in the labels and patroncards modules, compare all
the different view (undef the New and Manage button groups) with and
without this patch applied.
=> You should not see any differences.
This vulnerability has been reported by MDSec.
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit a70980d825)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
The code is duplicated, variable are not set ($_), code is hard to read,
not covered by tests and the subroutine has 2 completely different
behaviors depending on the presence of the "module" parameter.
No need more ti rewrite it.
Test plan:
- Use koha_perl_deps.pl with the different options (-u -m -a -i)
- Go on the about page, "Perl modules" tab
You should not see any differences from before and after this patch
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mason James <mtj@kohaaloha.com>
to test patch...
1/ set date between 00:00 and 00:59
$ sudo date -s 'Sun Jan 29 00:41:55 NZDT 2017'
2/ run prove, see fail
$ prove -v t/db_dependent/Members/IssueSlip.t
...
t/db_dependent/Members/IssueSlip.t (Wstat: 65280 Tests: 1 Failed: 0)
Result: FAIL
3/ apply patch
4/ run prove, see pass
$ prove -v t/db_dependent/Members/IssueSlip.t
...
All tests successful.
Result: PASS
The directory it attempts to create an xml file may not be writable for
the user running the test. By changing the directory from the current
directory to a temporary one, the test runs. After all 'chmod 777
t/db_dependent' is a bad idea.
TEST PLAN
---------
1) sudo koha-shell "prove t/db_dependent/Sitemapper.t" kohadev
-- fails
2) apply patch
3) sudo koha-shell "prove t/db_dependent/Sitemapper.t" kohadev
-- succeeds
4) run koha qa test tools
Tested without qa tools
Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 7401d9422b)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
UNIMARC inserts field 100. This interferes the field count and order
in the test.
Note: This is a quick fix. Will polish it after bug 17913.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
EDIT:
Adjusted three small typos that did not disturb the test in its current
form, but do when we are fixing bugs on bug 17913.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>
No spectacular things:
[1] Move the framework modifications to a sub. Use same style to create auth types and linked fields.
[2] Change some new Object occurrences to Object->new.
[3] Add tests for field count and field order in the first two subsets.
[4] Few whitespace changes (sorry) and comment lines.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>
Originally aimed for bug 9988. Adjusted in line with other subtests.
Will polish the three subtests a little more on a follow-up.
Test plan:
Run t/db_dependent/Authorities/Merge.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>
Based on original patch from Maxime Beaulieu on bug 11315.
Amended by Marcel de Rooy on report 17909.
EDIT:
Original tests have been adjusted in view of:
[1] Test on bug 11315 heavily leaned on DBD::Mock. Since we are
using Test::DBIx::Class on such tests now, this would need attention.
Moreover, the advantage of mocking the database in this case is at
least arguable.
[2] Matching the first (somewhat older) subtest of 11700.
[3] Simplification and readability.
Look e.g. at the use of $MARCto and $MARCfrom on 11315.
This made me merge them in the db_dependent counterpart.
Also note that this subtest adds another needed test case: the merge from
auth1 to modified auth1, while 11700 tested auth1 to auth2.
Test plan:
Just run t/db_dependent/Authorities/Merge.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>
Original patch from Julian Maurice on bug 11700.
With sign offs by:
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Amended by Marcel de Rooy on report 17909.
EDIT (January 2017):
Removed some tests not related to merge.
Put remaining tests in a subtest, made them working on current merge.
Slightly revised the mocking.
Note: I plan to move the zebra retrieval stuff outside merge in one of
the next stages, and replace it by calling Koha::SearchEngine. This will
reduce mocking complexity here.
Test plan:
Just run t/db_dependent/Authorities/Merge.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 2321ae0d79)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
EDIT (from Marcel):
Previous patch fixes issue caused by Search.t.
This patch still adds some small changes to check_sysprefs.t.
Signed-off-by: Grace McKenzie <grace.mcky@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>
The config tweaks from Search.t break check_sysprefs.t later on.
Clearing the cache resolves that.
Test plan:
[1] Run prove t/db_dependent/Search.t t/db_dependent/check_sysprefs.t (in
one statement) before and after applying this patch.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
A prove t/db_dependent did not fail check_sysprefs.t (only skipping two
tests: 00-strict.t and Koha/IssuingRules.t; not enough patience)
Signed-off-by: Mason James <mtj@kohaaloha.com>
Signed-off-by: Oliver Bock <oliver.bock@aei.mpg.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>
In the previous patch we use the constraint_exists subroutine to verify
if an index or a foreign key exists.
But the `SHOW INDEX` query does not return foreign keys (as its name
suggests!).
We need another subroutine foreign_key_exists to check the FK existence.
I have found that because t/db_dependent/TestBuilder.t fails on
oai_sets_biblios, because oai_sets_biblios_ibfk_1 has not been removed.
Test plan:
0/ Do not apply this patch
1/ Use a 3.20 DB
2/ update the DB
3/ SHOW CREATE TABLE oai_sets_biblios
will display oai_sets_biblios_ibfk_1
Apply the patch and repeat 1, 2, 3
=> Will not display oai_sets_biblios_ibfk_1
It has been removed as expected.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 05fdd855c8)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 435b7badc2)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
This adds two tests to t/db_dependent/Installer.t
TEST PLAN
---------
1) Apply patch
2) prove -v t/db_dependent/Installer.t
-- column and constraint tests were added.
3) run koha qa test tools
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 8938631f38)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
For no discernable reason, when AddIssue calls AddRenewal, it passes the
branchcode generated from _GetCircControlBranch. Assume
_GetCircControlBranch is set to return items.homebranch. So:
1) If an item owned by LibraryA is checked out at LibraryB, the
statistic line branchcode will be LibraryB
2) If an item is renewed via the ajax datatables renewal function, the
statistic line branchcode will be LibraryB the
3) If an item is renewed via scanning the item into the checkout again,
statistic line branchcode will be *LibraryA*
This is clearly improper behavior. The renewal is taking place at
LibraryB, so the branchcode passed to AddRenewal should be LibraryB,
the logged in library. This also jives with the documentation for
the subroutine.
Test Plan:
1) Set CircControl to "the library the item is from" aka ( ItemHomeLibrary )
2) Set HomeOrHoldingBranch to 'The library the items is from" ( aka homebranch )
3) Create item with homebranch of LibraryA and holdingbranch of LibraryB
4) Set the logged in library to LibraryB
4) Check the item out to a patron at LibraryB
5) Note the statistics line has a branchcode of LibraryB
6) Check the item out again to trigger a renewal, renew the item
7) Note the statistic line has a branchcode of LibraryA!
8) Apply this patch
9) Repeat step 6
10) Note the statistics line has a branchcode of LibraryB!
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: David Kuhn <kuhn@monterey.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mason James <mtj@kohaaloha.com>
Moreover we do not need to remove the existing issues and patrons
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>
Moves the getting of testing date for updated_on just after the storing
the test patron data to make the gap between generating data and now
date as short as possible
Fixes test 7
Use Koha::Database instead of C4::Context->dbh
Test plan
1. prove t/db_dependent/Patrons.t
2. read the diff
NOTE: Only minor improvement could be using test builder to
generate the category and branch codes, rather than assume
data exists. However, that is beyond scope of this bug.
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 0747a11b26)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Conflicts:
t/db_dependent/Koha/Patron/Categories.t
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>
Yes, we are fixing these four typos here.
Test plan:
[1] Read the changes.
[2] Run t/Auth_with_shibboleth.t
[3] Run git grep recieved| grep -v -e 'recievedlist' | grep -v -e 'serials-recieve.tt'
Note: serials-recieve.tt is just history. Bonus points for the one who makes
us get rid of that column recievedlist.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
EDIT:
Rebased. Change in Accounts has been corrected already.
Removed the po file.
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit b5c1a965b1)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
(cherry picked from commit 327c2ba636)
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
This patch introduces a regression test for exception_holidays. This routine
returns a list of datetimes to be used in date comparison and some datetimes don't exist
in some timezones, so floating timezones should be used instead.
To test:
- Apply the patch on master
- Run:
$ prove t/db_dependent/Holidays.t
=> FAIL: The new test fails
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit c6a0848ff0)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
(cherry picked from commit aa5c67486f)
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
When doing a search, with SearchHistory pref switched on:
DBD::mysql::st execute failed: Column 'time' cannot be null [for Statement "
INSERT INTO search_history(
userid, sessionid, query_desc, query_cgi, type, total, time
) VALUES(
?, ?, ?, ?, ?, ?, ?
)
" with ParamValues: 0="7874", 1='12338a0bd8da63e32c79ee84f8493a07', 2="kw,wrdl: d", 3='q=d', 4='biblio', 5=6104, 6=undef] at C4/Search/History.pm line 34.
Since MySLQ 5.7, undef does not mean "the default value".
To let the DBMS set the default value (CURRENT_TIMESTAMP), the parameter should not be passed
Test plan:
OPAC+Intranet, swith the 2 SearchHistory syspref on
Do a search
=> Without this patch, you will get an error in the log and the row will not be inserted in the table.
=> With this patch, everything must go fine
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Not verified with MySQL 5.7. But this looks good to me.
Works under Jessie.
Signed-off-by: Mason James <mtj@kohaaloha.com>
When a ccl query is used, the buildQuery subroutine does not handle
the available limit (not an index).
This available limit is handle later in the subroutine.
This affect the author links on the detail page for instance (an=xx).
A much better solution would be to keep an 'available' zebra index up-to-date.
Test plan:
(OPAC or staff interface, it does not matter)
- Launch a search, click on a result and then on an author link to
launch another query (an:xx)
- Limit to available items without the 'facet'
=> Without this patch you won't get any results
=> With this patch applied you should get relevant result (regarding the
known bugs 16970, 13715, 13658, 5463, etc.)
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>
This patch makes the tests clean the 'config' namespace for cache before
it runs any tests.
To test:
- Run:
$ prove t/db_dependent/Search.t
=> FAIL: Self explanatory
- Apply the patch
- Run:
$ prove t/db_dependent/Search.t
=> SUCCESS: All green
- Sign off :-D
Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
All test pass successfuly
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Cannot reproduce, but I trust everyone telling me it's failing ; )
No regressions spotted
Signed-off-by: Mason James <mtj@kohaaloha.com>
Marcel de Rooy
Jonathan Druart
David Cook
Mason James
Mark Tompsett
Julian Maurice
Josef Moravec
mbeaulieu
Alex Arnaud
Kyle M Hall
Tomas Cohen Arazi