Generic Login Error Message

[ximex]

We should change the behavior of the printed error messages at the login process.

Today:

• username found, password correct, activated: do login
• username found, password correct, not activated: not activated message
• username found, password false, activated: false password message
• username found, password false, not activated: not activated message
• username not found: not found message

Should change to:

• username found, password correct, activated: do login
• username found, password correct, not activated: not activated message
• username found, password false, activated: Login incorrect (username OR password)
• username found, password false, not activated: Login incorrect (username OR password)
• username not found: Login incorrect (username OR password)

We shouldn't give an attacker infos about if a username exists e.g. This way is the common secure way

[ximex]

it looks if this all has to be done in "login_check.php"

[Fasse]

+1 this should be done!

[ximex]

also change the case insensive username checking to a case sensitive check. WHERE UPPER(usr_login_name) LIKE UPPER(\''.$loginname.'\') to WHERE usr_login_name LIKE \''.$loginname.'\'

[ximex]

And PLEASE use Joins!!! the code looks like as it really needs a rewrite ;-)

[sistlind]

I think usernames should not case sensitive, there is not much security improvement, but it can be complicated, if there are several users differing only in case. Often people using their email and this is also not case-sensitive.

Perhaps the better way is to allow only lowercase usernames at all.

[Fasse]

The username should not be case sensitiv. This is easier for users to register.

[ximex]

The username should not be case sensitiv.

ok

[Fasse]

@ximex do you implement this?

[ximex]

if i found time for this yes. shouldn't be that hard

[Fasse]

The release of 3.1 is "far" away ;)

[ximex]

first part done: https://github.com/Admidio/admidio/commit/bd85d3cbfc93f88b98f604979f877bc64a561058

Edit: Updated commit id

[ximex]

Flow should be:

1. check input (not empty)
2. find user
3. check password
4. check if activated

[ximex]

ok we have to look at other things too: is the user currently a member of valid role? What should we do if:

• username & password correct & activated, currently member of invalid role
• username & password correct & activated, was/will be (set for in future) member of a valid role
• username & password correct & activated, isn't a member of any role

I think "Login incorrect (username OR password)" is wrong in all 3 possibilities

If "username & password correct & NOT activated" with any state of the membership or role status above should always the message "not activated"

so the new flow:

1. check input (not empty)
2. find user
3. check if max false passwords <= 3
4. check password
5. check if activated
6. check membership/role state

So we need the messages for the 3 possibilities above to set the behaviour of Point 5.

Edit: change flow (add check for 3 false passwords)

[ximex]

I'm nearly finished with the code. Now i have to change the code for the update login. The other things work. I used "Anmeldedaten korrekt, jedoch ist dieser User aktuell kein gültiges Mitglied einer Organisation." for all 3 possibilities from the last message.

[ximex]

@Fasse please check

One more message added for "no webadmin"

[Fasse]

looks good to me, some basic checks were ok. only changed some messages.