Allow users to set auth token expiration#3479
Allow users to set auth token expiration#3479williamjallen wants to merge 1 commit intoKitware:masterfrom
Conversation
The `TOKEN_DURATION` environment variable is used to set the length of time authentication tokens are valid. This commit changes `TOKEN_DURATION` to be an upper bound on token duration, allowing users to set shorter durations.
josephsnyder
left a comment
josephsnyder
left a comment
There was a problem hiding this comment.
Thanks for the update, Will. I've got a few things that I'd like to see thought about again.
| if (!is_numeric($duration) || (int) $duration < 0) { | ||
| Log::error("Invalid token_duration configuration {$duration}"); | ||
| throw new InvalidArgumentException('Invalid token_duration configuration'); | ||
| // The default expiration date is 1 year in the future. |
There was a problem hiding this comment.
The default expiration date seems to be 6 months. This token was created today with no entries in the expiration date widget. It also states 6 months in the .env description for token length
| <input | ||
| v-model="tokenexpiration" | ||
| type="date" | ||
| :min="minTokenExpiration" |
There was a problem hiding this comment.
Can we drop the max parameter? While it does nicely disable the dates on the calendar itself, it doesn't seem to stop the other parts of the widget from scrolling into those months or selecting them in the dropdown:
It just looks incomplete
| } else { | ||
| $params['expires'] = gmdate(FMT_DATETIME, $now + $duration); | ||
| if ($expiration->isNowOrPast()) { | ||
| throw new InvalidArgumentException('Token expiration cannot be in the past.'); |
There was a problem hiding this comment.
This error should probably bubble up to the user. AFAICT, it appears in the network and console tabs, but the page isn't updated to tell the user why their token wasn't created.
| throw new InvalidArgumentException('Token expiration cannot be in the past.'); | ||
| } | ||
|
|
||
| $params['expires'] = $expiration->min(self::getMaximumTokenExpiration())->toIso8601String(); |
There was a problem hiding this comment.
Should a token set to some to beyond the maximum also be an invalid argument? Creating a token for a year or two from now and creating it with a different date without an indication of the difference seems like a recipe for confusion. It might be a personal preference rather than a universal experience
2 participants
The
TOKEN_DURATIONenvironment variable is used to set the length of time authentication tokens are valid. This commit changesTOKEN_DURATIONto be an upper bound on token duration, allowing users to set shorter durations.