selfhosted
/
coder
mirror of https://github.com/coder/coder.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

chore: ensure github uids are unique (#11826)

This commit is contained in:
Steven Masley 2024-01-29 09:13:46 -06:00 committed by GitHub
parent d66e6e78ee
commit 04a23261e6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 135 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
coderd/coderdtest/oidctest/idp.go
View File

@ -16,6 +16,7 @@ import (
"net/http"
"net/http/cookiejar"
"net/http/httptest"
"net/http/httputil"
"net/url"
"strconv"
"strings"
@ -67,6 +68,9 @@ type FakeIDP struct {
handler http.Handler
cfg *oauth2.Config
// callbackPath allows changing where the callback path to coderd is expected.
// This only affects using the Login helper functions.
callbackPath string
// clientID to be used by coderd
clientID string
clientSecret string
@ -161,6 +165,12 @@ func WithDefaultExpire(d time.Duration) func(*FakeIDP) {
}
}
func WithCallbackPath(path string) func(*FakeIDP) {
return func(f *FakeIDP) {
f.callbackPath = path
}
}
func WithStaticCredentials(id, secret string) func(*FakeIDP) {
return func(f *FakeIDP) {
if id != "" {
@ -369,6 +379,12 @@ func (f *FakeIDP) Login(t testing.TB, client *codersdk.Client, idTokenClaims jwt
t.Helper()
client, resp := f.AttemptLogin(t, client, idTokenClaims, opts...)
if resp.StatusCode != http.StatusOK {
data, err := httputil.DumpResponse(resp, true)
if err == nil {
t.Logf("Attempt Login response payload\n%s", string(data))
}
}
require.Equal(t, http.StatusOK, resp.StatusCode, "client failed to login")
return client, resp
}
@ -398,7 +414,11 @@ func (f *FakeIDP) AttemptLogin(t testing.TB, client *codersdk.Client, idTokenCla
func (f *FakeIDP) LoginWithClient(t testing.TB, client *codersdk.Client, idTokenClaims jwt.MapClaims, opts ...func(r *http.Request)) (*codersdk.Client, *http.Response) {
t.Helper()
coderOauthURL, err := client.URL.Parse("/api/v2/users/oidc/callback")
path := "/api/v2/users/oidc/callback"
if f.callbackPath != "" {
path = f.callbackPath
}
coderOauthURL, err := client.URL.Parse(path)
require.NoError(t, err)
f.SetRedirect(t, coderOauthURL.String())

19
coderd/userauth.go
View File

@ -604,6 +604,25 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
return
}
// If we have a nil GitHub ID, that is a big problem. That would mean we link
// this user and all other users with this bug to the same uuid.
// We should instead throw an error. This should never occur in production.
//
// Verified that the lowest ID on GitHub is "1", so 0 should never occur.
if ghUser.GetID() == 0 {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "The GitHub user ID is missing, this should never happen. Please report this error.",
// If this happens, the User could either be:
// - Empty, in which case all these fields would also be empty.
// - Not a user, in which case the "Type" would be something other than "User"
Detail: fmt.Sprintf("Other user fields: name=%q, email=%q, type=%q",
ghUser.GetName(),
ghUser.GetEmail(),
ghUser.GetType(),
),
})
return
}
user, link, err := findLinkedUser(ctx, api.Database, githubLinkedID(ghUser), verifiedEmail.GetEmail())
if err != nil {
logger.Error(ctx, "oauth2: unable to find linked user", slog.F("gh_user", ghUser.Name), slog.Error(err))

97
coderd/userauth_test.go
View File

@ -14,6 +14,7 @@ import (
"github.com/golang-jwt/jwt/v4"
"github.com/google/go-github/v43/github"
"github.com/google/uuid"
"github.com/prometheus/client_golang/prometheus"
"github.com/stretchr/testify/require"
"golang.org/x/xerrors"
@ -25,6 +26,7 @@ import (
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbgen"
"github.com/coder/coder/v2/coderd/database/dbtestutil"
"github.com/coder/coder/v2/coderd/promoauth"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/testutil"
)
@ -205,6 +207,7 @@ func TestUserOAuth2Github(t *testing.T) {
},
AuthenticatedUser: func(ctx context.Context, client *http.Client) (*github.User, error) {
return &github.User{
ID: github.Int64(100),
Login: github.String("kyle"),
}, nil
},
@ -264,7 +267,9 @@ func TestUserOAuth2Github(t *testing.T) {
}}, nil
},
AuthenticatedUser: func(ctx context.Context, client *http.Client) (*github.User, error) {
return &github.User{}, nil
return &github.User{
ID: github.Int64(100),
}, nil
},
ListEmails: func(ctx context.Context, client *http.Client) ([]*github.UserEmail, error) {
return []*github.UserEmail{{
@ -295,7 +300,9 @@ func TestUserOAuth2Github(t *testing.T) {
}}, nil
},
AuthenticatedUser: func(ctx context.Context, client *http.Client) (*github.User, error) {
return &github.User{}, nil
return &github.User{
ID: github.Int64(100),
}, nil
},
ListEmails: func(ctx context.Context, client *http.Client) ([]*github.UserEmail, error) {
return []*github.UserEmail{{
@ -390,6 +397,7 @@ func TestUserOAuth2Github(t *testing.T) {
},
AuthenticatedUser: func(ctx context.Context, client *http.Client) (*github.User, error) {
return &github.User{
ID: github.Int64(100),
Login: github.String("kyle"),
}, nil
},
@ -442,6 +450,7 @@ func TestUserOAuth2Github(t *testing.T) {
},
AuthenticatedUser: func(ctx context.Context, client *http.Client) (*github.User, error) {
return &github.User{
ID: github.Int64(100),
Login: github.String("mathias"),
}, nil
},
@ -494,6 +503,7 @@ func TestUserOAuth2Github(t *testing.T) {
},
AuthenticatedUser: func(ctx context.Context, client *http.Client) (*github.User, error) {
return &github.User{
ID: github.Int64(100),
Login: github.String("mathias"),
}, nil
},
@ -532,6 +542,7 @@ func TestUserOAuth2Github(t *testing.T) {
},
AuthenticatedUser: func(ctx context.Context, client *http.Client) (*github.User, error) {
return &github.User{
ID: github.Int64(100),
Login: github.String("mathias"),
}, nil
},
@ -574,6 +585,7 @@ func TestUserOAuth2Github(t *testing.T) {
},
AuthenticatedUser: func(ctx context.Context, client *http.Client) (*github.User, error) {
return &github.User{
ID: github.Int64(100),
Login: github.String("kyle"),
}, nil
},
@ -591,6 +603,87 @@ func TestUserOAuth2Github(t *testing.T) {
require.Equal(t, http.StatusUnauthorized, resp.StatusCode)
})
t.Run("ChangedEmail", func(t *testing.T) {
t.Parallel()
fake := oidctest.NewFakeIDP(t,
oidctest.WithServing(),
oidctest.WithCallbackPath("/api/v2/users/oauth2/github/callback"),
)
const ghID = int64(7777)
auditor := audit.NewMock()
coderEmail := &github.UserEmail{
Email: github.String("alice@coder.com"),
Verified: github.Bool(true),
Primary: github.Bool(true),
}
gmailEmail := &github.UserEmail{
Email: github.String("alice@gmail.com"),
Verified: github.Bool(true),
Primary: github.Bool(false),
}
emails := []*github.UserEmail{
gmailEmail,
coderEmail,
}
client := coderdtest.New(t, &coderdtest.Options{
Auditor: auditor,
GithubOAuth2Config: &coderd.GithubOAuth2Config{
AllowSignups: true,
AllowEveryone: true,
OAuth2Config: promoauth.NewFactory(prometheus.NewRegistry()).NewGithub("test-github", fake.OIDCConfig(t, []string{})),
ListOrganizationMemberships: func(ctx context.Context, client *http.Client) ([]*github.Membership, error) {
return []*github.Membership{}, nil
},
TeamMembership: func(ctx context.Context, client *http.Client, org, team, username string) (*github.Membership, error) {
return nil, xerrors.New("no teams")
},
AuthenticatedUser: func(ctx context.Context, client *http.Client) (*github.User, error) {
return &github.User{
Login: github.String("alice"),
ID: github.Int64(ghID),
}, nil
},
ListEmails: func(ctx context.Context, client *http.Client) ([]*github.UserEmail, error) {
return emails, nil
},
},
})
ctx := testutil.Context(t, testutil.WaitMedium)
// This should register the user
client, _ = fake.Login(t, client, jwt.MapClaims{})
user, err := client.User(ctx, "me")
require.NoError(t, err)
userID := user.ID
require.Equal(t, user.Email, *coderEmail.Email)
// Now the user is registered, let's change their primary email.
coderEmail.Primary = github.Bool(false)
gmailEmail.Primary = github.Bool(true)
client, _ = fake.Login(t, client, jwt.MapClaims{})
user, err = client.User(ctx, "me")
require.NoError(t, err)
require.Equal(t, user.ID, userID)
require.Equal(t, user.Email, *gmailEmail.Email)
// Entirely change emails.
newEmail := "alice@newdomain.com"
emails = []*github.UserEmail{
{
Email: github.String("alice@newdomain.com"),
Primary: github.Bool(true),
Verified: github.Bool(true),
},
}
client, _ = fake.Login(t, client, jwt.MapClaims{})
user, err = client.User(ctx, "me")
require.NoError(t, err)
require.Equal(t, user.ID, userID)
require.Equal(t, user.Email, newEmail)
})
}
// nolint:bodyclose