Bug 25009 - opac-showmarc.pl allows fetching data directly from import batches
Summary: opac-showmarc.pl allows fetching data directly from import batches
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low critical (vote)
Assignee: Tomás Cohen Arazi
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-03-27 21:10 UTC by Tomás Cohen Arazi
Modified: 2020-04-27 09:48 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.05, 19.05.10, 18.11.16


Attachments
Bug 25009: Avoid leakages in opac-showmarc.pl (3.29 KB, patch)
2020-03-27 21:32 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 25009: Avoid leakages in opac-showmarc.pl (3.36 KB, patch)
2020-03-30 09:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 25009: Avoid leakages in opac-showmarc.pl (3.42 KB, patch)
2020-04-13 13:28 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 25009: [18.11.x] Add missing biblio_metadata stuffs (5.58 KB, patch)
2020-04-22 13:07 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 Tomás Cohen Arazi 2020-03-27 21:10:22 UTC
It seems, back in 2008, Joshua copied the original code from catalogue/showmarc.pl to produce the OPAC version and left that.

I haven't found any references to uses of the importid param in the repo:

18:06 $ git grep opac-showmarc.pl | grep -v release_notes
koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-MARCdetail.tt:                                <div id="switchview_div">[ <a id="switchview" href="/cgi-bin/koha/opac-showmarc.pl?id=[% biblio.biblionumber | html %]&amp;viewas=html">view plain</a> ]</div>
koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-MARCdetail.tt:                    var plain_marc = $.get( "/cgi-bin/koha/opac-showmarc.pl", { id: "[% biblio.biblionumber | html %]", viewas: "html" })


This is not a big deal and has been around for a little while already :-P
Comment 1 Tomás Cohen Arazi 2020-03-27 21:32:18 UTC
Created attachment 102020 [details] [review]
Bug 25009: Avoid leakages in opac-showmarc.pl

This patch cleans opac-showmarc.pl so it doesn't allow retrieving
records from import batches without requiring any permissions in the
OPAC.

it does so by just removing the code portion that does that.

It also cleans the record fetch operation and how the record processor
is initialized to it actually works :-D

To test:
1. Perform a successful Z39.50 search in cataloguing (this fetches 20
   records usually)
2. Query your DB for a valid import_record_id:
  $ koha-mysql kohadev
  > SELECT * FROM import_records LIMIT 1;
3. Notice some of the MARCXML details (title, author, etc), and the
   import_record_id
4. Point your browser to the opac-showmarc.pl URL like this:
   http://kohadev.mydnsname.org:8080/cgi-bin/koha/opac-showmarc.pl?importid=20
=> FAIL: You get the record! (Bonus: no field/subfield takes place)
5. Hide some obvious subfield on the framework for a known (to you)
   biblionumber
6. Point your browser to:
   http://kohadev.mydnsname.org:8080/cgi-bin/koha/opac-showmarc.pl?id=<biblionumber_here>
=> FAIL: No filtering takes place
7. Apply this patch
8. Repeat 4
=> SUCCESS: You get an error because you did a bad request (no id param)
9. Repeat 6
=> SUCCESS: Subfield filtering actually works!
10. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Jonathan Druart 2020-03-30 09:49:53 UTC
Created attachment 102076 [details] [review]
Bug 25009: Avoid leakages in opac-showmarc.pl

This patch cleans opac-showmarc.pl so it doesn't allow retrieving
records from import batches without requiring any permissions in the
OPAC.

it does so by just removing the code portion that does that.

It also cleans the record fetch operation and how the record processor
is initialized to it actually works :-D

To test:
1. Perform a successful Z39.50 search in cataloguing (this fetches 20
   records usually)
2. Query your DB for a valid import_record_id:
  $ koha-mysql kohadev
  > SELECT * FROM import_records LIMIT 1;
3. Notice some of the MARCXML details (title, author, etc), and the
   import_record_id
4. Point your browser to the opac-showmarc.pl URL like this:
   http://kohadev.mydnsname.org:8080/cgi-bin/koha/opac-showmarc.pl?importid=20
