From 23e287a6541ee935fff317fc7a8d588cc1c57f5d Mon Sep 17 00:00:00 2001
From: Adam Ruzicka <aruzicka@redhat.com>
Date: Thu, 24 Aug 2023 15:21:23 +0200
Subject: [PATCH] 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 ++++++++++++++++++
 test/models/personal_access_token_test.rb | 24 ++++++++++++++++++++++-
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/app/models/personal_access_token.rb b/app/models/personal_access_token.rb
index f09d60fa4092..c473cd808b66 100644
--- a/app/models/personal_access_token.rb
+++ b/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 && attributes['expires_at'].nil?
+      errors.add(:expires_at, "Could not parse timestamp '#{@expires_at_raw}'")
+    end
+  end
+
+  def expires_at_in_future
+    if changes.key?('expires_at') && expires_at.present? && expires_at < Time.zone.today
+      errors.add(:expires_at, "cannot be in the past")
+    end
+  end
 end
diff --git a/test/models/personal_access_token_test.rb b/test/models/personal_access_token_test.rb
index 15fdd4fd2a70..a087d427bfc5 100644
--- a/test/models/personal_access_token_test.rb
+++ b/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 err =~ /Could not parse timestamp/
+    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 err =~ /cannot be in the past/
+    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