Skip to content

Commit

Permalink
Fixes #36699 - Reject invalid expiration dates for PATs
Browse files Browse the repository at this point in the history
Previously invalid dates were silently discarded, resulting into PATs being
created without expiration dates.
  • Loading branch information
adamruzicka committed Aug 24, 2023
1 parent 0f1d142 commit 23e287a
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 1 deletion.
19 changes: 19 additions & 0 deletions app/models/personal_access_token.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)) ||
Expand Down Expand Up @@ -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
24 changes: 23 additions & 1 deletion test/models/personal_access_token_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 23e287a

Please sign in to comment.