From ab55c642bdb4624069c943e7739852d8d3961d56 Mon Sep 17 00:00:00 2001 From: Stephen Hosom Date: Tue, 22 Sep 2026 11:04:22 -0400 Subject: [PATCH 1/2] Stabilize entitlement calculations Keep one evaluation timestamp for each run, allow callers to set it explicitly, handle empty AND rules, and avoid caching in-progress calculation sentinels. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 10248eee-e35a-46a7-9705-9bebc7b5318b --- lib/entitlements.rb | 19 ++++++++++++++ .../data/groups/calculated/base.rb | 4 +-- .../groups/calculated/modifiers/expiration.rb | 2 +- .../data/groups/calculated/text.rb | 12 +++++---- .../data/groups/calculated/yaml.rb | 10 +++++--- .../data/groups/calculated/base_spec.rb | 9 +++++++ .../calculated/modifiers/expiration_spec.rb | 9 +++++++ .../data/groups/calculated/text_spec.rb | 8 ++++++ .../data/groups/calculated/yaml_spec.rb | 10 ++++++++ spec/unit/entitlements_spec.rb | 25 +++++++++++++++++++ 10 files changed, 96 insertions(+), 12 deletions(-) diff --git a/lib/entitlements.rb b/lib/entitlements.rb index 0239733..21dc118 100644 --- a/lib/entitlements.rb +++ b/lib/entitlements.rb @@ -90,6 +90,7 @@ def self.reset! @config = nil @config_file = nil @config_path_override = nil + @evaluation_time = nil @person_extra_methods = {} @statsd = nil @@ -97,6 +98,24 @@ def self.reset! Entitlements::Data::Groups::Calculated.reset! end + # Return the fixed time used for the current date-sensitive entitlement evaluation. + # + # Returns a Time. + Contract C::None => Time + def self.evaluation_time + @evaluation_time ||= Time.now + end + + # Set the time used for date-sensitive entitlement evaluation. + # + # value - A Time. + # + # Returns the supplied Time. + Contract Time => Time + def self.evaluation_time=(value) + @evaluation_time = value + end + def self.reset_extras! extras_loaded = @extras_loaded if extras_loaded diff --git a/lib/entitlements/data/groups/calculated/base.rb b/lib/entitlements/data/groups/calculated/base.rb index d19d08b..ee4cc32 100644 --- a/lib/entitlements/data/groups/calculated/base.rb +++ b/lib/entitlements/data/groups/calculated/base.rb @@ -217,7 +217,7 @@ def expired?(expiration, context) return false if expiration.nil? || expiration.strip.empty? if expiration =~ /\A(\d{4})-(\d{2})-(\d{2})\z/ year, month, day = Regexp.last_match(1).to_i, Regexp.last_match(2).to_i, Regexp.last_match(3).to_i - return Time.utc(year, month, day, 0, 0, 0) <= Time.now.utc + return Time.utc(year, month, day, 0, 0, 0) <= Entitlements.evaluation_time.utc end message = "Invalid expiration date #{expiration.inspect} in #{context} (expected format: YYYY-MM-DD)" raise ArgumentError, message @@ -340,7 +340,7 @@ def handle_or(rule) # Returns C::SetOf[Entitlements::Models::Person] from a recursive call. def handle_and(rule) ensure_type!("and", rule, Array) - return result unless rule.any? + return Set.new unless rule.any? first_rule = rule.shift ensure_type!("and", first_rule, Hash) diff --git a/lib/entitlements/data/groups/calculated/modifiers/expiration.rb b/lib/entitlements/data/groups/calculated/modifiers/expiration.rb index 4b57c7b..2436a4f 100644 --- a/lib/entitlements/data/groups/calculated/modifiers/expiration.rb +++ b/lib/entitlements/data/groups/calculated/modifiers/expiration.rb @@ -30,7 +30,7 @@ def modify(result) end # If the date is in the future, leave the entitlement unchanged. - return false if parse_date > Time.now.utc.to_date + return false if parse_date > Entitlements.evaluation_time.utc.to_date # Empty the group. Set metadata allowing no members. Return true to indicate modification. rs.metadata["no_members_ok"] = true diff --git a/lib/entitlements/data/groups/calculated/text.rb b/lib/entitlements/data/groups/calculated/text.rb index b78828a..1513778 100644 --- a/lib/entitlements/data/groups/calculated/text.rb +++ b/lib/entitlements/data/groups/calculated/text.rb @@ -21,10 +21,12 @@ class Text < Entitlements::Data::Groups::Calculated::Base # Returns a Set[String] with DN's of the people in the group. Contract C::None => C::Or[:calculating, C::SetOf[Entitlements::Models::Person]] def members - @members ||= begin - Entitlements.logger.debug "Calculating members from #{filename}" - members_from_rules(rules) - end + return @members if @members + + Entitlements.logger.debug "Calculating members from #{filename}" + result = members_from_rules(rules) + @members = result unless result == :calculating + result end # Standard interface: Get the description of this group. @@ -182,7 +184,7 @@ def rules if parsed_data.key?("modifier_expiration") && affirmative.empty? exp_date = parsed_data.fetch("modifier_expiration").fetch("=").first.fetch(:key) date = Entitlements::Util::Util.parse_date(exp_date) - return {"always" => false} if date <= Time.now.utc.to_date + return {"always" => false} if date <= Entitlements.evaluation_time.utc.to_date end # There has to be at least one affirmative condition, not just all negative ones. diff --git a/lib/entitlements/data/groups/calculated/yaml.rb b/lib/entitlements/data/groups/calculated/yaml.rb index 0a45a78..73f4136 100644 --- a/lib/entitlements/data/groups/calculated/yaml.rb +++ b/lib/entitlements/data/groups/calculated/yaml.rb @@ -18,10 +18,12 @@ class YAML < Entitlements::Data::Groups::Calculated::Base # Returns a Set[String] with DN's of the people in the group. Contract C::None => C::Or[:calculating, C::SetOf[Entitlements::Models::Person]] def members - @members ||= begin - Entitlements.logger.debug "Calculating members from #{filename}" - members_from_rules(rules) - end + return @members if @members + + Entitlements.logger.debug "Calculating members from #{filename}" + result = members_from_rules(rules) + @members = result unless result == :calculating + result end # Standard interface: Get the description of this group. diff --git a/spec/unit/entitlements/data/groups/calculated/base_spec.rb b/spec/unit/entitlements/data/groups/calculated/base_spec.rb index d1cd846..89fd5f2 100644 --- a/spec/unit/entitlements/data/groups/calculated/base_spec.rb +++ b/spec/unit/entitlements/data/groups/calculated/base_spec.rb @@ -158,6 +158,15 @@ end end + context "with an empty 'and' rule set" do + let(:file) { fixture("ldap-config/logic_tests/simple_and.yaml") } + let(:obj) { Entitlements::Data::Groups::Calculated::YAML.new(filename: file, config: config) } + + it "returns an empty set" do + expect(obj.send(:handle_and, [])).to eq(Set.new) + end + end + context "with a simple 'or' rule set" do let(:file) { fixture("ldap-config/logic_tests/simple_or.yaml") } let(:obj) { Entitlements::Data::Groups::Calculated::YAML.new(filename: file, config: config) } diff --git a/spec/unit/entitlements/data/groups/calculated/modifiers/expiration_spec.rb b/spec/unit/entitlements/data/groups/calculated/modifiers/expiration_spec.rb index 35d3944..e386ce1 100644 --- a/spec/unit/entitlements/data/groups/calculated/modifiers/expiration_spec.rb +++ b/spec/unit/entitlements/data/groups/calculated/modifiers/expiration_spec.rb @@ -50,6 +50,15 @@ obj = Entitlements::Data::Groups::Calculated.read("cn=expired-text-empty,ou=Felines,ou=Groups,dc=example,dc=net") expect(obj.members).to eq(Set.new) end + + it "uses the configured evaluation time" do + Entitlements.evaluation_time = Time.utc(2000, 1, 1) + allow(Entitlements::Util::Util).to receive(:path_for_group).with(ou_key).and_return(fixture("ldap-config/#{ou_key}")) + Entitlements::Data::Groups::Calculated.read_all(ou_key, cfg_obj) + obj = Entitlements::Data::Groups::Calculated.read("cn=expired-text-empty,ou=Felines,ou=Groups,dc=example,dc=net") + expected_result = %w[russianblue mainecoon] + expect(obj.members).to eq(Set.new(expected_result.map { |name| people_obj.read[name] })) + end end context "non-expired non-expired yaml file (date as date)" do diff --git a/spec/unit/entitlements/data/groups/calculated/text_spec.rb b/spec/unit/entitlements/data/groups/calculated/text_spec.rb index d1f5913..3a493dd 100644 --- a/spec/unit/entitlements/data/groups/calculated/text_spec.rb +++ b/spec/unit/entitlements/data/groups/calculated/text_spec.rb @@ -32,6 +32,14 @@ answer_set = Set.new(answer_array) expect(result_set).to eq(answer_set) end + + it "does not cache the calculating sentinel" do + members = Set.new([people_obj.read["blackmanx"]]) + allow(subject).to receive(:members_from_rules).and_return(:calculating, members) + + expect(subject.members).to eq(:calculating) + expect(subject.members).to eq(members) + end end describe "#description" do diff --git a/spec/unit/entitlements/data/groups/calculated/yaml_spec.rb b/spec/unit/entitlements/data/groups/calculated/yaml_spec.rb index 8a176ae..8916d41 100644 --- a/spec/unit/entitlements/data/groups/calculated/yaml_spec.rb +++ b/spec/unit/entitlements/data/groups/calculated/yaml_spec.rb @@ -21,6 +21,16 @@ expect(result.size).to eq(2) expect(result.map { |i| i.uid }.sort).to eq(answer) end + + it "does not cache the calculating sentinel" do + filename = fixture("ldap-config/filters/no-filters.yaml") + subject = described_class.new(filename: filename) + members = Set.new([people_obj.read["blackmanx"]]) + allow(subject).to receive(:members_from_rules).and_return(:calculating, members) + + expect(subject.members).to eq(:calculating) + expect(subject.members).to eq(members) + end end describe "#description" do diff --git a/spec/unit/entitlements_spec.rb b/spec/unit/entitlements_spec.rb index 390608f..cad08fb 100644 --- a/spec/unit/entitlements_spec.rb +++ b/spec/unit/entitlements_spec.rb @@ -5,6 +5,31 @@ describe Entitlements do let(:subject) { Entitlements } + describe "#evaluation_time" do + it "uses one timestamp until Entitlements state is reset" do + first_time = Time.utc(2026, 9, 16, 23, 59, 59) + next_time = Time.utc(2026, 9, 17, 0, 0, 0) + allow(Time).to receive(:now).and_return(first_time, next_time) + + expect(subject.evaluation_time).to eq(first_time) + expect(subject.evaluation_time).to eq(first_time) + + subject.reset! + + expect(subject.evaluation_time).to eq(next_time) + end + + it "preserves an explicitly configured evaluation timestamp" do + configured_time = Time.utc(2026, 9, 16, 12, 0, 0) + + subject.evaluation_time = configured_time + + expect(subject.evaluation_time).to eq(configured_time) + expect(Time).not_to receive(:now) + expect(subject.evaluation_time).to eq(configured_time) + end + end + describe "#config" do before(:each) do ENV["TEST_ERB_VARIABLE"] = "Hello, ERB world!" From 0c6e2997f7caa52a14939b976141692c14005993 Mon Sep 17 00:00:00 2001 From: Stephen Hosom Date: Tue, 22 Sep 2026 11:36:10 -0400 Subject: [PATCH 2/2] Bump entitlements-app to 1.2.4 Prepare the entitlement calculation correctness fixes for publication. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 10248eee-e35a-46a7-9705-9bebc7b5318b --- Gemfile.lock | 2 +- lib/version.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 5189114..17e5800 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - entitlements-app (1.2.3) + entitlements-app (1.2.4) concurrent-ruby (~> 1.3, >= 1.3.1) dogstatsd-ruby (~> 5.7) faraday (~> 2.0) diff --git a/lib/version.rb b/lib/version.rb index cd6c58b..6908551 100644 --- a/lib/version.rb +++ b/lib/version.rb @@ -2,6 +2,6 @@ module Entitlements module Version - VERSION = "1.2.3" + VERSION = "1.2.4" end end