fix: do not query user_link for deleted accounts (#12112)

This commit is contained in:
Steven Masley 2024-02-13 13:02:21 -06:00 committed by GitHub
parent 06f3ab1206
commit 5d483a7ea1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 39 additions and 8 deletions

View File

@ -3787,6 +3787,10 @@ func (q *FakeQuerier) GetUserLinkByLinkedID(_ context.Context, id string) (datab
defer q.mutex.RUnlock()
for _, link := range q.userLinks {
user, err := q.getUserByIDNoLock(link.UserID)
if err == nil && user.Deleted {
continue
}
if link.LinkedID == id {
return link, nil
}

View File

@ -7088,11 +7088,15 @@ func (q *sqlQuerier) InsertTemplateVersionVariable(ctx context.Context, arg Inse
const getUserLinkByLinkedID = `-- name: GetUserLinkByLinkedID :one
SELECT
user_id, login_type, linked_id, oauth_access_token, oauth_refresh_token, oauth_expiry, oauth_access_token_key_id, oauth_refresh_token_key_id, debug_context
user_links.user_id, user_links.login_type, user_links.linked_id, user_links.oauth_access_token, user_links.oauth_refresh_token, user_links.oauth_expiry, user_links.oauth_access_token_key_id, user_links.oauth_refresh_token_key_id, user_links.debug_context
FROM
user_links
INNER JOIN
users ON user_links.user_id = users.id
WHERE
linked_id = $1
AND
deleted = false
`
func (q *sqlQuerier) GetUserLinkByLinkedID(ctx context.Context, linkedID string) (UserLink, error) {

View File

@ -1,10 +1,14 @@
-- name: GetUserLinkByLinkedID :one
SELECT
*
user_links.*
FROM
user_links
INNER JOIN
users ON user_links.user_id = users.id
WHERE
linked_id = $1;
linked_id = $1
AND
deleted = false;
-- name: GetUserLinkByUserIDLoginType :one
SELECT

View File

@ -603,6 +603,11 @@ func TestUserOAuth2Github(t *testing.T) {
require.Equal(t, http.StatusUnauthorized, resp.StatusCode)
})
// The bug only is exercised when a deleted user with the same linked_id exists.
// Still related open issues:
// - https://github.com/coder/coder/issues/12116
// - https://github.com/coder/coder/issues/12115
t.Run("ChangedEmail", func(t *testing.T) {
t.Parallel()
@ -627,7 +632,7 @@ func TestUserOAuth2Github(t *testing.T) {
coderEmail,
}
client := coderdtest.New(t, &coderdtest.Options{
owner := coderdtest.New(t, &coderdtest.Options{
Auditor: auditor,
GithubOAuth2Config: &coderd.GithubOAuth2Config{
AllowSignups: true,
@ -650,9 +655,21 @@ func TestUserOAuth2Github(t *testing.T) {
},
},
})
coderdtest.CreateFirstUser(t, owner)
ctx := testutil.Context(t, testutil.WaitMedium)
// This should register the user
ctx := testutil.Context(t, testutil.WaitLong)
// Create the user, then delete the user, then create again.
// This causes the email change to fail.
client := codersdk.New(owner.URL)
client, _ = fake.Login(t, client, jwt.MapClaims{})
deleted, err := client.User(ctx, "me")
require.NoError(t, err)
err = owner.DeleteUser(ctx, deleted.ID)
require.NoError(t, err)
// Create the user again.
client, _ = fake.Login(t, client, jwt.MapClaims{})
user, err := client.User(ctx, "me")
require.NoError(t, err)
@ -666,7 +683,8 @@ func TestUserOAuth2Github(t *testing.T) {
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.ID, userID, "user_id is different, a new user was likely created")
require.Equal(t, user.Email, *gmailEmail.Email)
// Entirely change emails.
@ -681,7 +699,8 @@ func TestUserOAuth2Github(t *testing.T) {
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.ID, userID, "user_id is different, a new user was likely created")
require.Equal(t, user.Email, newEmail)
})
}