A fix for the BruteForceDetection module on Login/Security plugin. When the configs are set to disable login from all ips but never block some ips, the user gets blocked from being able to login because the first check is done on the blacklist instead of the whitelist to allow the user to login from the whitelist ips.
Check this post for your reference:
It says 'Never block these IPs from logging in', that's how I was not able to log in... I added the above on the configs although I was still unable to log in...
As it is implied that these ips will never get blocked shouldn't they be checked first?
I see. We might need to change the wording. I guess the blacklist generally wasn't meant as a "Block all" kind of feature. For this case the login whitelist feature be better suited.
I'm basically saying this to avoid later someone coming like "I have whitelisted a range of IPs but want to blacklist specific IPs and the blacklist should be checked first".
@mattab you are saying sounds good to merge but then giving a thumbs up on my indication not to merge.
Note that changing this would also break existing configurations and in above described use case clearly the wrong feature is used as mentioned. The correct feature to be used is definitely https://matomo.org/faq/how-to/faq_25543/ so I'd be rather keen to close the PR since it allows more flexibility overall I would say.
@tsteur I see what you mean now, I agree it's better not to merge and instead update wording. So would we just need to change the wording in the settings from 'Never block these IPs from logging in' to the actual meaning like 'Only allow these IPs to login' or so?
@markoboy could you use this Login Whitelist IPs feature instead?
We generally prefer to rather block an IP than to allow it. And changing the meaning of it would break the meaning for existing users and it is maybe a more safe approach to rather block the IP when we have an extra feature where it's possible to only allow specific IPs. Thanks for this again!