janko / rodauth-rails

Rails integration for Rodauth authentication framework
https://github.com/jeremyevans/rodauth
MIT License
584 stars 40 forks source link

inconsistencies for rodauth.authenticated_by between controllers and RoadauthApp #8

Closed nicolas-besnard closed 4 years ago

nicolas-besnard commented 4 years ago

This might be a Rodauth bug rather than a rodauth-rails one.

I've configured my RoadauthApp to use OTP.

class RodauthApp < Rodauth::Rails::App
  configure(json: true) do
    # List of authentication features that are loaded.
    enable :create_account, :verify_account, :verify_account_grace_period,
      :login, :remember, :logout,
      :close_account, :two_factor_base, :otp

  route do |r|
    rodauth.load_memory # autologin remembered users
    r.rodauth # route rodauth requests

    Rails.logger.info '  -- rodauth.authenticated_by'
    Rails.logger.info rodauth.authenticated_by.inspect
  end
end

When logging in, and entering OTP code:

So far, so good.

Now, I'm changing the "remember settings" by visiting /remember. When clicking "Remember me" then "Change Remember Setting", I still have the same output in the controller and in RoadauthApp (rodauth.authenticated_by # => ["password", "totp"])

However, after modifying the remember setting and trying to access a protected resource:

I'm not sure this is a bug, but the behavior seems weird.

I'll be happy to give you an app to reproduce this

janko commented 4 years ago

Thanks for the report. It's not likely that this is related to rodauth-rails, but an app that reproduces the issue would be great, because then we can find the problem and possibly send a patch to Rodauth.

nicolas-besnard commented 4 years ago

I have an app playground I am using to play with this lib. You will find it here: https://github.com/nicolas-besnard/rodauth-rails-otp

You should be able to see the reproduction. Let me know if there's any issue and I'll create a smaller app

janko commented 4 years ago

Ok, I was hoping the example app would be more minimal, but I will try to reproduce there. It could be related to the AJAX request from the other issue, where it somehow behaves differently than a normal request does (e.g. triggers loading from cookie).

janko commented 4 years ago

Actually, it will take a lot of time for me to debug that app. I believe it's AJAX-related, and that Rodauth does in fact show consistent results in both a Rails controller and the Rodauth app. Perhaps one of the logs came from the AJAX request and the other from a page request.

It's highly unlikely that this is a bug in rodauth-rails, and if it's a bug in Rodauth, then it should be reproducible in a Roda app (without Rails). I tried reproducing your issue with vanilla Roda, but for me everything is working correctly:

require "roda"
require "sequel"
require "capybara"
require "bcrypt"
require "securerandom"

DB = Sequel.sqlite
DB.extension :date_arithmetic
DB.create_table :accounts do
  primary_key :id
  String :email, null: false
  String :password_hash, null: false
end
DB.create_table :account_remember_keys do
  foreign_key :id, :accounts, primary_key: true
  String :key, null: false
  DateTime :deadline, null: false, default: Sequel.date_add(Sequel::CURRENT_TIMESTAMP, days: 14)
end
DB.create_table :account_otp_keys do
  foreign_key :id, :accounts, primary_key: true
  String :key, null: false
  Integer :num_failures, null: false, default: 0
  Time :last_use, null: false, default: Sequel.date_sub(Sequel::CURRENT_TIMESTAMP, seconds: 600)
end

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

  plugin :rodauth, json: true do
    enable :login, :remember, :otp
    account_password_hash_column :password_hash
    after_login { remember_login }
    after_load_memory { two_factor_update_session("totp") if two_factor_authentication_setup? }
  end

  route do |r|
    rodauth.load_memory
    r.rodauth

    rodauth.require_authentication

    r.root { "Authenticated by: #{rodauth.authenticated_by}" }
  end
end

account_id = DB[:accounts].insert(email: "foo@bar.com", password_hash: BCrypt::Password.create("secret"))

totp = ROTP::TOTP.new(ROTP::Base32.random_base32.downcase)

DB[:account_otp_keys].insert(id: account_id, key: totp.secret)

session = Capybara::Session.new(:rack_test, App)

session.visit "/login"
session.fill_in "Login", with: "foo@bar.com"
session.fill_in "Password", with: "secret"
session.click_on "Login"

session.fill_in "Authentication Code", with: totp.now
session.click_on "Authenticate Using TOTP"

session.visit "/remember"
session.choose "Remember Me"
session.click_on "Change Remember Setting"

session.visit "/"
puts session.html
Authenticated by: ["password", "totp"]
nicolas-besnard commented 4 years ago

