Commit 0c505e40 authored by Stan Hu's avatar Stan Hu

Avoid 500 errors with long expiration dates in tokens

If a user created a personal access token with an expiration date far in
advance, the date would be saved but a 500 error would be shown on
display:

```
PG::DatetimeFieldOverflow: ERROR:  date out of range for timestamp
```

To fix this, since we're comparing dates, we shouldn't use NOW in the
SQL query.  Instead, we use CURRENT_DATE.

This fix is similar to
https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/25806.

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/26306
parent 1457a8b9
...@@ -17,9 +17,9 @@ class PersonalAccessToken < ApplicationRecord ...@@ -17,9 +17,9 @@ class PersonalAccessToken < ApplicationRecord
before_save :ensure_token before_save :ensure_token
scope :active, -> { where("revoked = false AND (expires_at >= NOW() OR expires_at IS NULL)") } scope :active, -> { where("revoked = false AND (expires_at >= CURRENT_DATE OR expires_at IS NULL)") }
scope :expiring_and_not_notified, ->(date) { where(["revoked = false AND expire_notification_delivered = false AND expires_at >= NOW() AND expires_at <= ?", date]) } scope :expiring_and_not_notified, ->(date) { where(["revoked = false AND expire_notification_delivered = false AND expires_at >= CURRENT_DATE AND expires_at <= ?", date]) }
scope :inactive, -> { where("revoked = true OR expires_at < NOW()") } scope :inactive, -> { where("revoked = true OR expires_at < CURRENT_DATE") }
scope :with_impersonation, -> { where(impersonation: true) } scope :with_impersonation, -> { where(impersonation: true) }
scope :without_impersonation, -> { where(impersonation: false) } scope :without_impersonation, -> { where(impersonation: false) }
scope :revoked, -> { where(revoked: true) } scope :revoked, -> { where(revoked: true) }
......
---
title: Avoid 500 errors with long expiration dates in tokens
merge_request: 36657
author:
type: fixed
...@@ -17,7 +17,7 @@ module EE ...@@ -17,7 +17,7 @@ module EE
scope :with_no_expires_at, -> { where(revoked: false, expires_at: nil) } scope :with_no_expires_at, -> { where(revoked: false, expires_at: nil) }
scope :with_expires_at_after, ->(max_lifetime) { where(revoked: false).where('expires_at > ?', max_lifetime) } scope :with_expires_at_after, ->(max_lifetime) { where(revoked: false).where('expires_at > ?', max_lifetime) }
scope :expires_in, ->(within) { not_revoked.where('expires_at > NOW() AND expires_at <= ?', within) } scope :expires_in, ->(within) { not_revoked.where('expires_at > CURRENT_DATE AND expires_at <= ?', within) }
scope :created_on_or_after, ->(date) { active.where('created_at >= ?', date) } scope :created_on_or_after, ->(date) { active.where('created_at >= ?', date) }
with_options if: :expiration_policy_enabled? do with_options if: :expiration_policy_enabled? do
......
...@@ -6,6 +6,7 @@ RSpec.describe PersonalAccessToken do ...@@ -6,6 +6,7 @@ RSpec.describe PersonalAccessToken do
describe 'scopes' do describe 'scopes' do
let_it_be(:expired_token) { create(:personal_access_token, expires_at: 1.day.ago) } let_it_be(:expired_token) { create(:personal_access_token, expires_at: 1.day.ago) }
let_it_be(:valid_token) { create(:personal_access_token, expires_at: 1.day.from_now) } let_it_be(:valid_token) { create(:personal_access_token, expires_at: 1.day.from_now) }
let_it_be(:long_expiry_token) { create(:personal_access_token, expires_at: '999999-12-31'.to_date) }
let!(:pat) { create(:personal_access_token, expires_at: expiration_date) } let!(:pat) { create(:personal_access_token, expires_at: expiration_date) }
describe 'with_expires_at_after' do describe 'with_expires_at_after' do
...@@ -14,7 +15,7 @@ RSpec.describe PersonalAccessToken do ...@@ -14,7 +15,7 @@ RSpec.describe PersonalAccessToken do
let(:expiration_date) { 3.days.from_now } let(:expiration_date) { 3.days.from_now }
it 'includes the tokens with higher than the lifetime expires_at value' do it 'includes the tokens with higher than the lifetime expires_at value' do
expect(subject).to contain_exactly(pat) expect(subject).to contain_exactly(pat, long_expiry_token)
end end
it "doesn't contain expired tokens" do it "doesn't contain expired tokens" do
...@@ -43,6 +44,16 @@ RSpec.describe PersonalAccessToken do ...@@ -43,6 +44,16 @@ RSpec.describe PersonalAccessToken do
expect(subject).not_to include(valid_token) expect(subject).not_to include(valid_token)
end end
end end
describe 'expires_in' do
subject { described_class.expires_in(1.day.from_now) }
let(:expiration_date) { nil }
it 'only includes one token' do
expect(subject).to contain_exactly(valid_token)
end
end
end end
describe 'validations' do describe 'validations' do
......
...@@ -165,6 +165,7 @@ RSpec.describe PersonalAccessToken do ...@@ -165,6 +165,7 @@ RSpec.describe PersonalAccessToken do
let_it_be(:revoked_token) { create(:personal_access_token, revoked: true) } let_it_be(:revoked_token) { create(:personal_access_token, revoked: true) }
let_it_be(:valid_token_and_notified) { create(:personal_access_token, expires_at: 2.days.from_now, expire_notification_delivered: true) } let_it_be(:valid_token_and_notified) { create(:personal_access_token, expires_at: 2.days.from_now, expire_notification_delivered: true) }
let_it_be(:valid_token) { create(:personal_access_token, expires_at: 2.days.from_now) } let_it_be(:valid_token) { create(:personal_access_token, expires_at: 2.days.from_now) }
let_it_be(:long_expiry_token) { create(:personal_access_token, expires_at: '999999-12-31'.to_date) }
context 'in one day' do context 'in one day' do
it "doesn't have any tokens" do it "doesn't have any tokens" do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment