feat(cognito): support refresh token rotation#34360
feat(cognito): support refresh token rotation#34360mergify[bot] merged 50 commits intoaws:mainfrom iridescent99:cognito/refresh-token
Conversation
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
|
Thanks! This okay? |
leonmk-aws
left a comment
There was a problem hiding this comment.
Small comment, feel free to add your opinion on it
| * @see https://docs.aws.amazon.com/cognito/latest/developerguide/amazon-cognito-user-pools-using-the-refresh-token.html#using-the-refresh-token-rotation | ||
| * @default - undefined (refresh token rotation is disabled) | ||
| */ | ||
| readonly refreshTokenGracePeriod?: Duration; |
There was a problem hiding this comment.
I think having the variable named refreshTokenRotationGracePeriod would bring visibility that this is only effective when using refresh token rotation because people could get confused that this sets a grace period when not using the refresh token rotation feature. Also I think updating the documentation to make it clear that this enables the refresh token rotation feature would be valuable (the README is very clear in that regard, but having it here as well would be important even if the link to the doc is provided).
There was a problem hiding this comment.
To be more clear: before the changes of yesterday, it was very clear for the user that this field only is only effective when enabling refresh token rotation and I feel that when flattening we slightly lost this clarity and we can improve this.
There was a problem hiding this comment.
That's fair, I was having doubts about the description here as well. Sorry about the name, I didnt read that correctly.
So I will change the name to refreshTokenRotationGracePeriod.
and for the description this ok?
Enables refresh token rotation when set.
Defines the grace period for the original refresh token (0-60 seconds).
Can I still push these changes after approval?
There was a problem hiding this comment.
Yes, you can make changes after approval
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
This pull request has been removed from the queue for the following reason: The pull request can't be updated. You should update or rebase your pull request manually. If you do, this pull request will automatically be requeued once the queue conditions match again. |
Pull request has been modified.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
ozelalisen
left a comment
There was a problem hiding this comment.
Could you resolve merge conflicts?
Pull request has been modified.
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
This pull request has been removed from the queue for the following reason: The pull request can't be updated. You should update or rebase your pull request manually. If you do, this pull request will automatically be requeued once the queue conditions match again. |
Pull request has been modified.
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Comments on closed issues and PRs are hard for our team to see. |
Issue # (if applicable)
Closes #34344
Reason for this change
Cognito added support for short-lived refresh tokens.
Description of changes
Added refreshTokenRotationGracePeriod property to UserPoolClient
Describe any new or updated permissions being added
NA
Description of how you validated changes
Unit + integration tests
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license