diff --git a/go/teams/loader.go b/go/teams/loader.go index e3df11288272..19bcc32c67c6 100644 --- a/go/teams/loader.go +++ b/go/teams/loader.go @@ -102,8 +102,7 @@ func (l *TeamLoader) load1(ctx context.Context, me keybase1.UserVersion, lArg ke mungedWantMembers = nil } - var ret *keybase1.TeamData - ret, err = l.load2(ctx, load2ArgT{ + ret, err := l.load2(ctx, load2ArgT{ teamID: teamID, needAdmin: lArg.NeedAdmin, @@ -129,7 +128,7 @@ func (l *TeamLoader) load1(ctx context.Context, me keybase1.UserVersion, lArg ke // Sanity check that secretless teams never escape. // They are meant only to be returned by recursively called load2's // and used internally by ImplicitAdmins. - if ret.Secretless { + if ret.team.Secretless { return nil, fmt.Errorf("team loader fault: got secretless team") } @@ -138,12 +137,12 @@ func (l *TeamLoader) load1(ctx context.Context, me keybase1.UserVersion, lArg ke // because the cache is keyed by ID. if teamName != nil { // (TODO: this won't work for renamed level 3 teams or above. There's work on this in miles/teamloader-names) - if !teamName.Eq(ret.Name) { - return nil, fmt.Errorf("team name mismatch: %v != %v", ret.Name, teamName.String()) + if !teamName.Eq(ret.team.Name) { + return nil, fmt.Errorf("team name mismatch: %v != %v", ret.team.Name, teamName.String()) } } - return ret, nil + return &ret.team, nil } func (l *TeamLoader) checkArg(ctx context.Context, lArg keybase1.LoadTeamArg) error { @@ -189,9 +188,14 @@ type load2ArgT struct { me keybase1.UserVersion } +type load2ResT struct { + team keybase1.TeamData + didRepoll bool +} + // Load2 does the rest of the work loading a team. // It is `playchain` described in the pseudocode in teamplayer.txt -func (l *TeamLoader) load2(ctx context.Context, arg load2ArgT) (ret *keybase1.TeamData, err error) { +func (l *TeamLoader) load2(ctx context.Context, arg load2ArgT) (ret *load2ResT, err error) { ctx = libkb.WithLogTag(ctx, "LT") // Load Team traceLabel := fmt.Sprintf("TeamLoader#load2(%v)", arg.teamID) if len(arg.reason) > 0 { @@ -202,8 +206,9 @@ func (l *TeamLoader) load2(ctx context.Context, arg load2ArgT) (ret *keybase1.Te return ret, err } -func (l *TeamLoader) load2Inner(ctx context.Context, arg load2ArgT) (*keybase1.TeamData, error) { +func (l *TeamLoader) load2Inner(ctx context.Context, arg load2ArgT) (*load2ResT, error) { var err error + var didRepoll bool // Single-flight lock by team ID. lock := l.locktab.AcquireOnName(ctx, l.G(), arg.teamID.String()) @@ -255,6 +260,7 @@ func (l *TeamLoader) load2Inner(ctx context.Context, arg load2ArgT) (*keybase1.T if err != nil { return nil, err } + didRepoll = true } else { lastSeqno = ret.Chain.LastSeqno lastLinkID = ret.Chain.LastLinkID @@ -415,7 +421,10 @@ func (l *TeamLoader) load2Inner(ctx context.Context, arg load2ArgT) (*keybase1.T return nil, err } - return ret, nil + return &load2ResT{ + team: *ret, + didRepoll: didRepoll, + }, nil } // Decide whether to repoll merkle based on load arg. @@ -535,19 +544,12 @@ func (l *TeamLoader) satisfiesNeedAdmin(ctx context.Context, me keybase1.UserVer if !state.IsSubteam() { return false } - implicitAdmins, err := l.implicitAdminsAncestor(ctx, state.GetID(), state.GetParentID()) + yes, err := l.isImplicitAdminOf(ctx, state.GetID(), state.GetParentID(), me, me) if err != nil { - l.G().Log.CDebugf(ctx, "TeamLoader error getting implicit admins: %s", err) + l.G().Log.CDebugf(ctx, "TeamLoader error getting checking implicit admin: %s", err) return false } - found := false - for _, ia := range implicitAdmins { - if ia == me { - found = true - break - } - } - if !found { + if !yes { return false } } @@ -557,6 +559,83 @@ func (l *TeamLoader) satisfiesNeedAdmin(ctx context.Context, me keybase1.UserVer return true } +// Check whether a user is an implicit admin of a team. +func (l *TeamLoader) isImplicitAdminOf(ctx context.Context, teamID keybase1.TeamID, ancestorID *keybase1.TeamID, + me keybase1.UserVersion, uv keybase1.UserVersion) (bool, error) { + + // IDs of ancestors that were not freshly polled. + // Check them again with forceRepoll if the affirmitive is not found cached. + checkAgain := make(map[keybase1.TeamID]bool) + + check1 := func(chain *TeamSigChainState) bool { + role, err := chain.GetUserRole(uv) + if err != nil { + return false + } + return role.IsAdminOrAbove() + } + + i := 0 + for { + i++ + if i >= 100 { + // Break in case there's a bug in this loop. + return false, fmt.Errorf("stuck in a loop while checking for implicit admin: %v", ancestorID) + } + + // Use load2 so that we can use subteam-reader and get secretless teams. + ancestor, err := l.load2(ctx, load2ArgT{ + teamID: *ancestorID, + reason: "isImplicitAdminOf-1", + me: me, + readSubteamID: &teamID, + }) + if err != nil { + return false, err + } + // Be wary, `ancestor` could be, and is likely, a secretless team. + // Do not let it out of sight. + ancestorChain := TeamSigChainState{inner: ancestor.team.Chain} + + if !ancestor.didRepoll { + checkAgain[ancestorChain.GetID()] = true + } + + if check1(&ancestorChain) { + return true, nil + } + + if !ancestorChain.IsSubteam() { + break + } + // Get the next level up. + ancestorID = ancestorChain.GetParentID() + } + + // The answer was not found to be yes in the cache. + // Try again with the teams that were not polled as they might have unseen updates. + for ancestorID, _ := range checkAgain { + ancestor, err := l.load2(ctx, load2ArgT{ + teamID: ancestorID, + reason: "isImplicitAdminOf-again", + me: me, + forceRepoll: true, // Get the latest info. + readSubteamID: &teamID, + }) + if err != nil { + return false, err + } + // Be wary, `ancestor` could be, and is likely, a secretless team. + // Do not let it out of sight. + ancestorChain := TeamSigChainState{inner: ancestor.team.Chain} + if check1(&ancestorChain) { + return true, nil + } + } + + return false, nil +} + // Whether the snapshot has loaded at least up to the key generation. func (l *TeamLoader) satisfiesNeedKeyGeneration(ctx context.Context, needKeyGeneration keybase1.PerTeamKeyGeneration, state *keybase1.TeamData) error { if needKeyGeneration == 0 { @@ -747,7 +826,7 @@ func (l *TeamLoader) implicitAdminsAncestor(ctx context.Context, teamID keybase1 } // Be wary, `ancestor` could be, and is likely, a secretless team. // Do not let it out of sight. - ancestorChain := TeamSigChainState{inner: ancestor.Chain} + ancestorChain := TeamSigChainState{inner: ancestor.team.Chain} // Gather the admins. adminRoles := []keybase1.TeamRole{keybase1.TeamRole_OWNER, keybase1.TeamRole_ADMIN} @@ -797,7 +876,7 @@ func (l *TeamLoader) NotifyTeamRename(ctx context.Context, id keybase1.TeamID, n loopID := &id for loopID != nil { - team, err := l.load2(ctx, load2ArgT{ + load2Res, err := l.load2(ctx, load2ArgT{ teamID: *loopID, reason: "NotifyTeamRename-force", forceRepoll: true, @@ -808,7 +887,7 @@ func (l *TeamLoader) NotifyTeamRename(ctx context.Context, id keybase1.TeamID, n return err } ancestorIDs = append(ancestorIDs, *loopID) - chain := TeamSigChainState{inner: team.Chain} + chain := TeamSigChainState{inner: load2Res.team.Chain} if chain.IsSubteam() { loopID = chain.GetParentID() } else { diff --git a/go/teams/loader2.go b/go/teams/loader2.go index 1e01b2bb6bb2..01eff59218bc 100644 --- a/go/teams/loader2.go +++ b/go/teams/loader2.go @@ -242,10 +242,11 @@ func (l *TeamLoader) walkUpToAdmin( if target.Eq(*parent) { arg.needSeqnos = []keybase1.Seqno{admin.Seqno} } - team, err = l.load2(ctx, arg) + load2Res, err := l.load2(ctx, arg) if err != nil { return nil, err } + team = &load2Res.team } if team == nil { return nil, fmt.Errorf("teamloader fault: nil team after admin walk") @@ -476,7 +477,7 @@ func (l *TeamLoader) checkParentChildOperations(ctx context.Context, } for _, pco := range parentChildOperations { - parentChain := TeamSigChainState{inner: parent.Chain} + parentChain := TeamSigChainState{inner: parent.team.Chain} err = l.checkOneParentChildOperation(ctx, pco, loadingTeamID, &parentChain) if err != nil { return err @@ -802,7 +803,7 @@ func (l *TeamLoader) calculateName(ctx context.Context, // Swap out the parent name as the base of this name. // Check that the root ancestor name and depth still match the subteam chain. - newName, err = parent.Name.Append(string(chain.LatestLastNamePart())) + newName, err = parent.team.Name.Append(string(chain.LatestLastNamePart())) if err != nil { return newName, fmt.Errorf("invalid new subteam name: %v", err) }