client: auto re-invite idle-kicked GC members on return

client/client_groupchat.go

@@ -341,12 +341,15 @@ func (c *Client) handleGCInvite(ru *RemoteUser, invite rpc.RMGroupInvite) error
 	var iid uint64
 	err := c.dbUpdate(func(tx clientdb.ReadWriteTx) error {
 		var err error
-		_, err = c.db.GetGC(tx, invite.ID)
-		if !errors.Is(err, clientdb.ErrNotFound) {
-			if err == nil {
-				err = fmt.Errorf("can't accept gc invite: gc %q already exists",
-					invite.ID.String())
-			}
+		// A failure other than "not found" is a real error. Otherwise store
+		// the invite even when the GC already exists locally: a re-invite is
+		// how an admin re-adds a user whose local copy is stale (e.g. after
+		// restoring an old backup). The stale roster is refreshed by the
+		// normal RMGroupList update (maybeUpdateGC) once the invite is
+		// accepted, which already verifies the sender's admin permission and
+		// rejects generation backtracks.
+		if _, err = c.db.GetGC(tx, invite.ID); err != nil &&
+			!errors.Is(err, clientdb.ErrNotFound) {
 			return err
 		}
 
@@ -1243,9 +1246,18 @@ func (c *Client) handleGCMessage(ru *RemoteUser, gcm rpc.RMGroupMessage, ts time
 	}
 
 	if !found {
-		// The sender is not in the GC list we have.
-		c.log.Warnf("Received message in GC %q from non-member %s",
-			rgcm.GCAlias, ru)
+		// The sender is not in our member list. If they were idle-kicked
+		// and are messaging the GC again, re-add them; otherwise drop it.
+		if readded, rerr := c.maybeReAddIdleKickedMember(ru, gcm.ID); rerr != nil {
+			c.log.Warnf("Unable to re-add idle-kicked member %s to GC %s: %v",
+				ru, gcm.ID, rerr)
+		} else if readded {
+			c.log.Infof("Re-added idle-kicked member %s to GC %s after their message",
+				ru, gcm.ID)
+		} else {
+			c.log.Warnf("Received message in GC %q from non-member %s",
+				rgcm.GCAlias, ru)
+		}
 		return nil
 	}
 
@@ -1453,6 +1465,11 @@ func (c *Client) GCKick(gcID zkidentity.ShortID, uid UserID, reason string) erro
 		return err
 	}
 
+	// Track idle kicks so the member can be auto re-added if they return.
+	if reason == gcIdleKickReason {
+		c.recordIdleKick(gcID, uid)
+	}
+
 	rmgk := rpc.RMGroupKick{
 		Member:       uid,
 		Reason:       reason,
@@ -1521,9 +1538,115 @@ func (c *Client) handleGCKick(ru *RemoteUser, rmgk rpc.RMGroupKick, ts time.Time
 		rmgk.Reason, !rmgk.Parted)
 	c.notifyUpdatedGC(ru, oldGC.Metadata, rmgk.NewGroupList, ts)
 
+	// If an admin idle-kicked another user, record it so the local client
+	// (when it is also an admin) can auto re-add them when they return.
+	if !rmgk.Parted && rmgk.Reason == gcIdleKickReason && kickedUid != c.PublicID() {
+		c.recordIdleKick(rmgk.NewGroupList.ID, kickedUid)
+	}
+
 	return nil
 }
 
+// gcIdleKickReason is the reason string used when auto-removing an idle user
+// from a GC. It flags an idle kick so the member can be auto re-added if they
+// later message the GC again.
+const gcIdleKickReason = "User is idle for too long"
+
+// recordIdleKick persists (only when auto-remove-idle is enabled) that uid was
+// kicked from gcid for idleness, so it can be auto re-added on return.
+func (c *Client) recordIdleKick(gcid zkidentity.ShortID, uid clientintf.UserID) {
+	if c.cfg.AutoRemoveIdleUsersInterval <= 0 {
+		return
+	}
+	err := c.dbUpdate(func(tx clientdb.ReadWriteTx) error {
+		return c.db.AddToGCKickedMembers(tx, gcid, uid, true)
+	})
+	if err != nil {
+		c.log.Warnf("Unable to record idle-kicked member %s in GC %s: %v",
+			uid, gcid, err)
+	}
+}
+
+// maybeReAddIdleKickedMember re-adds ru to gcID when the local client admins the
+// GC, ru was idle-kicked from it, ru is not blocked, and auto-remove-idle is
+// enabled. The re-add appends the member, bumps the generation and broadcasts
+// the updated group list, which the returning member applies via the normal
+// group-list update (no invite needed - KX survives the kick). Returns whether
+// it re-added the member.
+func (c *Client) maybeReAddIdleKickedMember(ru *RemoteUser, gcID zkidentity.ShortID) (bool, error) {
+	if c.cfg.AutoRemoveIdleUsersInterval <= 0 {
+		return false, nil
+	}
+
+	uid := ru.ID()
+	var newMeta rpc.RMGroupList
+	var readded bool
+	err := c.dbUpdate(func(tx clientdb.ReadWriteTx) error {
+		gc, err := c.db.GetGC(tx, gcID)
+		if err != nil {
+			return err
+		}
+
+		// Only an admin can change the roster.
+		if err := c.uidHasGCPerm(&gc.Metadata, c.PublicID()); err != nil {
+			return nil
+		}
+
+		// Only re-add members that were idle-kicked.
+		kicked, err := c.db.GetGCKickedMembers(tx, gcID)
+		if err != nil {
+			return err
+		}
+		if !kicked.IsIdleKicked(uid) {
+			return nil
+		}
+
+		// Never re-add a blocked user; just drop the stale record.
+		gcbl, err := c.db.GetGCBlockList(tx, gcID)
+		if err != nil {
+			return err
+		}
+		if c.db.IsBlocked(tx, uid) || gcbl.IsBlocked(uid) {
+			return c.db.RemoveFromGCKickedMembers(tx, gcID, uid)
+		}
+
+		// If somehow already a member, just clear the record.
+		for i := range gc.Metadata.Members {
+			if gc.Metadata.Members[i] == uid {
+				return c.db.RemoveFromGCKickedMembers(tx, gcID, uid)
+			}
+		}
+
+		// Re-add: append, bump generation, save, clear the record.
+		gc.Metadata.Members = append(gc.Metadata.Members, uid)
+		gc.Metadata.Generation += 1
+		gc.Metadata.Timestamp = time.Now().Unix()
+		if err := c.db.SaveGC(tx, gc); err != nil {
+			return err
+		}
+		if err := c.db.RemoveFromGCKickedMembers(tx, gcID, uid); err != nil {
+			return err
+		}
+		c.db.LogGCMsg(tx, gc.Name(), gcID, true, "",
+			fmt.Sprintf("Re-added %s after they returned from being idle-kicked",
+				strescape.Nick(ru.Nick())), time.Now())
+		newMeta = gc.Metadata
+		readded = true
+		return nil
+	})
+	if err != nil || !readded {
+		return false, err
+	}
+
+	// Broadcast the updated member list. The re-added member applies it via
+	// the normal group-list update (maybeUpdateGC).
+	if err := c.sendToGCMembers(gcID, newMeta.Members, "sendlist", newMeta, nil); err != nil {
+		return true, err
+	}
+	c.ntfns.notifyOnAddedGCMembers(newMeta, []clientintf.UserID{uid})
+	return true, nil
+}
+
 // PartFromGC withdraws the local client from the given GC.
 func (c *Client) PartFromGC(gcID zkidentity.ShortID, reason string) error {
 	var gc clientdb.GroupChat

client/client_kx.go

@@ -984,7 +984,7 @@ func (c *Client) unsubIdleUsers() error {
 			gcid := gcid
 			uid := uid
 			go func() {
-				err := c.GCKick(gcid, uid, "User is idle for too long")
+				err := c.GCKick(gcid, uid, gcIdleKickReason)
 				if err != nil {
 					c.log.Warnf("Unable to remove user %s from GC %s: %v",
 						uid, gcid, err)

client/clientdb/gc.go

@@ -19,9 +19,10 @@ import (
 )
 
 const (
-	gcAliasesFile  = "gcaliases.json"
-	invitesTable   = "invites"
-	gcBlockListExt = ".blocklist"
+	gcAliasesFile      = "gcaliases.json"
+	invitesTable       = "invites"
+	gcBlockListExt     = ".blocklist"
+	gcKickedMembersExt = ".kickedmembers"
 )
 
 type GCInvite struct {
@@ -391,9 +392,13 @@ func (db *DB) DeleteGC(tx ReadWriteTx, gcID zkidentity.ShortID) error {
 	if err := os.Remove(filename); err != nil {
 		return err
 	}
-	blockListFname := filename + gcBlockListExt
-	if fileExists(blockListFname) {
-		return os.Remove(blockListFname)
+	for _, ext := range []string{gcBlockListExt, gcKickedMembersExt} {
+		auxFname := filename + ext
+		if fileExists(auxFname) {
+			if err := os.Remove(auxFname); err != nil {
+				return err
+			}
+		}
 	}
 	return nil
 }
@@ -546,6 +551,70 @@ func (db *DB) GetGCBlockList(tx ReadTx, gcid zkidentity.ShortID) (GCBlockList, e
 
 }
 
+// GCKickedMembers maps the UID of a member removed from a GC to whether the
+// removal was due to idleness. Used to auto re-invite idle-kicked members when
+// they message the GC again.
+type GCKickedMembers map[string]bool
+
+// IsIdleKicked returns true if the given UID is recorded as having been kicked
+// from the GC for idleness.
+func (gckm GCKickedMembers) IsIdleKicked(uid UserID) bool {
+	return gckm[uid.String()]
+}
+
+// AddToGCKickedMembers records that the given UID was kicked from the GC. idle
+// flags whether the removal was due to idleness.
+func (db *DB) AddToGCKickedMembers(tx ReadWriteTx, gcid zkidentity.ShortID, uid UserID, idle bool) error {
+	filename := filepath.Join(db.root, groupchatDir, gcid.String()+
+		gcKickedMembersExt)
+
+	var entries GCKickedMembers
+	err := db.readJsonFile(filename, &entries)
+	if err != nil && !errors.Is(err, ErrNotFound) {
+		return err
+	}
+
+	if entries == nil {
+		entries = make(GCKickedMembers)
+	}
+	entries[uid.String()] = idle
+
+	return db.saveJsonFile(filename, &entries)
+}
+
+// RemoveFromGCKickedMembers clears the kick record for the given UID in the GC.
+func (db *DB) RemoveFromGCKickedMembers(tx ReadWriteTx, gcid zkidentity.ShortID, uid UserID) error {
+	filename := filepath.Join(db.root, groupchatDir, gcid.String()+
+		gcKickedMembersExt)
+
+	var entries GCKickedMembers
+	err := db.readJsonFile(filename, &entries)
+	if errors.Is(err, ErrNotFound) {
+		// Nothing recorded. NOP.
+		return nil
+	}
+	if err != nil {
+		return err
+	}
+
+	delete(entries, uid.String())
+	return db.saveJsonFile(filename, entries)
+}
+
+// GetGCKickedMembers returns the kicked-members record of the specified GC.
+// Returns nil if there is none.
+func (db *DB) GetGCKickedMembers(tx ReadTx, gcid zkidentity.ShortID) (GCKickedMembers, error) {
+	filename := filepath.Join(db.root, groupchatDir, gcid.String()+
+		gcKickedMembersExt)
+
+	var entries GCKickedMembers
+	err := db.readJsonFile(filename, &entries)
+	if errors.Is(err, ErrNotFound) {
+		return nil, nil
+	}
+	return entries, err
+}
+
 // CacheReceivedGCM stores a cached received GC message.
 func (db *DB) CacheReceivedGCM(tx ReadWriteTx, rgcm clientintf.ReceivedGCMsg) error {
 	filename := filepath.Join(db.root, cachedGCMsDir, rgcm.MsgID.String())

internal/e2etests/gc_test.go

@@ -1320,3 +1320,74 @@ func TestGCInviteExpiration(t *testing.T) {
 	assert.NilErr(t, bob.AcceptGroupChatInvite(iid3))
 	assert.ChanWritten(t, bobJoinedGCChan)
 }
+
+// TestGCAutoReinviteIdleKickedMember verifies that, when auto-remove-idle is
+// enabled, a GC admin re-adds a member it previously idle-kicked once that
+// member sends a new GC message. This models a member that was offline when it
+// was idle-kicked, so it never saw the kick and still believes it is a member.
+func TestGCAutoReinviteIdleKickedMember(t *testing.T) {
+	t.Parallel()
+
+	tcfg := testScaffoldCfg{}
+	ts := newTestScaffold(t, tcfg)
+
+	// Alice is the admin. Leaving AutoRemoveIdleUsersInterval at its default
+	// (non-zero) value is what enables the auto re-invite feature.
+	alice := ts.newClient("alice")
+	bob := ts.newClient("bob")
+
+	ts.kxUsers(alice, bob)
+
+	// Alice creates a GC and Bob joins.
+	gcID, err := alice.NewGroupChat("test_gc")
+	assert.NilErr(t, err)
+	bobAcceptedChan := bob.acceptNextGCInvite(gcID)
+	assert.NilErr(t, alice.InviteToGroupChat(gcID, bob.PublicID()))
+	assert.NilErrFromChan(t, bobAcceptedChan)
+	assertClientInGC(t, bob, gcID)
+	assertClientSeesInGC(t, alice, gcID, bob.PublicID())
+
+	// Pause Bob right before he applies the incoming kick, so that (like an
+	// offline member) he never processes it, keeps the GC locally and still
+	// believes he is a member. The received notification fires on the RV
+	// manager goroutine before the kick handler runs, so blocking here keeps
+	// Bob from removing the GC.
+	bobGotKickChan := make(chan struct{}, 1)
+	bobContChan := make(chan struct{})
+	bob.handleSync(client.OnRMReceived(func(_ *client.RemoteUser, _ *rpc.RMHeader, p interface{}, _ time.Time) {
+		if _, ok := p.(rpc.RMGroupKick); ok {
+			bobGotKickChan <- struct{}{}
+			<-bobContChan
+		}
+	}))
+
+	// Track Alice re-adding members (the behavior under test). Registered
+	// after the join so it only captures the re-invite.
+	wantUIDs := []clientintf.UserID{bob.PublicID()}
+	aliceReAddedChan := make(chan []clientintf.UserID, 4)
+	alice.handle(client.OnAddedGCMembersNtfn(func(_ rpc.RMGroupList, uids []clientintf.UserID) {
+		aliceReAddedChan <- uids
+	}))
+
+	// Alice idle-kicks Bob. The reason must match the one the idle unsub
+	// machinery uses for the kick to be tracked for re-invite.
+	const idleReason = "User is idle for too long"
+	assert.NilErr(t, alice.GCKick(gcID, bob.PublicID(), idleReason))
+
+	// Bob receives the kick but is paused before applying it, so he still
+	// holds the GC.
+	assert.ChanWritten(t, bobGotKickChan)
+
+	// Bob, still believing he is a member, sends a new message. Alice should
+	// re-add Bob and broadcast the updated member list instead of dropping
+	// the message.
+	assert.NilErr(t, bob.GCMessage(gcID, "i am back", rpc.MessageModeNormal, nil))
+
+	// Alice re-adds Bob and lists him as a member again.
+	assert.ChanWrittenWithVal(t, aliceReAddedChan, wantUIDs)
+	assertClientSeesInGC(t, alice, gcID, bob.PublicID())
+
+	// Let Bob finish processing the kick to avoid leaking the paused
+	// goroutine.
+	close(bobContChan)
+}