Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,18 +96,32 @@ 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 through their browser. Timeout if we don't 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 {
return res, err
if errors.Is(err, context.Canceled) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to handle context.DeadlineExceeded here? We could even use the errdefs helper, but not sure if we want to import it here;

// IsContext returns if the passed in error is due to context cancellation or deadline exceeded.
func IsContext(err error) bool {
return errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded)
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, the purpose of that:

// if the caller canceled the context, continue
// and let the select hit the ctx.Done() branch

is just to make sure we hit the user canceled login error if the user cancels execution while API.getDeviceToken is going rather than the context canceled error from there, so I don't think we need it. This is also all inside /internal, so we don't expect others to import this, so I think it's fine. If the user does get a timeout while trying to connect to the tenant, I think we should error out and let them know.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right; yeah, was curious if we wanted to retry if the connection timed out. I guess that would be a bit of an exceptional case, so returning early in that case makes sense as well.

Thanks!

// if the caller canceled the context, continue
// and let the select hit the ctx.Done() branch
continue
}
return TokenResponse{}, err
}

if res.Error != nil {
Expand All @@ -119,14 +133,33 @@ 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) {
ctx, cancel := context.WithTimeout(ctx, 1*time.Minute)
defer cancel()

data := url.Values{
"client_id": {a.ClientID},
"grant_type": {"urn:ietf:params:oauth:grant-type:device_code"},
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