-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add Webhook authorization header (#20926)
_This is a different approach to #20267, I took the liberty of adapting some parts, see below_ ## Context In some cases, a weebhook endpoint requires some kind of authentication. The usual way is by sending a static `Authorization` header, with a given token. For instance: - Matrix expects a `Bearer <token>` (already implemented, by storing the header cleartext in the metadata - which is buggy on retry #19872) - TeamCity #18667 - Gitea instances #20267 - SourceHut https://man.sr.ht/graphql.md#authentication-strategies (this is my actual personal need :) ## Proposed solution Add a dedicated encrypt column to the webhook table (instead of storing it as meta as proposed in #20267), so that it gets available for all present and future hook types (especially the custom ones #19307). This would also solve the buggy matrix retry #19872. As a first step, I would recommend focusing on the backend logic and improve the frontend at a later stage. For now the UI is a simple `Authorization` field (which could be later customized with `Bearer` and `Basic` switches): ![2022-08-23-142911](https://user-images.githubusercontent.com/3864879/186162483-5b721504-eef5-4932-812e-eb96a68494cc.png) The header name is hard-coded, since I couldn't fine any usecase justifying otherwise. ## Questions - What do you think of this approach? @justusbunsi @Gusted @silverwind - ~~How are the migrations generated? Do I have to manually create a new file, or is there a command for that?~~ - ~~I started adding it to the API: should I complete it or should I drop it? (I don't know how much the API is actually used)~~ ## Done as well: - add a migration for the existing matrix webhooks and remove the `Authorization` logic there _Closes #19872_ Co-authored-by: Lunny Xiao <[email protected]> Co-authored-by: Gusted <[email protected]> Co-authored-by: delvh <[email protected]>
- Loading branch information
1 parent
085f717
commit b6e8135
Showing
25 changed files
with
671 additions
and
263 deletions.
There are no files selected for viewing
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
9 changes: 9 additions & 0 deletions
9
...s/migrations/fixtures/Test_addHeaderAuthorizationEncryptedColWebhook/expected_webhook.yml
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
# for matrix, the access_token has been moved to "header_authorization" | ||
- | ||
id: 1 | ||
meta: '{"homeserver_url":"https://matrix.example.com","room_id":"roomID","message_type":1}' | ||
header_authorization: "Bearer s3cr3t" | ||
- | ||
id: 2 | ||
meta: '' | ||
header_authorization: "" |
8 changes: 8 additions & 0 deletions
8
models/migrations/fixtures/Test_addHeaderAuthorizationEncryptedColWebhook/hook_task.yml
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
# unsafe payload | ||
- id: 1 | ||
hook_id: 1 | ||
payload_content: '{"homeserver_url":"https://matrix.example.com","room_id":"roomID","access_token":"s3cr3t","message_type":1}' | ||
# safe payload | ||
- id: 2 | ||
hook_id: 2 | ||
payload_content: '{"homeserver_url":"https://matrix.example.com","room_id":"roomID","message_type":1}' |
10 changes: 10 additions & 0 deletions
10
models/migrations/fixtures/Test_addHeaderAuthorizationEncryptedColWebhook/webhook.yml
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
# matrix webhook | ||
- id: 1 | ||
type: matrix | ||
meta: '{"homeserver_url":"https://matrix.example.com","room_id":"roomID","access_token":"s3cr3t","message_type":1}' | ||
header_authorization_encrypted: '' | ||
# gitea webhook | ||
- id: 2 | ||
type: gitea | ||
meta: '' | ||
header_authorization_encrypted: '' |
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,183 @@ | ||
// Copyright 2022 The Gitea Authors. All rights reserved. | ||
// Use of this source code is governed by a MIT-style | ||
// license that can be found in the LICENSE file. | ||
|
||
package v1_19 //nolint | ||
|
||
import ( | ||
"fmt" | ||
|
||
"code.gitea.io/gitea/models/webhook" | ||
"code.gitea.io/gitea/modules/json" | ||
"code.gitea.io/gitea/modules/secret" | ||
"code.gitea.io/gitea/modules/setting" | ||
api "code.gitea.io/gitea/modules/structs" | ||
|
||
"xorm.io/builder" | ||
"xorm.io/xorm" | ||
) | ||
|
||
func batchProcess[T any](x *xorm.Engine, buf []T, query func(limit, start int) *xorm.Session, process func(*xorm.Session, T) error) error { | ||
size := cap(buf) | ||
start := 0 | ||
for { | ||
err := query(size, start).Find(&buf) | ||
if err != nil { | ||
return err | ||
} | ||
if len(buf) == 0 { | ||
return nil | ||
} | ||
|
||
err = func() error { | ||
sess := x.NewSession() | ||
defer sess.Close() | ||
if err := sess.Begin(); err != nil { | ||
return fmt.Errorf("unable to allow start session. Error: %w", err) | ||
} | ||
for _, record := range buf { | ||
if err := process(sess, record); err != nil { | ||
return err | ||
} | ||
} | ||
return sess.Commit() | ||
}() | ||
if err != nil { | ||
return err | ||
} | ||
|
||
if len(buf) < size { | ||
return nil | ||
} | ||
start += size | ||
buf = buf[:0] | ||
} | ||
} | ||
|
||
func AddHeaderAuthorizationEncryptedColWebhook(x *xorm.Engine) error { | ||
// Add the column to the table | ||
type Webhook struct { | ||
ID int64 `xorm:"pk autoincr"` | ||
Type webhook.HookType `xorm:"VARCHAR(16) 'type'"` | ||
Meta string `xorm:"TEXT"` // store hook-specific attributes | ||
|
||
// HeaderAuthorizationEncrypted should be accessed using HeaderAuthorization() and SetHeaderAuthorization() | ||
HeaderAuthorizationEncrypted string `xorm:"TEXT"` | ||
} | ||
err := x.Sync(new(Webhook)) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
// Migrate the matrix webhooks | ||
|
||
type MatrixMeta struct { | ||
HomeserverURL string `json:"homeserver_url"` | ||
Room string `json:"room_id"` | ||
MessageType int `json:"message_type"` | ||
} | ||
type MatrixMetaWithAccessToken struct { | ||
MatrixMeta | ||
AccessToken string `json:"access_token"` | ||
} | ||
|
||
err = batchProcess(x, | ||
make([]*Webhook, 0, 50), | ||
func(limit, start int) *xorm.Session { | ||
return x.Where("type=?", "matrix").OrderBy("id").Limit(limit, start) | ||
}, | ||
func(sess *xorm.Session, hook *Webhook) error { | ||
// retrieve token from meta | ||
var withToken MatrixMetaWithAccessToken | ||
err := json.Unmarshal([]byte(hook.Meta), &withToken) | ||
if err != nil { | ||
return fmt.Errorf("unable to unmarshal matrix meta for webhook[id=%d]: %w", hook.ID, err) | ||
} | ||
if withToken.AccessToken == "" { | ||
return nil | ||
} | ||
|
||
// encrypt token | ||
authorization := "Bearer " + withToken.AccessToken | ||
hook.HeaderAuthorizationEncrypted, err = secret.EncryptSecret(setting.SecretKey, authorization) | ||
if err != nil { | ||
return fmt.Errorf("unable to encrypt access token for webhook[id=%d]: %w", hook.ID, err) | ||
} | ||
|
||
// remove token from meta | ||
withoutToken, err := json.Marshal(withToken.MatrixMeta) | ||
if err != nil { | ||
return fmt.Errorf("unable to marshal matrix meta for webhook[id=%d]: %w", hook.ID, err) | ||
} | ||
hook.Meta = string(withoutToken) | ||
|
||
// save in database | ||
count, err := sess.ID(hook.ID).Cols("meta", "header_authorization_encrypted").Update(hook) | ||
if count != 1 || err != nil { | ||
return fmt.Errorf("unable to update header_authorization_encrypted for webhook[id=%d]: %d,%w", hook.ID, count, err) | ||
} | ||
return nil | ||
}) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
// Remove access_token from HookTask | ||
|
||
type HookTask struct { | ||
ID int64 `xorm:"pk autoincr"` | ||
HookID int64 | ||
PayloadContent string `xorm:"LONGTEXT"` | ||
} | ||
|
||
type MatrixPayloadSafe struct { | ||
Body string `json:"body"` | ||
MsgType string `json:"msgtype"` | ||
Format string `json:"format"` | ||
FormattedBody string `json:"formatted_body"` | ||
Commits []*api.PayloadCommit `json:"io.gitea.commits,omitempty"` | ||
} | ||
type MatrixPayloadUnsafe struct { | ||
MatrixPayloadSafe | ||
AccessToken string `json:"access_token"` | ||
} | ||
|
||
err = batchProcess(x, | ||
make([]*HookTask, 0, 50), | ||
func(limit, start int) *xorm.Session { | ||
return x.Where(builder.And( | ||
builder.In("hook_id", builder.Select("id").From("webhook").Where(builder.Eq{"type": "matrix"})), | ||
builder.Like{"payload_content", "access_token"}, | ||
)).OrderBy("id").Limit(limit, 0) // ignore the provided "start", since other payload were already converted and don't contain 'payload_content' anymore | ||
}, | ||
func(sess *xorm.Session, hookTask *HookTask) error { | ||
// retrieve token from payload_content | ||
var withToken MatrixPayloadUnsafe | ||
err := json.Unmarshal([]byte(hookTask.PayloadContent), &withToken) | ||
if err != nil { | ||
return fmt.Errorf("unable to unmarshal payload_content for hook_task[id=%d]: %w", hookTask.ID, err) | ||
} | ||
if withToken.AccessToken == "" { | ||
return nil | ||
} | ||
|
||
// remove token from payload_content | ||
withoutToken, err := json.Marshal(withToken.MatrixPayloadSafe) | ||
if err != nil { | ||
return fmt.Errorf("unable to marshal payload_content for hook_task[id=%d]: %w", hookTask.ID, err) | ||
} | ||
hookTask.PayloadContent = string(withoutToken) | ||
|
||
// save in database | ||
count, err := sess.ID(hookTask.ID).Cols("payload_content").Update(hookTask) | ||
if count != 1 || err != nil { | ||
return fmt.Errorf("unable to update payload_content for hook_task[id=%d]: %d,%w", hookTask.ID, count, err) | ||
} | ||
return nil | ||
}) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
return nil | ||
} |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
// Copyright 2022 The Gitea Authors. All rights reserved. | ||
// Use of this source code is governed by a MIT-style | ||
// license that can be found in the LICENSE file. | ||
|
||
package v1_19 //nolint | ||
|
||
import ( | ||
"testing" | ||
|
||
"code.gitea.io/gitea/models/migrations/base" | ||
"code.gitea.io/gitea/models/webhook" | ||
"code.gitea.io/gitea/modules/json" | ||
"code.gitea.io/gitea/modules/secret" | ||
"code.gitea.io/gitea/modules/setting" | ||
|
||
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
func Test_addHeaderAuthorizationEncryptedColWebhook(t *testing.T) { | ||
// Create Webhook table | ||
type Webhook struct { | ||
ID int64 `xorm:"pk autoincr"` | ||
Type webhook.HookType `xorm:"VARCHAR(16) 'type'"` | ||
Meta string `xorm:"TEXT"` // store hook-specific attributes | ||
|
||
// HeaderAuthorizationEncrypted should be accessed using HeaderAuthorization() and SetHeaderAuthorization() | ||
HeaderAuthorizationEncrypted string `xorm:"TEXT"` | ||
} | ||
|
||
type ExpectedWebhook struct { | ||
ID int64 `xorm:"pk autoincr"` | ||
Meta string | ||
HeaderAuthorization string | ||
} | ||
|
||
type HookTask struct { | ||
ID int64 `xorm:"pk autoincr"` | ||
HookID int64 | ||
PayloadContent string `xorm:"LONGTEXT"` | ||
} | ||
|
||
// Prepare and load the testing database | ||
x, deferable := base.PrepareTestEnv(t, 0, new(Webhook), new(ExpectedWebhook), new(HookTask)) | ||
defer deferable() | ||
if x == nil || t.Failed() { | ||
return | ||
} | ||
|
||
if err := AddHeaderAuthorizationEncryptedColWebhook(x); err != nil { | ||
assert.NoError(t, err) | ||
return | ||
} | ||
|
||
expected := []ExpectedWebhook{} | ||
if err := x.Table("expected_webhook").Asc("id").Find(&expected); !assert.NoError(t, err) { | ||
return | ||
} | ||
|
||
got := []Webhook{} | ||
if err := x.Table("webhook").Select("id, meta, header_authorization_encrypted").Asc("id").Find(&got); !assert.NoError(t, err) { | ||
return | ||
} | ||
|
||
for i, e := range expected { | ||
assert.Equal(t, e.Meta, got[i].Meta) | ||
|
||
if e.HeaderAuthorization == "" { | ||
assert.Equal(t, "", got[i].HeaderAuthorizationEncrypted) | ||
} else { | ||
cipherhex := got[i].HeaderAuthorizationEncrypted | ||
cleartext, err := secret.DecryptSecret(setting.SecretKey, cipherhex) | ||
assert.NoError(t, err) | ||
assert.Equal(t, e.HeaderAuthorization, cleartext) | ||
} | ||
} | ||
|
||
// ensure that no hook_task has some remaining "access_token" | ||
hookTasks := []HookTask{} | ||
if err := x.Table("hook_task").Select("id, payload_content").Asc("id").Find(&hookTasks); !assert.NoError(t, err) { | ||
return | ||
} | ||
for _, h := range hookTasks { | ||
var m map[string]interface{} | ||
err := json.Unmarshal([]byte(h.PayloadContent), &m) | ||
assert.NoError(t, err) | ||
assert.Nil(t, m["access_token"]) | ||
} | ||
} |
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
Oops, something went wrong.