Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit9bc29db

Browse files
committed
chore: improve error handling for device code exchange failure (#11708)
* implement test using fake idp device flow* improve error handling around device code failure
1 parentaaca806 commit9bc29db

File tree

3 files changed

+98
-6
lines changed

3 files changed

+98
-6
lines changed

‎coderd/coderdtest/oidctest/idp.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import (
4141
"github.com/coder/coder/v2/coderd/promoauth"
4242
"github.com/coder/coder/v2/coderd/util/syncmap"
4343
"github.com/coder/coder/v2/codersdk"
44+
"github.com/coder/coder/v2/testutil"
4445
)
4546

4647
typetokenstruct {
@@ -484,6 +485,31 @@ func (f *FakeIDP) ExternalLogin(t testing.TB, client *codersdk.Client, opts ...f
484485
_=res.Body.Close()
485486
}
486487

488+
// DeviceLogin does the oauth2 device flow for external auth providers.
489+
func (*FakeIDP)DeviceLogin(t testing.TB,client*codersdk.Client,externalAuthIDstring) {
490+
// First we need to initiate the device flow. This will have Coder hit the
491+
// fake IDP and get a device code.
492+
device,err:=client.ExternalAuthDeviceByID(context.Background(),externalAuthID)
493+
require.NoError(t,err)
494+
495+
// Now the user needs to go to the fake IDP page and click "allow" and enter
496+
// the device code input. For our purposes, we just send an http request to
497+
// the verification url. No additional user input is needed.
498+
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitShort)
499+
defercancel()
500+
resp,err:=client.Request(ctx,http.MethodPost,device.VerificationURI,nil)
501+
require.NoError(t,err)
502+
deferresp.Body.Close()
503+
504+
// Now we need to exchange the device code for an access token. We do this
505+
// in this method because it is the user that does the polling for the device
506+
// auth flow, not the backend.
507+
err=client.ExternalAuthDeviceExchange(context.Background(),externalAuthID, codersdk.ExternalAuthDeviceExchange{
508+
DeviceCode:device.DeviceCode,
509+
})
510+
require.NoError(t,err)
511+
}
512+
487513
// CreateAuthCode emulates a user clicking "allow" on the IDP page. When doing
488514
// unit tests, it's easier to skip this step sometimes. It does make an actual
489515
// request to the IDP, so it should be equivalent to doing this "manually" with

‎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
iferr!=nil {
324-
// Some status codes do not return json payloads, and we should
325-
// return a better error.
326-
switchresp.StatusCode {
327-
casehttp.StatusTooManyRequests:
328-
returnnil,xerrors.New("rate limit hit, unable to authorize device. please try again later")
326+
mediaType,_,err:=mime.ParseMediaType(resp.Header.Get("Content-Type"))
327+
iferr!=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+
caseresp.StatusCode==http.StatusTooManyRequests:
334+
retryIn:="please try again later"
335+
resetIn:=resp.Header.Get("x-ratelimit-reset")
336+
ifresetIn!="" {
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+
iferr==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+
returnnil,xerrors.New(fmt.Sprintf("rate limit hit, unable to authorize device. %s",retryIn))
347+
casemediaType=="application/x-www-form-urlencoded":
348+
returnnil,xerrors.Errorf("status_code=%d, payload response is form-url encoded, expected a json payload",resp.StatusCode)
329349
default:
330-
returnnil,fmt.Errorf("status_code=%d: %w",resp.StatusCode,err)
350+
returnnil,fmt.Errorf("status_code=%d, mediaType=%s: %w",resp.StatusCode,mediaType,err)
331351
}
332352
}
333353
ifr.ErrorDescription!="" {

‎coderd/externalauth_test.go

Lines changed: 46 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"
@@ -264,6 +265,27 @@ func TestExternalAuthManagement(t *testing.T) {
264265

265266
funcTestExternalAuthDevice(t*testing.T) {
266267
t.Parallel()
268+
// This is an example test on how to do device auth flow using our fake idp.
269+
t.Run("WithFakeIDP",func(t*testing.T) {
270+
t.Parallel()
271+
fake:=oidctest.NewFakeIDP(t,oidctest.WithServing())
272+
externalID:="fake-idp"
273+
cfg:=fake.ExternalAuthConfig(t,externalID,&oidctest.ExternalAuthConfigOptions{
274+
UseDeviceAuth:true,
275+
})
276+
277+
client:=coderdtest.New(t,&coderdtest.Options{
278+
ExternalAuthConfigs: []*externalauth.Config{cfg},
279+
})
280+
coderdtest.CreateFirstUser(t,client)
281+
// Login!
282+
fake.DeviceLogin(t,client,externalID)
283+
284+
extAuth,err:=client.ExternalAuthByID(context.Background(),externalID)
285+
require.NoError(t,err)
286+
require.True(t,extAuth.Authenticated)
287+
})
288+
267289
t.Run("NotSupported",func(t*testing.T) {
268290
t.Parallel()
269291
client:=coderdtest.New(t,&coderdtest.Options{
@@ -363,6 +385,30 @@ func TestExternalAuthDevice(t *testing.T) {
363385
_,err:=client.ExternalAuthDeviceByID(context.Background(),"test")
364386
require.ErrorContains(t,err,"rate limit hit")
365387
})
388+
389+
// If we forget to add the accept header, we get a form encoded body instead.
390+
t.Run("FormEncodedBody",func(t*testing.T) {
391+
t.Parallel()
392+
srv:=httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter,r*http.Request) {
393+
w.Header().Set("Content-Type","application/x-www-form-urlencoded")
394+
_,_=w.Write([]byte(url.Values{"access_token": {"hey"}}.Encode()))
395+
}))
396+
defersrv.Close()
397+
client:=coderdtest.New(t,&coderdtest.Options{
398+
ExternalAuthConfigs: []*externalauth.Config{{
399+
ID:"test",
400+
DeviceAuth:&externalauth.DeviceAuth{
401+
ClientID:"test",
402+
CodeURL:srv.URL,
403+
Scopes: []string{"repo"},
404+
},
405+
}},
406+
})
407+
coderdtest.CreateFirstUser(t,client)
408+
_,err:=client.ExternalAuthDeviceByID(context.Background(),"test")
409+
require.Error(t,err)
410+
require.ErrorContains(t,err,"is form-url encoded")
411+
})
366412
}
367413

368414
// nolint:bodyclose

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp