Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CANTINA-954: Security: Use ambiguous error message in forgot password #4973

Merged
merged 4 commits into from
Oct 25, 2023

Conversation

rebeccahum
Copy link
Contributor

Description

We should use an ambiguous error message in the Forgot Password, since the messages confirm if a user has entered a correct username or email in the "Forgot Password" form.

Changelog Description

Updated: Forgot Password

When users hit the Forgot Password in wp-login, it will no longer confirm if their email or username exists.

Pre-review checklist

Please make sure the items below have been covered before requesting a review:

  • This change works and has been tested locally (or has an appropriate fallback).
  • This change works and has been tested on a Go sandbox.
  • This change has relevant unit tests (if applicable).
  • This change uses a rollout method to ease with deployment (if applicable - especially for large scale actions that require writes).
  • This change has relevant documentation additions / updates (if applicable).
  • I've created a changelog description that aligns with the provided examples.

Pre-deploy checklist

  • VIP staff: Ensure any alerts added/updated conform to internal standards (see internal documentation).

Steps to Test

  1. Go to wp-admin > Forgot password
  2. Test with an invalid email e.g. [email protected]
  3. Test with a valid username e.g. vipgo
  4. Both should display the same error message

@rebeccahum rebeccahum force-pushed the change-login-message branch from f14bd78 to e6cdf02 Compare October 24, 2023 18:03
@codecov
Copy link

codecov bot commented Oct 24, 2023

Codecov Report

Merging #4973 (9988539) into develop (15a311a) will increase coverage by 0.03%.
The diff coverage is 88.88%.

@@              Coverage Diff              @@
##             develop    #4973      +/-   ##
=============================================
+ Coverage      28.76%   28.79%   +0.03%     
  Complexity      4742     4742              
=============================================
  Files            278      278              
  Lines          20896    20904       +8     
=============================================
+ Hits            6010     6019       +9     
+ Misses         14886    14885       -1     
Files Coverage Δ
security/login-error.php 37.50% <88.88%> (+37.50%) ⬆️

@rebeccahum rebeccahum marked this pull request as ready for review October 24, 2023 18:34
@rebeccahum rebeccahum requested a review from a team as a code owner October 24, 2023 18:34
sjinks and others added 2 commits October 24, 2023 23:15
mebbe fix test with type checking
@rebeccahum rebeccahum force-pushed the change-login-message branch from a64c315 to 9988539 Compare October 24, 2023 21:28
@rebeccahum rebeccahum merged commit 09fffbe into develop Oct 25, 2023
41 checks passed
@rebeccahum rebeccahum deleted the change-login-message branch October 25, 2023 14:10
@@ -1,13 +1,17 @@
<?php
namespace Automattic\VIP\Security;

use WP_Error;

const FORGET_PWD_MESSAGE = 'If there is an account associated with the username/email address, you will receive an email with a link to reset your password.';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be a translatable string :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants