Merge branch 'new_user_auth'

app/controllers/admin/users_controller.rb

@@ -72,8 +72,7 @@ class Admin::UsersController < Admin::AdminController
 
   def log_out
     if @user
-      @user.auth_token = nil
-      @user.save!
+      @user.user_auth_tokens.destroy_all
       @user.logged_out
       render json: success_json
     else

app/controllers/users_controller.rb

@@ -417,7 +417,7 @@ class UsersController < ApplicationController
       else
         @user.password = params[:password]
         @user.password_required!
-        @user.auth_token = nil
+        @user.user_auth_tokens.destroy_all
         if @user.save
           Invite.invalidate_for_email(@user.email) # invite link can't be used to log in anymore
           secure_session["password-#{token}"] = nil
@@ -701,7 +701,7 @@ class UsersController < ApplicationController
   private
 
     def honeypot_value
-      Digest::SHA1::hexdigest("#{Discourse.current_hostname}:#{Discourse::Application.config.secret_token}")[0,15]
+      Digest::SHA1::hexdigest("#{Discourse.current_hostname}:#{GlobalSetting.safe_secret_key_base}")[0,15]
     end
 
     def challenge_value

app/jobs/scheduled/weekly.rb

@@ -13,6 +13,7 @@ module Jobs
       ScoreCalculator.new.calculate
       SchedulerStat.purge_old
       Draft.cleanup!
+      UserAuthToken.cleanup!
     end
   end
 end

app/models/global_setting.rb

@@ -6,6 +6,35 @@ class GlobalSetting
     end
   end
 
+  VALID_SECRET_KEY = /^[0-9a-f]{128}$/
+  # this is named SECRET_TOKEN as opposed to SECRET_KEY_BASE
+  # for legacy reasons
+  REDIS_SECRET_KEY = 'SECRET_TOKEN'
+
+  # In Rails secret_key_base is used to encrypt the cookie store
+  # the cookie store contains session data
+  # Discourse also uses this secret key to digest user auth tokens
+  # This method will
+  # - use existing token if already set in ENV or discourse.conf
+  # - generate a token on the fly if needed and cache in redis
+  # - enforce rules about token format falling back to redis if needed
+  def self.safe_secret_key_base
+    @safe_secret_key_base ||= begin
+      token = secret_key_base
+      if token.blank? || token !~ VALID_SECRET_KEY
+        token = $redis.without_namespace.get(REDIS_SECRET_KEY)
+        unless token && token =~ VALID_SECRET_KEY
+          token = SecureRandom.hex(64)
+          $redis.without_namespace.set(REDIS_SECRET_KEY,token)
+        end
+      end
+      if !secret_key_base.blank? && token != secret_key_base
+        STDERR.puts "WARNING: DISCOURSE_SECRET_KEY_BASE is invalid, it was re-generated"
+      end
+      token
+    end
+  end
+
   def self.load_defaults
     default_provider = FileProvider.from(File.expand_path('../../../config/discourse_defaults.conf', __FILE__))
     default_provider.keys.concat(@provider.keys).uniq.each do |key|

app/models/user.rb

@@ -41,6 +41,7 @@ class User < ActiveRecord::Base
   has_many :user_archived_messages, dependent: :destroy
   has_many :email_change_requests, dependent: :destroy
   has_many :directory_items, dependent: :delete_all
+  has_many :user_auth_tokens, dependent: :destroy
 
 
   has_one :user_option, dependent: :destroy
@@ -97,6 +98,7 @@ class User < ActiveRecord::Base
   before_save :update_username_lower
   before_save :ensure_password_is_hashed
 
+  after_save :expire_tokens_if_password_changed
   after_save :automatic_group_membership
   after_save :clear_global_notice_if_needed
   after_save :refresh_avatar
@@ -420,7 +422,6 @@ class User < ActiveRecord::Base
     # special case for passwordless accounts
     unless password.blank?
       @raw_password = password
-      self.auth_token = nil
     end
   end
 
@@ -971,6 +972,18 @@ class User < ActiveRecord::Base
     end
   end
 
