Yeah I know, the login has been rewritten for 1.8, but
- login methods email / email + username where not implemented correctly (I fixed this, but it's quite dirty due to the next point in my list)
- there are up to 4 database queries depending on the case fetching nearly the same data (failed login count, username, password, updating failed login count, fetching failed login count)
- Display number of failed login settings does not work (setting to yes does not show the remaining login attempts)
- you have an unlimited number of guesses for the Admin CP Pin
- The failed login count in the database is only used for the captcha setting, not for blocking the entire login
- When a captcha is required the login page does not show the captcha the first time it is required
I can create a thread per issue, but I think dealing with those problems in one step might be easier, because it would probably be a rewrite and dealing with one issue at a time might result in really weird code...
I agree - this part needs some love in future
Bumping this.
Everything except the 1st point still applies, am I correct Nik?
@Destroy666 I did not test this specifically since I posted this thread, but I think so (did not notice much work on the login code in member.php)
A few month later again: What has been fixed, what needs to be fixed and do we want to fix them at this point?
Login methods and limited PIN guesses were implemented, the rest probably not (I'll try to check it later today):
(2014-09-04, 11:38 AM)Nik101010 Wrote: [ -> ]- there are up to 4 database queries depending on the case fetching nearly the same data (failed login count, username, password, updating failed login count, fetching failed login count)
- Display number of failed login settings does not work (setting to yes does not show the remaining login attempts)
- The failed login count in the database is only used for the captcha setting, not for blocking the entire login
- When a captcha is required the login page does not show the captcha the first time it is required
At least the 2nd and 4th bullet should definitely be fixed IMO. 1st - optimization - wouldn't be bad either. 3rd one - not sure what was meant - the user isn't blocked from logging in after X attempts for a specific period of time or?
So the only issue left is the loginattempts check? There's some internal discussion about this so I'd keep this on hold as long as that hasn't been fixed (we may change some things related to it then).
(2015-07-02, 08:39 AM)Jones H Wrote: [ -> ]So the only issue left is the loginattempts check?
Noone said that, so not sure what you mean (unless you can't reproduce the rest yourself).
The 4 issues I quoted above are most likely still present, I can definitely confirm the 1st (some queries look like redundan duplicates, not sure if we can/should optimise them) and 2nd (I can't see the number of failed/remaining tries anywhere), the 4th is still there too IIRC (there is sometimes a captcha error without showing the captcha first).
The 3rd is connected with the internal discussion indeed, I'm not sure how exactly it works, the code is quite messy and spread among loginhandler, member.php login action and function file(s).
That was quite some time ago and I'm not that sure what still applies, because Omar fixed stuff around the login (