Handle used_invites with invite stubbing (keybase#22801)

`used_invites` checks assertions did not consider that invite links could be
stubbed, fix that.

go/protocol/keybase1/extras.go

@@ -2866,6 +2866,13 @@ func (req *TeamChangeReq) CompleteInviteID(inviteID TeamInviteID, uv UserVersion
 	req.CompletedInvites[inviteID] = uv
 }
 
+func (req *TeamChangeReq) UseInviteID(inviteID TeamInviteID, uv UserVersionPercentForm) {
+	req.UsedInvites = append(req.UsedInvites, TeamUsedInvite{
+		InviteID: inviteID,
+		Uv:       uv,
+	})
+}
+
 func (req *TeamChangeReq) GetAllAdds() (ret []UserVersion) {
 	ret = append(ret, req.RestrictedBotUVs()...)
 	ret = append(ret, req.Bots...)
@@ -3583,6 +3590,15 @@ func (s TeamSigChainState) KeySummary() string {
 	return fmt.Sprintf("{maxPTK:%d, ptk:%v}", s.MaxPerTeamKeyGeneration, v)
 }
 
+func (s TeamSigChainState) HasAnyStubbedLinks() bool {
+	for _, v := range s.StubbedLinks {
+		if v {
+			return true
+		}
+	}
+	return false
+}
+
 func (h *HiddenTeamChain) IsStale() bool {
 	if h == nil {
 		return false

go/teams/chain.go

@@ -2225,9 +2225,11 @@ func (t *teamSigchainPlayer) completeInvites(stateToUpdate *TeamSigChainState, c
 	for id := range completed {
 		invite, ok := stateToUpdate.inner.ActiveInvites[id]
 		if !ok {
-			// Invite doesn't exist - that's likely an error, but we've allowed
-			// this for so long that we have to prove that no team has done
-			// this in order to change this.
+			// Invite doesn't exist or we don't know about it because invite
+			// links were stubbed. We could do a similar check here that we do
+			// in teamSigchainPlayer.useInvites, but we haven't been doing it
+			// in the past, so we might have links with issues like that in the
+			// wild.
 			continue
 		}
 		if invite.MaxUses != nil {
@@ -2251,41 +2253,55 @@ func (t *teamSigchainPlayer) obsoleteInvites(stateToUpdate *TeamSigChainState, r
 }
 
 func (t *teamSigchainPlayer) useInvites(stateToUpdate *TeamSigChainState, roleUpdates chainRoleUpdates, used []SCMapInviteIDUVPair) error {
+	if len(used) == 0 {
+		return nil
+	}
+
+	hasStubbedLinks := stateToUpdate.HasAnyStubbedLinks()
 	for _, pair := range used {
 		inviteID, err := pair.InviteID.TeamInviteID()
 		if err != nil {
 			return err
 		}
-		invite, ok := stateToUpdate.inner.ActiveInvites[inviteID]
-		if !ok {
+
+		invite, foundInvite := stateToUpdate.inner.ActiveInvites[inviteID]
+		if foundInvite {
+			// We found the invite, check if current invite usage is valid.
+			if invite.MaxUses == nil {
+				return fmt.Errorf("`used_invites` for an invite that did not have `max_uses`: %s", inviteID)
+			}
+			maxUses := *invite.MaxUses
+			uses := len(stateToUpdate.inner.UsedInvites[inviteID])
+			if !maxUses.IsInfiniteUses() && uses >= int(maxUses) {
+				return fmt.Errorf("invite %s is expired after %d uses", inviteID, uses)
+			}
+		} else if !hasStubbedLinks {
+			// We couldn't find the invite, and we have no stubbed links, which
+			// means that inviteID is invalid.
 			return fmt.Errorf("could not find active invite ID in used_invites: %s", inviteID)
 		}
-		if invite.MaxUses == nil {
-			return fmt.Errorf("`used_invites` for an invite that did not have `max_uses`: %s", inviteID)
-		}
-		maxUses := *invite.MaxUses
-		uses := len(stateToUpdate.inner.UsedInvites[inviteID])
-		if !maxUses.IsInfiniteUses() && uses >= int(maxUses) {
-			return fmt.Errorf("invite %s is expired after %d uses", inviteID, uses)
-		}
+
 		uv, err := keybase1.ParseUserVersion(pair.UV)
 		if err != nil {
 			return err
 		}
-		var foundUV bool
-		for _, updatedUV := range roleUpdates[invite.Role] {
-			if uv.Eq(updatedUV) {
-				foundUV = true
-				break
+		if foundInvite {
+			// If we have the invite, also check if invite role matches role
+			// added.
+			var foundUV bool
+			for _, updatedUV := range roleUpdates[invite.Role] {
+				if uv.Eq(updatedUV) {
+					foundUV = true
+					break
+				}
+			}
+			if !foundUV {
+				return fmt.Errorf("used_invite for UV %s that was not added as role %s", pair.UV, invite.Role.HumanString())
 			}
 		}
-		if !foundUV {
-			return fmt.Errorf("used_invite for UV %s that was not added as role %s", pair.UV, invite.Role.HumanString())
-		}
+
 		logPoint := len(stateToUpdate.inner.UserLog[uv]) - 1
 		if logPoint < 0 {
-			// This check is redundant, but better to be safe here instead of
-			// storing wrong index to UserLog.
 			return fmt.Errorf("used_invite for UV %s that was not added to to the team", pair.UV)
 		}
 		stateToUpdate.inner.UsedInvites[inviteID] = append(stateToUpdate.inner.UsedInvites[inviteID],

go/teams/chain_parse.go

@@ -207,6 +207,20 @@ func (s *SCTeamMember) MarshalJSON() (b []byte, err error) {
 	return keybase1.Quote(keybase1.UserVersion(*s).PercentForm().String()), nil
 }
 
+func makeSCMapInviteIDUVMap(pairs []keybase1.TeamUsedInvite) (ret []SCMapInviteIDUVPair) {
+	if len(pairs) > 0 {
+		ret = make([]SCMapInviteIDUVPair, len(pairs))
+		for i, v := range pairs {
+			ret[i] = SCMapInviteIDUVPair{
+				InviteID: SCTeamInviteID(v.InviteID),
+				UV:       v.Uv,
+			}
+		}
+	}
+
+	return ret
+}
+
 // Non-team-specific stuff below the line
 // -------------------------
 

go/teams/multiple_use_invite_test.go

@@ -0,0 +1,106 @@
+package teams
+
+import (
+	"context"
+	"testing"
+
+	"github.com/keybase/client/go/kbtest"
+	"github.com/keybase/client/go/protocol/keybase1"
+	"github.com/stretchr/testify/require"
+)
+
+// TODO: This test might be obsolete by the time we are done with this project.
+// But it's the first test that adds and "uses" multiple use invite "for real",
+// that is, also sending it to the server, not just testing sigchain player.
+
+func TestTeamInviteStubbing(t *testing.T) {
+	tc := SetupTest(t, "team", 1)
+	defer tc.Cleanup()
+	_, err := kbtest.CreateAndSignupFakeUser("team", tc.G)
+	require.NoError(t, err)
+
+	tc2 := SetupTest(t, "team", 1)
+	defer tc2.Cleanup()
+	user2, err := kbtest.CreateAndSignupFakeUser("team", tc2.G)
+	require.NoError(t, err)
+
+	teamname := createTeam(tc)
+
+	t.Logf("Created team %s", teamname)
+
+	teamObj, err := Load(context.TODO(), tc.G, keybase1.LoadTeamArg{
+		Name:      teamname,
+		NeedAdmin: true,
+	})
+	require.NoError(t, err)
+
+	makeAndPostSeitanInviteLink := func(ctx context.Context, team *Team, role keybase1.TeamRole) SCTeamInvite {
+		ikey, err := GenerateIKey()
+		require.NoError(t, err)
+		sikey, err := ikey.GenerateSIKey()
+		require.NoError(t, err)
+		inviteID, err := sikey.GenerateTeamInviteID()
+		require.NoError(t, err)
+		label := keybase1.NewSeitanKeyLabelDefault(keybase1.SeitanKeyLabelType(0))
+		_, encoded, err := ikey.GeneratePackedEncryptedKey(ctx, team, label)
+		require.NoError(t, err)
+
+		maxUses := keybase1.TeamInviteMaxUses(10)
+		invite := SCTeamInvite{
+			Type:    "seitan_invite_token",
+			Name:    keybase1.TeamInviteName(encoded),
+			ID:      inviteID,
+			MaxUses: &maxUses,
+		}
+		err = team.postInvite(ctx, invite, role)
+		require.NoError(t, err)
+		return invite
+	}
+
+	scInvite := makeAndPostSeitanInviteLink(context.TODO(), teamObj, keybase1.TeamRole_READER)
+
+	teamObj, err = Load(context.TODO(), tc.G, keybase1.LoadTeamArg{
+		Name:      teamname,
+		NeedAdmin: true,
+	})
+	require.NoError(t, err)
+
+	changeReq := keybase1.TeamChangeReq{}
+	err = changeReq.AddUVWithRole(user2.GetUserVersion(), keybase1.TeamRole_READER, nil /* botSettings */)
+	require.NoError(t, err)
+	changeReq.UseInviteID(keybase1.TeamInviteID(scInvite.ID), user2.GetUserVersion().PercentForm())
+	err = teamObj.ChangeMembershipWithOptions(context.TODO(), changeReq, ChangeMembershipOptions{})
+	require.NoError(t, err)
+
+	// User 2 loads team
+
+	teamObj, err = Load(context.TODO(), tc2.G, keybase1.LoadTeamArg{
+		Name:      teamname,
+		NeedAdmin: false,
+	})
+	require.NoError(t, err)
+
+	inner := teamObj.chain().inner
+	require.Len(t, inner.ActiveInvites, 0)
+	require.Len(t, inner.UsedInvites, 1)
+	require.Len(t, inner.UsedInvites[keybase1.TeamInviteID(scInvite.ID)], 1)
+
+	// User 1 makes User 2 admin
+
+	err = SetRoleAdmin(context.TODO(), tc.G, teamname, user2.Username)
+	require.NoError(t, err)
+
+	// User 2 loads team again
+
+	teamObj, err = Load(context.TODO(), tc2.G, keybase1.LoadTeamArg{
+		Name:      teamname,
+		NeedAdmin: true,
+	})
+	require.NoError(t, err)
+
+	inner = teamObj.chain().inner
+	require.Len(t, inner.ActiveInvites, 1)
+	_, ok := inner.ActiveInvites[keybase1.TeamInviteID(scInvite.ID)]
+	require.True(t, ok, "invite found loaded by user 2")
+	require.Len(t, inner.UsedInvites[keybase1.TeamInviteID(scInvite.ID)], 1)
+}

go/teams/teams.go

@@ -1621,6 +1621,8 @@ func (t *Team) changeMembershipSection(ctx context.Context, req keybase1.TeamCha
 	}
 
 	section.CompletedInvites = req.CompletedInvites
+	section.UsedInvites = makeSCMapInviteIDUVMap(req.UsedInvites)
+
 	section.Implicit = t.IsImplicit()
 	section.Public = t.IsPublic()