Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: webhook header allowlist configuration option #4309

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+113 −27
Open

feat: webhook header allowlist configuration option #4309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions driver/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ const (
ViperKeyPasskeyRPDisplayName = "selfservice.methods.passkey.config.rp.display_name"
ViperKeyPasskeyRPID = "selfservice.methods.passkey.config.rp.id"
ViperKeyPasskeyRPOrigins = "selfservice.methods.passkey.config.rp.origins"
ViperKeyActionsWebhookHeaderAllowlist = "actions.web_hook.header_allowlist"
ViperKeyOAuth2ProviderURL = "oauth2_provider.url"
ViperKeyOAuth2ProviderHeader = "oauth2_provider.headers"
ViperKeyOAuth2ProviderOverrideReturnTo = "oauth2_provider.override_return_to"
Expand Down Expand Up @@ -954,6 +955,28 @@ func (p *Config) SelfAdminURL(ctx context.Context) *url.URL {
return p.baseURL(ctx, ViperKeyAdminBaseURL, ViperKeyAdminHost, ViperKeyAdminPort, 4434)
}

func (p *Config) ActionsWebhookHeaderAllowlist(ctx context.Context) []string {
return p.GetProvider(ctx).StringsF(ViperKeyActionsWebhookHeaderAllowlist, []string{
"Accept",
"Accept-Encoding",
"Accept-Language",
"Content-Length",
"Content-Type",
"Origin",
"Priority",
"Referer",
"Sec-Ch-Ua",
"Sec-Ch-Ua-Mobile",
"Sec-Ch-Ua-Platform",
"Sec-Fetch-Dest",
"Sec-Fetch-Mode",
"Sec-Fetch-Site",
"Sec-Fetch-User",
"True-Client-Ip",
"User-Agent",
})
}

