Bug 37543 - connexion_import_daemon.pl stopped working in 24.05 due to API changes related to CSRF-Token
Summary: connexion_import_daemon.pl stopped working in 24.05 due to API changes relate...
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: Rudolf Byker
QA Contact: Testopia
URL:
Keywords:
: 37318 (view as bug list)
Depends on:
Blocks: 37648 38230
  Show dependency treegraph
 
Reported: 2024-08-01 05:18 UTC by Rudolf Byker
Modified: 2024-10-22 16:27 UTC (History)
12 users (show)

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


Attachments
WorldShare User Preferences for TCP/IP export (25.64 KB, image/png)
2024-08-01 07:31 UTC, Rudolf Byker
Details
Use CSRF tokens in connexion_import_daemon.pl (6.08 KB, patch)
2024-08-01 16:56 UTC, Rudolf Byker
Details | Diff | Splinter Review
Test records for importing (596 bytes, text/plain)
2024-08-09 10:53 UTC, Nick Clemens (kidclamp)
Details
Bug 37543: (follow-up) Tidy (23.90 KB, patch)
2024-08-09 11:00 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 37543: Use CSRF tokens in connexion_import_daemon.pl (5.99 KB, patch)
2024-08-09 11:01 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 37543: (follow-up) Tidy (23.90 KB, patch)
2024-08-09 11:01 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 37543: Use CSRF tokens in connexion_import_daemon.pl (6.05 KB, patch)
2024-08-15 09:01 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37543: (follow-up) Tidy (23.96 KB, patch)
2024-08-15 09:01 UTC, Paul Derscheid
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Rudolf Byker 2024-08-01 05:18:09 UTC
Since the changes mentioned at https://wiki.koha-community.org/wiki/Koha_/svc/_HTTP_API#Changes_coming_in_Koha_24.05 in version 24.05, the `connexion_import_daemon.pl` no longer works.

Related discussions:
- https://chat.koha-community.org/koha-community/pl/o8yasuqrwpf8upynroed5fqz1o
- https://chat.koha-community.org/koha-community/pl/kyszm1fmr3dp5gd75demen3fpe

I'll try to fix it, but I'm very inexperienced with perl, although I know lots of other languages.
Comment 1 Rudolf Byker 2024-08-01 07:31:53 UTC
Created attachment 169926 [details]
WorldShare User Preferences for TCP/IP export

If we were to write automated tests for this, we need to do it without an OCLC WorldShare subscription. I could not find docs online showing exactly how their "Send to local system via TCP/IP" protocol works, but I can reverse engineer the spec. It seems to be raw text send over TCP (no HTTP protocol). What they send depends on exactly how the options in this screenshot are set up.
Comment 2 Rudolf Byker 2024-08-01 07:33:33 UTC
When not using authentication (a dumb idea, because then anyone can send stuff into your Koha instance), it's simply the raw MARC record being sent:

00595cam a2200205Ma 4500001001300000003000600013005001700019008004100036040002500077035002100102049000900123110003700132245003700169260003400206300005400240336002600294337002800320338002700348994001400375ocn863323135OCoLC20151201051249.5131120s1986    enka          000 0 eng d  aEQObengcEQOdOCLCQ  a(OCoLC)863323135  aRSCM1 aMicroprocessors and peripherals.10aMicroprocessors and peripherals.  aLondon :bMullard Ltd,c1986.  av, c700 pages in various pagings :billustrations  atextbtxt2rdacontent  aunmediatedbn2rdamedia  avolumebnc2rdacarrier  aC0bZARSC

When choosing the "Login ID" Authentication option, the MARC record is prefixed with something like A10my_user_idP11my_password where 10 and 11 seem to be the number of characters to consume for the username (A) and password (P) respectively:

