Summary: | Restrict OpacSuppression to IP addresses outside of an IP range | ||
---|---|---|---|
Product: | Koha | Reporter: | Mirko Tietgen <mirko> |
Component: | Searching | Assignee: | Mirko Tietgen <mirko> |
Status: | CLOSED FIXED | QA Contact: | Jonathan Druart <jonathan.druart> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | chris, jonathan.druart, paul.poulain, robin |
Version: | 3.10 | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Attachments: |
Bug 8492 [ENH] Restrict OpacSuppression to IP adresses outside of an IP range
Bug 8492 [ENH] Restrict OpacSuppression to IP adresses outside of an IP range [SIGNED-OFF] Bug 8492 [ENH] Restrict OpacSuppression to IP adresses outside of an IP range Bug 8492 [ENH] Restrict OpacSuppression to IP adresses outside of an IP range |
Description
Mirko Tietgen
2012-07-23 23:31:12 UTC
Created attachment 11094 [details] [review] Bug 8492 [ENH] Restrict OpacSuppression to IP adresses outside of an IP range This enhancement extends the OpacSuppression feature with an optional IP address range within which results are _not_ suppressed. To test * turn on OpacSuppression (Administration->System preferences->Cataloging) and enter an IP address range in the OpacSuppressionByIPRange field. * set at least one bibliographic record to suppress=1 (enter '1' in 942$n) * fully reindex your data * do an OPAC search that should bring up your suppressed record * try with IP ranges that match your IP and ranges that don't Hi Mirko, I am not an expert of the apache configuration but it seems it is possible to do that with the RewriteCond and RewriteRule directives. Did you have a look at these directives ? (In reply to comment #2) > Hi Mirko, > > I am not an expert of the apache configuration but it seems it is possible > to do that with the RewriteCond and RewriteRule directives. > > Did you have a look at these directives ? I'm not sure you could, in the same way. OpacSuppression lets you put something in 942 that makes an biblio not show in the OPAC. This would let those still show for certain ip ranges while suppressing them for others. I'm not sure how apache could know which items to hide. Unless you made a rule for every biblio. Mirko, something to consider is that many Koha systems are behind reverse-proxies of various forms. I'm not sure exactly what Apache puts into REMOTE_ADDR, but if it doesn't consider that, it may be worth checking the request headers too (e.g. if the request is from an RFC1918 address, check to see if there's an X-Forwarded-For: header too, or something like that) Jonathan: what Chris says. It's about filtering some, not all results (those marked in 942$c). I don't think that can be achieved with Apache config? Robin: good point, did a little reading on this. It seems like - it won't work with reverse proxies like it is now - you can spoof X-Forwarded-For headers, so using them should be optional A check for RFC1918 addresses sounds like an interesting idea, but is it safe to assume that there would always be a private address here? I doubt that, eg. my university uses external IP addresses everywhere. Data::Validate::IP could do these (and more) things is_innet_ipv4 to see if ip is in network given in syspref is_private_ipv4 to see if ip is private Dependencies on Debian Squeeze are available libdata-validate-ip-perl libnet-ipv6addr-perl libnet-netmask-perl libnetwork-ipv4addr-perl What are the rules for introducing perl modules/ dependencies? It could be like this but would introduce a lot of sysprefs option: use OpacSuppression yes/no option: match REMOTE_ADDR and network option: if exists, match X-Forward-For instead of REMOTE_ADDR option: only if REMOTE_ADDR is private IP I wouldn't expect most systems to be on real-world IP addresses if they're behind a proxy anyway, there's no point. And it's only the address of the proxy you care about. Though, thinking about it more, if you're attempting to restrict to part of an RFC1918 set, this would leave forgery open. A better idea would be to specify what your proxy IP/IP pattern is and look for the header only in that case. X-Forwarded-For can be forged, also can (legitimately) have multiple IP addresses in it, but it is possible to be sure about what you're getting. For example, if you know you're behind a proxy you can rely on that header being there, and that the last entry is the real source IP address. after thinking about it a little, I propose to - use this patch like it is to add the basic functionality without new dependencies to 3.8 - have a more advanced version using Data::Validate::IP in 3.10 Does that make sense? Makes sense to me, and a simple patch with no new dependencies, that does nothing until a syspref is checked is the perfect candidate for backport to 3.8.x. So if that makes this through QA and into master, im happy to put it into 3.8.x and then the follow up can extend it for master only. Created attachment 11156 [details] [review] Bug 8492 [ENH] Restrict OpacSuppression to IP adresses outside of an IP range This enhancement extends the OpacSuppression feature with an optional IP address range within which results are _not_ suppressed. To test * turn on OpacSuppression (Administration->System preferences->Cataloging) and enter an IP address range in the OpacSuppressionByIPRange field. * set at least one bibliographic record to suppress=1 (enter '1' in 942$n) * fully reindex your data * do an OPAC search that should bring up your suppressed record * try with IP ranges that match your IP and ranges that don't Rebased to master Created attachment 11206 [details] [review] [SIGNED-OFF] Bug 8492 [ENH] Restrict OpacSuppression to IP adresses outside of an IP range This enhancement extends the OpacSuppression feature with an optional IP address range within which results are _not_ suppressed. To test * turn on OpacSuppression (Administration->System preferences->Cataloging) and enter an IP address range in the OpacSuppressionByIPRange field. * set at least one bibliographic record to suppress=1 (enter '1' in 942$n) * fully reindex your data * do an OPAC search that should bring up your suppressed record * try with IP ranges that match your IP and ranges that don't Signed-off-by: Marc Veron <veron@veron.ch> Tested following the scenario above. Works as expected. Created attachment 11692 [details] [review] Bug 8492 [ENH] Restrict OpacSuppression to IP adresses outside of an IP range This enhancement extends the OpacSuppression feature with an optional IP address range within which results are _not_ suppressed. To test * turn on OpacSuppression (Administration->System preferences->Cataloging) and enter an IP address range in the OpacSuppressionByIPRange field. * set at least one bibliographic record to suppress=1 (enter '1' in 942$n) * fully reindex your data * do an OPAC search that should bring up your suppressed record * try with IP ranges that match your IP and ranges that don't Signed-off-by: Marc Veron <veron@veron.ch> Tested following the scenario above. Works as expected. Last patch is a rebased patch (conflict on updatedatabase.pl). QA Comment: Add a new syspref OpacSuppressionByIPRange. According to the comment 8, a followup is required for 3.10. Marked as Passed QA. Patch pushed, but a tiny question: isn't "Restrict the suppression to IP adresses outside of the IP range" a little bit complex to understand. Wouldn't it be more understandable as "Don't suppress items in this IP range" ? (note that I just tested my search is not broken, I haven't suppress index on my test computer) You are right, the wording is bad. I think it evolved from "restrict OpacSuppression to IP range" (which is technically wrong) to the monster "restrict to IP addresses outside of an IP range" which is quite hard to parse. I will try to avoid things like that in the future. (In reply to comment #15) > You are right, the wording is bad. I think it evolved from "restrict > OpacSuppression to IP range" (which is technically wrong) to the monster > "restrict to IP addresses outside of an IP range" which is quite hard to > parse. I will try to avoid things like that in the future. Feel free to add a follow-up to this one ;-) |