PLT-2170 Send payload in application/json for outgoing webhooks (#3160)
* Send payload in application/json for outgoing webhooks The Add outgoing webhook UI now has a 'Content-Type' field that allows to choose between application/x-www-form-urlencoded and application/json. All outgoing webhooks created before this change will be considered as x-www-form-urlencoded. There's also a minor change in the way the outgoing webhook summary is displayed: the 'Callback URLs' label was missing. * Fix JS formatting errors * Increase ContentType field length to 128
Этот коммит содержится в:
коммит произвёл
Christopher Speller
родитель
8f984771ae
Коммит
c226cabc04
58
api/post.go
58
api/post.go
@@ -7,6 +7,7 @@ import (
|
||||
"crypto/tls"
|
||||
"fmt"
|
||||
"html/template"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"path/filepath"
|
||||
@@ -362,30 +363,24 @@ func handleWebhookEvents(c *Context, post *model.Post, team *model.Team, channel
|
||||
}
|
||||
|
||||
hchan := Srv.Store.Webhook().GetOutgoingByTeam(c.TeamId)
|
||||
|
||||
hooks := []*model.OutgoingWebhook{}
|
||||
|
||||
if result := <-hchan; result.Err != nil {
|
||||
result := <-hchan
|
||||
if result.Err != nil {
|
||||
l4g.Error(utils.T("api.post.handle_webhook_events_and_forget.getting.error"), result.Err)
|
||||
return
|
||||
} else {
|
||||
hooks = result.Data.([]*model.OutgoingWebhook)
|
||||
}
|
||||
|
||||
hooks := result.Data.([]*model.OutgoingWebhook)
|
||||
if len(hooks) == 0 {
|
||||
return
|
||||
}
|
||||
|
||||
splitWords := strings.Fields(post.Message)
|
||||
|
||||
if len(splitWords) == 0 {
|
||||
return
|
||||
}
|
||||
|
||||
firstWord := splitWords[0]
|
||||
|
||||
relevantHooks := []*model.OutgoingWebhook{}
|
||||
|
||||
for _, hook := range hooks {
|
||||
if hook.ChannelId == post.ChannelId {
|
||||
if len(hook.TriggerWords) == 0 || hook.HasTriggerWord(firstWord) {
|
||||
@@ -398,25 +393,28 @@ func handleWebhookEvents(c *Context, post *model.Post, team *model.Team, channel
|
||||
|
||||
for _, hook := range relevantHooks {
|
||||
go func(hook *model.OutgoingWebhook) {
|
||||
p := url.Values{}
|
||||
p.Set("token", hook.Token)
|
||||
|
||||
p.Set("team_id", hook.TeamId)
|
||||
p.Set("team_domain", team.Name)
|
||||
|
||||
p.Set("channel_id", post.ChannelId)
|
||||
p.Set("channel_name", channel.Name)
|
||||
|
||||
p.Set("timestamp", strconv.FormatInt(post.CreateAt/1000, 10))
|
||||
|
||||
p.Set("user_id", post.UserId)
|
||||
p.Set("user_name", user.Username)
|
||||
|
||||
p.Set("post_id", post.Id)
|
||||
|
||||
p.Set("text", post.Message)
|
||||
p.Set("trigger_word", firstWord)
|
||||
|
||||
payload := &model.OutgoingWebhookPayload{
|
||||
Token: hook.Token,
|
||||
TeamId: hook.TeamId,
|
||||
TeamDomain: team.Name,
|
||||
ChannelId: post.ChannelId,
|
||||
ChannelName: channel.Name,
|
||||
Timestamp: post.CreateAt,
|
||||
UserId: post.UserId,
|
||||
UserName: user.Username,
|
||||
PostId: post.Id,
|
||||
Text: post.Message,
|
||||
TriggerWord: firstWord,
|
||||
}
|
||||
var body io.Reader
|
||||
var contentType string
|
||||
if hook.ContentType == "application/json" {
|
||||
body = strings.NewReader(payload.ToJSON())
|
||||
contentType = "application/json"
|
||||
} else {
|
||||
body = strings.NewReader(payload.ToFormValues())
|
||||
contentType = "application/x-www-form-urlencoded"
|
||||
}
|
||||
tr := &http.Transport{
|
||||
TLSClientConfig: &tls.Config{InsecureSkipVerify: *utils.Cfg.ServiceSettings.EnableInsecureOutgoingConnections},
|
||||
}
|
||||
@@ -424,8 +422,8 @@ func handleWebhookEvents(c *Context, post *model.Post, team *model.Team, channel
|
||||
|
||||
for _, url := range hook.CallbackURLs {
|
||||
go func(url string) {
|
||||
req, _ := http.NewRequest("POST", url, strings.NewReader(p.Encode()))
|
||||
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
||||
req, _ := http.NewRequest("POST", url, body)
|
||||
req.Header.Set("Content-Type", contentType)
|
||||
req.Header.Set("Accept", "application/json")
|
||||
if resp, err := client.Do(req); err != nil {
|
||||
l4g.Error(utils.T("api.post.handle_webhook_events_and_forget.event_post.error"), err.Error())
|
||||
|
||||
122
api/post_test.go
122
api/post_test.go
@@ -4,9 +4,11 @@
|
||||
package api
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strconv"
|
||||
"net/url"
|
||||
"reflect"
|
||||
"strings"
|
||||
|
||||
"fmt"
|
||||
@@ -110,7 +112,11 @@ func TestCreatePost(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestCreatePostWithOutgoingHook(t *testing.T) {
|
||||
func testCreatePostWithOutgoingHook(
|
||||
t *testing.T,
|
||||
hookContentType string,
|
||||
expectedContentType string,
|
||||
) {
|
||||
th := Setup().InitSystemAdmin()
|
||||
Client := th.SystemAdminClient
|
||||
team := th.SystemAdminTeam
|
||||
@@ -131,67 +137,52 @@ func TestCreatePostWithOutgoingHook(t *testing.T) {
|
||||
// success/failure.
|
||||
success := make(chan bool)
|
||||
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
err := r.ParseForm()
|
||||
if err != nil {
|
||||
t.Logf("Error parsing form: %q", err)
|
||||
requestContentType := r.Header.Get("Content-Type")
|
||||
if requestContentType != expectedContentType {
|
||||
t.Logf("Content-Type is %s, should be %s", requestContentType, expectedContentType)
|
||||
success <- false
|
||||
return
|
||||
}
|
||||
|
||||
if got, want := r.Form.Get("token"), hook.Token; got != want {
|
||||
t.Logf("Token is %s, should be %s", got, want)
|
||||
success <- false
|
||||
return
|
||||
expectedPayload := &model.OutgoingWebhookPayload{
|
||||
Token: hook.Token,
|
||||
TeamId: hook.TeamId,
|
||||
TeamDomain: team.Name,
|
||||
ChannelId: post.ChannelId,
|
||||
ChannelName: channel.Name,
|
||||
Timestamp: post.CreateAt,
|
||||
UserId: post.UserId,
|
||||
UserName: user.Username,
|
||||
PostId: post.Id,
|
||||
Text: post.Message,
|
||||
TriggerWord: strings.Fields(post.Message)[0],
|
||||
}
|
||||
if got, want := r.Form.Get("team_id"), hook.TeamId; got != want {
|
||||
t.Logf("TeamId is %s, should be %s", got, want)
|
||||
success <- false
|
||||
return
|
||||
}
|
||||
if got, want := r.Form.Get("team_domain"), team.Name; got != want {
|
||||
t.Logf("TeamDomain is %s, should be %s", got, want)
|
||||
success <- false
|
||||
return
|
||||
}
|
||||
if got, want := r.Form.Get("channel_id"), post.ChannelId; got != want {
|
||||
t.Logf("ChannelId is %s, should be %s", got, want)
|
||||
success <- false
|
||||
return
|
||||
}
|
||||
if got, want := r.Form.Get("channel_name"), channel.Name; got != want {
|
||||
t.Logf("ChannelName is %s, should be %s", got, want)
|
||||
success <- false
|
||||
return
|
||||
}
|
||||
if got, want := r.Form.Get("timestamp"), strconv.FormatInt(post.CreateAt/1000, 10); got != want {
|
||||
t.Logf("Timestamp is %s, should be %s", got, want)
|
||||
success <- false
|
||||
return
|
||||
}
|
||||
if got, want := r.Form.Get("user_id"), post.UserId; got != want {
|
||||
t.Logf("UserId is %s, should be %s", got, want)
|
||||
success <- false
|
||||
return
|
||||
}
|
||||
if got, want := r.Form.Get("user_name"), user.Username; got != want {
|
||||
t.Logf("Username is %s, should be %s", got, want)
|
||||
success <- false
|
||||
return
|
||||
}
|
||||
if got, want := r.Form.Get("post_id"), post.Id; got != want {
|
||||
t.Logf("PostId is %s, should be %s", got, want)
|
||||
success <- false
|
||||
return
|
||||
}
|
||||
if got, want := r.Form.Get("text"), post.Message; got != want {
|
||||
t.Logf("Message is %s, should be %s", got, want)
|
||||
success <- false
|
||||
return
|
||||
}
|
||||
if got, want := r.Form.Get("trigger_word"), strings.Fields(post.Message)[0]; got != want {
|
||||
t.Logf("TriggerWord is %s, should be %s", got, want)
|
||||
success <- false
|
||||
return
|
||||
|
||||
// depending on the Content-Type, we expect to find a JSON or form encoded payload
|
||||
if requestContentType == "application/json" {
|
||||
decoder := json.NewDecoder(r.Body)
|
||||
o := &model.OutgoingWebhookPayload{}
|
||||
decoder.Decode(&o)
|
||||
|
||||
if !reflect.DeepEqual(expectedPayload, o) {
|
||||
t.Logf("JSON payload is %+v, should be %+v", o, expectedPayload)
|
||||
success <- false
|
||||
return
|
||||
}
|
||||
} else {
|
||||
err := r.ParseForm()
|
||||
if err != nil {
|
||||
t.Logf("Error parsing form: %q", err)
|
||||
success <- false
|
||||
return
|
||||
}
|
||||
|
||||
expectedFormValues, _ := url.ParseQuery(expectedPayload.ToFormValues())
|
||||
if !reflect.DeepEqual(expectedFormValues, r.Form) {
|
||||
t.Logf("Form values are %q, should be %q", r.Form, expectedFormValues)
|
||||
success <- false
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
success <- true
|
||||
@@ -202,6 +193,7 @@ func TestCreatePostWithOutgoingHook(t *testing.T) {
|
||||
triggerWord := "bingo"
|
||||
hook = &model.OutgoingWebhook{
|
||||
ChannelId: channel.Id,
|
||||
ContentType: hookContentType,
|
||||
TriggerWords: []string{triggerWord},
|
||||
CallbackURLs: []string{ts.URL},
|
||||
}
|
||||
@@ -237,6 +229,20 @@ func TestCreatePostWithOutgoingHook(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestCreatePostWithOutgoingHook_form_urlencoded(t *testing.T) {
|
||||
testCreatePostWithOutgoingHook(t, "application/x-www-form-urlencoded", "application/x-www-form-urlencoded")
|
||||
}
|
||||
|
||||
func TestCreatePostWithOutgoingHook_json(t *testing.T) {
|
||||
testCreatePostWithOutgoingHook(t, "application/json", "application/json")
|
||||
}
|
||||
|
||||
// hooks created before we added the ContentType field should be considered as
|
||||
// application/x-www-form-urlencoded
|
||||
func TestCreatePostWithOutgoingHook_no_content_type(t *testing.T) {
|
||||
testCreatePostWithOutgoingHook(t, "", "application/x-www-form-urlencoded")
|
||||
}
|
||||
|
||||
func TestUpdatePost(t *testing.T) {
|
||||
th := Setup().InitBasic()
|
||||
Client := th.BasicClient
|
||||
|
||||
@@ -7,6 +7,8 @@ import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/url"
|
||||
"strconv"
|
||||
)
|
||||
|
||||
type OutgoingWebhook struct {
|
||||
@@ -22,6 +24,47 @@ type OutgoingWebhook struct {
|
||||
CallbackURLs StringArray `json:"callback_urls"`
|
||||
DisplayName string `json:"display_name"`
|
||||
Description string `json:"description"`
|
||||
ContentType string `json:"content_type"`
|
||||
}
|
||||
|
||||
type OutgoingWebhookPayload struct {
|
||||
Token string `json:"token"`
|
||||
TeamId string `json:"team_id"`
|
||||
TeamDomain string `json:"team_domain"`
|
||||
ChannelId string `json:"channel_id"`
|
||||
ChannelName string `json:"channel_name"`
|
||||
Timestamp int64 `json:"timestamp"`
|
||||
UserId string `json:"user_id"`
|
||||
UserName string `json:"user_name"`
|
||||
PostId string `json:"post_id"`
|
||||
Text string `json:"text"`
|
||||
TriggerWord string `json:"trigger_word"`
|
||||
}
|
||||
|
||||
func (o *OutgoingWebhookPayload) ToJSON() string {
|
||||
b, err := json.Marshal(o)
|
||||
if err != nil {
|
||||
return ""
|
||||
} else {
|
||||
return string(b)
|
||||
}
|
||||
}
|
||||
|
||||
func (o *OutgoingWebhookPayload) ToFormValues() string {
|
||||
v := url.Values{}
|
||||
v.Set("token", o.Token)
|
||||
v.Set("team_id", o.TeamId)
|
||||
v.Set("team_domain", o.TeamDomain)
|
||||
v.Set("channel_id", o.ChannelId)
|
||||
v.Set("channel_name", o.ChannelName)
|
||||
v.Set("timestamp", strconv.FormatInt(o.Timestamp/1000, 10))
|
||||
v.Set("user_id", o.UserId)
|
||||
v.Set("user_name", o.UserName)
|
||||
v.Set("post_id", o.PostId)
|
||||
v.Set("text", o.Text)
|
||||
v.Set("trigger_word", o.TriggerWord)
|
||||
|
||||
return v.Encode()
|
||||
}
|
||||
|
||||
func (o *OutgoingWebhook) ToJson() string {
|
||||
@@ -124,6 +167,10 @@ func (o *OutgoingWebhook) IsValid() *AppError {
|
||||
return NewLocAppError("OutgoingWebhook.IsValid", "model.outgoing_hook.is_valid.description.app_error", nil, "")
|
||||
}
|
||||
|
||||
if len(o.ContentType) > 128 {
|
||||
return NewLocAppError("OutgoingWebhook.IsValid", "model.outgoing_hook.is_valid.content_type.app_error", nil, "")
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
|
||||
@@ -4,6 +4,8 @@
|
||||
package model
|
||||
|
||||
import (
|
||||
"net/url"
|
||||
"reflect"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
@@ -107,8 +109,49 @@ func TestOutgoingWebhookIsValid(t *testing.T) {
|
||||
|
||||
o.Description = strings.Repeat("1", 128)
|
||||
if err := o.IsValid(); err != nil {
|
||||
t.Fatal("should be invalid")
|
||||
}
|
||||
|
||||
o.ContentType = strings.Repeat("1", 129)
|
||||
if err := o.IsValid(); err == nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
o.ContentType = strings.Repeat("1", 128)
|
||||
if err := o.IsValid(); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestOutgoingWebhookPayloadToFormValues(t *testing.T) {
|
||||
p := &OutgoingWebhookPayload{
|
||||
Token: "Token",
|
||||
TeamId: "TeamId",
|
||||
TeamDomain: "TeamDomain",
|
||||
ChannelId: "ChannelId",
|
||||
ChannelName: "ChannelName",
|
||||
Timestamp: 123000,
|
||||
UserId: "UserId",
|
||||
UserName: "UserName",
|
||||
PostId: "PostId",
|
||||
Text: "Text",
|
||||
TriggerWord: "TriggerWord",
|
||||
}
|
||||
v := url.Values{}
|
||||
v.Set("token", "Token")
|
||||
v.Set("team_id", "TeamId")
|
||||
v.Set("team_domain", "TeamDomain")
|
||||
v.Set("channel_id", "ChannelId")
|
||||
v.Set("channel_name", "ChannelName")
|
||||
v.Set("timestamp", "123")
|
||||
v.Set("user_id", "UserId")
|
||||
v.Set("user_name", "UserName")
|
||||
v.Set("post_id", "PostId")
|
||||
v.Set("text", "Text")
|
||||
v.Set("trigger_word", "TriggerWord")
|
||||
if got, want := p.ToFormValues(), v.Encode(); !reflect.DeepEqual(got, want) {
|
||||
t.Fatalf("Got %+v, wanted %+v", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestOutgoingWebhookPreSave(t *testing.T) {
|
||||
|
||||
@@ -33,6 +33,7 @@ func NewSqlWebhookStore(sqlStore *SqlStore) WebhookStore {
|
||||
tableo.ColMap("CallbackURLs").SetMaxSize(1024)
|
||||
tableo.ColMap("DisplayName").SetMaxSize(64)
|
||||
tableo.ColMap("Description").SetMaxSize(128)
|
||||
tableo.ColMap("ContentType").SetMaxSize(128)
|
||||
}
|
||||
|
||||
return s
|
||||
@@ -44,6 +45,7 @@ func (s SqlWebhookStore) UpgradeSchemaIfNeeded() {
|
||||
|
||||
s.CreateColumnIfNotExists("OutgoingWebhooks", "DisplayName", "varchar(64)", "varchar(64)", "")
|
||||
s.CreateColumnIfNotExists("OutgoingWebhooks", "Description", "varchar(128)", "varchar(128)", "")
|
||||
s.CreateColumnIfNotExists("OutgoingWebhooks", "ContentType", "varchar(128)", "varchar(128)", "")
|
||||
}
|
||||
|
||||
func (s SqlWebhookStore) CreateIndexesIfNotExists() {
|
||||
|
||||
@@ -21,6 +21,7 @@ export default class AddOutgoingWebhook extends React.Component {
|
||||
|
||||
this.updateDisplayName = this.updateDisplayName.bind(this);
|
||||
this.updateDescription = this.updateDescription.bind(this);
|
||||
this.updateContentType = this.updateContentType.bind(this);
|
||||
this.updateChannelId = this.updateChannelId.bind(this);
|
||||
this.updateTriggerWords = this.updateTriggerWords.bind(this);
|
||||
this.updateCallbackUrls = this.updateCallbackUrls.bind(this);
|
||||
@@ -28,6 +29,7 @@ export default class AddOutgoingWebhook extends React.Component {
|
||||
this.state = {
|
||||
displayName: '',
|
||||
description: '',
|
||||
contentType: 'application/x-www-form-urlencoded',
|
||||
channelId: '',
|
||||
triggerWords: '',
|
||||
callbackUrls: '',
|
||||
@@ -103,6 +105,7 @@ export default class AddOutgoingWebhook extends React.Component {
|
||||
trigger_words: triggerWords,
|
||||
callback_urls: callbackUrls,
|
||||
display_name: this.state.displayName,
|
||||
content_type: this.state.contentType,
|
||||
description: this.state.description
|
||||
};
|
||||
|
||||
@@ -132,6 +135,12 @@ export default class AddOutgoingWebhook extends React.Component {
|
||||
});
|
||||
}
|
||||
|
||||
updateContentType(e) {
|
||||
this.setState({
|
||||
contentType: e.target.value
|
||||
});
|
||||
}
|
||||
|
||||
updateChannelId(e) {
|
||||
this.setState({
|
||||
channelId: e.target.value
|
||||
@@ -151,6 +160,8 @@ export default class AddOutgoingWebhook extends React.Component {
|
||||
}
|
||||
|
||||
render() {
|
||||
const contentTypeOption1 = 'application/x-www-form-urlencoded';
|
||||
const contentTypeOption2 = 'application/json';
|
||||
return (
|
||||
<div className='backstage-content'>
|
||||
<BackstageHeader>
|
||||
@@ -209,6 +220,35 @@ export default class AddOutgoingWebhook extends React.Component {
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
<div className='form-group'>
|
||||
<label
|
||||
className='control-label col-sm-4'
|
||||
htmlFor='contentType'
|
||||
>
|
||||
<FormattedMessage
|
||||
id='add_outgoing_webhook.content_Type'
|
||||
defaultMessage='Content Type'
|
||||
/>
|
||||
</label>
|
||||
<div className='col-md-5 col-sm-8'>
|
||||
<select
|
||||
className='form-control'
|
||||
value={this.state.contentType}
|
||||
onChange={this.updateContentType}
|
||||
>
|
||||
<option
|
||||
value={contentTypeOption1}
|
||||
>
|
||||
{contentTypeOption1}
|
||||
</option>
|
||||
<option
|
||||
value={contentTypeOption2}
|
||||
>
|
||||
{contentTypeOption2}
|
||||
</option>
|
||||
</select>
|
||||
</div>
|
||||
</div>
|
||||
<div className='form-group'>
|
||||
<label
|
||||
className='control-label col-sm-4'
|
||||
|
||||
@@ -112,22 +112,19 @@ export default class InstalledOutgoingWebhook extends React.Component {
|
||||
);
|
||||
}
|
||||
|
||||
const urls = [];
|
||||
for (const url of outgoingWebhook.callback_urls) {
|
||||
urls.push(
|
||||
<div
|
||||
key={url}
|
||||
className='item-details__url'
|
||||
>
|
||||
{url}
|
||||
</div>
|
||||
);
|
||||
urls.push(
|
||||
<br
|
||||
key={'BR' + url}
|
||||
/>
|
||||
);
|
||||
}
|
||||
let urls = (
|
||||
<div className='item-details__row'>
|
||||
<span className='item-details__url'>
|
||||
<FormattedMessage
|
||||
id='installed_integrations.callback_urls'
|
||||
defaultMessage='Callback URLs: {urls}'
|
||||
values={{
|
||||
urls: outgoingWebhook.callback_urls.join(', ')
|
||||
}}
|
||||
/>
|
||||
</span>
|
||||
</div>
|
||||
);
|
||||
|
||||
return (
|
||||
<div className='backstage-list__item'>
|
||||
@@ -138,6 +135,17 @@ export default class InstalledOutgoingWebhook extends React.Component {
|
||||
</span>
|
||||
</div>
|
||||
{description}
|
||||
<div className='item-details__row'>
|
||||
<span className='item-details__content_type'>
|
||||
<FormattedMessage
|
||||
id='installed_integrations.content_type'
|
||||
defaultMessage='Content-Type: {contentType}'
|
||||
values={{
|
||||
contentType: outgoingWebhook.content_type || 'application/x-www-form-urlencoded'
|
||||
}}
|
||||
/>
|
||||
</span>
|
||||
</div>
|
||||
{triggerWords}
|
||||
<div className='item-details__row'>
|
||||
<span className='item-details__token'>
|
||||
@@ -162,11 +170,7 @@ export default class InstalledOutgoingWebhook extends React.Component {
|
||||
/>
|
||||
</span>
|
||||
</div>
|
||||
<div className='item-details__row'>
|
||||
<span className='item-details__urls'>
|
||||
{urls}
|
||||
</span>
|
||||
</div>
|
||||
{urls}
|
||||
</div>
|
||||
<div className='item-actions'>
|
||||
<a
|
||||
|
||||
@@ -220,6 +220,7 @@ body {
|
||||
}
|
||||
|
||||
.item-details__description,
|
||||
.item-details__content_type,
|
||||
.item-details__token,
|
||||
.item-details__trigger-words,
|
||||
.item-details__url,
|
||||
|
||||
Ссылка в новой задаче
Block a user