Bug 19752 - Improve authentication response in offline_circ/service.pl
Summary: Improve authentication response in offline_circ/service.pl
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Alex Arnaud
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 19689
  Show dependency treegraph
 
Reported: 2017-12-05 15:33 UTC by Alex Arnaud
Modified: 2018-10-24 09:13 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
offline_circ/service.pl return HTTP status 401 when authentication failed and add option nocookie. (1.78 KB, patch)
2017-12-05 15:41 UTC, Alex Arnaud
Details | Diff | Splinter Review
offline_circ/service.pl return HTTP status 401 when authentication failed and add option nocookie. (1.83 KB, patch)
2018-04-06 19:40 UTC, Maksim Sen
Details | Diff | Splinter Review
offline_circ/service.pl return HTTP status 401 when authentication failed and add option nocookie. (1.83 KB, patch)
2018-04-23 11:33 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 19752: offline_circ/service.pl - Return HTTP status 401 when authentication failed and add option nocookie (1.91 KB, patch)
2018-04-24 06:22 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19752: offline_circ/service.pl - Return HTTP status 401 when authentication failed and add option nocookie (2.00 KB, patch)
2018-04-24 15:16 UTC, Matthias Meusburger
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Arnaud 2017-12-05 15:33:52 UTC
offline_circ/service.pl currently returns a "200 OK" HTTP status with a "Authentication failed" string. We need a valid status code in this case (401).

We also need a nocookie option in this script. We added a test button in Koct FF extention that check if the entered login/password are correct. But if the user is currently authenticated on Koha, the script will use the cookie authentication which can be misleading. With nocookie parameter set to true, only the login/password will be checked.

Patch is coming
Comment 1 Alex Arnaud 2017-12-05 15:41:47 UTC
Created attachment 69511 [details] [review]
offline_circ/service.pl return HTTP status 401 when authentication failed and add option nocookie.

test plan:

- Apply this patch,
- log in to Koha,
- go to cgi-bin/koha/offline_circ/service.pl with no valid user
  and password as parameters and nocookie set to 1. i.e:
  cgi-bin/koha/offline_circ/service.pl?user=alex&password=wrongpass&nocookie=1,
- auth should fail
- check that the response code is 401
Comment 2 Maksim Sen 2018-03-30 20:40:41 UTC
Hey Alex,

I only get a 404 code and not 401 in the opac.
and I got nothing by using your link/path with the intranet.

Am I missing some steps?

Thanks,

Maksim Sen
Comment 3 Matthias Meusburger 2018-04-04 14:52:32 UTC
Hi Maksim,

service.pl is indeed an intranet page.

What do you mean you got nothing? A blank page?

Keep in mind that service.pl is meant to be called as a webservice, so it's not abnormal nothing is displayed in the page itself.

However, you'll be able to display the HTTP status code using firefox's debug tool (F12 key, then Network tab, then load the page)

By the way, I've tested this, and it behaves correctly: I have a 200 error code following the test plan before applying the patch, and a 401 error code with the patch.
Comment 4 M. Tompsett 2018-04-04 15:47:21 UTC
I had the FireFox Rester tool installed.
On a kohadevbox with Offline Circulation enabled.
Something is weird I think with the $lasttime and $timeout check in the function call.
Comment 5 Maksim Sen 2018-04-06 19:40:40 UTC
Created attachment 73799 [details] [review]
offline_circ/service.pl return HTTP status 401 when authentication failed and add option nocookie.

test plan:

- Apply this patch,
- log in to Koha,
- go to cgi-bin/koha/offline_circ/service.pl with no valid user
  and password as parameters and nocookie set to 1. i.e:
  cgi-bin/koha/offline_circ/service.pl?user=alex&password=wrongpass&nocookie=1,
- auth should fail
- check that the response code is 401

https://bugs.koha-community.org/show_bug.cgi?id=19752
Signed-off-by: Maksim Sen <maksim.sen@inlibro.com>
Comment 6 Maksim Sen 2018-04-06 19:44:23 UTC
I don't know why it showed this link at the end of my sign off:
https://bugs.koha-community.org/show_bug.cgi?id=19752

Anyway, I tried it with the dev tools, Matthias, and it works!
Comment 7 Katrin Fischer 2018-04-22 10:51:55 UTC
Test plan passes, but shouldn't the right credentials give me a 200? It seems I always get a 401.

