Skip to content

Simplifying ConfirmationsController show behavior #1075

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

Merged
merged 1 commit into from
Jan 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 14 additions & 19 deletions app/controllers/devise_token_auth/confirmations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,32 +5,27 @@ class ConfirmationsController < DeviseTokenAuth::ApplicationController
def show
@resource = resource_class.confirm_by_token(params[:confirmation_token])

if @resource.persisted?
expiry = nil
if defined?(@resource.sign_in_count) && @resource.sign_in_count > 0
expiry = (Time.zone.now + 1.second).to_i
end

client_id, token = @resource.create_token expiry: expiry

sign_in(@resource)
@resource.save!

if @resource.errors.empty?
yield @resource if block_given?

redirect_header_options = { account_confirmation_success: true }
redirect_headers = build_redirect_headers(token,
client_id,
redirect_header_options)

# give redirect value from params priority
@redirect_url = params[:redirect_url]
# give redirect value from params priority or fall back to default value if provided
redirect_url = params[:redirect_url] || DeviseTokenAuth.default_confirm_success_url

if signed_in?(resource_name)
client_id, token = signed_in_resource.create_token

# fall back to default value if provided
@redirect_url ||= DeviseTokenAuth.default_confirm_success_url
redirect_headers = build_redirect_headers(token,
client_id,
redirect_header_options)

redirect_to_link = signed_in_resource.build_auth_url(redirect_url, redirect_headers)
else
redirect_to_link = DeviseTokenAuth::Url.generate(redirect_url, redirect_header_options)
end

redirect_to(@resource.build_auth_url(@redirect_url, redirect_headers))
redirect_to(redirect_to_link)
else
raise ActionController::RoutingError, 'Not Found'
end
Expand Down
2 changes: 1 addition & 1 deletion app/models/devise_token_auth/concerns/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def self.tokens_match?(token_hash, token)
devise_modules.delete(:omniauthable)
else
devise :database_authenticatable, :registerable,
:recoverable, :trackable, :validatable, :confirmable
:recoverable, :validatable, :confirmable
end

if const_defined?('ActiveRecord') && ancestors.include?(ActiveRecord::Base)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,6 @@ class DeviseTokenAuthCreate<%= user_class.pluralize.gsub("::","") %> < ActiveRec
## Rememberable
t.datetime :remember_created_at

## Trackable
t.integer :sign_in_count, :default => 0, :null => false
t.datetime :current_sign_in_at
t.datetime :last_sign_in_at
t.string :current_sign_in_ip
t.string :last_sign_in_ip

## Confirmable
t.string :confirmation_token
t.datetime :confirmed_at
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,6 @@ class <%= user_class %>
## Rememberable
field :remember_created_at, type: Time

## Trackable
field :sign_in_count, type: Integer, default: 0
field :current_sign_in_at, type: Time
field :last_sign_in_at, type: Time
field :current_sign_in_ip, type: String
field :last_sign_in_ip, type: String

## Confirmable
field :confirmation_token, type: String
field :confirmed_at, type: Time
Expand Down
61 changes: 41 additions & 20 deletions test/controllers/devise_token_auth/confirmations_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ def token_and_client_config_from(body)
@new_user.send_confirmation_instructions(redirect_url: @redirect_url)
mail = ActionMailer::Base.deliveries.last
@token, @client_config = token_and_client_config_from(mail.body)
@token_params = %w[access-token client client_id config expiry token uid]
end

test 'should generate raw token' do
Expand All @@ -38,32 +39,52 @@ def token_and_client_config_from(body)
end

describe 'success' do
before do
get :show,
params: { confirmation_token: @token,
redirect_url: @redirect_url },
xhr: true
@resource = assigns(:resource)
end
describe 'when authenticated' do
before do
sign_in(@new_user)
get :show,
params: { confirmation_token: @token,
redirect_url: @redirect_url },
xhr: true
@resource = assigns(:resource)
end

test 'user should now be confirmed' do
assert @resource.confirmed?
end
test 'user should now be confirmed' do
assert @resource.confirmed?
end

