Commit graph

1690 commits

Author SHA1 Message Date
e9e5f3d380 Bug 19569: Set X-Frame-Options=SAMEORIGIN - opac-showmarc.ok
Before and after:
wget 'http://catalogue.kohadev.org/cgi-bin/koha/opac-showmarc.pl?id=1&viewas=html'
must be the same

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-01-09 17:23:14 -03:00
2ba4af723c Bug 19319: Only fetch the record if it exists
We already know if the bibliographic record exists (404 redirect),
we can avoid unecessary fetches

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-01-09 16:02:25 -03:00
950fc8e101 Bug 19319: Reflected XSS Vulnerability in opac-MARCdetail.pl
Try going to this URL on your site: /cgi-bin/koha/opac-MARCdetail.pl?biblionumber=2"><TEST>

Test Plan:
1) Go to /cgi-bin/koha/opac-MARCdetail.pl?biblionumber=2"><TEST>
2) Note <TEST> is embedded all over the html
3) Apply this patch
4) Refresh the page, note the injection is gone!
5) run koha qa test tools

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-01-09 16:02:25 -03:00
3a5534fcf5 Bug 19304: Move C4::Members::GetNoticeEmailAddress to Koha::Patron->notice_email_address
This subroutine is quite trivial and can be replaced easily with a new
method of Koha::Patron

Test plan:
Overdue notices and shelf sharing must be send the to an email address,
according to the value of the pref AutoEmailPrimaryAddress

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-01-02 11:46:40 -03:00
9782fa1846 Bug 19303: Move C4::Members::GetFirstValidEmailAddress to Koha::Patron->first_valid_email_address
This new Koha::Patron->first_valid_email_address already exists, it
should be called directly

Test plan:
- With a logged in user who have an email address defined, send a basket.
The email should be marked sent by this email address

- On the "Holds awaiting pickup", the email address must be displayed as well

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-01-02 11:46:40 -03:00
c78746d40d Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed
This patchset move The OPACItemHoldsAllowed logic
(issuingrules.opacitemholds) to a new class method of
Koha::IssuingRules: get_opacitemholds_policy

On the way, this patch will certainly fix the same problem as bug
19298 with onshelfholds.

Test plan:
Make sure the opacitemholds policy is correct when placing a hold at the
OPAC or the staff interface.

Followed test plan which worked as described
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-01-02 11:46:39 -03:00
325fddd8e9 Bug 19845: Do not generate a password if one is filled on selfreg
Bug 15343 allows patrons to choose their own passwords during self-registration.
But this does not work if the PatronSelfRegistrationVerifyByEmail preference is
set to "Require." If e-mail verification is required, whatever password the patron
supplied in the form is ignored, and they are given a randomly generated password
once they click on the verification link.

Test plan:
1. Make sure there is a valid e-mail stored in the KohaAdminEmailAddress preference.
2. Set PatronSelfRegistration to Allow.
3. Set PatronSelfRegistrationVerifyByEmail to Require.
3. Be sure "password" is listed in PatronSelfRegistrationBorrowerMandatoryField and
NOT listed in PatronSelfRegistrationBorrowerUnwantedField.
4. Be sure there is a valid patron category in PatronSelfRegistrationDefaultCategory.
5. Set PatronSelfRegistrationPrefillForm to "Display and prefill" so that you can see
the generated password.

Then fill out the self-registration form, include a valid e-mail address, and select
a password. Wait for the verification e-mail. Click on the link and you'll see that
the password you entered in the form is used.

Signed-off-by: Arturo <alongoria@sll.texas.gov>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-12-21 13:07:54 -03:00
11b562e0d1 Bug 19843: (bug 15839 follow-up) Set reviews.datereviewed when the review is made
Caused by
  commit 092ae340ce
    Bug 15839: Koha::Reviews - Remove savereview

the datereviewed=now() part of the query has not been translated.

