Skip to content

Commit

Permalink
Better webhooks validation
Browse files Browse the repository at this point in the history
  • Loading branch information
nmaupu committed Mar 31, 2021
1 parent cea9269 commit aa6e1ec
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 66 deletions.
21 changes: 21 additions & 0 deletions messagecard.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,9 @@ type MessageCard struct {

// Sections is a collection of sections to include in the card.
Sections []*MessageCardSection `json:"sections,omitempty"`

// ValidateFunc is a validation function that validates a MessageCard
ValidateFunc func() error `json:"-"`
}

// AddSection adds one or many additional MessageCardSection values to a
Expand Down Expand Up @@ -194,6 +197,24 @@ func (mc *MessageCard) AddSection(section ...*MessageCardSection) error {
return nil
}

// Validate validates a MessageCard calling ValidateFunc if defined,
// otherwise, a default validation occurs
func (mc *MessageCard) Validate() error {
if mc.ValidateFunc != nil {
return mc.ValidateFunc()
}

// Falling back to a default implementation
if (mc.Text == "") && (mc.Summary == "") {
// This scenario results in:
// 400 Bad Request
// Summary or Text is required.
return fmt.Errorf("invalid message card: summary or text field is required")
}

return nil
}

// AddFact adds one or many additional MessageCardSectionFact values to a
// MessageCardSection
func (mcs *MessageCardSection) AddFact(fact ...MessageCardSectionFact) error {
Expand Down
125 changes: 68 additions & 57 deletions send.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,17 +35,13 @@ const (
WebhookURLOrgWebhookPrefix = "https://example.webhook.office.com"
)

// DisableWebhookURLValidation is a special keyword used to indicate to
// validation function(s) that webhook URL validation should be disabled.
const DisableWebhookURLValidation string = "DISABLE_WEBHOOK_URL_VALIDATION"

// Regular Expression related constants that we can use to validate incoming
// webhook URLs provided by the user.
const (

// webhookURLRegexPrefixOnly is a minimal regex for matching known valid
// DefaultWebhookURLValidationPattern is a minimal regex for matching known valid
// webhook URL prefix patterns.
webhookURLRegexPrefixOnly = `^https:\/\/(?:.*\.webhook|outlook)\.office(?:365)?\.com`
DefaultWebhookURLValidationPattern = `^https:\/\/(?:.*\.webhook|outlook)\.office(?:365)?\.com`

// Note: The regex allows for capital letters in the GUID patterns. This is
// allowed based on light testing which shows that mixed case works and the
Expand All @@ -67,9 +63,9 @@ const ExpectedWebhookURLResponseText string = "1"
// before it times out and is cancelled.
const DefaultWebhookSendTimeout = 5 * time.Second

// ErrWebhookURLUnexpectedPrefix is returned when a provided webhook URL does
// not match a set of confirmed webhook URL prefixes.
var ErrWebhookURLUnexpectedPrefix = errors.New("webhook URL does not contain expected prefix")
// ErrWebhookURLUnexpected is returned when a provided webhook URL does
// not match a set of confirmed webhook URL patterns.
var ErrWebhookURLUnexpected = errors.New("webhook URL does not match one of expected patterns")

// ErrInvalidWebhookURLResponseText is returned when the remote webhook
// endpoint indicates via response text that a message submission was
Expand All @@ -81,12 +77,15 @@ type API interface {
Send(webhookURL string, webhookMessage MessageCard) error
SendWithContext(ctx context.Context, webhookURL string, webhookMessage MessageCard) error
SendWithRetry(ctx context.Context, webhookURL string, webhookMessage MessageCard, retries int, retriesDelay int) error
SkipWebhookURLValidationOnSend(skip bool)
SkipWebhookURLValidationOnSend(skip bool) API
AddWebhookURLValidationPatterns(patterns ...string) API
ValidateWebhook(webhookURL string) error
}

type teamsClient struct {
httpClient *http.Client
skipWebhookURLValidation bool
httpClient *http.Client
skipWebhookURLValidation bool
webhookURLValidationPatterns []string
}

func init() {
Expand Down Expand Up @@ -122,6 +121,11 @@ func NewClient() API {
return &client
}

func (c *teamsClient) AddWebhookURLValidationPatterns(patterns ...string) API {
c.webhookURLValidationPatterns = append(c.webhookURLValidationPatterns, patterns...)
return c
}

// Send is a wrapper function around the SendWithContext method in order to
// provide backwards compatibility.
func (c teamsClient) Send(webhookURL string, webhookMessage MessageCard) error {
Expand All @@ -139,14 +143,12 @@ func (c teamsClient) SendWithContext(ctx context.Context, webhookURL string, web
logger.Printf("SendWithContext: Webhook message received: %#v\n", webhookMessage)

// optionally skip webhook validation
webhookURLToValidate := webhookURL
if c.skipWebhookURLValidation {
logger.Printf("SendWithContext: Webhook URL will not be validated: %#v\n", webhookURL)
webhookURLToValidate = DisableWebhookURLValidation
}

// Validate input data
if valid, err := IsValidInput(webhookMessage, webhookURLToValidate); !valid {
if err := c.validateInput(webhookMessage, webhookURL); err != nil {
return err
}

Expand Down Expand Up @@ -307,15 +309,57 @@ func (c teamsClient) SendWithRetry(ctx context.Context, webhookURL string, webho

// SkipWebhookURLValidationOnSend allows the caller to optionally disable
// webhook URL validation.
func (c *teamsClient) SkipWebhookURLValidationOnSend(skip bool) {
func (c *teamsClient) SkipWebhookURLValidationOnSend(skip bool) API {
c.skipWebhookURLValidation = skip
return c
}

// validateInput verifies if the input parameters are valid
func (c teamsClient) validateInput(webhookMessage MessageCard, webhookURL string) error {
// validate url
if err := c.ValidateWebhook(webhookURL); err != nil {
return err
}

// validate message
return webhookMessage.Validate()
}

func (c teamsClient) ValidateWebhook(webhookURL string) error {
if c.skipWebhookURLValidation {
return nil
}

u, err := url.Parse(webhookURL)
if err != nil {
return fmt.Errorf("unable to parse webhook URL %q: %w", webhookURL, err)
}

patterns := c.webhookURLValidationPatterns
if len(patterns) == 0 {
patterns = []string{DefaultWebhookURLValidationPattern}
}

// Return true if at least one pattern matches
for _, pat := range patterns {
matched, err := regexp.MatchString(pat, webhookURL)
if err != nil {
return err
}
if matched {
return nil
}
}

return fmt.Errorf("%w; got: %q, patterns: %s", ErrWebhookURLUnexpected, u.String(), strings.Join(patterns, ","))
}

// helper --------------------------------------------------------------------------------------------------------------
// old deprecated helper functions --------------------------------------------------------------------------------------------------------------

// IsValidInput is a validation "wrapper" function. This function is intended
// to run current validation checks and offer easy extensibility for future
// validation requirements.
// Deprecated: still here for retro compatibility
func IsValidInput(webhookMessage MessageCard, webhookURL string) (bool, error) {
// validate url
if valid, err := IsValidWebhookURL(webhookURL); !valid {
Expand All @@ -332,50 +376,17 @@ func IsValidInput(webhookMessage MessageCard, webhookURL string) (bool, error) {

// IsValidWebhookURL performs validation checks on the webhook URL used to
// submit messages to Microsoft Teams.
// Deprecated: still here for retro compatibility
func IsValidWebhookURL(webhookURL string) (bool, error) {
// Skip validation if requested
if webhookURL == DisableWebhookURLValidation {
return true, nil
}

u, err := url.Parse(webhookURL)
if err != nil {
return false, fmt.Errorf(
"unable to parse webhook URL %q: %w",
webhookURL,
err,
)
}

matched, err := regexp.MatchString(webhookURLRegexPrefixOnly, webhookURL)
if !matched {
userProvidedWebhookURLPrefix := u.Scheme + "://" + u.Host

errMsg := "validation failed"
if err != nil {
errMsg = err.Error()
}

return false, fmt.Errorf(
"webhook URL %q received; %v: %w",
userProvidedWebhookURLPrefix,
errMsg,
ErrWebhookURLUnexpectedPrefix,
)
}

return true, nil
c := teamsClient{}
err := c.ValidateWebhook(webhookURL)
return err == nil, err
}

// IsValidMessageCard performs validation/checks for known issues with
// MessardCard values.
// Deprecated: still here for retro compatibility
func IsValidMessageCard(webhookMessage MessageCard) (bool, error) {
if (webhookMessage.Text == "") && (webhookMessage.Summary == "") {
// This scenario results in:
// 400 Bad Request
// Summary or Text is required.
return false, fmt.Errorf("invalid message card: summary or text field is required")
}

return true, nil
err := webhookMessage.Validate()
return err == nil, err
}
42 changes: 33 additions & 9 deletions send_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,14 @@ func TestTeamsClientSend(t *testing.T) {
simpleMsgCard := NewMessageCard()
simpleMsgCard.Text = "Hello World"
var tests = []struct {
reqURL string
reqMsg MessageCard
resStatus int // httpClient response status
resBody string // httpClient response body text
resError error // httpClient error
error error // method error
skipURLVal bool // whether webhook URL validation is applied (e.g., GH-68)
reqURL string
reqMsg MessageCard
resStatus int // httpClient response status
resBody string // httpClient response body text
resError error // httpClient error
error error // method error
skipURLVal bool // whether webhook URL validation is applied (e.g., GH-68)
validationURLPatterns []string
}{
// invalid webhookURL - url.Parse error
{
Expand All @@ -54,7 +55,7 @@ func TestTeamsClientSend(t *testing.T) {
resStatus: 0,
resBody: "invalid",
resError: nil,
error: ErrWebhookURLUnexpectedPrefix,
error: ErrWebhookURLUnexpected,
skipURLVal: false,
},
// invalid httpClient.Do call
Expand Down Expand Up @@ -127,7 +128,7 @@ func TestTeamsClientSend(t *testing.T) {
resStatus: 0,
resBody: "",
resError: nil,
error: ErrWebhookURLUnexpectedPrefix,
error: ErrWebhookURLUnexpected,
skipURLVal: false,
},
// custom webhook domain (e.g., GH-68) with validation disabled
Expand All @@ -145,6 +146,28 @@ func TestTeamsClientSend(t *testing.T) {
error: nil,
skipURLVal: true,
},
// custom webhook domain with custom validation patterns
{
reqURL: "https://arbitrary.domain.com/webhook/xxx",
reqMsg: simpleMsgCard,
resStatus: 200,
resBody: ExpectedWebhookURLResponseText,
resError: nil,
error: nil,
skipURLVal: false,
validationURLPatterns: []string{DefaultWebhookURLValidationPattern, "arbitrary.domain.com"},
},
// custom webhook domain with custom validation patterns not matching requirements
{
reqURL: "https://arbitrary.test.com/webhook/xxx",
reqMsg: simpleMsgCard,
resStatus: 200,
resBody: ExpectedWebhookURLResponseText,
resError: ErrWebhookURLUnexpected,
error: ErrWebhookURLUnexpected,
skipURLVal: false,
validationURLPatterns: []string{DefaultWebhookURLValidationPattern, "arbitrary.domain.com"},
},
}
for idx, test := range tests {
// Create range scoped var for use within closure
Expand Down Expand Up @@ -182,6 +205,7 @@ func TestTeamsClientSend(t *testing.T) {
}, nil
})
c := &teamsClient{httpClient: client}
c.AddWebhookURLValidationPatterns(test.validationURLPatterns...)

// Disable webhook URL prefix validation if specified by table test
// entry. See GH-68 for additional details.
Expand Down

0 comments on commit aa6e1ec

Please sign in to comment.