passwords should not be filtered on input

[cash]

This has significant effects if authenticating against external source - whether slaving Elgg to another app or using something like ldap.

[brettp]

This can break existing logins. Appropriate for a bugfix release?

[ewinslow]

Do we have a workaround for those cases?

[brettp]

A few options:

1. Upon login, check for passwords that would be changed by filtering and let them pass through with a prompt to change the password.
2. Check for bad passwords upon login, force reset the password.
3. Do nothing, expect users to reset their password when they can't log in.

[cash]

This is a bug. It seems fixing it in a bugfix release is appropriate.

Here's the main issue: those who use external authentication (like LDAP) need the actual password. Someone posted in the community forums about having trouble with this recently which motivated me to schedule this. This is really hard to track down, too.

Because we cannot assume a site is using the built-in password checking, we cannot force a password reset or let a user in if the password fails. This leaves the options:

1. Let a user reset the password (maybe letting the user know they need to reset their password)
2. Add an admin notice that has the user's username so the admin can assist the user

We had about 3K users before the first time someone reported being unable to log in. That makes me think that most people do not use <> characters in their passwords.

[mrclay]

Replying to brettp:

1. Upon login, check for passwords that would be changed by filtering and let them pass through with a prompt to change the password.

Define "pass through" and "bad passwords"?

3. Do nothing, expect users to reset their password when they can't log in.

Although a bummer for some users, I think this is the only option that isn't going to require storing a flag and generate a lot of messy code that we'd have to keep for a long time. E.g. consider this 4th option:

Capture the login password as both $unescaped and $filtered. If the user has a flag, use $unescaped to verify the hash. If not, check that $filtered matches the hash and that $unescaped passes the password validation. If so, hash and store $unescaped and store the user flag. If $unescaped is suddenly not a valid password (a case like "abc&"), initiate a password reset.

Good: most users won't even notice. Bad: more complexity, need for flag storage, seals in messy code.

Fun Aside: The password "a&<bcd1234>" (a very strong password!) is filtered to "a&amp;", which (currently) passes the 6-char minimum, letting you log in with just "a&" :)

[brettp]

Replying to mrclay:

Replying to brettp:

1. Upon login, check for passwords that would be changed by filtering and let them pass through with a prompt to change the password.

Define "pass through" and "bad passwords"?

"pass through" == "login"
"bad passwords" == "passwords that would be escaped"

3. Do nothing, expect users to reset their password when they can't log in.

Although a bummer for some users, I think this is the only option that isn't going to require storing a flag and generate a lot of messy code that we'd have to keep for a long time.

Cash convinced me this is the way to go. I'm planning do this with a system_message() call for the user.

Capture the login password as both $unescaped and $filtered. If the user has a flag, use $unescaped to verify the hash. If not, check that $filtered matches the hash and that $unescaped passes the password validation. If so, hash and store $unescaped and store the user flag. If $unescaped is suddenly not a valid password (a case like "abc&"), initiate a password reset.

Good: most users won't even notice. Bad: more complexity, need for flag storage, seals in messy code.

Yeah I agree the bad outweighs the good on this one.

Fun Aside: The password "a&<bcd1234>" (a very strong password!) is filtered to "a&amp;", which (currently) passes the 6-char minimum, letting you log in with just "a&" :)

I'll use that for my tests ;)

[brettp]

What about another option: An upgrade to add a persistent admin notice saying we've changed how passwords are handled and it could affect a small number of users who should reset their password if they can't log in. We wouldn't have to keep any extra code around in core for that at all.

[ewinslow]

Posting an admin notice sounds like sufficient notification about this.

[Brett Profitt]

Fixes #1301. Not filtering passwords.

Changeset: 8aad9f081b9fd83f8cd8358547234fbdcdaf9611

[Brett Profitt]

Fixes #1301. Not filtering passwords.

Changeset: 8aad9f081b9fd83f8cd8358547234fbdcdaf9611