Skip to content
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

aws/cognito: Extend validity of the CLI's refresh tokens from 30 → 90 days #776

Merged
merged 1 commit into from
Jan 10, 2024

Conversation

tsibley
Copy link
Member

@tsibley tsibley commented Jan 9, 2024

In practice, 30 days seems too short.

I've observed users who get the CLI installed and logged in but then don't get to uploading their datasets until more than 30 days later (i.e. they spend time producing the datasets or on other work) and so then have to log in again when it's time to upload. This has been a minor frustration and stumbling block for them.

In a different use case, @corneliusroemer has also found 30 days to be too short in automation contexts. Although I'd suggest a different approach for automation¹, I suspect his approach will not be an uncommon one for users to take so we should ensure it's not too onerous when they do.

Resolves nextstrain/cli#337.

¹ nextstrain/cli#337 (comment)

Checklist

  • Checks pass

… days

In practice, 30 days seems too short.

I've observed users who get the CLI installed and logged in but then
don't get to uploading their datasets until more than 30 days later
(i.e. they spend time producing the datasets or on other work) and so
then have to log in again when it's time to upload.  This has been a
minor frustration and stumbling block for them.

In a different use case, @corneliusroemer has also found 30 days to be
too short in automation contexts.  Although I'd suggest a different
approach for automation¹, I suspect his approach will not be an uncommon
one for users to take so we should ensure it's not too onerous when they
do.

Resolves <nextstrain/cli#337>.

¹ <nextstrain/cli#337 (comment)>
@tsibley tsibley requested review from corneliusroemer and a team January 9, 2024 18:09
@nextstrain-bot nextstrain-bot temporarily deployed to nextstrain-s-trs-cli-lo-hyvtgj January 9, 2024 18:09 Inactive
@tsibley
Copy link
Member Author

tsibley commented Jan 9, 2024

Terraform plans for this change look like this, BTW, for production:

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place                                                                            
                                                                                               
Terraform will perform the following actions:                                                  
                                                                                               
  # module.cognito.aws_cognito_user_pool_client.nextstrain-cli will be updated in-place                                                                                                        
  ~ resource "aws_cognito_user_pool_client" "nextstrain-cli" {                                
        id                                            = "2vmc93kj4fiul8uv40uqge93m5"                                                                                                           
        name                                          = "nextstrain-cli"                      
      ~ refresh_token_validity                        = 30 -> 90                                                                                                                               
        # (15 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

and testing:

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # module.cognito.aws_cognito_user_pool_client.nextstrain-cli will be updated in-place
  ~ resource "aws_cognito_user_pool_client" "nextstrain-cli" {
        id                                            = "9opa27o74f4jsq8g4a34e1mqr"
        name                                          = "nextstrain-cli"
      ~ refresh_token_validity                        = 30 -> 90
        # (14 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

@tsibley tsibley merged commit b480751 into master Jan 10, 2024
7 checks passed
@tsibley tsibley deleted the trs/cli-login-validity branch January 10, 2024 22:06
@tsibley
Copy link
Member Author

tsibley commented Jan 10, 2024

Merged and deployed with:

terraform -chdir=env/testing plan -out=plan
terraform -chdir=env/production plan -out=plan

terraform -chdir=env/testing apply plan
terraform -chdir=env/production apply plan

@tsibley
Copy link
Member Author

tsibley commented Jan 10, 2024

I expect the 90 day lifetime won't be in effect for existing sessions, only newly established ones, but it depends on how Cognito handles it behind the scenes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extend validity of login refresh tokens to 90 days from 30 days
3 participants