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

Commit04a2326

Browse files
authored
chore: ensure github uids are unique (#11826)
1 parentd66e6e7 commit04a2326

File tree

3 files changed

+135
-3
lines changed

3 files changed

+135
-3
lines changed

‎coderd/coderdtest/oidctest/idp.go

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"net/http"
1717
"net/http/cookiejar"
1818
"net/http/httptest"
19+
"net/http/httputil"
1920
"net/url"
2021
"strconv"
2122
"strings"
@@ -67,6 +68,9 @@ type FakeIDP struct {
6768
handler http.Handler
6869
cfg*oauth2.Config
6970

71+
// callbackPath allows changing where the callback path to coderd is expected.
72+
// This only affects using the Login helper functions.
73+
callbackPathstring
7074
// clientID to be used by coderd
7175
clientIDstring
7276
clientSecretstring
@@ -161,6 +165,12 @@ func WithDefaultExpire(d time.Duration) func(*FakeIDP) {
161165
}
162166
}
163167

168+
funcWithCallbackPath(pathstring)func(*FakeIDP) {
169+
returnfunc(f*FakeIDP) {
170+
f.callbackPath=path
171+
}
172+
}
173+
164174
funcWithStaticCredentials(id,secretstring)func(*FakeIDP) {
165175
returnfunc(f*FakeIDP) {
166176
ifid!="" {
@@ -369,6 +379,12 @@ func (f *FakeIDP) Login(t testing.TB, client *codersdk.Client, idTokenClaims jwt
369379
t.Helper()
370380

371381
client,resp:=f.AttemptLogin(t,client,idTokenClaims,opts...)
382+
ifresp.StatusCode!=http.StatusOK {
383+
data,err:=httputil.DumpResponse(resp,true)
384+
iferr==nil {
385+
t.Logf("Attempt Login response payload\n%s",string(data))
386+
}
387+
}
372388
require.Equal(t,http.StatusOK,resp.StatusCode,"client failed to login")
373389
returnclient,resp
374390
}
@@ -398,7 +414,11 @@ func (f *FakeIDP) AttemptLogin(t testing.TB, client *codersdk.Client, idTokenCla
398414
func (f*FakeIDP)LoginWithClient(t testing.TB,client*codersdk.Client,idTokenClaims jwt.MapClaims,opts...func(r*http.Request)) (*codersdk.Client,*http.Response) {
399415
t.Helper()
400416

401-
coderOauthURL,err:=client.URL.Parse("/api/v2/users/oidc/callback")
417+
path:="/api/v2/users/oidc/callback"
418+
iff.callbackPath!="" {
419+
path=f.callbackPath
420+
}
421+
coderOauthURL,err:=client.URL.Parse(path)
402422
require.NoError(t,err)
403423
f.SetRedirect(t,coderOauthURL.String())
404424

‎coderd/userauth.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -604,6 +604,25 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
604604
return
605605
}
606606

607+
// If we have a nil GitHub ID, that is a big problem. That would mean we link
608+
// this user and all other users with this bug to the same uuid.
609+
// We should instead throw an error. This should never occur in production.
610+
//
611+
// Verified that the lowest ID on GitHub is "1", so 0 should never occur.
612+
ifghUser.GetID()==0 {
613+
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
614+
Message:"The GitHub user ID is missing, this should never happen. Please report this error.",
615+
// If this happens, the User could either be:
616+
// - Empty, in which case all these fields would also be empty.
617+
// - Not a user, in which case the "Type" would be something other than "User"
618+
Detail:fmt.Sprintf("Other user fields: name=%q, email=%q, type=%q",
619+
ghUser.GetName(),
620+
ghUser.GetEmail(),
621+
ghUser.GetType(),
622+
),
623+
})
624+
return
625+
}
607626
user,link,err:=findLinkedUser(ctx,api.Database,githubLinkedID(ghUser),verifiedEmail.GetEmail())
608627
iferr!=nil {
609628
logger.Error(ctx,"oauth2: unable to find linked user",slog.F("gh_user",ghUser.Name),slog.Error(err))

‎coderd/userauth_test.go

Lines changed: 95 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/golang-jwt/jwt/v4"
1515
"github.com/google/go-github/v43/github"
1616
"github.com/google/uuid"
17+
"github.com/prometheus/client_golang/prometheus"
1718
"github.com/stretchr/testify/require"
1819
"golang.org/x/xerrors"
1920

@@ -25,6 +26,7 @@ import (
2526
"github.com/coder/coder/v2/coderd/database"
2627
"github.com/coder/coder/v2/coderd/database/dbgen"
2728
"github.com/coder/coder/v2/coderd/database/dbtestutil"
29+
"github.com/coder/coder/v2/coderd/promoauth"
2830
"github.com/coder/coder/v2/codersdk"
2931
"github.com/coder/coder/v2/testutil"
3032
)
@@ -205,6 +207,7 @@ func TestUserOAuth2Github(t *testing.T) {
205207
},
206208
AuthenticatedUser:func(ctx context.Context,client*http.Client) (*github.User,error) {
207209
return&github.User{
210+
ID:github.Int64(100),
208211
Login:github.String("kyle"),
209212
},nil
210213
},
@@ -264,7 +267,9 @@ func TestUserOAuth2Github(t *testing.T) {
264267
}},nil
265268
},
266269
AuthenticatedUser:func(ctx context.Context,client*http.Client) (*github.User,error) {
267-
return&github.User{},nil
270+
return&github.User{
271+
ID:github.Int64(100),
272+
},nil
268273
},
269274
ListEmails:func(ctx context.Context,client*http.Client) ([]*github.UserEmail,error) {
270275
return []*github.UserEmail{{
@@ -295,7 +300,9 @@ func TestUserOAuth2Github(t *testing.T) {
295300
}},nil
296301
},
297302
AuthenticatedUser:func(ctx context.Context,client*http.Client) (*github.User,error) {
298-
return&github.User{},nil
303+
return&github.User{
304+
ID:github.Int64(100),
305+
},nil
299306
},
300307
ListEmails:func(ctx context.Context,client*http.Client) ([]*github.UserEmail,error) {
301308
return []*github.UserEmail{{
@@ -390,6 +397,7 @@ func TestUserOAuth2Github(t *testing.T) {
390397
},
391398
AuthenticatedUser:func(ctx context.Context,client*http.Client) (*github.User,error) {
392399
return&github.User{
400+
ID:github.Int64(100),
393401
Login:github.String("kyle"),
394402
},nil
395403
},
@@ -442,6 +450,7 @@ func TestUserOAuth2Github(t *testing.T) {
442450
},
443451
AuthenticatedUser:func(ctx context.Context,client*http.Client) (*github.User,error) {
444452
return&github.User{
453+
ID:github.Int64(100),
445454
Login:github.String("mathias"),
446455
},nil
447456
},
@@ -494,6 +503,7 @@ func TestUserOAuth2Github(t *testing.T) {
494503
},
495504
AuthenticatedUser:func(ctx context.Context,client*http.Client) (*github.User,error) {
496505
return&github.User{
506+
ID:github.Int64(100),
497507
Login:github.String("mathias"),
498508
},nil
499509
},
@@ -532,6 +542,7 @@ func TestUserOAuth2Github(t *testing.T) {
532542
},
533543
AuthenticatedUser:func(ctx context.Context,client*http.Client) (*github.User,error) {
534544
return&github.User{
545+
ID:github.Int64(100),
535546
Login:github.String("mathias"),
536547
},nil
537548
},
@@ -574,6 +585,7 @@ func TestUserOAuth2Github(t *testing.T) {
574585
},
575586
AuthenticatedUser:func(ctx context.Context,client*http.Client) (*github.User,error) {
576587
return&github.User{
588+
ID:github.Int64(100),
577589
Login:github.String("kyle"),
578590
},nil
579591
},
@@ -591,6 +603,87 @@ func TestUserOAuth2Github(t *testing.T) {
591603

592604
require.Equal(t,http.StatusUnauthorized,resp.StatusCode)
593605
})
606+
t.Run("ChangedEmail",func(t*testing.T) {
607+
t.Parallel()
608+
609+
fake:=oidctest.NewFakeIDP(t,
610+
oidctest.WithServing(),
611+
oidctest.WithCallbackPath("/api/v2/users/oauth2/github/callback"),
612+
)
613+
constghID=int64(7777)
614+
auditor:=audit.NewMock()
615+
coderEmail:=&github.UserEmail{
616+
Email:github.String("alice@coder.com"),
617+
Verified:github.Bool(true),
618+
Primary:github.Bool(true),
619+
}
620+
gmailEmail:=&github.UserEmail{
621+
Email:github.String("alice@gmail.com"),
622+
Verified:github.Bool(true),
623+
Primary:github.Bool(false),
624+
}
625+
emails:= []*github.UserEmail{
626+
gmailEmail,
627+
coderEmail,
628+
}
629+
630+
client:=coderdtest.New(t,&coderdtest.Options{
631+
Auditor:auditor,
632+
GithubOAuth2Config:&coderd.GithubOAuth2Config{
633+
AllowSignups:true,
634+
AllowEveryone:true,
635+
OAuth2Config:promoauth.NewFactory(prometheus.NewRegistry()).NewGithub("test-github",fake.OIDCConfig(t, []string{})),
636+
ListOrganizationMemberships:func(ctx context.Context,client*http.Client) ([]*github.Membership,error) {
637+
return []*github.Membership{},nil
638+
},
639+
TeamMembership:func(ctx context.Context,client*http.Client,org,team,usernamestring) (*github.Membership,error) {
640+
returnnil,xerrors.New("no teams")
641+
},
642+
AuthenticatedUser:func(ctx context.Context,client*http.Client) (*github.User,error) {
643+
return&github.User{
644+
Login:github.String("alice"),
645+
ID:github.Int64(ghID),
646+
},nil
647+
},
648+
ListEmails:func(ctx context.Context,client*http.Client) ([]*github.UserEmail,error) {
649+
returnemails,nil
650+
},
651+
},
652+
})
653+
654+
ctx:=testutil.Context(t,testutil.WaitMedium)
655+
// This should register the user
656+
client,_=fake.Login(t,client, jwt.MapClaims{})
657+
user,err:=client.User(ctx,"me")
658+
require.NoError(t,err)
659+
userID:=user.ID
660+
require.Equal(t,user.Email,*coderEmail.Email)
661+
662+
// Now the user is registered, let's change their primary email.
663+
coderEmail.Primary=github.Bool(false)
664+
gmailEmail.Primary=github.Bool(true)
665+
666+
client,_=fake.Login(t,client, jwt.MapClaims{})
667+
user,err=client.User(ctx,"me")
668+
require.NoError(t,err)
669+
require.Equal(t,user.ID,userID)
670+
require.Equal(t,user.Email,*gmailEmail.Email)
671+
672+
// Entirely change emails.
673+
newEmail:="alice@newdomain.com"
674+
emails= []*github.UserEmail{
675+
{
676+
Email:github.String("alice@newdomain.com"),
677+
Primary:github.Bool(true),
678+
Verified:github.Bool(true),
679+
},
680+
}
681+
client,_=fake.Login(t,client, jwt.MapClaims{})
682+
user,err=client.User(ctx,"me")
683+
require.NoError(t,err)
684+
require.Equal(t,user.ID,userID)
685+
require.Equal(t,user.Email,newEmail)
686+
})
594687
}
595688

596689
// nolint:bodyclose

0 commit comments

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