+  def expire_tokens_if_password_changed
+    # NOTE: setting raw password is the only valid way of changing a password
+    # the password field in the DB is actually hashed, nobody should be amending direct
+    if @raw_password
+      # Association in model may be out-of-sync
+      UserAuthToken.where(user_id: id).destroy_all
+      # We should not carry this around after save
+      @raw_password = nil
+    end
+  end
+
+
   def hash_password(password, salt)
     raise StandardError.new("password is too long") if password.size > User.max_password_length
     Pbkdf2.hash_password(password, salt, Rails.configuration.pbkdf2_iterations, Rails.configuration.pbkdf2_algorithm)
@@ -1076,7 +1089,7 @@ end
 #  username                :string(60)       not null
 #  created_at              :datetime         not null
 #  updated_at              :datetime         not null
-#  name                    :string
+#  name                    :string(255)
 #  seen_notification_id    :integer          default(0), not null
 #  last_posted_at          :datetime
 #  email                   :string(513)      not null
@@ -1101,7 +1114,7 @@ end
 #  ip_address              :inet
 #  moderator               :boolean          default(FALSE)
 #  blocked                 :boolean          default(FALSE)
-#  title                   :string
+#  title                   :string(255)
 #  uploaded_avatar_id      :integer
 #  locale                  :string(10)
 #  primary_group_id        :integer

app/models/user_auth_token.rb

@@ -0,0 +1,115 @@
+# frozen_string_literal: true
+require 'digest/sha1'
+
+class UserAuthToken < ActiveRecord::Base
+  belongs_to :user
+
+  ROTATE_TIME = 10.minutes
+  # used when token did not arrive at client
+  URGENT_ROTATE_TIME = 1.minute
+
+  attr_accessor :unhashed_auth_token
+
+  def self.generate!(info)
+    token = SecureRandom.hex(16)
+    hashed_token = hash_token(token)
+    user_auth_token = UserAuthToken.create!(
+      user_id: info[:user_id],
+      user_agent: info[:user_agent],
+      client_ip: info[:client_ip],
+      auth_token: hashed_token,
+      prev_auth_token: hashed_token,
+      rotated_at: Time.zone.now
+    )
+    user_auth_token.unhashed_auth_token = token
+    user_auth_token
+  end
+
+  def self.lookup(unhashed_token, opts=nil)
+
+    mark_seen = opts && opts[:seen]
+
+    token = hash_token(unhashed_token)
+    expire_before = SiteSetting.maximum_session_age.hours.ago
+
+    user_token = find_by("(auth_token = :token OR
+                          prev_auth_token = :token OR
+                          (auth_token = :unhashed_token AND legacy)) AND created_at > :expire_before",
+                          token: token, unhashed_token: unhashed_token, expire_before: expire_before)
+
+    if user_token &&
+       user_token.auth_token_seen &&
+       user_token.prev_auth_token == token &&
+       user_token.prev_auth_token != user_token.auth_token &&
+       user_token.rotated_at > 1.minute.ago
+      return nil
+    end
+
+    if mark_seen && user_token && !user_token.auth_token_seen && user_token.auth_token == token
+      user_token.update_columns(auth_token_seen: true)
+    end
+
+    user_token
+  end
+
+  def self.hash_token(token)
+    Digest::SHA1.base64digest("#{token}#{GlobalSetting.safe_secret_key_base}")
+  end
+
+  def self.cleanup!
+    where('rotated_at < :time',
+          time: SiteSetting.maximum_session_age.hours.ago - ROTATE_TIME).delete_all
+
+  end
+
+  def rotate!(info=nil)
+    user_agent = (info && info[:user_agent] || self.user_agent)
+    client_ip = (info && info[:client_ip] || self.client_ip)
+
+    token = SecureRandom.hex(16)
+
+    result = UserAuthToken.exec_sql("
+  UPDATE user_auth_tokens
+  SET
+    auth_token_seen = false,
+    user_agent = :user_agent,
+    client_ip = :client_ip,
+    prev_auth_token = case when auth_token_seen then auth_token else prev_auth_token end,
+    auth_token = :new_token,
+    rotated_at = :now
+  WHERE id = :id AND (auth_token_seen or rotated_at < :safeguard_time)
+", id: self.id,
+   user_agent: user_agent,
+   client_ip: client_ip&.to_s,
+   now: Time.zone.now,
+   new_token: UserAuthToken.hash_token(token),
+   safeguard_time: 30.seconds.ago
+  )
+
+    if result.cmdtuples > 0
+      reload
+      self.unhashed_auth_token = token
+      true
+    else
+      false
+    end
+
+  end
+end
+
+# == Schema Information
+#
+# Table name: user_auth_tokens
+#
+#  id              :integer          not null, primary key
+#  user_id         :integer          not null
+#  auth_token      :string           not null
+#  prev_auth_token :string
+#  user_agent      :string
+#  auth_token_seen :boolean          default(FALSE), not null
+#  legacy          :boolean          default(FALSE), not null
+#  client_ip       :inet
+#  rotated_at      :datetime
+#  created_at      :datetime
+#  updated_at      :datetime
+#

