[MI-51272]: Refactored and removed unused code. (#150)

* [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 <[email protected]> Co-authored-by: Abhishek Verma <[email protected]> * [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 <[email protected]> Co-authored-by: raghavaggarwal2308 <[email protected]> * [Fix]: Updated plugin version in manifest file * [MI-51272]: Removed unused code * [MI-51272]: Handled non base64 chars for webhooksecret * [MI-51272]: escaped webhook secret query param chars * [MI-51272]: Changed function name * [MI-51247]: Used directly for webhook secret --------- Co-authored-by: Abhishek Verma <[email protected]> Co-authored-by: raghavaggarwal2308 <[email protected]>
mattermost-community · Mar 28, 2023 · b866a70 · b866a70
1 parent f499afd
commit b866a70
Show file tree

Hide file tree

Showing 5 changed files with 7 additions and 22 deletions.
diff --git a/plugin.json b/plugin.json
@@ -46,7 +46,7 @@
                 "key": "EncryptionSecret",
                 "display_name": "Encryption Secret:",
                 "type": "generated",
-                "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."
+                "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."
             },
             {
                 "key": "WebhookSecret",

diff --git a/server/plugin/api.go b/server/plugin/api.go
@@ -545,7 +545,7 @@ func (p *Plugin) getPipelineReleaseEnvironmentList(environments []*serializers.E
 
 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 {
+	if status, err := p.VerifyWebhookSecret(webhookSecret); err != nil {
 		p.API.LogError(constants.ErrorUnauthorisedSubscriptionsWebhookRequest, "Error", err.Error())
 		p.handleError(w, r, &serializers.Error{Code: status, Message: constants.ErrorUnauthorisedSubscriptionsWebhookRequest})
 		return

diff --git a/server/plugin/api_test.go b/server/plugin/api_test.go
@@ -1063,7 +1063,7 @@ func TestHandleSubscriptionNotifications(t *testing.T) {
 				return time.Time{}, testCase.parseTimeError
 			})
 
-			monkey.PatchInstanceMethod(reflect.TypeOf(p), "VerifyEncryptedWebhookSecret", func(_ *Plugin, _ string) (int, error) {
+			monkey.PatchInstanceMethod(reflect.TypeOf(p), "VerifyWebhookSecret", func(_ *Plugin, _ string) (int, error) {
 				return testCase.statusCode, testCase.err
 			})
 

diff --git a/server/plugin/client.go b/server/plugin/client.go
@@ -232,14 +232,9 @@ func (c *client) CreateSubscription(body *serializers.CreateSubscriptionRequestP
 	}
 	createSubscriptionPath := 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)
-
+	webhookSecret := url.QueryEscape(c.plugin.getConfiguration().WebhookSecret)
 	consumerInputs := serializers.ConsumerInputs{
-		URL: fmt.Sprintf("%s%s?%s=%s&%s=%s", strings.TrimRight(pluginURL, "/"), constants.PathSubscriptionNotifications, constants.AzureDevopsQueryParamChannelID, channelID, constants.AzureDevopsQueryParamWebhookSecret, encodedWebhookSecret),
+		URL: fmt.Sprintf("%s%s?%s=%s&%s=%s", strings.TrimRight(pluginURL, "/"), constants.PathSubscriptionNotifications, constants.AzureDevopsQueryParamChannelID, channelID, constants.AzureDevopsQueryParamWebhookSecret, webhookSecret),
 	}
 
 	payload := serializers.CreateSubscriptionBodyPayload{

diff --git a/server/plugin/utils.go b/server/plugin/utils.go
@@ -460,18 +460,8 @@ func (p *Plugin) deleteSubscription(subscription *serializers.SubscriptionDetail
 	return http.StatusOK, 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) {
+func (p *Plugin) VerifyWebhookSecret(received string) (status int, err error) {
+	if p.getConfiguration().WebhookSecret != received {
 		return http.StatusForbidden, errors.New(constants.ErrorUnauthorisedSubscriptionsWebhookRequest)
 	}