Bug 38826 - C4::Auth::check_api_auth sometimes returns $session and sometimes returns $sessionID
Summary: C4::Auth::check_api_auth sometimes returns $session and sometimes returns $se...
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Authentication (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: David Cook
QA Contact: Nick Clemens (kidclamp)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2025-01-05 23:42 UTC by David Cook
Modified: 2025-01-13 01:15 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes authentication checking so that the $sessionID is consistently returned (sometimes it was returning the session object). (Note: $sessionID is returned on a successful login, while $session is returned when there is a cookie for an authenticated session.)
Version(s) released in:
25.05.00
Circulation function:


Attachments
Bug 38826: Make check_api_auth $sessionID consistent (1.89 KB, patch)
2025-01-05 23:53 UTC, David Cook
Details | Diff | Splinter Review
Bug 38826: Make check_api_auth $sessionID consistent (1.94 KB, patch)
2025-01-06 19:06 UTC, Jan Kissig
Details | Diff | Splinter Review
Bug 38826: Make check_api_auth $sessionID consistent (2.00 KB, patch)
2025-01-07 12:32 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2025-01-05 23:42:05 UTC
As I discovered while troubleshooting an issue with bug 37060, it appears that C4::Auth::check_api_auth sometimes returns $session and sometimes returns $sessionID.

After grepping through the code, it seems that the only file that uses the $sessionID return value from C4::Auth::check_api_auth is svc/authentication so it looks like it's the only affected file.

Note: $sessionID is returned on a successful login, while $session is returned when we have a cookie for an authenticated session.
Comment 1 David Cook 2025-01-05 23:53:03 UTC
Created attachment 176145 [details] [review]
Bug 38826: Make check_api_auth $sessionID consistent

This change makes $sessionID consistently return using the session
ID and not sometimes the session object.

Test plan:
0. Apply the patch
1. koha-plack --restart kohadev
2. curl -v 'http://localhost:8081/cgi-bin/koha/svc/authentication' --cookie-jar /tmp/test.cookies
3. Note the Csrf-Token header value
4. Replace the <CSRF-TOKEN>, <USERID>, and <PASSWORD> tokens in the next step
using the appropriate values (eg the CSRF-TOKEN is from the previous step)
5. curl -v -H "Content-Type: application/x-www-form-urlencoded" \
    -H "Csrf-Token: <CSRF-TOKEN>" -XPOST \
    'http://localhost:8081/cgi-bin/koha/svc/authentication' \
    -d "login_userid=<USERID>&login_password=<PASSWORD>" \
    --cookie /tmp/test.cookies --cookie-jar /tmp/test.cookies
6. curl -XGET -v 'http://localhost:8081/cgi-bin/koha/svc/bib/29' \
    --cookie /tmp/test.cookies > rec.marcxml
7. curl -v 'http://localhost:8081/cgi-bin/koha/svc/authentication' \
    --cookie /tmp/test.cookies --cookie-jar /tmp/test.cookies
8. Note the Csrf-Token header value
9. curl -H "Content-Type: text/xml" -H "Csrf-Token: <CSRF-TOKEN>" \
    -XPOST -v 'http://localhost:8081/cgi-bin/koha/svc/bib/29' \
    --cookie /tmp/test.cookies --data @rec.marcxml
Comment 2 David Cook 2025-01-06 00:04:43 UTC
Note: This bug causes the Csrf-Token returned by svc/authentication to be incorrect when the user sends a cookie for an authenticated session, since it's using something like "CGI::Session=HASH(0x5592c03f5508)" instead of the actual session ID to create the token.
Comment 3 Jan Kissig 2025-01-06 19:06:41 UTC
Created attachment 176164 [details] [review]
Bug 38826: Make check_api_auth $sessionID consistent

This change makes $sessionID consistently return using the session
ID and not sometimes the session object.

Test plan:
0. Apply the patch
1. koha-plack --restart kohadev
2. curl -v 'http://localhost:8081/cgi-bin/koha/svc/authentication' --cookie-jar /tmp/test.cookies
3. Note the Csrf-Token header value
4. Replace the <CSRF-TOKEN>, <USERID>, and <PASSWORD> tokens in the next step
using the appropriate values (eg the CSRF-TOKEN is from the previous step)
5. curl -v -H "Content-Type: application/x-www-form-urlencoded" \
    -H "Csrf-Token: <CSRF-TOKEN>" -XPOST \
    'http://localhost:8081/cgi-bin/koha/svc/authentication' \
    -d "login_userid=<USERID>&login_password=<PASSWORD>" \
    --cookie /tmp/test.cookies --cookie-jar /tmp/test.cookies
6. curl -XGET -v 'http://localhost:8081/cgi-bin/koha/svc/bib/29' \
    --cookie /tmp/test.cookies > rec.marcxml
7. curl -v 'http://localhost:8081/cgi-bin/koha/svc/authentication' \
    --cookie /tmp/test.cookies --cookie-jar /tmp/test.cookies
8. Note the Csrf-Token header value
9. curl -H "Content-Type: text/xml" -H "Csrf-Token: <CSRF-TOKEN>" \
    -XPOST -v 'http://localhost:8081/cgi-bin/koha/svc/bib/29' \
    --cookie /tmp/test.cookies --data @rec.marcxml

Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Comment 4 Katrin Fischer 2025-01-07 08:37:22 UTC
Updating severity as I believe that will help to fix the KOCT plugin?
Comment 5 Nick Clemens (kidclamp) 2025-01-07 12:32:34 UTC
Created attachment 176187 [details] [review]
Bug 38826: Make check_api_auth $sessionID consistent

This change makes $sessionID consistently return using the session
ID and not sometimes the session object.

Test plan:
0. Apply the patch
1. koha-plack --restart kohadev
2. curl -v 'http://localhost:8081/cgi-bin/koha/svc/authentication' --cookie-jar /tmp/test.cookies
3. Note the Csrf-Token header value
4. Replace the <CSRF-TOKEN>, <USERID>, and <PASSWORD> tokens in the next step
using the appropriate values (eg the CSRF-TOKEN is from the previous step)
5. curl -v -H "Content-Type: application/x-www-form-urlencoded" \
    -H "Csrf-Token: <CSRF-TOKEN>" -XPOST \
    'http://localhost:8081/cgi-bin/koha/svc/authentication' \
    -d "login_userid=<USERID>&login_password=<PASSWORD>" \
    --cookie /tmp/test.cookies --cookie-jar /tmp/test.cookies
6. curl -XGET -v 'http://localhost:8081/cgi-bin/koha/svc/bib/29' \
    --cookie /tmp/test.cookies > rec.marcxml
7. curl -v 'http://localhost:8081/cgi-bin/koha/svc/authentication' \
    --cookie /tmp/test.cookies --cookie-jar /tmp/test.cookies
8. Note the Csrf-Token header value
9. curl -H "Content-Type: text/xml" -H "Csrf-Token: <CSRF-TOKEN>" \
    -XPOST -v 'http://localhost:8081/cgi-bin/koha/svc/bib/29' \
    --cookie /tmp/test.cookies --data @rec.marcxml

Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 6 Katrin Fischer 2025-01-07 14:40:05 UTC
Nice mix of people here! :)
Comment 7 Katrin Fischer 2025-01-07 14:49:25 UTC
Pushed for 25.05!

Well done everyone, thank you!
Comment 8 David Nind 2025-01-10 22:42:47 UTC
I had a go at a release note, but I'm having understanding the "why" and the "effect" of this change - where this would be noticed. Only the KOCT browser addon?
Comment 9 David Cook 2025-01-12 23:00:59 UTC
(In reply to David Nind from comment #8)
> I had a go at a release note, but I'm having understanding the "why" and the
> "effect" of this change - where this would be noticed. Only the KOCT browser
> addon?

Thanks, David. I'll take a look.
Comment 10 David Cook 2025-01-12 23:03:10 UTC
(In reply to David Nind from comment #8)
> I had a go at a release note, but I'm having understanding the "why" and the
> "effect" of this change - where this would be noticed. Only the KOCT browser
> addon?

The release text looks good to me.

At the moment, it only affects any third-party integration that uses the svc/authentication API to check an authenticated session. I'm guessing not many integrations do it, or they've been not working and no one has said anything.
Comment 11 David Cook 2025-01-13 01:15:31 UTC
(In reply to Katrin Fischer from comment #4)
> Updating severity as I believe that will help to fix the KOCT plugin?

That's true. We should probably backport it back to 24.11 and 24.05 as well then.