A10my_user_idP11my_password00595cam a2200205Ma 4500001001300000003000600013005001700019008004100036040002500077035002100102049000900123110003700132245003700169260003400206300005400240336002600294337002800320338002700348994001400375ocn863323135OCoLC20151201051249.5131120s1986    enka          000 0 eng d  aEQObengcEQOdOCLCQ  a(OCoLC)863323135  aRSCM1 aMicroprocessors and peripherals.10aMicroprocessors and peripherals.  aLondon :bMullard Ltd,c1986.  av, c700 pages in various pagings :billustrations  atextbtxt2rdacontent  aunmediatedbn2rdamedia  avolumebnc2rdacarrier  aC0bZARSC

When choosing the "User ID" Authentication option, the MARC record is prefixed with something like @10my_user_id where 10 seems to be the number of characters to consume for the username, this time designated by @ instead of A:

@10my_user_id00595cam a2200205Ma 4500001001300000003000600013005001700019008004100036040002500077035002100102049000900123110003700132245003700169260003400206300005400240336002600294337002800320338002700348994001400375ocn863323135OCoLC20151201051249.5131120s1986    enka          000 0 eng d  aEQObengcEQOdOCLCQ  a(OCoLC)863323135  aRSCM1 aMicroprocessors and peripherals.10aMicroprocessors and peripherals.  aLondon :bMullard Ltd,c1986.  av, c700 pages in various pagings :billustrations  atextbtxt2rdacontent  aunmediatedbn2rdamedia  avolumebnc2rdacarrier  aC0bZARSC

It would be great to link to some official docs for this, but I could not find any.
Comment 3 Rudolf Byker 2024-08-01 16:56:37 UTC
Created attachment 169958 [details] [review]
Use CSRF tokens in connexion_import_daemon.pl

