From 71f33564089c49c990d5313019b0a0a865db6d7c Mon Sep 17 00:00:00 2001
From: Abhishek Verma <72438220+avas27JTG@users.noreply.github.com>
Date: Wed, 4 Jan 2023 16:08:32 +0530
Subject: [PATCH 1/9] [MI-2504][server+webapp]: Generated manifest files and
 Changed "Hide" to "Close" on filter popover. (#17) (#21)

* [MI-2504][webapp]: Changed Hide to Close on filter popover

* [MI-2504][server]: Generated manifest files

* [MI-2504][server]: Updated version in manifest

Co-authored-by: Abhishek Verma <abhishek.verma@brightscout.com>

Co-authored-by: Abhishek Verma <abhishek.verma@brightscout.com>
---
 Makefile                                      |  5 ++
 build/manifest/main.go                        | 51 +++++++++++++++++++
 server/plugin/manifest.go                     | 11 ++++
 .../containers/Rhs/projectDetails/header.tsx  |  2 +-
 4 files changed, 68 insertions(+), 1 deletion(-)
 create mode 100644 server/plugin/manifest.go

diff --git a/Makefile b/Makefile
index f98b74e8..e9d579a0 100644
--- a/Makefile
+++ b/Makefile
@@ -31,6 +31,11 @@ ifneq ($(wildcard build/custom.mk),)
 	include build/custom.mk
 endif
 
+## Propagates plugin manifest information into the server/ and webapp/ folders as required.
+.PHONY: apply
+apply:
+	./build/bin/manifest apply
+
 ## Checks the code style, tests, builds and bundles the plugin.
 .PHONY: all
 all: check-style test dist
diff --git a/build/manifest/main.go b/build/manifest/main.go
index 1c6da231..9d744694 100644
--- a/build/manifest/main.go
+++ b/build/manifest/main.go
@@ -3,12 +3,33 @@ package main
 import (
 	"encoding/json"
 	"fmt"
+	"io/ioutil"
 	"os"
 
 	"github.com/mattermost/mattermost-server/v5/model"
 	"github.com/pkg/errors"
 )
 
+const pluginIDGoFileTemplate = `// This file is automatically generated. Do not modify it manually.
+
+package plugin
+
+var manifest = struct {
+	ID      string
+	Version string
+}{
+	ID:      "%s",
+	Version: "%s",
+}
+`
+
+const pluginIDTSFileTemplate = `import manifest from '../../plugin.json';
+
+export default manifest;
+export const id = manifest.id;
+export const version = manifest.version;
+`
+
 func main() {
 	if len(os.Args) <= 1 {
 		panic("no cmd specified")
@@ -37,6 +58,11 @@ func main() {
 			fmt.Printf("true")
 		}
 
+	case "apply":
+		if err := applyManifest(manifest); err != nil {
+			panic("failed to apply manifest: " + err.Error())
+		}
+
 	default:
 		panic("unrecognized command: " + cmd)
 	}
@@ -74,3 +100,28 @@ func dumpPluginID(manifest *model.Manifest) {
 func dumpPluginVersion(manifest *model.Manifest) {
 	fmt.Printf("%s", manifest.Version)
 }
+
+// applyManifest propagates the plugin_id into the server and webapp folders, as necessary
+func applyManifest(manifest *model.Manifest) error {
+	if manifest.HasServer() {
+		if err := ioutil.WriteFile(
+			"server/plugin/manifest.go",
+			[]byte(fmt.Sprintf(pluginIDGoFileTemplate, manifest.Id, manifest.Version)),
+			0600,
+		); err != nil {
+			return errors.Wrap(err, "failed to write server/plugin/manifest.go")
+		}
+	}
+
+	if manifest.HasWebapp() {
+		if err := ioutil.WriteFile(
+			"webapp/src/manifest.ts",
+			[]byte(pluginIDTSFileTemplate),
+			0600,
+		); err != nil {
+			return errors.Wrap(err, "failed to write webapp/src/manifest.ts")
+		}
+	}
+
+	return nil
+}
diff --git a/server/plugin/manifest.go b/server/plugin/manifest.go
new file mode 100644
index 00000000..27a69ba3
--- /dev/null
+++ b/server/plugin/manifest.go
@@ -0,0 +1,11 @@
+// This file is automatically generated. Do not modify it manually.
+
+package plugin
+
+var manifest = struct {
+	ID      string
+	Version string
+}{
+	ID:      "mattermost-plugin-azure-devops",
+	Version: "2.2.1",
+}
diff --git a/webapp/src/containers/Rhs/projectDetails/header.tsx b/webapp/src/containers/Rhs/projectDetails/header.tsx
index 7bb9cbe6..ea7b3abd 100644
--- a/webapp/src/containers/Rhs/projectDetails/header.tsx
+++ b/webapp/src/containers/Rhs/projectDetails/header.tsx
@@ -234,7 +234,7 @@ const Header = ({projectDetails, showAllSubscriptions, handlePagination, setShow
                                 isDisabled={!isFilterApplied()}
                             />
                             <Button
-                                text='Hide'
+                                text='Close'
                                 onClick={() => setShowFilter(false)}
                             />
                         </div>

From 9c9c68c78a8a64fb28cfa5f083adaace33489144 Mon Sep 17 00:00:00 2001
From: Abhishek Verma <72438220+avas27JTG@users.noreply.github.com>
Date: Thu, 19 Jan 2023 15:47:20 +0530
Subject: [PATCH 2/9] [MI-2505]: Added logic to protect subscriptions
 notification webhook API and fixed Boards update subscription. (#22)

* [MI-2505][server]: Added logic to protect subscriptions notification webhook API and fixed Boards update subscription.

* [MI-2505][MI-2518] Fix failing testcases

* [MI-2505]:Added webhook secret encoding and review fixes

* [MI-2505]:Added webhook secret encryption

* [MI-2505]: Fixed CI

* [MI-2505]: Reverted change of auth scopes

* [MI-2505]: Fixed CI

* [MI-2505][MI-2603] Fixed testcases

* [MI-2505]: Used constant for path

* [MI-2505]: Refinded message

* [MI-2505]: Minor review fixes

* [MI-2505][MI-2603] Review fix

Co-authored-by: Abhishek Verma <abhishek.verma@brightscout.com>
Co-authored-by: raghavaggarwal2308 <raghav.aggarwal@brightscout.com>
---
 plugin.json                         |  8 ++-
 server/config/config.go             |  5 ++
 server/config/config_test.go        | 17 ++++++-
 server/constants/constants.go       |  4 ++
 server/constants/messages.go        | 77 +++++++++++++++--------------
 server/plugin/api.go                | 19 ++++---
 server/plugin/api_test.go           | 39 ++++++++++++---
 server/plugin/client.go             |  8 ++-
 server/plugin/utils.go              | 18 +++++++
 server/serializers/subscriptions.go | 10 ++--
 10 files changed, 146 insertions(+), 59 deletions(-)

diff --git a/plugin.json b/plugin.json
index 724f39f5..063e08c3 100644
--- a/plugin.json
+++ b/plugin.json
@@ -43,7 +43,13 @@
                 "key": "EncryptionSecret",
                 "display_name": "Encryption Secret:",
                 "type": "generated",
-                "help_text": "The secret key used to encrypt and decrypt the OAuth token.\nRegenerating the secret will require all users to re-connect their accounts to Azure DevOps."
+                "help_text": "The secret key used to encrypt and decrypt the OAuth token and webhook secret.\nRegenerating the secret will require all users to re-connect their accounts to Azure DevOps and create new subscriptions as existing ones will no longer be authenticated."
+            },
+            {
+                "key": "WebhookSecret",
+                "display_name": "Webhook Secret:",
+                "type": "generated",
+                "help_text": "The secret key used to authenticate incoming requests to notification webhook used for subscriptions.\nRegenerating the secret will require all users to create new subscriptions as existing ones will no longer be authenticated."
             }
         ]
     }
diff --git a/server/config/config.go b/server/config/config.go
index 6612a65e..40c885bd 100644
--- a/server/config/config.go
+++ b/server/config/config.go
@@ -23,6 +23,7 @@ type Configuration struct {
 	AzureDevopsOAuthAppID        string `json:"azureDevopsOAuthAppID"`
 	AzureDevopsOAuthClientSecret string `jso:"azureDevopsOAuthClientSecret"`
 	EncryptionSecret             string `json:"EncryptionSecret"`
+	WebhookSecret                string `json:"WebhookSecret"`
 	MattermostSiteURL            string
 }
 
@@ -39,6 +40,7 @@ func (c *Configuration) ProcessConfiguration() error {
 	c.AzureDevopsOAuthAppID = strings.TrimSpace(c.AzureDevopsOAuthAppID)
 	c.AzureDevopsOAuthClientSecret = strings.TrimSpace(c.AzureDevopsOAuthClientSecret)
 	c.EncryptionSecret = strings.TrimSpace(c.EncryptionSecret)
+	c.WebhookSecret = strings.TrimSpace(c.WebhookSecret)
 
 	return nil
 }
@@ -57,6 +59,9 @@ func (c *Configuration) IsValid() error {
 	if c.EncryptionSecret == "" {
 		return errors.New(constants.EmptyEncryptionSecretError)
 	}
+	if c.WebhookSecret == "" {
+		return errors.New(constants.EmptyWebhookSecretError)
+	}
 
 	return nil
 }
diff --git a/server/config/config_test.go b/server/config/config_test.go
index 8759982b..bcd7d6a3 100644
--- a/server/config/config_test.go
+++ b/server/config/config_test.go
@@ -22,8 +22,8 @@ func TestIsValid(t *testing.T) {
 				AzureDevopsOAuthAppID:        "mockAzureDevopsOAuthAppID",
 				AzureDevopsOAuthClientSecret: "mockAzureDevopsOAuthClientSecret",
 				EncryptionSecret:             "mockEncryptionSecret",
+				WebhookSecret:                "mockWebhookSecret",
 			},
-			errMsg: "",
 		},
 		{
 			description: "configuration: empty AzureDevopsAPIBaseURL",
@@ -32,6 +32,7 @@ func TestIsValid(t *testing.T) {
 				AzureDevopsOAuthAppID:        "mockAzureDevopsOAuthAppID",
 				AzureDevopsOAuthClientSecret: "mockAzureDevopsOAuthClientSecret",
 				EncryptionSecret:             "mockEncryptionSecret",
+				WebhookSecret:                "mockWebhookSecret",
 			},
 			errMsg: constants.EmptyAzureDevopsAPIBaseURLError,
 		},
@@ -42,6 +43,7 @@ func TestIsValid(t *testing.T) {
 				AzureDevopsOAuthAppID:        "",
 				AzureDevopsOAuthClientSecret: "mockAzureDevopsOAuthClientSecret",
 				EncryptionSecret:             "mockEncryptionSecret",
+				WebhookSecret:                "mockWebhookSecret",
 			},
 			errMsg: constants.EmptyAzureDevopsOAuthAppIDError,
 		},
@@ -52,6 +54,7 @@ func TestIsValid(t *testing.T) {
 				AzureDevopsOAuthAppID:        "mockAzureDevopsOAuthAppID",
 				AzureDevopsOAuthClientSecret: "",
 				EncryptionSecret:             "mockEncryptionSecret",
+				WebhookSecret:                "mockWebhookSecret",
 			},
 			errMsg: constants.EmptyAzureDevopsOAuthClientSecretError,
 		},
