If koha_trusted_proxies contains an invalid or unparseable value, say due to a typo, like "1.1.1.", it'll treat it as a 0.0.0.0/0 mask and match against everything. In my tests, this leads to REMOTE_USER being set to undef. (On the plus side, at least a 0.0.0.0/0 mask leads to zero privileges rather than all privileges.)
The solution should probably use either the new2 constructor (code smell...) or check the $objet->{'ERROR'} value as per https://metacpan.org/pod/Net::Netmask#CONSTRUCTING. (Note that I actually found this while testing a patch for https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24537)
Created attachment 99232 [details] [review] Bug 24538: Add unit tests for Koha::Middleware::RealIP This patch adds some unit tests for Koha::Middleware::RealIP
Created attachment 99233 [details] [review] Bug 24538: Handle Net::Netmask parser errors This patch switches from the new() to new2() constructor, which will return an undef value when it fails to parse a value. This patch warns on parser failures, but otherwise silently drops the invalid value, and returns objects for any valid input it can parse. This way one mistake won't disable the whole feature. To test: 1. Run the unit test t/Koha/Middlware/RealIP.t
I'd suggest applying the "Add unit tests..." patch first, running it, seeing the failures, and then applying the 2nd patch, and then running the unit test again (and seeing the successes).
Created attachment 99243 [details] [review] Bug 24538: Add unit tests for Koha::Middleware::RealIP This patch adds some unit tests for Koha::Middleware::RealIP Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 99244 [details] [review] Bug 24538: Add unit tests for Koha::Middleware::RealIP This patch adds some unit tests for Koha::Middleware::RealIP Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 99245 [details] [review] Bug 24538: Handle Net::Netmask parser errors This patch switches from the new() to new2() constructor, which will return an undef value when it fails to parse a value. This patch warns on parser failures, but otherwise silently drops the invalid value, and returns objects for any valid input it can parse. This way one mistake won't disable the whole feature. To test: 1. Run the unit test t/Koha/Middlware/RealIP.t Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 99283 [details] [review] Bug 24538: Add unit tests for Koha::Middleware::RealIP This patch adds some unit tests for Koha::Middleware::RealIP Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 99284 [details] [review] Bug 24538: Handle Net::Netmask parser errors This patch switches from the new() to new2() constructor, which will return an undef value when it fails to parse a value. This patch warns on parser failures, but otherwise silently drops the invalid value, and returns objects for any valid input it can parse. This way one mistake won't disable the whole feature. To test: 1. Run the unit test t/Koha/Middlware/RealIP.t Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Cheers guys, Passing QA
Thanks for the quick turnaround on this, guys. Really appreciated. I love Koha::Middleware::RealIP, so glad I was able to contribute :D.
Nice work everyone! Pushed to master for 20.05
Pushed to 19.11.x branch for 19.11.04