MM-52476 Fix guest users access to playbooks. (#23279)

* Fix guest users access to playbooks.

* Fix guest access.

* Add guests list restriction.

server/playbooks/server/api/graphql_root_playbook.go

@@ -63,6 +63,11 @@ func (r *PlaybookRootResolver) Playbooks(ctx context.Context, args struct {
 		}
 	}
 
+	isGuest, err := app.IsGuest(userID, c.api)
+	if err != nil {
+		return nil, err
+	}
+
 	requesterInfo := app.RequesterInfo{
 		UserID:  userID,
 		TeamID:  args.TeamID,
@@ -74,7 +79,7 @@ func (r *PlaybookRootResolver) Playbooks(ctx context.Context, args struct {
 		Direction:          app.SortDirection(args.Direction),
 		SearchTerm:         args.SearchTerm,
 		WithArchived:       args.WithArchived,
-		WithMembershipOnly: args.WithMembershipOnly,
+		WithMembershipOnly: isGuest || args.WithMembershipOnly, // Guests can only see playbooks if they are invited to them
 		Page:               0,
 		PerPage:            10000,
 	}

server/playbooks/server/api/playbooks.go

@@ -345,6 +345,16 @@ func (h *PlaybookHandler) getPlaybooks(c *Context, w http.ResponseWriter, r *htt
 		IsAdmin: app.IsSystemAdmin(userID, h.api),
 	}
 
+	isGuest, err := app.IsGuest(userID, h.api)
+	if err != nil {
+		h.HandleErrorWithCode(w, c.logger, http.StatusForbidden, "", err)
+		return
+	}
+
+	if isGuest {
+		opts.WithMembershipOnly = true
+	}
+
 	playbookResults, err := h.playbookService.GetPlaybooksForTeam(requesterInfo, teamID, opts)
 	if err != nil {
 		h.HandleError(w, c.logger, err)

server/playbooks/server/api_graphql_playbooks_test.go

@@ -607,3 +607,85 @@ func TestGraphQLPlaybooksMetrics(t *testing.T) {
 		require.Len(t, updatedPlaybook.Metrics, 1)
 	})
 }
+
+func gqlTestPlaybookUpdateGuest(e *TestEnvironment, t *testing.T, playbookID string, updates map[string]interface{}) error {
+	testPlaybookMutateQuery := `
+	mutation UpdatePlaybook($id: String!, $updates: PlaybookUpdates!) {
+	updatePlaybook(id: $id, updates: $updates)
+	}
+		`
+	var response graphql.Response
+	err := e.PlaybooksClientGuest.DoGraphql(context.Background(), &client.GraphQLInput{
+		Query:         testPlaybookMutateQuery,
+		OperationName: "UpdatePlaybook",
+		Variables:     map[string]interface{}{"id": playbookID, "updates": updates},
+	}, &response)
+
+	if err != nil {
+		return errors.Wrapf(err, "gqlTestPlaybookUpdate graphql failure")
+	}
+
+	if len(response.Errors) != 0 {
+		return errors.Errorf("gqlTestPlaybookUpdate graphql failure %+v", response.Errors)
+	}
+
+	return err
+}
+
+func TestGraphQLPlaybooksGuests(t *testing.T) {
+	e := Setup(t)
+	e.SetE20Licence()
+	e.CreateBasic()
+	e.CreateGuest()
+
+	t.Run("update playbook guest not member", func(t *testing.T) {
+		err := gqlTestPlaybookUpdateGuest(e, t, e.BasicPlaybook.ID, map[string]interface{}{"title": "mutated"})
+		require.Error(t, err)
+	})
+
+	t.Run("basic get guest not member", func(t *testing.T) {
+		testPlaybookQuery := `
+			query Playbook($id: String!) {
+				playbook(id: $id) {
+					id
+					title
+				}
+			}
+			`
+		var response graphql.Response
+		err := e.PlaybooksClientGuest.DoGraphql(context.Background(), &client.GraphQLInput{
+			Query:         testPlaybookQuery,
+			OperationName: "Playbook",
+			Variables:     map[string]interface{}{"id": e.BasicPlaybook.ID},
+		}, &response)
+		require.NoError(t, err)
+		require.NotZero(t, len(response.Errors))
+	})
+
+	t.Run("list guest", func(t *testing.T) {
+		var pbResultTest struct {
+			Data struct {
+				Playbooks []struct {
+					ID    string
+					Title string
+				}
+			}
+		}
+		testPlaybookQuery := `
+			query Playbooks {
+				playbooks {
+					id
+					title
+				}
+			}
+			`
+		err := e.PlaybooksClientGuest.DoGraphql(context.Background(), &client.GraphQLInput{
+			Query:         testPlaybookQuery,
+			OperationName: "Playbooks",
+		}, &pbResultTest)
+		require.NoError(t, err)
+
+		assert.Len(t, pbResultTest.Data.Playbooks, 0)
+	})
+
+}

server/playbooks/server/api_playbooks_test.go

@@ -1514,3 +1514,30 @@ func TestPlaybookChecklistCleanup(t *testing.T) {
 		require.Equal(t, pb.Checklists, actual)
 	})
 }
+
+func TestPlaybooksGuests(t *testing.T) {
+	e := Setup(t)
+	e.SetE20Licence()
+	e.CreateBasic()
+	e.CreateGuest()
+
+	t.Run("guests can't create playbooks", func(t *testing.T) {
+		_, err := e.PlaybooksClientGuest.Playbooks.Create(context.Background(), client.PlaybookCreateOptions{
+			Title:  "test4",
+			TeamID: e.BasicTeam.Id,
+			Public: false,
+		})
+		assert.Error(t, err)
+	})
+
+	t.Run("get playbook guest", func(t *testing.T) {
+		_, err := e.PlaybooksClientGuest.Playbooks.Get(context.Background(), e.BasicPlaybook.ID)
+		require.Error(t, err)
+	})
+
+	t.Run("update playbook properties", func(t *testing.T) {
+		e.BasicPlaybook.Description = "This is the updated description"
+		err := e.PlaybooksClientGuest.Playbooks.Update(context.Background(), *e.BasicPlaybook)
+		require.Error(t, err)
+	})
+}

server/playbooks/server/app/permissions_service.go

@@ -69,10 +69,13 @@ func (p *PermissionsService) getPlaybookRole(userID string, playbook Playbook) [
 
 	// Public playbooks
 	if playbook.Public {
-		if playbook.DefaultPlaybookMemberRole == "" {
+		// Public playbooks are public to those who can list channels on a team. (Not guests)
-			return []string{playbook.DefaultPlaybookMemberRole}
+		if p.api.HasPermissionToTeam(userID, playbook.TeamID, model.PermissionListTeamChannels) {
+			if playbook.DefaultPlaybookMemberRole == "" {
+				return []string{playbook.DefaultPlaybookMemberRole}
+			}
+			return []string{PlaybookRoleMember}
 		}
-		return []string{PlaybookRoleMember}
 	}
 
 	return []string{}
@@ -103,6 +106,7 @@ func (p *PermissionsService) canViewTeam(userID string, teamID string) bool {
 		return false
 	}
 
+	// This is list team channels so that Guests are excluded.
 	return p.api.HasPermissionToTeam(userID, teamID, model.PermissionViewTeam)
 }
 
@@ -363,9 +367,10 @@ func (p *PermissionsService) PlaybookViewWithPlaybook(userID string, playbook Pl
 		return errors.Wrapf(noAccessErr, "no playbook access; no team view permission for team `%s`", playbook.TeamID)
 	}
 
-	// If the playbook is public team access is enough to view
 	if p.PlaybookIsPublic(playbook) {
-		return nil
+		if p.hasPermissionsToPlaybook(userID, playbook, model.PermissionPublicPlaybookView) {
+			return nil
+		}
 	}
 
 	if p.hasPermissionsToPlaybook(userID, playbook, model.PermissionPrivatePlaybookView) {

server/playbooks/server/main_test.go

@@ -60,6 +60,7 @@ type TestEnvironment struct {
 	PlaybooksClient          *client.Client
 	PlaybooksClient2         *client.Client
 	PlaybooksClientNotInTeam *client.Client
+	PlaybooksClientGuest     *client.Client
 
 	UnauthenticatedPlaybooksClient *client.Client
 
@@ -78,6 +79,7 @@ type TestEnvironment struct {
 	RegularUser              *model.User
 	RegularUser2             *model.User
 	RegularUserNotInTeam     *model.User
+	GuestUser                *model.User
 }
 
 func getEnvWithDefault(name, defaultValue string) string {
@@ -430,6 +432,37 @@ func (e *TestEnvironment) CreateAdditionalPlaybooks() {
 	e.ArchivedPlaybook = archivedPlaybook
 }
 
+func (e *TestEnvironment) CreateGuest() {
+	cfg := e.Srv.Config()
+	cfg.GuestAccountsSettings.Enable = model.NewBool(true)
+	_, _, err := e.ServerAdminClient.UpdateConfig(cfg)
+	require.NoError(e.T, err)
+
+	userPassword := "password123!"
+	guest, appErr := e.A.CreateGuest(request.EmptyContext(e.logger), &model.User{
+		Email:    "playbookguest@example.com",
+		Username: "playbookguest",
+		Password: userPassword,
+	})
+	require.Nil(e.T, appErr)
+	e.GuestUser = guest
+
+	_, _, err = e.ServerAdminClient.AddTeamMember(e.BasicPublicChannel.TeamId, e.GuestUser.Id)
+	require.NoError(e.T, err)
+
+	_, _, err = e.ServerAdminClient.AddChannelMember(e.BasicPublicChannel.Id, e.GuestUser.Id)
+	require.NoError(e.T, err)
+
+	siteURL := fmt.Sprintf("http://localhost:%v", e.A.Srv().ListenAddr.Port)
+	serverClientGuest := model.NewAPIv4Client(siteURL)
+	_, _, err = serverClientGuest.Login(e.GuestUser.Email, userPassword)
+	require.NoError(e.T, err)
+
+	playbooksClientGuest, err := client.New(serverClientGuest)
+	require.NoError(e.T, err)
+	e.PlaybooksClientGuest = playbooksClientGuest
+}
+
 func (e *TestEnvironment) RemoveLicence() {
 	e.Srv.SetLicense(nil)
 }