Skip to content

Commit 60d0450

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 60d0450

2 files changed

Lines changed: 38 additions & 5 deletions

File tree

cli/internal/oauth/api/api.go

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,18 +96,32 @@ 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+
// The tenant tells us for as long as we can poll it for credentials
104+
// while the user logs in through their browser. Timeout if we don't get
105+
// credentials within this period.
106+
timeout := time.NewTimer(state.ExpiryDuration())
107+
defer timeout.Stop()
102108

103109
for {
110+
resetTimer(ticker, state.IntervalDuration())
104111
select {
105112
case <-ctx.Done():
113+
// user canceled login
106114
return TokenResponse{}, ctx.Err()
107115
case <-ticker.C:
116+
// tick, check for user login
108117
res, err := a.getDeviceToken(ctx, state)
109118
if err != nil {
110-
return res, err
119+
if errors.Is(err, context.Canceled) {
120+
// if the caller canceled the context, continue
121+
// and let the select hit the ctx.Done() branch
122+
continue
123+
}
124+
return TokenResponse{}, err
111125
}
112126

113127
if res.Error != nil {
@@ -119,14 +133,33 @@ func (a API) WaitForDeviceToken(ctx context.Context, state State) (TokenResponse
119133
}
120134

121135
return res, nil
122-
case <-timeout:
136+
case <-timeout.C:
137+
// login timed out
123138
return TokenResponse{}, ErrTimeout
124139
}
125140
}
126141
}
127142

143+
// resetTimer is a helper function thatstops, drains and resets the timer.
144+
// This is necessary in go versions <1.23, since the timer isn't stopped +
145+
// the timer's channel isn't drained on timer.Reset.
146+
// See: https://go-review.googlesource.com/c/go/+/568341
147+
// FIXME: remove/simplify this after we update to go1.23
148+
func resetTimer(t *time.Timer, d time.Duration) {
149+
if !t.Stop() {
150+
select {
151+
case <-t.C:
152+
default:
153+
}
154+
}
155+
t.Reset(d)
156+
}
157+
128158
// getToken calls the token endpoint of Auth0 and returns the response.
129159
func (a API) getDeviceToken(ctx context.Context, state State) (TokenResponse, error) {
160+
ctx, cancel := context.WithTimeout(ctx, 1*time.Minute)
161+
defer cancel()
162+
130163
data := url.Values{
131164
"client_id": {a.ClientID},
132165
"grant_type": {"urn:ietf:params:oauth:grant-type:device_code"},

cli/internal/oauth/api/api_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ func TestWaitForDeviceToken(t *testing.T) {
198198
state := State{
199199
DeviceCode: "aDeviceCode",
200200
UserCode: "aUserCode",
201-
Interval: 1,
201+
Interval: 5,
202202
ExpiresIn: 1,
203203
}
204204

0 commit comments

Comments
 (0)