config/discourse_defaults.conf

@@ -147,3 +147,6 @@ relative_url_root =
 # this ensures backlog (ability of channels to catch up are capped)
 # message bus default cap is 1000, we are winding it down to 100
 message_bus_max_backlog_size = 100
+
+# must be a 64 byte hex string, anything else will be ignored with a warning
+secret_key_base =

config/initializers/100-secret_token.rb

@@ -1,14 +1,4 @@
-# We have had lots of config issues with SECRET_TOKEN to avoid this mess we are moving it to redis
-#  if you feel strongly that it does not belong there use ENV['SECRET_TOKEN']
-#
-token = ENV['SECRET_TOKEN']
-unless token
-  token = $redis.get('SECRET_TOKEN')
-  unless token && token.length == 128
-    token = SecureRandom.hex(64)
-    $redis.set('SECRET_TOKEN',token)
-  end
-end
-
-Discourse::Application.config.secret_token = token
-Discourse::Application.config.secret_key_base = token
+# Not fussed setting secret_token anymore, that is only required for
+# backwards support of "seamless" upgrade from Rails 3.
+# Discourse has shipped Rails 3 for a very long time.
+Discourse::Application.config.secret_key_base = GlobalSetting.safe_secret_key_base

db/migrate/20170124181409_add_user_auth_tokens.rb

@@ -0,0 +1,43 @@
+class AddUserAuthTokens < ActiveRecord::Migration
+  def down
+    add_column :users, :auth_token, :string
+    add_column :users, :auth_token_updated_at, :datetime
+    execute <<SQL
+      UPDATE users
+        SET auth_token = user_auth_tokens.auth_token,
+            auth_token_updated_at = user_auth_tokens.created_at
+      FROM user_auth_tokens
+      WHERE legacy AND user_auth_tokens.user_id = users.id
+SQL
+
+    drop_table :user_auth_tokens
+
+  end
+
+  def up
+    create_table :user_auth_tokens do |t|
+      t.integer :user_id, null: false
+      t.string  :auth_token, null: false
+      t.string  :prev_auth_token, null: false
+      t.string  :user_agent
+      t.boolean :auth_token_seen, default: false, null: false
+      t.boolean :legacy, default: false, null: false
+      t.inet    :client_ip
+      t.datetime :rotated_at, null: false
+      t.timestamps
+    end
+
+    add_index :user_auth_tokens, [:auth_token]
+    add_index :user_auth_tokens, [:prev_auth_token]
+
+    execute <<SQL
+    INSERT INTO user_auth_tokens(user_id, auth_token, prev_auth_token, legacy, created_at, rotated_at)
+    SELECT id, auth_token, auth_token, true, auth_token_updated_at, auth_token_updated_at
+    FROM users
+    WHERE auth_token_updated_at IS NOT NULL AND auth_token IS NOT NULL
+SQL
+
+    remove_column :users, :auth_token
+    remove_column :users, :auth_token_updated_at
+  end
+end

lib/auth/default_current_user_provider.rb

@@ -44,10 +44,8 @@ class Auth::DefaultCurrentUserProvider
       limiter = RateLimiter.new(nil, "cookie_auth_#{request.ip}", COOKIE_ATTEMPTS_PER_MIN ,60)
 
       if limiter.can_perform?
-        current_user = User.where(auth_token: auth_token)
-                         .where('auth_token_updated_at IS NULL OR auth_token_updated_at > ?',
-                                  SiteSetting.maximum_session_age.hours.ago)
-                         .first
+        @user_token = UserAuthToken.lookup(auth_token, seen: true)
+        current_user = @user_token.try(:user)
       end
 
       unless current_user
@@ -105,35 +103,46 @@ class Auth::DefaultCurrentUserProvider
   end
 
   def refresh_session(user, session, cookies)
-    return if is_api?
 
