Skip to content

Commit

Permalink
Return a 422 from FailureApp when login fails
Browse files Browse the repository at this point in the history
Devise `FailureApp` now returns a `422` status when running the `recall` 
flow. This, combined with heartcombo/responders#223, enables Devise to 
work correctly with the [new Rails defaults](rails/rails#41026) for form 
errors, as well as with new libraries like Turbo.

By default, the `recall` flow only runs on the 
[`SessionsController`](https://github.com/heartcombo/devise/blob/v4.7.3/app/controllers/devise/sessions_controller.rb#L48).
  • Loading branch information
baarkerlounger committed Feb 28, 2022
1 parent b9e9767 commit 5329ea5
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 7 deletions.
26 changes: 26 additions & 0 deletions CHANGELOG.md
Expand Up @@ -2,6 +2,32 @@

* enhancements
* Add support for Rails 7.0. Please note that Turbo integration is not fully supported by Devise yet.
* Devise now treats `:turbo_stream` as a navigational format.
- For Turbo Drive users this ensures that Devise will return a redirect, rather than a `401`, where appropriate (this may be a **breaking change**).
- This has no impact if you are not using [turbo-rails](https://github.com/hotwired/turbo-rails) or declaring an equivalent MIME type.
* Devise `FailureApp` now returns a `422` status when running the `recall` flow.
- This, combined with https://github.com/heartcombo/responders/pull/223, enables Devise to work correctly with the [new Rails defaults](https://github.com/rails/rails/pull/41026) for form errors, as well as with new libraries like Turbo.
- By default, the `recall` flow only runs on the [`SessionsController`](https://github.com/heartcombo/devise/blob/v4.7.3/app/controllers/devise/sessions_controller.rb#L48), but you should check if it is required in other flows too.
- This may be a **breaking change**.
- If you want to keep the old behavior, or return a different code, you can override `recall_response_code` in a custom failure app:

```ruby
# config/initializers/devise.rb
class MyFailureApp < Devise::FailureApp
def recall_response_code(app_response_code)
# Return any HTTP status code you like, or return `app_response_code` to keep whatever your app returned and not override it.
# By default this method returns `422`.
app_response_code
end
end
Devise.setup do |config|
# ...
config.warden do |manager|
manager.failure_app = MyFailureApp
end
# ...
end
```

### 4.8.0 - 2021-04-29

Expand Down
2 changes: 1 addition & 1 deletion lib/devise.rb
Expand Up @@ -217,7 +217,7 @@ module Test

# Which formats should be treated as navigational.
mattr_accessor :navigational_formats
@@navigational_formats = ["*/*", :html]
@@navigational_formats = ["*/*", :html, :turbo_stream]

# When set to true, signing out a user signs out all other scopes.
mattr_accessor :sign_out_all_scopes
Expand Down
10 changes: 8 additions & 2 deletions lib/devise/failure_app.rb
Expand Up @@ -71,7 +71,9 @@ def recall
end

flash.now[:alert] = i18n_message(:invalid) if is_flashing_format?
self.response = recall_app(warden_options[:recall]).call(request.env)
response_from_app = recall_app(warden_options[:recall]).call(request.env)
response_from_app[0] = recall_response_code(response_from_app[0])
self.response = response_from_app
end

def redirect
Expand All @@ -89,6 +91,10 @@ def redirect

protected

def recall_response_code(_original_response_code)
422
end

def i18n_options(options)
options
end
Expand Down Expand Up @@ -170,7 +176,7 @@ def scope_url
end

def skip_format?
%w(html */*).include? request_format.to_s
%w(html turbo_stream */*).include? request_format.to_s
end

# Choose whether we should respond in an HTTP authentication fashion,
Expand Down
2 changes: 1 addition & 1 deletion lib/generators/templates/devise.rb
Expand Up @@ -263,7 +263,7 @@
# should add them to the navigational formats lists.
#
# The "*/*" below is required to match Internet Explorer requests.
# config.navigational_formats = ['*/*', :html]
# config.navigational_formats = ['*/*', :html, :turbo_stream]

# The default HTTP method used to sign out a resource. Default is :delete.
config.sign_out_via = :delete
Expand Down
6 changes: 3 additions & 3 deletions test/controllers/sessions_controller_test.rb
Expand Up @@ -19,7 +19,7 @@ class SessionsControllerTest < Devise::ControllerTestCase
password: "wrongpassword"
}
}
assert_equal 200, @response.status
assert_equal 422, @response.status
ensure
ActiveSupport::Notifications.unsubscribe(subscriber)
end
Expand All @@ -29,7 +29,7 @@ class SessionsControllerTest < Devise::ControllerTestCase
swap Devise, scoped_views: true do
request.env["devise.mapping"] = Devise.mappings[:user]
post :create
assert_equal 200, @response.status
assert_equal 422, @response.status
assert_template "users/sessions/new"
end
end
Expand Down Expand Up @@ -70,7 +70,7 @@ class SessionsControllerTest < Devise::ControllerTestCase
password: "wevdude"
}
}
assert_equal 200, @response.status
assert_equal 422, @response.status
assert_template "devise/sessions/new"
end

Expand Down
16 changes: 16 additions & 0 deletions test/failure_app_test.rb
Expand Up @@ -238,9 +238,20 @@ def call_failure(env_params = {})
test 'redirects the correct format if it is a non-html format request' do
swap Devise, navigational_formats: [:js] do
call_failure('formats' => Mime[:js])
assert_equal 302, @response.first
assert_equal 'http://test.host/users/sign_in.js', @response.second["Location"]
end
end

test 'redirects turbo_stream correctly' do
Mime::Type.register "text/vnd.turbo-stream.html", :turbo_stream

swap Devise, navigational_formats: [:html, :turbo_stream] do
call_failure('formats' => Mime[:turbo_stream])
assert_equal 302, @response.first
assert_equal 'http://test.host/users/sign_in', @response.second["Location"]
end
end
end

context 'For HTTP request' do
Expand Down Expand Up @@ -335,6 +346,7 @@ def call_failure(env_params = {})
"warden" => stub_everything
}
call_failure(env)
assert_equal 422, @response.first
assert_includes @response.third.body, '<h2>Log in</h2>'
assert_includes @response.third.body, 'Invalid email or password.'
end
Expand All @@ -346,6 +358,7 @@ def call_failure(env_params = {})
"warden" => stub_everything
}
call_failure(env)
assert_equal 422, @response.first
assert_includes @response.third.body, '<h2>Log in</h2>'
assert_includes @response.third.body, 'You have to confirm your email address before continuing.'
end
Expand All @@ -357,6 +370,7 @@ def call_failure(env_params = {})
"warden" => stub_everything
}
call_failure(env)
assert_equal 422, @response.first
assert_includes @response.third.body, '<h2>Log in</h2>'
assert_includes @response.third.body, 'Your account is not activated yet.'
end
Expand All @@ -370,6 +384,7 @@ def call_failure(env_params = {})
"warden" => stub_everything
}
call_failure(env)
assert_equal 422, @response.first
assert_includes @response.third.body, '<h2>Log in</h2>'
assert_includes @response.third.body, 'Invalid email or password.'
assert_equal '/sample', @request.env["SCRIPT_NAME"]
Expand All @@ -384,6 +399,7 @@ def call_failure(env_params = {})
assert_equal "yes it does", Devise::FailureApp.new.lazy_loading_works?
end
end

context "Without Flash Support" do
test "returns to the default redirect location without a flash message" do
call_failure request_klass: RequestWithoutFlashSupport
Expand Down

0 comments on commit 5329ea5

Please sign in to comment.