nextstrain / cli

The Nextstrain command-line interface (CLI)—a program called nextstrain—which aims to provide a consistent way to run and visualize pathogen builds and access Nextstrain components like Augur and Auspice across computing environments such as Docker, Conda, and AWS Batch.
https://docs.nextstrain.org/projects/cli/
MIT License
27 stars 20 forks source link

Uncaught ImmatureSignatureError: The token is not yet valid (iat) #307

Closed corneliusroemer closed 1 month ago

corneliusroemer commented 1 year ago

Current Behavior

When I run nextstrain login I get an uncaught error nextstrain.cli.aws.cognito.TokenError: ImmatureSignatureError: The token is not yet valid (iat)

Expected behavior

Error is caught and wrapped into something useful for the enduser

How to reproduce

  1. Screw up your local system time
  2. Try nextstrain login

Example stack trace:

$ nextstrain login
Logging into Nextstrain.org…

Username: XXXXXX
Password: 

Traceback (most recent call last):
  File "nextstrain.cli.aws.cognito", line 249, in _verify_token
  File "jwt.api_jwt", line 210, in decode
  File "jwt.api_jwt", line 162, in decode_complete
  File "jwt.api_jwt", line 242, in _validate_claims
  File "jwt.api_jwt", line 278, in _validate_iat
jwt.exceptions.ImmatureSignatureError: The token is not yet valid (iat)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "runpy", line 196, in _run_module_as_main
  File "runpy", line 86, in _run_code
  File "nextstrain.cli.__main__", line 55, in <module>
  File "nextstrain.cli.__main__", line 19, in main
  File "nextstrain.cli", line 36, in run
  File "nextstrain.cli.command.login", line 103, in run
  File "nextstrain.cli.authn", line 69, in login
  File "nextstrain.cli.aws.cognito", line 167, in authenticate
  File "nextstrain.cli.aws.cognito", line 232, in verify_tokens
  File "nextstrain.cli.aws.cognito", line 261, in _verify_token
nextstrain.cli.aws.cognito.TokenError: ImmatureSignatureError: The token is not yet valid (iat)

Your environment: if running Nextstrain locally

Workaround

macOS: run sudo sntp -sS time.apple.com to sync my clock and that fixed things. My time was apparently 2 seconds off.

+2.158983 +/- 0.022257 time.apple.com 17.253.14.123

Possible solutions

  1. Add some more leeway for clock skew.
  2. Catch the error and raise another error with suggestions of how to fix.
  3. Catch the error and print a warning. Continue by trying again with Issued At verification disabled ("verify_iat": False).
  4. Disable the verification directly ("verify_iat": False) (This has been proposed as default behavior in pyjwt)

Links

corneliusroemer commented 1 year ago

Maybe relevant: https://github.com/jpadilla/pyjwt/issues/814

corneliusroemer commented 1 year ago

Maybe we could increase the leeway for clock skew? In my case it appears to have been "just" 2 seconds. Not sure whether the we can increase leeway though. https://pyjwt.readthedocs.io/en/latest/usage.html#not-before-time-claim-nbf

tsibley commented 1 year ago

It would certainly be friendlier in these cases to catch ImmatureSignatureError and issue a nice error message with suggestions of how to fix. And maybe adding some more leeway for clock skew.

victorlin commented 3 months ago

How catching the error with a warning and trying again with "verify_iat": False? Example:

$ nextstrain login
Logging into https://nextstrain.org…

...
WARNING: Your computer clock time is out of sync. Some things may not work as expected.

Logged into https://nextstrain.org as victorlin.

$ nextstrain remote download nextstrain.org/zika ~/tmp
WARNING: Your computer clock time is out of sync. Some things may not work as expected.
Downloading https://nextstrain.org/zika as /Users/vlin/tmp/zika.json
jameshadfield commented 1 month ago

Bumping this as it's happened twice now with LIMC collaborators. On a related slack thread there's a few votes to stop our CLI rejecting iat values from the future. @tsibley would you support a PR changing to "verify_iat": False, either outright or as Victor suggests?

tsibley commented 1 month ago

Yes, I think it'd be fine to set "verify_iat": False. Besides disabling the iat <= (now + leeway) check, it also disables the check that iat is an integer (via int(iat) not throwing an exception). I think that side-effect is fine as we don't rely on int(iat) anywhere in Nextstrain CLI AFAIK.