func (p *Config) OAuth2ProviderHeader(ctx context.Context) http.Header {
hh := map[string]string{}
if err := p.GetProvider(ctx).Unmarshal(ViperKeyOAuth2ProviderHeader, &hh); err != nil {
Expand Down
40 changes: 40 additions & 0 deletions embedx/config.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2986,6 +2986,46 @@
}
},
"additionalProperties": false
},
"actions": {
"title": "Global actions-specific settings settings",
"type": "object",
"properties": {
"web_hook": {
"title": "Global web_hook action configuration",
"description": "Configure how the web_hook action behaves on all hooks.",
"type": "object",
"properties": {
"header_allowlist": {
"title": "Allowed request headers",
"description": "List of request headers that are forwarded to the web hook target in canonical form.",
"type": "array",
"items": {
"type": "string"
},
"default": [
"Accept",
"Accept-Encoding",
"Accept-Language",
"Content-Length",
"Content-Type",
"Origin",
"Priority",
"Referer",
"Sec-Ch-Ua",
"Sec-Ch-Ua-Mobile",
"Sec-Ch-Ua-Platform",
"Sec-Fetch-Dest",
"Sec-Fetch-Mode",
"Sec-Fetch-Site",
"Sec-Fetch-User",
"True-Client-Ip",
"User-Agent"
]
}
}
}
}
}
},
"allOf": [
Expand Down
33 changes: 9 additions & 24 deletions selfservice/hook/web_hook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
grpccodes "google.golang.org/grpc/codes"

"github.com/ory/herodot"
"github.com/ory/kratos/driver/config"
"github.com/ory/kratos/identity"
"github.com/ory/kratos/request"
"github.com/ory/kratos/schema"
Expand Down Expand Up @@ -74,6 +75,7 @@ type (
x.HTTPClientProvider
x.TracingProvider
jsonnetsecure.VMProvider
config.Provider
}

templateContext struct {
Expand Down Expand Up @@ -359,7 +361,7 @@ func (e *WebHook) execute(ctx context.Context, data *templateContext) error {
attribute.Bool("webhook.response.parse", parseResponse),
)

removeDisallowedHeaders(data)
removeDisallowedHeaders(data, e.deps.Config().ActionsWebhookHeaderAllowlist(ctx))

req, err := builder.BuildRequest(ctx, data)
if errors.Is(err, request.ErrCancel) {
Expand Down Expand Up @@ -429,32 +431,15 @@ func (e *WebHook) execute(ctx context.Context, data *templateContext) error {
return nil
}

// RequestHeaderAllowList contains the allowed request headers that are forwarded
// to the web hook target in canonical form (textproto.CanonicalMIMEHeaderKey).
var RequestHeaderAllowList = map[string]struct{}{
"Accept": {},
"Accept-Encoding": {},
"Accept-Language": {},
"Content-Length": {},
"Content-Type": {},
"Origin": {},
"Priority": {},
"Referer": {},
"Sec-Ch-Ua": {},
"Sec-Ch-Ua-Mobile": {},
"Sec-Ch-Ua-Platform": {},
"Sec-Fetch-Dest": {},
"Sec-Fetch-Mode": {},
"Sec-Fetch-Site": {},
"Sec-Fetch-User": {},
"True-Client-Ip": {},
"User-Agent": {},
}
func removeDisallowedHeaders(data *templateContext, headerAllowlist []string) {
allowedMap := make(map[string]struct{})
for _, header := range headerAllowlist {
allowedMap[header] = struct{}{}
}

func removeDisallowedHeaders(data *templateContext) {
headers := maps.Clone(data.RequestHeaders)
maps.DeleteFunc(headers, func(key string, _ []string) bool {
_, found := RequestHeaderAllowList[textproto.CanonicalMIMEHeaderKey(key)]
_, found := allowedMap[textproto.CanonicalMIMEHeaderKey(key)]
return !found
})
data.RequestHeaders = headers
Expand Down
44 changes: 41 additions & 3 deletions selfservice/hook/web_hook_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/sirupsen/logrus/hooks/test"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/tidwall/gjson"
"github.com/tidwall/sjson"
"go.opentelemetry.io/otel/attribute"
sdktrace "go.opentelemetry.io/otel/sdk/trace"
Expand Down Expand Up @@ -60,14 +61,39 @@ var transientPayload = json.RawMessage(`{
}`)

func TestWebHooks(t *testing.T) {
_, reg := internal.NewFastRegistryWithMocks(t)
ctx := context.Background()
conf, reg := internal.NewFastRegistryWithMocks(t)
logger := logrusx.New("kratos", "test")

conf.Set(ctx, config.ViperKeyActionsWebhookHeaderAllowlist, []string{
"Accept",
"Accept-Encoding",
"Accept-Language",
"Content-Length",
"Content-Type",
"Origin",
"Priority",
"Referer",
"Sec-Ch-Ua",
"Sec-Ch-Ua-Mobile",
"Sec-Ch-Ua-Platform",
"Sec-Fetch-Dest",
"Sec-Fetch-Mode",
"Sec-Fetch-Site",
"Sec-Fetch-User",
"True-Client-Ip",
"User-Agent",
"Valid-Header",
})

whDeps := struct {
x.SimpleLoggerWithClient
*jsonnetsecure.TestProvider
config.Provider
}{
x.SimpleLoggerWithClient{L: logger, C: reg.HTTPClient(context.Background()), T: otelx.NewNoop(logger, &otelx.Config{ServiceName: "kratos"})},
x.SimpleLoggerWithClient{L: logger, C: reg.HTTPClient(ctx), T: otelx.NewNoop(logger, &otelx.Config{ServiceName: "kratos"})},
jsonnetsecure.NewTestProvider(t),
reg,
}
type WebHookRequest struct {
Body string
Expand Down Expand Up @@ -337,7 +363,8 @@ func TestWebHooks(t *testing.T) {
Header: map[string][]string{
"Some-Header": {"Some-Value"},
"User-Agent": {"Foo-Bar-Browser"},
"Invalid-Header": {"ignored"},
"Invalid-Header": {"should be ignored"},
"Valid-Header": {"should not be ignored"},
"Cookie": {"Some-Cookie-1=Some-Cookie-Value; Some-Cookie-2=Some-other-Cookie-Value", "Some-Cookie-3=Third-Cookie-Value"},
},
RequestURI: "/some_end_point",
Expand Down Expand Up @@ -393,6 +420,11 @@ func TestWebHooks(t *testing.T) {
// According to the HTTP spec any request method, but TRACE is allowed to
// have a body. Even this is a really bad practice for some of them, like for
// GET
assert.Zero(t, gjson.Get(whr.Body, "headers.Invalid-Header"))
assert.NotZero(t, gjson.Get(whr.Body, "headers.Valid-Header"))
whr.Body, err = sjson.Delete(whr.Body, "headers.Valid-Header")
assert.NoError(t, err)

assert.JSONEq(t, tc.expectedBody(req, f, s), whr.Body)
} else {
assert.Emptyf(t, whr.Body, "HTTP %s is not allowed to have a body", method)
Expand Down Expand Up @@ -1005,9 +1037,11 @@ func TestDisallowPrivateIPRanges(t *testing.T) {
whDeps := struct {
x.SimpleLoggerWithClient
*jsonnetsecure.TestProvider
config.Provider
}{
x.SimpleLoggerWithClient{L: logger, C: reg.HTTPClient(context.Background()), T: otelx.NewNoop(logger, &otelx.Config{ServiceName: "kratos"})},
jsonnetsecure.NewTestProvider(t),
reg,
}

req := &http.Request{
Expand Down Expand Up @@ -1075,9 +1109,11 @@ func TestAsyncWebhook(t *testing.T) {
whDeps := struct {
x.SimpleLoggerWithClient
*jsonnetsecure.TestProvider
config.Provider
}{
x.SimpleLoggerWithClient{L: logger, C: reg.HTTPClient(context.Background()), T: otelx.NewNoop(logger, &otelx.Config{ServiceName: "kratos"})},
jsonnetsecure.NewTestProvider(t),
reg,
}

req := &http.Request{
Expand Down Expand Up @@ -1155,9 +1191,11 @@ func TestWebhookEvents(t *testing.T) {
whDeps := struct {
x.SimpleLoggerWithClient
*jsonnetsecure.TestProvider
config.Provider
}{
x.SimpleLoggerWithClient{L: logger, C: reg.HTTPClient(context.Background()), T: otelx.NewNoop(logger, &otelx.Config{ServiceName: "kratos"})},
jsonnetsecure.NewTestProvider(t),
reg,
}

req := &http.Request{
Expand Down
Loading