MM-63791: guest permissions to teams (#30789)

* improve th.CreateGuestAndClient

* test coverage for guest user access to teams

* restrict guest access to public teams unless a member

server/channels/api4/apitestlib.go

@@ -695,7 +695,8 @@ func (th *TestHelper) CreateUserWithAuth(authService string) *model.User {
 // CreateGuestAndClient creates a guest user, adds them to the basic
 // team, basic channel and basic private channel, and generates an API
 // client ready to use
-func (th *TestHelper) CreateGuestAndClient() (*model.User, *model.Client4) {
+func (th *TestHelper) CreateGuestAndClient(tb testing.TB) (*model.User, *model.Client4) {
+	tb.Helper()
 	id := model.NewId()
 
 	// create a guest user and add it to the basic team and public/private channels
@@ -706,23 +707,17 @@ func (th *TestHelper) CreateGuestAndClient() (*model.User, *model.Client4) {
 		Password:      "Password1",
 		EmailVerified: true,
 	})
-	if cgErr != nil {
+	require.Nil(tb, cgErr)
-		panic(cgErr)
-	}
 
-	_, _, tErr := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, guest.Id, th.SystemAdminUser.Id)
+	_, _, appErr := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, guest.Id, th.SystemAdminUser.Id)
-	if tErr != nil {
+	require.Nil(tb, appErr)
-		panic(tErr)
-	}
 	th.AddUserToChannel(guest, th.BasicChannel)
 	th.AddUserToChannel(guest, th.BasicPrivateChannel)
 
 	// create a client and login the guest
 	guestClient := th.CreateClient()
-	_, _, lErr := guestClient.Login(context.Background(), guest.Username, "Password1")
+	_, _, err := guestClient.Login(context.Background(), guest.Email, "Password1")
-	if lErr != nil {
+	require.NoError(tb, err)
-		panic(lErr)
-	}
 
 	return guest, guestClient
 }

server/channels/api4/channel_bookmark_test.go

@@ -41,7 +41,7 @@ func TestCreateChannelBookmark(t *testing.T) {
 	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GuestAccountsSettings.Enable = true })
 	th.App.Srv().SetLicense(model.NewTestLicense())
 
