jeremyevans / rodauth

Ruby's Most Advanced Authentication Framework
http://rodauth.jeremyevans.net
MIT License
1.69k stars 95 forks source link

Checking whether account is in an unverified grace period fails with a NoMethodError #167

Closed janko closed 3 years ago

janko commented 3 years ago

In the verify_account_grace_period feature, when the grace period expires, the #account_in_unverified_grace_period? method raises a NoMethodError due to account being nil. Here is a minimal self-contained example that reproduces the problem:

require "roda"
require "sequel"
require "capybara"
require "mail"
require "securerandom"

DB = Sequel.sqlite
DB.create_table :accounts do
  primary_key :id
  String :email, null: false
  String :password_hash
  Integer :status_id, default: 1
end
DB.create_table :account_verification_keys do
  foreign_key :id, :accounts, primary_key: true, type: :Bignum
  String :key, null: false
  DateTime :requested_at, null: false, default: Sequel::CURRENT_TIMESTAMP
  DateTime :email_last_sent, null: false, default: Sequel::CURRENT_TIMESTAMP
end

Mail.defaults { delivery_method :test }

class App < Roda
  plugin :render, layout: false
  plugin :sessions, secret: SecureRandom.hex(32)

  plugin :rodauth do
    enable :create_account, :verify_account, :verify_account_grace_period
    account_password_hash_column :password_hash
    verify_account_grace_period 1
  end

  route do |r|
    r.rodauth
    r.root { "Unverified grace period? #{rodauth.send(:account_in_unverified_grace_period?)}" }
  end
end

session = Capybara::Session.new(:rack_test, App)
session.visit "/create-account"
session.fill_in "Login", with: "foo@example.com"
session.fill_in "Password", with: "secret"
session.fill_in "Confirm Password", with: "secret"
session.click_on "Create Account"
session.html # => "Unverified grace period? true"
sleep 1 # let unverified grade period expire
session.visit "/" # ~> NoMethodError: undefined method `[]' for nil:NilClass

The error happens on this line, due to #account_from_session method not loading the @account. It's most likely due to #_account_from_session adding #account_session_status_filter to the account dataset filter, which filters out accounts that have unverified status once the grace period expires.

Note that I'm not calling this method directly in my app, but I have a condition on #uses_two_factor_authentication?, which together with email_auth feature calls #possible_authentication_methods => #allow_email_auth? => #account_in_unverified_grace_period?.

jeremyevans commented 3 years ago

Thank you for your excellent work in diagnosing the cause of the problem. Is this something you feel comfortable submitting a spec and fix? If not, I can do so. I assume the simplest fix is adding return false unless to the previous line, but I haven't tested that.

janko commented 3 years ago

I'm actually a bit busy at the moment (I'm behind on a technical review for a certain book about ruby programming 😉), so if you have time to look into it, that would be great 🙂

jeremyevans commented 3 years ago

I understand completely. :) I should be able to take care of this today. Thanks!