If user is a global/team observer/observer+, 'teams' endpoints should not include secrets (#12216)

Fixed auth. issue with Obs/Obs+

changes/bug-2939-obs-shouldnot-see-secrets

@@ -0,0 +1 @@
+- Users with Observer/Observer+ role should not be able to see team secrets.

ee/server/service/teams.go

@@ -21,6 +21,34 @@ import (
 	"github.com/go-kit/kit/log/level"
 )
 
+func obfuscateSecrets(user *fleet.User, teams []*fleet.Team) error {
+	if user == nil {
+		return &authz.Forbidden{}
+	}
+
+	isGlobalObs := user.IsGlobalObserver()
+
+	teamMemberships := user.TeamMembership(func(t fleet.UserTeam) bool {
+		return true
+	})
+	obsMembership := user.TeamMembership(func(t fleet.UserTeam) bool {
+		return t.Role == fleet.RoleObserver || t.Role == fleet.RoleObserverPlus
+	})
+
+	for _, t := range teams {
+		if t == nil {
+			continue
+		}
+		// User does not belong to the team or is a global/team observer/observer+
+		if isGlobalObs || user.GlobalRole == nil && (!teamMemberships[t.ID] || obsMembership[t.ID]) {
+			for _, s := range t.Secrets {
+				s.Secret = fleet.MaskedPassword
+			}
+		}
+	}
+	return nil
+}
+
 func (svc *Service) NewTeam(ctx context.Context, p fleet.TeamPayload) (*fleet.Team, error) {
 	if err := svc.authz.Authorize(ctx, &fleet.Team{}, fleet.ActionWrite); err != nil {
 		return nil, err
@@ -372,7 +400,16 @@ func (svc *Service) ListTeams(ctx context.Context, opt fleet.ListOptions) ([]*fl
 	}
 	filter := fleet.TeamFilter{User: vc.User, IncludeObserver: true}
 
-	return svc.ds.ListTeams(ctx, filter, opt)
+	teams, err := svc.ds.ListTeams(ctx, filter, opt)
+	if err != nil {
+		return nil, err
+	}
+
+	if err = obfuscateSecrets(vc.User, teams); err != nil {
+		return nil, err
+	}
+
+	return teams, nil
 }
 
 func (svc *Service) ListAvailableTeamsForUser(ctx context.Context, user *fleet.User) ([]*fleet.TeamSummary, error) {
@@ -475,7 +512,21 @@ func (svc *Service) GetTeam(ctx context.Context, teamID uint) (*fleet.Team, erro
 
 	logging.WithExtras(ctx, "id", teamID)
 
-	return svc.ds.Team(ctx, teamID)
+	vc, ok := viewer.FromContext(ctx)
+	if !ok {
+		return nil, fleet.ErrNoContext
+	}
+
+	team, err := svc.ds.Team(ctx, teamID)
+	if err != nil {
+		return nil, err
+	}
+
+	if err = obfuscateSecrets(vc.User, []*fleet.Team{team}); err != nil {
+		return nil, err
+	}
+
+	return team, nil
 }
 
 func (svc *Service) TeamEnrollSecrets(ctx context.Context, teamID uint) ([]*fleet.EnrollSecret, error) {
@@ -483,7 +534,34 @@ func (svc *Service) TeamEnrollSecrets(ctx context.Context, teamID uint) ([]*flee
 		return nil, err
 	}
 
-	return svc.ds.TeamEnrollSecrets(ctx, teamID)
+	vc, ok := viewer.FromContext(ctx)
+	if !ok {
+		return nil, fleet.ErrNoContext
+	}
+
+	secrets, err := svc.ds.TeamEnrollSecrets(ctx, teamID)
+	if err != nil {
+		return nil, err
+	}
+
+	isGlobalObs := vc.User.IsGlobalObserver()
+	teamMemberships := vc.User.TeamMembership(func(t fleet.UserTeam) bool {
+		return true
+	})
+	obsMembership := vc.User.TeamMembership(func(t fleet.UserTeam) bool {
+		return t.Role == fleet.RoleObserver || t.Role == fleet.RoleObserverPlus
+	})
+
+	for _, s := range secrets {
+		if s == nil {
+			continue
+		}
+		if isGlobalObs || vc.User.GlobalRole == nil && (!teamMemberships[*s.TeamID] || obsMembership[*s.TeamID]) {
+			s.Secret = fleet.MaskedPassword
+		}
+	}
+
+	return secrets, nil
 }
 
 func (svc *Service) ModifyTeamEnrollSecrets(ctx context.Context, teamID uint, secrets []fleet.EnrollSecret) ([]*fleet.EnrollSecret, error) {

ee/server/service/teams_test.go

@@ -0,0 +1,96 @@
+package service
+
+import (
+	"testing"
+
+	"github.com/fleetdm/fleet/v4/server/fleet"
+	"github.com/fleetdm/fleet/v4/server/ptr"
+	"github.com/stretchr/testify/require"
+)
+
+func TestObfuscateSecrets(t *testing.T) {
+	buildTeams := func(n int) []*fleet.Team {
+		r := make([]*fleet.Team, 0, n)
+		for i := 1; i <= n; i++ {
+			r = append(r, &fleet.Team{
+				ID: uint(i),
+				Secrets: []*fleet.EnrollSecret{
+					{Secret: "abc"},
+					{Secret: "123"},
+				},
+			})
+		}
+		return r
+	}
+
+	t.Run("no user", func(t *testing.T) {
+		err := obfuscateSecrets(nil, nil)
+		require.Error(t, err)
+	})
+
+	t.Run("no teams", func(t *testing.T) {
+		user := fleet.User{}
+		err := obfuscateSecrets(&user, nil)
+		require.NoError(t, err)
+	})
+
+	t.Run("user is not a global observer", func(t *testing.T) {
+		user := fleet.User{GlobalRole: ptr.String(fleet.RoleAdmin)}
+		teams := buildTeams(3)
+
+		err := obfuscateSecrets(&user, teams)
+		require.NoError(t, err)
+
+		for _, team := range teams {
+			for _, s := range team.Secrets {
+				require.NotEqual(t, fleet.MaskedPassword, s.Secret)
+			}
+		}
+	})
+
+	t.Run("user is global observer", func(t *testing.T) {
+		roles := []*string{ptr.String(fleet.RoleObserver), ptr.String(fleet.RoleObserverPlus)}
+		for _, r := range roles {
+			user := &fleet.User{GlobalRole: r}
+			teams := buildTeams(3)
+
+			err := obfuscateSecrets(user, teams)
+			require.NoError(t, err)
+
+			for _, team := range teams {
+				for _, s := range team.Secrets {
+					require.Equal(t, fleet.MaskedPassword, s.Secret)
+				}
+			}
+		}
+	})
+
+	t.Run("user is observer in some teams", func(t *testing.T) {
+		teams := buildTeams(4)
+
+		// Make user an observer in the 'even' teams
+		user := &fleet.User{Teams: []fleet.UserTeam{
+			{
+				Team: *teams[1],
+				Role: fleet.RoleObserver,
+			},
+			{
+				Team: *teams[2],
+				Role: fleet.RoleAdmin,
+			},
+			{
+				Team: *teams[3],
+				Role: fleet.RoleObserverPlus,
+			},
+		}}
+
+		err := obfuscateSecrets(user, teams)
+		require.NoError(t, err)
+
+		for i, team := range teams {
+			for _, s := range team.Secrets {
+				require.Equal(t, fleet.MaskedPassword == s.Secret, i == 0 || i == 1 || i == 3)
+			}
+		}
+	})
+}

server/fleet/users.go

@@ -32,6 +32,25 @@ type User struct {
 	Teams []UserTeam `json:"teams"`
 }
 
+// IsGlobalObserver returns true if user is either a Global Observer or a Global Observer+
+func (u *User) IsGlobalObserver() bool {
+	if u.GlobalRole == nil {
+		return false
+	}
+	return *u.GlobalRole == RoleObserver || *u.GlobalRole == RoleObserverPlus
+}
+
+// TeamMembership returns a map whose keys are the TeamIDs of the teams for which pred evaluates to true
+func (u *User) TeamMembership(pred func(UserTeam) bool) map[uint]bool {
+	result := make(map[uint]bool)
+	for _, t := range u.Teams {
+		if pred(t) {
+			result[t.ID] = true
+		}
+	}
+	return result
+}
+
 func (u *User) IsAdminForcedPasswordReset() bool {
 	if u.SSOEnabled {
 		return false

server/fleet/users_test.go

@@ -10,6 +10,87 @@ import (
 	"golang.org/x/crypto/bcrypt"
 )
 
+func TestIsGlobalObserver(t *testing.T) {
+	testCases := []struct {
+		GlobalRole *string
+		Expected   bool
+	}{
+		{
+			GlobalRole: nil,
+		},
+		{
+			GlobalRole: ptr.String(RoleAdmin),
+		},
+		{
+			GlobalRole: ptr.String(RoleObserver),
+			Expected:   true,
+		},
+		{
+			GlobalRole: ptr.String(RoleObserverPlus),
+			Expected:   true,
+		},
+	}
+
+	for _, tC := range testCases {
+		sut := User{GlobalRole: tC.GlobalRole}
+		require.Equal(t, sut.IsGlobalObserver(), tC.Expected)
+	}
+}
+
+func TestTeamMembership(t *testing.T) {
+	teams := []UserTeam{
+		{
+			Role: RoleAdmin,
+			Team: Team{
+				ID: 1,
+			},
+		},
+		{
+			Role: RoleGitOps,
+			Team: Team{
+				ID: 2,
+			},
+		},
+		{
+			Role: RoleObserver,
+			Team: Team{
+				ID: 3,
+			},
+		},
+		{
+			Role: RoleObserver,
+			Team: Team{
+				ID: 4,
+			},
+		},
+	}
+
+	sut := User{}
+	require.Empty(t, sut.TeamMembership(func(ut UserTeam) bool {
+		return true
+	}))
+
+	sut.Teams = teams
+
+	var result []uint
+	pred := func(ut UserTeam) bool {
+		return ut.Role == RoleGitOps || ut.Role == RoleObserver
+	}
+	for k := range sut.TeamMembership(pred) {
+		result = append(result, k)
+	}
+	require.ElementsMatch(t, result, []uint{2, 3, 4})
+
+	result = make([]uint, 0, len(teams))
+	pred = func(ut UserTeam) bool {
+		return true
+	}
+	for k := range sut.TeamMembership(pred) {
+		result = append(result, k)
+	}
+	require.ElementsMatch(t, result, []uint{1, 2, 3, 4})
+}
+
 func TestValidatePassword(t *testing.T) {
 	passwordTests := []struct {
 		Password, Email      string

server/service/integration_enterprise_test.go

@@ -764,7 +764,7 @@ func (s *integrationEnterpriseTestSuite) TestTeamEndpoints() {
 	defaultFeatures := fleet.Features{}
 	defaultFeatures.ApplyDefaultsForNewInstalls()
 	mysql.ExecAdhocSQL(t, s.ds, func(db sqlx.ExtContext) error {
-		_, err := db.ExecContext(context.Background(), `UPDATE teams SET config = NULL WHERE id = ? `, team.ID)
+		_, err := db.ExecContext(context.Background(), `UPDATE teams SET config = NULL WHERE id = ? `, tm1ID)
 		return err
 	})
 	tmResp.Team = nil
@@ -773,7 +773,7 @@ func (s *integrationEnterpriseTestSuite) TestTeamEndpoints() {
 
 	// modify a team with an empty config
 	mysql.ExecAdhocSQL(t, s.ds, func(db sqlx.ExtContext) error {
-		_, err := db.ExecContext(context.Background(), `UPDATE teams SET config = '{}' WHERE id = ? `, team.ID)
+		_, err := db.ExecContext(context.Background(), `UPDATE teams SET config = '{}' WHERE id = ? `, tm1ID)
 		return err
 	})
 	tmResp.Team = nil
@@ -919,6 +919,186 @@ func (s *integrationEnterpriseTestSuite) TestTeamEndpoints() {
 	s.DoJSON("DELETE", fmt.Sprintf("/api/latest/fleet/teams/%d", tm1ID), nil, http.StatusNotFound, &delResp)
 }
 
+func (s *integrationEnterpriseTestSuite) TestTeamSecretsAreObfuscated() {
+	t := s.T()
+
+	// -----------------
+	// Set up test data
+	// -----------------
+	teams := []*fleet.Team{
+		{
+			Name:        "Team One",
+			Description: "Team description",
+			Secrets:     []*fleet.EnrollSecret{{Secret: "DEF"}},
+		},
+		{
+			Name:        "Team Two",
+			Description: "Team Two description",
+			Secrets:     []*fleet.EnrollSecret{{Secret: "ABC"}},
+		},
+	}
+	for _, team := range teams {
+		_, err := s.ds.NewTeam(context.Background(), team)
+		require.NoError(t, err)
+	}
+
+	global_obs := &fleet.User{
+		Name:       "Global Obs",
+		Email:      "global_obs@example.com",
+		GlobalRole: ptr.String(fleet.RoleObserver),
+	}
+	global_obs_plus := &fleet.User{
+		Name:       "Global Obs+",
+		Email:      "global_obs_plus@example.com",
+		GlobalRole: ptr.String(fleet.RoleObserverPlus),
+	}
+	team_obs := &fleet.User{
+		Name:  "Team Obs",
+		Email: "team_obs@example.com",
+		Teams: []fleet.UserTeam{
+			{
+				Team: *teams[0],
+				Role: fleet.RoleObserver,
+			},
+			{
+				Team: *teams[1],
+				Role: fleet.RoleAdmin,
+			},
+		},
+	}
+	team_obs_plus := &fleet.User{
+		Name:  "Team Obs Plus",
+		Email: "team_obs_plus@example.com",
+		Teams: []fleet.UserTeam{
+			{
+				Team: *teams[0],
+				Role: fleet.RoleAdmin,
+			},
+			{
+				Team: *teams[1],
+				Role: fleet.RoleObserverPlus,
+			},
+		},
+	}
+	users := []*fleet.User{global_obs, global_obs_plus, team_obs, team_obs_plus}
+	for _, u := range users {
+		require.NoError(t, u.SetPassword(test.GoodPassword, 10, 10))
+		_, err := s.ds.NewUser(context.Background(), u)
+		require.NoError(t, err)
+	}
+
+	// --------------------------------------------------------------------
+	// Global obs/obs+ should not be able to see any team secrets
+	// --------------------------------------------------------------------
+	for _, u := range []*fleet.User{global_obs, global_obs_plus} {
+
+		s.setTokenForTest(t, u.Email, test.GoodPassword)
+
+		// list all teams
+		var listResp listTeamsResponse
+		s.DoJSON("GET", "/api/latest/fleet/teams", nil, http.StatusOK, &listResp)
+
+		require.Len(t, listResp.Teams, len(teams))
+		require.NoError(t, listResp.Err)
+
+		for _, team := range listResp.Teams {
+			for _, secret := range team.Secrets {
+				require.Equal(t, fleet.MaskedPassword, secret.Secret)
+			}
+		}
+
+		// listing a team / team secrets
+		for _, team := range teams {
+			var getResp getTeamResponse
+			s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/teams/%d", team.ID), nil, http.StatusOK, &getResp)
+
+			require.NoError(t, getResp.Err)
+			for _, secret := range getResp.Team.Secrets {
+				require.Equal(t, fleet.MaskedPassword, secret.Secret)
+			}
+
+			var secResp teamEnrollSecretsResponse
+			s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/teams/%d/secrets", team.ID), nil, http.StatusOK, &secResp)
+
+			require.Len(t, secResp.Secrets, 1)
+			require.NoError(t, secResp.Err)
+			for _, secret := range secResp.Secrets {
+				require.Equal(t, fleet.MaskedPassword, secret.Secret)
+			}
+		}
+	}
+
+	// --------------------------------------------------------------------
+	// Team obs/obs+ should not be able to see their team secrets
+	// --------------------------------------------------------------------
+	for _, u := range []*fleet.User{team_obs, team_obs_plus} {
+
+		s.setTokenForTest(t, u.Email, test.GoodPassword)
+
+		// list all teams
+		var listResp listTeamsResponse
+		s.DoJSON("GET", "/api/latest/fleet/teams", nil, http.StatusOK, &listResp)
+
+		require.Len(t, listResp.Teams, len(u.Teams))
+		require.NoError(t, listResp.Err)
+
+		for _, team := range listResp.Teams {
+			for _, secret := range team.Secrets {
+				// team_obs has RoleObserver in Team 1, and an RoleAdmin in Team 2
+				// so it should be able to see the secrets in Team 1
+				if u.ID == team_obs.ID {
+					require.Equal(t, fleet.MaskedPassword == secret.Secret, team.ID == teams[0].ID)
+					require.Equal(t, fleet.MaskedPassword != secret.Secret, team.ID == teams[1].ID)
+				}
+
+				// team_obs_plus should not be able to see any Team Secret
+				if u.ID == team_obs_plus.ID {
+					require.Equal(t, fleet.MaskedPassword == secret.Secret, team.ID == teams[1].ID)
+					require.Equal(t, fleet.MaskedPassword != secret.Secret, team.ID == teams[0].ID)
+				}
+			}
+		}
+
+		// listing a team / team secrets
+		for _, team := range teams {
+			var getResp getTeamResponse
+			s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/teams/%d", team.ID), nil, http.StatusOK, &getResp)
+
+			require.NoError(t, getResp.Err)
+			// team_obs has RoleObserver in Team 1, and an RoleAdmin in Team 2
+			// so it should be able to see the secrets in Team 1
+			for _, secret := range getResp.Team.Secrets {
+				if u.ID == team_obs.ID {
+					require.Equal(t, fleet.MaskedPassword == secret.Secret, team.ID == teams[0].ID)
+					require.Equal(t, fleet.MaskedPassword != secret.Secret, team.ID == teams[1].ID)
+				}
+
+				if u.ID == team_obs_plus.ID {
+					require.Equal(t, fleet.MaskedPassword == secret.Secret, team.ID == teams[1].ID)
+					require.Equal(t, fleet.MaskedPassword != secret.Secret, team.ID == teams[0].ID)
+				}
+			}
+
+			var secResp teamEnrollSecretsResponse
+			s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/teams/%d/secrets", team.ID), nil, http.StatusOK, &secResp)
+
+			require.Len(t, secResp.Secrets, 1)
+			require.NoError(t, secResp.Err)
+			for _, secret := range secResp.Secrets {
+				if u.ID == team_obs.ID {
+					require.Equal(t, fleet.MaskedPassword == secret.Secret, team.ID == teams[0].ID)
+					require.Equal(t, fleet.MaskedPassword != secret.Secret, team.ID == teams[1].ID)
+				}
+
+				if u.ID == team_obs_plus.ID {
+					require.Equal(t, fleet.MaskedPassword == secret.Secret, team.ID == teams[1].ID)
+					require.Equal(t, fleet.MaskedPassword != secret.Secret, team.ID == teams[0].ID)
+				}
+			}
+		}
+	}
+}
+
 func (s *integrationEnterpriseTestSuite) TestExternalIntegrationsTeamConfig() {
 	t := s.T()