WordPress.org

Make WordPress Core

Opened 6 weeks ago

Closed 6 weeks ago

#51922 closed defect (bug) (fixed)

Application Passwords: Ambiguous error when recording password usage

Reported by: dlh Owned by: TimothyBlynJacobs
Milestone: 5.6 Priority: normal
Severity: normal Version: 5.6
Component: Application Passwords Keywords: has-patch commit dev-reviewed
Focuses: Cc:

Description

WP_Application_Passwords::record_application_password_usage() returns "True if the usage was recorded, a WP_Error if an error occurs."

If a password has already been used in the last day, the new usage isn't recorded, and an error is returned. However, the error code given is application_password_not_found, which doesn't seem accurate.

The attached patch would have the method return true when a password's usage has already been recorded for the day on the theory that, at the time, no error occurred.

Alternatively, a WP_Error with a different error code specific to this case could be returned on the theory that the usage was not recorded this time.

Attachments (1)

51922.diff (494 bytes) - added by dlh 6 weeks ago.

Download all attachments as: .zip

Change History (7)

@dlh
6 weeks ago

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


6 weeks ago

#2 @SergeyBiryukov
6 weeks ago

  • Milestone changed from Awaiting Review to 5.6

Thanks for the patch! Moving to the milestone for visibility.

#3 @TimothyBlynJacobs
6 weeks ago

Great catch @dlh! Yeah I think returning true makes sense here. I agree it isn't a 5.6 showstopper, will defer to @SergeyBiryukov on whether we should backport.

#4 @TimothyBlynJacobs
6 weeks ago

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

In 49739:

App Passwords: Return true when rate limiting a password's last used time.

Previously we returned a WP_Error instance saying that the password was not found which is inaccurate.

Props dlh.
Fixes #51922.

#5 @SergeyBiryukov
6 weeks ago

  • Keywords commit dev-reviewed added
  • Resolution fixed deleted
  • Status changed from closed to reopened

[49739] looks good to backport.

#6 @SergeyBiryukov
6 weeks ago

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

In 49740:

App Passwords: Return true when rate limiting a password's last used time.

Previously we returned a WP_Error instance saying that the password was not found which is inaccurate.

Props dlh.
Reviewed by TimothyBlynJacobs, SergeyBiryukov.
Merges [49739] to the 5.6 branch.
Fixes #51922.

Note: See TracTickets for help on using tickets.