[Rejected] Rewrite of the login rewrite
#1
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...
You found my signature? Well done : - )
Reply
#2
I agree - this part needs some love in future
[MyBB 1.8 Czech translation] [MyBB 1.8 plugins]: Prune old PMs + optimize DB plugin --- Thank you/like system
Reply
#3
Bumping this.

Everything except the 1st point still applies, am I correct Nik?
Reply
#4
@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)
You found my signature? Well done : - )
Reply
#5
A few month later again: What has been fixed, what needs to be fixed and do we want to fix them at this point?
Support PMs will be ignored!
Reply
#6
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?
Reply
#7
Ufff Big Grin That was quite some time ago and I'm not that sure what still applies, because Omar fixed stuff around the login (https://github.com/mybb/mybb/issues/1376)

The third point - https://github.com/mybb/mybb/blob/featur....php#L6185
Only the cookie is checked, but not the database entry for the user that tries to login... I'm not sure, if this is intended, because deleting cookies is not that hard...
You found my signature? Well done : - )
Reply
#8
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).
Support PMs will be ignored!
Reply
#9
(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).
Reply


Forum Jump:


Users browsing this thread: 1 Guest(s)