-
Notifications
You must be signed in to change notification settings - Fork 990
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes #36699 - Reject invalid expiration dates for PATs #9811
Conversation
Issues: #36699 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small nits to use a more precise assertion, but otherwise 👍
Checked using the related test in robottelo automation against patched stream build:
|
52d0cea
to
33bc3d7
Compare
app/models/personal_access_token.rb
Outdated
|
||
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") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we translate the errors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, fixed
Previously invalid dates were silently discarded, resulting into PATs being created without expiration dates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @adamruzicka, checked with hammer.
Before:
$ hammer user access create --user user --name name --expires-at 2021-12-12
Personal access token [name] created:
cMDyzdVNTMpOG8FI1cZALw
$ hammer user access create --user user --name name --expires-at 2024-44-44
Personal access token [name] created:
iavA1FNNMtoSQkvV8TtoZA
After:
$ hammer user access create --user user --name name --expires-at 2021-12-12
Could not create personal access token:
Expires at cannot be in the past
$ hammer user access create --user user --name name --expires-at 2024-44-44
Could not create personal access token:
Expires at Could not parse timestamp '2024-44-44'
Side-note: currently the issue seems to appear only for API/hammer side. In UI we do these checks, so the PAT won't be created unless it's valid per there checks as well.
Previously invalid dates were silently discarded, resulting into PATs being created without expiration dates.