Bug 16805 - Log in with database admin user breaks OPAC
Summary: Log in with database admin user breaks OPAC
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Mark Tompsett
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-06-23 18:58 UTC by Mark Tompsett
Modified: 2017-06-14 22:13 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 16805: Log in with database admin user breaks OPAC (3.36 KB, patch)
2016-06-23 19:01 UTC, Mark Tompsett
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 16805: Log in with database admin user breaks OPAC (3.59 KB, patch)
2016-06-23 21:29 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
Bug 16805: Add test to getalert (no param passed) (1.60 KB, patch)
2016-06-24 16:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16805: Add test to getalert (no param passed) (1.75 KB, patch)
2016-06-24 17:02 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 16805: Tweak new test message to say getalert (1.46 KB, patch)
2016-06-24 17:02 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 16805: Log in with database admin user breaks OPAC (4.09 KB, patch)
2016-06-24 17:02 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 16805: Add test to getalert (no param passed) (1.75 KB, patch)
2016-06-26 02:07 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 16805: Tweak new test message to say getalert (1.35 KB, patch)
2016-06-26 02:08 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 16805: Log in with database admin user breaks OPAC (4.10 KB, patch)
2016-06-26 02:08 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 16805: Add test to getalert (no param passed) (1.83 KB, patch)
2016-06-26 07:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16805: Tweak new test message to say getalert (1.42 KB, patch)
2016-06-26 07:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16805: Log in with database admin user breaks OPAC (4.18 KB, patch)
2016-06-26 07:08 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Tompsett 2016-06-23 18:58:16 UTC
If a user attempts to log into the OPAC with the database admin account the following is generated:

Software error:

DBD::mysql::st execute failed: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 1 [for Statement "SELECT a.*, b.branchcode FROM alert a JOIN borrowers b USING(borrowernumber) WHERE"] at /home/mtompset/kohaclone/C4/Letters.pm line 331.
For help, please send mail to the webmaster (webmaster@ubuntu), giving this error message and the time and date of the error.

A tweak to the SQL and a warning would be nice, much like the Intranet warns those logging into the staff client with the database admin account.
Comment 1 Mark Tompsett 2016-06-23 19:01:23 UTC Comment hidden (obsolete)
Comment 2 Héctor Eduardo Castro Avalos 2016-06-23 21:29:33 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2016-06-24 16:04:59 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2016-06-24 16:06:52 UTC
I'd not recommend to give such info on the public interface (You never know what might happen with students).
Of course Koha is open source and we could trace back the error message, but... just in case :)
Comment 5 Mark Tompsett 2016-06-24 17:02:13 UTC Comment hidden (obsolete)
Comment 6 Mark Tompsett 2016-06-24 17:02:31 UTC Comment hidden (obsolete)
Comment 7 Mark Tompsett 2016-06-24 17:02:46 UTC Comment hidden (obsolete)
Comment 8 Mark Tompsett 2016-06-24 17:08:34 UTC
(In reply to Jonathan Druart from comment #4)
> I'd not recommend to give such info on the public interface (You never know
> what might happen with students).
> Of course Koha is open source and we could trace back the error message,
> but... just in case :)

Valid point, but who is going to hand out their database admin account to students?! This is more likely to be administrators (like me) being unwise and logging in with the database admin account. This, however, will be a huge security issue if some administrator somewhere has done this or will do this. So, I could understand QA wanting to revert the opac file changes in the third patch.
Comment 9 Jonathan Druart 2016-06-25 14:02:00 UTC
There is not point to catch the mysql error in the tests. It is going to be fixed by this patchset. The eval is not needed here.

(In reply to M. Tompsett from comment #8)
> So, I could understand QA wanting to revert the opac file changes in
> the third patch.

I suggested to replace it with something lighter: "You are not using a valid user". But nevermind, I won't fight ages for that.
Comment 10 Mark Tompsett 2016-06-26 02:07:50 UTC Comment hidden (obsolete)
Comment 11 Mark Tompsett 2016-06-26 02:08:03 UTC Comment hidden (obsolete)
Comment 12 Mark Tompsett 2016-06-26 02:08:15 UTC Comment hidden (obsolete)
Comment 13 Jonathan Druart 2016-06-26 07:08:37 UTC
Created attachment 52880 [details] [review]
Bug 16805: Add test to getalert (no param passed)

TEST PLAN
---------
1) apply patch
2) prove t/db_dependent/Letters.t
   -- fails
3) run koha qa test tools

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 14 Jonathan Druart 2016-06-26 07:08:44 UTC
Created attachment 52881 [details] [review]
Bug 16805: Tweak new test message to say getalert

It said 'addalert', which is likely a cut-paste typo.
Plus, let's keep running the other tests if that one
goes bad.

TEST PLAN
---------
1) Apply Jonathan's test patch
2) prove t/db_dependent/Letters.t
   -- dies before finishing tests
3) Apply this test patch
4) prove t/db_dependent/Letters.t
   -- still dies before finished tests,
      'addalert' now is 'getalert' in message.
5) run koha qa test tools.

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 15 Jonathan Druart 2016-06-26 07:08:48 UTC
Created attachment 52882 [details] [review]
Bug 16805: Log in with database admin user breaks OPAC

TEST PLAN
---------
1) Apply Jonathan's test patch
2) prove t/db_dependent/Letters.t
   -- dies before finishing tests
3) Apply second test patch
4) prove t/db_dependent/Letters.t
   -- dies before finishing tests
   -- 'addalert' is changed to 'getalert'
5) Log into OPAC with database admin user.
   -- see error given in comment #0
6) Apply this patch
7) prove t/db_dependent/Letters.t
   -- says 'getalert'
   -- all tests pass.
8) Log into OPAC with database admin user.
   -- logs in, but gives warning with a nice logout button.
9) run koha qa test tools.

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
No kaha qa errors
In debian display diferent error:
Can't use an undefined value as an ARRAY reference at /usr/lib/perl5/DBI.pm line 2054.
Works as advertised

NOTE: Revised test plan, as Jonathan added useful test case.
      Works as I've tested above.
      Hector tested older test plan which was steps
      5,6,8 and 9.

      Revised test plan again while tweaking to address comment #9.

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 16 Kyle M Hall (khall) 2016-07-08 14:47:01 UTC
Pushed to master for 16.11, thanks Jonathan, Mark!
Comment 17 Frédéric Demians 2016-08-01 13:36:05 UTC
Pushed in 16.05. Will be in 16.05.02.
Comment 18 Julian Maurice 2016-08-16 15:45:11 UTC
Patches pushed to 3.22.x, will be in 3.22.10