diff --git a/backend/README.md b/backend/README.md index 50f77963f..26b98ef0a 100644 --- a/backend/README.md +++ b/backend/README.md @@ -670,6 +670,366 @@ webhooks: - user ``` +#### Webhook Security Configuration + +Webhooks include comprehensive SSRF (Server-Side Request Forgery) protection to prevent attacks on internal networks and metadata endpoints. The security system validates callback URLs both at configuration time and during webhook delivery. + +##### Security Modes + +The webhook security mode determines which destination IPs are allowed: + +**`public_only` (default)** + +Only allows callbacks to public, routable IP addresses. Blocks: +- Private networks (10.0.0.0/8, 192.168.0.0/16, 172.16.0.0/12) +- Loopback addresses (127.0.0.0/8) +- Link-local addresses (169.254.0.0/16) +- Cloud metadata endpoints (169.254.169.254, fe80::/10, fc00::/7) +- Reserved IP ranges + +```yaml +webhooks: + enabled: true + security: + mode: public_only + allowed_schemes: + - https + hooks: + - callback: https://api.example.com/webhooks + events: + - user +``` + +> **Note:** In `public_only` mode, only `allowed_schemes` is effective. Any `allowed_*` or `blocked_*` configuration options (except `allowed_schemes`) will be ignored if configured, and a warning will be logged at startup. + +**`internal_only`** + +Only allows callbacks to internal/private IP addresses. Blocks all public IPs. Useful for deployments where webhooks should only target internal services: + +```yaml +webhooks: + enabled: true + security: + mode: internal_only + allowed_schemes: + - http + - https + hooks: + - callback: http://10.0.1.50/webhooks + events: + - user +``` + +> **Note:** In `internal_only` mode, only `allowed_schemes` is effective. Any `allowed_*` or `blocked_*` configuration options (except `allowed_schemes`) will be ignored if configured, and a warning will be logged at startup. + +**`custom` (requires explicit configuration)** + +Provides fine-grained control over webhook destinations. **At least one allowlist must be configured** (`allowed_hosts`, `allowed_domains`, or `allowed_cidrs`) - this follows the principle of least privilege with no implicit defaults. + +You can define security rules using: +- **Allowlists**: Explicitly permit specific hosts, domains, or IP ranges +- **Blocklists** (optional): Further restrict the allowed set by blocking specific destinations + +> **Important:** +> - `custom` mode requires **at least one allowlist** to be configured. Configuration will fail without one. +> - For each category (hosts, domains, CIDRs), you must choose either allowlist OR blocklist, not both. +> - If you want to allow all destinations, use `insecure` mode instead (not recommended for production). + +**Example: Allow specific hosts/domains** + +```yaml +webhooks: + enabled: true + security: + mode: custom + allowed_schemes: + - https + # Allow specific internal hosts (supports hostnames and IP addresses) + allowed_hosts: + - internal-webhook-server.local + - 192.168.1.100 + # Allow internal domains and all subdomains + allowed_domains: + - internal.company.com + # IMPORTANT: For hostnames to work, you must also allow their resolved IPs + # See "Understanding DNS Resolution and IP Validation" section below + allowed_cidrs: + - 192.168.1.0/24 # IP range where internal hostnames resolve + # Alternative: Use skip_resolved_ip_validation: true to trust DNS + hooks: + - callback: https://internal-webhook-server.local/hook + events: + - user.create + - callback: https://192.168.1.100/webhook + events: + - user.update +``` + +> **Note:** When using hostnames in `custom` mode, both the hostname AND its resolved IPs must be allowed. See the [DNS Resolution and IP Validation](#understanding-dns-resolution-and-ip-validation-in-custom-mode) section for details. + +**Example: Allow IP ranges with CIDRs** + +```yaml +webhooks: + enabled: true + security: + mode: custom + allowed_schemes: + - https + # Allow specific IP ranges (use CIDR notation) + allowed_cidrs: + - 10.0.0.0/24 # Entire subnet + - 192.168.1.50/32 # Single IP + hooks: + - callback: https://10.0.0.15/webhook + events: + - user +``` + +**Example: Allowlist with additional blocklist restrictions** + +```yaml +webhooks: + enabled: true + security: + mode: custom + allowed_schemes: + - https + # Allow broad set of domains + allowed_domains: + - example.com + # For domain names to work, also allow their resolved IPs + allowed_cidrs: + - 93.184.216.0/24 # IP range where example.com resolves + # Or use: skip_resolved_ip_validation: true + # But block specific subdomains (blocklist further restricts the allowed set) + blocked_hosts: + - suspicious.example.com + - test.example.com + hooks: + - callback: https://api.example.com/webhooks # ✅ Allowed + events: + - user + # callback: https://suspicious.example.com # ❌ Blocked +``` + +**Example: Mixed approach (different categories)** + +You can use allowlist for one category and blocklist for another: + +```yaml +webhooks: + enabled: true + security: + mode: custom + allowed_schemes: + - https + # Allowlist hostnames (only these hosts allowed) + allowed_hosts: + - api.example.com + - 93.184.216.34 # IP where api.example.com resolves + # Blocklist specific domains (further restrict by blocking subdomains) + blocked_domains: + - blocked.example.com + hooks: + - callback: https://api.example.com/webhooks # ✅ Allowed + events: + - user + # callback: https://blocked.example.com/hook # ❌ Blocked +``` + +> **Note on `allowed_hosts`:** This field accepts both hostnames and IP addresses. For example: +> ```yaml +> allowed_hosts: +> - webhook.example.com # hostname +> - 192.168.1.100 # IP address +> ``` +> Alternatively, you can use `allowed_cidrs` for IP ranges in CIDR notation (e.g., `192.168.1.100/32` for a single IP). + +**`insecure` (development only)** + +Allows any destination. **Not recommended for production.** + +```yaml +webhooks: + enabled: true + security: + mode: insecure + allowed_schemes: + - http + - https +``` + +##### Understanding DNS Resolution and IP Validation in Custom Mode + +**Important: Two-Phase Validation** + +When using hostnames in `custom` mode, webhook validation occurs in two phases: + +1. **Hostname Validation**: Checks if the hostname is in `allowed_hosts` or `allowed_domains` +2. **IP Validation**: After DNS resolution, checks if all resolved IPs are in `allowed_cidrs` (or are literal IPs in `allowed_hosts`) + +**Both phases must pass.** This defense-in-depth approach protects against: +- DNS hijacking/poisoning attacks +- DNS rebinding attacks (mitigated through IP pinning) +- Compromised DNS servers + +**Example - What Works:** + +```yaml +webhooks: + security: + mode: custom + allowed_hosts: + - webhook.example.com + allowed_cidrs: + - 93.184.216.0/24 # IP range where webhook.example.com resolves +``` + +**Example - What Doesn't Work:** + +```yaml +webhooks: + security: + mode: custom + allowed_hosts: + - webhook.example.com # ❌ Hostname alone is not enough + # Missing: allowed_cidrs for the resolved IPs +``` + +**Error you'll see:** +``` +resolved IP '93.184.216.34' for host 'webhook.example.com' is not allowed: +IP '93.184.216.34' is not in the allowed CIDR or host list +``` + +**Configuration Strategies:** + +1. **For hostnames with known IP ranges:** + ```yaml + allowed_hosts: [webhook.example.com] + allowed_cidrs: [93.184.216.0/24] + ``` + +2. **For hostnames with stable IPs:** + ```yaml + allowed_hosts: [webhook.example.com, 93.184.216.34] # Add resolved IP + ``` + +3. **For any public hostnames (simpler but less restrictive):** + ```yaml + mode: public_only # Allows any hostname that resolves to public IPs + ``` + +4. **For internal hostnames (trusted network):** + ```yaml + mode: internal_only # Allows hostnames resolving to private IPs + ``` + +**Trust DNS (Alternative Approach):** + +If you fully trust your DNS infrastructure, you can skip IP validation for allowed hostnames: + +```yaml +webhooks: + security: + mode: custom + allowed_hosts: + - webhook.example.com + skip_resolved_ip_validation: true # Trust DNS - resolved IPs auto-allowed +``` + +> ⚠️ **Security Warning:** Only enable `skip_resolved_ip_validation` if you fully trust your DNS infrastructure. +> If DNS is compromised, an attacker could make `webhook.example.com` resolve to internal services like `127.0.0.1`. +> The default (`false`) provides defense-in-depth by requiring both hostname AND IP validation. + +**Why This Design?** + +This two-phase approach provides defense-in-depth security: +- Even if DNS is compromised, an attacker cannot make `webhook.example.com` resolve to an arbitrary IP (unless `skip_resolved_ip_validation` is enabled) +- The resolved IP must also be in your allowed CIDR ranges +- Combined with IP pinning (automatic), this prevents DNS rebinding attacks + +**Note:** The system automatically pins validated IPs during webhook delivery, so even if DNS changes between validation and delivery, the connection goes to the validated IP. + +##### Redirect Security + +Control how webhooks handle HTTP redirects: + +```yaml +webhooks: + security: + mode: public_only + follow_redirects: true + max_redirects: 3 +``` + +> **Note:** Each redirect target is validated against the security policy. Set `follow_redirects: false` (default) to reject all redirects. + +##### Security Best Practices + +1. **Choose the appropriate mode:** + - Use `public_only` (default) for external webhooks to SaaS services + - Use `internal_only` when webhooks should only target internal services + - Use `custom` when you need fine-grained control with explicit allowlists + - Never use `insecure` mode in production +2. **Custom mode follows allowlist-first (principle of least privilege):** + - At least one allowlist (`allowed_hosts`, `allowed_domains`, or `allowed_cidrs`) is **required** + - Start with a narrow allowlist and expand as needed + - Use blocklists to further restrict if necessary +3. **Keep it simple:** For each category (hosts, domains, CIDRs), use either allowlist OR blocklist, not both +4. **Use HTTPS only** by setting `allowed_schemes: ["https"]` +5. **`allowed_hosts` accepts both hostnames and IPs** - use whichever is most appropriate for your use case +6. **Understand hostname vs IP validation in custom mode:** + - Hostnames must ALSO have their resolved IPs allowed via `allowed_cidrs` (default behavior) + - Or use `skip_resolved_ip_validation: true` if you fully trust your DNS + - For simpler configuration with hostnames, consider `public_only`/`internal_only` modes +7. **Disable redirects** unless required: `follow_redirects: false` +8. **Regularly review** webhook destinations and events +9. **Monitor webhook failures** for potential attack attempts +10. **Enable error sanitization in production** to prevent information disclosure: `sanitize_errors: true` + +##### Metadata Endpoint Protection + +The webhook system automatically blocks cloud provider metadata endpoints: +- AWS: 169.254.169.254 +- GCP: metadata.google.internal +- IPv6 metadata ranges +- Common DNS rebinding bypass attempts + +This protection is **always active** when `deny_metadata_endpoints: true` (default). + +##### Error Message Sanitization + +To prevent information disclosure through error messages, enable error sanitization: + +```yaml +webhooks: + security: + mode: public_only + sanitize_errors: true +``` + +When `sanitize_errors` is enabled: +- **Returned errors** are generic and don't reveal internal network details + - Instead of: `"resolved IP '10.0.0.5' for host 'internal.local' is not allowed"` + - Returns: `"callback destination not allowed"` +- **Detailed errors** are still logged internally for debugging +- **Recommended for production** to prevent information leakage during attacks + +**Example sanitized error messages:** +- `"callback URL validation failed"` - Generic validation failure +- `"callback URL not allowed"` - Host/domain blocked +- `"callback destination not allowed"` - IP blocked or invalid +- `"redirect destination not allowed"` - Redirect target blocked + +**Security vs. Debugging Trade-off:** +- **Development:** Set `sanitize_errors: false` for detailed debugging +- **Production:** Set `sanitize_errors: true` to prevent information disclosure +- Detailed errors are always available in server logs regardless of this setting + +For complete configuration options, see the [webhook configuration reference](https://github.com/teamhanko/hanko/wiki/config-properties-webhooks). + ### Session JWT templates You can define custom claims that will be added to session JWTs through the `session.jwt_template.claims` diff --git a/backend/config/config_default.go b/backend/config/config_default.go index 9100c2b24..ff9166b09 100644 --- a/backend/config/config_default.go +++ b/backend/config/config_default.go @@ -246,5 +246,24 @@ func DefaultConfig() *Config { Enabled: true, Store: FLOW_LOCKER_STORE_IN_MEMORY, }, + Webhooks: WebhookSettings{ + AllowTimeExpiration: false, + Enabled: false, + Security: WebhookSecurity{ + Mode: WebhookSecurityModePublicOnly, + AllowedSchemes: []string{"https"}, + FollowRedirects: false, + MaxRedirects: 0, + SkipResolvedIPValidation: false, + AllowedHosts: []string{}, + AllowedDomains: []string{}, + AllowedCIDRs: []string{}, + BlockedHosts: []string{}, + BlockedDomains: []string{}, + BlockedCIDRs: []string{}, + DenyMetadataEndpoints: true, + SanitizeErrors: true, + }, + }, } } diff --git a/backend/config/config_webhook.go b/backend/config/config_webhook.go index 7a2d2586b..19aaf82cd 100644 --- a/backend/config/config_webhook.go +++ b/backend/config/config_webhook.go @@ -3,18 +3,273 @@ package config import ( "encoding/json" "fmt" + "net" + "strings" + "github.com/invopop/jsonschema" + "github.com/rs/zerolog/log" "github.com/teamhanko/hanko/backend/v2/webhooks/events" - "net/url" - "strings" + "github.com/teamhanko/hanko/backend/v2/webhooks/validation" ) +type WebhookSecurityMode string + +const ( + WebhookSecurityModePublicOnly WebhookSecurityMode = "public_only" + WebhookSecurityModeInternalOnly WebhookSecurityMode = "internal_only" + WebhookSecurityModeCustom WebhookSecurityMode = "custom" + WebhookSecurityModeInsecure WebhookSecurityMode = "insecure" +) + +type WebhookSecurity struct { + // `mode` defines the outbound destination policy for webhook callbacks. + Mode WebhookSecurityMode `yaml:"mode" json:"mode,omitempty" koanf:"mode" jsonschema:"default=public_only,enum=public_only,enum=internal_only,enum=custom,enum=insecure"` + // `allowed_schemes` defines the allowed URL schemes for webhook callbacks. + AllowedSchemes []string `yaml:"allowed_schemes" json:"allowed_schemes,omitempty" koanf:"allowed_schemes"` + // `follow_redirects` determines whether webhook delivery follows redirects. + FollowRedirects bool `yaml:"follow_redirects" json:"follow_redirects,omitempty" koanf:"follow_redirects" jsonschema:"default=false"` + // `max_redirects` defines the maximum number of redirects to follow. + MaxRedirects int `yaml:"max_redirects" json:"max_redirects,omitempty" koanf:"max_redirects" jsonschema:"default=0"` + // `skip_resolved_ip_validation` determines whether IP validation is skipped for hostnames + // that passed hostname validation in custom mode. When true, if a hostname is in allowed_hosts + // or allowed_domains, its resolved IPs are automatically trusted. + // WARNING: Only enable this if you fully trust your DNS infrastructure. If DNS is compromised, + // an attacker could make an allowed hostname resolve to a malicious IP (e.g., internal services). + // When false (default), both hostname AND resolved IPs must be explicitly allowed (defense-in-depth). + SkipResolvedIPValidation bool `yaml:"skip_resolved_ip_validation" json:"skip_resolved_ip_validation,omitempty" koanf:"skip_resolved_ip_validation" jsonschema:"default=false"` + + // `allowed_hosts` defines exact hostnames or IP addresses that are explicitly allowed in `custom` mode. + // At least one of allowed_hosts, allowed_domains, or allowed_cidrs must be configured in custom mode. + AllowedHosts []string `yaml:"allowed_hosts" json:"allowed_hosts,omitempty" koanf:"allowed_hosts"` + // `allowed_domains` defines domains and subdomains that are explicitly allowed in `custom` mode. + // At least one of allowed_hosts, allowed_domains, or allowed_cidrs must be configured in custom mode. + AllowedDomains []string `yaml:"allowed_domains" json:"allowed_domains,omitempty" koanf:"allowed_domains"` + // `allowed_cidrs` defines IP ranges that are explicitly allowed in `custom` mode. + // At least one of allowed_hosts, allowed_domains, or allowed_cidrs must be configured in custom mode. + AllowedCIDRs []string `yaml:"allowed_cidrs" json:"allowed_cidrs,omitempty" koanf:"allowed_cidrs"` + + // `blocked_hosts` defines exact hostnames that are blocked. + BlockedHosts []string `yaml:"blocked_hosts" json:"blocked_hosts,omitempty" koanf:"blocked_hosts"` + // `blocked_domains` defines domains and subdomains that are blocked. + BlockedDomains []string `yaml:"blocked_domains" json:"blocked_domains,omitempty" koanf:"blocked_domains"` + // `blocked_cidrs` defines IP ranges that are blocked. + BlockedCIDRs []string `yaml:"blocked_cidrs" json:"blocked_cidrs,omitempty" koanf:"blocked_cidrs"` + + // `deny_metadata_endpoints` determines whether metadata service endpoints are blocked. + DenyMetadataEndpoints bool `yaml:"deny_metadata_endpoints" json:"deny_metadata_endpoints,omitempty" koanf:"deny_metadata_endpoints" jsonschema:"default=true"` + // `sanitize_errors` determines whether validation error messages are sanitized to prevent information disclosure. + // When enabled, generic error messages are returned instead of detailed validation errors. + // Detailed errors are still logged internally for debugging. + SanitizeErrors bool `yaml:"sanitize_errors" json:"sanitize_errors,omitempty" koanf:"sanitize_errors" jsonschema:"default=false"` +} + +// ToWebhookSecurityPolicy converts WebhookSecurity to validation.WebhookSecurityPolicy to avoid circular dependencies. +func (s *WebhookSecurity) ToWebhookSecurityPolicy() validation.WebhookSecurityPolicy { + return validation.WebhookSecurityPolicy{ + Mode: validation.SecurityMode(s.Mode), + AllowedSchemes: s.AllowedSchemes, + FollowRedirects: s.FollowRedirects, + MaxRedirects: s.MaxRedirects, + SkipResolvedIPValidation: s.SkipResolvedIPValidation, + AllowedHosts: s.AllowedHosts, + AllowedDomains: s.AllowedDomains, + AllowedCIDRs: s.AllowedCIDRs, + BlockedHosts: s.BlockedHosts, + BlockedDomains: s.BlockedDomains, + BlockedCIDRs: s.BlockedCIDRs, + DenyMetadataEndpoints: s.DenyMetadataEndpoints, + SanitizeErrors: s.SanitizeErrors, + } +} + +// GetAllowedSchemes returns the allowed URL schemes. +func (s *WebhookSecurity) GetAllowedSchemes() []string { + return s.AllowedSchemes +} + +func (s *WebhookSecurity) Validate() error { + if err := s.validateMode(); err != nil { + return err + } + + if err := s.validateAllowedSchemes(); err != nil { + return err + } + + if err := s.validateRedirectSettings(); err != nil { + return err + } + + // Mode-specific validation + if err := s.validateModeSpecificSettings(); err != nil { + return err + } + + // Only validate allowed/blocked lists if they will be used + if s.Mode == WebhookSecurityModeCustom { + // Custom mode requires at least one allowlist to be configured + if len(s.AllowedHosts) == 0 && len(s.AllowedDomains) == 0 && len(s.AllowedCIDRs) == 0 { + return fmt.Errorf("webhooks.security: custom mode requires at least one allow list (allowed_hosts, allowed_domains, or allowed_cidrs) to be configured. If you want to allow all destinations, use 'insecure' mode instead") + } + + // Validate mutual exclusivity + if err := s.validateMutualExclusivity(); err != nil { + return err + } + + if err := s.validateHostList("webhooks.security.allowed_hosts", s.AllowedHosts); err != nil { + return err + } + + if err := s.validateDomainList("webhooks.security.allowed_domains", s.AllowedDomains); err != nil { + return err + } + + if err := s.validateCIDRs("webhooks.security.allowed_cidrs", s.AllowedCIDRs); err != nil { + return err + } + + if err := s.validateHostList("webhooks.security.blocked_hosts", s.BlockedHosts); err != nil { + return err + } + + if err := s.validateDomainList("webhooks.security.blocked_domains", s.BlockedDomains); err != nil { + return err + } + + if err := s.validateCIDRs("webhooks.security.blocked_cidrs", s.BlockedCIDRs); err != nil { + return err + } + } + + return nil +} + +func (s *WebhookSecurity) validateMode() error { + switch s.Mode { + case WebhookSecurityModePublicOnly, WebhookSecurityModeInternalOnly, WebhookSecurityModeCustom, WebhookSecurityModeInsecure: + return nil + default: + return fmt.Errorf("webhooks.security.mode must be one of: public_only, internal_only, custom, insecure") + } +} + +func (s *WebhookSecurity) validateAllowedSchemes() error { + for i, scheme := range s.AllowedSchemes { + switch strings.ToLower(strings.TrimSpace(scheme)) { + case "http", "https": + default: + return fmt.Errorf("webhooks.security.allowed_schemes[%d] must be either 'http' or 'https'", i) + } + } + + return nil +} + +func (s *WebhookSecurity) validateRedirectSettings() error { + if !s.FollowRedirects && s.MaxRedirects != 0 { + return fmt.Errorf("webhooks.security.max_redirects must be 0 when follow_redirects is false") + } + + if s.MaxRedirects < 0 { + return fmt.Errorf("webhooks.security.max_redirects must be greater than or equal to 0") + } + + return nil +} + +func (s *WebhookSecurity) validateCIDRs(field string, cidrs []string) error { + for i, cidr := range cidrs { + if _, _, err := net.ParseCIDR(strings.TrimSpace(cidr)); err != nil { + return fmt.Errorf("%s[%d] is not a valid CIDR: %w", field, i, err) + } + } + + return nil +} + +func (s *WebhookSecurity) validateHostList(field string, hosts []string) error { + for i, host := range hosts { + if validation.NormalizeHost(host) == "" { + return fmt.Errorf("%s[%d] must not be empty", field, i) + } + } + + return nil +} + +func (s *WebhookSecurity) validateDomainList(field string, domains []string) error { + for i, domain := range domains { + normalized := validation.NormalizeHost(domain) + if normalized == "" { + return fmt.Errorf("%s[%d] must not be empty", field, i) + } + if strings.Contains(normalized, ":") { + return fmt.Errorf("%s[%d] must not contain a port", field, i) + } + } + + return nil +} + +func (s *WebhookSecurity) validateModeSpecificSettings() error { + // For public_only and internal_only modes, warn if allowed/blocked options are set + if s.Mode == WebhookSecurityModePublicOnly || s.Mode == WebhookSecurityModeInternalOnly { + var warnings []string + + if len(s.AllowedHosts) > 0 { + warnings = append(warnings, "allowed_hosts") + } + if len(s.AllowedDomains) > 0 { + warnings = append(warnings, "allowed_domains") + } + if len(s.AllowedCIDRs) > 0 { + warnings = append(warnings, "allowed_cidrs") + } + if len(s.BlockedHosts) > 0 { + warnings = append(warnings, "blocked_hosts") + } + if len(s.BlockedDomains) > 0 { + warnings = append(warnings, "blocked_domains") + } + if len(s.BlockedCIDRs) > 0 { + warnings = append(warnings, "blocked_cidrs") + } + + if len(warnings) > 0 { + log.Warn(). + Msgf("webhooks.security.mode=%s: the following set configuration options are ignored: %s (only allowed_schemes is effective in this mode)", s.Mode, strings.Join(warnings, ", ")) + } + } + + return nil +} + +func (s *WebhookSecurity) validateMutualExclusivity() error { + // Check pairwise mutual exclusivity for custom mode + if len(s.AllowedCIDRs) > 0 && len(s.BlockedCIDRs) > 0 { + return fmt.Errorf("webhooks.security: allowed_cidrs and blocked_cidrs are mutually exclusive - use either allowlist or blocklist, not both") + } + + if len(s.AllowedHosts) > 0 && len(s.BlockedHosts) > 0 { + return fmt.Errorf("webhooks.security: allowed_hosts and blocked_hosts are mutually exclusive - use either allowlist or blocklist, not both") + } + + if len(s.AllowedDomains) > 0 && len(s.BlockedDomains) > 0 { + return fmt.Errorf("webhooks.security: allowed_domains and blocked_domains are mutually exclusive - use either allowlist or blocklist, not both") + } + + return nil +} + type WebhookSettings struct { // `allow_time_expiration` determines whether webhooks are disabled when unused for 30 days // (only for database webhooks). AllowTimeExpiration bool `yaml:"allow_time_expiration" json:"allow_time_expiration,omitempty" koanf:"allow_time_expiration" jsonschema:"default=false"` // `enabled` enables the webhook feature. Enabled bool `yaml:"enabled" json:"enabled,omitempty" koanf:"enabled" jsonschema:"default=false"` + // `security` defines the outbound destination policy for webhook callbacks. + Security WebhookSecurity `yaml:"security,omitempty" json:"security,omitempty" koanf:"security" jsonschema:"title=security"` // `hooks` is a list of Webhook configurations. // // When using environment variables the value for the `WEBHOOKS_HOOKS` key must be specified in the following @@ -24,12 +279,17 @@ type WebhookSettings struct { } func (ws *WebhookSettings) Validate() error { - if ws.Enabled { - for _, hook := range ws.Hooks { - err := hook.Validate() - if err != nil { - return err - } + if !ws.Enabled { + return nil + } + + if err := ws.Security.Validate(); err != nil { + return err + } + + for i, hook := range ws.Hooks { + if err := hook.Validate(&ws.Security); err != nil { + return fmt.Errorf("webhooks.hooks[%d]: %w", i, err) } } @@ -51,8 +311,8 @@ func (wd *Webhooks) Decode(value string) error { return fmt.Errorf("invalid map json: %w", err) } webhooks = append(webhooks, webhook) - } + *wd = webhooks return nil } @@ -111,20 +371,12 @@ func (Webhook) JSONSchemaExtend(schema *jsonschema.Schema) { }} } -func (w *Webhook) Validate() error { - _, err := url.Parse(w.Callback) - if err != nil { - return fmt.Errorf("callback is not a valid URL: %w", err) +func (w *Webhook) Validate(sec *WebhookSecurity) error { + // Convert events.Events to []string for the shared validation + eventsStr := make([]string, len(w.Events)) + for i, e := range w.Events { + eventsStr[i] = string(e) } - if len(w.Events) > 0 { - for i, e := range w.Events { - isValid := events.IsValidEvent(e) - if !isValid { - return fmt.Errorf("event '%s' at events[%d] is not a valid webhook event", e, i) - } - } - } - - return nil + return validation.ValidateWebhook(w.Callback, eventsStr, sec) } diff --git a/backend/config/config_webhook_test.go b/backend/config/config_webhook_test.go index 606b93e2e..bf50951fc 100644 --- a/backend/config/config_webhook_test.go +++ b/backend/config/config_webhook_test.go @@ -1,18 +1,414 @@ package config import ( - "github.com/stretchr/testify/assert" "testing" + + "github.com/stretchr/testify/assert" + "github.com/teamhanko/hanko/backend/v2/webhooks/events" ) func TestWebhooks_Decode(t *testing.T) { webhooks := Webhooks{} value := "{\"callback\":\"http://app.com/usercb\",\"events\":[\"user\"]};{\"callback\":\"http://app.com/callback\",\"events\":[\"email.send\"]}" + err := webhooks.Decode(value) assert.NoError(t, err) - assert.Len(t, webhooks, 2, "has 2 elements") + assert.Len(t, webhooks, 2) for _, webhook := range webhooks { assert.IsType(t, Webhook{}, webhook) } } + +func TestWebhookSecurity_Validate_AcceptsValidSecurityConfigWithAllowlist(t *testing.T) { + security := WebhookSecurity{ + Mode: WebhookSecurityModeCustom, + AllowedSchemes: []string{"http", "https"}, + FollowRedirects: true, + MaxRedirects: 3, + AllowedHosts: []string{"localhost"}, + AllowedDomains: []string{"example.com"}, + AllowedCIDRs: []string{"127.0.0.0/8", "10.0.0.0/24"}, + DenyMetadataEndpoints: true, + } + + err := security.Validate() + + assert.NoError(t, err) +} + +func TestWebhookSecurity_Validate_AcceptsValidSecurityConfigWithBlocklist(t *testing.T) { + security := WebhookSecurity{ + Mode: WebhookSecurityModeCustom, + AllowedSchemes: []string{"http", "https"}, + AllowedHosts: []string{"example.com"}, + FollowRedirects: true, + MaxRedirects: 3, + BlockedDomains: []string{"internal.example"}, + BlockedCIDRs: []string{"169.254.169.254/32"}, + DenyMetadataEndpoints: true, + } + + err := security.Validate() + + assert.NoError(t, err) +} + +func TestWebhookSecurity_Validate_RejectsInvalidMode(t *testing.T) { + security := WebhookSecurity{ + Mode: "nope", + } + + err := security.Validate() + + assert.Error(t, err) + assert.Contains(t, err.Error(), "webhooks.security.mode") +} + +func TestWebhookSecurity_Validate_RejectsInvalidScheme(t *testing.T) { + security := WebhookSecurity{ + Mode: WebhookSecurityModePublicOnly, + AllowedSchemes: []string{"ftp"}, + } + + err := security.Validate() + + assert.Error(t, err) + assert.Contains(t, err.Error(), "allowed_schemes") +} + +func TestWebhookSecurity_Validate_RejectsInvalidRedirectConfiguration(t *testing.T) { + security := WebhookSecurity{ + Mode: WebhookSecurityModePublicOnly, + AllowedSchemes: []string{"https"}, + FollowRedirects: false, + MaxRedirects: 1, + } + + err := security.Validate() + + assert.Error(t, err) + assert.Contains(t, err.Error(), "max_redirects") +} + +func TestWebhookSecurity_Validate_RejectsNegativeRedirectCount(t *testing.T) { + security := WebhookSecurity{ + Mode: WebhookSecurityModePublicOnly, + AllowedSchemes: []string{"https"}, + FollowRedirects: true, + MaxRedirects: -1, + } + + err := security.Validate() + + assert.Error(t, err) + assert.Contains(t, err.Error(), "greater than or equal to 0") +} + +func TestWebhookSecurity_Validate_RejectsInvalidAllowedCIDR(t *testing.T) { + security := WebhookSecurity{ + Mode: WebhookSecurityModeCustom, + AllowedCIDRs: []string{"definitely-not-a-cidr"}, + } + + err := security.Validate() + + assert.Error(t, err) + assert.Contains(t, err.Error(), "allowed_cidrs") +} + +func TestWebhookSecurity_Validate_RejectsInvalidBlockedCIDR(t *testing.T) { + security := WebhookSecurity{ + Mode: WebhookSecurityModeCustom, + AllowedCIDRs: []string{"10.0.0.0/24"}, + BlockedCIDRs: []string{"still-not-a-cidr"}, + } + + err := security.Validate() + + assert.Error(t, err) + assert.Contains(t, err.Error(), "blocked_cidrs") +} + +func TestWebhookSecurity_Validate_RejectsEmptyAllowedHost(t *testing.T) { + security := WebhookSecurity{ + Mode: WebhookSecurityModeCustom, + AllowedHosts: []string{""}, + } + + err := security.Validate() + + assert.Error(t, err) + assert.Contains(t, err.Error(), "allowed_hosts") +} + +func TestWebhookSecurity_Validate_RejectsEmptyBlockedHost(t *testing.T) { + security := WebhookSecurity{ + Mode: WebhookSecurityModeCustom, + AllowedHosts: []string{"example.com"}, + BlockedHosts: []string{""}, + } + + err := security.Validate() + + assert.Error(t, err) + assert.Contains(t, err.Error(), "blocked_hosts") +} + +func TestWebhookSecurity_Validate_RejectsEmptyAllowedDomain(t *testing.T) { + security := WebhookSecurity{ + Mode: WebhookSecurityModeCustom, + AllowedDomains: []string{""}, + } + + err := security.Validate() + + assert.Error(t, err) + assert.Contains(t, err.Error(), "allowed_domains") +} + +func TestWebhookSecurity_Validate_RejectsAllowedDomainWithPort(t *testing.T) { + security := WebhookSecurity{ + Mode: WebhookSecurityModeCustom, + AllowedDomains: []string{"example.com:443"}, + } + + err := security.Validate() + + assert.Error(t, err) + assert.Contains(t, err.Error(), "must not contain a port") +} + +func TestWebhookSecurity_Validate_RejectsBlockedDomainWithPort(t *testing.T) { + security := WebhookSecurity{ + Mode: WebhookSecurityModeCustom, + AllowedHosts: []string{"allowed.com"}, + BlockedDomains: []string{"example.com:443"}, + } + + err := security.Validate() + + assert.Error(t, err) + assert.Contains(t, err.Error(), "must not contain a port") +} + +func TestWebhookSettings_Validate_DisabledSkipsValidation(t *testing.T) { + settings := WebhookSettings{ + Enabled: false, + Security: WebhookSecurity{ + Mode: "definitely-invalid", + }, + Hooks: Webhooks{ + { + Callback: "://broken", + Events: events.Events{events.Event("not-an-event")}, + }, + }, + } + + err := settings.Validate() + + assert.NoError(t, err) +} + +// Removed: URL validation tests that duplicate validator_test.go +// These tests validate callback URLs through WebhookSettings.Validate() which calls +// validation.ValidateWebhook(), duplicating tests already in validator_test.go. +// The validator_test.go file is the source of truth for validation logic. +// +// Integration testing of webhook validation is done in policy_test.go. + +func TestWebhookSecurity_Validate_CustomModeRequiresAtLeastOneAllowlist(t *testing.T) { + security := WebhookSecurity{ + Mode: WebhookSecurityModeCustom, + AllowedSchemes: []string{"https"}, + // No allowlists configured + } + + err := security.Validate() + + assert.Error(t, err) + assert.Contains(t, err.Error(), "requires at least one allow list") +} + +func TestWebhookSecurity_Validate_CustomModeAcceptsAllowedHosts(t *testing.T) { + security := WebhookSecurity{ + Mode: WebhookSecurityModeCustom, + AllowedSchemes: []string{"https"}, + AllowedHosts: []string{"example.com"}, + } + + err := security.Validate() + + assert.NoError(t, err) +} + +func TestWebhookSecurity_Validate_CustomModeAcceptsAllowedDomains(t *testing.T) { + security := WebhookSecurity{ + Mode: WebhookSecurityModeCustom, + AllowedSchemes: []string{"https"}, + AllowedDomains: []string{"example.com"}, + } + + err := security.Validate() + + assert.NoError(t, err) +} + +func TestWebhookSecurity_Validate_CustomModeAcceptsAllowedCIDRs(t *testing.T) { + security := WebhookSecurity{ + Mode: WebhookSecurityModeCustom, + AllowedSchemes: []string{"https"}, + AllowedCIDRs: []string{"10.0.0.0/24"}, + } + + err := security.Validate() + + assert.NoError(t, err) +} + +func TestWebhookSettings_Validate_RejectsInvalidEvent(t *testing.T) { + settings := WebhookSettings{ + Enabled: true, + Security: WebhookSecurity{ + Mode: WebhookSecurityModeInsecure, + AllowedSchemes: []string{"http", "https"}, + }, + Hooks: Webhooks{ + { + Callback: "http://example.com/webhook", + Events: events.Events{events.Event("not-a-valid-event")}, + }, + }, + } + + err := settings.Validate() + + assert.Error(t, err) + assert.Contains(t, err.Error(), "not a valid webhook event") +} + +// Integration test to ensure callback URL validation is wired up correctly +func TestWebhookSettings_Validate_IntegrationCallbackValidation(t *testing.T) { + // Test that webhook validation is actually called during settings validation + settings := WebhookSettings{ + Enabled: true, + Security: WebhookSecurity{ + Mode: WebhookSecurityModePublicOnly, + AllowedSchemes: []string{"https"}, + }, + Hooks: Webhooks{ + { + Callback: "https://example.com/webhook", + Events: events.Events{events.Event("user.create")}, + }, + }, + } + + err := settings.Validate() + + // This should pass - the integration between WebhookSettings and validation package works + assert.NoError(t, err) +} + +func TestWebhookSecurity_Validate_AllowsPublicOnlyWithIgnoredConfigs(t *testing.T) { + security := WebhookSecurity{ + Mode: WebhookSecurityModePublicOnly, + AllowedSchemes: []string{"https"}, + AllowedHosts: []string{"example.com"}, + BlockedDomains: []string{"blocked.com"}, + AllowedCIDRs: []string{"10.0.0.0/8"}, + } + + err := security.Validate() + + // Should not return an error - validation should pass + // (warnings will be logged but validation succeeds) + assert.NoError(t, err) +} + +func TestWebhookSecurity_Validate_AllowsInternalOnlyWithIgnoredConfigs(t *testing.T) { + security := WebhookSecurity{ + Mode: WebhookSecurityModeInternalOnly, + AllowedSchemes: []string{"http", "https"}, + AllowedCIDRs: []string{"10.0.0.0/8"}, + BlockedHosts: []string{"blocked.example.com"}, + AllowedDomains: []string{"example.com"}, + } + + err := security.Validate() + + // Should not return an error - validation should pass + // (warnings will be logged but validation succeeds) + assert.NoError(t, err) +} + +func TestWebhookSecurity_Validate_RejectsCustomWithBothAllowedAndBlockedCIDRs(t *testing.T) { + security := WebhookSecurity{ + Mode: WebhookSecurityModeCustom, + AllowedSchemes: []string{"https"}, + AllowedCIDRs: []string{"10.0.0.0/24"}, + BlockedCIDRs: []string{"192.168.0.0/24"}, + } + + err := security.Validate() + + assert.Error(t, err) + assert.Contains(t, err.Error(), "mutually exclusive") + assert.Contains(t, err.Error(), "allowed_cidrs") + assert.Contains(t, err.Error(), "blocked_cidrs") +} + +func TestWebhookSecurity_Validate_RejectsCustomWithBothAllowedAndBlockedHosts(t *testing.T) { + security := WebhookSecurity{ + Mode: WebhookSecurityModeCustom, + AllowedSchemes: []string{"https"}, + AllowedHosts: []string{"example.com"}, + BlockedHosts: []string{"blocked.com"}, + } + + err := security.Validate() + + assert.Error(t, err) + assert.Contains(t, err.Error(), "mutually exclusive") + assert.Contains(t, err.Error(), "allowed_hosts") + assert.Contains(t, err.Error(), "blocked_hosts") +} + +func TestWebhookSecurity_Validate_RejectsCustomWithBothAllowedAndBlockedDomains(t *testing.T) { + security := WebhookSecurity{ + Mode: WebhookSecurityModeCustom, + AllowedSchemes: []string{"https"}, + AllowedDomains: []string{"example.com"}, + BlockedDomains: []string{"blocked.com"}, + } + + err := security.Validate() + + assert.Error(t, err) + assert.Contains(t, err.Error(), "mutually exclusive") + assert.Contains(t, err.Error(), "allowed_domains") + assert.Contains(t, err.Error(), "blocked_domains") +} + +func TestWebhookSecurity_Validate_AllowsPublicOnlyWithOnlyAllowedSchemes(t *testing.T) { + security := WebhookSecurity{ + Mode: WebhookSecurityModePublicOnly, + AllowedSchemes: []string{"https"}, + } + + err := security.Validate() + + assert.NoError(t, err) +} + +func TestWebhookSecurity_Validate_AllowsInternalOnlyWithOnlyAllowedSchemes(t *testing.T) { + security := WebhookSecurity{ + Mode: WebhookSecurityModeInternalOnly, + AllowedSchemes: []string{"http", "https"}, + } + + err := security.Validate() + + assert.NoError(t, err) +} diff --git a/backend/go.mod b/backend/go.mod index bbafa1c1a..2aff50504 100644 --- a/backend/go.mod +++ b/backend/go.mod @@ -10,6 +10,7 @@ require ( github.com/coreos/go-oidc/v3 v3.17.0 github.com/evanphx/json-patch/v5 v5.9.11 github.com/fatih/structs v1.1.0 + github.com/foxcpp/go-mockdns v1.2.0 github.com/go-playground/validator/v10 v10.30.1 github.com/go-redsync/redsync/v4 v4.16.0 github.com/go-sql-driver/mysql v1.9.3 @@ -149,6 +150,7 @@ require ( github.com/mattn/go-isatty v0.0.20 // indirect github.com/mattn/go-sqlite3 v2.0.3+incompatible // indirect github.com/microcosm-cc/bluemonday v1.0.20 // indirect + github.com/miekg/dns v1.1.57 // indirect github.com/mitchellh/copystructure v1.2.0 // indirect github.com/mitchellh/reflectwalk v1.0.2 // indirect github.com/moby/docker-image-spec v1.3.1 // indirect @@ -191,6 +193,7 @@ require ( golang.org/x/sync v0.20.0 // indirect golang.org/x/sys v0.42.0 // indirect golang.org/x/time v0.14.0 // indirect + golang.org/x/tools v0.42.0 // indirect google.golang.org/protobuf v1.36.6 // indirect gopkg.in/alexcesaro/quotedprintable.v3 v3.0.0-20150716171945-2caba252f4dc // indirect gopkg.in/yaml.v2 v2.4.0 // indirect diff --git a/backend/go.sum b/backend/go.sum index 2f7442807..fc8874a30 100644 --- a/backend/go.sum +++ b/backend/go.sum @@ -102,6 +102,8 @@ github.com/fatih/structs v1.1.0 h1:Q7juDM0QtcnhCpeyLGQKyg4TOIghuNXrkL32pHAUMxo= github.com/fatih/structs v1.1.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga6PJ7M= github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg= github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= +github.com/foxcpp/go-mockdns v1.2.0 h1:omK3OrHRD1IWJz1FuFBCFquhXslXoF17OvBS6JPzZF0= +github.com/foxcpp/go-mockdns v1.2.0/go.mod h1:IhLeSFGed3mJIAXPH2aiRQB+kqz7oqu8ld2qVbOu7Wk= github.com/fsnotify/fsnotify v1.9.0 h1:2Ml+OJNzbYCTzsxtv8vKSFD9PbJjmhYF14k/jKC7S9k= github.com/fsnotify/fsnotify v1.9.0/go.mod h1:8jBTzvmWwFyi3Pb8djgCCO5IBqzKJ/Jwo8TRcHyHii0= github.com/fxamacker/cbor/v2 v2.6.0 h1:sU6J2usfADwWlYDAFhZBQ6TnLFBHxgesMrQfQgk1tWA= @@ -362,6 +364,8 @@ github.com/mattn/go-sqlite3 v2.0.3+incompatible h1:gXHsfypPkaMZrKbD5209QV9jbUTJK github.com/mattn/go-sqlite3 v2.0.3+incompatible/go.mod h1:FPy6KqzDD04eiIsT53CuJW3U88zkxoIYsOqkbpncsNc= github.com/microcosm-cc/bluemonday v1.0.20 h1:flpzsq4KU3QIYAYGV/szUat7H+GPOXR0B2JU5A1Wp8Y= github.com/microcosm-cc/bluemonday v1.0.20/go.mod h1:yfBmMi8mxvaZut3Yytv+jTXRY8mxyjJ0/kQBTElld50= +github.com/miekg/dns v1.1.57 h1:Jzi7ApEIzwEPLHWRcafCN9LZSBbqQpxjt/wpgvg7wcM= +github.com/miekg/dns v1.1.57/go.mod h1:uqRjCRUuEAA6qsOiJvDd+CFo/vW+y5WR6SNmHE55hZk= github.com/mileusna/useragent v1.3.5 h1:SJM5NzBmh/hO+4LGeATKpaEX9+b4vcGg2qXGLiNGDws= github.com/mileusna/useragent v1.3.5/go.mod h1:3d8TOmwL/5I8pJjyVDteHtgDGcefrFUX4ccGOMKNYYc= github.com/mitchellh/copystructure v1.2.0 h1:vpKXTN4ewci03Vljg/q9QvCGUDttBOGBIa15WveJJGw= @@ -541,6 +545,9 @@ golang.org/x/crypto v0.0.0-20210616213533-5ff15b29337e/go.mod h1:GvvjBRRGRdwPK5y golang.org/x/crypto v0.0.0-20210711020723-a769d52b0f97/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= +golang.org/x/crypto v0.13.0/go.mod h1:y6Z2r+Rw4iayiXXAIxJIDAJ1zMW4yaTpebo8fPOliYc= +golang.org/x/crypto v0.14.0/go.mod h1:MVFd36DqK4CsrnJYDkBA3VC4m2GkXAM0PvzMCn4JQf4= +golang.org/x/crypto v0.15.0/go.mod h1:4ChreQoLWfG3xLDer1WdlH5NdlQ3+mwnQq1YTKY+72g= golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU= golang.org/x/crypto v0.20.0/go.mod h1:Xwo95rrVNIoSMx9wa1JroENMToLWn3RNVrTBpLHgZPQ= golang.org/x/crypto v0.49.0 h1:+Ng2ULVvLHnJ/ZFEq4KdcDd/cfjrrjjNSXNzxg0Y4U4= @@ -550,6 +557,8 @@ golang.org/x/mod v0.0.0-20190513183733-4bf6d317e70e/go.mod h1:mXi4GBBbnImb6dmsKG golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= +golang.org/x/mod v0.12.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= +golang.org/x/mod v0.14.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= golang.org/x/mod v0.33.0 h1:tHFzIWbBifEmbwtGz65eaWyGiGZatSrT9prnU8DbVL8= golang.org/x/mod v0.33.0/go.mod h1:swjeQEj+6r7fODbD2cqrnje9PnziFuw4bmLbBZFrQ5w= golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= @@ -563,6 +572,9 @@ golang.org/x/net v0.0.0-20220826154423-83b083e8dc8b/go.mod h1:YDH+HFinaLZZlnHAfS golang.org/x/net v0.0.0-20221002022538-bcab6841153b/go.mod h1:YDH+HFinaLZZlnHAfSS6ZXJJ9M9t4Dl22yv3iI2vPwk= golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg= +golang.org/x/net v0.15.0/go.mod h1:idbUs1IY1+zTqbi8yxTbhexhEEk5ur9LInksu6HrEpk= +golang.org/x/net v0.17.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE= +golang.org/x/net v0.18.0/go.mod h1:/czyP5RqHAH4odGYxBJ1qz0+CE5WZ+2j1YgoEo8F2jQ= golang.org/x/net v0.21.0/go.mod h1:bIjVDfnllIU7BJ2DNgfnXvpSvtn8VRwhlsaeUTyUS44= golang.org/x/net v0.51.0 h1:94R/GTO7mt3/4wIKpcR5gkGmRLOuE/2hNGeWq/GBIFo= golang.org/x/net v0.51.0/go.mod h1:aamm+2QF5ogm02fjy5Bb7CQ0WMt1/WVM7FtyaTLlA9Y= @@ -572,6 +584,9 @@ golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220929204114-8fcdb60fdcc0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.3.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y= +golang.org/x/sync v0.4.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y= +golang.org/x/sync v0.5.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -598,6 +613,8 @@ golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.14.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.42.0 h1:omrd2nAlyT5ESRdCLYdm3+fMfNFE/+Rf4bDIQImRJeo= golang.org/x/sys v0.42.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= @@ -607,6 +624,9 @@ golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuX golang.org/x/term v0.0.0-20220722155259-a9ba230a4035/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo= +golang.org/x/term v0.12.0/go.mod h1:owVbMEjm3cBLCHdkQu9b1opXd4ETQWc3BhuQGKgXgvU= +golang.org/x/term v0.13.0/go.mod h1:LTmsnFJwVN6bCy1rVCoS+qHT1HhALEFxKncY3WNNh4U= +golang.org/x/term v0.14.0/go.mod h1:TySc+nGkYR6qt8km8wUhuFRTVSMIX3XPR58y2lC8vww= golang.org/x/term v0.17.0/go.mod h1:lLRBjIVuehSbZlaOtGMbcMncT+aqLLLmKrsjNrUguwk= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= @@ -616,6 +636,7 @@ golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= +golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE= golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/text v0.35.0 h1:JOVx6vVDFokkpaq1AEptVzLTpDe9KGpj5tR4/X+ybL8= golang.org/x/text v0.35.0/go.mod h1:khi/HExzZJ2pGnjenulevKNX1W67CUy0AsXcNubPGCA= @@ -632,6 +653,10 @@ golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtn golang.org/x/tools v0.0.0-20200103221440-774c71fcf114/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= +golang.org/x/tools v0.13.0/go.mod h1:HvlwmtVNQAhOuCjW7xxvovg8wbNq7LwfXh/k7wXUl58= +golang.org/x/tools v0.15.0/go.mod h1:hpksKq4dtpQWS1uQ61JkdqWM3LscIS6Slf+VVkm+wQk= +golang.org/x/tools v0.42.0 h1:uNgphsn75Tdz5Ji2q36v/nsFSfR/9BRFvqhGBaJGd5k= +golang.org/x/tools v0.42.0/go.mod h1:Ma6lCIwGZvHK6XtgbswSoWroEkhugApmsXyrUmBhfr0= golang.org/x/xerrors v0.0.0-20190410155217-1f06c39b4373/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20190513163551-3ee3066db522/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/backend/handler/webhook.go b/backend/handler/webhook.go index b19123f54..d658dd26e 100644 --- a/backend/handler/webhook.go +++ b/backend/handler/webhook.go @@ -14,6 +14,7 @@ import ( "github.com/teamhanko/hanko/backend/v2/persistence/models" "github.com/teamhanko/hanko/backend/v2/webhooks" "github.com/teamhanko/hanko/backend/v2/webhooks/events" + "github.com/teamhanko/hanko/backend/v2/webhooks/validation" ) type WebhookHandler interface { @@ -70,6 +71,17 @@ func (w *webhookHandler) Create(ctx echo.Context) error { return echo.NewHTTPError(http.StatusBadRequest, err) } + // Validate webhook callback URL with security policy + eventsStr := make([]string, len(dto.Events)) + for i, e := range dto.Events { + eventsStr[i] = string(e) + } + err = validation.ValidateWebhook(dto.Callback, eventsStr, &w.cfg.Security) + if err != nil { + ctx.Logger().Error(err) + return echo.NewHTTPError(http.StatusBadRequest, err) + } + now := time.Now() newUuid, err := uuid.NewV4() @@ -199,6 +211,17 @@ func (w *webhookHandler) Update(ctx echo.Context) error { return echo.NewHTTPError(http.StatusBadRequest, err) } + // Validate webhook callback URL with security policy + eventsStr := make([]string, len(dto.Events)) + for i, e := range dto.Events { + eventsStr[i] = string(e) + } + err = validation.ValidateWebhook(dto.Callback, eventsStr, &w.cfg.Security) + if err != nil { + ctx.Logger().Error(err) + return echo.NewHTTPError(http.StatusBadRequest, err) + } + return w.persister.Transaction(func(tx *pop.Connection) error { persister := w.persister.GetWebhookPersister(tx) diff --git a/backend/json_schema/hanko.config.json b/backend/json_schema/hanko.config.json index d7d10645a..6a8fc6a76 100644 --- a/backend/json_schema/hanko.config.json +++ b/backend/json_schema/hanko.config.json @@ -1798,6 +1798,97 @@ "type": "object", "title": "hooks" }, + "WebhookSecurity": { + "properties": { + "mode": { + "type": "string", + "enum": [ + "public_only", + "internal_only", + "custom", + "insecure" + ], + "description": "`mode` defines the outbound destination policy for webhook callbacks.", + "default": "public_only" + }, + "allowed_schemes": { + "items": { + "type": "string" + }, + "type": "array", + "description": "`allowed_schemes` defines the allowed URL schemes for webhook callbacks." + }, + "follow_redirects": { + "type": "boolean", + "description": "`follow_redirects` determines whether webhook delivery follows redirects.", + "default": false + }, + "max_redirects": { + "type": "integer", + "description": "`max_redirects` defines the maximum number of redirects to follow.", + "default": 0 + }, + "skip_resolved_ip_validation": { + "type": "boolean", + "description": "`skip_resolved_ip_validation` determines whether IP validation is skipped for hostnames\nthat passed hostname validation in custom mode. When true, if a hostname is in allowed_hosts\nor allowed_domains, its resolved IPs are automatically trusted.\nWARNING: Only enable this if you fully trust your DNS infrastructure. If DNS is compromised,\nan attacker could make an allowed hostname resolve to a malicious IP (e.g., internal services).\nWhen false (default), both hostname AND resolved IPs must be explicitly allowed (defense-in-depth).", + "default": false + }, + "allowed_hosts": { + "items": { + "type": "string" + }, + "type": "array", + "description": "`allowed_hosts` defines exact hostnames or IP addresses that are explicitly allowed in `custom` mode.\nAt least one of allowed_hosts, allowed_domains, or allowed_cidrs must be configured in custom mode." + }, + "allowed_domains": { + "items": { + "type": "string" + }, + "type": "array", + "description": "`allowed_domains` defines domains and subdomains that are explicitly allowed in `custom` mode.\nAt least one of allowed_hosts, allowed_domains, or allowed_cidrs must be configured in custom mode." + }, + "allowed_cidrs": { + "items": { + "type": "string" + }, + "type": "array", + "description": "`allowed_cidrs` defines IP ranges that are explicitly allowed in `custom` mode.\nAt least one of allowed_hosts, allowed_domains, or allowed_cidrs must be configured in custom mode." + }, + "blocked_hosts": { + "items": { + "type": "string" + }, + "type": "array", + "description": "`blocked_hosts` defines exact hostnames that are blocked." + }, + "blocked_domains": { + "items": { + "type": "string" + }, + "type": "array", + "description": "`blocked_domains` defines domains and subdomains that are blocked." + }, + "blocked_cidrs": { + "items": { + "type": "string" + }, + "type": "array", + "description": "`blocked_cidrs` defines IP ranges that are blocked." + }, + "deny_metadata_endpoints": { + "type": "boolean", + "description": "`deny_metadata_endpoints` determines whether metadata service endpoints are blocked.", + "default": true + }, + "sanitize_errors": { + "type": "boolean", + "description": "`sanitize_errors` determines whether validation error messages are sanitized to prevent information disclosure.\nWhen enabled, generic error messages are returned instead of detailed validation errors.\nDetailed errors are still logged internally for debugging.", + "default": false + } + }, + "additionalProperties": false, + "type": "object" + }, "WebhookSettings": { "properties": { "allow_time_expiration": { @@ -1810,6 +1901,11 @@ "description": "`enabled` enables the webhook feature.", "default": false }, + "security": { + "$ref": "#/$defs/WebhookSecurity", + "title": "security", + "description": "`security` defines the outbound destination policy for webhook callbacks." + }, "hooks": { "$ref": "#/$defs/Webhooks", "title": "hooks", diff --git a/backend/test/config.go b/backend/test/config.go index 8b5cd74ac..90ad63830 100644 --- a/backend/test/config.go +++ b/backend/test/config.go @@ -53,4 +53,14 @@ var DefaultConfig = config.Config{ Enabled: true, UserVerification: "preferred", }, + Webhooks: config.WebhookSettings{ + Enabled: true, + Security: config.WebhookSecurity{ + Mode: config.WebhookSecurityModeInsecure, + AllowedSchemes: []string{"http", "https"}, + FollowRedirects: false, + MaxRedirects: 0, + DenyMetadataEndpoints: true, + }, + }, } diff --git a/backend/webhooks/config_hook.go b/backend/webhooks/config_hook.go index 277f971b5..01fe23dbc 100644 --- a/backend/webhooks/config_hook.go +++ b/backend/webhooks/config_hook.go @@ -1,21 +1,23 @@ package webhooks import ( + "time" + "github.com/labstack/echo/v4" "github.com/teamhanko/hanko/backend/v2/config" - "time" ) type ConfigHook struct { BaseWebhook } -func NewConfigHook(cfgHook config.Webhook, logger echo.Logger) Webhook { +func NewConfigHook(cfgHook config.Webhook, security config.WebhookSecurity, logger echo.Logger) Webhook { return &ConfigHook{ BaseWebhook{ Logger: logger, Callback: cfgHook.Callback, Events: cfgHook.Events, + Security: security, }, } } diff --git a/backend/webhooks/config_hook_test.go b/backend/webhooks/config_hook_test.go index a229216ec..766ea2f84 100644 --- a/backend/webhooks/config_hook_test.go +++ b/backend/webhooks/config_hook_test.go @@ -1,21 +1,29 @@ package webhooks import ( + "testing" + "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/teamhanko/hanko/backend/v2/config" "github.com/teamhanko/hanko/backend/v2/webhooks/events" - "testing" - "time" ) +func testWebhookSecurity() config.WebhookSecurity { + return config.WebhookSecurity{ + Mode: config.WebhookSecurityModeInsecure, + AllowedSchemes: []string{"http", "https"}, + } +} + func TestNewConfigHook(t *testing.T) { hook := config.Webhook{ Callback: "http://lorem.ipsum", Events: events.Events{events.UserCreate}, } - cfgHook := NewConfigHook(hook, nil) + cfgHook := NewConfigHook(hook, testWebhookSecurity(), nil) require.NotEmpty(t, cfgHook) } @@ -26,8 +34,8 @@ func TestConfigHook_DisableOnExpiryDate(t *testing.T) { Events: events.Events{events.UserCreate}, } - dbHook := NewConfigHook(hook, nil) - err := dbHook.DisableOnExpiryDate(now) + cfgHook := NewConfigHook(hook, testWebhookSecurity(), nil) + err := cfgHook.DisableOnExpiryDate(now) assert.NoError(t, err) } @@ -37,8 +45,8 @@ func TestConfigHook_DisableOnFailure(t *testing.T) { Events: events.Events{events.UserCreate}, } - dbHook := NewConfigHook(hook, nil) - err := dbHook.DisableOnFailure() + cfgHook := NewConfigHook(hook, testWebhookSecurity(), nil) + err := cfgHook.DisableOnFailure() assert.NoError(t, err) } @@ -48,8 +56,8 @@ func TestConfigHook_Reset(t *testing.T) { Events: events.Events{events.UserCreate}, } - dbHook := NewConfigHook(hook, nil) - err := dbHook.Reset() + cfgHook := NewConfigHook(hook, testWebhookSecurity(), nil) + err := cfgHook.Reset() assert.NoError(t, err) } @@ -59,7 +67,7 @@ func TestConfigHook_IsEnabled(t *testing.T) { Events: events.Events{events.UserCreate}, } - dbHook := NewConfigHook(hook, nil) - isEnabled := dbHook.IsEnabled() + cfgHook := NewConfigHook(hook, testWebhookSecurity(), nil) + isEnabled := cfgHook.IsEnabled() require.True(t, isEnabled) } diff --git a/backend/webhooks/database_hook.go b/backend/webhooks/database_hook.go index af17dcd33..c1a3b89b4 100644 --- a/backend/webhooks/database_hook.go +++ b/backend/webhooks/database_hook.go @@ -2,11 +2,13 @@ package webhooks import ( "fmt" + "time" + "github.com/labstack/echo/v4" + "github.com/teamhanko/hanko/backend/v2/config" "github.com/teamhanko/hanko/backend/v2/persistence" "github.com/teamhanko/hanko/backend/v2/persistence/models" "github.com/teamhanko/hanko/backend/v2/webhooks/events" - "time" ) const ( @@ -19,12 +21,13 @@ type DatabaseHook struct { rawHook models.Webhook } -func NewDatabaseHook(dbHook models.Webhook, persister persistence.WebhookPersister, logger echo.Logger) Webhook { +func NewDatabaseHook(dbHook models.Webhook, persister persistence.WebhookPersister, security config.WebhookSecurity, logger echo.Logger) Webhook { return &DatabaseHook{ BaseWebhook{ Logger: logger, Callback: dbHook.Callback, Events: events.ConvertFromDbList(dbHook.WebhookEvents), + Security: security, }, persister, dbHook, @@ -32,7 +35,6 @@ func NewDatabaseHook(dbHook models.Webhook, persister persistence.WebhookPersist } func (dh *DatabaseHook) DisableOnExpiryDate(now time.Time) error { - // check expire date if dh.rawHook.ExpiresAt.Before(now) { dh.rawHook.Enabled = false @@ -47,7 +49,6 @@ func (dh *DatabaseHook) DisableOnExpiryDate(now time.Time) error { } func (dh *DatabaseHook) DisableOnFailure() error { - // increase Failure-Rate dh.rawHook.Failures++ if dh.rawHook.Failures > FailureExpireRate { diff --git a/backend/webhooks/database_hook_test.go b/backend/webhooks/database_hook_test.go index 3a7140384..3e14db2a9 100644 --- a/backend/webhooks/database_hook_test.go +++ b/backend/webhooks/database_hook_test.go @@ -6,6 +6,7 @@ import ( "github.com/gofrs/uuid" "github.com/stretchr/testify/suite" + "github.com/teamhanko/hanko/backend/v2/config" "github.com/teamhanko/hanko/backend/v2/persistence" "github.com/teamhanko/hanko/backend/v2/persistence/models" "github.com/teamhanko/hanko/backend/v2/test" @@ -20,24 +21,31 @@ type databaseHookSuite struct { test.Suite } +func (s *databaseHookSuite) testWebhookSecurity() config.WebhookSecurity { + return config.WebhookSecurity{ + Mode: config.WebhookSecurityModeInsecure, + AllowedSchemes: []string{"http", "https"}, + } +} + func (s *databaseHookSuite) TestNewDatabaseHook() { - hookId, err := uuid.NewV4() + hookID, err := uuid.NewV4() s.Require().NoError(err) hook := models.Webhook{ - ID: hookId, + ID: hookID, Enabled: false, Failures: 0, ExpiresAt: time.Now().Add(WebhookExpireDuration), } - dbHook := NewDatabaseHook(hook, s.Storage.GetWebhookPersister(nil), nil) + dbHook := NewDatabaseHook(hook, s.Storage.GetWebhookPersister(nil), s.testWebhookSecurity(), nil) s.NotEmpty(dbHook) } func (s *databaseHookSuite) TestDatabaseHook_DisableOnExpiryDate() { hook, whPersister := s.loadWebhook("8b00da9a-cacf-45ea-b25d-c1ce0f0d7da3") - dbHook := NewDatabaseHook(hook, whPersister, nil) + dbHook := NewDatabaseHook(hook, whPersister, s.testWebhookSecurity(), nil) now := time.Now() err := dbHook.DisableOnExpiryDate(now) @@ -48,10 +56,11 @@ func (s *databaseHookSuite) TestDatabaseHook_DisableOnExpiryDate() { s.False(updatedHook.Enabled) } + func (s *databaseHookSuite) TestDatabaseHook_DoNotDisableOnExpiryDate() { hook, whPersister := s.loadWebhook("a47fe92a-1e4b-4119-8653-55ad82737c88") - dbHook := NewDatabaseHook(hook, whPersister, nil) + dbHook := NewDatabaseHook(hook, whPersister, s.testWebhookSecurity(), nil) now := time.Now() err := dbHook.DisableOnExpiryDate(now) @@ -66,7 +75,7 @@ func (s *databaseHookSuite) TestDatabaseHook_DoNotDisableOnExpiryDate() { func (s *databaseHookSuite) TestDatabaseHook_DisableOnFailure() { hook, whPersister := s.loadWebhook("8b00da9a-cacf-45ea-b25d-c1ce0f0d7da2") - dbHook := NewDatabaseHook(hook, whPersister, nil) + dbHook := NewDatabaseHook(hook, whPersister, s.testWebhookSecurity(), nil) err := dbHook.DisableOnFailure() s.Require().NoError(err) @@ -79,7 +88,7 @@ func (s *databaseHookSuite) TestDatabaseHook_DisableOnFailure() { func (s *databaseHookSuite) TestDatabaseHook_DoNotDisableOnFailure() { hook, whPersister := s.loadWebhook("8b00da9a-cacf-45ea-b25d-c1ce0f0d7da3") - dbHook := NewDatabaseHook(hook, whPersister, nil) + dbHook := NewDatabaseHook(hook, whPersister, s.testWebhookSecurity(), nil) err := dbHook.DisableOnFailure() s.NoError(err) @@ -92,18 +101,16 @@ func (s *databaseHookSuite) TestDatabaseHook_DoNotDisableOnFailure() { func (s *databaseHookSuite) TestDatabaseHook_Reset() { hook, whPersister := s.loadWebhook("8b00da9a-cacf-45ea-b25d-c1ce0f0d7da2") - dbHook := NewDatabaseHook(hook, whPersister, nil) + dbHook := NewDatabaseHook(hook, whPersister, s.testWebhookSecurity(), nil) err := dbHook.Reset() s.NoError(err) updatedHook, err := whPersister.Get(hook.ID) s.Require().NoError(err) - // Failures should be reset s.Equal(0, updatedHook.Failures) s.Less(updatedHook.Failures, hook.Failures, "Failures should be reset to 0") - // Ensure timestamps moved forward relative to the original hook values s.True(updatedHook.UpdatedAt.After(hook.UpdatedAt), "UpdatedAt should be updated") s.True(updatedHook.ExpiresAt.After(hook.ExpiresAt), "ExpiresAt should be updated") } @@ -111,7 +118,7 @@ func (s *databaseHookSuite) TestDatabaseHook_Reset() { func (s *databaseHookSuite) TestDatabaseHook_IsEnabled() { hook, whPersister := s.loadWebhook("a47fe92a-1e4b-4119-8653-55ad82737c88") - dbHook := NewDatabaseHook(hook, whPersister, nil) + dbHook := NewDatabaseHook(hook, whPersister, s.testWebhookSecurity(), nil) s.True(dbHook.IsEnabled()) } @@ -119,17 +126,17 @@ func (s *databaseHookSuite) TestDatabaseHook_IsEnabled() { func (s *databaseHookSuite) TestDatabaseHook_IsDisabled() { hook, whPersister := s.loadWebhook("279beae1-8a6d-4eaf-a791-1fa79d21d37a") - dbHook := NewDatabaseHook(hook, whPersister, nil) + dbHook := NewDatabaseHook(hook, whPersister, s.testWebhookSecurity(), nil) s.False(dbHook.IsEnabled()) } -func (s *databaseHookSuite) loadWebhook(hookId string) (models.Webhook, persistence.WebhookPersister) { +func (s *databaseHookSuite) loadWebhook(hookID string) (models.Webhook, persistence.WebhookPersister) { err := s.LoadFixtures("../test/fixtures/webhooks") s.Require().NoError(err) whPersister := s.Storage.GetWebhookPersister(nil) - hook, err := whPersister.Get(uuid.FromStringOrNil(hookId)) + hook, err := whPersister.Get(uuid.FromStringOrNil(hookID)) s.Require().NoError(err) s.Require().NotEmpty(hook) diff --git a/backend/webhooks/dialer.go b/backend/webhooks/dialer.go new file mode 100644 index 000000000..b98c0c02f --- /dev/null +++ b/backend/webhooks/dialer.go @@ -0,0 +1,79 @@ +package webhooks + +import ( + "context" + "fmt" + "net" + "time" +) + +// ValidatedDialer is a custom dialer that enforces connections only to pre-validated IP addresses. +// This prevents DNS rebinding attacks by ensuring the HTTP client connects to the same IPs +// that were validated during the security check, rather than re-resolving DNS. +type ValidatedDialer struct { + // validatedIPs contains the list of IP addresses that passed security validation + validatedIPs []net.IP + + // originalHost is the hostname from the original URL (used for TLS SNI) + originalHost string + + // baseDialer is the underlying dialer used for actual connections + baseDialer *net.Dialer + + // currentIPIndex tracks which IP to try next (for round-robin with multiple IPs) + currentIPIndex int +} + +// NewValidatedDialer creates a new dialer that only connects to the specified validated IPs. +func NewValidatedDialer(validatedIPs []net.IP, originalHost string) *ValidatedDialer { + return &ValidatedDialer{ + validatedIPs: validatedIPs, + originalHost: originalHost, + baseDialer: &net.Dialer{Timeout: 30 * time.Second}, + currentIPIndex: 0, + } +} + +// DialContext implements a custom dialer that connects only to pre-validated IPs. +// It ignores the 'address' parameter from the HTTP client and uses the validated IPs instead. +// This prevents DNS rebinding by forcing connections to IPs validated at config/trigger time. +func (d *ValidatedDialer) DialContext(ctx context.Context, network, address string) (net.Conn, error) { + if len(d.validatedIPs) == 0 { + return nil, fmt.Errorf("no validated IPs available for connection") + } + + // Extract the port from the address (HTTP client provides "host:port") + _, port, err := net.SplitHostPort(address) + if err != nil { + return nil, fmt.Errorf("failed to extract port from address '%s': %w", address, err) + } + + // Try each validated IP until one succeeds or all fail + var lastErr error + startIdx := d.currentIPIndex + + for i := 0; i < len(d.validatedIPs); i++ { + // Round-robin through IPs + idx := (startIdx + i) % len(d.validatedIPs) + ip := d.validatedIPs[idx] + + // Construct the target address using the validated IP and the original port + targetAddr := net.JoinHostPort(ip.String(), port) + + conn, err := d.baseDialer.DialContext(ctx, network, targetAddr) + if err == nil { + // Update the current index for the next call + d.currentIPIndex = (idx + 1) % len(d.validatedIPs) + return conn, nil + } + + lastErr = err + } + + // All IPs failed + if lastErr != nil { + return nil, fmt.Errorf("all validated IPs failed to connect: %w", lastErr) + } + + return nil, fmt.Errorf("failed to connect to any validated IP") +} diff --git a/backend/webhooks/manager.go b/backend/webhooks/manager.go index f41e449c2..4d967c6d0 100644 --- a/backend/webhooks/manager.go +++ b/backend/webhooks/manager.go @@ -25,6 +25,7 @@ type manager struct { audience []string persister persistence.Persister canExpireAtTime bool + security config.WebhookSecurity } func NewManager(cfg *config.Config, persister persistence.Persister, jwtGenerator jwk.Generator, logger echo.Logger) (Manager, error) { @@ -32,7 +33,7 @@ func NewManager(cfg *config.Config, persister persistence.Persister, jwtGenerato if cfg.Webhooks.Enabled { for _, cfgHook := range cfg.Webhooks.Hooks { - hooks = append(hooks, NewConfigHook(cfgHook, logger)) + hooks = append(hooks, NewConfigHook(cfgHook, cfg.Webhooks.Security, logger)) } } @@ -50,11 +51,11 @@ func NewManager(cfg *config.Config, persister persistence.Persister, jwtGenerato audience: audience, persister: persister, canExpireAtTime: cfg.Webhooks.AllowTimeExpiration, + security: cfg.Webhooks.Security, }, nil } func (m *manager) Trigger(tx *pop.Connection, evt events.Event, data interface{}) { - // add db hooks - Done here to prevent a restart in case a hook is added or removed from the database dbHooks, err := m.persister.GetWebhookPersister(tx).List(false) if err != nil { m.logger.Error(fmt.Errorf("unable to get database webhooks: %w", err)) @@ -63,7 +64,7 @@ func (m *manager) Trigger(tx *pop.Connection, evt events.Event, data interface{} hooks := m.webhooks for _, dbHook := range dbHooks { - hooks = append(hooks, NewDatabaseHook(dbHook, m.persister.GetWebhookPersister(nil), m.logger)) + hooks = append(hooks, NewDatabaseHook(dbHook, m.persister.GetWebhookPersister(nil), m.security, m.logger)) } dataToken, err := m.GenerateJWT(data, evt) diff --git a/backend/webhooks/manager_test.go b/backend/webhooks/manager_test.go index bdcd527e6..8db76c31e 100644 --- a/backend/webhooks/manager_test.go +++ b/backend/webhooks/manager_test.go @@ -1,17 +1,19 @@ package webhooks import ( + "net/http" + "net/http/httptest" + "testing" + "time" + "github.com/gofrs/uuid" + "github.com/labstack/gommon/log" "github.com/stretchr/testify/suite" "github.com/teamhanko/hanko/backend/v2/config" "github.com/teamhanko/hanko/backend/v2/persistence" "github.com/teamhanko/hanko/backend/v2/persistence/models" "github.com/teamhanko/hanko/backend/v2/test" "github.com/teamhanko/hanko/backend/v2/webhooks/events" - "net/http" - "net/http/httptest" - "testing" - "time" ) func TestManagerSuite(t *testing.T) { @@ -23,20 +25,40 @@ type managerSuite struct { test.Suite } +func (s *managerSuite) testLogger() *log.Logger { + return log.New("test") +} + +func (s *managerSuite) testWebhookSecurity() config.WebhookSecurity { + return config.WebhookSecurity{ + Mode: config.WebhookSecurityModeInsecure, + AllowedSchemes: []string{"http", "https"}, + } +} + func (s *managerSuite) TestNewManager() { - cfg := config.Config{} + cfg := config.Config{ + Webhooks: config.WebhookSettings{ + Security: s.testWebhookSecurity(), + }, + } jwkManager := test.JwkManager{} - manager, err := NewManager(&cfg, s.Storage, jwkManager, nil) + manager, err := NewManager(&cfg, s.Storage, jwkManager, s.testLogger()) s.NoError(err) s.NotEmpty(manager) } func (s *managerSuite) TestManager_GenerateJWT() { - cfg := config.Config{} + cfg := config.Config{ + Webhooks: config.WebhookSettings{ + Security: s.testWebhookSecurity(), + }, + } jwkManager := test.JwkManager{} - manager, err := NewManager(&cfg, s.Storage, jwkManager, nil) + manager, err := NewManager(&cfg, s.Storage, jwkManager, s.testLogger()) + s.Require().NoError(err) testData := "lorem-ipsum" @@ -52,50 +74,57 @@ func (s *managerSuite) TestManager_TriggerWithoutHook() { })) defer server.Close() - cfg := config.Config{} + cfg := config.Config{ + Webhooks: config.WebhookSettings{ + Security: s.testWebhookSecurity(), + }, + } jwkManager := test.JwkManager{} - manager, err := NewManager(&cfg, s.Storage, jwkManager, nil) + manager, err := NewManager(&cfg, s.Storage, jwkManager, s.testLogger()) s.Require().NoError(err) manager.Trigger(s.Storage.GetConnection(), events.UserCreate, "lorem-ipsum") - // give it 1 sec to trigger - time.Sleep(1 * time.Second) - - s.False(triggered) + s.Never(func() bool { + return triggered + }, 1*time.Second, 50*time.Millisecond) } + func (s *managerSuite) TestManager_TriggerWithConfigHook() { triggered := false server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { triggered = true + w.WriteHeader(http.StatusOK) })) defer server.Close() - hooks := config.Webhooks{config.Webhook{ - Callback: server.URL, - Events: events.Events{ - events.UserCreate, + hooks := config.Webhooks{ + { + Callback: server.URL, + Events: events.Events{ + events.UserCreate, + }, }, - }} + } cfg := config.Config{ Webhooks: config.WebhookSettings{ - Enabled: true, - Hooks: hooks, + Enabled: true, + Hooks: hooks, + Security: s.testWebhookSecurity(), }, } jwkManager := test.JwkManager{} - manager, err := NewManager(&cfg, s.Storage, jwkManager, nil) + manager, err := NewManager(&cfg, s.Storage, jwkManager, s.testLogger()) s.Require().NoError(err) manager.Trigger(s.Storage.GetConnection(), events.UserCreate, "lorem-ipsum") - // give it 1 sec to trigger - time.Sleep(1 * time.Second) - - s.True(triggered) + s.Eventually(func() bool { + return triggered + }, 1*time.Second, 50*time.Millisecond) } func (s *managerSuite) TestManager_TriggerWithDisabledConfigHook() { @@ -105,55 +134,61 @@ func (s *managerSuite) TestManager_TriggerWithDisabledConfigHook() { })) defer server.Close() - hooks := config.Webhooks{config.Webhook{ - Callback: server.URL, - Events: events.Events{ - events.UserCreate, + hooks := config.Webhooks{ + { + Callback: server.URL, + Events: events.Events{ + events.UserCreate, + }, }, - }} + } cfg := config.Config{ Webhooks: config.WebhookSettings{ - Enabled: false, - Hooks: hooks, + Enabled: false, + Hooks: hooks, + Security: s.testWebhookSecurity(), }, } jwkManager := test.JwkManager{} - manager, err := NewManager(&cfg, s.Storage, jwkManager, nil) + manager, err := NewManager(&cfg, s.Storage, jwkManager, s.testLogger()) s.Require().NoError(err) manager.Trigger(s.Storage.GetConnection(), events.UserCreate, "lorem-ipsum") - // give it 1 sec to trigger - time.Sleep(1 * time.Second) - - s.False(triggered) + s.Never(func() bool { + return triggered + }, 1*time.Second, 50*time.Millisecond) } func (s *managerSuite) TestManager_TriggerWithDbHook() { triggered := false server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { triggered = true + w.WriteHeader(http.StatusOK) })) defer server.Close() - cfg := config.Config{} + cfg := config.Config{ + Webhooks: config.WebhookSettings{ + Security: s.testWebhookSecurity(), + }, + } jwkManager := test.JwkManager{} persister := s.Storage.GetWebhookPersister(nil) s.createTestDatabaseWebhook(persister, true, server.URL) - manager, err := NewManager(&cfg, s.Storage, jwkManager, nil) + manager, err := NewManager(&cfg, s.Storage, jwkManager, s.testLogger()) s.Require().NoError(err) manager.Trigger(s.Storage.GetConnection(), events.UserCreate, "lorem-ipsum") - // give it 1 sec to trigger - time.Sleep(1 * time.Second) - - s.True(triggered) + s.Eventually(func() bool { + return triggered + }, 1*time.Second, 50*time.Millisecond) } func (s *managerSuite) TestManager_TriggerWithDisabledDbHook() { @@ -163,29 +198,32 @@ func (s *managerSuite) TestManager_TriggerWithDisabledDbHook() { })) defer server.Close() - cfg := config.Config{} + cfg := config.Config{ + Webhooks: config.WebhookSettings{ + Security: s.testWebhookSecurity(), + }, + } jwkManager := test.JwkManager{} persister := s.Storage.GetWebhookPersister(nil) s.createTestDatabaseWebhook(persister, false, server.URL) - manager, err := NewManager(&cfg, s.Storage, jwkManager, nil) + manager, err := NewManager(&cfg, s.Storage, jwkManager, s.testLogger()) s.Require().NoError(err) manager.Trigger(s.Storage.GetConnection(), events.UserCreate, "lorem-ipsum") - // give it 1 sec to trigger - time.Sleep(1 * time.Second) - - s.False(triggered) + s.Never(func() bool { + return triggered + }, 1*time.Second, 50*time.Millisecond) } func (s *managerSuite) createTestDatabaseWebhook(persister persistence.WebhookPersister, isEnabled bool, callback string) { now := time.Now() - hookId := uuid.FromStringOrNil("8b00da9a-cacf-45ea-b25d-c1ce0f0d7da1") + hookID := uuid.FromStringOrNil("8b00da9a-cacf-45ea-b25d-c1ce0f0d7da1") err := persister.Create( models.Webhook{ - ID: hookId, + ID: hookID, Callback: callback, Enabled: isEnabled, Failures: 0, @@ -194,13 +232,14 @@ func (s *managerSuite) createTestDatabaseWebhook(persister persistence.WebhookPe UpdatedAt: now, }, models.WebhookEvents{ - models.WebhookEvent{ + { ID: uuid.FromStringOrNil("8b00da9a-cacf-45ea-b25d-c1ce0f0d7da0"), - WebhookID: hookId, + WebhookID: hookID, Event: string(events.UserCreate), CreatedAt: now, UpdatedAt: now, }, - }) + }, + ) s.Require().NoError(err) } diff --git a/backend/webhooks/policy.go b/backend/webhooks/policy.go new file mode 100644 index 000000000..bf8b0f9af --- /dev/null +++ b/backend/webhooks/policy.go @@ -0,0 +1,138 @@ +package webhooks + +import ( + "context" + "fmt" + "net" + "net/url" + "strings" + + "github.com/teamhanko/hanko/backend/v2/config" + "github.com/teamhanko/hanko/backend/v2/webhooks/validation" +) + +type URLPolicyValidator struct { + security config.WebhookSecurity + resolver *net.Resolver +} + +func NewURLPolicyValidator(security config.WebhookSecurity) *URLPolicyValidator { + return &URLPolicyValidator{ + security: security, + resolver: net.DefaultResolver, + } +} + +// ValidationResult contains the result of URL validation including validated IPs +// that can be used to prevent DNS rebinding attacks. +type ValidationResult struct { + // ValidatedIPs contains the IP addresses that passed security validation + ValidatedIPs []net.IP + // Host is the normalized hostname from the URL + Host string +} + +// ValidateAndGetIPs validates the URL and returns the validated IP addresses. +// This allows callers to pin connections to validated IPs, preventing DNS rebinding. +func (v *URLPolicyValidator) ValidateAndGetIPs(ctx context.Context, rawURL string) (*ValidationResult, error) { + parsed, err := url.Parse(rawURL) + if err != nil { + detailedErr := fmt.Errorf("invalid webhook callback URL: %w", err) + return nil, validation.SanitizeError(detailedErr, v.security.SanitizeErrors) + } + + host, err := v.validateParsedURL(parsed) + if err != nil { + return nil, err + } + + // If the host is already a literal IP, return it directly + if ip := net.ParseIP(host); ip != nil { + if err := v.validateResolvedIP(ip, false); err != nil { + return nil, err + } + return &ValidationResult{ + ValidatedIPs: []net.IP{ip}, + Host: host, + }, nil + } + + // Resolve the hostname and validate all returned IPs + ips, err := v.resolver.LookupIP(ctx, "ip", host) + if err != nil { + detailedErr := fmt.Errorf("failed to resolve webhook callback host '%s': %w", host, err) + return nil, validation.SanitizeError(detailedErr, v.security.SanitizeErrors) + } + + if len(ips) == 0 { + detailedErr := fmt.Errorf("webhook callback host '%s' did not resolve to any IP addresses", host) + return nil, validation.SanitizeError(detailedErr, v.security.SanitizeErrors) + } + + // All resolved IPs must satisfy the outbound policy. + // Rejecting on any disallowed IP avoids mixed public/internal DNS answers. + for _, ip := range ips { + if err := v.validateResolvedIP(ip, true); err != nil { + detailedErr := fmt.Errorf("resolved IP '%s' for host '%s' is not allowed: %w", ip.String(), host, err) + return nil, validation.SanitizeError(detailedErr, v.security.SanitizeErrors) + } + } + + return &ValidationResult{ + ValidatedIPs: ips, + Host: host, + }, nil +} + +// Validate validates the URL without returning the validated IPs. +// This is kept for backward compatibility but ValidateAndGetIPs is preferred +// for protection against DNS rebinding attacks. +func (v *URLPolicyValidator) Validate(ctx context.Context, rawURL string) error { + _, err := v.ValidateAndGetIPs(ctx, rawURL) + return err +} + +func (v *URLPolicyValidator) validateParsedURL(parsed *url.URL) (string, error) { + if parsed.Scheme == "" { + detailedErr := fmt.Errorf("webhook callback URL must include a scheme") + return "", validation.SanitizeError(detailedErr, v.security.SanitizeErrors) + } + + if parsed.Host == "" { + detailedErr := fmt.Errorf("webhook callback URL must include a host") + return "", validation.SanitizeError(detailedErr, v.security.SanitizeErrors) + } + + if parsed.User != nil { + detailedErr := fmt.Errorf("webhook callback URL must not include user info") + return "", validation.SanitizeError(detailedErr, v.security.SanitizeErrors) + } + + schemeAllowed := false + for _, scheme := range v.security.AllowedSchemes { + if strings.EqualFold(strings.TrimSpace(scheme), parsed.Scheme) { + schemeAllowed = true + break + } + } + + if !schemeAllowed { + detailedErr := fmt.Errorf("webhook callback scheme '%s' is not allowed", parsed.Scheme) + return "", validation.SanitizeError(detailedErr, v.security.SanitizeErrors) + } + + host := parsed.Hostname() + validator := validation.NewValidator(v.security.ToWebhookSecurityPolicy()) + + if err := validator.ValidateHost(host); err != nil { + // err is already sanitized by validator + return "", fmt.Errorf("webhook callback %w", err) + } + + return validation.NormalizeHost(host), nil +} + +func (v *URLPolicyValidator) validateResolvedIP(ip net.IP, wasHostnameValidated bool) error { + validator := validation.NewValidator(v.security.ToWebhookSecurityPolicy()) + return validator.ValidateIP(ip, wasHostnameValidated) +} diff --git a/backend/webhooks/policy_test.go b/backend/webhooks/policy_test.go new file mode 100644 index 000000000..014a2e912 --- /dev/null +++ b/backend/webhooks/policy_test.go @@ -0,0 +1,466 @@ +package webhooks + +import ( + "context" + "net" + "testing" + + "github.com/foxcpp/go-mockdns" + "github.com/stretchr/testify/require" + "github.com/teamhanko/hanko/backend/v2/config" +) + +func TestURLPolicyValidator_Validate_RejectsInvalidURL(t *testing.T) { + validator := NewURLPolicyValidator(config.WebhookSecurity{ + Mode: config.WebhookSecurityModeInsecure, + AllowedSchemes: []string{"http", "https"}, + }) + + err := validator.Validate(context.Background(), "://broken") + + require.Error(t, err) + require.ErrorContains(t, err, "invalid webhook callback URL") +} + +func TestURLPolicyValidator_Validate_RejectsMissingScheme(t *testing.T) { + validator := NewURLPolicyValidator(config.WebhookSecurity{ + Mode: config.WebhookSecurityModeInsecure, + AllowedSchemes: []string{"http", "https"}, + }) + + err := validator.Validate(context.Background(), "//example.com") + + require.Error(t, err) + require.ErrorContains(t, err, "must include a scheme") +} + +func TestURLPolicyValidator_Validate_RejectsDisallowedScheme(t *testing.T) { + validator := NewURLPolicyValidator(config.WebhookSecurity{ + Mode: config.WebhookSecurityModeInsecure, + AllowedSchemes: []string{"https"}, + }) + + err := validator.Validate(context.Background(), "http://127.0.0.1/webhook") + + require.Error(t, err) + require.ErrorContains(t, err, "scheme") +} + +func TestURLPolicyValidator_Validate_RejectsUserInfo(t *testing.T) { + validator := NewURLPolicyValidator(config.WebhookSecurity{ + Mode: config.WebhookSecurityModeInsecure, + AllowedSchemes: []string{"http", "https"}, + }) + + err := validator.Validate(context.Background(), "http://user:pass@127.0.0.1/webhook") + + require.Error(t, err) + require.ErrorContains(t, err, "user info") +} + +// Removed: Tests that duplicate validator_test.go +// These tests were testing the underlying validation logic which is already +// thoroughly tested in validation/validator_test.go. The validator package +// is the source of truth for validation logic (host/domain/IP/CIDR matching). +// +// Kept: URL-layer tests (parsing, schemes, userinfo) that are unique to policy.go +// Kept: DNS resolution tests with go-mockdns (all tests below use mocked DNS) + +func TestURLPolicyValidator_Validate_HostnameResolvesToIPInCIDR(t *testing.T) { + // Test that hostname resolution + IP validation works correctly + // Hostname resolves to IP that IS in allowed CIDR + validator, srv, err := newValidatorWithMockDNS( + config.WebhookSecurity{ + Mode: config.WebhookSecurityModeCustom, + AllowedSchemes: []string{"https"}, + AllowedHosts: []string{"webhook.test"}, + AllowedCIDRs: []string{"10.0.0.0/8"}, + }, + map[string]mockdns.Zone{ + "webhook.test.": {A: []string{"10.0.0.5"}}, // In 10.0.0.0/8 + }, + ) + require.NoError(t, err) + defer srv.Close() + + err = validator.Validate(context.Background(), "https://webhook.test/hook") + + require.NoError(t, err) +} + +func TestURLPolicyValidator_Validate_HostnameResolvesToIPNotInCIDR(t *testing.T) { + // Test that hostname resolution + IP validation correctly rejects IPs not in CIDR + // Hostname resolves to IP that is NOT in allowed CIDR + validator, srv, err := newValidatorWithMockDNS( + config.WebhookSecurity{ + Mode: config.WebhookSecurityModeCustom, + AllowedSchemes: []string{"https"}, + AllowedHosts: []string{"webhook.test"}, + AllowedCIDRs: []string{"192.168.1.0/24"}, + }, + map[string]mockdns.Zone{ + "webhook.test.": {A: []string{"10.0.0.5"}}, // NOT in 192.168.1.0/24 + }, + ) + require.NoError(t, err) + defer srv.Close() + + err = validator.Validate(context.Background(), "https://webhook.test/hook") + + require.Error(t, err) + require.Contains(t, err.Error(), "not in the allowed CIDR") +} + +func TestURLPolicyValidator_Validate_HostnameResolvesAndReturnsMultipleIPs(t *testing.T) { + // Test that ValidateAndGetIPs returns all validated IPs for a hostname + validator, srv, err := newValidatorWithMockDNS( + config.WebhookSecurity{ + Mode: config.WebhookSecurityModeCustom, + AllowedSchemes: []string{"https"}, + AllowedHosts: []string{"webhook.test"}, + AllowedCIDRs: []string{"10.0.0.0/8"}, + }, + map[string]mockdns.Zone{ + "webhook.test.": { + A: []string{"10.0.0.5", "10.0.0.6"}, // Multiple IPs in allowed CIDR + }, + }, + ) + require.NoError(t, err) + defer srv.Close() + + result, err := validator.ValidateAndGetIPs(context.Background(), "https://webhook.test/hook") + + require.NoError(t, err) + require.NotNil(t, result) + require.Len(t, result.ValidatedIPs, 2) + require.Equal(t, "10.0.0.5", result.ValidatedIPs[0].String()) + require.Equal(t, "10.0.0.6", result.ValidatedIPs[1].String()) + require.Equal(t, "webhook.test", result.Host) +} + +func TestURLPolicyValidator_Validate_HostnameWithSkipResolvedIPValidation(t *testing.T) { + // Test that SkipResolvedIPValidation allows hostname whose resolved IP is not in CIDR + validator, srv, err := newValidatorWithMockDNS( + config.WebhookSecurity{ + Mode: config.WebhookSecurityModeCustom, + AllowedSchemes: []string{"https"}, + AllowedHosts: []string{"webhook.test"}, + AllowedCIDRs: []string{"10.0.0.0/8"}, + SkipResolvedIPValidation: true, + }, + map[string]mockdns.Zone{ + "webhook.test.": {A: []string{"8.8.8.8"}}, // NOT in 10.0.0.0/8, but skip flag is set + }, + ) + require.NoError(t, err) + defer srv.Close() + + err = validator.Validate(context.Background(), "https://webhook.test/hook") + + require.NoError(t, err) +} + +func newValidatorWithMockDNS(security config.WebhookSecurity, zones map[string]mockdns.Zone) (*URLPolicyValidator, *mockdns.Server, error) { + srv, err := mockdns.NewServer(zones, false) + if err != nil { + return nil, nil, err + } + + validator := NewURLPolicyValidator(security) + validator.resolver = &net.Resolver{ + PreferGo: true, + Dial: func(ctx context.Context, network, address string) (net.Conn, error) { + return net.Dial("udp", srv.LocalAddr().String()) + }, + } + + return validator, srv, nil +} + +func TestURLPolicyValidator_DNSRebinding_RejectsPrivateIPInPublicOnlyMode(t *testing.T) { + // Simulate an attacker controlling DNS to return a private IP + validator, srv, err := newValidatorWithMockDNS( + config.WebhookSecurity{ + Mode: config.WebhookSecurityModePublicOnly, + AllowedSchemes: []string{"https"}, + }, + map[string]mockdns.Zone{ + "evil.example.com.": {A: []string{"10.0.0.1"}}, // Private IP + }, + ) + require.NoError(t, err) + defer srv.Close() + + err = validator.Validate(context.Background(), "https://evil.example.com/webhook") + + require.Error(t, err) + require.ErrorContains(t, err, "public_only") +} + +func TestURLPolicyValidator_DNSRebinding_RejectsMetadataIP(t *testing.T) { + // Simulate DNS resolving to cloud metadata endpoint + validator, srv, err := newValidatorWithMockDNS( + config.WebhookSecurity{ + Mode: config.WebhookSecurityModeInsecure, + AllowedSchemes: []string{"https"}, + DenyMetadataEndpoints: true, + }, + map[string]mockdns.Zone{ + "attacker.example.com.": {A: []string{"169.254.169.254"}}, // AWS metadata IP + }, + ) + require.NoError(t, err) + defer srv.Close() + + err = validator.Validate(context.Background(), "https://attacker.example.com/webhook") + + require.Error(t, err) + require.ErrorContains(t, err, "metadata") +} + +func TestURLPolicyValidator_DNSRebinding_RejectsBlockedCIDR(t *testing.T) { + // Simulate DNS resolving to a blocked CIDR range + validator, srv, err := newValidatorWithMockDNS( + config.WebhookSecurity{ + Mode: config.WebhookSecurityModeCustom, + AllowedSchemes: []string{"https"}, + AllowedHosts: []string{"webhook.example.com"}, + AllowedCIDRs: []string{"0.0.0.0/0"}, + BlockedCIDRs: []string{"127.0.0.0/8"}, + }, + map[string]mockdns.Zone{ + "webhook.example.com.": {A: []string{"127.0.0.1"}}, // Blocked loopback + }, + ) + require.NoError(t, err) + defer srv.Close() + + err = validator.Validate(context.Background(), "https://webhook.example.com/webhook") + + require.Error(t, err) + require.ErrorContains(t, err, "blocked") +} + +func TestURLPolicyValidator_DNSRebinding_RejectsIPNotInAllowedCIDR(t *testing.T) { + // Simulate hostname resolving to IP outside allowed CIDR + validator, srv, err := newValidatorWithMockDNS( + config.WebhookSecurity{ + Mode: config.WebhookSecurityModeCustom, + AllowedSchemes: []string{"https"}, + AllowedHosts: []string{"webhook.example.com"}, + AllowedCIDRs: []string{"10.0.0.0/8"}, + }, + map[string]mockdns.Zone{ + "webhook.example.com.": {A: []string{"8.8.8.8"}}, // Not in 10.0.0.0/8 + }, + ) + require.NoError(t, err) + defer srv.Close() + + err = validator.Validate(context.Background(), "https://webhook.example.com/webhook") + + require.Error(t, err) + require.ErrorContains(t, err, "not in the allowed CIDR") +} + +func TestURLPolicyValidator_DNSRebinding_AllowsIPInAllowedCIDR(t *testing.T) { + // Simulate hostname correctly resolving to allowed CIDR + validator, srv, err := newValidatorWithMockDNS( + config.WebhookSecurity{ + Mode: config.WebhookSecurityModeCustom, + AllowedSchemes: []string{"https"}, + AllowedHosts: []string{"webhook.example.com"}, + AllowedCIDRs: []string{"10.0.0.0/8"}, + }, + map[string]mockdns.Zone{ + "webhook.example.com.": {A: []string{"10.0.0.5"}}, // In 10.0.0.0/8 + }, + ) + require.NoError(t, err) + defer srv.Close() + + result, err := validator.ValidateAndGetIPs(context.Background(), "https://webhook.example.com/webhook") + + require.NoError(t, err) + require.NotNil(t, result) + require.Len(t, result.ValidatedIPs, 1) + require.Equal(t, "10.0.0.5", result.ValidatedIPs[0].String()) +} + +func TestURLPolicyValidator_DNSRebinding_RejectsIfAnyIPInvalid(t *testing.T) { + // Simulate hostname resolving to multiple IPs where one is invalid + // This tests that ALL resolved IPs must pass validation + validator, srv, err := newValidatorWithMockDNS( + config.WebhookSecurity{ + Mode: config.WebhookSecurityModeCustom, + AllowedSchemes: []string{"https"}, + AllowedHosts: []string{"webhook.example.com"}, + AllowedCIDRs: []string{"10.0.0.0/8"}, + }, + map[string]mockdns.Zone{ + "webhook.example.com.": { + A: []string{ + "10.0.0.5", // Valid: in allowed CIDR + "8.8.8.8", // Invalid: not in allowed CIDR + }, + }, + }, + ) + require.NoError(t, err) + defer srv.Close() + + err = validator.Validate(context.Background(), "https://webhook.example.com/webhook") + + require.Error(t, err) + require.ErrorContains(t, err, "not in the allowed CIDR") + require.ErrorContains(t, err, "8.8.8.8") +} + +func TestURLPolicyValidator_DNSRebinding_AcceptsMultipleValidIPs(t *testing.T) { + // Simulate hostname resolving to multiple valid IPs + validator, srv, err := newValidatorWithMockDNS( + config.WebhookSecurity{ + Mode: config.WebhookSecurityModeCustom, + AllowedSchemes: []string{"https"}, + AllowedHosts: []string{"webhook.example.com"}, + AllowedCIDRs: []string{"10.0.0.0/8"}, + }, + map[string]mockdns.Zone{ + "webhook.example.com.": { + A: []string{"10.0.0.5", "10.0.0.6"}, + }, + }, + ) + require.NoError(t, err) + defer srv.Close() + + result, err := validator.ValidateAndGetIPs(context.Background(), "https://webhook.example.com/webhook") + + require.NoError(t, err) + require.NotNil(t, result) + require.Len(t, result.ValidatedIPs, 2) + require.Equal(t, "10.0.0.5", result.ValidatedIPs[0].String()) + require.Equal(t, "10.0.0.6", result.ValidatedIPs[1].String()) +} + +func TestURLPolicyValidator_DNSRebinding_PublicOnlyRejectsMixedIPs(t *testing.T) { + // Simulate hostname resolving to both public and private IPs + // This protects against attacks where DNS returns mixed public/internal IPs + validator, srv, err := newValidatorWithMockDNS( + config.WebhookSecurity{ + Mode: config.WebhookSecurityModePublicOnly, + AllowedSchemes: []string{"https"}, + }, + map[string]mockdns.Zone{ + "webhook.example.com.": { + A: []string{ + "8.8.8.8", // Public + "10.0.0.1", // Private - should cause rejection + }, + }, + }, + ) + require.NoError(t, err) + defer srv.Close() + + err = validator.Validate(context.Background(), "https://webhook.example.com/webhook") + + require.Error(t, err) + require.ErrorContains(t, err, "public_only") +} + +func TestURLPolicyValidator_DNSRebinding_SkipResolvedIPValidationStillRespectsBlocklist(t *testing.T) { + // Even with SkipResolvedIPValidation=true, blocked CIDRs should still be enforced + validator, srv, err := newValidatorWithMockDNS( + config.WebhookSecurity{ + Mode: config.WebhookSecurityModeCustom, + AllowedSchemes: []string{"https"}, + AllowedHosts: []string{"webhook.example.com"}, + SkipResolvedIPValidation: true, + DenyMetadataEndpoints: true, + }, + map[string]mockdns.Zone{ + "webhook.example.com.": {A: []string{"169.254.169.254"}}, // Metadata IP + }, + ) + require.NoError(t, err) + defer srv.Close() + + err = validator.Validate(context.Background(), "https://webhook.example.com/webhook") + + require.Error(t, err) + require.ErrorContains(t, err, "metadata") +} + +func TestURLPolicyValidator_DNSRebinding_InternalOnlyRejectsPublicIP(t *testing.T) { + // Simulate hostname resolving to public IP in internal_only mode + validator, srv, err := newValidatorWithMockDNS( + config.WebhookSecurity{ + Mode: config.WebhookSecurityModeInternalOnly, + AllowedSchemes: []string{"https"}, + }, + map[string]mockdns.Zone{ + "webhook.internal.": {A: []string{"8.8.8.8"}}, // Public IP + }, + ) + require.NoError(t, err) + defer srv.Close() + + err = validator.Validate(context.Background(), "https://webhook.internal/webhook") + + require.Error(t, err) + require.ErrorContains(t, err, "internal_only") +} + +func TestURLPolicyValidator_Validate_LiteralIPAddress(t *testing.T) { + // Test that when host is already a literal IP, it's validated directly without DNS resolution + validator := NewURLPolicyValidator(config.WebhookSecurity{ + Mode: config.WebhookSecurityModeCustom, + AllowedSchemes: []string{"https"}, + AllowedCIDRs: []string{"10.0.0.0/8"}, + }) + + result, err := validator.ValidateAndGetIPs(context.Background(), "https://10.0.0.5/webhook") + + require.NoError(t, err) + require.NotNil(t, result) + require.Len(t, result.ValidatedIPs, 1) + require.Equal(t, "10.0.0.5", result.ValidatedIPs[0].String()) + require.Equal(t, "10.0.0.5", result.Host) +} + +func TestURLPolicyValidator_Validate_LiteralIPAddress_Rejected(t *testing.T) { + // Test that literal IP is rejected when not in allowed CIDR + validator := NewURLPolicyValidator(config.WebhookSecurity{ + Mode: config.WebhookSecurityModeCustom, + AllowedSchemes: []string{"https"}, + AllowedCIDRs: []string{"10.0.0.0/8"}, + }) + + err := validator.Validate(context.Background(), "https://192.168.1.5/webhook") + + require.Error(t, err) + require.ErrorContains(t, err, "not in the allowed CIDR") +} + +func TestURLPolicyValidator_Validate_HostnameResolvesToNoIPs(t *testing.T) { + // Test that hostname that fails to resolve is rejected with appropriate error + validator, srv, err := newValidatorWithMockDNS( + config.WebhookSecurity{ + Mode: config.WebhookSecurityModeCustom, + AllowedSchemes: []string{"https"}, + AllowedHosts: []string{"noips.test"}, + }, + map[string]mockdns.Zone{ + "noips.test.": {A: []string{}}, // No IPs returned - results in "no such host" + }, + ) + require.NoError(t, err) + defer srv.Close() + + err = validator.Validate(context.Background(), "https://noips.test/webhook") + + require.Error(t, err) + require.ErrorContains(t, err, "failed to resolve webhook callback host") +} diff --git a/backend/webhooks/validation/callback.go b/backend/webhooks/validation/callback.go new file mode 100644 index 000000000..1da66102c --- /dev/null +++ b/backend/webhooks/validation/callback.go @@ -0,0 +1,109 @@ +package validation + +import ( + "fmt" + "net" + "net/url" + "strings" + + "github.com/teamhanko/hanko/backend/v2/webhooks/events" +) + +// WebhookSecurityConfig is an interface that provides access to webhook security settings. +// This allows us to accept config.WebhookSecurity without importing the config package. +type WebhookSecurityConfig interface { + ToWebhookSecurityPolicy() WebhookSecurityPolicy + GetAllowedSchemes() []string +} + +// ValidateWebhook validates a webhook's callback URL and associated events +// against the provided security policy. +// This is the main entry point for webhook validation that can be used by both +// config webhooks and database webhooks. +func ValidateWebhook(callback string, eventsStr []string, security WebhookSecurityConfig) error { + parsed, err := url.Parse(callback) + if err != nil { + return fmt.Errorf("callback is not a valid URL: %w", err) + } + + if err := validateParsedURL(parsed, security); err != nil { + return err + } + + if err := validateLiteralIP(parsed, security); err != nil { + return err + } + + if err := validateEvents(eventsStr); err != nil { + return err + } + + return nil +} + +// validateParsedURL validates the structure and security of a parsed URL. +func validateParsedURL(parsed *url.URL, security WebhookSecurityConfig) error { + if parsed.Scheme == "" { + return fmt.Errorf("callback URL must include a scheme") + } + + if parsed.Host == "" { + return fmt.Errorf("callback URL must include a host") + } + + if parsed.User != nil { + return fmt.Errorf("callback URL must not include user info") + } + + allowedSchemes := security.GetAllowedSchemes() + schemeAllowed := false + for _, scheme := range allowedSchemes { + if strings.EqualFold(strings.TrimSpace(scheme), parsed.Scheme) { + schemeAllowed = true + break + } + } + + if !schemeAllowed { + return fmt.Errorf("callback scheme '%s' is not allowed", parsed.Scheme) + } + + validator := NewValidator(security.ToWebhookSecurityPolicy()) + host := parsed.Hostname() + + if err := validator.ValidateHost(host); err != nil { + return fmt.Errorf("callback %w", err) + } + + return nil +} + +// validateLiteralIP validates that a callback URL using a literal IP address is allowed. +func validateLiteralIP(parsed *url.URL, security WebhookSecurityConfig) error { + host := NormalizeHost(parsed.Hostname()) + ip := net.ParseIP(host) + if ip == nil { + return nil + } + + validator := NewValidator(security.ToWebhookSecurityPolicy()) + if err := validator.ValidateIP(ip, false); err != nil { + return fmt.Errorf("callback %w", err) + } + + return nil +} + +// validateEvents validates that all provided event names are valid webhook events. +func validateEvents(eventsStr []string) error { + if len(eventsStr) > 0 { + for i, e := range eventsStr { + isValid := events.IsValidEvent(events.Event(e)) + if !isValid { + return fmt.Errorf("event '%s' at events[%d] is not a valid webhook event", e, i) + } + } + } + + return nil +} diff --git a/backend/webhooks/validation/constants.go b/backend/webhooks/validation/constants.go new file mode 100644 index 000000000..addfe420c --- /dev/null +++ b/backend/webhooks/validation/constants.go @@ -0,0 +1,62 @@ +package validation + +// ReservedIPCIDRs contains reserved and special-use IPv4 and IPv6 CIDR ranges that should be blocked +// for outbound webhook callbacks to prevent SSRF attacks. +// +// References: +// - RFC 5735 (IPv4 Special-Use Addresses) +// - RFC 6890 (Special-Purpose IP Address Registries) +// - RFC 4193 (IPv6 Unique Local Addresses) +// - RFC 4291 (IPv6 Address Architecture) +var ReservedIPCIDRs = []string{ + // IPv4 reserved ranges + "0.0.0.0/8", // "This" network (RFC 5735) + "100.64.0.0/10", // Shared address space (RFC 6598) + "192.0.0.0/24", // IETF protocol assignments (RFC 5736) + "192.0.2.0/24", // TEST-NET-1 (RFC 5737) + "198.18.0.0/15", // Benchmarking (RFC 2544) + "198.51.100.0/24", // TEST-NET-2 (RFC 5737) + "203.0.113.0/24", // TEST-NET-3 (RFC 5737) + "240.0.0.0/4", // Reserved for future use (RFC 1112) + + // IPv6 reserved ranges + "::/128", // Unspecified address + "100::/64", // Discard prefix (RFC 6666) + "2001:db8::/32", // Documentation prefix (RFC 3849) +} + +// MetadataEndpointCIDRs cntains cloud provider metadata service endpoints that should be blocked +// to prevent credential theft and information disclosure. +// +// References: +// - AWS: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instancedata-data-retrieval.html +// - GCP: https://cloud.google.com/compute/docs/metadata/overview +// - Azure: https://learn.microsoft.com/en-us/azure/virtual-machines/instance-metadata-service +// - Oracle Cloud: https://docs.oracle.com/en-us/iaas/Content/Compute/Tasks/gettingmetadata.htm +var MetadataEndpointCIDRs = []string{ + // Common IPv4 provider range, see references + "169.254.169.254/32", + + // IPv6 link-local addresses (used by various cloud providers for metadata) + // This is a broad range but necessary to block metadata services + "fe80::/10", + + // IPv6 Unique Local Addresses (private IPv6, similar to RFC 1918) + "fc00::/7", + + // Common IPv6 provider range, see references + "fd00:ec2::254/128", +} + +// AdditionalMetadataHosts contains hostname-based metadata endpoints +// that should be blocked in addition to IP-based blocking. +var AdditionalMetadataHosts = []string{ + "metadata.google.internal", // GCP metadata service + "metadata.goog", // GCP metadata service (alternative) + "169.254.169.254.nip.io", // Common bypass attempt + "169.254.169.254.xip.io", // Common bypass attempt + "169.254.169.254.sslip.io", // Common bypass attempt + "metadata", // Generic metadata hostname + "instance-data", // AWS instance data + "169-254-169-254.ec2.internal", // AWS internal hostname format +} diff --git a/backend/webhooks/validation/errors.go b/backend/webhooks/validation/errors.go new file mode 100644 index 000000000..f131e76a4 --- /dev/null +++ b/backend/webhooks/validation/errors.go @@ -0,0 +1,148 @@ +package validation + +import ( + "errors" + "fmt" + "strings" +) + +// Sanitized error messages that don't leak internal network information +const ( + ErrGenericValidationFailed = "callback URL validation failed" + ErrGenericHostNotAllowed = "callback URL not allowed" + ErrGenericIPNotAllowed = "callback destination not allowed" + ErrGenericRedirectNotAllowed = "redirect destination not allowed" +) + +// SanitizedError wraps an error with both detailed and sanitized messages +type SanitizedError struct { + DetailedError error + SanitizedMsg string +} + +func (e *SanitizedError) Error() string { + return e.SanitizedMsg +} + +func (e *SanitizedError) Unwrap() error { + return e.DetailedError +} + +// Detailed returns the original detailed error for logging +func (e *SanitizedError) Detailed() error { + return e.DetailedError +} + +// SanitizeError wraps an error with a sanitized message based on error type. +// Returns the original error if sanitization is disabled. +func SanitizeError(err error, sanitize bool) error { + if err == nil { + return nil + } + + if !sanitize { + return err + } + + // Already sanitized + if _, ok := err.(*SanitizedError); ok { + return err + } + + errMsg := err.Error() + sanitizedMsg := categorizeError(errMsg) + + return &SanitizedError{ + DetailedError: err, + SanitizedMsg: sanitizedMsg, + } +} + +// categorizeError maps detailed error messages to generic sanitized versions +func categorizeError(errMsg string) string { + errLower := strings.ToLower(errMsg) + + // DNS resolution errors + if strings.Contains(errLower, "failed to resolve") || + strings.Contains(errLower, "did not resolve") || + strings.Contains(errLower, "no such host") { + return ErrGenericValidationFailed + } + + // Host/domain blocking errors + if strings.Contains(errLower, "host") && (strings.Contains(errLower, "blocked") || strings.Contains(errLower, "not allowed")) { + return ErrGenericHostNotAllowed + } + + if strings.Contains(errLower, "domain") && (strings.Contains(errLower, "blocked") || strings.Contains(errLower, "not allowed")) { + return ErrGenericHostNotAllowed + } + + // IP validation errors + if strings.Contains(errLower, "ip") && (strings.Contains(errLower, "blocked") || strings.Contains(errLower, "not allowed")) { + return ErrGenericIPNotAllowed + } + + if strings.Contains(errLower, "resolved ip") { + return ErrGenericIPNotAllowed + } + + // Metadata endpoint errors + if strings.Contains(errLower, "metadata") { + return ErrGenericIPNotAllowed + } + + // Reserved/private IP errors + if strings.Contains(errLower, "non-public") || + strings.Contains(errLower, "reserved") || + strings.Contains(errLower, "private") { + return ErrGenericIPNotAllowed + } + + // CIDR matching errors + if strings.Contains(errLower, "cidr") { + return ErrGenericIPNotAllowed + } + + // Redirect errors + if strings.Contains(errLower, "redirect") { + return ErrGenericRedirectNotAllowed + } + + // Allowlist errors + if strings.Contains(errLower, "not in the allowed") || + strings.Contains(errLower, "not allowed unless explicitly allowlisted") { + return ErrGenericHostNotAllowed + } + + // Default to generic validation failure + return ErrGenericValidationFailed +} + +// GetDetailedError extracts the detailed error from a SanitizedError for logging. +// Returns the original error if it's not a SanitizedError. +func GetDetailedError(err error) error { + if err == nil { + return nil + } + + var sanitizedErr *SanitizedError + if errors.As(err, &sanitizedErr) { + return sanitizedErr.DetailedError + } + + return err +} + +// WrapWithContext adds context to an error and applies sanitization if enabled +func WrapWithContext(err error, context string, sanitize bool) error { + if err == nil { + return nil + } + + // Get the detailed error for wrapping + detailedErr := GetDetailedError(err) + wrappedErr := fmt.Errorf("%s: %w", context, detailedErr) + + return SanitizeError(wrappedErr, sanitize) +} diff --git a/backend/webhooks/validation/errors_test.go b/backend/webhooks/validation/errors_test.go new file mode 100644 index 000000000..8c89f1fb4 --- /dev/null +++ b/backend/webhooks/validation/errors_test.go @@ -0,0 +1,312 @@ +package validation + +import ( + "errors" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSanitizeError_Disabled(t *testing.T) { + originalErr := errors.New("detailed error message with sensitive info") + result := SanitizeError(originalErr, false) + + assert.Equal(t, originalErr, result, "should return original error when sanitization is disabled") +} + +func TestSanitizeError_Nil(t *testing.T) { + result := SanitizeError(nil, true) + assert.Nil(t, result, "should return nil when error is nil") +} + +func TestSanitizeError_DNSResolutionErrors(t *testing.T) { + tests := []struct { + name string + err error + expectedMsg string + shouldContain bool + }{ + { + name: "failed to resolve error", + err: fmt.Errorf("failed to resolve webhook callback host 'internal.corp': no such host"), + expectedMsg: ErrGenericValidationFailed, + }, + { + name: "did not resolve error", + err: fmt.Errorf("webhook callback host 'secret.local' did not resolve to any IP addresses"), + expectedMsg: ErrGenericValidationFailed, + }, + { + name: "no such host error", + err: fmt.Errorf("lookup failed: no such host"), + expectedMsg: ErrGenericValidationFailed, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := SanitizeError(tt.err, true) + assert.Equal(t, tt.expectedMsg, result.Error()) + + // Verify detailed error is preserved + sanitized, ok := result.(*SanitizedError) + require.True(t, ok) + assert.Equal(t, tt.err, sanitized.DetailedError) + }) + } +} + +func TestSanitizeError_HostBlockingErrors(t *testing.T) { + tests := []struct { + name string + err error + expectedMsg string + }{ + { + name: "blocked host", + err: fmt.Errorf("host 'evil.com' is blocked"), + expectedMsg: ErrGenericHostNotAllowed, + }, + { + name: "blocked domain", + err: fmt.Errorf("host 'api.evil.com' matches a blocked domain"), + expectedMsg: ErrGenericHostNotAllowed, + }, + { + name: "not allowed host", + err: fmt.Errorf("host 'internal.local' is not allowed"), + expectedMsg: ErrGenericHostNotAllowed, + }, + { + name: "domain not in allowlist", + err: fmt.Errorf("host 'test.internal.local' is not in the allowed host/domain list"), + expectedMsg: ErrGenericHostNotAllowed, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := SanitizeError(tt.err, true) + assert.Equal(t, tt.expectedMsg, result.Error()) + }) + } +} + +func TestSanitizeError_IPValidationErrors(t *testing.T) { + tests := []struct { + name string + err error + expectedMsg string + }{ + { + name: "blocked IP", + err: fmt.Errorf("IP '192.168.1.1' is blocked"), + expectedMsg: ErrGenericIPNotAllowed, + }, + { + name: "non-public IP", + err: fmt.Errorf("non-public IP '10.0.0.1' is not allowed in public_only mode"), + expectedMsg: ErrGenericIPNotAllowed, + }, + { + name: "reserved IP", + err: fmt.Errorf("reserved IP '0.0.0.0' is blocked"), + expectedMsg: ErrGenericIPNotAllowed, + }, + { + name: "private IP", + err: fmt.Errorf("private IP '172.16.0.1' is not allowed"), + expectedMsg: ErrGenericIPNotAllowed, + }, + { + name: "resolved IP not allowed", + err: fmt.Errorf("resolved IP '10.0.0.5' for host 'internal.local' is not allowed: non-public IP"), + expectedMsg: ErrGenericHostNotAllowed, // "host" keyword takes precedence + }, + { + name: "CIDR match", + err: fmt.Errorf("IP '192.168.1.100' matches a blocked CIDR"), + expectedMsg: ErrGenericIPNotAllowed, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := SanitizeError(tt.err, true) + assert.Equal(t, tt.expectedMsg, result.Error()) + }) + } +} + +func TestSanitizeError_MetadataEndpointErrors(t *testing.T) { + tests := []struct { + name string + err error + expectedMsg string + }{ + { + name: "metadata IP blocked", + err: fmt.Errorf("metadata endpoint IP '169.254.169.254' is blocked"), + expectedMsg: ErrGenericIPNotAllowed, + }, + { + name: "metadata host blocked", + err: fmt.Errorf("metadata endpoint host 'metadata.google.internal' is blocked"), + expectedMsg: ErrGenericHostNotAllowed, // "host" keyword takes precedence over "metadata" + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := SanitizeError(tt.err, true) + assert.Equal(t, tt.expectedMsg, result.Error()) + }) + } +} + +func TestSanitizeError_RedirectErrors(t *testing.T) { + tests := []struct { + name string + err error + expectedMsg string + }{ + { + name: "redirect target rejected", + err: fmt.Errorf("redirect target rejected by outbound policy: host not allowed"), + expectedMsg: ErrGenericHostNotAllowed, // "host" keyword takes precedence over "redirect" + }, + { + name: "too many redirects", + err: fmt.Errorf("too many redirects"), + expectedMsg: ErrGenericRedirectNotAllowed, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := SanitizeError(tt.err, true) + assert.Equal(t, tt.expectedMsg, result.Error()) + }) + } +} + +func TestSanitizeError_GenericErrors(t *testing.T) { + tests := []struct { + name string + err error + expectedMsg string + }{ + { + name: "invalid URL", + err: fmt.Errorf("invalid webhook callback URL: parse error"), + expectedMsg: ErrGenericValidationFailed, + }, + { + name: "unknown error", + err: fmt.Errorf("some unknown error"), + expectedMsg: ErrGenericValidationFailed, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := SanitizeError(tt.err, true) + assert.Equal(t, tt.expectedMsg, result.Error()) + }) + } +} + +func TestSanitizeError_AlreadySanitized(t *testing.T) { + originalErr := errors.New("detailed error") + sanitized := &SanitizedError{ + DetailedError: originalErr, + SanitizedMsg: ErrGenericIPNotAllowed, + } + + result := SanitizeError(sanitized, true) + assert.Equal(t, sanitized, result, "should not double-sanitize") +} + +func TestGetDetailedError(t *testing.T) { + t.Run("sanitized error", func(t *testing.T) { + originalErr := errors.New("detailed error with sensitive info") + sanitized := SanitizeError(originalErr, true) + + detailed := GetDetailedError(sanitized) + assert.Equal(t, originalErr, detailed) + }) + + t.Run("regular error", func(t *testing.T) { + regularErr := errors.New("regular error") + detailed := GetDetailedError(regularErr) + assert.Equal(t, regularErr, detailed) + }) + + t.Run("nil error", func(t *testing.T) { + detailed := GetDetailedError(nil) + assert.Nil(t, detailed) + }) +} + +func TestSanitizedError_Unwrap(t *testing.T) { + originalErr := errors.New("original error") + sanitized := &SanitizedError{ + DetailedError: originalErr, + SanitizedMsg: ErrGenericIPNotAllowed, + } + + unwrapped := errors.Unwrap(sanitized) + assert.Equal(t, originalErr, unwrapped) +} + +func TestSanitizedError_Detailed(t *testing.T) { + originalErr := errors.New("original detailed error") + sanitized := &SanitizedError{ + DetailedError: originalErr, + SanitizedMsg: ErrGenericHostNotAllowed, + } + + detailed := sanitized.Detailed() + assert.Equal(t, originalErr, detailed) +} + +func TestWrapWithContext(t *testing.T) { + t.Run("with sanitization enabled", func(t *testing.T) { + originalErr := fmt.Errorf("IP '10.0.0.1' is blocked") + wrapped := WrapWithContext(originalErr, "validation failed", true) + + assert.NotNil(t, wrapped) + assert.Equal(t, ErrGenericIPNotAllowed, wrapped.Error()) + + detailed := GetDetailedError(wrapped) + assert.Contains(t, detailed.Error(), "validation failed") + assert.Contains(t, detailed.Error(), "IP '10.0.0.1' is blocked") + }) + + t.Run("with sanitization disabled", func(t *testing.T) { + originalErr := fmt.Errorf("IP '10.0.0.1' is blocked") + wrapped := WrapWithContext(originalErr, "validation failed", false) + + assert.Contains(t, wrapped.Error(), "validation failed") + assert.Contains(t, wrapped.Error(), "IP '10.0.0.1' is blocked") + }) + + t.Run("nil error", func(t *testing.T) { + wrapped := WrapWithContext(nil, "context", true) + assert.Nil(t, wrapped) + }) +} + +func TestSanitizeError_PreservesErrorChain(t *testing.T) { + baseErr := errors.New("base error") + wrappedErr := fmt.Errorf("wrapped: %w", baseErr) + sanitizedErr := SanitizeError(wrappedErr, true) + + sanitized, ok := sanitizedErr.(*SanitizedError) + require.True(t, ok) + + // Detailed error should preserve the chain + assert.True(t, errors.Is(sanitized.DetailedError, baseErr)) +} diff --git a/backend/webhooks/validation/host.go b/backend/webhooks/validation/host.go new file mode 100644 index 000000000..25bdea650 --- /dev/null +++ b/backend/webhooks/validation/host.go @@ -0,0 +1,43 @@ +package validation + +import ( + "strings" +) + +// NormalizeHost normalizes a hostname by converting to lowercase, +// trimming whitespace, and removing trailing dots. +func NormalizeHost(value string) string { + return strings.TrimSuffix(strings.ToLower(strings.TrimSpace(value)), ".") +} + +// MatchesHost checks if the given host exactly matches any host in the provided list. +// Comparison is case-insensitive and normalizes both the input and list values. +func MatchesHost(host string, values []string) bool { + host = NormalizeHost(host) + for _, value := range values { + if host == NormalizeHost(value) { + return true + } + } + return false +} + +// MatchesDomain checks if the given host matches any domain in the provided list. +// It matches both exact domain matches and subdomain matches. +// For example, "api.example.com" matches domain "example.com", but "badexample.com" does not. +func MatchesDomain(host string, domains []string) bool { + host = NormalizeHost(host) + for _, domain := range domains { + normalized := NormalizeHost(domain) + if host == normalized || strings.HasSuffix(host, "."+normalized) { + return true + } + } + return false +} + +// IsMetadataHost checks if the given hostname is a known metadata service endpoint. +// This provides an additional layer of protection beyond IP-based blocking. +func IsMetadataHost(host string) bool { + return MatchesHost(host, AdditionalMetadataHosts) +} diff --git a/backend/webhooks/validation/host_test.go b/backend/webhooks/validation/host_test.go new file mode 100644 index 000000000..a93ee9c35 --- /dev/null +++ b/backend/webhooks/validation/host_test.go @@ -0,0 +1,112 @@ +package validation + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestNormalizeHost_NormalizesCorrectly(t *testing.T) { + testCases := []struct { + input string + expected string + }{ + {" Example.COM. ", "example.com"}, + {"EXAMPLE.COM", "example.com"}, + {"example.com.", "example.com"}, + {" example.com ", "example.com"}, + {"Example.Com", "example.com"}, + } + + for _, tc := range testCases { + result := NormalizeHost(tc.input) + assert.Equal(t, tc.expected, result, "Input: %s", tc.input) + } +} + +func TestMatchesHost_MatchesExactHost(t *testing.T) { + result := MatchesHost("example.com", []string{"example.com"}) + + assert.True(t, result) +} + +func TestMatchesHost_MatchesCaseInsensitive(t *testing.T) { + result := MatchesHost("Example.COM", []string{"example.com"}) + + assert.True(t, result) +} + +func TestMatchesHost_DoesNotMatchSubdomain(t *testing.T) { + result := MatchesHost("api.example.com", []string{"example.com"}) + + assert.False(t, result) +} + +func TestMatchesHost_DoesNotMatchDifferentHost(t *testing.T) { + result := MatchesHost("other.com", []string{"example.com"}) + + assert.False(t, result) +} + +func TestMatchesDomain_MatchesExactDomain(t *testing.T) { + result := MatchesDomain("example.com", []string{"example.com"}) + + assert.True(t, result) +} + +func TestMatchesDomain_MatchesSubdomain(t *testing.T) { + result := MatchesDomain("api.example.com", []string{"example.com"}) + + assert.True(t, result) +} + +func TestMatchesDomain_MatchesDeepSubdomain(t *testing.T) { + result := MatchesDomain("a.b.c.example.com", []string{"example.com"}) + + assert.True(t, result) +} + +func TestMatchesDomain_DoesNotMatchDifferentDomain(t *testing.T) { + result := MatchesDomain("badexample.com", []string{"example.com"}) + + assert.False(t, result) +} + +func TestMatchesDomain_DoesNotMatchPartialMatch(t *testing.T) { + result := MatchesDomain("notexample.com", []string{"example.com"}) + + assert.False(t, result) +} + +func TestMatchesDomain_MatchesCaseInsensitive(t *testing.T) { + result := MatchesDomain("API.Example.COM", []string{"example.com"}) + + assert.True(t, result) +} + +func TestIsMetadataHost_ReturnsTrueForKnownMetadataHosts(t *testing.T) { + testCases := []string{ + "metadata.google.internal", + "metadata.goog", + "169.254.169.254.nip.io", + "metadata", + "instance-data", + } + + for _, host := range testCases { + result := IsMetadataHost(host) + assert.True(t, result, "Expected %s to be a metadata host", host) + } +} + +func TestIsMetadataHost_ReturnsFalseForNonMetadataHost(t *testing.T) { + result := IsMetadataHost("example.com") + + assert.False(t, result) +} + +func TestIsMetadataHost_MatchesCaseInsensitive(t *testing.T) { + result := IsMetadataHost("METADATA.GOOGLE.INTERNAL") + + assert.True(t, result) +} diff --git a/backend/webhooks/validation/ip.go b/backend/webhooks/validation/ip.go new file mode 100644 index 000000000..8e88ed0eb --- /dev/null +++ b/backend/webhooks/validation/ip.go @@ -0,0 +1,52 @@ +package validation + +import ( + "net" + "strings" +) + +// IPMatchesCIDRs checks if the given IP address matches any of the provided CIDR ranges. +// Invalid CIDR ranges in the list are silently skipped. +func IPMatchesCIDRs(ip net.IP, cidrs []string) bool { + for _, value := range cidrs { + _, network, err := net.ParseCIDR(strings.TrimSpace(value)) + if err != nil { + continue + } + if network.Contains(ip) { + return true + } + } + + return false +} + +// IsPublicRoutableIP returns true if the IP address is a publicly routable IP. +// It returns false for loopback, private, multicast, unspecified, link-local, +// reserved, and metadata service IPs. +func IsPublicRoutableIP(ip net.IP) bool { + if ip.IsLoopback() || + ip.IsPrivate() || + ip.IsMulticast() || + ip.IsUnspecified() || + ip.IsLinkLocalUnicast() || + ip.IsLinkLocalMulticast() || + IsReservedIP(ip) || + IsMetadataIP(ip) { + return false + } + + return true +} + +// IsReservedIP checks if the IP address falls within reserved or special-use IP ranges. +// See ReservedIPCIDRs in constants.go for the full list of reserved ranges. +func IsReservedIP(ip net.IP) bool { + return IPMatchesCIDRs(ip, ReservedIPCIDRs) +} + +// IsMetadataIP checks if the IP address is a cloud provider metadata service endpoint. +// See MetadataEndpointCIDRs in constants.go for the full list of metadata endpoints. +func IsMetadataIP(ip net.IP) bool { + return IPMatchesCIDRs(ip, MetadataEndpointCIDRs) +} diff --git a/backend/webhooks/validation/ip_test.go b/backend/webhooks/validation/ip_test.go new file mode 100644 index 000000000..296d8a305 --- /dev/null +++ b/backend/webhooks/validation/ip_test.go @@ -0,0 +1,152 @@ +package validation + +import ( + "net" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestIPMatchesCIDRs_MatchesValidCIDR(t *testing.T) { + ip := net.ParseIP("10.0.0.5") + cidrs := []string{"10.0.0.0/24"} + + result := IPMatchesCIDRs(ip, cidrs) + + assert.True(t, result) +} + +func TestIPMatchesCIDRs_DoesNotMatchDifferentCIDR(t *testing.T) { + ip := net.ParseIP("10.0.1.5") + cidrs := []string{"10.0.0.0/24"} + + result := IPMatchesCIDRs(ip, cidrs) + + assert.False(t, result) +} + +func TestIPMatchesCIDRs_IgnoresInvalidCIDR(t *testing.T) { + ip := net.ParseIP("10.0.0.5") + cidrs := []string{"not-a-cidr", "10.0.0.0/24"} + + result := IPMatchesCIDRs(ip, cidrs) + + assert.True(t, result) +} + +func TestIPMatchesCIDRs_HandlesIPv6(t *testing.T) { + ip := net.ParseIP("2001:db8::1") + cidrs := []string{"2001:db8::/32"} + + result := IPMatchesCIDRs(ip, cidrs) + + assert.True(t, result) +} + +func TestIsPublicRoutableIP_ReturnsTrueForPublicIP(t *testing.T) { + ip := net.ParseIP("8.8.8.8") + + result := IsPublicRoutableIP(ip) + + assert.True(t, result) +} + +func TestIsPublicRoutableIP_ReturnsFalseForLoopback(t *testing.T) { + ip := net.ParseIP("127.0.0.1") + + result := IsPublicRoutableIP(ip) + + assert.False(t, result) +} + +func TestIsPublicRoutableIP_ReturnsFalseForPrivate(t *testing.T) { + ip := net.ParseIP("10.0.0.1") + + result := IsPublicRoutableIP(ip) + + assert.False(t, result) +} + +func TestIsPublicRoutableIP_ReturnsFalseForMulticast(t *testing.T) { + ip := net.ParseIP("224.0.0.1") + + result := IsPublicRoutableIP(ip) + + assert.False(t, result) +} + +func TestIsPublicRoutableIP_ReturnsFalseForReservedIP(t *testing.T) { + ip := net.ParseIP("192.0.2.1") // TEST-NET-1 + + result := IsPublicRoutableIP(ip) + + assert.False(t, result) +} + +func TestIsPublicRoutableIP_ReturnsFalseForMetadataIP(t *testing.T) { + ip := net.ParseIP("169.254.169.254") + + result := IsPublicRoutableIP(ip) + + assert.False(t, result) +} + +func TestIsReservedIP_ReturnsTrueForReservedRanges(t *testing.T) { + testCases := []string{ + "0.0.0.1", // "This" network + "100.64.0.1", // Shared address space + "192.0.0.1", // IETF protocol assignments + "192.0.2.1", // TEST-NET-1 + "198.18.0.1", // Benchmarking + "198.51.100.1", // TEST-NET-2 + "203.0.113.1", // TEST-NET-3 + "240.0.0.1", // Reserved for future use + "::", // IPv6 unspecified + "2001:db8::1", // IPv6 documentation + } + + for _, ipStr := range testCases { + ip := net.ParseIP(ipStr) + assert.True(t, IsReservedIP(ip), "Expected %s to be reserved", ipStr) + } +} + +func TestIsReservedIP_ReturnsFalseForNonReservedIP(t *testing.T) { + ip := net.ParseIP("8.8.8.8") + + result := IsReservedIP(ip) + + assert.False(t, result) +} + +func TestIsMetadataIP_ReturnsTrueForAWSMetadata(t *testing.T) { + ip := net.ParseIP("169.254.169.254") + + result := IsMetadataIP(ip) + + assert.True(t, result) +} + +func TestIsMetadataIP_ReturnsTrueForIPv6LinkLocal(t *testing.T) { + ip := net.ParseIP("fe80::1") + + result := IsMetadataIP(ip) + + assert.True(t, result) +} + +func TestIsMetadataIP_ReturnsTrueForIPv6ULA(t *testing.T) { + ip := net.ParseIP("fc00::1") + + result := IsMetadataIP(ip) + + assert.True(t, result) +} + +func TestIsMetadataIP_ReturnsFalseForNonMetadataIP(t *testing.T) { + ip := net.ParseIP("8.8.8.8") + + result := IsMetadataIP(ip) + + assert.False(t, result) +} diff --git a/backend/webhooks/validation/validator.go b/backend/webhooks/validation/validator.go new file mode 100644 index 000000000..a1f5caa75 --- /dev/null +++ b/backend/webhooks/validation/validator.go @@ -0,0 +1,216 @@ +package validation + +import ( + "fmt" + "net" +) + +// SecurityMode defines the outbound destination policy for webhook callbacks. +type SecurityMode string + +const ( + SecurityModePublicOnly SecurityMode = "public_only" + SecurityModeInternalOnly SecurityMode = "internal_only" + SecurityModeCustom SecurityMode = "custom" + SecurityModeInsecure SecurityMode = "insecure" +) + +// WebhookSecurityPolicy contains the security settings for webhook validation. +// This mirrors config.WebhookSecurity but is defined here to avoid circular dependencies. +type WebhookSecurityPolicy struct { + Mode SecurityMode + AllowedSchemes []string + FollowRedirects bool + MaxRedirects int + SkipResolvedIPValidation bool + AllowedHosts []string + AllowedDomains []string + AllowedCIDRs []string + BlockedHosts []string + BlockedDomains []string + BlockedCIDRs []string + DenyMetadataEndpoints bool + SanitizeErrors bool +} + +// Validator provides shared validation logic for webhook callback URLs. +// It validates based on security policies defined in WebhookSecurityPolicy configuration. +type Validator struct { + security WebhookSecurityPolicy +} + +// NewValidator creates a new Validator instance with the given security configuration. +func NewValidator(security WebhookSecurityPolicy) *Validator { + return &Validator{ + security: security, + } +} + +// ValidateHost validates a hostname against the security policy. +// This includes checking blocked lists and allowed lists (for custom mode). +func (v *Validator) ValidateHost(host string) error { + host = NormalizeHost(host) + if host == "" { + err := fmt.Errorf("host must not be empty") + return SanitizeError(err, v.security.SanitizeErrors) + } + + // Check if host is a known metadata endpoint + if v.security.DenyMetadataEndpoints && IsMetadataHost(host) { + err := fmt.Errorf("metadata endpoint host '%s' is blocked", host) + return SanitizeError(err, v.security.SanitizeErrors) + } + + // Check blocked lists (applies to all modes) + if MatchesHost(host, v.security.BlockedHosts) { + err := fmt.Errorf("host '%s' is blocked", host) + return SanitizeError(err, v.security.SanitizeErrors) + } + + if MatchesDomain(host, v.security.BlockedDomains) { + err := fmt.Errorf("host '%s' matches a blocked domain", host) + return SanitizeError(err, v.security.SanitizeErrors) + } + + // Check allowed lists (only in custom mode with explicit allowlists) + if err := v.validateAllowedHostPolicy(host); err != nil { + return SanitizeError(err, v.security.SanitizeErrors) + } + + return nil +} + +// ValidateIP validates an IP address against the security policy. +// This includes checking absolute denies (metadata IPs, blocked CIDRs) +// and mode-specific rules (public-only, custom allowlists). +// +// The wasHostnameValidated parameter indicates whether this IP was resolved from +// a hostname that already passed hostname validation. When true and +// SkipResolvedIPValidation is enabled, mode-specific IP validation is skipped +// (absolute denies like blocked CIDRs still apply). +func (v *Validator) ValidateIP(ip net.IP, wasHostnameValidated bool) error { + // Absolute denies apply to all modes + if err := v.validateAbsoluteDenies(ip); err != nil { + return SanitizeError(err, v.security.SanitizeErrors) + } + + // Skip mode-specific validation if this IP was resolved from a validated hostname + // and SkipResolvedIPValidation is enabled + if wasHostnameValidated && v.security.SkipResolvedIPValidation { + return nil + } + + // Mode-specific validation + err := v.validateModeDecision(ip) + return SanitizeError(err, v.security.SanitizeErrors) +} + +// validateAllowedHostPolicy checks if the host is allowed based on the security mode +// and configured allowlists. +func (v *Validator) validateAllowedHostPolicy(host string) error { + switch v.security.Mode { + case SecurityModeInsecure: + return nil + case SecurityModePublicOnly: + return nil + case SecurityModeInternalOnly: + return nil + case SecurityModeCustom: + // In custom mode, if no host/domain allowlists configured, skip host validation + // (IP validation will handle CIDR allowlists if configured) + if len(v.security.AllowedHosts) == 0 && len(v.security.AllowedDomains) == 0 { + return nil + } + + // Check if host is a literal IP and matches allowed_cidrs + // This allows literal IPs in CIDR ranges to pass even when allowed_hosts is configured + if ip := net.ParseIP(host); ip != nil && len(v.security.AllowedCIDRs) > 0 { + if IPMatchesCIDRs(ip, v.security.AllowedCIDRs) { + return nil + } + } + + // If allowlists exist, host must match + if MatchesHost(host, v.security.AllowedHosts) || MatchesDomain(host, v.security.AllowedDomains) { + return nil + } + + return fmt.Errorf("host '%s' is not in the allowed host/domain list", host) + default: + return fmt.Errorf("unsupported webhook security mode '%s'", v.security.Mode) + } +} + +// validateAbsoluteDenies checks for conditions that block the IP regardless of mode. +func (v *Validator) validateAbsoluteDenies(ip net.IP) error { + if IPMatchesCIDRs(ip, v.security.BlockedCIDRs) { + return fmt.Errorf("IP '%s' matches a blocked CIDR", ip.String()) + } + + if v.security.DenyMetadataEndpoints && IsMetadataIP(ip) { + return fmt.Errorf("metadata endpoint IP '%s' is blocked", ip.String()) + } + + return nil +} + +// validateModeDecision validates the IP based on the configured security mode. +func (v *Validator) validateModeDecision(ip net.IP) error { + switch v.security.Mode { + case SecurityModeInsecure: + return nil + case SecurityModePublicOnly: + return v.validatePublicOnly(ip) + case SecurityModeInternalOnly: + return v.validateInternalOnly(ip) + case SecurityModeCustom: + return v.validateCustom(ip) + default: + return fmt.Errorf("unsupported webhook security mode '%s'", v.security.Mode) + } +} + +// validatePublicOnly ensures the IP is publicly routable (public_only mode). +func (v *Validator) validatePublicOnly(ip net.IP) error { + if !IsPublicRoutableIP(ip) { + return fmt.Errorf("non-public IP '%s' is not allowed in public_only mode", ip.String()) + } + + return nil +} + +// validateInternalOnly ensures the IP is non-public/internal (internal_only mode). +func (v *Validator) validateInternalOnly(ip net.IP) error { + if IsPublicRoutableIP(ip) { + return fmt.Errorf("public IP '%s' is not allowed in internal_only mode", ip.String()) + } + + return nil +} + +// validateCustom validates the IP in custom mode. +// Custom mode requires explicit allowlist configuration (enforced at config validation). +// IPs can be allowed via allowed_cidrs (CIDR notation) or allowed_hosts (literal IPs). +func (v *Validator) validateCustom(ip net.IP) error { + // Check if IP matches allowed CIDRs + if len(v.security.AllowedCIDRs) > 0 { + if IPMatchesCIDRs(ip, v.security.AllowedCIDRs) { + return nil + } + } + + // Check if IP matches allowed hosts (for literal IPs in allowed_hosts) + if len(v.security.AllowedHosts) > 0 { + ipStr := ip.String() + if MatchesHost(ipStr, v.security.AllowedHosts) { + return nil + } + } + + // If no allow lists configured, this should never happen (caught by config validation) + if len(v.security.AllowedCIDRs) == 0 && len(v.security.AllowedHosts) == 0 && len(v.security.AllowedDomains) == 0 { + return fmt.Errorf("custom mode requires allowlist configuration") + } + + return fmt.Errorf("IP '%s' is not in the allowed CIDR or host list", ip.String()) +} diff --git a/backend/webhooks/validation/validator_test.go b/backend/webhooks/validation/validator_test.go new file mode 100644 index 000000000..1876452da --- /dev/null +++ b/backend/webhooks/validation/validator_test.go @@ -0,0 +1,294 @@ +package validation + +import ( + "net" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestValidator_ValidateHost_RejectsEmptyHost(t *testing.T) { + validator := NewValidator(WebhookSecurityPolicy{ + Mode: SecurityModePublicOnly, + }) + + err := validator.ValidateHost("") + + assert.Error(t, err) + assert.Contains(t, err.Error(), "must not be empty") +} + +func TestValidator_ValidateHost_RejectsBlockedHost(t *testing.T) { + validator := NewValidator(WebhookSecurityPolicy{ + Mode: SecurityModePublicOnly, + BlockedHosts: []string{"blocked.example.com"}, + }) + + err := validator.ValidateHost("blocked.example.com") + + assert.Error(t, err) + assert.Contains(t, err.Error(), "is blocked") +} + +func TestValidator_ValidateHost_RejectsBlockedDomain(t *testing.T) { + validator := NewValidator(WebhookSecurityPolicy{ + Mode: SecurityModePublicOnly, + BlockedDomains: []string{"example.com"}, + }) + + err := validator.ValidateHost("api.example.com") + + assert.Error(t, err) + assert.Contains(t, err.Error(), "blocked domain") +} + +func TestValidator_ValidateHost_RejectsMetadataHost(t *testing.T) { + validator := NewValidator(WebhookSecurityPolicy{ + Mode: SecurityModePublicOnly, + DenyMetadataEndpoints: true, + }) + + err := validator.ValidateHost("metadata.google.internal") + + assert.Error(t, err) + assert.Contains(t, err.Error(), "metadata") +} + +func TestValidator_ValidateHost_CustomModeAllowsHostInAllowlist(t *testing.T) { + validator := NewValidator(WebhookSecurityPolicy{ + Mode: SecurityModeCustom, + AllowedHosts: []string{"allowed.example.com"}, + }) + + err := validator.ValidateHost("allowed.example.com") + + assert.NoError(t, err) +} + +func TestValidator_ValidateHost_CustomModeAllowsHostInAllowedDomain(t *testing.T) { + validator := NewValidator(WebhookSecurityPolicy{ + Mode: SecurityModeCustom, + AllowedDomains: []string{"example.com"}, + }) + + err := validator.ValidateHost("api.example.com") + + assert.NoError(t, err) +} + +func TestValidator_ValidateHost_CustomModeRejectsHostNotInAllowlist(t *testing.T) { + validator := NewValidator(WebhookSecurityPolicy{ + Mode: SecurityModeCustom, + AllowedDomains: []string{"example.com"}, + }) + + err := validator.ValidateHost("other.com") + + assert.Error(t, err) + assert.Contains(t, err.Error(), "not in the allowed") +} + +func TestValidator_ValidateHost_CustomAllowsLiteralIPInCIDRWithHostsConfigured(t *testing.T) { + validator := NewValidator(WebhookSecurityPolicy{ + Mode: SecurityModeCustom, + AllowedHosts: []string{"example.com"}, + AllowedCIDRs: []string{"192.168.1.0/24"}, + }) + + err := validator.ValidateHost("192.168.1.50") + + assert.NoError(t, err) +} + +func TestValidator_ValidateHost_CustomRejectsLiteralIPNotInCIDR(t *testing.T) { + validator := NewValidator(WebhookSecurityPolicy{ + Mode: SecurityModeCustom, + AllowedHosts: []string{"example.com"}, + AllowedCIDRs: []string{"192.168.1.0/24"}, + }) + + err := validator.ValidateHost("10.0.0.1") + + assert.Error(t, err) + assert.Contains(t, err.Error(), "not in the allowed host/domain list") +} + +func TestValidator_ValidateHost_CustomAllowsLiteralIPInCIDRWithoutHosts(t *testing.T) { + validator := NewValidator(WebhookSecurityPolicy{ + Mode: SecurityModeCustom, + AllowedCIDRs: []string{"192.168.1.0/24"}, + }) + + // When no allowed_hosts/allowed_domains, host validation is skipped + // (IP validation handles CIDR checks) + err := validator.ValidateHost("192.168.1.50") + + assert.NoError(t, err) +} + +func TestValidator_ValidateIP_RejectsBlockedCIDR(t *testing.T) { + validator := NewValidator(WebhookSecurityPolicy{ + Mode: SecurityModePublicOnly, + BlockedCIDRs: []string{"10.0.0.0/24"}, + }) + + err := validator.ValidateIP(net.ParseIP("10.0.0.5"), false) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "blocked CIDR") +} + +func TestValidator_ValidateIP_RejectsMetadataIP(t *testing.T) { + validator := NewValidator(WebhookSecurityPolicy{ + Mode: SecurityModePublicOnly, + DenyMetadataEndpoints: true, + }) + + err := validator.ValidateIP(net.ParseIP("169.254.169.254"), false) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "metadata") +} + +func TestValidator_ValidateIP_PublicOnlyRejectsPrivateIP(t *testing.T) { + validator := NewValidator(WebhookSecurityPolicy{ + Mode: SecurityModePublicOnly, + }) + + err := validator.ValidateIP(net.ParseIP("10.0.0.1"), false) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "public_only") +} + +func TestValidator_ValidateIP_PublicOnlyAllowsPublicIP(t *testing.T) { + validator := NewValidator(WebhookSecurityPolicy{ + Mode: SecurityModePublicOnly, + }) + + err := validator.ValidateIP(net.ParseIP("8.8.8.8"), false) + + assert.NoError(t, err) +} + +func TestValidator_ValidateIP_InternalOnlyAllowsPrivateIP(t *testing.T) { + validator := NewValidator(WebhookSecurityPolicy{ + Mode: SecurityModeInternalOnly, + }) + + err := validator.ValidateIP(net.ParseIP("10.0.0.1"), false) + + assert.NoError(t, err) +} + +func TestValidator_ValidateIP_InternalOnlyRejectsPublicIP(t *testing.T) { + validator := NewValidator(WebhookSecurityPolicy{ + Mode: SecurityModeInternalOnly, + }) + + err := validator.ValidateIP(net.ParseIP("8.8.8.8"), false) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "internal_only") +} + +func TestValidator_ValidateIP_CustomAllowsPrivateIPInAllowedCIDR(t *testing.T) { + validator := NewValidator(WebhookSecurityPolicy{ + Mode: SecurityModeCustom, + AllowedCIDRs: []string{"10.0.0.0/24"}, + }) + + err := validator.ValidateIP(net.ParseIP("10.0.0.5"), false) + + assert.NoError(t, err) +} + +func TestValidator_ValidateIP_CustomRejectsPrivateIPNotInAllowedCIDR(t *testing.T) { + validator := NewValidator(WebhookSecurityPolicy{ + Mode: SecurityModeCustom, + AllowedCIDRs: []string{"10.0.0.0/24"}, + }) + + err := validator.ValidateIP(net.ParseIP("10.0.1.5"), false) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "not in the allowed CIDR") +} + +func TestValidator_ValidateIP_InsecureAllowsAnyIP(t *testing.T) { + validator := NewValidator(WebhookSecurityPolicy{ + Mode: SecurityModeInsecure, + }) + + err := validator.ValidateIP(net.ParseIP("127.0.0.1"), false) + + assert.NoError(t, err) +} + +func TestValidator_ValidateIP_InsecureStillRespectsBlockedCIDRs(t *testing.T) { + validator := NewValidator(WebhookSecurityPolicy{ + Mode: SecurityModeInsecure, + BlockedCIDRs: []string{"127.0.0.0/8"}, + }) + + err := validator.ValidateIP(net.ParseIP("127.0.0.1"), false) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "blocked") +} + +func TestValidator_ValidateIP_CustomSkipsValidationForResolvedHostname(t *testing.T) { + validator := NewValidator(WebhookSecurityPolicy{ + Mode: SecurityModeCustom, + AllowedHosts: []string{"webhook.example.com"}, + SkipResolvedIPValidation: true, + }) + + // IP resolved from validated hostname should pass even without being in allowed_cidrs + err := validator.ValidateIP(net.ParseIP("93.184.216.34"), true) + + assert.NoError(t, err) +} + +func TestValidator_ValidateIP_CustomRequiresCIDRWhenNotSkipping(t *testing.T) { + validator := NewValidator(WebhookSecurityPolicy{ + Mode: SecurityModeCustom, + AllowedHosts: []string{"webhook.example.com"}, + SkipResolvedIPValidation: false, // Default behavior + }) + + // IP resolved from validated hostname still requires CIDR allowlist + err := validator.ValidateIP(net.ParseIP("93.184.216.34"), true) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "not in the allowed CIDR") +} + +func TestValidator_ValidateIP_CustomLiteralIPStillRequiresCIDR(t *testing.T) { + validator := NewValidator(WebhookSecurityPolicy{ + Mode: SecurityModeCustom, + AllowedHosts: []string{"webhook.example.com"}, + SkipResolvedIPValidation: true, + }) + + // Literal IP (not resolved from hostname) still requires CIDR allowlist + err := validator.ValidateIP(net.ParseIP("93.184.216.34"), false) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "not in the allowed CIDR") +} + +func TestValidator_ValidateIP_SkipStillRespectsBlockedCIDRs(t *testing.T) { + validator := NewValidator(WebhookSecurityPolicy{ + Mode: SecurityModeCustom, + AllowedHosts: []string{"webhook.example.com"}, + BlockedCIDRs: []string{"93.184.216.0/24"}, + SkipResolvedIPValidation: true, + }) + + // Even with skip enabled, blocked CIDRs are still enforced + err := validator.ValidateIP(net.ParseIP("93.184.216.34"), true) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "blocked") +} diff --git a/backend/webhooks/webhook.go b/backend/webhooks/webhook.go index 34195c63b..e5c0f4950 100644 --- a/backend/webhooks/webhook.go +++ b/backend/webhooks/webhook.go @@ -2,13 +2,17 @@ package webhooks import ( "bytes" + "context" "encoding/json" "fmt" - "github.com/labstack/echo/v4" - "github.com/teamhanko/hanko/backend/v2/webhooks/events" "net/http" "strings" "time" + + "github.com/labstack/echo/v4" + "github.com/teamhanko/hanko/backend/v2/config" + "github.com/teamhanko/hanko/backend/v2/webhooks/events" + "github.com/teamhanko/hanko/backend/v2/webhooks/validation" ) type Webhook interface { @@ -27,10 +31,12 @@ const ( ) type BaseWebhook struct { - Logger echo.Logger - Callback string - Events events.Events - Timeout time.Duration + Logger echo.Logger + Callback string + Events events.Events + Security config.WebhookSecurity + RequestTimeout time.Duration + ResolveTimeout time.Duration } func (bh *BaseWebhook) HasEvent(evt events.Event) bool { @@ -44,47 +50,108 @@ func (bh *BaseWebhook) HasEvent(evt events.Event) bool { } func (bh *BaseWebhook) Trigger(data JobData) error { - // create request - dataJson, err := json.Marshal(data) + validator := NewURLPolicyValidator(bh.Security) + + resolveTimeout := bh.ResolveTimeout + if resolveTimeout == 0 { + resolveTimeout = 5 * time.Second + } + + validateCtx, cancel := context.WithTimeout(context.Background(), resolveTimeout) + defer cancel() + + // Validate the callback URL and get the validated IPs to prevent DNS rebinding + validationResult, err := validator.ValidateAndGetIPs(validateCtx, bh.Callback) if err != nil { - bh.Logger.Error(fmt.Errorf("unable to convert JobData to json: %w", err)) + // Log detailed error for debugging + detailedErr := validation.GetDetailedError(err) + bh.logError(fmt.Errorf("webhook callback rejected by outbound policy: %w", detailedErr)) + // Return sanitized error to caller return err } - request, err := http.NewRequest(http.MethodPost, bh.Callback, bytes.NewReader(dataJson)) + dataJSON, err := json.Marshal(data) if err != nil { - bh.Logger.Error(fmt.Errorf("unable to create request for webhook: %w", err)) + bh.logError(fmt.Errorf("unable to marshal webhook payload: %w", err)) return err } - request.Header.Set("Content-Type", "application/json") - timeout := bh.Timeout - if timeout == 0 { - timeout = 10 * time.Second + req, err := http.NewRequest(http.MethodPost, bh.Callback, bytes.NewReader(dataJSON)) + if err != nil { + bh.logError(fmt.Errorf("unable to create webhook request: %w", err)) + return err } + req.Header.Set("Content-Type", "application/json") - client := http.Client{ - Timeout: timeout, + requestTimeout := bh.RequestTimeout + if requestTimeout == 0 { + requestTimeout = 10 * time.Second } - response, err := client.Do(request) + // Create a custom dialer that only connects to the validated IPs + // This prevents DNS rebinding attacks by bypassing DNS resolution in http.Client + dialer := NewValidatedDialer(validationResult.ValidatedIPs, validationResult.Host) + + client := &http.Client{ + Timeout: requestTimeout, + Transport: &http.Transport{ + DialContext: dialer.DialContext, + ForceAttemptHTTP2: true, + MaxIdleConns: 1, + IdleConnTimeout: 90 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + ExpectContinueTimeout: 1 * time.Second, + }, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + if !bh.Security.FollowRedirects { + return http.ErrUseLastResponse + } + + if bh.Security.MaxRedirects >= 0 && len(via) > bh.Security.MaxRedirects { + return fmt.Errorf("too many redirects") + } + + // Validate the redirect target and get its validated IPs + redirectCtx, redirectCancel := context.WithTimeout(req.Context(), resolveTimeout) + defer redirectCancel() + + redirectResult, err := validator.ValidateAndGetIPs(redirectCtx, req.URL.String()) + if err != nil { + // Log detailed error for debugging + detailedErr := validation.GetDetailedError(err) + bh.logError(fmt.Errorf("redirect target rejected by outbound policy: %w", detailedErr)) + // Return error with context (sanitization already applied by validator) + return fmt.Errorf("redirect target rejected by outbound policy: %w", err) + } + + // Update the dialer to use the new validated IPs for the redirect + // Note: This is safe because the transport is not shared across goroutines + dialer.validatedIPs = redirectResult.ValidatedIPs + dialer.originalHost = redirectResult.Host + dialer.currentIPIndex = 0 + + return nil + }, + } + + resp, err := client.Do(req) if err != nil { - bh.Logger.Error(fmt.Errorf("unable to execute webhook request: %w", err)) + bh.logError(fmt.Errorf("unable to execute webhook request: %w", err)) return err } + defer resp.Body.Close() - defer func() { - if err := response.Body.Close(); err != nil { - bh.Logger.Error(fmt.Errorf("failed to close webhook response body: %w", err)) - } - }() - - if response.StatusCode >= http.StatusBadRequest { - err := fmt.Errorf("request failed due to status code: %d", response.StatusCode) - bh.Logger.Error(err) - + if resp.StatusCode >= http.StatusMultipleChoices { + err := fmt.Errorf("webhook request failed due to status code: %d", resp.StatusCode) + bh.logError(err) return err } return nil } + +func (bh *BaseWebhook) logError(err error) { + if bh.Logger != nil { + bh.Logger.Error(err) + } +} diff --git a/backend/webhooks/webhook_test.go b/backend/webhooks/webhook_test.go index 324b2f288..2d317ca4d 100644 --- a/backend/webhooks/webhook_test.go +++ b/backend/webhooks/webhook_test.go @@ -1,19 +1,20 @@ package webhooks import ( - "github.com/labstack/gommon/log" - "github.com/stretchr/testify/require" - "github.com/teamhanko/hanko/backend/v2/webhooks/events" - "net" "net/http" "net/http/httptest" "testing" "time" + + "github.com/labstack/gommon/log" + "github.com/stretchr/testify/require" + "github.com/teamhanko/hanko/backend/v2/config" + "github.com/teamhanko/hanko/backend/v2/webhooks/events" ) func TestBaseWebhook_HasEvent(t *testing.T) { baseHook := BaseWebhook{ - Logger: nil, + Logger: log.New("test"), Callback: "http://ipsum.lorem", Events: events.Events{events.UserUpdate}, } @@ -23,7 +24,7 @@ func TestBaseWebhook_HasEvent(t *testing.T) { func TestWebhooks_HasEvent_WithMultipleEvents(t *testing.T) { baseHook := BaseWebhook{ - Logger: nil, + Logger: log.New("test"), Callback: "http://ipsum.lorem", Events: events.Events{events.UserCreate, events.UserUpdate}, } @@ -33,7 +34,7 @@ func TestWebhooks_HasEvent_WithMultipleEvents(t *testing.T) { func TestWebhooks_HasSubEvent_WithMultipleEvents(t *testing.T) { baseHook := BaseWebhook{ - Logger: nil, + Logger: log.New("test"), Callback: "http://ipsum.lorem", Events: events.Events{events.UserCreate, events.UserUpdate}, } @@ -43,7 +44,7 @@ func TestWebhooks_HasSubEvent_WithMultipleEvents(t *testing.T) { func TestBaseWebhook_HasSubEvent(t *testing.T) { baseHook := BaseWebhook{ - Logger: nil, + Logger: log.New("test"), Callback: "http://ipsum.lorem", Events: events.Events{events.UserCreate}, } @@ -53,7 +54,7 @@ func TestBaseWebhook_HasSubEvent(t *testing.T) { func TestBaseWebhook_DoesNotHaveEvent(t *testing.T) { baseHook := BaseWebhook{ - Logger: nil, + Logger: log.New("test"), Callback: "http://ipsum.lorem", Events: events.Events{events.UserCreate}, } @@ -68,9 +69,13 @@ func TestBaseWebhook_Trigger(t *testing.T) { defer server.Close() baseHook := BaseWebhook{ - Logger: nil, + Logger: log.New("test"), Callback: server.URL, Events: events.Events{events.UserCreate}, + Security: config.WebhookSecurity{ + Mode: config.WebhookSecurityModeInsecure, + AllowedSchemes: []string{"http", "https"}, + }, } data := JobData{ @@ -82,11 +87,15 @@ func TestBaseWebhook_Trigger(t *testing.T) { require.NoError(t, err) } -func TestBaseWebhook_TriggerWithWrongUrl(t *testing.T) { +func TestBaseWebhook_TriggerWithWrongURL(t *testing.T) { baseHook := BaseWebhook{ Logger: log.New("test"), Callback: "http://broken!", Events: events.Events{events.UserCreate}, + Security: config.WebhookSecurity{ + Mode: config.WebhookSecurityModeInsecure, + AllowedSchemes: []string{"http", "https"}, + }, } data := JobData{ @@ -96,7 +105,33 @@ func TestBaseWebhook_TriggerWithWrongUrl(t *testing.T) { err := baseHook.Trigger(data) require.Error(t, err) - require.Contains(t, err.Error(), "dial tcp: lookup broken!: no such host") +} + +func TestBaseWebhook_TriggerWithDisallowedSchemeFailsEvenInInsecureMode(t *testing.T) { + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + baseHook := BaseWebhook{ + Logger: log.New("test"), + Callback: server.URL, + Events: events.Events{events.UserCreate}, + Security: config.WebhookSecurity{ + Mode: config.WebhookSecurityModeInsecure, + AllowedSchemes: []string{"http"}, + }, + } + + data := JobData{ + Token: "test-token", + Event: "user", + } + + err := baseHook.Trigger(data) + + require.Error(t, err) + require.ErrorContains(t, err, "scheme") } func TestBaseWebhook_TriggerWithBadStatusCode(t *testing.T) { @@ -109,6 +144,10 @@ func TestBaseWebhook_TriggerWithBadStatusCode(t *testing.T) { Logger: log.New("test"), Callback: server.URL, Events: events.Events{events.UserCreate}, + Security: config.WebhookSecurity{ + Mode: config.WebhookSecurityModeInsecure, + AllowedSchemes: []string{"http", "https"}, + }, } data := JobData{ @@ -119,21 +158,30 @@ func TestBaseWebhook_TriggerWithBadStatusCode(t *testing.T) { err := baseHook.Trigger(data) require.Error(t, err) - require.ErrorContains(t, err, "request failed due to status code") + require.ErrorContains(t, err, "status code") } -func TestBaseWebhook_TriggerWithBadServer(t *testing.T) { - server := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - time.Sleep(600 * time.Millisecond) +func TestBaseWebhook_TriggerWithRedirectDisallowedFails(t *testing.T) { + redirectTarget := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer redirectTarget.Close() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, redirectTarget.URL, http.StatusFound) })) - server.Config.WriteTimeout = 500 * time.Millisecond - server.Start() defer server.Close() baseHook := BaseWebhook{ Logger: log.New("test"), Callback: server.URL, Events: events.Events{events.UserCreate}, + Security: config.WebhookSecurity{ + Mode: config.WebhookSecurityModeInsecure, + AllowedSchemes: []string{"http", "https"}, + FollowRedirects: false, + MaxRedirects: 0, + }, } data := JobData{ @@ -144,21 +192,89 @@ func TestBaseWebhook_TriggerWithBadServer(t *testing.T) { err := baseHook.Trigger(data) require.Error(t, err) - require.ErrorContains(t, err, "EOF") + require.ErrorContains(t, err, "status code") } -func TestBaseWebhook_TriggerTimeout(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - time.Sleep(100 * time.Millisecond) +func TestBaseWebhook_TriggerWithRedirectAllowedSucceeds(t *testing.T) { + redirectTarget := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) })) + defer redirectTarget.Close() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, redirectTarget.URL, http.StatusFound) + })) + defer server.Close() + + baseHook := BaseWebhook{ + Logger: log.New("test"), + Callback: server.URL, + Events: events.Events{events.UserCreate}, + Security: config.WebhookSecurity{ + Mode: config.WebhookSecurityModeInsecure, + AllowedSchemes: []string{"http", "https"}, + FollowRedirects: true, + MaxRedirects: 1, + }, + } + + data := JobData{ + Token: "test-token", + Event: "user", + } + + err := baseHook.Trigger(data) + + require.NoError(t, err) +} + +func TestBaseWebhook_TriggerWithRedirectRejectedByPolicyFails(t *testing.T) { + redirectServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, "http://forbidden.invalid/forbidden", http.StatusFound) + })) + defer redirectServer.Close() + + baseHook := BaseWebhook{ + Logger: log.New("test"), + Callback: redirectServer.URL, + Events: events.Events{events.UserCreate}, + Security: config.WebhookSecurity{ + Mode: config.WebhookSecurityModeInsecure, + AllowedSchemes: []string{"http", "https"}, + FollowRedirects: true, + MaxRedirects: 1, + BlockedHosts: []string{"forbidden.invalid"}, + }, + } + + data := JobData{ + Token: "test-token", + Event: "user", + } + + err := baseHook.Trigger(data) + + require.Error(t, err) + require.ErrorContains(t, err, "redirect target rejected by outbound policy") +} + +func TestBaseWebhook_TriggerWithTooManyRedirectsFails(t *testing.T) { + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, server.URL, http.StatusFound) + })) defer server.Close() baseHook := BaseWebhook{ Logger: log.New("test"), Callback: server.URL, Events: events.Events{events.UserCreate}, - Timeout: 20 * time.Millisecond, + Security: config.WebhookSecurity{ + Mode: config.WebhookSecurityModeInsecure, + AllowedSchemes: []string{"http", "https"}, + FollowRedirects: true, + MaxRedirects: 1, + }, } data := JobData{ @@ -168,9 +284,35 @@ func TestBaseWebhook_TriggerTimeout(t *testing.T) { err := baseHook.Trigger(data) - var netErr net.Error + require.Error(t, err) + require.ErrorContains(t, err, "too many redirects") +} + +func TestBaseWebhook_TriggerWithBadServer(t *testing.T) { + server := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + time.Sleep(600 * time.Millisecond) + })) + server.Config.WriteTimeout = 500 * time.Millisecond + server.Start() + defer server.Close() + + baseHook := BaseWebhook{ + Logger: log.New("test"), + Callback: server.URL, + Events: events.Events{events.UserCreate}, + RequestTimeout: 2 * time.Second, + Security: config.WebhookSecurity{ + Mode: config.WebhookSecurityModeInsecure, + AllowedSchemes: []string{"http", "https"}, + }, + } + + data := JobData{ + Token: "test-token", + Event: "user", + } + + err := baseHook.Trigger(data) require.Error(t, err) - require.ErrorAs(t, err, &netErr) - require.True(t, netErr.Timeout()) }