Skip to content

Commit

Permalink
oauth/api: drain timer channel on each iteration
Browse files Browse the repository at this point in the history
Previously, if while polling for oauth device-code login results a user
suspended the process (such as with CTRL-Z) and then restored it with
`fg`, an error might occur in the form of:

```
failed waiting for authentication: You are polling faster than the specified interval of 5 seconds.
```

This is due to our use of a `time.Ticker` here - if no receiver drains
the ticker channel (and timers/tickers use a buffered channel behind the
scenes), more than one tick will pile up, causing the program to "tick"
twice, in fast succession, after it is resumed.

The new implementation replaces the `time.Ticker` with a `time.Timer`
(`time.Ticker` is just a nice wrapper) and introduces a helper function
`resetTimer` to ensure that before every `select`, the timer is stopped
and it's channel is drained.

Signed-off-by: Laura Brehm <[email protected]>
  • Loading branch information
laurazard committed Aug 29, 2024
1 parent 3826f5a commit a29bec2
Showing 1 changed file with 37 additions and 4 deletions.
41 changes: 37 additions & 4 deletions cli/internal/oauth/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,16 +96,33 @@ func tryDecodeOAuthError(resp *http.Response) error {
// authenticated or we have reached the time limit for authenticating (based on
// the response from GetDeviceCode).
func (a API) WaitForDeviceToken(ctx context.Context, state State) (TokenResponse, error) {
ticker := time.NewTicker(state.IntervalDuration())
// Ticker for polling tenant for login – based on the interval
// specified by the tenant response.
ticker := time.NewTimer(state.IntervalDuration())
defer ticker.Stop()
timeout := time.After(state.ExpiryDuration())

// Timeout context, for quitting polling for user credentials if
// we reach the timeout specified by the tenant.
// Does not get canceled if the user cancels execution.
timeoutCtx, cancel := context.WithTimeout(
context.Background(), state.ExpiryDuration())
defer cancel()

// Context for calling getDeviceToken – gets cancelled whether
// the caller cancels login or we hit the timeout.
getDeviceTokenCtx, getDeviceTokenCancel := context.WithTimeout(
ctx, state.ExpiryDuration())
defer getDeviceTokenCancel()

for {
resetTimer(ticker, state.IntervalDuration())
select {
case <-ctx.Done():
// user canceled login
return TokenResponse{}, ctx.Err()
case <-ticker.C:
res, err := a.getDeviceToken(ctx, state)
// tick, check for user login
res, err := a.getDeviceToken(getDeviceTokenCtx, state)
if err != nil {
return res, err
}
Expand All @@ -119,12 +136,28 @@ func (a API) WaitForDeviceToken(ctx context.Context, state State) (TokenResponse
}

return res, nil
case <-timeout:
case <-timeoutCtx.Done():
// login timed out
return TokenResponse{}, ErrTimeout
}
}
}

// resetTimer is a helper function thatstops, drains and resets the timer.
// This is necessary in go versions <1.23, since the timer isn't stopped +
// the timer's channel isn't drained on timer.Reset.
// See: https://go-review.googlesource.com/c/go/+/568341
// FIXME: remove/simplify this after we update to go1.23
func resetTimer(t *time.Timer, d time.Duration) {
if !t.Stop() {
select {
case <-t.C:
default:
}
}
t.Reset(d)
}

// getToken calls the token endpoint of Auth0 and returns the response.
func (a API) getDeviceToken(ctx context.Context, state State) (TokenResponse, error) {
data := url.Values{
Expand Down

0 comments on commit a29bec2

Please sign in to comment.