@@ -62,9 +65,21 @@ func TestIsValid(t *testing.T) {
 				AzureDevopsOAuthAppID:        "mockAzureDevopsOAuthAppID",
 				AzureDevopsOAuthClientSecret: "mockAzureDevopsOAuthClientSecret",
 				EncryptionSecret:             "",
+				WebhookSecret:                "mockWebhookSecret",
 			},
 			errMsg: constants.EmptyEncryptionSecretError,
 		},
+		{
+			description: "configuration: empty WebhookSecret",
+			config: &Configuration{
+				AzureDevopsAPIBaseURL:        "mockAzureDevopsAPIBaseURL",
+				AzureDevopsOAuthAppID:        "mockAzureDevopsOAuthAppID",
+				AzureDevopsOAuthClientSecret: "mockAzureDevopsOAuthClientSecret",
+				EncryptionSecret:             "mockEncryptionSecret",
+				WebhookSecret:                "",
+			},
+			errMsg: constants.EmptyWebhookSecretError,
+		},
 	} {
 		t.Run(testCase.description, func(t *testing.T) {
 			err := testCase.config.IsValid()
diff --git a/server/constants/constants.go b/server/constants/constants.go
index f2e563ef..ee1276cf 100644
--- a/server/constants/constants.go
+++ b/server/constants/constants.go
@@ -95,6 +95,10 @@ const (
 	GetTasks    = "/%s/_apis/wit/workitems"
 	StaticFiles = "%s/plugins/%s/static/%s"
 
+	// Query params sent to Azure DevOps APIs
+	AzureDevopsQueryParamWebhookSecret = "webhookSecret"
+	AzureDevopsQueryParamChannelID     = "channelID"
+
 	PageQueryParam       = "$top"
 	APIVersionQueryParam = "api-version"
 	IDsQueryParam        = "ids"
diff --git a/server/constants/messages.go b/server/constants/messages.go
index 680f6142..ce3c95d8 100644
--- a/server/constants/messages.go
+++ b/server/constants/messages.go
@@ -24,53 +24,56 @@ const (
 	EventTypeRequired               = "event type is required"
 	ServiceTypeRequired             = "service type is required"
 	ChannelIDRequired               = "channel ID is required"
+	WebhookSecretRequired           = "webhook secret is required"
 	MMUserIDRequired                = "mattermsot user ID is required"
 	EmptyAzureDevopsAPIBaseURLError = "azure devops API base URL should not be empty"
 	EmptyAzureDevopsOAuthAppIDError = "azure devops OAuth app id should not be empty"
 	// #nosec G101 -- This is a false positive. The below line is not a hardcoded credential
 	EmptyAzureDevopsOAuthClientSecretError = "azure devops OAuth client secret should not be empty"
 	EmptyEncryptionSecretError             = "encryption secret should not be empty"
+	EmptyWebhookSecretError                = "webhook secret should not be empty"
 	ProjectIDRequired                      = "project ID is required"
 	FiltersRequired                        = "filters required"
 )
 
 const (
 	// Error messages
-	Error                                      = "Error"
-	NotAuthorized                              = "Not authorized"
-	UnableToDisconnectUser                     = "Unable to disconnect user"
-	UnableToDMBot                              = "Unable to send DM to bot"
-	UnableToCheckIfAlreadyConnected            = "Unable to check if user account is already connected"
-	UnableToStoreOauthState                    = "Unable to store oAuth state for the userID %s"
-	AuthAttemptExpired                         = "Authentication attempt expired, please try again"
-	InvalidAuthState                           = "Invalid oauth state, please try again"
-	GetProjectListError                        = "Error in getting project list"
-	ErrorFetchProjectList                      = "Error in fetching project list"
-	ErrorDecodingBody                          = "Error in decoding body"
-	ErrorCreateTask                            = "Error in creating task"
-	ErrorLinkProject                           = "Error in linking the project"
-	FetchSubscriptionListError                 = "Error in fetching subscription list"
-	FetchFilteredSubscriptionListError         = "Error in fetching filtered subscription list"
-	CreateSubscriptionError                    = "Error in creating subscription"
-	ErrorCheckingProjectAdmin                  = "Error in checking if user is an admin on the project %s"
-	ProjectNotLinked                           = "Requested project is not linked"
-	GetSubscriptionListError                   = "Error getting subscription list"
-	SubscriptionAlreadyPresent                 = "Requested subscription already exists"
-	SubscriptionNotFound                       = "Requested subscription does not exists"
-	ErrorLoadingUserData                       = "Error in loading user data"
-	ErrorLoadingDataFromKVStore                = "Error in loading data from KV store"
-	ProjectNotFound                            = "Requested project does not exist"
-	ErrorUnlinkProject                         = "Error in unlinking the project"
-	InvalidChannelID                           = "Invalid channel ID"
-	DeleteSubscriptionError                    = "Error in deleting subscription"
-	GetChannelError                            = "Error in getting channels for team and user"
-	GetUserError                               = "Error in getting Mattermost user details"
-	InvalidPaginationQueryParam                = "Invalid value for query param(s) page or per_page"
-	ErrorAdminAccess                           = "Cannot delete the subscription, looks like you do not have access to add/delete a subscription for this project. Please make sure you are a project or team administrator for this project"
-	ErrorFetchSubscriptionList                 = "Error in fetching subscription list"
-	ErrorMessageForAdmin                       = "There is no registered handler for the service hooks event type %s"
-	AccessDenied                               = "Access Denied"
-	ErrorOrganizationOrProjectQueryParam       = "Invalid organization or project name"
-	ErrorRepositoryPathParam                   = "Invalid organization, project or repository params"
-	ErrorFetchSubscriptionFilterPossibleValues = "Error in fetching subscription filter possible values"
+	Error                                        = "Error"
+	NotAuthorized                                = "Not authorized"
+	UnableToDisconnectUser                       = "Unable to disconnect user"
+	UnableToDMBot                                = "Unable to send DM to bot"
+	UnableToCheckIfAlreadyConnected              = "Unable to check if user account is already connected"
+	UnableToStoreOauthState                      = "Unable to store oAuth state for the userID %s"
+	AuthAttemptExpired                           = "Authentication attempt expired, please try again"
+	InvalidAuthState                             = "Invalid oauth state, please try again"
+	GetProjectListError                          = "Error in getting project list"
+	ErrorFetchProjectList                        = "Error in fetching project list"
+	ErrorDecodingBody                            = "Error in decoding body"
+	ErrorCreateTask                              = "Error in creating task"
+	ErrorLinkProject                             = "Error in linking the project"
+	FetchSubscriptionListError                   = "Error in fetching subscription list"
+	FetchFilteredSubscriptionListError           = "Error in fetching filtered subscription list"
+	CreateSubscriptionError                      = "Error in creating subscription"
+	ErrorCheckingProjectAdmin                    = "Error in checking if user is an admin on the project %s"
+	ProjectNotLinked                             = "Requested project is not linked"
+	GetSubscriptionListError                     = "Error getting subscription list"
+	SubscriptionAlreadyPresent                   = "Requested subscription already exists"
+	SubscriptionNotFound                         = "Requested subscription does not exists"
+	ErrorLoadingUserData                         = "Error in loading user data"
+	ErrorLoadingDataFromKVStore                  = "Error in loading data from KV store"
+	ProjectNotFound                              = "Requested project does not exist"
+	ErrorUnlinkProject                           = "Error in unlinking the project"
+	InvalidChannelID                             = "Invalid channel ID"
+	DeleteSubscriptionError                      = "Error in deleting subscription"
+	GetChannelError                              = "Error in getting channels for team and user"
+	GetUserError                                 = "Error in getting Mattermost user details"
+	InvalidPaginationQueryParam                  = "Invalid value for query param(s) page or per_page"
+	ErrorAdminAccess                             = "Cannot delete the subscription, looks like you do not have access to add/delete a subscription for this project. Please make sure you are a project or team administrator for this project"
+	ErrorFetchSubscriptionList                   = "Error in fetching subscription list"
+	ErrorMessageForAdmin                         = "There is no registered handler for the service hooks event type %s"
+	AccessDenied                                 = "Access Denied"
+	ErrorOrganizationOrProjectQueryParam         = "Invalid organization or project name"
+	ErrorRepositoryPathParam                     = "Invalid organization, project or repository params"
+	ErrorFetchSubscriptionFilterPossibleValues   = "Error in fetching subscription filter possible values"
+	ErrorUnauthorisedSubscriptionsWebhookRequest = "missing or invalid webhook secret for subscriptions notification"
 )
diff --git a/server/plugin/api.go b/server/plugin/api.go
index 567bb442..40f6dba5 100644
--- a/server/plugin/api.go
+++ b/server/plugin/api.go
@@ -489,6 +489,13 @@ func (p *Plugin) getReviewersListString(reviewersList []serializers.Reviewer) st
 }
 
 func (p *Plugin) handleSubscriptionNotifications(w http.ResponseWriter, r *http.Request) {
+	webhookSecret := r.URL.Query().Get(constants.AzureDevopsQueryParamWebhookSecret)
+	if status, err := p.VerifyEncryptedWebhookSecret(webhookSecret); err != nil {
+		p.API.LogError(constants.ErrorUnauthorisedSubscriptionsWebhookRequest, "Error", err.Error())
+		p.handleError(w, r, &serializers.Error{Code: status, Message: constants.ErrorUnauthorisedSubscriptionsWebhookRequest})
+		return
+	}
+
 	body, err := serializers.SubscriptionNotificationFromJSON(r.Body)
 	if err != nil {
 		p.API.LogError("Error in decoding the body for creating notifications", "Error", err.Error())
@@ -496,7 +503,7 @@ func (p *Plugin) handleSubscriptionNotifications(w http.ResponseWriter, r *http.
 		return
 	}
 
-	channelID := r.URL.Query().Get("channelID")
+	channelID := r.URL.Query().Get(constants.AzureDevopsQueryParamChannelID)
 	if channelID == "" {
 		p.API.LogError(constants.ChannelIDRequired)
 		p.handleError(w, r, &serializers.Error{Code: http.StatusBadRequest, Message: constants.ChannelIDRequired})
@@ -517,7 +524,7 @@ func (p *Plugin) handleSubscriptionNotifications(w http.ResponseWriter, r *http.
 			AuthorIcon: fmt.Sprintf(constants.StaticFiles, p.GetSiteURL(), constants.PluginID, constants.FileNameBoardsIcon),
 			Color:      constants.IconColorBoards,
 			Pretext:    body.Message.Markdown,
-			Title:      body.Resource.Fields.Title,
+			Title:      body.Resource.Fields.Title.(string),
 			Fields: []*model.SlackAttachmentField{
 				{
 					Title: "Area Path",
@@ -534,7 +541,7 @@ func (p *Plugin) handleSubscriptionNotifications(w http.ResponseWriter, r *http.
 					Value: body.Resource.Fields.WorkItemType,
 				},
 			},
-			Footer:     body.Resource.Fields.ProjectName,
+			Footer:     body.Resource.Fields.ProjectName.(string),
 			FooterIcon: fmt.Sprintf(constants.StaticFiles, p.GetSiteURL(), constants.PluginID, constants.FileNameProjectIcon),
 		}
 	case constants.SubscriptionEventWorkItemCommented:
@@ -548,7 +555,7 @@ func (p *Plugin) handleSubscriptionNotifications(w http.ResponseWriter, r *http.
 			Pretext:    body.Message.Markdown,
 			Title:      "Comment",
 			Text:       strings.TrimSpace(comment[len(comment)-1]),
-			Footer:     body.Resource.Fields.ProjectName,
+			Footer:     body.Resource.Fields.ProjectName.(string),
 			FooterIcon: fmt.Sprintf(constants.StaticFiles, p.GetSiteURL(), constants.PluginID, constants.FileNameProjectIcon),
 		}
 	case constants.SubscriptionEventWorkItemUpdated:
@@ -557,7 +564,7 @@ func (p *Plugin) handleSubscriptionNotifications(w http.ResponseWriter, r *http.
 			AuthorIcon: fmt.Sprintf(constants.StaticFiles, p.GetSiteURL(), constants.PluginID, constants.FileNameBoardsIcon),
 			Color:      constants.IconColorBoards,
 			Pretext:    body.Message.Markdown,
-			Title:      body.Resource.Revision.Fields.Title,
+			Title:      body.Resource.Revision.Fields.Title.(string),
 			Fields: []*model.SlackAttachmentField{
 				{
 					Title: "Area Path",
@@ -574,7 +581,7 @@ func (p *Plugin) handleSubscriptionNotifications(w http.ResponseWriter, r *http.
 					Value: body.Resource.Revision.Fields.WorkItemType,
 				},
 			},
-			Footer:     body.Resource.Revision.Fields.ProjectName,
+			Footer:     body.Resource.Revision.Fields.ProjectName.(string),
 			FooterIcon: fmt.Sprintf(constants.StaticFiles, p.GetSiteURL(), constants.PluginID, constants.FileNameProjectIcon),
 		}
 	case constants.SubscriptionEventPullRequestCreated, constants.SubscriptionEventPullRequestUpdated, constants.SubscriptionEventPullRequestMerged:
diff --git a/server/plugin/api_test.go b/server/plugin/api_test.go
index a9984cdb..d9d86302 100644
--- a/server/plugin/api_test.go
+++ b/server/plugin/api_test.go
@@ -21,6 +21,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/mattermost/mattermost-plugin-azure-devops/mocks"
+	"github.com/mattermost/mattermost-plugin-azure-devops/server/config"
 	"github.com/mattermost/mattermost-plugin-azure-devops/server/constants"
 	"github.com/mattermost/mattermost-plugin-azure-devops/server/serializers"
 	"github.com/mattermost/mattermost-plugin-azure-devops/server/testutils"
@@ -40,6 +41,10 @@ func setupMockPlugin(api *plugintest.API, store *mocks.MockKVStore, client *mock
 		p.Store = store
 	}
 
+	p.setConfiguration(&config.Configuration{
+		WebhookSecret: "mockWebhookSecret",
+	})
+
 	if client != nil {
 		p.Client = client
 	}
@@ -744,6 +749,7 @@ func TestHandleSubscriptionNotifications(t *testing.T) {
 		isValidChannelID bool
 		err              error
 		statusCode       int
+		webhookSecret    string
 	}{
 		{
 			description: "SubscriptionNotifications: valid",
@@ -755,31 +761,32 @@ func TestHandleSubscriptionNotifications(t *testing.T) {
 			channelID:        "mockChannelID",
 			isValidChannelID: true,
 			statusCode:       http.StatusOK,
+			webhookSecret:    "mockWebhookSecret",
 		},
 		{
 			description:      "SubscriptionNotifications: empty body",
 			body:             `{}`,
-			err:              errors.New("mockError"),
 			channelID:        "mockChannelID",
 			isValidChannelID: true,
 			statusCode:       http.StatusOK,
+			webhookSecret:    "mockWebhookSecret",
 		},
 		{
-			description: "SubscriptionNotifications: invalid channel ID",
-			body:        `{}`,
-			err:         errors.New("mockError"),
-			channelID:   "mockChannelID",
-			statusCode:  http.StatusBadRequest,
+			description:   "SubscriptionNotifications: invalid channel ID",
+			body:          `{}`,
+			channelID:     "mockChannelID",
+			statusCode:    http.StatusBadRequest,
+			webhookSecret: "mockWebhookSecret",
 		},
 		{
 			description: "SubscriptionNotifications: invalid body",
 			body: `{
 				"detailedMessage": {
 					"markdown": "mockMarkdown"`,
-			err:              errors.New("mockError"),
 			channelID:        "mockChannelID",
 			isValidChannelID: true,
 			statusCode:       http.StatusBadRequest,
+			webhookSecret:    "mockWebhookSecret",
 		},
 		{
 			description: "SubscriptionNotifications: without channelID",
@@ -790,6 +797,18 @@ func TestHandleSubscriptionNotifications(t *testing.T) {
 				}`,
 			isValidChannelID: true,
 			statusCode:       http.StatusBadRequest,
+			webhookSecret:    "mockWebhookSecret",
+		},
+		{
+			description: "SubscriptionNotifications: without webhookSecret",
+			body: `{
+				"detailedMessage": {
+					"markdown": "mockMarkdown"
+					}
+				}`,
+			isValidChannelID: true,
+			statusCode:       http.StatusUnauthorized,
+			err:              errors.New("webhook secret is absent"),
 		},
 	} {
 		t.Run(testCase.description, func(t *testing.T) {
@@ -800,7 +819,11 @@ func TestHandleSubscriptionNotifications(t *testing.T) {
 				return testCase.isValidChannelID
 			})
 
-			req := httptest.NewRequest(http.MethodPost, fmt.Sprintf("/notification?channelID=%s", testCase.channelID), bytes.NewBufferString(testCase.body))
+			monkey.PatchInstanceMethod(reflect.TypeOf(p), "VerifyEncryptedWebhookSecret", func(_ *Plugin, _ string) (int, error) {
+				return testCase.statusCode, testCase.err
+			})
+
+			req := httptest.NewRequest(http.MethodPost, fmt.Sprintf("%s?%s=%s&%s=%s", constants.PathSubscriptionNotifications, constants.AzureDevopsQueryParamChannelID, testCase.channelID, constants.AzureDevopsQueryParamWebhookSecret, testCase.webhookSecret), bytes.NewBufferString(testCase.body))
 
 			w := httptest.NewRecorder()
 			p.handleSubscriptionNotifications(w, req)
diff --git a/server/plugin/client.go b/server/plugin/client.go
index 3e2524be..8dee48db 100644
--- a/server/plugin/client.go
+++ b/server/plugin/client.go
@@ -139,8 +139,14 @@ func (c *client) callFormURLEncoded(url, path, method string, out interface{}, f
 func (c *client) CreateSubscription(body *serializers.CreateSubscriptionRequestPayload, project *serializers.ProjectDetails, channelID, pluginURL, mattermostUserID string) (*serializers.SubscriptionValue, int, error) {
 	subscriptionURL := fmt.Sprintf(constants.CreateSubscription, body.Organization)
 
+	encryptedWebhookSecret, err := c.plugin.Encrypt([]byte(c.plugin.getConfiguration().WebhookSecret), []byte(c.plugin.getConfiguration().EncryptionSecret))
+	if err != nil {
+		return nil, http.StatusInternalServerError, errors.Wrap(err, "failed to encrypt webhook secret")
+	}
+	encodedWebhookSecret := c.plugin.Encode(encryptedWebhookSecret)
+
 	consumerInputs := serializers.ConsumerInputs{
-		URL: fmt.Sprintf("%s%s?channelID=%s", strings.TrimRight(pluginURL, "/"), constants.PathSubscriptionNotifications, channelID),
+		URL: fmt.Sprintf("%s%s?%s=%s&%s=%s", strings.TrimRight(pluginURL, "/"), constants.PathSubscriptionNotifications, constants.AzureDevopsQueryParamChannelID, channelID, constants.AzureDevopsQueryParamWebhookSecret, encodedWebhookSecret),
 	}
 
 	payload := serializers.CreateSubscriptionBodyPayload{
diff --git a/server/plugin/utils.go b/server/plugin/utils.go
index f18b836a..bd837d0e 100644
--- a/server/plugin/utils.go
+++ b/server/plugin/utils.go
@@ -343,3 +343,21 @@ func (p *Plugin) GetSubscriptionsForAccessibleChannelsOrProjects(subscriptionLis
 
 	return filteredSubscriptionList, nil
 }
+
+func (p *Plugin) VerifyEncryptedWebhookSecret(received string) (status int, err error) {
+	decodedWebhookSecret, err := p.Decode(received)
+	if err != nil {
+		return http.StatusInternalServerError, errors.New("failed to decode webhook secret")
+	}
+
+	decryptedWebhookSecret, err := p.Decrypt(decodedWebhookSecret, []byte(p.getConfiguration().EncryptionSecret))
+	if err != nil {
+		return http.StatusInternalServerError, errors.New("failed to decrypt webhook secret")
+	}
+
+	if p.getConfiguration().WebhookSecret != string(decryptedWebhookSecret) {
+		return http.StatusForbidden, errors.New(constants.ErrorUnauthorisedSubscriptionsWebhookRequest)
+	}
+
+	return 0, nil
+}
diff --git a/server/serializers/subscriptions.go b/server/serializers/subscriptions.go
index 8e02d42a..5ee6e4f2 100644
--- a/server/serializers/subscriptions.go
+++ b/server/serializers/subscriptions.go
@@ -172,11 +172,11 @@ type Revision struct {
 }
 
 type Fields struct {
-	ProjectName  string `json:"System.TeamProject"`
-	AreaPath     string `json:"System.AreaPath"`
-	State        string `json:"System.State"`
-	WorkItemType string `json:"System.WorkItemType"`
-	Title        string `json:"System.Title"`
+	ProjectName  interface{} `json:"System.TeamProject"`
+	AreaPath     interface{} `json:"System.AreaPath"`
+	State        interface{} `json:"System.State"`
+	WorkItemType interface{} `json:"System.WorkItemType"`
+	Title        interface{} `json:"System.Title"`
 }
 
 type RefUpdates struct {

From 1495a4b2fabf71fc9afd45539837157f23b4f9d2 Mon Sep 17 00:00:00 2001
From: Abhishek Verma <abhishek.verma@brightscout.com>
Date: Mon, 27 Feb 2023 15:35:00 +0530
Subject: [PATCH 3/9] [Fix]: Updated plugin version in manifest file

---
 server/plugin/manifest.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/server/plugin/manifest.go b/server/plugin/manifest.go
index 27a69ba3..db977e8b 100644
--- a/server/plugin/manifest.go
+++ b/server/plugin/manifest.go
@@ -7,5 +7,5 @@ var manifest = struct {
 	Version string
 }{
 	ID:      "mattermost-plugin-azure-devops",
-	Version: "2.2.1",
+	Version: "3.1.1",
 }

From 2a31abe3a259b7214f179a890e04553315e244ea Mon Sep 17 00:00:00 2001
From: Abhishek Verma <abhishek.verma@brightscout.com>
Date: Wed, 15 Mar 2023 16:09:28 +0530
Subject: [PATCH 4/9] [MM-51253]: Add check for showing user's fullname on
 subscription details

---
 server/plugin/api.go | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/server/plugin/api.go b/server/plugin/api.go
index b88c05d0..3d5cca83 100644
--- a/server/plugin/api.go
+++ b/server/plugin/api.go
@@ -339,10 +339,15 @@ func (p *Plugin) handleCreateSubscription(w http.ResponseWriter, r *http.Request
 		return
 	}
 
-	createdByDisplayName := fmt.Sprintf("%s %s", user.FirstName, user.LastName)
-	if len(strings.TrimSpace(createdByDisplayName)) == 0 {
-		createdByDisplayName = user.Username // If user's first/last name doesn't exist then show username as fallback
+	createdByDisplayName := user.Username
+
+	showFullName := p.API.GetConfig().PrivacySettings.ShowFullName
+	// If "PrivacySettings.ShowFullName" is true then show user's first/last name
+	// If user's first/last name doesn't exist then show username as fallback
+	if showFullName != nil && *showFullName && (user.FirstName != "" || user.LastName != "") {
+		createdByDisplayName = fmt.Sprintf("%s %s", user.FirstName, user.LastName)
 	}
+
 	if storeErr := p.Store.StoreSubscription(&serializers.SubscriptionDetails{
 		MattermostUserID: mattermostUserID,
 		ProjectName:      body.Project,

From fc9aca8526e8680f632a50605c2d805af371d87c Mon Sep 17 00:00:00 2001
From: Abhishek Verma <abhishek.verma@brightscout.com>
Date: Wed, 15 Mar 2023 16:24:48 +0530
Subject: [PATCH 5/9] [MM-51253]: Modified test case

---
 server/plugin/api_test.go | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/server/plugin/api_test.go b/server/plugin/api_test.go
index b42348c5..91c94438 100644
--- a/server/plugin/api_test.go
+++ b/server/plugin/api_test.go
@@ -662,6 +662,11 @@ func TestHandleCreateSubscriptions(t *testing.T) {
 				FirstName: "mockCreatedBy",
 			}, nil)
 
+			privacySettings := model.PrivacySettings{}
+			showFullName := true
+			privacySettings.ShowFullName = &showFullName
+			mockAPI.On("GetConfig", mock.AnythingOfType("string")).Return(&model.Config{PrivacySettings: privacySettings}, nil)
+
 			monkey.Patch(json.Marshal, func(interface{}) ([]byte, error) {
 				return []byte{}, testCase.marshalError
 			})

From 793d0dd13562a129dadfbc27f19eb06524bf1f18 Mon Sep 17 00:00:00 2001
From: Abhishek Verma <abhishek.verma@brightscout.com>
Date: Wed, 15 Mar 2023 16:29:01 +0530
Subject: [PATCH 6/9] [MM-51275]: modified flow

---
 server/plugin/oAuth.go | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/server/plugin/oAuth.go b/server/plugin/oAuth.go
index 3750c55d..2d2a0b76 100644
--- a/server/plugin/oAuth.go
+++ b/server/plugin/oAuth.go
@@ -126,9 +126,6 @@ func (p *Plugin) GenerateOAuthToken(code, state string) error {
 	mattermostUserID := strings.Split(state, "_")[1]
 
 	if err := p.Store.VerifyOAuthState(mattermostUserID, state); err != nil {
-		if _, DMErr := p.DM(mattermostUserID, constants.GenericErrorMessage, false); DMErr != nil {
-			return DMErr
-		}
 		return errors.Wrap(err, "failed to verify oAuth state")
 	}
 

From 43b07f59339ef63c942f72265371d308ffc5c452 Mon Sep 17 00:00:00 2001
From: Abhishek Verma <abhishek.verma@brightscout.com>
Date: Wed, 15 Mar 2023 17:02:35 +0530
Subject: [PATCH 7/9] [MM-51253]: Review fix

---
 server/plugin/api.go      | 4 ++--
 server/plugin/api_test.go | 3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/server/plugin/api.go b/server/plugin/api.go
index 3d5cca83..32a93180 100644
--- a/server/plugin/api.go
+++ b/server/plugin/api.go
@@ -342,8 +342,8 @@ func (p *Plugin) handleCreateSubscription(w http.ResponseWriter, r *http.Request
 	createdByDisplayName := user.Username
 
 	showFullName := p.API.GetConfig().PrivacySettings.ShowFullName
-	// If "PrivacySettings.ShowFullName" is true then show user's first/last name
-	// If user's first/last name doesn't exist then show username as fallback
+	// If "PrivacySettings.ShowFullName" is true then show the user's first/last name
+	// If the user's first/last name doesn't exist then show the username as fallback
 	if showFullName != nil && *showFullName && (user.FirstName != "" || user.LastName != "") {
 		createdByDisplayName = fmt.Sprintf("%s %s", user.FirstName, user.LastName)
 	}
diff --git a/server/plugin/api_test.go b/server/plugin/api_test.go
index 91c94438..2c56b8f0 100644
--- a/server/plugin/api_test.go
+++ b/server/plugin/api_test.go
@@ -662,9 +662,8 @@ func TestHandleCreateSubscriptions(t *testing.T) {
 				FirstName: "mockCreatedBy",
 			}, nil)
 
-			privacySettings := model.PrivacySettings{}
 			showFullName := true
-			privacySettings.ShowFullName = &showFullName
+			privacySettings := model.PrivacySettings{ShowFullName: &showFullName}
 			mockAPI.On("GetConfig", mock.AnythingOfType("string")).Return(&model.Config{PrivacySettings: privacySettings}, nil)
 
 			monkey.Patch(json.Marshal, func(interface{}) ([]byte, error) {

From ef03cf0c962d4f40cc81b96c17779bc4260a385b Mon Sep 17 00:00:00 2001
From: Abhishek Verma <abhishek.verma@brightscout.com>
Date: Mon, 27 Mar 2023 19:25:55 +0530
Subject: [PATCH 8/9] [MM-51253]: Added check for user id

---
 server/plugin/api.go   |  4 ++--
 server/plugin/oAuth.go | 14 +++++++-------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/server/plugin/api.go b/server/plugin/api.go
index 32a93180..b8027948 100644
--- a/server/plugin/api.go
+++ b/server/plugin/api.go
@@ -38,8 +38,8 @@ func (p *Plugin) InitRoutes() {
 	s := p.router.PathPrefix(constants.APIPrefix).Subrouter()
 
 	// OAuth
-	s.HandleFunc(constants.PathOAuthConnect, p.OAuthConnect).Methods(http.MethodGet)
-	s.HandleFunc(constants.PathOAuthCallback, p.OAuthComplete).Methods(http.MethodGet)
+	s.HandleFunc(constants.PathOAuthConnect, p.handleAuthRequired(p.OAuthConnect)).Methods(http.MethodGet)
+	s.HandleFunc(constants.PathOAuthCallback, p.handleAuthRequired(p.OAuthComplete)).Methods(http.MethodGet)
 	// Plugin APIs
 	s.HandleFunc(constants.PathCreateTasks, p.handleAuthRequired(p.checkOAuth(p.handleCreateTask))).Methods(http.MethodPost)
 	s.HandleFunc(constants.PathLinkProject, p.handleAuthRequired(p.checkOAuth(p.handleLink))).Methods(http.MethodPost)
diff --git a/server/plugin/oAuth.go b/server/plugin/oAuth.go
index 2d2a0b76..1e11fba2 100644
--- a/server/plugin/oAuth.go
+++ b/server/plugin/oAuth.go
@@ -67,11 +67,6 @@ func (p *Plugin) GenerateOAuthConnectURL(mattermostUserID string) string {
 // OAuthConnect redirects to the OAuth authorization URL
 func (p *Plugin) OAuthConnect(w http.ResponseWriter, r *http.Request) {
 	mattermostUserID := r.Header.Get(constants.HeaderMattermostUserID)
-	// TODO: use checkAuth middleware for this
-	if mattermostUserID == "" {
-		http.Error(w, "Not authorized", http.StatusUnauthorized)
-		return
-	}
 
 	if isConnected := p.MattermostUserAlreadyConnected(mattermostUserID); isConnected {
 		p.CloseBrowserWindowWithHTTPResponse(w)
@@ -107,7 +102,8 @@ func (p *Plugin) OAuthComplete(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if err := p.GenerateOAuthToken(code, state); err != nil {
+	mattermostUserID := r.Header.Get(constants.HeaderMattermostUserID)
+	if err := p.GenerateOAuthToken(code, state, mattermostUserID); err != nil {
 		if strings.Contains(err.Error(), "already connected") {
 			p.API.LogError(constants.UnableToCompleteOAuth, "Error", constants.ErrorMessageAzureDevopsAccountAlreadyConnected)
 			http.Error(w, err.Error(), http.StatusForbidden)
@@ -122,9 +118,13 @@ func (p *Plugin) OAuthComplete(w http.ResponseWriter, r *http.Request) {
 }
 
 // GenerateOAuthToken generates OAuth token after successful authorization
-func (p *Plugin) GenerateOAuthToken(code, state string) error {
+func (p *Plugin) GenerateOAuthToken(code, state, authenticatedMattermostUserID string) error {
 	mattermostUserID := strings.Split(state, "_")[1]
 
+	if mattermostUserID != authenticatedMattermostUserID {
+		return errors.New("failed to complete oAuth, mattermost user is not authenticated")
+	}
+
 	if err := p.Store.VerifyOAuthState(mattermostUserID, state); err != nil {
 		return errors.Wrap(err, "failed to verify oAuth state")
 	}

From 70e6a7fe575a8153e753619c0a6d0366f159d1bf Mon Sep 17 00:00:00 2001
From: Abhishek Verma <abhishek.verma@brightscout.com>
Date: Mon, 27 Mar 2023 20:12:55 +0530
Subject: [PATCH 9/9] [MI-2858]: Modified test cases

---
 server/plugin/oAuth_test.go | 46 ++++++++++++++++---------------------
 1 file changed, 20 insertions(+), 26 deletions(-)

diff --git a/server/plugin/oAuth_test.go b/server/plugin/oAuth_test.go
index 12db97df..c7fc4031 100644
--- a/server/plugin/oAuth_test.go
+++ b/server/plugin/oAuth_test.go
@@ -3,6 +3,7 @@ package plugin
 import (
 	"bytes"
 	"errors"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -31,33 +32,25 @@ func TestOAuthConnect(t *testing.T) {
 	mockAPI := &plugintest.API{}
 	p.API = mockAPI
 	for _, testCase := range []struct {
-		description      string
-		isConnected      bool
-		mattermostUserID string
-		DMErr            error
-		statusCode       int
+		description string
+		isConnected bool
+		DMErr       error
+		statusCode  int
 	}{
 		{
-			description:      "OAuthConnect: valid",
-			mattermostUserID: testutils.MockMattermostUserID,
-			statusCode:       http.StatusFound,
+			description: "OAuthConnect: valid",
+			statusCode:  http.StatusFound,
 		},
 		{
-			description: "OAuthConnect: without mattermostUserID",
-			statusCode:  http.StatusUnauthorized,
-		},
-		{
-			description:      "OAuthConnect: user already connected",
-			isConnected:      true,
-			mattermostUserID: testutils.MockMattermostUserID,
-			statusCode:       http.StatusBadRequest,
+			description: "OAuthConnect: user already connected",
+			isConnected: true,
+			statusCode:  http.StatusBadRequest,
 		},
 		{
-			description:      "OAuthConnect: user already connected and failed to DM",
-			isConnected:      true,
-			DMErr:            &model.AppError{},
-			mattermostUserID: testutils.MockMattermostUserID,
-			statusCode:       http.StatusInternalServerError,
+			description: "OAuthConnect: user already connected and failed to DM",
+			isConnected: true,
+			DMErr:       &model.AppError{},
+			statusCode:  http.StatusInternalServerError,
 		},
 	} {
 		t.Run(testCase.description, func(t *testing.T) {
@@ -73,7 +66,6 @@ func TestOAuthConnect(t *testing.T) {
 			})
 
 			req := httptest.NewRequest(http.MethodGet, "/oauth/connect", bytes.NewBufferString(`{}`))
-			req.Header.Add(constants.HeaderMattermostUserID, testCase.mattermostUserID)
 
 			res := httptest.NewRecorder()
 
@@ -139,7 +131,7 @@ func TestOAuthComplete(t *testing.T) {
 		},
 	} {
 		t.Run(testCase.description, func(t *testing.T) {
-			monkey.PatchInstanceMethod(reflect.TypeOf(&p), "GenerateOAuthToken", func(_ *Plugin, _, _ string) error {
+			monkey.PatchInstanceMethod(reflect.TypeOf(&p), "GenerateOAuthToken", func(_ *Plugin, _, _, _ string) error {
 				return testCase.oAuthTokenErr
 			})
 			monkey.PatchInstanceMethod(reflect.TypeOf(&p), "CloseBrowserWindowWithHTTPResponse", func(_ *Plugin, _ http.ResponseWriter) {})
@@ -170,6 +162,7 @@ func TestGenerateOAuthToken(t *testing.T) {
 		description      string
 		code             string
 		state            string
+		mmuserID         string
 		verifyOAuthError error
 		expectedError    string
 		DMError          error
@@ -177,7 +170,8 @@ func TestGenerateOAuthToken(t *testing.T) {
 		{
 			description: "GenerateOAuthToken: valid",
 			code:        "mockCode",
-			state:       "mock_state",
+			state:       fmt.Sprintf("mockState_%s", testutils.MockMattermostUserID),
+			mmuserID:    testutils.MockMattermostUserID,
 		},
 	} {
 		t.Run(testCase.description, func(t *testing.T) {
@@ -191,10 +185,10 @@ func TestGenerateOAuthToken(t *testing.T) {
 			})
 
 			if testCase.expectedError == "" {
-				mockedStore.EXPECT().VerifyOAuthState("state", testCase.state).Return(testCase.verifyOAuthError)
+				mockedStore.EXPECT().VerifyOAuthState(testCase.mmuserID, testCase.state).Return(testCase.verifyOAuthError)
 			}
 
-			err := p.GenerateOAuthToken(testCase.code, testCase.state)
+			err := p.GenerateOAuthToken(testCase.code, testCase.state, testCase.mmuserID)
 			if testCase.expectedError != "" {
 				assert.NotNil(t, err)
 				return