Skip to content
Browse files

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

  • Loading branch information...
1 parent 4d916f4 commit 585f328e6bbe9e8f429ddb2ccc97fb119da959b7 @GregSutcliffe GregSutcliffe committed with domcleal
Showing with 30 additions and 19 deletions.
  1. +1 −1 Rakefile
  2. +4 −5 app/models/host/managed.rb
  3. +1 −1 app/observers/host_observer.rb
  4. +20 −2 test/unit/host_test.rb
  5. +4 −10 test/unit/token_test.rb
View
2 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
View
9 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
View
2 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
View
22 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)
View
14 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

0 comments on commit 585f328

Please sign in to comment.
Something went wrong with that request. Please try again.