I wanted to write unit tests for this, but I need a few more days to get my perl skills up to date and read more developer wiki pages on how to do that... So for now I wrote a manual test plan.
Comment 4 David Cook 2024-08-07 07:19:04 UTC
Apologies for the delay on this one. I am getting here! (Although not today ﷐[U+1F605]﷑)
Comment 5 Nick Clemens (kidclamp) 2024-08-07 18:48:43 UTC
*** Bug 37318 has been marked as a duplicate of this bug. ***
Comment 6 Nick Clemens (kidclamp) 2024-08-07 18:51:57 UTC
(In reply to David Cook from comment #4)
> Apologies for the delay on this one. I am getting here! (Although not today
> ﷐[U+1F605]﷑)

See bug 33418 for test plan on setting up and testing connexion - just ignore all the bits about frameworks and basically check that things work

(In reply to Rudolf Byker from comment #3)
> Created attachment 169958 [details] [review] [review]
> Use CSRF tokens in connexion_import_daemon.pl
> 
> I wanted to write unit tests for this, but I need a few more days to get my
> perl skills up to date and read more developer wiki pages on how to do
> that... So for now I wrote a manual test plan.

We don't generally require unit tests for things outside of modules, it would be lovely to have some here - they may take a little more infrastructure to make happen though, so I would file that as another bug
Comment 7 Katrin Fischer 2024-08-07 20:02:42 UTC
(In reply to Nick Clemens (kidclamp) from comment #6)
> (In reply to David Cook from comment #4)
> > Apologies for the delay on this one. I am getting here! (Although not today
> > ﷐[U+1F605]﷑)
> 
> See bug 33418 for test plan on setting up and testing connexion - just
> ignore all the bits about frameworks and basically check that things work
> 
> (In reply to Rudolf Byker from comment #3)
> > Created attachment 169958 [details] [review] [review] [review]
> > Use CSRF tokens in connexion_import_daemon.pl
> > 
> > I wanted to write unit tests for this, but I need a few more days to get my
> > perl skills up to date and read more developer wiki pages on how to do
> > that... So for now I wrote a manual test plan.
> 
> We don't generally require unit tests for things outside of modules, it
> would be lovely to have some here - they may take a little more
> infrastructure to make happen though, so I would file that as another bug

+1
Comment 8 Nick Clemens (kidclamp) 2024-08-09 10:53:59 UTC
Created attachment 170177 [details]
Test records for importing
Comment 9 Nick Clemens (kidclamp) 2024-08-09 11:00:19 UTC Comment hidden (obsolete)
Comment 10 Nick Clemens (kidclamp) 2024-08-09 11:01:06 UTC
Created attachment 170179 [details] [review]
Bug 37543: Use CSRF tokens in connexion_import_daemon.pl

Since version 24.05, due to the changes mentioned at
https://wiki.koha-community.org/wiki/Koha_/svc/_HTTP_API#Changes_coming_in_Koha_24.05 ,
the `connexion_import_daemon.pl` stopped working. The reason for this is that
it did not use CSRF tokens.

To test:
1. Get a Koha instance on 24.05+, before applying the patch.
2. Create a plain text file somewhere on the server containing a raw MARC
   record (not XML). Let's call it `marc.txt`.
3. On the server, create a config file like this:
```
host: 0.0.0.0
port: 5500
koha: http://localhost:82  # Where 82 is the port of the Koha staff interface.
user: foo  # A Koha staff user.
password: Fooo1234  # The Koha staff user's password.
import_mode: stage
```
4. Run `./connexion_import_daemon.pl --config the-config-file-path`
5. In another terminal on the same server (or from anywhere that can reach the
   port opened by the `connexion_import_daemon.pl` script,
   run `nc localhost 5500 < marc.txt`
6. Observe in the stderr of the daemon script: `Response: Unsuccessful request`
7. Stop the daemon script.
8. Apply the patch and repeat steps 4 and 5.
9. Observe in the stderr of the daemon script:
   `Response: Success.  Batch number ... - biblio record number HASH(...) added to Koha`
10. Check at /cgi-bin/koha/tools/manage-marc-import.pl for a batch named
    `(webservice)`. It should contain one record now. This is how we know that
    authentication between the daemon and Koha worked, which is what this
    patch tries to address.

Thanks-to: David Cook <dcook@prosentient.com.au>
Sponsored-by: Reformational Study Centre <www.refstudycentre.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 11 Nick Clemens (kidclamp) 2024-08-09 11:01:08 UTC
Created attachment 170180 [details] [review]
Bug 37543: (follow-up) Tidy

Tidy the whole thing

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 12 Paul Derscheid 2024-08-15 08:55:59 UTC
The test plan is missing that these three fields need values set:
- overlay_action
- nomatch_action
- item_action

Otherwise you'll get a:
C4::ImportBatch::AddImportBatch(): DBI Exception: DBD::mysql::db do failed: Data truncated for column '<one of the above>' at row 1 at /kohadevbox/koha/C4/ImportBatch.pm line 173
Comment 13 Paul Derscheid 2024-08-15 09:01:05 UTC
Created attachment 170339 [details] [review]
Bug 37543: Use CSRF tokens in connexion_import_daemon.pl

Since version 24.05, due to the changes mentioned at
https://wiki.koha-community.org/wiki/Koha_/svc/_HTTP_API#Changes_coming_in_Koha_24.05 ,
the `connexion_import_daemon.pl` stopped working. The reason for this is that
it did not use CSRF tokens.

To test:
1. Get a Koha instance on 24.05, before applying the patch.
2. Create a plain text file somewhere on the server containing a raw MARC
   record (not XML). Let's call it `marc.txt`.
3. On the server, create a config file like this:
```
host: 0.0.0.0
port: 5500
koha: http://localhost:82  # Where 82 is the port of the Koha staff interface.
user: foo  # A Koha staff user.
password: Fooo1234  # The Koha staff user's password.
import_mode: stage
```
4. Run `./connexion_import_daemon.pl --config the-config-file-path`
5. In another terminal on the same server (or from anywhere that can reach the
   port opened by the `connexion_import_daemon.pl` script,
   run `nc localhost 5500 < marc.txt`
6. Observe in the stderr of the daemon script: `Response: Unsuccessful request`
7. Stop the daemon script.
8. Apply the patch and repeat steps 4 and 5.
9. Observe in the stderr of the daemon script:
   `Response: Success.  Batch number ... - biblio record number HASH(...) added to Koha`
10. Check at /cgi-bin/koha/tools/manage-marc-import.pl for a batch named
    `(webservice)`. It should contain one record now. This is how we know that
    authentication between the daemon and Koha worked, which is what this
    patch tries to address.

Thanks-to: David Cook <dcook@prosentient.com.au>
Sponsored-by: Reformational Study Centre <www.refstudycentre.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 14 Paul Derscheid 2024-08-15 09:01:07 UTC
Created attachment 170340 [details] [review]
Bug 37543: (follow-up) Tidy

Tidy the whole thing

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 15 Paul Derscheid 2024-08-15 09:20:11 UTC
I added a new bug for the three fields I mentioned in comment #12. I think these should have default values (ignore) and warn about them being set automatically.

Check out bug 37648.
Comment 16 Rudolf Byker 2024-08-15 10:08:45 UTC
> The test plan is missing that these three fields need values set

Strange... I am 99% certain that I tested without setting those three fields.
Comment 17 Paul Derscheid 2024-08-15 10:10:20 UTC
I doesn't impact that the fix works, therefore I passed QA but please take another look.
Comment 18 Rudolf Byker 2024-08-15 10:11:37 UTC
Will do.
Comment 19 Katrin Fischer 2024-08-16 14:48:20 UTC
Congrats on your first QA, Paul :)(In reply to Rudolf Byker from comment #18)
> Will do.

Please open a new bug if necessary.

Congrats on your first QA, Paul!
Comment 20 Katrin Fischer 2024-08-16 15:37:26 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 21 Dani Elder 2024-08-20 16:35:53 UTC
Thank to you all for fixing this.  Any chance it will be backported to 24.05? We use this feature in Koha regularly, and our lone cataloger is much bereaved over the loss of it.
Comment 22 Lucas Gass (lukeg) 2024-09-06 16:42:59 UTC
Backported to 24.05.x for upcoming 24.05.04
Comment 23 Fridolin Somers 2024-09-17 14:02:58 UTC
Not for 23.11.x
Comment 24 Dani Elder 2024-09-24 16:25:56 UTC
Thanks, Lucas, for the backport and all for the work to fix this.
Comment 25 Heather 2024-10-22 13:45:18 UTC
I *think* I may have this fix (on 24.05.03) and for one record that I sent from OCLC Connexion Client, I got, "Success.  Batch number 8989 - biblio record number HASH(0x5571396a8fd8) added to Koha."  This wasn't helpful at all--there was no record number mentioned, let alone a URL of the record so that I could look at it quickly.

I had to go to Manage Staged Records to look at the record, and it had imported that single OCLC record *twice,* i.e. created two records from my one record--I had one record with the items on it, and a dup bib (with no items, of course, because they would have been dup barcodes in the items and my Koha won't allow it).

I know this is all "fixed," so is it a separate bug to stop it from giving a message like, "Success.  Batch number 8989 - biblio record number HASH(0x5571396a8fd8) added to Koha" when exporting *ONE* record, and have it go back to only creating ONE Koha record per ONE bib from OCLC, with a URL with the record number in it like it used to?

Or should that be a new bug?

Thank you!!!
Comment 26 Katrin Fischer 2024-10-22 14:15:59 UTC
Hi Heather, it probably only needs a small fix to get you back the right success message, but yes, as this has already been released: please file a new bug and use the depends on field to link to this one.
Comment 27 Heather 2024-10-22 16:05:09 UTC
Okey-dokey!  Will do!

Thank you, Katrin!!!
--h2
Comment 28 Heather 2024-10-22 16:27:24 UTC
Bug 38230 filed to fix the "success" message!