Skip to content

Commit

Permalink
Always return an error when confirmation_token is blank (heartcombo…
Browse files Browse the repository at this point in the history
…#5132)

As reported in heartcombo#5071, if
for some reason, a user in the database had the `confirmation_token`
column as a blank string, Devise would confirm that user after receiving
a request with a blank `confirmation_token` parameter.
After this commit, a request sending a blank `confirmation_token`
parameter will receive a validation error.
For applications that have users with a blank `confirmation_token` in
the database, it's recommended to manually regenerate or to nullify
them.
  • Loading branch information
tegon authored and scbafk committed Mar 3, 2020
1 parent fb48336 commit 1b0feec
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 0 deletions.
12 changes: 12 additions & 0 deletions lib/devise/models/confirmable.rb
Expand Up @@ -300,7 +300,19 @@ def send_confirmation_instructions(attributes={})
# If the user is already confirmed, create an error for the user
# Options must have the confirmation_token
def confirm_by_token(confirmation_token)
# When the `confirmation_token` parameter is blank, if there are any users with a blank
# `confirmation_token` in the database, the first one would be confirmed here.
# The error is being manually added here to ensure no users are confirmed by mistake.
# This was done in the model for convenience, since validation errors are automatically
# displayed in the view.
if confirmation_token.blank?
confirmable = new
confirmable.errors.add(:confirmation_token, :blank)
return confirmable
end

confirmable = find_first_by_auth_conditions(confirmation_token: confirmation_token)

unless confirmable
confirmation_digest = Devise.token_generator.digest(self, :confirmation_token, confirmation_token)
confirmable = find_or_initialize_with_error_by(:confirmation_token, confirmation_digest)
Expand Down
30 changes: 30 additions & 0 deletions test/integration/confirmable_test.rb
Expand Up @@ -173,6 +173,36 @@ def resend_confirmation
assert_current_url '/users/sign_in'
end

test "should not be able to confirm an email with a blank confirmation token" do
visit_user_confirmation_with_token("")

assert_contain "Confirmation token can't be blank"
end

test "should not be able to confirm an email with a nil confirmation token" do
visit_user_confirmation_with_token(nil)

assert_contain "Confirmation token can't be blank"
end

test "should not be able to confirm user with blank confirmation token" do
user = create_user(confirm: false)
user.update_attribute(:confirmation_token, "")

visit_user_confirmation_with_token("")

assert_contain "Confirmation token can't be blank"
end

test "should not be able to confirm user with nil confirmation token" do
user = create_user(confirm: false)
user.update_attribute(:confirmation_token, nil)

visit_user_confirmation_with_token(nil)

assert_contain "Confirmation token can't be blank"
end

test 'error message is configurable by resource name' do
store_translations :en, devise: {
failure: { user: { unconfirmed: "Not confirmed user" } }
Expand Down
18 changes: 18 additions & 0 deletions test/models/confirmable_test.rb
Expand Up @@ -64,6 +64,24 @@ def setup
assert_equal "can't be blank", confirmed_user.errors[:confirmation_token].join
end

test 'should return a new record with errors when a blank token is given and a record exists on the database' do
user = create_user(confirmation_token: '')

confirmed_user = User.confirm_by_token('')

refute user.reload.confirmed?
assert_equal "can't be blank", confirmed_user.errors[:confirmation_token].join
end

test 'should return a new record with errors when a nil token is given and a record exists on the database' do
user = create_user(confirmation_token: nil)

confirmed_user = User.confirm_by_token(nil)

refute user.reload.confirmed?
assert_equal "can't be blank", confirmed_user.errors[:confirmation_token].join
end

test 'should generate errors for a user email if user is already confirmed' do
user = create_user
user.confirmed_at = Time.now
Expand Down

0 comments on commit 1b0feec

Please sign in to comment.