I added an AJAX request in your example, and I have the expected result.

I feel like it's working because the routes are in the same context (a Roda route), as in my example, I'm sharing Roda's routes and Rails' routes.

I'll try to do a shorter example with unit tests that reproduce that tomorrow morning.

Thank you for taking the time to look at my issue 🙏

nicolas-besnard commented 4 years ago

I found the issue! You just need to add :jwt and jwt_secret SecureRandom.hex(32) and you'll get 2 different values for rodauth.authenticated_by

require "roda"
require "sequel"
require "capybara"
require "webdrivers/chromedriver"
require "bcrypt"
require "securerandom"
require "byebug"

DB = Sequel.sqlite
DB.extension :date_arithmetic
DB.create_table :accounts do
  primary_key :id
  String :email, null: false
  String :password_hash, null: false
end
DB.create_table :account_remember_keys do
  foreign_key :id, :accounts, primary_key: true
  String :key, null: false
  DateTime :deadline, null: false, default: Sequel.date_add(Sequel::CURRENT_TIMESTAMP, days: 14)
end
DB.create_table :account_otp_keys do
  foreign_key :id, :accounts, primary_key: true
  String :key, null: false
  Integer :num_failures, null: false, default: 0
  Time :last_use, null: false, default: Sequel.date_sub(Sequel::CURRENT_TIMESTAMP, seconds: 600)
end

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

  plugin :rodauth, json: true do
    enable :login, :remember, :otp, :jwt

    jwt_secret SecureRandom.hex(32)
    account_password_hash_column :password_hash
    after_login { remember_login }
    after_load_memory { two_factor_update_session("totp") if two_factor_authentication_setup? }
  end

  route do |r|
    rodauth.load_memory
    r.rodauth

    if rodauth.logged_in?
      puts '  -- rodauth.authenticated_by'
      puts rodauth.authenticated_by.inspect
    end

    rodauth.require_authentication

    r.on "posts" do
      r.is do
        r.get do
          puts '  ---- IN /POSTS -- rodauth.authenticated_by'
          puts rodauth.authenticated_by.inspect
          rodauth.authenticated_by
        end
      end
    end

    r.root do
      <<-HTML
        <button class="ajax">CLICK</button>
        <div class="result"></div>
        <script>
          const button = document.querySelector('.ajax')
          button.addEventListener('click', () => {
            fetch('/posts', {
              credentials: 'include',
              headers: {
                'Content-Type': 'application/json'
              }
            })
              .then(data => data.text())
              .then(data => {
                const result = document.querySelector('.result')
                result.innerHTML = data
              })
          })
        </script>
      HTML
    end
  end
end

account_id = DB[:accounts].insert(email: "foo@bar.com", password_hash: BCrypt::Password.create("secret"))

totp = ROTP::TOTP.new(ROTP::Base32.random_base32.downcase)

DB[:account_otp_keys].insert(id: account_id, key: totp.secret)

Capybara.register_driver :chrome do |app|
  capabilities = Selenium::WebDriver::Remote::Capabilities.chrome("goog:chromeOptions": {
    args: %W[window-size=1680,1050]
  })
  driver_options = {
    browser: :chrome,
    clear_local_storage: true,
    desired_capabilities: capabilities
  }
  Capybara::Selenium::Driver.new(app, driver_options)
end

session = Capybara::Session.new(:chrome, App)

session.visit "/login"
session.fill_in "Login", with: "foo@bar.com"
session.fill_in "Password", with: "secret"
session.click_on "Login"

session.fill_in "Authentication Code", with: totp.now
session.click_on "Authenticate Using TOTP"

session.visit "/remember"
session.choose "Remember Me"
session.click_on "Change Remember Setting"

session.visit "/"
session.click_on "CLICK"

sleep 0.2

puts ' -- AJAX Response'
puts session.find('.result').text
janko commented 4 years ago

Perfect, thank you, I will look into this tomorrow and let you know what I find 😉

nicolas-besnard commented 4 years ago

The base.rb file define a session method that use the scope of the app

On the other hand, jwt.rb redefines this method in case of a JSON request (with Content-Type: application/json header). Maybe this method should call super in case it's a JSON request but their no JWT token? I tried that locally and it's working. I'll need more time to see the impact of this change

janko commented 4 years ago

Good catch, yeah, that might the the issue. I don't yet fully understand why JWT is implemented this way, i.e. why does the session need to be cached in an instance variable.

janko commented 4 years ago

I'll consider this one resolved as you seem to have found the solution in https://github.com/jeremyevans/rodauth/issues/101.