test 'should redirect to success url' do
assert_redirected_to(/^#{@redirect_url}/)
end
test 'should redirect to success url' do
assert_redirected_to(/^#{@redirect_url}/)
end

test 'the sign_in_count should be 1' do
assert @resource.sign_in_count == 1
test 'redirect url includes token params' do
assert @token_params.all? { |param| response.body.include?(param) }
assert response.body.include?('account_confirmation_success')
end
end

test 'User shoud have the signed in info filled' do
assert @resource.current_sign_in_at?
end
describe 'when unauthenticated' do
before do
sign_out(@new_user)
get :show,
params: { confirmation_token: @token,
redirect_url: @redirect_url },
xhr: true
@resource = assigns(:resource)
end

test 'User shoud have the Last checkin filled' do
assert @resource.last_sign_in_at?
test 'user should now be confirmed' do
assert @resource.confirmed?
end

test 'should redirect to success url' do
assert_redirected_to(/^#{@redirect_url}/)
end

test 'redirect url does not include token params' do
refute @token_params.any? { |param| response.body.include?(param) }
assert response.body.include?('account_confirmation_success')
end
end
end

Expand Down
38 changes: 0 additions & 38 deletions test/controllers/devise_token_auth/sessions_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,6 @@ class DeviseTokenAuth::SessionsControllerTest < ActionController::TestCase

describe 'success' do
before do
@old_sign_in_count = @existing_user.sign_in_count
@old_current_sign_in_at = @existing_user.current_sign_in_at
@old_last_sign_in_at = @existing_user.last_sign_in_at
@old_sign_in_ip = @existing_user.current_sign_in_ip
@old_last_sign_in_ip = @existing_user.last_sign_in_ip

post :create,
params: {
email: @existing_user.email,
Expand All @@ -31,12 +25,6 @@ class DeviseTokenAuth::SessionsControllerTest < ActionController::TestCase

@resource = assigns(:resource)
@data = JSON.parse(response.body)

@new_sign_in_count = @resource.sign_in_count
@new_current_sign_in_at = @resource.current_sign_in_at
@new_last_sign_in_at = @resource.last_sign_in_at
@new_sign_in_ip = @resource.current_sign_in_ip
@new_last_sign_in_ip = @resource.last_sign_in_ip
end

test 'request should succeed' do
Expand All @@ -47,32 +35,6 @@ class DeviseTokenAuth::SessionsControllerTest < ActionController::TestCase
assert_equal @existing_user.email, @data['data']['email']
end

describe 'trackable' do
test 'sign_in_count incrementns' do
assert_equal @old_sign_in_count + 1, @new_sign_in_count
end

test 'current_sign_in_at is updated' do
refute @old_current_sign_in_at
assert @new_current_sign_in_at
end

test 'last_sign_in_at is updated' do
refute @old_last_sign_in_at
assert @new_last_sign_in_at
end

test 'sign_in_ip is updated' do
refute @old_sign_in_ip
assert_equal '0.0.0.0', @new_sign_in_ip
end

test 'last_sign_in_ip is updated' do
refute @old_last_sign_in_ip
assert_equal '0.0.0.0', @new_last_sign_in_ip
end
end

describe "with multiple clients and headers don't change in each request" do
before do
# Set the max_number_of_devices to a lower number
Expand Down
2 changes: 1 addition & 1 deletion test/dummy/app/active_record/scoped_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
class ScopedUser < ActiveRecord::Base
# Include default devise modules.
devise :database_authenticatable, :registerable,
:recoverable, :rememberable, :trackable,
:recoverable, :rememberable,
:validatable, :confirmable, :omniauthable
include DeviseTokenAuth::Concerns::User
end
2 changes: 1 addition & 1 deletion test/dummy/app/active_record/unconfirmable_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
class UnconfirmableUser < ActiveRecord::Base
# Include default devise modules.
devise :database_authenticatable, :registerable,
:recoverable, :rememberable, :trackable,
:recoverable, :rememberable,
:validatable, :omniauthable
include DeviseTokenAuth::Concerns::User
end
2 changes: 1 addition & 1 deletion test/dummy/app/active_record/unregisterable_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
class UnregisterableUser < ActiveRecord::Base
# Include default devise modules.
devise :database_authenticatable, :recoverable,
:trackable, :validatable, :confirmable,
:validatable, :confirmable,
:omniauthable
include DeviseTokenAuth::Concerns::User
end
7 changes: 0 additions & 7 deletions test/dummy/app/mongoid/mang.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,6 @@ class Mang
## Rememberable
field :remember_created_at, type: Time

## Trackable
field :sign_in_count, type: Integer, default: 0
field :current_sign_in_at, type: Time
field :last_sign_in_at, type: Time
field :current_sign_in_ip, type: String
field :last_sign_in_ip, type: String

## Confirmable
field :confirmation_token, type: String
field :confirmed_at, type: Time
Expand Down
7 changes: 0 additions & 7 deletions test/dummy/app/mongoid/scoped_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,6 @@ class ScopedUser
## Rememberable
field :remember_created_at, type: Time

## Trackable
field :sign_in_count, type: Integer, default: 0
field :current_sign_in_at, type: Time
field :last_sign_in_at, type: Time
field :current_sign_in_ip, type: String
field :last_sign_in_ip, type: String

## Confirmable
field :confirmation_token, type: String
field :confirmed_at, type: Time
Expand Down
7 changes: 0 additions & 7 deletions test/dummy/app/mongoid/unconfirmable_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,6 @@ class UnconfirmableUser
## Rememberable
field :remember_created_at, type: Time

## Trackable
field :sign_in_count, type: Integer, default: 0
field :current_sign_in_at, type: Time
field :last_sign_in_at, type: Time
field :current_sign_in_ip, type: String
field :last_sign_in_ip, type: String

## Required
field :provider, type: String
field :uid, type: String, default: ''
Expand Down
7 changes: 0 additions & 7 deletions test/dummy/app/mongoid/unregisterable_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,6 @@ class UnregisterableUser
field :reset_password_redirect_url, type: String
field :allow_password_change, type: Boolean, default: false

## Trackable
field :sign_in_count, type: Integer, default: 0
field :current_sign_in_at, type: Time
field :last_sign_in_at, type: Time
field :current_sign_in_ip, type: String
field :last_sign_in_ip, type: String

## Confirmable
field :confirmation_token, type: String
field :confirmed_at, type: Time
Expand Down
7 changes: 0 additions & 7 deletions test/dummy/app/mongoid/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,6 @@ class User
## Rememberable
field :remember_created_at, type: Time

## Trackable
field :sign_in_count, type: Integer, default: 0
field :current_sign_in_at, type: Time
field :last_sign_in_at, type: Time
field :current_sign_in_ip, type: String
field :last_sign_in_ip, type: String

## Confirmable
field :confirmation_token, type: String
field :confirmed_at, type: Time
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,6 @@ def change
## Rememberable
t.datetime :remember_created_at

## Trackable
t.integer :sign_in_count, default: 0, null: false
t.datetime :current_sign_in_at
t.datetime :last_sign_in_at
t.string :current_sign_in_ip
t.string :last_sign_in_ip

## Confirmable
t.string :confirmation_token
t.datetime :confirmed_at
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,6 @@ def change
## Rememberable
t.datetime :remember_created_at

## Trackable
t.integer :sign_in_count, default: 0, null: false
t.datetime :current_sign_in_at
t.datetime :last_sign_in_at
t.string :current_sign_in_ip
t.string :last_sign_in_ip

## Confirmable
t.string :confirmation_token
t.datetime :confirmed_at
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,6 @@ def change
## Rememberable
#t.datetime :remember_created_at

## Trackable
#t.integer :sign_in_count, :default => 0, :null => false
#t.datetime :current_sign_in_at
#t.datetime :last_sign_in_at
#t.string :current_sign_in_ip
#t.string :last_sign_in_ip

## Confirmable
#t.string :confirmation_token
#t.datetime :confirmed_at
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,6 @@ def change
## Rememberable
t.datetime :remember_created_at

## Trackable
t.integer :sign_in_count, default: 0, null: false
t.datetime :current_sign_in_at
t.datetime :last_sign_in_at
t.string :current_sign_in_ip
t.string :last_sign_in_ip

## Confirmable
t.string :confirmation_token
t.datetime :confirmed_at
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,6 @@ def change
## Rememberable
t.datetime :remember_created_at

## Trackable
t.integer :sign_in_count, default: 0, null: false
t.datetime :current_sign_in_at
t.datetime :last_sign_in_at
t.string :current_sign_in_ip
t.string :last_sign_in_ip

## Confirmable
# t.string :confirmation_token
# t.datetime :confirmed_at
Expand Down
Loading