Test plan:
Add a review from the OPAC
Confirm that the reviews.datereviewd has been correctly set. Without
this patch it is set to NULL.

I have no idea how we could update existing data :-/

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-12-21 11:07:51 -03:00
7f1c847aba Bug 19808: Handle deleted reviewers gracefully - opac-showreview
Test plan:
- Create reviews
- Delete some reviewers
- Enable OpacShowRecentComments
- Go to opac/opac-showreviews.pl
- Go to opac-showreviews.pl?format=rss

Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-12-21 11:07:37 -03:00
2cd52f68cb Bug 19808: (follow-up) Handle deleted reviewers gracefully - opac-detail
Make the tests easier to read

Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-12-21 11:07:37 -03:00
Victor Grousset
12882d824e Bug 19808: Handle deleted reviewers gracefully - opac-detail
And other display issues when the patron was NULL.
Which allows to keep the review even if it has no patron.
Because it might be useful.

For example when disconnected, the borrowernumber is null. So the
comments from deleted patrons were displayed as if the disconnected
user wrote them. So it had the edit button...

And fix borrowernumber not being passed to the template when
OpacStarRatings was false.

Test plan
1. Log in as a patron
2. Leave a comment/review on a record
3. Librarian: approve this comment
4. Delete the borrower
5. See the record (opac:/cgi-bin/koha/opac-detail.pl?biblionumber=RELEVANT_BIB_NUMBER)
6. Then you should see an error
7. Apply this patch
8. Refresh the page
9. Then you should see the record page with the comment

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-12-21 11:07:37 -03:00
ad2bd6a36d Bug 19439: (follow-up) Remove four spaces from lines 133-149
Speaks for itself. No test plan.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-12-11 11:34:24 -03:00
e6a4978f19 Bug 19439: Remove eval with exit statements from opac/unapi
The script contains some error responses that get lost when using them
in an eval statement. (Since exit should not be used within an eval
construction.) The eval is removed now.

Test plan:
[1] Before applying this patch, you could look at the current responses
    from unapi for the calls in the next steps. Apply the patch now.
[2] Call http(s)://[your-server]/cgi-bin/koha/unapi with only a format
    parameter.
    Check if you have a 400 status response. (In Firefox, look at Network
    tab of Developer Tools.)
    Note: Have to admit (reluctantly) that MS Edge shows the status code in
    the browser rightaway in contrast with Firefox.
[3] Call unapi with a format=marcxml&id=999
    The wrong id parameter should trigger a 404 response.
[4] Call unapi with format=marcxml&id=koha:biblionumber:[notexist]
    where notexist is a biblionumber that not exists.
    This should trigger again a 404 response.
[5] Call unapi with format=marcxmlx&id=koha:biblionumber:[exist]
    where exist should be a good biblionumber.
    The wrong format should trigger a 406 response.
[6] Bonus: The 500 response can be tested by manipulating a XSLT file.
    Create invalid xml in the file for marcxml (identity.xsl in intranet
    xslt folder).
    Call unapi with format=marcxml&id=koha:biblionumber:[exist].
    You should get a 500 response and have warnings in your logfile.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-12-11 11:34:24 -03:00
2d10327e98 Bug 19496: ReplytoDefault is a pref, not a C4::Context method
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-12-07 10:52:28 -03:00
Aleisha Amohia
d5292616ac Bug 19496: Fixing to_address for patron notes
This patch ensures the patron note to_address is the branch email or the
Koha Admin email, and the from_address is the patron's email.

To test:
1) Ensure syspref 'allowcheckoutnotes' is turned on
2) Go to OPAC, leave a note on a checkout
3) Confirm that the note is received (in mysql message_queue) but the
to_address is the patron's email - this is wrong
4) Apply patch and refresh OPAC
5) Leave a note on a checkout
6) Confirm that the note is received in message queue
7) Confirm that the to_address is one of:
- branch email
- branch reply to
- syspref ReplytoDefault
- syspref KohaAdminEmailAddress
8) Confirm that the from_address is one of:
- patron email
- patron emailpro
- patron B_email

