8000 improve error handling around device code failure · coder/coder@f3ac7d0 · GitHub
[go: up one dir, main page]
Skip to content

Commit f3ac7d0

Browse files
EmyrkEmyrk
committed
improve error handling around device code failure
1 parent 3ce4374 commit f3ac7d0

File tree

2 files changed

+51
-6
lines changed

2 files changed

+51
-6
lines changed

‎coderd/externalauth/externalauth.go

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@ import (
66
"encoding/json"
77
"fmt"
88
"io"
9+
"mime"
910
"net/http"
1011
"net/url"
1112
"regexp"
13+
"strconv"
1214
"strings"
1315
"time"
1416

@@ -321,13 +323,31 @@ func (c *DeviceAuth) AuthorizeDevice(ctx context.Context) (*codersdk.ExternalAut
321323
}
322324
err = json.NewDecoder(resp.Body).Decode(&r)
323325
if err != nil {
324-
// Some status codes do not return json payloads, and we should
325-
// return a better error.
326-
switch resp.StatusCode {
327-
case http.StatusTooManyRequests:
328-
return nil, xerrors.New("rate limit hit, unable to authorize device. please try again later")
326+
mediaType, _, err := mime.ParseMediaType(resp.Header.Get("Content-Type"))
327+
if err != nil {
328+
mediaType = "unknown"
329+
}
330+
331+
// If the json fails to decode, do a best effort to return a better error.
332+
switch {
333+
case resp.StatusCode == http.StatusTooManyRequests:
334+
retryIn := "please try again later"
335+
resetIn := resp.Header.Get("x-ratelimit-reset")
336+
if resetIn != "" {
337+
// Best effort to tell the user exactly how long they need
338+
// to wait for.
339+
unix, err := strconv.ParseInt(resetIn, 10, 64)
340+
if err == nil {
341+
waitFor := time.Unix(unix, 0).Sub(time.Now().Truncate(time.Second))
342+
retryIn = fmt.Sprintf(" retry after %s", waitFor.Truncate(time.Second))
343+
}
344+
}
345+
// 429 returns a plaintext payload with a message.
346+
return nil, xerrors.New(fmt.Sprintf("rate limit hit, unable to authorize device. %s", retryIn))
347+
case mediaType == "application/x-www-form-urlencoded":
348+
return nil, xerrors.Errorf("%s payload response is form-url encoded, expected a json payload", resp.StatusCode)
329349
default:
330-
return nil, fmt.Errorf("status_code=%d: %w", resp.StatusCode, err)
350+
return nil, fmt.Errorf("status_code=%d, mediaType=%s: %w", resp.StatusCode, mediaType, err)
331351
}
332352
}
333353
if r.ErrorDescription != "" {

‎coderd/externalauth_test.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"net/http"
77
"net/http/httptest"
8+
"net/url"
89
"regexp"
910
"strings"
1011
"testing"
@@ -363,6 +364,30 @@ func TestExternalAuthDevice(t *testing.T) {
363364
_, err := client.ExternalAuthDeviceByID(context.Background(), "test")
364365
require.ErrorContains(t, err, "rate limit hit")
365366
})
367+
368+
// If we forget to add the accept header, we get a form encoded body instead.
369+
t.Run("FormEncodedBody", func(t *testing.T) {
370+
t.Parallel()
371+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
372+
w.Header().Set("Content-Type", "application/x-www-form-urlencoded")
373+
_, _ = w.Write([]byte(url.Values{"access_token": {"hey"}}.Encode()))
374+
}))
375+
defer srv.Close()
376+
client := coderdtest.New(t, &coderdtest.Options{
377+
ExternalAuthConfigs: []*externalauth.Config{{
378+
ID: "test",
379+
DeviceAuth: &externalauth.DeviceAuth{
380+
ClientID: "test",
381+
CodeURL: srv.URL,
382+
Scopes: []string{"repo"},
383+
},
384+
}},
385+
})
386+
coderdtest.CreateFirstUser(t, client)
387+
_, err := client.ExternalAuthDeviceByID(context.Background(), "test")
388+
require.Error(t, err)
389+
require.ErrorContains(t, err, "is form-url encoded")
390+
})
366391
}
367392

368393
// nolint:bodyclose

0 commit comments

Comments
 (0)
0