If ACTIVESTATE_API_KEY is set, try to authenticate with it, or explicitly error out.
#3520
Conversation
mitchell-as
commented
Oct 2, 2024
•
mitchell-as
commented
 DX-3080 We error out if the stored API key is invalid
|
…icitly error out.
| if apiKey := os.Getenv(constants.APIKeyEnvVarName); apiKey != "" { | ||
| err := auth.AuthenticateWithToken(apiKey, a.Auth) | ||
| if err != nil { | ||
| return locale.WrapError(err, "err_auth_api_key", "Failed to authenticate with [ACTIONABLE]{{.V0}}[/RESET] environment variable", constants.APIKeyEnvVarName) | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
I wonder if this error reporting should be in the lower level auth package. The user would only get this error message if they explicitly ran state auth but we are attempting to authenticate with the api key environment variable through multiple code paths in the auth package. See here for one.
There was a problem hiding this comment.
That's what I did originally, but I couldn't trigger it without state auth. We have lots of places that check auth.Authenticated() and bail out with "you must be logged in" if not. I was only able to show my message using state auth. The only callers of your linked method are "Sync" (not applicable to this PR), and "Client". All instances of the latter appear to be walled behind auth.Authenticated() checks, so in practice, I think we're okay.
Given that the confusion arises around the whole state auth flow, I figured this was an okay place to put it.
There was a problem hiding this comment.
This is addressing ONLY the state auth case. The issue here is when we pass the API key through an env var, which is the more common use-case. This scenario would not give any sort of error.
Also re the slack thread with Damien raising the 60 second polling of auth which seemed to come from this PR; we should ensure that we do not retry when the API key is known to be invalid.
I'll update the story to clarify these points.
|
This jumped up in complexity, so closing. |