-    if user && (!user.auth_token_updated_at || user.auth_token_updated_at <= 1.hour.ago)
-      user.update_column(:auth_token_updated_at, Time.zone.now)
-      cookies[TOKEN_COOKIE] = cookie_hash(user)
+    # if user was not loaded, no point refreshing session
+    # it could be an anonymous path, this would add cost
+    return if is_api? || !@env.key?(CURRENT_USER_KEY)
+
+    if @user_token && @user_token.user == user
+      rotated_at = @user_token.rotated_at
+
+      needs_rotation = @user_token.auth_token_seen ? rotated_at < UserAuthToken::ROTATE_TIME.ago : rotated_at < UserAuthToken::URGENT_ROTATE_TIME.ago
+
+      if !@user_token.legacy && needs_rotation
+        if @user_token.rotate!(user_agent: @env['HTTP_USER_AGENT'],
+                              client_ip: @request.ip)
+          cookies[TOKEN_COOKIE] = cookie_hash(@user_token.unhashed_auth_token)
+        end
+      elsif @user_token.legacy
+        # make a new token
+        log_on_user(user, session, cookies)
+      end
     end
+
     if !user && cookies.key?(TOKEN_COOKIE)
       cookies.delete(TOKEN_COOKIE)
     end
   end
 
   def log_on_user(user, session, cookies)
-    legit_token = user.auth_token && user.auth_token.length == 32
-    expired_token = user.auth_token_updated_at && user.auth_token_updated_at < SiteSetting.maximum_session_age.hours.ago
+    @user_token = UserAuthToken.generate!(user_id: user.id,
+                                          user_agent: @env['HTTP_USER_AGENT'],
+                                          client_ip: @request.ip)
 
-    if !legit_token || expired_token
-      user.update_columns(auth_token: SecureRandom.hex(16),
-                          auth_token_updated_at: Time.zone.now)
-    end
-
-    cookies[TOKEN_COOKIE] = cookie_hash(user)
+    cookies[TOKEN_COOKIE] = cookie_hash(@user_token.unhashed_auth_token)
     make_developer_admin(user)
     enable_bootstrap_mode(user)
     @env[CURRENT_USER_KEY] = user
   end
 