=> FAIL: You get the record! (Bonus: no field/subfield takes place)
5. Hide some obvious subfield on the framework for a known (to you)
   biblionumber
6. Point your browser to:
   http://kohadev.mydnsname.org:8080/cgi-bin/koha/opac-showmarc.pl?id=<biblionumber_here>
=> FAIL: No filtering takes place
7. Apply this patch
8. Repeat 4
=> SUCCESS: You get an error because you did a bad request (no id param)
9. Repeat 6
=> SUCCESS: Subfield filtering actually works!
10. Sign off :-D

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 3 Katrin Fischer 2020-04-13 13:28:02 UTC
Created attachment 102832 [details] [review]
Bug 25009: Avoid leakages in opac-showmarc.pl

This patch cleans opac-showmarc.pl so it doesn't allow retrieving
records from import batches without requiring any permissions in the
OPAC.

it does so by just removing the code portion that does that.

It also cleans the record fetch operation and how the record processor
is initialized to it actually works :-D

To test:
1. Perform a successful Z39.50 search in cataloguing (this fetches 20
   records usually)
2. Query your DB for a valid import_record_id:
  $ koha-mysql kohadev
  > SELECT * FROM import_records LIMIT 1;
3. Notice some of the MARCXML details (title, author, etc), and the
   import_record_id
4. Point your browser to the opac-showmarc.pl URL like this:
   http://kohadev.mydnsname.org:8080/cgi-bin/koha/opac-showmarc.pl?importid=20
=> FAIL: You get the record! (Bonus: no field/subfield takes place)
5. Hide some obvious subfield on the framework for a known (to you)
   biblionumber
6. Point your browser to:
   http://kohadev.mydnsname.org:8080/cgi-bin/koha/opac-showmarc.pl?id=<biblionumber_here>
=> FAIL: No filtering takes place
7. Apply this patch
8. Repeat 4
=> SUCCESS: You get an error because you did a bad request (no id param)
9. Repeat 6
=> SUCCESS: Subfield filtering actually works!
10. Sign off :-D

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

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 4 Joy Nelson 2020-04-17 19:09:51 UTC
Pushed to 19.11.x for 19.11.05
Comment 5 Hayley Mapley 2020-04-20 02:16:04 UTC
This patch applies cleanly to 18.11.x, however from step 6 I only get 404 errors, including after I have applied the patch. Can someone please let me know where I've gone wrong or verify for me that this patch works for 18.11.x?
Comment 6 Hayley Mapley 2020-04-20 02:22:03 UTC
(In reply to Hayley Mapley from comment #5)
> This patch applies cleanly to 18.11.x, however from step 6 I only get 404
> errors, including after I have applied the patch. Can someone please let me
> know where I've gone wrong or verify for me that this patch works for
> 18.11.x?

Sorry, 400 errors rather (when hitting those URLs)
Comment 7 Tomás Cohen Arazi 2020-04-20 02:46:46 UTC
(In reply to Hayley Mapley from comment #6)
> (In reply to Hayley Mapley from comment #5)
> > This patch applies cleanly to 18.11.x, however from step 6 I only get 404
> > errors, including after I have applied the patch. Can someone please let me
> > know where I've gone wrong or verify for me that this patch works for
> > 18.11.x?
> 
> Sorry, 400 errors rather (when hitting those URLs)

I can take a look tomorrow!
Comment 8 Jonathan Druart 2020-04-22 13:07:38 UTC
Created attachment 103474 [details] [review]
Bug 25009: [18.11.x] Add missing biblio_metadata stuffs

It adds methods, DBIC rs and Exceptions class for biblio metadata.

Note that schema is marcflavour in 18.11
Comment 9 Jonathan Druart 2020-04-22 13:07:55 UTC
Hayley, could you try again with that patch?
Comment 10 Hayley Mapley 2020-04-23 01:24:36 UTC
Thanks Jonathan, looks more as expected now!
Comment 11 Hayley Mapley 2020-04-23 01:24:56 UTC
Backported to 18.11.x for 18.11.16
Comment 12 Martin Renvoize 2020-04-27 09:46:00 UTC
Nice work everyone!

Pushed to master for 20.05