[Bug 21325] New: Should we still allow user and password via GET parameters?
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 Bug ID: 21325 Summary: Should we still allow user and password via GET parameters? Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Authentication Assignee: koha-bugs@lists.koha-community.org Reporter: m.de.rooy@rijksmuseum.nl QA Contact: testopia@bugs.koha-community.org CC: dpavlin@rot13.org In testing bug 13779, I just noticed that an URL like this works: https://[your_domain]/cgi-bin/koha/opac-reserve.pl?biblionumber=[some_biblionumber]&userid=[your_userid]&password=[your_password] Obviously, we should not expose credentials like that. But it raises the question: should we allow it then? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=13779 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=22543 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #1 from David Cook <dcook@prosentient.com.au> --- (In reply to Marcel de Rooy from comment #0)
In testing bug 13779, I just noticed that an URL like this works:
https://[your_domain]/cgi-bin/koha/opac-reserve. pl?biblionumber=[some_biblionumber]&userid=[your_userid]&password=[your_passw ord]
Obviously, we should not expose credentials like that. But it raises the question: should we allow it then?
In my opinion, we 100% should not allow it. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Should we still allow user |Prevent authentication when |and password via GET |sending userid and password |parameters? |via querystring parameters -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 --- Comment #2 from David Cook <dcook@prosentient.com.au> --- Created attachment 114695 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114695&action=edit Bug 21325: Prevent authentication when sending userid and password in querystring This patch permits authentication via userid/password only when the HTTP method is POST when using C4::Auth::checkauth(). The goal is to stop people from supplying userid and password in querystrings in order to log into web pages. Test plan: 0. Do not apply patch yet 1. Open a new browser (ie we don't want any existing CGISESSID cookies available - opening a new tab/window isn't enough. It must be a new instance or you can clear your cookies) 2. Go to http://localhost:8080/cgi-bin/koha/opac-reserve.pl?biblionumber=29&userid=koha&password=koha 3. Note the user has been logged in and is being asked to confirm hold. 4. Apply the patch 5. Go to http://localhost:8080/cgi-bin/koha/opac-reserve.pl?biblionumber=29&userid=koha&password=koha 6. Note the user is not logged in and the user is presented with a login screen -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 --- Comment #3 from David Cook <dcook@prosentient.com.au> --- Created attachment 114696 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114696&action=edit Bug 21325: Fix t/db_dependent/Auth.t test This patch fixes some warnings in t/db_dependent/Auth.t. Note that it doesn't add any tests. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 --- Comment #4 from David Cook <dcook@prosentient.com.au> --- It's a quick and dirty patch, but it works. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |dcook@prosentient.com.au |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114695|0 |1 is obsolete| | Attachment #114696|0 |1 is obsolete| | --- Comment #5 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 119600 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=119600&action=edit Bug 21325: Prevent authentication when sending userid and password in querystring This patch permits authentication via userid/password only when the HTTP method is POST when using C4::Auth::checkauth(). The goal is to stop people from supplying userid and password in querystrings in order to log into web pages. Test plan: 0. Do not apply patch yet 1. Open a new browser (ie we don't want any existing CGISESSID cookies available - opening a new tab/window isn't enough. It must be a new instance or you can clear your cookies) 2. Go to http://localhost:8080/cgi-bin/koha/opac-reserve.pl?biblionumber=29&userid=koha&password=koha 3. Note the user has been logged in and is being asked to confirm hold. 4. Apply the patch 5. Go to http://localhost:8080/cgi-bin/koha/opac-reserve.pl?biblionumber=29&userid=koha&password=koha 6. Note the user is not logged in and the user is presented with a login screen Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 --- Comment #6 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 119601 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=119601&action=edit Bug 21325: Fix t/db_dependent/Auth.t test This patch fixes some warnings in t/db_dependent/Auth.t. Note that it doesn't add any tests. Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |BLOCKED --- Comment #7 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA: Looking here -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #119600|0 |1 is obsolete| | --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 119697 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=119697&action=edit Bug 21325: Prevent authentication when sending userid and password in querystring This patch permits authentication via userid/password only when the HTTP method is POST when using C4::Auth::checkauth(). The goal is to stop people from supplying userid and password in querystrings in order to log into web pages. Test plan: 0. Do not apply patch yet 1. Open a new browser (ie we don't want any existing CGISESSID cookies available - opening a new tab/window isn't enough. It must be a new instance or you can clear your cookies) 2. Go to http://localhost:8080/cgi-bin/koha/opac-reserve.pl?biblionumber=29&userid=koha&password=koha 3. Note the user has been logged in and is being asked to confirm hold. 4. Apply the patch 5. Go to http://localhost:8080/cgi-bin/koha/opac-reserve.pl?biblionumber=29&userid=koha&password=koha 6. Note the user is not logged in and the user is presented with a login screen Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #119601|0 |1 is obsolete| | --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 119698 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=119698&action=edit Bug 21325: Fix t/db_dependent/Auth.t test This patch fixes some warnings in t/db_dependent/Auth.t. Note that it doesn't add any tests. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Few minor questions/remarks: Should we add a warn close to the POST test when we encounter a GET request (or even another) ? Or silently ignore like we do now? (out of scope) While glancing thru Auth, I was surprised that we do not seem to check if the password is not empty. We always pass it to checkpw. You touched the test, but did not add tests. I think the benefits of this small change outweigh the lack of new tests. Lets see if RM thinks so too. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 119721 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=119721&action=edit Bug 21325: Add tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |21.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 21.05, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |release-notes-needed --- Comment #13 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I assume we should add something to the release notes about this as it might be a breaking change for some customizations? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=27305 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 --- Comment #14 from David Cook <dcook@prosentient.com.au> --- (In reply to Katrin Fischer from comment #13)
I assume we should add something to the release notes about this as it might be a breaking change for some customizations?
Not only customization but also creative authentication by end users. See Bug 27305. Nico was very creative using the querystring to authenticate Google Calendar with Koha. I originally worked on this to stop him from being able to do that, although I feel bad that we don't have a ready feature to replace it for him. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This change may break release notes| |custom or creative (yet | |insecure) authentication | |integration using GET | |requests. These auth | |requests do not exist in | |Koha, but they may be used | |by extensions, | |customizations, or clever | |end users. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=27199 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 --- Comment #15 from David Cook <dcook@prosentient.com.au> --- *** Bug 27199 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com --- Comment #16 from Nick Clemens <nick@bywatersolutions.com> --- Should we do the same for svc scripts? https://wiki.koha-community.org/wiki/Koha_/svc/_HTTP_API#GET_.2Fsvc.2Fbib.2F... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 --- Comment #17 from David Cook <dcook@prosentient.com.au> --- (In reply to Nick Clemens from comment #16)
Should we do the same for svc scripts? https://wiki.koha-community.org/wiki/Koha_/svc/_HTTP_API#GET_.2Fsvc.2Fbib.2F. 24biblio
It would probably be a good idea although more likely to break things. I suppose we should actually double-check all instances of checkpw(). They're in checkauth() but also check_api_auth and a few other places like C4/ILSDI/Services.pm, opac/sco/sco-main.pl, etc. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|release-notes-needed | CC| |jonathan.druart@bugs.koha-c | |ommunity.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #18 from Fridolin Somers <fridolin.somers@biblibre.com> --- I propose we don't backport to stable branches to avoid breaking existing authentications. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 --- Comment #19 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Fridolin Somers from comment #18)
I propose we don't backport to stable branches to avoid breaking existing authentications.
I agree, this could block libraries depending on it from getting other bugfixes. Better to give them a little bit of time. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21325 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |28660 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28660 [Bug 28660] Self checkout is not automatically logging in -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org