Skip incomplete users in listUsersWithSecrets (#38655)

lib/services/local/users.go

@@ -93,78 +93,60 @@ func (s *IdentityService) listUsersWithSecrets(ctx context.Context, pageSize int
 	rangeStream := backend.StreamRange(ctx, s.Backend, rangeStart, rangeEnd, limit)
 
 	var (
-		out      []types.User
-		prevUser string
-		items    userItems
+		out []types.User
+
+		name  string
+		items userItems
 	)
 	for rangeStream.Next() {
 		item := rangeStream.Item()
 
-		name, suffix, err := splitUsernameAndSuffix(string(item.Key))
+		itemName, suffix, err := splitUsernameAndSuffix(string(item.Key))
 		if err != nil {
 			return nil, "", trace.Wrap(err)
 		}
 
-		if prevUser == "" {
-			prevUser = name
+		if itemName == name {
+			items.Set(suffix, item)
+			continue
 		}
 
-		if name != prevUser {
-			user, err := userFromUserItems(prevUser, items)
+		// we exclude user item sets that don't have a /params subitem because
+		// SSO users have an expiration time on /params but their /mfa devices
+		// are persistent
+		if items.complete() {
+			user, err := userFromUserItems(name, items)
 			if err != nil {
 				return nil, "", trace.Wrap(err)
 			}
-
 			out = append(out, user)
 
-			prevUser = name
-			items = userItems{}
+			if len(out) >= pageSize {
+				if err := rangeStream.Done(); err != nil {
+					return nil, "", trace.Wrap(err)
+				}
 
-			if len(out) == pageSize {
-				break
+				return out, nextUserToken(user), nil
 			}
 		}
 
+		name = itemName
+		items = userItems{}
 		items.Set(suffix, item)
 	}
-
-	next := rangeStream.Next()
-
-	if len(out) == 0 {
-		// When there is only a single user the transition logic to detect when a user
-		// is complete will never be hit. If the stream is exhausted and the items have
-		// processed a user resource then return it.
-		if !next && items.complete() {
-			user, err := userFromUserItems(prevUser, items)
-			if err != nil {
-				return nil, "", trace.Wrap(err)
-			}
-
-			out = append(out, user)
-		}
-	} else {
-		// If there are no more users it is possible that the previous user being processed
-		// was never added to out because there was no additional user to transition to.
-		// Add the user from the collected items and return the full output.
-		if len(out) != pageSize && out[len(out)-1].GetName() != prevUser {
-			user, err := userFromUserItems(prevUser, items)
-			if err != nil {
-				return nil, "", trace.Wrap(err)
-			}
-
-			out = append(out, user)
-		}
+	if err := rangeStream.Done(); err != nil {
+		return nil, "", trace.Wrap(err)
 	}
 
-	var nextToken string
-	// If the stream has more data or while processing the stream the last user
-	// caused a transition but was not added to out we want to send a token so
-	// that the final user may be returned in the next page.
-	if next || len(out) > 0 && out[len(out)-1].GetName() != prevUser {
-		nextToken = nextUserToken(out[len(out)-1])
+	if items.complete() {
+		user, err := userFromUserItems(name, items)
+		if err != nil {
+			return nil, "", trace.Wrap(err)
+		}
+		out = append(out, user)
 	}
 
-	return out, nextToken, trace.Wrap(rangeStream.Done())
+	return out, "", nil
 }
 
 // nextUserToken returns the last token for the given user. This

lib/services/local/users_test.go

@@ -53,7 +53,7 @@ func newIdentityService(t *testing.T, clock clockwork.Clock) *local.IdentityServ
 	t.Helper()
 	backend, err := memory.New(memory.Config{
 		Context: context.Background(),
-		Clock:   clockwork.NewFakeClock(),
+		Clock:   clock,
 	})
 	require.NoError(t, err)
 	return local.NewIdentityService(backend)
@@ -1135,6 +1135,23 @@ func TestIdentityService_ListUsers(t *testing.T) {
 		return strings.Compare(a.GetName(), b.GetName())
 	})
 	require.Empty(t, cmp.Diff(expectedUsers, retrieved, cmpopts.SortSlices(devicesSort)), "not all users returned from listing operation")
+
+	ssoUser := expectedUsers[2]
+	expectedUsers = slices.Delete(expectedUsers, 2, 3)
+	ssoUser.SetExpiry(clock.Now().UTC().Add(time.Minute))
+
+	_, err = identity.UpsertUser(ctx, ssoUser)
+	assert.NoError(t, err, "failed to upsert SSO user")
+
+	clock.Advance(time.Hour)
+
+	retrieved, next, err = identity.ListUsers(ctx, 0, "", true)
+	assert.NoError(t, err, "got an error while listing over an expired user")
+	assert.Empty(t, next, "next page token returned from listing all users")
+	slices.SortFunc(retrieved, func(a, b types.User) int {
+		return strings.Compare(a.GetName(), b.GetName())
+	})
+	require.Empty(t, cmp.Diff(expectedUsers, retrieved, cmpopts.SortSlices(devicesSort)), "not all users returned from listing operation")
 }
 
 func TestCompareAndSwapUser(t *testing.T) {