-  def cookie_hash(user)
+  def cookie_hash(unhashed_auth_token)
     {
-      value: user.auth_token,
+      value: unhashed_auth_token,
       httponly: true,
       expires: SiteSetting.maximum_session_age.hours.from_now,
       secure: SiteSetting.force_https
@@ -155,9 +164,9 @@ class Auth::DefaultCurrentUserProvider
   end
 
   def log_off_user(session, cookies)
-    if SiteSetting.log_out_strict && (user = current_user)
-      user.auth_token = nil
-      user.save!
+    user = current_user
+    if SiteSetting.log_out_strict && user
+      user.user_auth_tokens.destroy_all
 
       if user.admin && defined?(Rack::MiniProfiler)
         # clear the profiling cookie to keep stuff tidy
@@ -165,6 +174,8 @@ class Auth::DefaultCurrentUserProvider
       end
 
       user.logged_out
+    elsif user && @user_token
+      @user_token.destroy
     end
     cookies.delete(TOKEN_COOKIE)
   end

lib/wizard.rb

@@ -1,6 +1,7 @@
 require_dependency 'wizard/step'
 require_dependency 'wizard/field'
 require_dependency 'wizard/step_updater'
+require_dependency 'wizard/builder'
 
 class Wizard
   attr_reader :steps, :user
@@ -76,11 +77,10 @@ class Wizard
   def requires_completion?
     return false unless SiteSetting.wizard_enabled?
 
-
     first_admin = User.where(admin: true)
                       .where.not(id: Discourse.system_user.id)
-                      .where.not(auth_token_updated_at: nil)
-                      .order(:auth_token_updated_at)
+                      .joins(:user_auth_tokens)
+                      .order('user_auth_tokens.created_at')
 
     if @user.present? && first_admin.first == @user && (Topic.count < 15)
       !Wizard::Builder.new(@user).build.completed?

spec/components/auth/default_current_user_provider_spec.rb

@@ -3,10 +3,17 @@ require_dependency 'auth/default_current_user_provider'
 
 describe Auth::DefaultCurrentUserProvider do
 
+  class TestProvider < Auth::DefaultCurrentUserProvider
+    attr_reader :env
+    def initialize(env)
+      super(env)
+    end
+  end
+
   def provider(url, opts=nil)
     opts ||= {method: "GET"}
     env = Rack::MockRequest.env_for(url, opts)
-    Auth::DefaultCurrentUserProvider.new(env)
+    TestProvider.new(env)
   end
 
   it "raises errors for incorrect api_key" do
@@ -73,21 +80,83 @@ describe Auth::DefaultCurrentUserProvider do
     expect(provider("/topic/anything/goes", method: "GET").should_update_last_seen?).to eq(true)
   end
 
-  it "correctly renews session once an hour" do
+  it "correctly supports legacy tokens" do
+    user = Fabricate(:user)
+    token = SecureRandom.hex(16)
+    user_token = UserAuthToken.create!(user_id: user.id, auth_token: token,
+                                       prev_auth_token: token, legacy: true,
+                                       rotated_at: Time.zone.now
+                                      )
+
+    prov = provider("/", "HTTP_COOKIE" => "_t=#{user_token.auth_token}")
+    expect(prov.current_user.id).to eq(user.id)
+
+    # sets a new token up cause it got a global token
+    cookies = {}
+    prov.refresh_session(user, {}, cookies)
+    user.reload
+
+    expect(user.user_auth_tokens.count).to eq(2)
+    expect(cookies["_t"][:value]).not_to eq(token)
+  end
+
+  it "correctly rotates tokens" do
     SiteSetting.maximum_session_age = 3
     user = Fabricate(:user)
-    provider('/').log_on_user(user, {}, {})
-
-    freeze_time 2.hours.from_now
+    @provider = provider('/')
     cookies = {}
-    provider("/", "HTTP_COOKIE" => "_t=#{user.auth_token}").refresh_session(user, {}, cookies)
+    @provider.log_on_user(user, {}, cookies)
+
+    unhashed_token = cookies["_t"][:value]
+
+    token = UserAuthToken.find_by(user_id: user.id)
+
+    expect(token.auth_token_seen).to eq(false)
+    expect(token.auth_token).not_to eq(unhashed_token)
+    expect(token.auth_token).to eq(UserAuthToken.hash_token(unhashed_token))
+
+    # at this point we are going to try to rotate token
+    freeze_time 20.minutes.from_now
+
+    provider2 = provider("/", "HTTP_COOKIE" => "_t=#{unhashed_token}")
+    provider2.current_user
+
+    token.reload
+    expect(token.auth_token_seen).to eq(true)
+
+    cookies = {}
+    provider2.refresh_session(user, {}, cookies)
+    expect(cookies["_t"][:value]).not_to eq(unhashed_token)
+
+    token.reload
+    expect(token.auth_token_seen).to eq(false)
+
+
+    freeze_time 21.minutes.from_now
+
+
+    old_token = token.prev_auth_token
+    unverified_token = token.auth_token
+
+    # old token should still work
+    provider2 = provider("/", "HTTP_COOKIE" => "_t=#{unhashed_token}")
+    expect(provider2.current_user.id).to eq(user.id)
+
+    provider2.refresh_session(user, {}, cookies)
+
+    token.reload
+
+    # because this should cause a rotation since we can safely
+    # assume it never reached the client
+    expect(token.prev_auth_token).to eq(old_token)
+    expect(token.auth_token).not_to eq(unverified_token)
 
-    expect(user.auth_token_updated_at - Time.now).to eq(0)
   end
 
   it "can only try 10 bad cookies a minute" do
-
     user = Fabricate(:user)
+    token = UserAuthToken.generate!(user_id: user.id)
+
     provider('/').log_on_user(user, {}, {})
 
     RateLimiter.stubs(:disabled?).returns(false)
@@ -107,12 +176,15 @@ describe Auth::DefaultCurrentUserProvider do
     }.to raise_error(Discourse::InvalidAccess)
 
     expect {
-      env["HTTP_COOKIE"] = "_t=#{user.auth_token}"
+      env["HTTP_COOKIE"] = "_t=#{token.unhashed_auth_token}"
       provider("/", env).current_user
     }.to raise_error(Discourse::InvalidAccess)
 
     env["REMOTE_ADDR"] = "10.0.0.2"
-    provider('/', env).current_user
+
+    expect {
+      provider('/', env).current_user
+    }.not_to raise_error
   end
 
   it "correctly removes invalid cookies" do