Sponsored-by: Catalyst IT

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-12-07 10:26:53 -03:00
Victor Grousset
14f8bce9ae Bug 12497: Fix OPAC search history reachable by URL even when disabled
Test plan:
1. Set EnableOpacSearchHistory syspref to "Keep"
2. Make a search in the OPAC
3. Go to /cgi-bin/koha/opac-search-history.pl
4. Set the EnableOpacSearchHistory syspref to "Don't keep"
5. Refresh the OPAC page to show that history is still accessible
6. Apply the patch
7. Refresh the OPAC page, you should end on the 404 page
8. Set EnableOpacSearchHistory syspref to "Keep"
9. Go to /cgi-bin/koha/opac-search-history.pl which should be reachable

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-12-07 10:09:52 -03:00
Victor Grousset
edc8031f4a Bug 12497: Fix search history non-accessible when OPAC was private
(when the user didn't have the "borrowers" permission)

Test plan:
1. Set syspref "OpacPublic" to false (Disable)
2. Log in as a user without any permission
3. Try to access the search history. The link is next to "Log out"
4. Then you should see "Access denied"
5. Apply this patch
6. Refresh the page
7. Then you should see the search history

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-12-07 10:09:52 -03:00
1beb65ed4f Bug 19640: fix OPAC IdRef webservice display
The OPAC webservice IdRef display was broken.
The call returns results but citations where empty.
Maybe the webservice has changed.

This patch corrects the fetch of datas in result.
Also fixes a small HTML missing tag tr.
Also adds a comment with link to official doc :
http://documentation.abes.fr/aideidrefdeveloppeur/index.html#MicroWebBiblio

Test plan :
1) Enable system preference IdRef
2) Choose an existing PPN like 032581270
3) Look result on : https://www.idref.fr/services/biblio/032581270.json
4) Call in OPAC website : /cgi-bin/koha/opac-idref.pl?unimarc3=032581270
5) Without patch you see only one role containing empty rows
6) With patch you see all roles with correct rows

Signed-off-by: Simon Pouchol <simon.pouchol@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Amended: removed unused @unimarc3 (not to be confused with $unimarc3).

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-12-07 09:37:09 -03:00
06f9e5fe3a Bug 7317: Handle backend absense more gracefuly
5/ This patch makes Koha::Illrequest->load_backend raise an exception
if the passed backend is invalid. This way we will catch more errors introduced.

The patch also disables the 'New Ill request' when no backends are available. Gets
rid of a related warnings.

Both OPAC and Intranet now display a warning message when no backends
are available.

Tests are added for the load_backend changes.

4/ This patch fixes the path for the checkboxes jquery plugin, and removes the include
for tablesorter, as this implementation uses Datatables. This is obviously code for older
Koha, ported to master.

TODO: There's something wrong on the styling. My idea is to get rid
of the custom column visualization tool, and have it display as regular
DataTables. We can then introduce the use of colvis on a separate bug
report.

Note: POD coverage for the exceptions file is wrongly tested. It is a false positive.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-11-09 11:42:14 -03:00
Alex Sassmannshausen
8e86b5e093 Bug 7317: Interlibrary loans framework for Koha.
This Commit is at the heart of adding an interlibrary loans framework
for Koha.  The framework does not prescribe a particular workflow.
Instead it provides a general framework that can be extended &
implemented by individual backends whose responsibility it is to
implement a specific workflow.

The module is largely self-sufficient: it adds new tables to the Koha
database and touches only a few files in the Koha source tree.

Primarily, we add our files to the Makefile and the koha-conf.xml,
define ill paths for the REST API, and introduce links from the main
intranet, opac pages & user permissions.

