Monitor permissions (#714)

* refactor client API to return whole app

* monitor for expected permissions

* MM-59549: monitor permissions

To aid in debugging deployments, let's periodically check the
application permissions and log those missing or redundant.

Fixes: https://mattermost.atlassian.net/browse/MM-59549

server/credentials.go

@@ -1,14 +1,95 @@
 package main
 
 import (
+	"fmt"
 	"runtime/debug"
 	"sort"
 	"strings"
 	"time"
 
 	"github.com/mattermost/mattermost-plugin-msteams/server/metrics"
+	"github.com/mattermost/mattermost-plugin-msteams/server/msteams/clientmodels"
 )
 
+const (
+	ResourceAccessTypeScope = "Scope"
+	ResourceAccessTypeRole  = "Role"
+)
+
+type expectedPermission struct {
+	Name           string
+	ResourceAccess clientmodels.ResourceAccess
+}
+
+// getResourceAccessKey makes a map key for the resource access that simplifies checking
+// for the resource access in question. (Technically, we could use the struct itself, but
+// this insulates us from unexpected upstream changes.)
+func getResourceAccessKey(resourceAccess clientmodels.ResourceAccess) string {
+	return fmt.Sprintf("%s+%s", resourceAccess.ID, resourceAccess.Type)
+}
+
+// describeResourceAccessType annotates the resource access type with the user facing term
+// shown in the Azure Tenant UI (Application vs. Delegated).
+func describeResourceAccessType(resourceAccess clientmodels.ResourceAccess) string {
+	switch resourceAccess.Type {
+	case ResourceAccessTypeRole:
+		return "Role (Application)"
+	case ResourceAccessTypeScope:
+		return "Scope (Delegated)"
+	default:
+		return resourceAccess.Type
+	}
+}
+
+// getExpectedPermissions returns the set of expected permissions, keyed by the
+// name the enduser would expect to see in the Azure tenant.
+func getExpectedPermissions() []expectedPermission {
+	return []expectedPermission{
+		{
+			Name: "https://graph.microsoft.com/Chat.Read",
+			ResourceAccess: clientmodels.ResourceAccess{
+				ID:   "f501c180-9344-439a-bca0-6cbf209fd270",
+				Type: "Scope",
+			},
+		},
+		{
+			Name: "https://graph.microsoft.com/ChatMessage.Read",
+			ResourceAccess: clientmodels.ResourceAccess{
+				ID:   "cdcdac3a-fd45-410d-83ef-554db620e5c7",
+				Type: "Scope",
+			},
+		},
+		{
+			Name: "https://graph.microsoft.com/Files.Read.All",
+			ResourceAccess: clientmodels.ResourceAccess{
+				ID:   "df85f4d6-205c-4ac5-a5ea-6bf408dba283",
+				Type: "Scope",
+			},
+		},
+		{
+			Name: "https://graph.microsoft.com/offline_access",
+			ResourceAccess: clientmodels.ResourceAccess{
+				ID:   "7427e0e9-2fba-42fe-b0c0-848c9e6a8182",
+				Type: "Scope",
+			},
+		},
+		{
+			Name: "https://graph.microsoft.com/User.Read",
+			ResourceAccess: clientmodels.ResourceAccess{
+				ID:   "e1fe6dd8-ba31-4d61-89e7-88639da4683d",
+				Type: "Scope",
+			},
+		},
+		{
+			Name: "https://graph.microsoft.com/Chat.Read.All",
+			ResourceAccess: clientmodels.ResourceAccess{
+				ID:   "6b7d71aa-70aa-4810-a8d9-5d9fb2830017",
+				Type: "Role",
+			},
+		},
+	}
+}
+
 func (p *Plugin) checkCredentials() {
 	defer func() {
 		if r := recover(); r != nil {
@@ -22,12 +103,14 @@ func (p *Plugin) checkCredentials() {
 
 	p.API.LogInfo("Running the check credentials job")
 
-	credentials, err := p.GetClientForApp().GetAppCredentials(p.getConfiguration().ClientID)
+	app, err := p.GetClientForApp().GetApp(p.getConfiguration().ClientID)
 	if err != nil {
 		p.API.LogWarn("Failed to get app credentials", "error", err.Error())
 		return
 	}
 
+	credentials := app.Credentials
+
 	// We sort by earliest end date to cover the unlikely event we encounter two credentials
 	// with the same hint when reporting the single metric below.
 	sort.SliceStable(credentials, func(i, j int) bool {
@@ -59,4 +142,63 @@ func (p *Plugin) checkCredentials() {
 		p.API.LogWarn("Failed to find credential matching configuration")
 		p.GetMetrics().ObserveClientSecretEndDateTime(time.Time{})
 	}
+
+	missingPermissions, redundantResourceAccess := p.checkPermissions(app)
+	for _, permission := range missingPermissions {
+		p.API.LogWarn(
+			"Application missing required API Permission",
+			"permission", permission.Name,
+			"resource_id", permission.ResourceAccess.ID,
+			"type", describeResourceAccessType(permission.ResourceAccess),
+			"application_id", p.getConfiguration().ClientID,
+		)
+	}
+
+	for _, resourceAccess := range redundantResourceAccess {
+		p.API.LogWarn(
+			"Application has redundant API Permission",
+			"resource_id", resourceAccess.ID,
+			"type", describeResourceAccessType(resourceAccess),
+			"application_id", p.getConfiguration().ClientID,
+		)
+	}
+}
+
+func (p *Plugin) checkPermissions(app *clientmodels.App) ([]expectedPermission, []clientmodels.ResourceAccess) {
+	// Build a map and log what we find at the same time.
+	actualRequiredResources := make(map[string]clientmodels.ResourceAccess)
+	for _, requiredResource := range app.RequiredResources {
+		actualRequiredResources[getResourceAccessKey(requiredResource)] = requiredResource
+		p.API.LogDebug(
+			"Found API Permission",
+			"resource_id", requiredResource.ID,
+			"type", describeResourceAccessType(requiredResource),
+			"application_id", p.getConfiguration().ClientID,
+		)
+	}
+
+	expectedPermissions := getExpectedPermissions()
+	expectedPermissionsMap := make(map[string]expectedPermission, len(expectedPermissions))
+	for _, expectedPermission := range expectedPermissions {
+		expectedPermissionsMap[getResourceAccessKey(expectedPermission.ResourceAccess)] = expectedPermission
+	}
+
+	var missing []expectedPermission
+	var redundant []clientmodels.ResourceAccess
+
+	// Verify all expected permissions are present.
+	for _, permission := range expectedPermissions {
+		if _, ok := actualRequiredResources[getResourceAccessKey(permission.ResourceAccess)]; !ok {
+			missing = append(missing, permission)
+		}
+	}
+
+	// Check for unnecessary permissions.
+	for _, requiredResource := range app.RequiredResources {
+		if _, ok := expectedPermissionsMap[getResourceAccessKey(requiredResource)]; !ok {
+			redundant = append(redundant, requiredResource)
+		}
+	}
+
+	return missing, redundant
 }

server/credentials_test.go

@@ -0,0 +1,95 @@
+package main
+
+import (
+	"testing"
+
+	"github.com/mattermost/mattermost-plugin-msteams/server/msteams/clientmodels"
+	"github.com/mattermost/mattermost/server/public/model"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestDescribeResourceAccessType(t *testing.T) {
+	assert.Equal(t, "Scope (Delegated)", describeResourceAccessType(clientmodels.ResourceAccess{
+		ID:   model.NewId(),
+		Type: ResourceAccessTypeScope,
+	}))
+	assert.Equal(t, "Role (Application)", describeResourceAccessType(clientmodels.ResourceAccess{
+		ID:   model.NewId(),
+		Type: ResourceAccessTypeRole,
+	}))
+	assert.Equal(t, "Unknown", describeResourceAccessType(clientmodels.ResourceAccess{
+		ID:   model.NewId(),
+		Type: "Unknown",
+	}))
+}
+
+func TestCheckPermissions(t *testing.T) {
+	th := setupTestHelper(t)
+
+	t.Run("no permissions", func(t *testing.T) {
+		th.Reset(t)
+
+		var app clientmodels.App
+
+		missing, redundant := th.p.checkPermissions(&app)
+
+		assert.Equal(t, getExpectedPermissions(), missing)
+		assert.Empty(t, redundant)
+	})
+
+	t.Run("missing and redundant permissions", func(t *testing.T) {
+		th.Reset(t)
+
+		var app clientmodels.App
+
+		var changedResourceAccess clientmodels.ResourceAccess
+		for i, expectedPermission := range getExpectedPermissions() {
+			if i == 0 {
+				// Skip the first permission altogether
+				continue
+			} else if i == 1 {
+				// Change the type of the second permission
+				changedResourceAccess = expectedPermission.ResourceAccess
+				if changedResourceAccess.Type == ResourceAccessTypeScope {
+					changedResourceAccess.Type = ResourceAccessTypeRole
+				} else {
+					changedResourceAccess.Type = ResourceAccessTypeScope
+				}
+				app.RequiredResources = append(app.RequiredResources, changedResourceAccess)
+			} else {
+				app.RequiredResources = append(app.RequiredResources, expectedPermission.ResourceAccess)
+			}
+		}
+
+		// Add an extra permission beyond the changed one above.
+		extraResourceAccess := clientmodels.ResourceAccess{
+			ID:   model.NewId(),
+			Type: ResourceAccessTypeRole,
+		}
+		app.RequiredResources = append(app.RequiredResources, extraResourceAccess)
+
+		missing, redundant := th.p.checkPermissions(&app)
+		assert.Equal(t, []expectedPermission{
+			getExpectedPermissions()[0],
+			getExpectedPermissions()[1],
+		}, missing)
+		assert.Equal(t, []clientmodels.ResourceAccess{
+			changedResourceAccess,
+			extraResourceAccess,
+		}, redundant)
+	})
+
+	t.Run("expected permissions", func(t *testing.T) {
+		th.Reset(t)
+
+		var app clientmodels.App
+
+		for _, expectedPermission := range getExpectedPermissions() {
+			app.RequiredResources = append(app.RequiredResources, expectedPermission.ResourceAccess)
+		}
+
+		missing, redundant := th.p.checkPermissions(&app)
+		assert.Empty(t, missing)
+		assert.Empty(t, redundant)
+	})
+}

server/msteams/client.go

@@ -278,22 +278,34 @@ func (tc *ClientImpl) RefreshToken(token *oauth2.Token) (*oauth2.Token, error) {
 	return conf.TokenSource(context.Background(), token).Token()
 }
 
-func (tc *ClientImpl) GetAppCredentials(applicationID string) ([]clientmodels.Credential, error) {
+func (tc *ClientImpl) GetApp(applicationID string) (*clientmodels.App, error) {
 	application, err := tc.client.ApplicationsWithAppId(&applicationID).Get(tc.ctx, nil)
 	if err != nil {
 		return nil, err
 	}
-	credentials := []clientmodels.Credential{}
+
+	var app clientmodels.App
+
 	credentialsList := application.GetPasswordCredentials()
 	for _, credential := range credentialsList {
-		credentials = append(credentials, clientmodels.Credential{
+		app.Credentials = append(app.Credentials, clientmodels.Credential{
 			ID:          credential.GetKeyId().String(),
 			Name:        *credential.GetDisplayName(),
 			EndDateTime: *credential.GetEndDateTime(),
 			Hint:        *credential.GetHint(),
 		})
 	}
-	return credentials, nil
+
+	for _, requiredResourceAccess := range application.GetRequiredResourceAccess() {
+		for _, requiredResource := range requiredResourceAccess.GetResourceAccess() {
+			app.RequiredResources = append(app.RequiredResources, clientmodels.ResourceAccess{
+				ID:   requiredResource.GetId().String(),
+				Type: *requiredResource.GetTypeEscaped(),
+			})
+		}
+	}
+
+	return &app, nil
 }
 
 func (tc *ClientImpl) Connect() error {

server/msteams/clientmodels/clientmodels.go

@@ -104,3 +104,13 @@ type Credential struct {
 	EndDateTime time.Time
 	Hint        string
 }
+
+type ResourceAccess struct {
+	ID   string
+	Type string
+}
+
+type App struct {
+	Credentials       []Credential
+	RequiredResources []ResourceAccess
+}

server/msteams/interface.go

@@ -56,5 +56,5 @@ type Client interface {
 	ListChannels(teamID string) ([]clientmodels.Channel, error)
 	ListChannelMessages(teamID, channelID string, since time.Time) ([]*clientmodels.Message, error)
 	ListChatMessages(chatID string, since time.Time) ([]*clientmodels.Message, error)
-	GetAppCredentials(applicationID string) ([]clientmodels.Credential, error)
+	GetApp(applicationID string) (*clientmodels.App, error)
 }