@@ -123,35 +195,26 @@ describe Auth::DefaultCurrentUserProvider do
     expect(cookies.key?("_t")).to eq(false)
   end
 
-  it "recycles existing auth_token correctly" do
-    SiteSetting.maximum_session_age = 3
+  it "logging on user always creates a new token" do
     user = Fabricate(:user)
-    provider('/').log_on_user(user, {}, {})
-
-    original_auth_token = user.auth_token
-
-    freeze_time 2.hours.from_now
-    provider('/').log_on_user(user, {}, {})
-
-    user.reload
-    expect(user.auth_token).to eq(original_auth_token)
-
-    freeze_time 10.hours.from_now
 
     provider('/').log_on_user(user, {}, {})
-    user.reload
-    expect(user.auth_token).not_to eq(original_auth_token)
+    provider('/').log_on_user(user, {}, {})
+
+    expect(UserAuthToken.where(user_id: user.id).count).to eq(2)
   end
 
   it "correctly expires session" do
     SiteSetting.maximum_session_age = 2
     user = Fabricate(:user)
+    token = UserAuthToken.generate!(user_id: user.id)
+
     provider('/').log_on_user(user, {}, {})
 
-    expect(provider("/", "HTTP_COOKIE" => "_t=#{user.auth_token}").current_user.id).to eq(user.id)
+    expect(provider("/", "HTTP_COOKIE" => "_t=#{token.unhashed_auth_token}").current_user.id).to eq(user.id)
 
     freeze_time 3.hours.from_now
-    expect(provider("/", "HTTP_COOKIE" => "_t=#{user.auth_token}").current_user).to eq(nil)
+    expect(provider("/", "HTTP_COOKIE" => "_t=#{token.unhashed_auth_token}").current_user).to eq(nil)
   end
 
   context "user api" do

spec/components/current_user_spec.rb

@@ -3,10 +3,10 @@ require_dependency 'current_user'
 
 describe CurrentUser do
   it "allows us to lookup a user from our environment" do
-    user = Fabricate(:user, auth_token: EmailToken.generate_token, active: true)
-    EmailToken.confirm(user.auth_token)
+    user = Fabricate(:user, active: true)
+    token = UserAuthToken.generate!(user_id: user.id)
 
-    env = Rack::MockRequest.env_for("/test", "HTTP_COOKIE" => "_t=#{user.auth_token};")
+    env = Rack::MockRequest.env_for("/test", "HTTP_COOKIE" => "_t=#{token.unhashed_auth_token};")
     expect(CurrentUser.lookup_from_env(env)).to eq(user)
   end
 

spec/components/wizard_spec.rb

@@ -122,7 +122,8 @@ describe Wizard do
 
     it "it's true for the first admin who logs in" do
       admin = Fabricate(:admin)
-      second_admin = Fabricate(:admin, auth_token_updated_at: Time.now)
+      second_admin = Fabricate(:admin)
+      UserAuthToken.generate!(user_id: second_admin.id)
 
       expect(build_simple(admin).requires_completion?).to eq(false)
       expect(build_simple(second_admin).requires_completion?).to eq(true)

spec/controllers/session_controller_spec.rb

@@ -505,8 +505,8 @@ describe SessionController do
           user.reload
 
           expect(session[:current_user_id]).to eq(user.id)
-          expect(user.auth_token).to be_present
-          expect(cookies[:_t]).to eq(user.auth_token)
+          expect(user.user_auth_tokens.count).to eq(1)
+          expect(UserAuthToken.hash_token(cookies[:_t])).to eq(user.user_auth_tokens.first.auth_token)
         end
       end
 

spec/controllers/users_controller_spec.rb

@@ -155,21 +155,13 @@ describe UsersController do
           put :perform_account_activation, token: 'asdfasdf'
         end
 
-        it 'returns success' do
+        it 'correctly logs on user' do
           expect(response).to be_success
-        end
-
-        it "doesn't set an error" do
           expect(flash[:error]).to be_blank
-        end
-
-        it 'logs in as the user' do
           expect(session[:current_user_id]).to be_present
-        end
-
-        it "doesn't set @needs_approval" do
           expect(assigns[:needs_approval]).to be_blank
         end
+
       end
 
       context 'user is not approved' do
@@ -241,7 +233,7 @@ describe UsersController do
         render_views
 
         it 'renders referrer never on get requests' do
