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

Bug fix for password not being required when used with database_authenticatable #56

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
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
39 changes: 21 additions & 18 deletions lib/devise/models/magic_link_authenticatable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,29 @@ module Models
module MagicLinkAuthenticatable
extend ActiveSupport::Concern

# Models using the :database_authenticatable strategy will already
# have #password_required? and #password defined - we will defer
# to those methods if they already exist so that people can use
# both strategies together. Otherwise, for :magic_link_authenticatable-
# only users, we define them in order to disable password validations:

unless instance_methods.include?(:password_required?)
def password_required?
false
included do
# Models using the :database_authenticatable strategy will already
# have #password defined and models using the :validatable strategy
# will already have #password_required? defined - we will defer
# to those methods if they already exist so that people can use
# both strategies together. Otherwise, for :magic_link_authenticatable-
# only users, we define them in order to disable password validations:

unless instance_methods.include?(:password_required?)
def password_required?
false
end
end
end

unless instance_methods.include?(:password)
# Not having a #password method breaks the :validatable module
#
# NOTE I proposed a change to Devise to fix this:
# https://github.com/heartcombo/devise/issues/5346#issuecomment-822022834
# As of yet it hasn't been accepted due to unknowns of the legacy code's purpose
def password
nil
unless instance_methods.include?(:password)
# Not having a #password method breaks the :validatable module
#
# NOTE I proposed a change to Devise to fix this:
# https://github.com/heartcombo/devise/issues/5346#issuecomment-822022834
# As of yet it hasn't been accepted due to unknowns of the legacy code's purpose
def password
nil
end
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,25 @@
expect(page).to have_css("p.combined_user span.email", text: email)
expect(page).to have_text("Welcome! You have signed up successfully.")
end

it "validates password" do
email = "[email protected]"

visit "/combined_users/sign_up"

expect(page.status_code).to be(200)
expect(page).to have_css("h2", text: "Sign up")

# It creates a user in the database
fill_in "Email", with: email
expect { click_button "Sign up" }.not_to change { CombinedUser.count }

# No emails are sent
expect(ActionMailer::Base.deliveries).to be_empty

# Sign in fails with custom error message
expect(page).to have_css("h2", text: "Sign up")
expect(page).to have_css("#error_explanation", text: "1 error prohibited this combined user from being saved:\nPassword can't be blank")
expect(page).not_to have_text("Welcome! You have signed up successfully.")
end
end
Loading