Skip to content

Commit

Permalink
Patches for v2 (#716)
Browse files Browse the repository at this point in the history
  • Loading branch information
@lieut-data
lieut-data authored Jul 30, 2024
1 parent 9b59708 commit f079753
Show file tree
Hide file tree
Showing 23 changed files with 958 additions and 19,120 deletions.
16 changes: 4 additions & 12 deletions server/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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)
Expand Down
114 changes: 59 additions & 55 deletions server/api_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -222,7 +221,7 @@ func TestProcessLifecycle(t *testing.T) {
Resource: "mockResource",
ChangeType: "mockChangeType",
ClientState: "mockClientState",
LifecycleEvent: "mockLifecycleEvent",
LifecycleEvent: "reauthorizationRequired",
},
}

Expand All @@ -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{
Expand All @@ -264,7 +280,7 @@ func TestProcessLifecycle(t *testing.T) {
Resource: "mockResource",
ClientState: "webhooksecret",
ChangeType: "mockChangeType",
LifecycleEvent: "mockLifecycleEvent",
LifecycleEvent: "unknownEvent",
},
}

Expand All @@ -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{
Expand All @@ -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)
})
}

Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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)
})
}

Expand Down
12 changes: 6 additions & 6 deletions server/configuration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions server/connect.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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've been invited by your administrator to connect your Mattermost account with Microsoft Teams.", user.Username)
invitePost := &model.Post{
Message: message,
UserId: p.botUserID,
Expand All @@ -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")
}
Expand Down
38 changes: 27 additions & 11 deletions server/handlers.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

import (
"database/sql"
"errors"
"regexp"
"strings"
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 3 additions & 5 deletions server/metrics/dashboards/README.md
View file
Original file line number Diff line number Diff line change
@@ -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)


Loading

0 comments on commit f079753

Please sign in to comment.