-	guest, guestClient := th.CreateGuestAndClient()
+	guest, guestClient := th.CreateGuestAndClient(t)
 
 	t.Run("a user should be able to create a channel bookmark in a public channel", func(t *testing.T) {
 		channelBookmark := &model.ChannelBookmark{
@@ -297,7 +297,7 @@ func TestEditChannelBookmark(t *testing.T) {
 	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GuestAccountsSettings.Enable = true })
 	th.App.Srv().SetLicense(model.NewTestLicense())
 
-	guest, guestClient := th.CreateGuestAndClient()
+	guest, guestClient := th.CreateGuestAndClient(t)
 
 	t.Run("a user editing a channel bookmark in public and private channels", func(t *testing.T) {
 		testCases := []struct {
@@ -732,7 +732,7 @@ func TestUpdateChannelBookmarkSortOrder(t *testing.T) {
 	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GuestAccountsSettings.Enable = true })
 	th.App.Srv().SetLicense(model.NewTestLicense())
 
-	guest, guestClient := th.CreateGuestAndClient()
+	guest, guestClient := th.CreateGuestAndClient(t)
 
 	t.Run("a user updating a bookmark's order in public and private channels", func(t *testing.T) {
 		testCases := []struct {
@@ -1121,7 +1121,7 @@ func TestDeleteChannelBookmark(t *testing.T) {
 	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GuestAccountsSettings.Enable = true })
 	th.App.Srv().SetLicense(model.NewTestLicense())
 
-	guest, guestClient := th.CreateGuestAndClient()
+	guest, guestClient := th.CreateGuestAndClient(t)
 
 	t.Run("a user deleting bookmarks in public and private channels", func(t *testing.T) {
 		testCases := []struct {
@@ -1494,7 +1494,7 @@ func TestListChannelBookmarksForChannel(t *testing.T) {
 	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GuestAccountsSettings.Enable = true })
 	th.App.Srv().SetLicense(model.NewTestLicense())
 
-	guest, guestClient := th.CreateGuestAndClient()
+	guest, guestClient := th.CreateGuestAndClient(t)
 
 	publicBookmark1 := createBookmark("one", th.BasicChannel.Id)
 	publicBookmark2 := createBookmark("two", th.BasicChannel.Id)

server/channels/api4/channel_test.go

@@ -4102,7 +4102,7 @@ func TestAddChannelMemberGuestAccessControl(t *testing.T) {
 	th.App.Srv().SetLicense(model.NewTestLicense())
 
 	// Create a guest user
-	guest, guestClient := th.CreateGuestAndClient()
+	guest, guestClient := th.CreateGuestAndClient(t)
 
 	// Create a public channel to which the guest doesn't belong
 	publicChannel := th.CreatePublicChannel()

server/channels/api4/team.go

@@ -151,7 +151,16 @@ func getTeam(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if (!team.AllowOpenInvite || team.Type != model.TeamOpen) && !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), team.Id, model.PermissionViewTeam) {
+	isPublicTeam := team.AllowOpenInvite && team.Type == model.TeamOpen
+	hasPermissionViewTeam := c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), team.Id, model.PermissionViewTeam)
+
+	if !isPublicTeam && !hasPermissionViewTeam {
+		c.SetPermissionError(model.PermissionViewTeam)
+		return
+	}
+
+	if isPublicTeam && !hasPermissionViewTeam && !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionListPublicTeams) {
+		// Fail with PermissionViewTeam, not PermissionListPublicTeams.
 		c.SetPermissionError(model.PermissionViewTeam)
 		return
 	}

server/channels/api4/team_guest_test.go

@@ -0,0 +1,99 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package api4
+
+import (
+	"context"
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/mattermost/mattermost/server/public/model"
+)
+
+func TestGetTeamAsGuest(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterprise))
+	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GuestAccountsSettings.Enable = true })
+
+	guest, guestClient := th.CreateGuestAndClient(t)
+
+	publicTeamNotAMember := &model.Team{
+		DisplayName:     "Public Team (guest is not a member)",
+		Name:            GenerateTestTeamName(),
+		Email:           th.GenerateTestEmail(),
+		Type:            model.TeamOpen,
+		AllowOpenInvite: true,
+	}
+	publicTeamNotAMember, _, err := th.SystemAdminClient.CreateTeam(context.Background(), publicTeamNotAMember)
+	require.NoError(t, err)
+
+	publicTeamIsAMember := &model.Team{
+		DisplayName:     "Public Team (guest is a member)",
+		Name:            GenerateTestTeamName(),
+		Email:           th.GenerateTestEmail(),
+		Type:            model.TeamOpen,
+		AllowOpenInvite: true,
+	}
+	publicTeamIsAMember, _, err = th.SystemAdminClient.CreateTeam(context.Background(), publicTeamIsAMember)
+	require.NoError(t, err)
+
+	_, _, err = th.SystemAdminClient.AddTeamMember(context.Background(), publicTeamIsAMember.Id, guest.Id)
+	require.NoError(t, err)
+
+	privateTeamNotAMember := &model.Team{
+		DisplayName:     "Private Team (guest is not a member)",
+		Name:            GenerateTestTeamName(),
+		Email:           th.GenerateTestEmail(),
+		Type:            model.TeamInvite,
+		AllowOpenInvite: false,
+	}
+	privateTeamNotAMember, _, err = th.SystemAdminClient.CreateTeam(context.Background(), privateTeamNotAMember)
+	require.NoError(t, err)
+
+	privateTeamIsAMember := &model.Team{
+		DisplayName:     "Private Team (guest is not a member)",
+		Name:            GenerateTestTeamName(),
+		Email:           th.GenerateTestEmail(),
+		Type:            model.TeamInvite,
+		AllowOpenInvite: false,
+	}
+	privateTeamIsAMember, _, err = th.SystemAdminClient.CreateTeam(context.Background(), privateTeamIsAMember)
+	require.NoError(t, err)
+
+	_, _, err = th.SystemAdminClient.AddTeamMember(context.Background(), privateTeamIsAMember.Id, guest.Id)
+	require.NoError(t, err)
+
+	t.Run("guest cannot view public team they are not a member of", func(t *testing.T) {
+		team, resp, err := guestClient.GetTeam(context.Background(), publicTeamNotAMember.Id, "")
+		require.Error(t, err)
+		require.Equal(t, http.StatusForbidden, resp.StatusCode)
+		assert.Nil(t, team)
+	})
+
+	t.Run("guest can view public team they are a member of", func(t *testing.T) {
+		team, resp, err := guestClient.GetTeam(context.Background(), publicTeamIsAMember.Id, "")
+		require.NoError(t, err)
+		require.Equal(t, http.StatusOK, resp.StatusCode)
+		assert.Equal(t, publicTeamIsAMember.Id, team.Id)
+	})
+
+	t.Run("guest can not view private team they are not a member of", func(t *testing.T) {
+		team, resp, err := guestClient.GetTeam(context.Background(), privateTeamNotAMember.Id, "")
+		require.Error(t, err)
+		require.Equal(t, http.StatusForbidden, resp.StatusCode)
+		assert.Nil(t, team)
+	})
+
+	t.Run("guest can view private team they are a member of", func(t *testing.T) {
+		team, resp, err := guestClient.GetTeam(context.Background(), privateTeamIsAMember.Id, "")
+		require.NoError(t, err)
+		require.Equal(t, http.StatusOK, resp.StatusCode)
+		assert.Equal(t, privateTeamIsAMember.Id, team.Id)
+	})
+}