User ban

Review Request #175 - Created Sept. 15, 2011 and updated

Serban Andrei Stan
I've implemented most of the banip utility. At the request of some admins, I've posted my work, to ease the debugging while there are still few lines of code.

The banip utility works in the following way:

There are two types of ban: for username, and for userip. At user login, it verifies if the user is banned. If he is banned, then access to his account is not permitted (though access to the site is unrestrained).

Now for the ban functions: 
   - migrate-ban-ip was created in order to modify the db
   - I've modified login.php to acces the verify ban functions (placed in common/ban.php)
   - The ban functions verify if a user is banned, and if he is, he is not permitted login, otherwise if he was banned and the ban date expired, he is removed from the ban tables [DONE with keeping the user in the ban tables]
   - I've created a ban form in www/views/form_ban, very similar to the login form
   - Ban.php in /controllers retrieves the information from the form and accesses the functions in common/ban.php

What remains to be done:
    1)FIRST of all, permissions for the ban page [DONE]
    2)the possibility to ban a substring for an ip (i.e. 192.168.0.*, but 192.16*.0.1 does not work) [DONE]
    3)adding a button on each user's profile page, that redirects the current admin to the ban page, autocomplete-ing the username and IP form fields [DONE]
    4)the automatic logout of a banned user [DONE]
    5)on the ban page, for each userip the possibility to show all the accounts that utilised the current ip.
    6)an unban page (the functions that work with the database are already implemented in /common/ban.php) (eventualy the possibility to show all the elements in the ban tables).

How I have tested the changes so far:
    -I've tested the ban functions directly in a mysql console, adding users and deleting them to test the variables introduced
    -I didn't test the banip form so thoroughly, seems it works though



  • 2
  • 30
  • 2
  • 34
Description From Last Updated
If you want a single line add to the MySql query LIMIT 1, otherwise the query will go through all ... Adrian Budau Adrian Budau
use SELECT COUNT FROM so it returns just the number. It's faster Adrian Budau Adrian Budau
Serban Andrei Stan
Savin  Tiberiu
Serban Andrei Stan
Serban Andrei Stan
Serban Andrei Stan
Savin  Tiberiu
Valentin Stanciu
Adrian Budau

trunk/common/ban.php (Diff revision 9)
You don't need the information from the db_query now. You can remove $query = 
trunk/common/ban.php (Diff revision 9)
If you want a single line add to the MySql query LIMIT 1, otherwise the query will go through all the records.
  1. You can also get only the third column. Don't just SELECT everything when you dont need. This goes for everything.
trunk/common/ban.php (Diff revision 9)
use SELECT COUNT FROM so it returns just the number. It's faster
trunk/common/ban.php (Diff revision 9)
return true
Don't do extra stuff unnecessary
trunk/common/ban.php (Diff revision 9)
the same.
And also for comparing things with * you should've used the LIKE keyword. I don't see it here
  1. Nevermind the thing with the LIKE keyword
trunk/scripts/migrate-ban-ip.php (Diff revision 9)
add INDEX for username and expire so the query will go faster. After PRIMARY KEY (id) and before the closing bracket add this 
, INDEX username(`username`)
, INDEX expire(`expire`)
trunk/scripts/migrate-ban-ip.php (Diff revision 9)
same here just for username. I don't think you'll use a query SELECT * FROM ia_user_ip WHERE ip = something. 
trunk/www/controllers/login.php (Diff revision 9)
If the user is good and he is banned $errors is true while $user is also true. Shouldn't this else go elsewhere?
Bogdan-Cristian Tătăroiu
Is this the last version of the diff? I see a review from Adrian with issues marked resolved and dropped but no update.