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.

Sign up for GitHub

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

THREESCALE-10663: Do not reveal that email doesn't exist on Forgot password form #3837

Merged
merged 3 commits into from
Jul 5, 2024
Merged

THREESCALE-10663: Do not reveal that email doesn't exist on Forgot password form #3837

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4911613
Do not reveal that email doesn't exist on Forgot password form
mayorova Jun 25, 2024
a97e02f
Some fixes
mayorova Jun 26, 2024
5226eda
Reorganize cucumbers for password reset
mayorova Jul 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions features/old/accounts/buyers/buyer_password_reset.feature
View file
Open in desktop
mayorova marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ Feature: Buyer password reset
Given I follow "Forgot password?"
And I fill in "Email" with "zed@3scale.localhost"
And I press "Send instructions"
Then I should see "A password reset link has been emailed to you."
Then I should see "A password reset link has been emailed to: zed@3scale.localhost."
When I follow the link found in the password reset email send to "zed@3scale.localhost"
And I fill in "Password" with "monkey"
And I fill in "Password confirmation" with "monkey"
Expand All @@ -32,7 +32,8 @@ Feature: Buyer password reset
And I follow "Forgot password?"
And I fill in "Email" with "bob@3scale.localhost"
And I press "Send instructions"
Then I should see "Email not found."
Then I should see "A password reset link has been emailed to: bob@3scale.localhost."
And "bob@3scale.localhost" should receive no emails

Scenario: Wrong confirmation
Given I follow "Forgot password?"
Expand Down Expand Up @@ -65,4 +66,4 @@ Feature: Buyer password reset
When I follow "Forgot password?"
And I fill in "Email" with "zed@3scale.localhost"
And I press "Send instructions"
Then I should see "A password reset link has been emailed to you."
Then I should see "A password reset link has been emailed to: zed@3scale.localhost."
8 changes: 4 additions & 4 deletions lib/developer_portal/app/controllers/developer_portal/admin/account/passwords_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ class DeveloperPortal::Admin::Account::PasswordsController < ::DeveloperPortal::
def create
return redirect_to_request_password('Bot protection failed.') unless bot_check({ flash: false })

user = @provider.buyer_users.find_by_email(params[:email])
return redirect_to_request_password('Email not found.') unless user
email = params[:email]
user = @provider.buyer_users.find_by(email: email)
user&.generate_lost_password_token!

user.generate_lost_password_token!
flash[:notice] = 'A password reset link has been emailed to you.'
flash[:notice] = "A password reset link has been emailed to: #{email}."
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
flash[:notice] = "A password reset link has been emailed to: #{email}."
flash[:notice] = "A password reset link will be emailed if a user exists registered with #{email}."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds reasonable, but I just followed the example we have for the admin portal password reset:

porta/config/locales/en.yml

Lines 397 to 398 in cfc866f

We sent an email with password reset instructions to: %{email}. If you do not
receive it soon, check your spam folder or request a password reset again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, in this case this message appears in a flash message, so we need to keep it pretty short. Let me see...

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I prefer the short version

Copy link
Contributor Author

@mayorova mayorova Jun 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Actually, I think the longer version doesn't look too bad:

Screenshot from 2024-06-27 11-34-51

(I tweaked a bit the suggestion)

redirect_to login_url
end

Expand Down
6 changes: 3 additions & 3 deletions test/integration/developer_portal/passwords_controller_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,14 @@ def test_update_password
post developer_portal.admin_account_password_path(email: user.email)
end
assert_in_delta Time.now, user.reload.lost_password_token_generated_at, 2.seconds
assert_equal 'A password reset link has been emailed to you.', flash[:notice]
assert_equal "A password reset link has been emailed to: #{user.email}.", flash[:notice]
assert_redirected_to developer_portal.login_path
end

test 'create renders the right error message when the email is not found' do
post developer_portal.admin_account_password_path(email: 'fake@example.com')
assert_equal 'Email not found.', flash[:error]
assert_redirected_to developer_portal.new_admin_account_password_path(request_password_reset: true)
assert_equal "A password reset link has been emailed to: fake@example.com.", flash[:notice]
assert_redirected_to developer_portal.login_path
end

private
Expand Down