Fixes #36699 - Reject invalid expiration dates for PATs

Previously invalid dates were silently discarded, resulting into PATs being
created without expiration dates.

app/models/personal_access_token.rb

@@ -19,6 +19,8 @@ class PersonalAccessToken < ApplicationRecord
   scope :inactive, -> { where(revoked: true).or(where("expires_at < ?", Time.current.utc)) }
 
   attr_accessor :token_value
+  validate :expires_at_in_future
+  validate :valid_expires_at
 
   def self.authenticate_user(user, token)
     token = active.find_by(user: user, token: hash_token(user, token, :pbkdf2sha1)) ||
@@ -76,4 +78,21 @@ def active?
   def used?
     last_used_at.present?
   end
+
+  def expires_at=(value)
+    @expires_at_raw = value
+    super
+  end
+
+  def valid_expires_at
+    if @expires_at_raw && expires_at.nil?
+      errors.add(:expires_at, _("Could not parse timestamp '%{timestamp}'") % { timestamp: @expires_at_raw })
+    end
+  end
+
+  def expires_at_in_future
+    if changes.key?('expires_at') && expires_at.present? && expires_at < Time.zone.now
+      errors.add(:expires_at, _("cannot be in the past"))
+    end
+  end
 end

test/models/personal_access_token_test.rb

@@ -59,11 +59,33 @@ class PersonalAccessTokenTest < ActiveSupport::TestCase
       assert_includes PersonalAccessToken.active.where(:user => user), token
       refute_includes PersonalAccessToken.inactive.where(:user => user), token
       token.expires_at = Date.yesterday
-      token.save
+      token.save(validate: false)
       assert_equal true, token.expires?
       assert_equal false, token.active?
       assert_includes PersonalAccessToken.inactive.where(:user => user), token
       refute_includes PersonalAccessToken.active.where(:user => user), token
     end
+
+    test 'bogus dates expiry dates are not accepted' do
+      token.expires_at = '2023-08-34T12:00:00.000Z'
+      refute token.valid?
+      err, * = token.errors['expires_at']
+      assert_match(/Could not parse timestamp/, err)
+    end
+
+    test 'dates in the past are not accepted' do
+      token.expires_at = '1970-01-01T12:00:00.000Z'
+      refute token.valid?
+      err, * = token.errors['expires_at']
+      assert_match(/cannot be in the past/, err)
+    end
+
+    test 'token with date in the past can be updated' do
+      token.expires_at = Date.yesterday
+      assert token.save(validate: false)
+      token.name = token.name + '1'
+      token.expires_at = Date.yesterday
+      assert token.valid?
+    end
   end
 end

test/models/user_test.rb

@@ -1176,7 +1176,12 @@ def editing_self_helper
       token.save
       token_value
     end
-    let(:expired_token) { FactoryBot.create(:personal_access_token, :user => user, :expires_at => 4.weeks.ago) }
+    let(:expired_token) do
+      token = FactoryBot.create(:personal_access_token, :user => user)
+      token.expires_at = 4.weeks.ago
+      token.save(validate: false)
+      token
+    end
     let(:expired_token_value) do
       token_value = expired_token.generate_token
       expired_token.save