From 78c6395e5f42ce0f5f4570fb35676eb669e52430 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 10 Apr 2019 12:41:21 +0200 Subject: [PATCH] Respond with 404 when confirming an invalid token We were getting a 500 Internal Server Error because `find_by` returned `nil`, but the code assumed it returned an object responding to `encrypted_password`. In this case, maybe some other status code (like 400 or 401) might be more appropriate, but I've kept 404 because it was easier to implement and I wasn't sure which one was better. Also note ideally we would test the controller using: expect(response).to have_http_status(:not_found) However, we would need to configure the test to show exceptions and not to consider all requests local. I haven't been able to do so for controller tests, and doing so for feature/request specs seems to require changes in the test environment configuration which would affect other tests. --- app/controllers/users/confirmations_controller.rb | 2 +- .../users/confirmations_controller_spec.rb | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 spec/controllers/users/confirmations_controller_spec.rb diff --git a/app/controllers/users/confirmations_controller.rb b/app/controllers/users/confirmations_controller.rb index de65e592a..3ffced63e 100644 --- a/app/controllers/users/confirmations_controller.rb +++ b/app/controllers/users/confirmations_controller.rb @@ -27,7 +27,7 @@ class Users::ConfirmationsController < Devise::ConfirmationsController def show # In the default implementation, this already confirms the resource: # self.resource = self.resource = resource_class.confirm_by_token(params[:confirmation_token]) - self.resource = resource_class.find_by(confirmation_token: params[:confirmation_token]) + self.resource = resource_class.find_by!(confirmation_token: params[:confirmation_token]) yield resource if block_given? diff --git a/spec/controllers/users/confirmations_controller_spec.rb b/spec/controllers/users/confirmations_controller_spec.rb new file mode 100644 index 000000000..bcb185ef8 --- /dev/null +++ b/spec/controllers/users/confirmations_controller_spec.rb @@ -0,0 +1,13 @@ +require "rails_helper" + +describe Users::ConfirmationsController do + before do + @request.env["devise.mapping"] = Devise.mappings[:user] + end + + describe "GET show" do + it "returns a 404 code with a wrong token" do + expect { get :show, token: "non_existent" }.to raise_error ActiveRecord::RecordNotFound + end + end +end