[MM-51404]: Added logic to generate unique webhook secret for subscriptions #152
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… 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>
…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>
* [MI-2877]: Fix for unbounded read in Azure DevOps API client * [MI-2877]: Review fixes --------- Co-authored-by: Abhishek Verma <abhishek.verma@brightscout.com>
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #152 +/- ##
==========================================
- Coverage 65.20% 64.75% -0.45%
==========================================
Files 15 15
Lines 3411 3439 +28
==========================================
+ Hits 2224 2227 +3
- Misses 1040 1065 +25
Partials 147 147
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
mickmister
approved these changes
Apr 3, 2023
server/store/subscriptions.go
Outdated
Comment on lines
217
to
224
| func (s *Store) GetSubscriptionChannelID(subscriptionID string) (*SubscriptionWebhookSecretAndChannelMap, error) { | ||
| var storedWebhookSecret SubscriptionWebhookSecretAndChannelMap | ||
| if err := s.LoadJSON(subscriptionID, &storedWebhookSecret); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return &storedWebhookSecret, nil | ||
| } |
There was a problem hiding this comment.
It seems off that the method GetSubscriptionChannelID is returning something called a webhook secret. The returned value is not a string either, which it seems like secret would be a string
|
|
||
| func (s *Store) StoreSubscriptionChannelID(subscriptionID, webhookSecret, channelID string) error { | ||
| if err := s.StoreJSON(subscriptionID, SubscriptionWebhookSecretAndChannelMap{ | ||
| webhookSecret: channelID, |
There was a problem hiding this comment.
This reads a little weird, because it's not obvious that StoreSubscriptionChannelID is literally a map, and not a struct with a field named webhookSecret
m1lt0n
approved these changes
Apr 4, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added logic to generate unique webhook secret for subscriptions