fix: include suspended and dormant users in acl query

code-asher · code-asher · commit 4035f9ac283b · 2024-08-27T14:42:56.000-08:00
This fixes two issues:

1. When you add a dormant/suspended user, you would not get any feedback
   in the UI because they were filtered out.
2. They were also filtered out when we created the audit log, so
   although users were added there is no record of it.
diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go
@@ -9884,7 +9884,7 @@ func (q *FakeQuerier) GetTemplateUserRoles(_ context.Context, id uuid.UUID) ([]d
 			continue
 		}
 
-		if user.Deleted || user.Status == database.UserStatusSuspended {
+		if user.Deleted {
 			continue
 		}
 
diff --git a/coderd/database/modelqueries.go b/coderd/database/modelqueries.go
@@ -165,9 +165,7 @@ func (q *sqlQuerier) GetTemplateUserRoles(ctx context.Context, id uuid.UUID) ([]
 	ON
 		users.id::text = perms.key
 	WHERE
-		users.deleted = false
-	AND
-		users.status = 'active';
+		users.deleted = false;
 	`
 
 	var tus []TemplateUser
diff --git a/enterprise/coderd/templates_test.go b/enterprise/coderd/templates_test.go
@@ -846,21 +846,52 @@ func TestTemplateACL(t *testing.T) {
 		}})
 		anotherClient, _ := coderdtest.CreateAnotherUser(t, client, user.OrganizationID, rbac.RoleTemplateAdmin())
 
+		// Create an active user.
 		_, user2 := coderdtest.CreateAnotherUser(t, client, user.OrganizationID)
-		_, user3 := coderdtest.CreateAnotherUser(t, client, user.OrganizationID)
-		version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
-		template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
 
+		// Create a dormant user.
 		ctx := testutil.Context(t, testutil.WaitLong)
+		user3, err := client.CreateUserWithOrgs(ctx, codersdk.CreateUserRequestWithOrgs{
+			Email:           "coder@coder.com",
+			Username:        "coder",
+			Password:        "SomeStrongPassword!",
+			OrganizationIDs: []uuid.UUID{user.OrganizationID},
+		})
+		require.NoError(t, err)
+		require.Equal(t, codersdk.UserStatusDormant, user3.Status)
 
-		err := anotherClient.UpdateTemplateACL(ctx, template.ID, codersdk.UpdateTemplateACL{
+		// Create a suspended user.
+		user4, err := client.CreateUserWithOrgs(ctx, codersdk.CreateUserRequestWithOrgs{
+			Email:           "coder2@coder.com",
+			Username:        "coder2",
+			Password:        "SomeStrongPassword!",
+			OrganizationIDs: []uuid.UUID{user.OrganizationID},
+		})
+		require.NoError(t, err)
+
+		user4, err = client.UpdateUserStatus(ctx, user4.ID.String(), codersdk.UserStatusSuspended)
+		require.NoError(t, err)
+		require.Equal(t, codersdk.UserStatusSuspended, user4.Status)
+
+		// Create a user to delete.
+		_, user5 := coderdtest.CreateAnotherUser(t, client, user.OrganizationID)
+
+		version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
+		template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
+
+		err = anotherClient.UpdateTemplateACL(ctx, template.ID, codersdk.UpdateTemplateACL{
 			UserPerms: map[string]codersdk.TemplateRole{
 				user2.ID.String(): codersdk.TemplateRoleUse,
 				user3.ID.String(): codersdk.TemplateRoleAdmin,
+				user4.ID.String(): codersdk.TemplateRoleUse,
+				user5.ID.String(): codersdk.TemplateRoleUse,
 			},
 		})
 		require.NoError(t, err)
 
+		err = client.DeleteUser(ctx, user5.ID)
+		require.NoError(t, err)
+
 		acl, err := anotherClient.TemplateACL(ctx, template.ID)
 		require.NoError(t, err)
 
@@ -874,9 +905,15 @@ func TestTemplateACL(t *testing.T) {
 			Role: codersdk.TemplateRoleAdmin,
 		}
 
-		require.Len(t, acl.Users, 2)
+		templateUser4 := codersdk.TemplateUser{
+			User: user4,
+			Role: codersdk.TemplateRoleUse,
+		}
+
+		require.Len(t, acl.Users, 3)
 		require.Contains(t, acl.Users, templateUser2)
 		require.Contains(t, acl.Users, templateUser3)
+		require.Contains(t, acl.Users, templateUser4)
 	})
 
 	t.Run("everyoneGroup", func(t *testing.T) {

Original file line number	Diff line number	Diff line change
`@@ -9884,7 +9884,7 @@ func (q *FakeQuerier) GetTemplateUserRoles(_ context.Context, id uuid.UUID) ([]d`
`9884`	`9884`	`continue`
`9885`	`9885`	`}`
`9886`	`9886`
`9887`		`- if user.Deleted \|\| user.Status == database.UserStatusSuspended {`
	`9887`	`+ if user.Deleted {`
`9888`	`9888`	`continue`
`9889`	`9889`	`}`
`9890`	`9890`