Patches for v2

server/api.go

@@ -86,8 +86,8 @@ func NewAPI(p *Plugin, store store.Store) *API {
 
 // returnJSON writes the given data as json with the provided httpStatus
 func (a *API) returnJSON(w http.ResponseWriter, data any) {
-	w.WriteHeader(http.StatusOK)
 	w.Header().Set("Content-Type", "application/json")
+	w.WriteHeader(http.StatusOK)
 
 	if err := json.NewEncoder(w).Encode(data); err != nil {
 		a.p.API.LogWarn("Failed to write to http.ResponseWriter", "error", err.Error())
@@ -879,31 +879,23 @@ func (a *API) siteStats(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "unable to get whitelisted users count", http.StatusInternalServerError)
 		return
 	}
-	receiving, err := a.p.store.GetActiveUsersReceivingCount(metricsActiveUsersRange)
+	totalActiveUsers, err := a.p.store.GetActiveUsersCount(metricsActiveUsersRange)
 	if err != nil {
 		a.p.API.LogWarn("Failed to get users receiving count", "error", err.Error())
 		http.Error(w, "unable to get users receiving count", http.StatusInternalServerError)
 		return
 	}
-	sending, err := a.p.store.GetActiveUsersSendingCount(metricsActiveUsersRange)
-	if err != nil {
-		a.p.API.LogWarn("Failed to get sending users count", "error", err.Error())
-		http.Error(w, "unable to get sending users count", http.StatusInternalServerError)
-		return
-	}
 
 	siteStats := struct {
 		TotalConnectedUsers   int64 `json:"total_connected_users"`
 		PendingInvitedUsers   int64 `json:"pending_invited_users"`
 		CurrentWhitelistUsers int64 `json:"current_whitelist_users"`
-		UserReceivingMessages int64 `json:"total_users_receiving"`
-		UserSendingMessages   int64 `json:"total_users_sending"`
+		TotalActiveUsers      int64 `json:"total_active_users"`
 	}{
 		TotalConnectedUsers:   connectedUsersCount,
 		PendingInvitedUsers:   int64(pendingInvites),
 		CurrentWhitelistUsers: int64(whitelistedUsers),
-		UserReceivingMessages: receiving,
-		UserSendingMessages:   sending,
+		TotalActiveUsers:      totalActiveUsers,
 	}
 
 	a.returnJSON(w, siteStats)

server/api_test.go

@@ -154,7 +154,6 @@ func TestProcessActivity(t *testing.T) {
 func TestProcessLifecycle(t *testing.T) {
 	th := setupTestHelper(t)
 	apiURL := th.pluginURL(t, "lifecycle")
-	team := th.SetupTeam(t)
 
 	sendRequest := func(t *testing.T, activities []msteams.Activity) (*http.Response, string) {
 		t.Helper()
@@ -222,7 +221,7 @@ func TestProcessLifecycle(t *testing.T) {
 				Resource:       "mockResource",
 				ChangeType:     "mockChangeType",
 				ClientState:    "mockClientState",
-				LifecycleEvent: "mockLifecycleEvent",
+				LifecycleEvent: "reauthorizationRequired",
 			},
 		}
 
@@ -231,31 +230,48 @@ func TestProcessLifecycle(t *testing.T) {
 		assert.Equal(t, "Invalid webhook secret\n", bodyString)
 	})
 
-	t.Run("valid event, no refresh needed", func(t *testing.T) {
+	t.Run("valid payload, unknown subscription", func(t *testing.T) {
 		th.Reset(t)
 
-		channel := th.SetupPublicChannel(t, team)
+		activities := []msteams.Activity{
+			{
+				SubscriptionID: model.NewId(),
+				Resource:       "mockResource",
+				ClientState:    "webhooksecret",
+				ChangeType:     "mockChangeType",
+				LifecycleEvent: "reauthorizationRequired",
+			},
+		}
 
-		subscription := storemodels.ChannelSubscription{
+		response, bodyString := sendRequest(t, activities)
+		assert.Equal(t, http.StatusOK, response.StatusCode)
+		assert.Empty(t, bodyString)
+
+		assert.Eventually(t, func() bool {
+			return th.getRelativeCounter(t,
+				"msteams_connect_events_lifecycle_events_total",
+				withLabel("event_type", "reauthorizationRequired"),
+				withLabel("discarded_reason", metrics.DiscardedReasonUnusedSubscription),
+			) == 1
+		}, 5*time.Second, 500*time.Millisecond)
+		assert.Never(t, func() bool {
+			return th.getRelativeCounter(t,
+				"msteams_connect_events_subscriptions_total",
+				withLabel("action", metrics.SubscriptionRefreshed),
+			) > 0
+		}, 5*time.Second, 500*time.Millisecond)
+	})
+
+	t.Run("valid payload, unknown event", func(t *testing.T) {
+		th.Reset(t)
+
+		subscription := storemodels.GlobalSubscription{
 			SubscriptionID: model.NewId(),
-			TeamID:         model.NewId(),
-			ChannelID:      model.NewId(),
+			Type:           "allChats",
 			ExpiresOn:      time.Now().Add(10 * time.Minute),
 			Secret:         th.p.getConfiguration().WebhookSecret,
 		}
-		err := th.p.GetStore().SaveChannelSubscription(subscription)
-		require.NoError(t, err)
-
-		link := &storemodels.ChannelLink{
-			MattermostTeamID:      channel.TeamId,
-			MattermostTeamName:    team.Name,
-			MattermostChannelID:   channel.Id,
-			MattermostChannelName: channel.Name,
-			MSTeamsTeam:           subscription.TeamID,
-			MSTeamsChannel:        subscription.ChannelID,
-			Creator:               "creator_id",
-		}
-		err = th.p.GetStore().StoreChannelLink(link)
+		err := th.p.GetStore().SaveGlobalSubscription(subscription)
 		require.NoError(t, err)
 
 		activities := []msteams.Activity{
@@ -264,7 +280,7 @@ func TestProcessLifecycle(t *testing.T) {
 				Resource:       "mockResource",
 				ClientState:    "webhooksecret",
 				ChangeType:     "mockChangeType",
-				LifecycleEvent: "mockLifecycleEvent",
+				LifecycleEvent: "unknownEvent",
 			},
 		}
 
@@ -275,37 +291,28 @@ func TestProcessLifecycle(t *testing.T) {
 		assert.Eventually(t, func() bool {
 			return th.getRelativeCounter(t,
 				"msteams_connect_events_lifecycle_events_total",
-				withLabel("event_type", "mockLifecycleEvent"),
-				withLabel("discarded_reason", metrics.DiscardedReasonNone),
+				withLabel("event_type", "unknownEvent"),
+				withLabel("discarded_reason", metrics.DiscardedReasonUnknownLifecycleEvent),
 			) == 1
 		}, 5*time.Second, 500*time.Millisecond)
+		assert.Never(t, func() bool {
+			return th.getRelativeCounter(t,
+				"msteams_connect_events_subscriptions_total",
+				withLabel("action", metrics.SubscriptionRefreshed),
+			) > 0
+		}, 5*time.Second, 500*time.Millisecond)
 	})
 
 	t.Run("valid event, refresh needed", func(t *testing.T) {
 		th.Reset(t)
 
-		channel := th.SetupPublicChannel(t, team)
-
-		subscription := storemodels.ChannelSubscription{
+		subscription := storemodels.GlobalSubscription{
 			SubscriptionID: model.NewId(),
-			TeamID:         model.NewId(),
-			ChannelID:      model.NewId(),
+			Type:           "allChats",
 			ExpiresOn:      time.Now().Add(10 * time.Minute),
 			Secret:         th.p.getConfiguration().WebhookSecret,
 		}
-		err := th.p.GetStore().SaveChannelSubscription(subscription)
-		require.NoError(t, err)
-
-		link := &storemodels.ChannelLink{
-			MattermostTeamID:      channel.TeamId,
-			MattermostTeamName:    team.Name,
-			MattermostChannelID:   channel.Id,
-			MattermostChannelName: channel.Name,
-			MSTeamsTeam:           subscription.TeamID,
-			MSTeamsChannel:        subscription.ChannelID,
-			Creator:               "creator_id",
-		}
-		err = th.p.GetStore().StoreChannelLink(link)
+		err := th.p.GetStore().SaveGlobalSubscription(subscription)
 		require.NoError(t, err)
 
 		activities := []msteams.Activity{
@@ -332,6 +339,12 @@ func TestProcessLifecycle(t *testing.T) {
 				withLabel("discarded_reason", metrics.DiscardedReasonNone),
 			) == 1
 		}, 5*time.Second, 500*time.Millisecond)
+		assert.Eventually(t, func() bool {
+			return th.getRelativeCounter(t,
+				"msteams_connect_events_subscriptions_total",
+				withLabel("action", metrics.SubscriptionRefreshed),
+			) == 1
+		}, 5*time.Second, 500*time.Millisecond)
 	})
 }
 
@@ -935,7 +948,7 @@ func TestGetSiteStats(t *testing.T) {
 
 		response, bodyString := sendRequest(t, sysadmin)
 		assert.Equal(t, http.StatusOK, response.StatusCode)
-		assert.JSONEq(t, `{"current_whitelist_users":0, "pending_invited_users":0, "total_connected_users":0, "total_users_receiving":0, "total_users_sending":0}`, bodyString)
+		assert.JSONEq(t, `{"current_whitelist_users":0, "pending_invited_users":0, "total_connected_users":0, "total_active_users":0}`, bodyString)
 	})
 
 	t.Run("1 connected user", func(t *testing.T) {
@@ -945,14 +958,12 @@ func TestGetSiteStats(t *testing.T) {
 		user1 := th.SetupUser(t, team)
 		th.ConnectUser(t, user1.Id)
 
-		err := th.p.store.SetUserLastChatSentAt(user1.Id, time.Now().Add(-3*24*time.Hour).UnixMicro())
-		require.NoError(t, err)
-		err = th.p.store.SetUserLastChatReceivedAt(user1.Id, time.Now().Add(-4*24*time.Hour).UnixMicro())
+		err := th.p.store.SetUserLastChatReceivedAt(user1.Id, time.Now().Add(-4*24*time.Hour).UnixMicro())
 		require.NoError(t, err)
 
 		response, bodyString := sendRequest(t, sysadmin)
 		assert.Equal(t, http.StatusOK, response.StatusCode)
-		assert.JSONEq(t, `{"current_whitelist_users":0, "pending_invited_users":0, "total_connected_users":1,"total_users_receiving":1, "total_users_sending":1}`, bodyString)
+		assert.JSONEq(t, `{"current_whitelist_users":0, "pending_invited_users":0, "total_connected_users":1,"total_active_users":1}`, bodyString)
 	})
 
 	t.Run("1 invited user, 2 whitelisted users", func(t *testing.T) {
@@ -969,7 +980,7 @@ func TestGetSiteStats(t *testing.T) {
 
 		response, bodyString := sendRequest(t, sysadmin)
 		assert.Equal(t, http.StatusOK, response.StatusCode)
-		assert.JSONEq(t, `{"current_whitelist_users":2, "pending_invited_users":1, "total_connected_users":0,"total_users_receiving":0, "total_users_sending":0}`, bodyString)
+		assert.JSONEq(t, `{"current_whitelist_users":2, "pending_invited_users":1, "total_connected_users":0,"total_active_users":0}`, bodyString)
 	})
 
 	t.Run("10 connected users", func(t *testing.T) {
@@ -987,18 +998,11 @@ func TestGetSiteStats(t *testing.T) {
 				err := th.p.store.SetUserLastChatReceivedAt(user.Id, time.Now().Add(-8*24*time.Hour).UnixMicro())
 				require.NoError(t, err)
 			}
-			if i < 2 {
-				err := th.p.store.SetUserLastChatSentAt(user.Id, time.Now().Add(-3*24*time.Hour).UnixMicro())
-				require.NoError(t, err)
-			} else {
-				err := th.p.store.SetUserLastChatSentAt(user.Id, time.Now().Add(-8*24*time.Hour).UnixMicro())
-				require.NoError(t, err)
-			}
 		}
 
 		response, bodyString := sendRequest(t, sysadmin)
 		assert.Equal(t, http.StatusOK, response.StatusCode)
-		assert.JSONEq(t, `{"current_whitelist_users":0, "pending_invited_users":0, "total_connected_users":10,"total_users_receiving":5, "total_users_sending":2}`, bodyString)
+		assert.JSONEq(t, `{"current_whitelist_users":0, "pending_invited_users":0, "total_connected_users":10,"total_active_users":5}`, bodyString)
 	})
 }
 

server/configuration.go

@@ -40,6 +40,12 @@ func (c *configuration) ProcessConfiguration() {
 	c.ClientSecret = strings.TrimSpace(c.ClientSecret)
 	c.EncryptionKey = strings.TrimSpace(c.EncryptionKey)
 	c.WebhookSecret = strings.TrimSpace(c.WebhookSecret)
+	if c.MaxSizeForCompleteDownload < 0 {
+		c.MaxSizeForCompleteDownload = 0
+	}
+	if c.BufferSizeForFileStreaming <= 0 {
+		c.BufferSizeForFileStreaming = 20
+	}
 }
 
 func (p *Plugin) validateConfiguration(configuration *configuration) error {
@@ -59,12 +65,6 @@ func (p *Plugin) validateConfiguration(configuration *configuration) error {
 	if configuration.WebhookSecret == "" {
 		return errors.New("webhook secret should not be empty")
 	}
-	if configuration.MaxSizeForCompleteDownload < 0 {
-		return errors.New("max size for complete single download should not be negative")
-	}
-	if configuration.BufferSizeForFileStreaming <= 0 {
-		return errors.New("buffer size for file streaming should be greater than zero")
-	}
 
 	return nil
 }

server/connect.go

@@ -86,7 +86,7 @@ func (p *Plugin) SendInviteMessage(user *model.User, pendingSince time.Time, cur
 		return errors.Wrapf(err, "failed to get bot DM channel with user_id %s", user.Id)
 	}
 
-	message := fmt.Sprintf("@%s, you’re invited to use the Microsoft Teams connected experience for Mattermost. ", user.Username)
+	message := fmt.Sprintf("@%s, you're been invited by your administrator to connect your Mattermost account with Microsoft Teams.", user.Username)
 	invitePost := &model.Post{
 		Message:   message,
 		UserId:    p.botUserID,
@@ -97,7 +97,7 @@ func (p *Plugin) SendInviteMessage(user *model.User, pendingSince time.Time, cur
 	}
 
 	connectURL := fmt.Sprintf(p.GetURL()+"/connect?post_id=%s&channel_id=%s", invitePost.Id, channel.Id)
-	invitePost.Message = fmt.Sprintf("%s [Click here to activate the integration in a minute or less](%s).", invitePost.Message, connectURL)
+	invitePost.Message = fmt.Sprintf("%s [Click here to connect your account](%s).", invitePost.Message, connectURL)
 	if err := p.apiClient.Post.UpdatePost(invitePost); err != nil {
 		return errors.Wrapf(err, "error sending bot message")
 	}

server/handlers.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"database/sql"
 	"errors"
 	"regexp"
 	"strings"
@@ -141,18 +142,33 @@ func (ah *ActivityHandler) Handle(activity msteams.Activity) error {
 }
 
 func (ah *ActivityHandler) HandleLifecycleEvent(event msteams.Activity) {
-	if event.LifecycleEvent == "reauthorizationRequired" {
-		expiresOn, err := ah.plugin.GetClientForApp().RefreshSubscription(event.SubscriptionID)
-		if err != nil {
-			ah.plugin.GetAPI().LogWarn("Unable to refresh the subscription", "error", err.Error())
-			ah.plugin.GetMetrics().ObserveLifecycleEvent(event.LifecycleEvent, metrics.DiscardedReasonFailedToRefresh)
-			return
-		}
+	if event.LifecycleEvent != "reauthorizationRequired" {
+		ah.plugin.GetMetrics().ObserveLifecycleEvent(event.LifecycleEvent, metrics.DiscardedReasonUnknownLifecycleEvent)
+		return
+	}
 
-		ah.plugin.GetMetrics().ObserveSubscription(metrics.SubscriptionRefreshed)
-		if err = ah.plugin.GetStore().UpdateSubscriptionExpiresOn(event.SubscriptionID, *expiresOn); err != nil {
-			ah.plugin.GetAPI().LogWarn("Unable to store the subscription new expiry date", "subscription_id", event.SubscriptionID, "error", err.Error())
-		}
+	// Ignore subscriptions we aren't tracking locally. For now, that's just the single global chats subscription.
+	if _, err := ah.plugin.GetStore().GetGlobalSubscription(event.SubscriptionID); err == sql.ErrNoRows {
+		ah.plugin.GetAPI().LogWarn("Ignoring reauthorizationRequired lifecycle event for unused subscription", "subscription_id", event.SubscriptionID)
+		ah.plugin.GetMetrics().ObserveLifecycleEvent(event.LifecycleEvent, metrics.DiscardedReasonUnusedSubscription)
+		return
+	} else if err != nil {
+		ah.plugin.GetAPI().LogWarn("Failed to lookup subscription, refreshing anyway", "subscription_id", event.SubscriptionID, "error", err.Error())
+	}
+
+	ah.plugin.GetAPI().LogWarn("Refreshing subscription", "subscription_id", event.SubscriptionID)
+	expiresOn, err := ah.plugin.GetClientForApp().RefreshSubscription(event.SubscriptionID)
+	if err != nil {
+		ah.plugin.GetAPI().LogWarn("Unable to refresh the subscription", "subscription_id", event.SubscriptionID, "error", err.Error())
+		ah.plugin.GetMetrics().ObserveLifecycleEvent(event.LifecycleEvent, metrics.DiscardedReasonFailedToRefresh)
+		return
+	}
+
+	ah.plugin.GetAPI().LogWarn("Refreshed subscription", "subscription_id", event.SubscriptionID, "expires_on", expiresOn.Format("2006-01-02 15:04:05.000 Z07:00"))
+	ah.plugin.GetMetrics().ObserveSubscription(metrics.SubscriptionRefreshed)
+
+	if err = ah.plugin.GetStore().UpdateSubscriptionExpiresOn(event.SubscriptionID, *expiresOn); err != nil {
+		ah.plugin.GetAPI().LogWarn("Unable to store the subscription new expiry date", "subscription_id", event.SubscriptionID, "error", err.Error())
 	}
 
 	ah.plugin.GetMetrics().ObserveLifecycleEvent(event.LifecycleEvent, metrics.DiscardedReasonNone)

server/metrics/dashboards/README.md

@@ -1,11 +1,9 @@
 ### Grafana Dashboards
 
-This folder contains Grafana dashboards for use with [performance metrics](https://docs.mattermost.com/scale/performance-monitoring.html) and the MS Teams Connect plugin.
+This folder contains Grafana dashboards for use with [performance metrics](https://docs.mattermost.com/scale/performance-monitoring.html) and the MS Teams plugin.
 
-#### [cloud.json](cloud.json)
+#### [dashboard.json](dashboard.json)
 
-This dashboard is designed for use in Mattermost Cloud, filtering to a given `namespace`. Modifications will be necessary for use in self-managed environments, but the overall graphs are nevertheless a useful starting point.
-
-![cloud dashboard](cloud.png)
+![dashboard](dashboard.png)