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

Emyrk · web-flow · commit 5d483a7ea161 · 2024-02-13T13:02:21.000-06:00
diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go
@@ -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
 		}
diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go
diff --git a/coderd/database/queries/user_links.sql b/coderd/database/queries/user_links.sql
@@ -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
diff --git a/coderd/userauth_test.go b/coderd/userauth_test.go
@@ -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.WaitLong)
+		// Create the user, then delete the user, then create again.
+		// This causes the email change to fail.
+		client := codersdk.New(owner.URL)
 
-		ctx := testutil.Context(t, testutil.WaitMedium)
-		// This should register the user
+		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)
 	})
 }