Outside of this we simply add new files & functionality.

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-11-09 11:42:12 -03:00
Mark Tompsett
a6d709dcc3 Bug 19576: Remove extra 'use Koha::Biblios' statement
Marcel noticed this while QA'ing another bug.

TEST PLAN
---------
Apply patch and confirm the page still loads and works as expected.
Run Koha QA Test tools

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-11-03 12:59:10 -03:00
bc0012de2e Bug 17989: Final changes
[1] The template sco/printslip.tt is in the regular modules directory.
Should not be prefixed with a slash.

Test plan:
[1] Try to find another occurrence just like the one corrected in
    sco/printslip.pl where an absolute path is passed to gettemplate or
    get_template_and_user. I already tried several regex variations
    while git grepping template_name, but you may still find one..

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Amended: Removed the changes for svc/members/search. Not needed.
Commit message adjusted accordingly.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-11-01 13:10:18 -03:00
8c510e1a92 Bug 17989: Include full path logic in _get_template_file
Similar to the full path test in sub themelanguage, this patch makes a
change in _get_template_file. This allows you to pass a template
outside the modules directory to get_template_and_user. (Note: the sub
badtemplatecheck already blocks bad paths.)

Especially, this would be helpful for plugins using templates. As can be
seen in Templates.pm, a change was made earlier to overwrite the filename
for a plugin in sub gettemplate. This exception can now be removed.

Also note the small change in Koha/Plugin/Base.pm; mbf_path is already
absolute and if we pass a full path, we do not need it. This allows use of
a regular Koha template or a shared template between plugins (as long as
badtemplatecheck allows the path).

What are the side-effects of this change?
[1] We should not pass absolute paths if we mean relative ones.
    A follow-up patch deals with one occurrence in the codebase.
    No regressions for regular use.
[2] Plugins can call get_template_and_user directly or go via get_template
    in Koha/Plugin/Base (absolute paths don't go via mbf_path).

Note: replaced two single quotes in Auth.pm to show template name in test
description.

Test plan:
[1] Open some page on OPAC or staff client to trigger a template.
[2] Run t/db_dependent/Auth.t to verify not allowing some bad templates.
[3] Run t/db_dependent/Templates.t to verify an absolute path.
[4] Run t/db_dependent/Plugins.t to verify using templates in a plugin.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-11-01 13:10:17 -03:00
Aleisha Amohia
3fe241f610 Bug 19212: Preventing warns when asking for discharge
To test:
1) Ensure the useDischarge syspref is enabled
2) Log in to OPAC
3) Go to 'ask for a discharge'
4) Notice warns
5) Apply patch and refresh page
6) Notice warns are gone

Sponsored-by: Catalyst IT

Warnings reproduced in plack-error.log, with patch they are gone.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-27 16:05:55 -03:00
Marc Véron
3df0c9ac70 Bug 13796: OPAC: Display rental charge for renewing
This patch displays the rental fee that will be applied for
renewing (near "Renew" link on patron's summary page)

To test:
- Apply patch
- Enable system preference 'OpacRenewalAllowed'
- Verify that you have item types with and without rental fee
  for testing
- Issue items with an without rental fee to a patron
- In OPAC, display patron's summary page
- Verify in table 'Checked out', items with rental charge display an information as
  appropriate (near the link 'Renew')

Patch rewritten because of merge issues with previous patches. 2017-04-21 mv

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-27 16:05:51 -03:00
Aleisha Amohia
506d669424 Bug 2093: (follow-up) Add OPAC dashboard for logged-in users
This patch:
- hides the dashboard if there is no dashboard information to display
- changes '5.00 due' to '5.00 due in fines and charges' for translation
- uses Koha::Holds in place of deprecated C4::Reserves methods

To test, confirm all the right information for holds still shows, and
confirm the dashboard is hidden if there are no checkouts, holds, fines
or overdues.

Signed-off-by: Hugo Agud <hagud@orex.es>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-27 14:09:05 -03:00
Aleisha Amohia
657c417a87 Bug 2093: Add OPAC Summary for logged-in users
This patch adds a summary to the OPAC once the user has logged in that
shows the users number of checkouts, overdues, holds pending, holds
waiting and total fines. We also have a syspref OPACUserSummary to turn
this feature on and off. Default is ON.

To test:
1) Apply patch and update database
2) Set up some checkouts, overdues, holds pending AND waiting and fines
for a user
3) Log into OPAC as that user, see summary. Confirm links all work as
expected
4) Confirm that if there are no checkouts / overdues etc that the link
disappears from the summary
5) Turn OPACUserSummary OFF and confirm the summary does not show on the
mainpage.

