Skip to content

Commit a29bec2

Browse files
committed
oauth/api: drain timer channel on each iteration
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 <laurabrehm@hey.com>
1 parent 3826f5a commit a29bec2

File tree

1 file changed

+37
-4
lines changed

1 file changed

+37
-4
lines changed

cli/internal/oauth/api/api.go

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,16 +96,33 @@ func tryDecodeOAuthError(resp *http.Response) error {
9696
// authenticated or we have reached the time limit for authenticating (based on
9797
// the response from GetDeviceCode).
9898
func (a API) WaitForDeviceToken(ctx context.Context, state State) (TokenResponse, error) {
99-
ticker := time.NewTicker(state.IntervalDuration())
99+
// Ticker for polling tenant for login – based on the interval
100+
// specified by the tenant response.
101+
ticker := time.NewTimer(state.IntervalDuration())
100102
defer ticker.Stop()
101-
timeout := time.After(state.ExpiryDuration())
103+
104+
// Timeout context, for quitting polling for user credentials if
105+
// we reach the timeout specified by the tenant.
106+
// Does not get canceled if the user cancels execution.
107+
timeoutCtx, cancel := context.WithTimeout(
108+
context.Background(), state.ExpiryDuration())
109+
defer cancel()
110+
111+
// Context for calling getDeviceToken – gets cancelled whether
112+
// the caller cancels login or we hit the timeout.
113+
getDeviceTokenCtx, getDeviceTokenCancel := context.WithTimeout(
114+
ctx, state.ExpiryDuration())
115+
defer getDeviceTokenCancel()
102116

103117
for {
118+
resetTimer(ticker, state.IntervalDuration())
104119
select {
105120
case <-ctx.Done():
121+
// user canceled login
106122
return TokenResponse{}, ctx.Err()
107123
case <-ticker.C:
108-
res, err := a.getDeviceToken(ctx, state)
124+
// tick, check for user login
125+
res, err := a.getDeviceToken(getDeviceTokenCtx, state)
109126
if err != nil {
110127
return res, err
111128
}
@@ -119,12 +136,28 @@ func (a API) WaitForDeviceToken(ctx context.Context, state State) (TokenResponse
119136
}
120137

121138
return res, nil
122-
case <-timeout:
139+
case <-timeoutCtx.Done():
140+
// login timed out
123141
return TokenResponse{}, ErrTimeout
124142
}
125143
}
126144
}
127145

146+
// resetTimer is a helper function thatstops, drains and resets the timer.
147+
// This is necessary in go versions <1.23, since the timer isn't stopped +
148+
// the timer's channel isn't drained on timer.Reset.
149+
// See: https://go-review.googlesource.com/c/go/+/568341
150+
// FIXME: remove/simplify this after we update to go1.23
151+
func resetTimer(t *time.Timer, d time.Duration) {
152+
if !t.Stop() {
153+
select {
154+
case <-t.C:
155+
default:
156+
}
157+
}
158+
t.Reset(d)
159+
}
160+
128161
// getToken calls the token endpoint of Auth0 and returns the response.
129162
func (a API) getDeviceToken(ctx context.Context, state State) (TokenResponse, error) {
130163
data := url.Values{

0 commit comments

Comments
 (0)