Skip to content

Conversation

@ernie
Copy link

@ernie ernie commented Dec 17, 2015

The overridden methods in UserPasswordsController were inadvertently
missing out on Devise's "paranoid mode", which protects against user
enumeration attacks.

Current dependency is Devise ~> 3.4.1 according to the gemspec, which
calls a separate method to determine redirect location.

All existing specs still pass. Added a spec for nonexistent users. The
situation with existing users requires a good deal more setup since it
would trigger the e-mail.

The overridden methods in UserPasswordsController were inadvertently
missing out on Devise's "paranoid mode", which protects against user
enumeration attacks.

Current dependency is Devise ~> 3.4.1 according to the gemspec, which
calls a separate method to determine redirect location.

All existing specs still pass. Added a spec for nonexistent users. The
situation with existing users requires a good deal more setup since it
would trigger the e-mail.
Copy link
Contributor

@kushniryb kushniryb left a comment

Choose a reason for hiding this comment

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

Please rebase against master & fix comments

expect(flash[:notice]).to eq I18n.t(:send_paranoid_instructions, scope: [:devise, :user_passwords, :spree_user])
end
end

Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove empty line

end

end

Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove empty line


context 'with paranoid mode' do
before { Devise.paranoid = true }
after { Devise.paranoid = false }
Copy link
Contributor

Choose a reason for hiding this comment

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

Add empty line after after


context 'when resetting password' do
it 'puts an error on the object' do
spree_post :create, spree_user: {email: 'made-up-email@made-up-domain.com'}
Copy link
Contributor

Choose a reason for hiding this comment

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

Add empty line after spree_post & add space after hash key and hash value

before { Devise.paranoid = true }
after { Devise.paranoid = false }
it 'does not indicate whether the user exists' do
spree_post :create, spree_user: {email: 'made-up-email@made-up-domain.com'}
Copy link
Contributor

Choose a reason for hiding this comment

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

Add empty line & fix hash formatting

it 'does not indicate whether the user exists' do
spree_post :create, spree_user: {email: 'made-up-email@made-up-domain.com'}
expect(response).to redirect_to spree.login_path
expect(flash[:notice]).to eq I18n.t(:send_paranoid_instructions, scope: [:devise, :user_passwords, :spree_user])
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use %i[] instead of [:a, :b]

spree_post :create, spree_user: {email: 'made-up-email@made-up-domain.com'}
expect(response).to be_success
expect(assigns(:spree_user).kind_of?(Spree::User)).to eq true
expect(assigns(:spree_user).errors.messages[:email].first).to eq I18n.t(:not_found, scope: [:errors, :messages])
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use %i[] instead of [:a, :b]

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants