Bug 6804 - If IndependantBranches and AutoLocation enabled, OPAC login does not redirect properly outside IP range
Summary: If IndependantBranches and AutoLocation enabled, OPAC login does not redirect...
Status: RESOLVED DUPLICATE of bug 9569
Alias: None
Product: Koha
Classification: Unclassified
Component: Authentication (show other bugs)
Version: Main
Hardware: All All
: PATCH-Sent (DO NOT USE) minor
Assignee: Ian Walls
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-29 15:32 UTC by Ian Walls
Modified: 2016-08-16 13:17 UTC (History)
5 users (show)

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


Attachments
Proposed Patch (1.83 KB, patch)
2011-08-29 16:06 UTC, Ian Walls
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6804: if IndependantBranches & AutoLocation, OPAC login redirect fails (1.91 KB, patch)
2011-11-04 13:30 UTC, Sophie MEYNIEUX
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Ian Walls 2011-08-29 15:32:21 UTC
In the event that a library has IndependantBranches turned on, as well as AutoLocation turned on and valid IP ranges in place for their branches, the OPAC login will not redirect to the My Summary area; it will instead show another login prompt.  The login is indeed successful, and if the patron navigates to other pages, they will find the link to their account in the upper right corner as normal, but this initial prompt is VERY discouraging.
Comment 1 Ian Walls 2011-08-29 16:06:56 UTC Comment hidden (obsolete)
Comment 2 Sophie MEYNIEUX 2011-11-04 13:30:21 UTC
Created attachment 6198 [details] [review]
[SIGNED-OFF] Bug 6804: if IndependantBranches & AutoLocation, OPAC login redirect fails

AutoLocation should only apply to the staff client login, but when IndependantBranches
was activated, it applied to the OPAC login, as well.  Login was not prevented, but the
patron was greeted with another login prompt, instead of the My Summary page.  Very confusing.

This patch adds a check for whether this is an OPAC login before executing the particular code
that was causing this.

To test:
1.  Enable IndependantBranches & AutoLocation (be sure your branches have IP ranges configured!)
2.  Select a staff account, and note the branch
3.  Attempt to login to the staff client from within the IP range for the branch: this should work
4.  Attempt to login to the staff client from outside the IP range:  this should be blocked
5.  Attempt to login to the OPAC from within the IP range: this should work
6.  Attempt to login to the OPAC from outside the IP range: this should also work

Signed-off-by: Sophie Meynieux <sophie.meynieux@biblibre.com>
Comment 3 Sophie MEYNIEUX 2011-11-04 13:40:35 UTC
I have tested this patch as described. 
As my koha server is local, the IP address I have to authorize for the site was 127.0.0.1
Laurent Ducos draws our attention on the fact that this IP check can be easily bypassed by IP spoofing
Comment 4 Marcel de Rooy 2011-12-05 09:43:16 UTC
QA Comment:
I did not test this patch, but have some comments on the code that is not directly touched by this patch but is in the direct context.

I think that the check if ($ip !~ /^$domain/) does not work. If domain is a full IP address it works. But if domain is a subnet mask like 199.1.*, the check will also approve addresses like 199.11.12.13 which it should not (.* will `eat` 1.12.13).

From a QA standpoint, I do not like it that the code here takes its regex here directly from the database. The point in the database column is not meant as a regex wildcard.

My recommendation therefore is that this patch should also correct this regex, since it directly deals with the if-condition on top of it.
Comment 5 Ian Walls 2011-12-05 14:45:56 UTC
Adding See Also link to bug 3068
Comment 6 Martin Renvoize (ashimema) 2014-12-30 18:33:08 UTC
Looks like this has completely fallen off the radar... do we know if it's still a bug in current master?
Comment 7 Jonathan Druart 2016-08-16 13:17:50 UTC
Will be fixed by bug 9569.

*** This bug has been marked as a duplicate of bug 9569 ***