Skip to content

DRAFT: Test we validate keys properly in the future

This is a Proof-of-Concept / additional test case for #1007 (closed). It can be merged in to the 1007 if desired.

As noted in !75098 (comment 743111208), the current logic for checking expires_at_before_max_expiry_date will fail to be correct when:

  1. A key already exists with an expiry in the future AND
  2. An admin enforces a max lifetime which is LESS than the expiry of the key.

For the days between key.expires_at - application.max_key_lifetime and key.expires_at, the key will become valid again. The key should remain invalid.

In the following spec, the admin has configured a max lifetime of 10 days. The first key, having an expires_at > 10 days, should always be invalid. However, on Day 19, we can see by the failing test case that it is evaluating as valid? == true, making the test fail

      context 'when keys and max expiry are set' do
        where(:key, :max_ssh_key_lifetime, :valid) do
          build(:personal_key, expires_at:  20.days.from_now) |  10 | false
          build(:personal_key, expires_at:   7.days.from_now) |  10 | true
        end

        with_them do
          it 'checks validity properly in the future too' do
            # Travel to the day before the key is set to
            # 'expire'. max_ssh_key_lifetime should still be
            # enforced correctly.
            travel_to(key.expires_at - 1) do
              expect(key.valid?).to eq(valid)
            end
          end
        end
      end
gitlab git:(1007-nmalcolm) ✗ ss ee/spec/models/ee/key_spec.rb
Running via Spring preloader in process 3040
Run options: include {:focus=>true}

All examples were filtered out; ignoring {:focus=>true}

Test environment set up in 8.887059 seconds
.........F...........

Failures:

  1) Key validations #expires_at_before_max_expiry_date when keys and max expiry are set key: #<Key id: nil, user_id: nil, created_at: nil, updated_at: nil, key: [FILTERED], title: [FILTERED], type: nil, fingerprint: nil, public: false, last_used_at: nil, fingerprint_sha256: nil, expires_at: "2021-12-15 22:22:26.951664000 +0000", expiry_notification_delivered_at: nil, before_expiry_notification_delivered_at: nil>, max_ssh_key_lifetime: 10, valid: false checks properly in the future too
     Failure/Error: expect(key.valid?).to eq(valid)

       expected: false
            got: true

       (compared using ==)

...

Finished in 11.72 seconds (files took 5.58 seconds to load)
21 examples, 1 failure

Failed examples:

rspec './ee/spec/models/ee/key_spec.rb[1:1:2:2:1:1]' # Key validations #expires_at_before_max_expiry_date when keys and max expiry are set key: #<Key id: nil, user_id: nil, created_at: nil, updated_at: nil, key: [FILTERED], title: [FILTERED], type: nil, fingerprint: nil, public: false, last_used_at: nil, fingerprint_sha256: nil, expires_at: "2021-12-15 22:22:26.951664000 +0000", expiry_notification_delivered_at: nil, before_expiry_notification_delivered_at: nil>, max_ssh_key_lifetime: 10, valid: false checks properly in the future too

Merge request reports