From b866a7048a121353ffe2c43041832d49109c8852 Mon Sep 17 00:00:00 2001 From: Abhishek Verma <72438220+avas27JTG@users.noreply.github.com> Date: Wed, 29 Mar 2023 05:10:35 +0530 Subject: [PATCH] [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 Co-authored-by: Abhishek Verma * [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 Co-authored-by: raghavaggarwal2308 * [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 Co-authored-by: raghavaggarwal2308 --- plugin.json | 2 +- server/plugin/api.go | 2 +- server/plugin/api_test.go | 2 +- server/plugin/client.go | 9 ++------- server/plugin/utils.go | 14 ++------------ 5 files changed, 7 insertions(+), 22 deletions(-) diff --git a/plugin.json b/plugin.json index f62f5119..e6a71606 100644 --- 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 index b8027948..7e16854a 100644 --- 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 index 2c56b8f0..ed1dc04f 100644 --- 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 index 41273eb9..f99d779c 100644 --- 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 index e01e5490..a88c3ea7 100644 --- 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) }