Skip to:
Content

BuddyPress.org

Opened 5 years ago

Closed 5 years ago

#6111 closed defect (bug) (fixed)

User can input old password as new

Reported by: slaFFik Owned by: imath
Milestone: 2.3 Priority: normal
Severity: normal Version:
Component: Settings Keywords: good-first-bug has-patch needs-testing
Cc:

Description

... when changing the password on /members/slaffik/settings/

Not sure this is a right decision. IMO we should check that they are different (old vs new)

Attachments (2)

6111.diff (1.2 KB) - added by henry.wright 5 years ago.
6111.02.diff (1.2 KB) - added by henry.wright 5 years ago.

Download all attachments as: .zip

Change History (14)

#1 @r-a-y
5 years ago

That would require keeping track of passwords.

If we decided to do this, do we keep this as a user meta entry with an array of the older hashed passwords? This doesn't sound good from a security standpoint:
http://resources.infosecinstitute.com/wordpress-password-hashes/

The default hashing algorithm can be cracked with brute-force.

#2 @DJPaul
5 years ago

  • Keywords needs-patch good-first-bug added
  • Milestone changed from Awaiting Review to Future Release

I think slaFFik is saying you can complete this form by putting the same password in the "old" and "new" screens, and is suggesting we ought to displaying an error if the new password is identical. I think it's worth adding such feedback to the UI, or, surfacing WordPress' error messages if this is another one we are hiding.

#3 @boonebgorges
5 years ago

Yeah, assuming that DJPaul's interpretation is correct, let's do this. If this is about keeping password history, I say wontfix - this is WP territory.

#4 @henry.wright
5 years ago

Just to chip in with my two pennies worth...

If a member has changed their password, then that indicates either a) they had forgotten their old password or b) their old password had been compromised. If it's the case of the latter and at a later date the member again changes their password (this time to the old compromised password perhaps forgetting it was compromised in the past) then that could pose a security threat.

If remembering old passwords is a trivial thing to do then I'd opt for that approach.

Last edited 5 years ago by henry.wright (previous) (diff)

#5 @slaFFik
5 years ago

In this ticket I meant exactly what Paul described.

#6 @henry.wright
5 years ago

  • Keywords has-patch added; needs-patch removed

@slaFFik cool :)

In that case, please see 6111.diff which checks if the user-submitted new password is different to the user-submitted current password. If they're the same then an error message is given.

@henry.wright
5 years ago

#7 @DJPaul
5 years ago

  • Keywords needs-testing added
  • Milestone changed from Future Release to 2.3

#8 @slaFFik
5 years ago

Thanks for the patch!

Regarding different to in error message:

http://www.oxforddictionaries.com/words/different-from-than-or-to

Perhaps, different from should be used instead?

Last edited 5 years ago by slaFFik (previous) (diff)

#9 @henry.wright
5 years ago

You're right. "different from" is more commonly used. I blame my local dialect :)

See 6111.02.diff

@henry.wright
5 years ago

#10 @imath
5 years ago

  • Owner set to imath
  • Resolution set to fixed
  • Status changed from new to closed

In 9628:

In user settings, make sure current password does not match new password

When the user is changing his password, it will display an error message if the new password is the same as the current password.

Props henry.wright

Fixes #6111

#11 @imath
5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

We were too quick on the latest commit. The super admin do not have to specify his current password to change his password ( the "pwd" input is not set ).

As a result when a super admin changes his password from the user settings, we get a notice.

#12 @imath
5 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 9631:

Settings component: skip the check between new and current password for the super administrator

Fixes #6111

Note: See TracTickets for help on using tickets.