Fixes theforeman#3468 - Move token expiry to scope to avoid FK issues

Rakefile

@@ -2,6 +2,6 @@ require File.expand_path('../config/application', __FILE__)
 require 'rake'
 include Rake::DSL
 
-SingleTest.load_tasks if defined? SingleTest
+require 'single_test/tasks' if defined? SingleTest
 
 Foreman::Application.load_tasks

app/models/host/managed.rb

@@ -19,7 +19,7 @@ class Host::Managed < Host::Base
   belongs_to :location
   belongs_to :organization
 
-  has_one :token, :foreign_key => :host_id, :dependent => :destroy, :conditions => Proc.new {"expires >= '#{Time.now.utc.to_s(:db)}'"}
+  has_one :token, :foreign_key => :host_id, :dependent => :destroy
 
   # Define custom hook that can be called in model by magic methods (before, after, around)
   define_model_callbacks :build, :only => :after
@@ -104,7 +104,7 @@ class Jail < ::Safemode::Jail
     end
   }
 
-  scope :for_token, lambda { |token| joins(:token).where(:tokens => { :value => token }).select('hosts.*') }
+  scope :for_token, lambda { |token| joins(:token).where(:tokens => { :value => token }).where("expires >= ?", Time.now.utc.to_s(:db)).select('hosts.*') }
 
   # audit the changes to this model
   audited :except => [:last_report, :puppet_status, :last_compile], :allow_mass_assignment => true
@@ -187,9 +187,8 @@ def set_token
                       :expires => Time.now.utc + Setting[:token_duration].minutes)
   end
 
-  def expire_tokens
-    # this clean up other hosts as well, but reduce the need for another task to cleanup tokens.
-    Token.where(["expires < ? or host_id = ?", Time.now.utc.to_s(:db), id]).delete_all
+  def expire_token
+    self.token.delete if self.token.present?
   end
 
   # Called from the host build post install process to indicate that the base build has completed

app/observers/host_observer.rb

@@ -12,7 +12,7 @@ def after_validation(host)
     end
     # existing server change build mode
     if host.respond_to?(:old) and host.old and host.build? != host.old.build?
-      host.build? ? host.set_token : host.expire_tokens
+      host.build? ? host.set_token : host.expire_token
     end
   end
 

test/unit/host_test.rb

@@ -771,7 +771,7 @@ class Host::Valid < Host::Base ; belongs_to :domain ; end
     h = hosts(:one)
     h.create_token(:value => "aaaaaa", :expires => Time.now)
     assert_equal Token.all.size, 1
-    h.expire_tokens
+    h.expire_token
     assert_equal Token.all.size, 0
   end
 
@@ -780,7 +780,7 @@ class Host::Valid < Host::Base ; belongs_to :domain ; end
     h = hosts(:one)
     h.create_token(:value => "aaaaaa", :expires => Time.now)
     assert_equal Token.all.size, 1
-    h.expire_tokens
+    h.expire_token
     assert_equal Token.all.size, 0
   end
 
@@ -799,6 +799,24 @@ class Host::Valid < Host::Base ; belongs_to :domain ; end
     assert_equal h.token, nil
   end
 
+  test "a token can be matched to a host" do
+    h = hosts(:one)
+    h.create_token(:value => "aaaaaa", :expires => Time.now + 1.minutes)
+    assert_equal h, Host.for_token("aaaaaa").first
+  end
+
+  test "a token cannot be matched to a host when expired" do
+    h = hosts(:one)
+    h.create_token(:value => "aaaaaa", :expires => 1.minutes.ago)
+    refute Host.for_token("aaaaaa").first
+  end
+
+  test "deleting an host with an expired token does not cause a Foreign Key error" do
+    h=hosts(:one)
+    h.create_token(:value => "aaaaaa", :expires => 5.minutes.ago)
+    assert_nothing_raised(ActiveRecord::InvalidForeignKey) {h.reload.destroy}
+  end
+
   test "can search hosts by hostgroup" do
     #setup - add parent to hostgroup :common (not in fixtures, since no field parent_id)
     hostgroup = hostgroups(:db)

test/unit/token_test.rb

@@ -30,12 +30,6 @@ class TokenTest < ActiveSupport::TestCase
     assert_equal Token.first.host_id, h.id
   end
 
-  test "a token can be matched to a host" do
-    h = hosts(:one)
-    h.create_token(:value => "aaaaaa", :expires => Time.now + 1.minutes)
-    assert_equal h, Host.for_token("aaaaaa").first
-  end
-
   test "a host can delete its token" do
     h = hosts(:one)
     h.create_token(:value => "aaaaaa", :expires => Time.now + 1.minutes)
@@ -54,14 +48,14 @@ class TokenTest < ActiveSupport::TestCase
     assert_equal Token.all.size, 1
   end
 
-  test "all expired tokens should be removed" do
+  test "not all expired tokens should be removed" do
     h1 = hosts(:one)
     h2 = hosts(:two)
     h1.create_token(:value => "aaaaaa", :expires => Time.now + 1.minutes)
     h2.create_token(:value => "bbbbbb", :expires => Time.now - 1.minutes)
-    assert_equal Token.count, 2
-    h1.expire_tokens
-    assert_equal 0, Token.count
+    assert_equal 2, Token.count
+    h1.expire_token
+    assert_equal 1, Token.count
   end
 
 end