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

Add failing test case for bad parameter sanitizing when failing to login #2452

Merged

Conversation

latortuga
Copy link
Contributor

Failing test for #2440. This was hard to track down because the implementation for StrongParameters' action_on_unpermitted_parameters either doesn't work or isn't documented correctly and it doesn't seem to raise an error as expected.

@josevalim
Copy link
Contributor

@latortuga Great work! Can you provide a fix as well @latortuga? We should just add the missing ones to our list. :)

@josevalim
Copy link
Contributor

@latortuga Hey, can you help us tidy this up? Feel free to merge your tests and the proper fix. :) ❤️

@latortuga
Copy link
Contributor Author

Yes, I'll try to get it today or tomorrow. I've been out of town and work has been pretty busy, sorry for the delay!

This includes a failing test case that hooks into ActiveSupport
Notifications to catch the param permit error.
josevalim pushed a commit that referenced this pull request Jul 9, 2013
…ogin

Add failing test case for bad parameter sanitizing when failing to login
@josevalim josevalim merged commit bc598b9 into heartcombo:master Jul 9, 2013
@joaodiogocosta
Copy link

@josevalim
Copy link
Contributor

It is pretty hard to help if you don't include at least your Devise and Rails versions. Please take a look at our CONTRIBUTING.md guide for some tips on how to write good reports.

@joaodiogocosta
Copy link

I'm sorry, my bad. Here it is: Rails 4 and Devise 3.0.0rc

@josevalim
Copy link
Contributor

Have you tried Devise 3.0.0 final? It was launched this sunday.

@joaodiogocosta
Copy link

Nice, didn't know that. I'll try and tell about it later. Thanks

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

Successfully merging this pull request may close these issues.

None yet

3 participants