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

Support Devise paranoid mode #310

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

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.

None yet

2 participants