Sponsored-by: Catalyst IT

Signed-off-by: Hugo Agud <hagud@orex.es>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-27 14:09:04 -03:00
Srdjan
bf6caa81e5 Bug 16660: Moved Opac Supression filtering from opac-search.pl to Zebra::QueryBuilder
To test:
OPAC: Both SearchEngine "Elasticsearch" and "Zebra" should work with
OpacSuppression set to "yes"

NB: OPAC suppression is not implemented for Elasticsearch

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-27 14:09:02 -03:00
27b99bb80b Bug 18118: Unexpected behaviour with 'GoogleOpenIDConnect' and 'OpacPublic' syspref combination.
TEST PLAN
----------
1/ configure a working 'GoogleOpenIDConnect' account

See comment #5 which also links back to
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16892#c3

2/ set 'OpacPublic' (under OPAC) to 'Disabled' and
   'GoogleOpenIDConnect' (under Administration) to 'Yes'.

3/ log in user successfully via google-auth, observe redirect to
   opac-user.pl (bad)

4/ apply patch
   -- on kohadevbox remember to restart all! Plack is unforgiving. :)

5/ log in user successfully via google-auth, observe expected
   redirect to opac-main.pl (good)

While I would normally suggest running koha qa test tools, because
this file doesn't end in .pl, it doesn't get picked up by them.

6/ perlcritic -4 opac/svc/auth/googleopenidconnect
   -- notice this is a level better than required. :)

This also eyeballs easily well.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-27 14:09:02 -03:00
Mark Tompsett
c4113dce70 Bug 18956: Fix empty to in message queue
Follow the test plan in comment #20.
Also tweaked string, because it was really 'or' before too.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Amended text in added comment.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-27 13:57:10 -03:00
6336e53aed Bug 18956: (QA follow-up) Resolve a CGI::Param in list context warn
From the plack-error.log:
CGI::param called in list context from package CGI::Compile::ROOT::usr_share_koha_masterclone_opac_opac_2dpassword_2drecovery_2epl line 129, this can lead to vulnerabilities. See the warning in "Fetching the value or values of a single named parameter" at /usr/share/perl5/CGI.pm line 436.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-27 13:56:59 -03:00
Mark Tompsett
e08a0afa26 Bug 18956: Prevent leaking during password recovery
TEST PLAN
---------

It is assumed you have set the OpacResetPassword to 'allowed',
and likely in combination with OpacPasswordChange to 'Allowed'.

You will have two patrons: one with and another without
any email address entered. You will want to test this test plan
with both patrons.

$ git checkout -b bug_18956 origin/master

Prepend the following as understood between step sections:
opac -> forgot password and then enter...

correct login/cardnumber, it will email
delete from borrower_password_recovery;

correct email, it will email
delete from borrower_password_recovery;

correct login/cardnumber && correct email, it will email
delete from borrower_password_recovery;

wrong login/cardnumber && correct email, error page as expected
delete from borrower_password_recovery;

correct login/cardnumber && wrong email, error page as expected
delete from borrower_password_recovery;

wrong login/cardnumber && wrong email, error page as expected
delete from borrower_password_recovery;

submit empty -- INTERNAL SERVER ERROR?!
delete from borrower_password_recovery;

-- None of the above step sections displayed email.

correct login/cardnumber, it will email

correct login/cardnumber again, but it leaks email address!
delete from borrower_password_recovery;

correct email, it will email

correct email again, but it leaks login/cardnumber!
delete from borrower_password_recovery;

$ git bz apply 18956
-- choose interactive, and choose this counter patch.

repeat the same test set again
-- no leaks will occur, error message pages returned should
   be reasonable, code should read reasonably.

run koha qa test tools.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-27 10:59:05 -03:00
e711c8e418 Bug 19038: Remove the OPACShowBarcode syspref
This patch removes the OPACShowBarcode syspref in favour of the new
columns settings option introduced by bug 16759.

On the upgrade step, it picks the value for OPACShowBarcode and uses it
to populate the columns_settings table.

To test:
- Verify the upgrade process maintains the current behaviour

Regards

Sponsored-by: Dover

Followed test plan and works as expected. Functionality of patch from bug 16759
appears intact too.
Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-25 16:12:46 -03:00
662a98345a Bug 19028: Add 'shelving location' to holdings table in detail page
This patch adds the option to show shelving locations on a separate
column. This is controlled by a new syspref, 'OpacLocationOnDetail',
which replaces 'OpacLocationBranchToDisplayShelving', adding a
conveniente 'column' option.

The new 'Shelving location' column is conveniently added to the columns
configuration entry added by bug 16759 for this purpose.

The current behaviour is preserved.

To test:
- Apply this patches
- Run the upgrade:
  $ sudo koha-shell kohadev
 k$ cd kohaclone
 k$ perl installer/data/mysql/updatedatabase.pl
=> SUCCESS: Upgrade doesn't fail
- Have an item with shelving location set to something not void
- Have the item set home and holding libraries for testing purposes.
- Set 'OpacLocationBranchToDisplay' to 'home and holding libraries' [*]
- Visit the OPAC detail page for the record containing the item
=> SUCCESS: Both home and holding libraries are displayed.
- Loop through all OpacLocationOnDetail options (except from 'column', we leave it for later).
=> SUCCESS: Works as expected.
- Go to Administration > Columns settings
- Make item_shelving_location available in the OPAC section
- Reload the OPAC detail page
=> SUCCESS: No change
- Set OpacLocationOnDetail to 'on a separate column'
- Reload the OPAC detail page
=> SUCCESS: Shelving location is displayed on a separate column
- Sign off :-D

Sponsored-by: Dover

[*] For testing purposes

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-25 12:14:41 -03:00
db14275db4 Bug 19298: Placing a hold from a list at the OPAC should respect issuing rules
The issuing rule retrieve to know if a hold can be placed on a record of
a list is not correct.

Test plan:
0/ With item-level_itypes = item level
1/ Define a item.itype=BK and biblioitems.itemtype=CF
2/ Create a default rule to allow on shelf holds
3/ Create a specific rule for CF with on shelf holds="If any
unavailable"
4/ Add this bibliographic record to a list and view the list
=> Without this patch you will not see "Place hold"
=> With this patch applied you will see the "Place hold" button,
respecting the correct issuing rule

Followed test plan, patches worked as described.

Note: Just to clarify the test plan slightly in step 4 where it says you will not see 'Place Hold' it means to
the left of the 'Save to another List' link below the item availability
in the opac-shelves.pl page. Not the 'Place hold' button in the grey
page header box.

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-25 12:14:39 -03:00
3f9da34683 Bug 18298: Add server-side checks and refactor stuffs
Now that we have a check client-side, nothing prevents us from a smart guy to
bypass it and force an invalid password.
This patch adds two new subroutines to Koha::AuthUtils to check the
validity of passwords and generate a password server-side. It is used
only once (self-registration) but could be useful later.

Moreover the 3 different cases of password rejection (too leak, too
short, contains leading or trailing whitespaces) were not tested
everywhere. Now they are!

