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 Sep 2, 2024
1 parent 3826f5a commit 290cf9e
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 4 deletions.
36 changes: 33 additions & 3 deletions cli/internal/oauth/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,17 +96,31 @@ 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())
// The tenant tells us for as long as we can poll it for credentials,
// while the user logs in in the browser. Timeout if we don't get get
// credentials within this period.
timeout := time.NewTimer(state.ExpiryDuration())
defer timeout.Stop()

for {
resetTimer(ticker, state.IntervalDuration())
select {
case <-ctx.Done():
// user canceled login
return TokenResponse{}, ctx.Err()
case <-ticker.C:
// tick, check for user login
res, err := a.getDeviceToken(ctx, state)
if err != nil {
if errors.Is(err, context.Canceled) {
// if the caller canceled the context, continue
// and let the select hit the ctx.Done() branch
continue
}
return res, err
}

Expand All @@ -119,12 +133,28 @@ func (a API) WaitForDeviceToken(ctx context.Context, state State) (TokenResponse
}

return res, nil
case <-timeout:
case <-timeout.C:
// 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
2 changes: 1 addition & 1 deletion cli/internal/oauth/api/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ func TestWaitForDeviceToken(t *testing.T) {
state := State{
DeviceCode: "aDeviceCode",
UserCode: "aUserCode",
Interval: 1,
Interval: 5,
ExpiresIn: 1,
}

Expand Down

0 comments on commit 290cf9e

Please sign in to comment.