fix: Dbfake delete group member fixed

Emyrk · Emyrk · commit 8d3caa94c90c · 2023-03-09T23:04:15.000-06:00
diff --git a/coderd/database/dbfake/databasefake.go b/coderd/database/dbfake/databasefake.go
@@ -3905,13 +3905,22 @@ func (q *fakeQuerier) DeleteGroupMembersByOrgAndUser(_ context.Context, arg data
 
 	newMembers := q.groupMembers[:0]
 	for _, member := range q.groupMembers {
-		if member.UserID == arg.UserID {
+		if member.UserID != arg.UserID {
+			// Do not delete the other members
+			newMembers = append(newMembers, member)
+		} else if member.UserID == arg.UserID {
+			// We only want to delete from groups in the organization in the args.
 			for _, group := range q.groups {
-				if group.ID == member.GroupID && group.OrganizationID == arg.OrganizationID {
-					continue
+				// Find the group that the member is apartof.
+				if group.ID == member.GroupID {
+					// Only add back the member if the organization ID does not match
+					// the arg organization ID. Since the arg is saying which
+					// org to delete.
+					if group.OrganizationID != arg.OrganizationID {
+						newMembers = append(newMembers, member)
+					}
+					break
 				}
-
-				newMembers = append(newMembers, member)
 			}
 		}
 	}
diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go
diff --git a/coderd/database/queries/groupmembers.sql b/coderd/database/queries/groupmembers.sql
@@ -37,8 +37,8 @@ FROM
 DELETE FROM
 	group_members
 WHERE
-		group_members.user_id = @user_id
-  AND group_id = ANY(SELECT id FROM groups WHERE organization_id = @organization_id);
+	group_members.user_id = @user_id
+	AND group_id = ANY(SELECT id FROM groups WHERE organization_id = @organization_id);
 
 -- name: InsertGroupMember :exec
 INSERT INTO
diff --git a/enterprise/coderd/userauth_test.go b/enterprise/coderd/userauth_test.go
@@ -7,6 +7,8 @@ import (
 	"net/http"
 	"testing"
 
+	"github.com/google/uuid"
+
 	"github.com/golang-jwt/jwt"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -66,6 +68,72 @@ func TestUserOIDC(t *testing.T) {
 			require.NoError(t, err)
 			require.Len(t, group.Members, 1)
 		})
+		t.Run("AddThenRemove", func(t *testing.T) {
+			t.Parallel()
+
+			ctx, _ := testutil.Context(t)
+			conf := coderdtest.NewOIDCConfig(t, "")
+
+			config := conf.OIDCConfig(t, jwt.MapClaims{})
+			config.AllowSignups = true
+
+			client := coderdenttest.New(t, &coderdenttest.Options{
+				Options: &coderdtest.Options{
+					OIDCConfig: config,
+				},
+			})
+			firstUser := coderdtest.CreateFirstUser(t, client)
+			coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{
+				AllFeatures: true,
+			})
+
+			// Add some extra users/groups that should be asserted after.
+			// Adding this user as there was a bug that removing 1 user removed
+			// all users from the group.
+			_, extra := coderdtest.CreateAnotherUser(t, client, firstUser.OrganizationID)
+			groupName := "bingbong"
+			group, err := client.CreateGroup(ctx, firstUser.OrganizationID, codersdk.CreateGroupRequest{
+				Name: groupName,
+			})
+			require.NoError(t, err, "create group")
+
+			group, err = client.PatchGroup(ctx, group.ID, codersdk.PatchGroupRequest{
+				AddUsers: []string{
+					firstUser.UserID.String(),
+					extra.ID.String(),
+				},
+			})
+			require.NoError(t, err, "patch group")
+			require.Len(t, group.Members, 2, "expect both members")
+
+			// Now add OIDC user into the group
+			resp := oidcCallback(t, client, conf.EncodeClaims(t, jwt.MapClaims{
+				"email":  "colin@coder.com",
+				"groups": []string{groupName},
+			}))
+			assert.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode)
+
+			group, err = client.Group(ctx, group.ID)
+			require.NoError(t, err)
+			require.Len(t, group.Members, 3)
+
+			// Login to remove the OIDC user from the group
+			resp = oidcCallback(t, client, conf.EncodeClaims(t, jwt.MapClaims{
+				"email":  "colin@coder.com",
+				"groups": []string{},
+			}))
+			assert.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode)
+
+			group, err = client.Group(ctx, group.ID)
+			require.NoError(t, err)
+			require.Len(t, group.Members, 2)
+			var expected []uuid.UUID
+			for _, mem := range group.Members {
+				expected = append(expected, mem.ID)
+			}
+			require.ElementsMatchf(t, expected, []uuid.UUID{firstUser.UserID, extra.ID}, "expected members")
+		})
+
 		t.Run("NoneMatch", func(t *testing.T) {
 			t.Parallel()
 