This patch makes things consistent everywhere and clean up some code.

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-16 09:44:32 -03:00
ec4e666bc5 Bug 18298: minPaswordLength should not be < 3
Indeed if RequireStrongPassword is set we need at least 3 characters to
match 1 upper, 1 lower and 1 digit.
We could make things more complicated to allow minPasswordLength < 3
but, really, 3 is already too low...

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-16 09:44:32 -03:00
f18af55a39 Bug 15685: (follow-up) K:A:O->find and ->fetch are no longer used
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-11 13:08:46 -03:00
Jesse Weaver
b29493265b Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket
This adds a new basket attribute (create_items) that can optionally be
set to override AcqCreateItem.

The following have been modified to reflect this (with the value of
create_items that causes them to behave differently in parentheses):
  * Cancelling receipt of an order (receiving)
  * Creating an order by hand or from MARC (ordering)
  * Receiving an order (receiving)
  * Showing orders with uncertain price (ordering)
  * Showing orders (receiving)
  * Showing acquisition details in the OPAC (ordering)

Test plan:
  1) Create baskets with "Create items when:" set to ordering,
     receiving, cataloging and unset.
  2) Test each of the above for each of these baskets, verifying that
     the basket-specific attribute overrides AcqCreateItem if set and
     falls back to the syspref otherwise.

NOTE: A check of AcqCreateItem in opac-detail.tt was removed because it
was redundant; the code path in question cannot be triggered unless
create_items/AcqCreateItems is set to the correct value anyway.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Barbara Fondren <bfondren@roundrocktexas.gov>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-11 13:06:06 -03:00
fed86d5042 Bug 19440: Existing calls need to be done in scalar context
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-09 16:15:52 -03:00
Aleisha Amohia
69ee06a490 Bug 13205: [FOLLOW-UP] Fixing math and variable names
Have changed
  my $last_page = $pages * ( $results_per_page - 1 );
to
  my $last_page = ( $pages - 1) * $results_per_page;
which seems to fix the 'last' button offset! (Comment 10)

Will add the box to jump to a page in a separate patch.
Adding the pagination to the top on the staff client will be dealt with
in Bug 18916 as it is slightly out of the scope of this bug.

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-09 16:15:49 -03:00
Aleisha Amohia
5fc0a1f5b1 Bug 13205: [FOLLOW-UP] Some extra logic
See Comment 8.

Test:
When on first page of results, confirm that the 'First' and 'Previous'
buttons do not show. Confirm they come back on the second page and every
page after.
When on last page of results, confirm that the 'Last' and 'Next' buttons
do not show. Confirm they come back on all previous pages.
Check on both staff side and OPAC.

Sponsored-by: Catalyst IT

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-09 16:15:49 -03:00
Aleisha Amohia
1a157575d3 Bug 13205: Last/first page options for result list paging for OPAC
Same fix but on OPAC side. Same test plan

Sponsored-by: Catalyst IT

Signed-off-by: Dilan Johnpulle <dilan@calyx.net.au>
Signed-off-by: Your Full Name <your_email>

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-09 16:15:49 -03:00
David Bourgault
078eb77451 Bug 19345: Add missing error flag to template
Adds a missing error flag to the template->param { } call.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-06 12:09:55 -03:00
Aleisha Amohia
89528af3b0 Bug 16463: Replace discharge link with error message if user has checked out items
To test:
1) Ensure the useDischarge syspref is enabled
2) Check out an item to a borrower
3) Log in to the OPAC as this borrower
4) Click the 'ask for a discharge' link in the nav
5) Click the 'Ask for a discharge' link
6) Notice you cannot be discharged because you have checkouts
7) Apply the patch, click the 'ask for a discharge' link in the nav
8) Notice the link has been replaced with an appropriate error message
9) Attempt to force the discharge URL:
/cgi-bin/koha/opac-discharge?op=request
10) Notice the message and you cannot be discharged.
11) Confirm that when you check in your item, the discharge link shows
again and works as expected.

