Ignore implicit memberships in Requester.getAllTeams #101
Conversation
|
This probably needs a test or something similar, I've yet to figure out how to do that though. Seems to be working locally, though. |
src/kssh/requester.go
Outdated
| } | ||
| for _, m := range memberships { | ||
| teams = append(teams, m.FqName) | ||
| if m.Role != keybase1.TeamRole_NONE { |
There was a problem hiding this comment.
this might be more edge-case safe if we also != TeamRole_RESTRICTEDBOT
There was a problem hiding this comment.
I should probably also add this in
func (b *Bot) getAllTeams() (teams []string, err error) {
, right?
There was a problem hiding this comment.
And there's also similar code in
func getPrincipals(conf config.Config, sr shared.SignatureRequest) (string, error) {
in sshutils.go
There was a problem hiding this comment.
Good idea, let's move this line to be within both getAllTeams functions.
|
yep thx! I'm still on this but something else also came up today |
|
@mmou hey, I changed some things around here and also tested the entire thing using couple of virtual machines and it seems to work! My real account also hit this bug because it's an implicit admins for some subteams. |
src/shared/teams.go
Outdated
| // GetAllTeams makes an API call and returns list of team names readable for | ||
| // current user. | ||
| func GetAllTeams(api *kbchat.API) (teams []string, err error) { | ||
| // TODO: dedup with same method in keybaseca/bot |
There was a problem hiding this comment.
my bad. I'm so careless :(
When current user is an implicit admin in any team, loading kssh configs from Keybase KV store will fail for that team, failing the entire process. KV store is not available unless the user is an explicit member. This PR changes
Requester.getAllTeamsto skip teams where user is not an explicit member.