-          user = Fabricate(:user, auth_token: SecureRandom.hex(16))
+          user = Fabricate(:user)
           token = user.email_tokens.create(email: user.email).token
           get :password_reset, token: token
 
@@ -250,25 +242,25 @@ describe UsersController do
       end
 
       it 'returns success' do
-        user = Fabricate(:user, auth_token: SecureRandom.hex(16))
+        user = Fabricate(:user)
+        user_auth_token = UserAuthToken.generate!(user_id: user.id)
         token = user.email_tokens.create(email: user.email).token
 
-        old_token = user.auth_token
-
         get :password_reset, token: token
         put :password_reset, token: token, password: 'hg9ow8yhg98o'
+
         expect(response).to be_success
         expect(assigns[:error]).to be_blank
 
         user.reload
-        expect(user.auth_token).to_not eq old_token
-        expect(user.auth_token.length).to eq 32
+
         expect(session["password-#{token}"]).to be_blank
+
+        expect(UserAuthToken.where(id: user_auth_token.id).count).to eq(0)
       end
 
       it 'disallows double password reset' do
-
-        user = Fabricate(:user, auth_token: SecureRandom.hex(16))
+        user = Fabricate(:user)
         token = user.email_tokens.create(email: user.email).token
 
         get :password_reset, token: token
@@ -277,10 +269,13 @@ describe UsersController do
 
         user.reload
         expect(user.confirm_password?('hg9ow8yhg98o')).to eq(true)
+
+        # logged in now
+        expect(user.user_auth_tokens.count).to eq(1)
       end
 
       it "redirects to the wizard if you're the first admin" do
-        user = Fabricate(:admin, auth_token: SecureRandom.hex(16), auth_token_updated_at: Time.now)
+        user = Fabricate(:admin)
         token = user.email_tokens.create(email: user.email).token
         get :password_reset, token: token
         put :password_reset, token: token, password: 'hg9ow8yhg98oadminlonger'
@@ -288,13 +283,17 @@ describe UsersController do
       end
 
       it "doesn't invalidate the token when loading the page" do
-        user = Fabricate(:user, auth_token: SecureRandom.hex(16))
+        user = Fabricate(:user)
+        user_token = UserAuthToken.generate!(user_id: user.id)
+
         email_token = user.email_tokens.create(email: user.email)
 
         get :password_reset, token: email_token.token
 
         email_token.reload
+
         expect(email_token.confirmed).to eq(false)
+        expect(UserAuthToken.where(id: user_token.id).count).to eq(1)
       end
     end
 

spec/models/user_auth_token_spec.rb