Sponsored-by: Catalyst IT
Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-09-29 12:37:00 -03:00
ae02cf97e4 Bug 19366: Do not block patron's detail update if EmailMustBeUnique
If the pref PatronSelfRegistrationEmailMustBeUnique is set ("consider"),
a patron is not allowed to register with an existing email address.
The existing code is wrong and reject a patron that is updating their
personal details with "This email address already exists in our
database.", even if the patron did not modify their email address.

This is caused by the query we made, we must search for patron with this
email address but who is not the current patron.

Test plan:
- Set PatronSelfRegistrationEmailMustBeUnique to "consider"
- Register a new patron with an existing email address
=> you should not be allowed
- Use a non-existent email address
=> You should be allowed
- Edit your patron details
- Modify some infos
=> Should pass
- Modify your email address with an existing one
=> You should not be allowed to do that

Followed test plan, patches worked as described
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-09-29 12:36:59 -03:00
402c7f7567 Bug 19173: Add opac payment and marc conversion plugins to the pulldown filter list
Edit: fixed tab-for-space errors (tcohen).

Signed-off-by: Magnus Enger <magnus@libriotech.no>
New categories are added to the pulldown and work as expected.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-09-19 14:15:52 -03:00
323b1553b0 Bug 19173: Make OPAC online payments pluggable
While PayPal is fairly universal, there is a plethora of online
payment system that are far more localized, servicing a single
country ( e.g. Bug 18968 ) or even a single  city! Instead of
adding support for each and every one of these payment options
directly into Koha, it makes more sense to add the ability to
create online payment plugins.

Test Plan:
1) Apply this patch
2) Download and install the Kitchen Sink plugin version 2.1.1 or later
   https://github.com/bywatersolutions/koha-plugin-kitchen-sink/releases
3) In the plugin options, enable the opac payments option
4) Create a patron with one or more fines
5) Log into the opac as that patron, note you now have the option
   to pay online via KitchenSink ImaginaryPay
6) Make an online payment
7) Note the payment was processed correctly

Sponsored-by: Washoe County Library System

Signed-off-by: Kyle M Hall <kyle@gmail.com>

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Awesome enhancement! I know we want to add at least one Norwegian
payment service at some point.
I followed the test plan and everything works as advertised. Turning
off the "opac payments option" makes the option dissappear cleanly
from the OPAC. I have *not* looked at the code or done any
considerations about security.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-09-19 14:15:52 -03:00
e722c0a29f Bug 18149: Move CountUsage calls to Koha namespace
After the introduction of Koha::Authorities->get_usage_count with bug
9988, we can now replace the remaining occurrences of CountUsage.

At the same time we remove CountUsageChildren. This was an empty sub.
The typo get_count_usage in a subtest title is adjusted.

Test plan:
[1] Run t/db_dependent/Koha/Authorities.t
[2] Perform a search on authorities-home.pl and verify that you see
    plausible numbers for 'used in xx records'.
[3] Click on Details for one authority. See the same number?
[4] Do the same as in 2/3 for Authority search on OPAC.
[5] Remember the authid and enter this in the record numbers box on
    tools/batch_delete_records.pl. Select Authorities and click
    Continue. The next form shows a column "Used in". Do you see
    the same count again?
[6] Git grep CountUsage.
    You should see just one hit in a comment that can be kept in
    Koha/Authorities.pm.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-09-19 11:47:32 -03:00
82115d164a Bug 19059: Move C4::Reserves::CancelReserve to Koha::Hold->cancel
This patch adds a new Koha::Hold->cancel method and replaces the calls
to C4::Reserves::CancelReserve with it.

Test plan:
- Add and cancel holds
- Change priority of holds

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-09-12 12:42:58 -03:00