Alex, can you extend the test plan a bit to explain the expected outcomes?
- What happens with and without nocookie?
- What happens with and without the right credentials?
Comment 8 Matthias Meusburger 2018-04-23 08:36:16 UTC
If I'm correct, the following behavior is expected:

 - right credentials, with a cookie, without the nocookie option: 200
 - wrong credentials, with a cookie, without the nocookie option: 200

 - right credentials, with a cookie, with the nocookie option: 200
 - wrong credentials, with a cookie, with the nocookie option: 401

 - right credentials, without a cookie, without the nocookie option: 200
 - wrong credentials, without a cookie, without the nocookie option: 401

 - right credentials, without a cookie, with the nocookie option: 200
 - wrong credentials, without a cookie, with the nocookie option: 401
Comment 9 Matthias Meusburger 2018-04-23 08:39:01 UTC
The second line:

 - wrong credentials, with a cookie, without the nocookie option: 200

is the current behavior in Koha, and is the reason the nocookie option is introduced, in order the be able to have this behavior:

 - wrong credentials, with a cookie, with the nocookie option: 401
Comment 10 Katrin Fischer 2018-04-23 08:41:05 UTC
Hi Matts, can you maybe check if it works for you? My test was nocookie=1 with correct credentials and I still got a 401.
Comment 11 Matthias Meusburger 2018-04-23 08:55:09 UTC
You're right Katrin, there is a problem:

 - right credentials, without a cookie, with the nocookie option returns 401 instead of 200. (check_api_auth returns status="expired")
Comment 12 Matthias Meusburger 2018-04-23 09:02:34 UTC
I found the problem, the test plan was incorrect, the parameter to use is userid and not user:

cgi-bin/koha/offline_circ/service.pl?userid=alex&password=wrongpass&nocookie=1

When using the correct parameters, it works as expected.
Comment 13 Katrin Fischer 2018-04-23 09:48:28 UTC
We need to remember to fix the commit message then. Could you do it?
Comment 14 Matthias Meusburger 2018-04-23 11:33:22 UTC
Created attachment 74712 [details] [review]
offline_circ/service.pl return HTTP status 401 when authentication failed and add option nocookie.

test plan:

- Apply this patch,
- log in to Koha,
- go to cgi-bin/koha/offline_circ/service.pl with no valid user
  and password as parameters and nocookie set to 1. i.e:
  cgi-bin/koha/offline_circ/service.pl?userid=alex&password=wrongpass&nocookie=1,
- auth should fail
- check that the response code is 401

https://bugs.koha-community.org/show_bug.cgi?id=19752
Signed-off-by: Maksim Sen <maksim.sen@inlibro.com>
Comment 15 Katrin Fischer 2018-04-24 06:22:34 UTC
Created attachment 74776 [details] [review]
Bug 19752: offline_circ/service.pl - Return HTTP status 401 when authentication failed and add option nocookie

Test plan:

- Apply this patch,
- log in to Koha,
- go to cgi-bin/koha/offline_circ/service.pl with no valid user
  and password as parameters and nocookie set to 1. i.e:
  cgi-bin/koha/offline_circ/service.pl?userid=alex&password=wrongpass&nocookie=1,
- auth should fail
- check that the response code is 401

https://bugs.koha-community.org/show_bug.cgi?id=19752
Signed-off-by: Maksim Sen <maksim.sen@inlibro.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 16 Jonathan Druart 2018-04-24 14:51:16 UTC
Since KOCT 0.4.5, that's it?
Maybe we should add a comment close to the parameter explaining where does it comes from?
Comment 17 Matthias Meusburger 2018-04-24 15:08:56 UTC
Yes, that's right, since 0.4.5
I'll add a comment to explain.
Comment 18 Matthias Meusburger 2018-04-24 15:16:03 UTC
Created attachment 74814 [details] [review]
Bug 19752: offline_circ/service.pl - Return HTTP status 401 when authentication failed and add option nocookie

Test plan:

- Apply this patch,
- log in to Koha,
- go to cgi-bin/koha/offline_circ/service.pl with no valid user
  and password as parameters and nocookie set to 1. i.e:
  cgi-bin/koha/offline_circ/service.pl?userid=alex&password=wrongpass&nocookie=1,
- auth should fail
- check that the response code is 401

https://bugs.koha-community.org/show_bug.cgi?id=19752
Signed-off-by: Maksim Sen <maksim.sen@inlibro.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 19 Jonathan Druart 2018-04-24 15:26:20 UTC
Thanks Matts!
Comment 20 Jonathan Druart 2018-05-03 16:40:47 UTC
Pushed to master for 18.05, thanks to everybody involved!