@@ -0,0 +1,123 @@
+require 'rails_helper'
+
+describe UserAuthToken do
+
+  it "can remove old expired tokens" do
+
+    freeze_time Time.zone.now
+    SiteSetting.maximum_session_age = 1
+
+    user = Fabricate(:user)
+    token = UserAuthToken.generate!(user_id: user.id,
+                                    user_agent: "some user agent 2",
+                                    client_ip: "1.1.2.3")
+
+    freeze_time 1.hour.from_now
+    UserAuthToken.cleanup!
+
+    expect(UserAuthToken.where(id: token.id).count).to eq(1)
+
+    freeze_time 1.second.from_now
+    UserAuthToken.cleanup!
+
+    expect(UserAuthToken.where(id: token.id).count).to eq(1)
+
+    freeze_time UserAuthToken::ROTATE_TIME.from_now
+    UserAuthToken.cleanup!
+
+    expect(UserAuthToken.where(id: token.id).count).to eq(0)
+
+  end
+
+  it "can lookup both hashed and unhashed" do
+    user = Fabricate(:user)
+
+    token = UserAuthToken.generate!(user_id: user.id,
+                                    user_agent: "some user agent 2",
+                                    client_ip: "1.1.2.3")
+
+    lookup_token = UserAuthToken.lookup(token.unhashed_auth_token)
+
+    expect(user.id).to eq(lookup_token.user.id)
+
+    lookup_token = UserAuthToken.lookup(token.auth_token)
+
+    expect(lookup_token).to eq(nil)
+
+    token.update_columns(legacy: true)
+
+    lookup_token = UserAuthToken.lookup(token.auth_token)
+
+    expect(user.id).to eq(lookup_token.user.id)
+  end
+
+  it "can validate token was seen at lookup time" do
+
+    user = Fabricate(:user)
+
+    user_token = UserAuthToken.generate!(user_id: user.id,
+                                    user_agent: "some user agent 2",
+                                    client_ip: "1.1.2.3")
+
+    expect(user_token.auth_token_seen).to eq(false)
+
+    UserAuthToken.lookup(user_token.unhashed_auth_token, seen: true)
+
+    user_token.reload
+    expect(user_token.auth_token_seen).to eq(true)
+
+  end
+
+  it "can rotate with no params maintaining data" do
+
+    user = Fabricate(:user)
+
+    user_token = UserAuthToken.generate!(user_id: user.id,
+                                    user_agent: "some user agent 2",
+                                    client_ip: "1.1.2.3")
+
+    user_token.update_columns(auth_token_seen: true)
+    expect(user_token.rotate!).to eq(true)
+    user_token.reload
+    expect(user_token.client_ip.to_s).to eq("1.1.2.3")
+    expect(user_token.user_agent).to eq("some user agent 2")
+  end
+
+  it "can properly rotate tokens" do
+
+    user = Fabricate(:user)
+
+    user_token = UserAuthToken.generate!(user_id: user.id,
+                                    user_agent: "some user agent 2",
+                                    client_ip: "1.1.2.3")
+
+    prev_auth_token = user_token.auth_token
+    unhashed_prev = user_token.unhashed_auth_token
+
+    rotated = user_token.rotate!(user_agent: "a new user agent", client_ip: "1.1.2.4")
+    expect(rotated).to eq(false)
+
+    user_token.update_columns(auth_token_seen: true)
+
+    rotated = user_token.rotate!(user_agent: "a new user agent", client_ip: "1.1.2.4")
+    expect(rotated).to eq(true)
+
+    user_token.reload
+
+    expect(user_token.rotated_at).to be_within(5.second).of(Time.zone.now)
+    expect(user_token.client_ip).to eq("1.1.2.4")
+    expect(user_token.user_agent).to eq("a new user agent")
+    expect(user_token.auth_token_seen).to eq(false)
+    expect(user_token.prev_auth_token).to eq(prev_auth_token)
+
+    # ability to auth using an old token
+    looked_up = UserAuthToken.lookup(unhashed_prev)
+    expect(looked_up.id).to eq(user_token.id)
+
+    freeze_time(2.minute.from_now) do
+      looked_up = UserAuthToken.lookup(unhashed_prev)
+      expect(looked_up).to eq(nil)
+    end
+  end
+
+end

spec/models/user_spec.rb

@@ -592,21 +592,18 @@ describe User do
       @user.password = "ilovepasta"
       @user.save!
 
-      @user.auth_token = SecureRandom.hex(16)
-      @user.save!
-
       expect(@user.active).to eq(false)
       expect(@user.confirm_password?("ilovepasta")).to eq(true)
 
-
       email_token = @user.email_tokens.create(email: 'pasta@delicious.com')
 
-      old_token = @user.auth_token
+      UserAuthToken.generate!(user_id: @user.id)
+
       @user.password = "passwordT"
       @user.save!
 
       # must expire old token on password change
-      expect(@user.auth_token).to_not eq(old_token)
+      expect(@user.user_auth_tokens.count).to eq(0)
 
       email_token.reload
       expect(email_token.expired).to eq(true)

spec/services/user_anonymizer_spec.rb

@@ -4,7 +4,7 @@ describe UserAnonymizer do
 
   describe "make_anonymous" do
     let(:admin) { Fabricate(:admin) }
-    let(:user) { Fabricate(:user, username: "edward", auth_token: "mysecretauthtoken") }
+    let(:user) { Fabricate(:user, username: "edward") }
 
     subject(:make_anonymous) { described_class.make_anonymous(user, admin) }
 
@@ -51,6 +51,8 @@ describe UserAnonymizer do
 
         prev_username = user.username
 
+        UserAuthToken.generate!(user_id: user.id)
+
         make_anonymous
         user.reload
 
@@ -58,7 +60,7 @@ describe UserAnonymizer do
         expect(user.name).not_to be_present
         expect(user.date_of_birth).to eq(nil)
         expect(user.title).not_to be_present
-        expect(user.auth_token).to eq(nil)
+        expect(user.user_auth_tokens.count).to eq(0)
 
         profile = user.user_profile(true)
         expect(profile.location).to eq(nil)