Bug 21301 - Restriction of the informations given by GetRecords ILS-DI service
Summary: Restriction of the informations given by GetRecords ILS-DI service
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Web services (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Fridolin SOMERS
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks: 18081
  Show dependency treegraph
 
Reported: 2018-08-31 09:42 UTC by abernaud
Modified: 2020-02-04 16:25 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 21301: Remove patron informations in issues for GetRecords ILS-DI service (4.99 KB, patch)
2019-10-02 15:03 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Bug 21301: Remove patron informations in reserves for GetRecords ILS-DI service (1.56 KB, patch)
2019-10-03 13:45 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Bug 21301: Remove patron informations in issues for GetRecords ILS-DI service (5.05 KB, patch)
2019-11-07 14:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 21301: Remove patron informations in reserves for GetRecords ILS-DI service (1.62 KB, patch)
2019-11-07 14:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 21301: (QA follow-up) Fix number of tests (714 bytes, patch)
2019-11-08 07:31 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description abernaud 2018-08-31 09:42:59 UTC
Actually, the GetRecords service gives many information :
bibliographic data (marcxml field)
item data (items field)
issue data (issues field)

The issue data should not be sent by these service.
For example, this service is actually used by EDS clients to display real-time item informations on EDS. Ebsco should not have access to the issues information, even if they are partially anonymised (only the firstname of the patron is sent) in 16.05 and maybe in the later versions of Koha.

This would be more compliant with GDPR (see bug 18081).
Thanks,
Anne-Claire
Comment 1 Fridolin SOMERS 2019-10-02 15:03:06 UTC
Created attachment 93495 [details] [review]
Bug 21301: Remove patron informations in issues for GetRecords ILS-DI service

Actually, the GetRecords service gives many information :
bibliographic data (marcxml field)
item data (items field)
issue data (issues field)

The issue data should not be sent by these service, its a privacy problem.
Biblio title and author are redundant.
This patch changes to use Koha::Checkouts to get issues informations with only the addition of item barcode.

Test plan :
1) Enable ILS-DI webservice
2) Checkout an item
3) Go to {opac}/cgi-bin/koha/ilsdi.pl?service=GetRecords&id={biblionumber}
4) Check the issues tag content
Comment 2 Fridolin SOMERS 2019-10-03 13:45:55 UTC
Created attachment 93603 [details] [review]
Bug 21301: Remove patron informations in reserves for GetRecords ILS-DI service

Actually, the GetRecords service gives many information :
borrowernumber (reserves field)

The issue data should not be sent by these service, its a privacy problem.

Test plan :
1) Enable ILS-DI webservice
2) Place hold on an item
3) Go to {opac}/cgi-bin/koha/ilsdi.pl?service=GetRecords&id={biblionumber}
4) Check the reserves tag content
Comment 3 Brendan Gallagher 2019-10-04 10:42:27 UTC
Failed the PODcoverage in the qa-tools tests.
Comment 4 Martin Renvoize 2019-11-07 14:28:05 UTC
False positive on the POD issue, we remove a whole routine and as such remove the corresponding POD too.
Comment 5 Martin Renvoize 2019-11-07 14:28:37 UTC
Created attachment 95158 [details] [review]
Bug 21301: Remove patron informations in issues for GetRecords ILS-DI service

Actually, the GetRecords service gives many information :
bibliographic data (marcxml field)
item data (items field)
issue data (issues field)

The issue data should not be sent by these service, its a privacy problem.
Biblio title and author are redundant.
This patch changes to use Koha::Checkouts to get issues informations with only the addition of item barcode.

Test plan :
1) Enable ILS-DI webservice
2) Checkout an item
3) Go to {opac}/cgi-bin/koha/ilsdi.pl?service=GetRecords&id={biblionumber}
4) Check the issues tag content

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize 2019-11-07 14:28:41 UTC
Created attachment 95159 [details] [review]
Bug 21301: Remove patron informations in reserves for GetRecords ILS-DI service

Actually, the GetRecords service gives many information :
borrowernumber (reserves field)

The issue data should not be sent by these service, its a privacy problem.

Test plan :
1) Enable ILS-DI webservice
2) Place hold on an item
3) Go to {opac}/cgi-bin/koha/ilsdi.pl?service=GetRecords&id={biblionumber}
4) Check the reserves tag content

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2019-11-07 14:39:03 UTC
Works as expected, checked if the online documentation needed any updating to reflect the change, but neither issues nor reservations have any details in the examples.

Looks good to me, signing off.
Comment 8 Marcel de Rooy 2019-11-08 07:17:59 UTC
(In reply to Fridolin SOMERS from comment #2)
> The issue data should not be sent by these service, its a privacy problem.

<reserve>
<suspend>0</suspend>
<biblionumber>633</biblionumber>
<timestamp>2019-11-08 07:06:05</timestamp>
<borrowernumber>5</borrowernumber>
<lowestPriority>0</lowestPriority>
<reservedate>2019-11-08</reservedate>
<priority>1</priority>
<reserve_id>110</reserve_id>
<branchcode>CPL</branchcode>
<item_level_hold>0</item_level_hold>
</reserve>

Is this really a privacy issue? In order to identify the person associated with borrowernumber 5, you'll need additional information.
So if this is just theoretical, then removing the borrowernumber might not be enough ;) If I have biblionumber and reservedata or timestamp, I might also be able to find the associated borrower?

No blocker, just pushing a bit..
Comment 9 Marcel de Rooy 2019-11-08 07:23:36 UTC
More serious:

<issue>
<issuedate>2019-11-08 07:21:08</issuedate>
<onsite_checkout>0</onsite_checkout>
<itemnumber>598</itemnumber>
<branchcode>CPL</branchcode>
<date_due>2019-11-22 23:59:00</date_due>
<barcode>perl01</barcode>
<issue_id>690</issue_id>
<renewals>0</renewals>
<timestamp>2019-11-08 07:21:08</timestamp>
<auto_renew>0</auto_renew>
<borrowernumber>5</borrowernumber>
</issue>

If you are concerned about borrowernumber in hold, then you should be for issue too. Inconsistent !
Comment 10 Marcel de Rooy 2019-11-08 07:26:14 UTC
Lowering the severity to Major
Comment 11 Marcel de Rooy 2019-11-08 07:27:10 UTC
I do expect additional tests here too. Your test should show that the sensitive information is no longer in the output of GetRecords etc.
Comment 12 Marcel de Rooy 2019-11-08 07:31:02 UTC
Created attachment 95178 [details] [review]
Bug 21301: (QA follow-up) Fix number of tests
Comment 13 Jonathan Druart 2019-12-27 16:31:40 UTC
Did not we display old issues prior to this patch?