Harden outbound SSO and webhook URL handling

internal/api/outbound_url.go

@@ -0,0 +1,170 @@
+package api
+
+import (
+	"context"
+	"crypto/tls"
+	"fmt"
+	"net"
+	"net/http"
+	"net/url"
+	"strings"
+	"time"
+)
+
+var resolveOutboundFetchIPs = net.DefaultResolver.LookupIPAddr
+var allowLoopbackOutboundFetch bool
+
+type outboundURLOptions struct {
+	allowPrivateIPs bool
+}
+
+func validateOutboundFetchURL(ctx context.Context, rawURL string, allowedSchemes []string, opts outboundURLOptions) (*url.URL, error) {
+	if strings.TrimSpace(rawURL) == "" || len(rawURL) > maxURLLength {
+		return nil, fmt.Errorf("invalid URL length")
+	}
+
+	parsed, err := url.ParseRequestURI(rawURL)
+	if err != nil {
+		return nil, fmt.Errorf("invalid URL format")
+	}
+	if parsed.User != nil {
+		return nil, fmt.Errorf("embedded credentials are not allowed")
+	}
+	if parsed.Fragment != "" {
+		return nil, fmt.Errorf("URL fragments are not allowed")
+	}
+	if parsed.Hostname() == "" {
+		return nil, fmt.Errorf("URL missing hostname")
+	}
+
+	allowed := false
+	for _, scheme := range allowedSchemes {
+		if strings.EqualFold(parsed.Scheme, scheme) {
+			allowed = true
+			break
+		}
+	}
+	if !allowed {
+		return nil, fmt.Errorf("URL scheme must be one of: %s", strings.Join(allowedSchemes, ", "))
+	}
+
+	if err := validateOutboundFetchHost(ctx, parsed.Hostname(), opts); err != nil {
+		return nil, err
+	}
+	return parsed, nil
+}
+
+func validateOutboundFetchHost(ctx context.Context, host string, opts outboundURLOptions) error {
+	host = strings.TrimSpace(host)
+	if host == "" {
+		return fmt.Errorf("URL missing hostname")
+	}
+
+	switch strings.ToLower(host) {
+	case "metadata.google.internal", "metadata.goog":
+		return fmt.Errorf("metadata service host is not allowed")
+	}
+
+	if ip := net.ParseIP(host); ip != nil {
+		return validateOutboundFetchIP(ip, opts)
+	}
+
+	resolveCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
+	defer cancel()
+
+	addrs, err := resolveOutboundFetchIPs(resolveCtx, host)
+	if err != nil {
+		return fmt.Errorf("failed to resolve hostname %s: %w", host, err)
+	}
+	if len(addrs) == 0 {
+		return fmt.Errorf("hostname %s did not resolve", host)
+	}
+
+	for _, addr := range addrs {
+		if err := validateOutboundFetchIP(addr.IP, opts); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func validateOutboundFetchIP(ip net.IP, opts outboundURLOptions) error {
+	if ip == nil {
+		return fmt.Errorf("invalid IP address")
+	}
+	if ip.IsLoopback() {
+		if allowLoopbackOutboundFetch {
+			return nil
+		}
+		return fmt.Errorf("loopback addresses are not allowed")
+	}
+	if ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() {
+		return fmt.Errorf("link-local addresses are not allowed")
+	}
+	if ip.IsMulticast() {
+		return fmt.Errorf("multicast addresses are not allowed")
+	}
+	if ip.IsUnspecified() {
+		return fmt.Errorf("unspecified addresses are not allowed")
+	}
+	if ip.Equal(net.ParseIP("169.254.169.254")) {
+		return fmt.Errorf("metadata service address is not allowed")
+	}
+	if !opts.allowPrivateIPs && isPrivateOutboundIP(ip) {
+		return fmt.Errorf("private addresses are not allowed")
+	}
+	return nil
+}
+
+func isPrivateOutboundIP(ip net.IP) bool {
+	privateRanges := []string{
+		"10.0.0.0/8",
+		"172.16.0.0/12",
+		"192.168.0.0/16",
+		"127.0.0.0/8",
+		"169.254.0.0/16",
+		"::1/128",
+		"fe80::/10",
+		"fc00::/7",
+	}
+
+	for _, cidr := range privateRanges {
+		_, ipnet, err := net.ParseCIDR(cidr)
+		if err != nil {
+			continue
+		}
+		if ipnet.Contains(ip) {
+			return true
+		}
+	}
+	return false
+}
+
+func sameOriginRedirectPolicy(allowedSchemes []string, opts outboundURLOptions) func(req *http.Request, via []*http.Request) error {
+	return func(req *http.Request, via []*http.Request) error {
+		if len(via) == 0 {
+			return nil
+		}
+		validated, err := validateOutboundFetchURL(req.Context(), req.URL.String(), allowedSchemes, opts)
+		if err != nil {
+			return err
+		}
+		origin := via[0].URL
+		if !strings.EqualFold(validated.Scheme, origin.Scheme) || !strings.EqualFold(validated.Host, origin.Host) {
+			return fmt.Errorf("redirects must stay on the same origin")
+		}
+		return nil
+	}
+}
+
+func newRestrictedOutboundHTTPClient(timeout time.Duration, opts outboundURLOptions) *http.Client {
+	transport := http.DefaultTransport.(*http.Transport).Clone()
+	transport.TLSClientConfig = &tls.Config{
+		MinVersion: tls.VersionTLS12,
+	}
+	return &http.Client{
+		Transport:     transport,
+		Timeout:       timeout,
+		CheckRedirect: sameOriginRedirectPolicy([]string{"https", "http"}, opts),
+	}
+}

internal/api/outbound_url_test.go

@@ -0,0 +1,37 @@
+package api
+
+import (
+	"context"
+	"net/http"
+	"testing"
+)
+
+func TestValidateSSOFetchURLRejectsEmbeddedCredentials(t *testing.T) {
+	_, err := validateSSOFetchURL(context.Background(), "https://user:pass@example.com/.well-known/openid-configuration")
+	if err == nil {
+		t.Fatal("expected embedded credentials to be rejected")
+	}
+}
+
+func TestValidateSSOFetchURLRejectsLoopbackByDefault(t *testing.T) {
+	_, err := validateSSOFetchURL(context.Background(), "http://127.0.0.1:8080/metadata")
+	if err == nil {
+		t.Fatal("expected loopback URL to be rejected")
+	}
+}
+
+func TestSameOriginRedirectPolicyRejectsCrossOrigin(t *testing.T) {
+	redirectedReq, err := http.NewRequest(http.MethodGet, "https://idp-two.example.com/metadata", nil)
+	if err != nil {
+		t.Fatalf("new redirected request: %v", err)
+	}
+	originReq, err := http.NewRequest(http.MethodGet, "https://idp-one.example.com/metadata", nil)
+	if err != nil {
+		t.Fatalf("new origin request: %v", err)
+	}
+
+	err = sameOriginRedirectPolicy([]string{"https", "http"}, outboundURLOptions{allowPrivateIPs: true})(redirectedReq, []*http.Request{originReq})
+	if err == nil {
+		t.Fatal("expected cross-origin redirect to be rejected")
+	}
+}

internal/api/saml_service.go

@@ -3,7 +3,6 @@ package api
 import (
 	"context"
 	"crypto/rsa"
-	"crypto/tls"
 	"crypto/x509"
 	"encoding/pem"
 	"encoding/xml"
@@ -13,6 +12,7 @@ import (
 	"net/http"
 	"net/url"
 	"os"
+	"path/filepath"
 	"strings"
 	"sync"
 	"time"
@@ -73,14 +73,9 @@ func NewSAMLService(ctx context.Context, providerID string, cfg *config.SAMLProv
 }
 
 func newSAMLHTTPClient() *http.Client {
-	transport := http.DefaultTransport.(*http.Transport).Clone()
-	transport.TLSClientConfig = &tls.Config{
-		MinVersion: tls.VersionTLS12,
-	}
-	return &http.Client{
-		Transport: transport,
-		Timeout:   30 * time.Second,
-	}
+	return newRestrictedOutboundHTTPClient(30*time.Second, outboundURLOptions{
+		allowPrivateIPs: true,
+	})
 }
 
 // loadIDPMetadata loads Identity Provider metadata from URL or XML
@@ -121,7 +116,12 @@ func (s *SAMLService) loadIDPMetadata(ctx context.Context) error {
 }
 
 func (s *SAMLService) fetchIDPMetadataFromURL(ctx context.Context, metadataURL string) (*saml.EntityDescriptor, error) {
-	req, err := http.NewRequestWithContext(ctx, http.MethodGet, metadataURL, nil)
+	validatedURL, err := validateSSOFetchURL(ctx, metadataURL)
+	if err != nil {
+		return nil, err
+	}
+
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, validatedURL.String(), nil)
 	if err != nil {
 		return nil, err
 	}
@@ -227,7 +227,7 @@ func (s *SAMLService) addIDPCertificate(metadata *saml.EntityDescriptor) error {
 	var err error
 
 	if s.config.IDPCertFile != "" {
-		certData, err = os.ReadFile(s.config.IDPCertFile)
+		certData, err = readSAMLCredentialFile(s.config.IDPCertFile)
 		if err != nil {
 			return fmt.Errorf("failed to read idp certificate file: %w", err)
 		}
@@ -341,7 +341,7 @@ func (s *SAMLService) loadSPCredentials() (*x509.Certificate, *rsa.PrivateKey, e
 
 	// Load certificate
 	if s.config.SPCertFile != "" {
-		certData, err = os.ReadFile(s.config.SPCertFile)
+		certData, err = readSAMLCredentialFile(s.config.SPCertFile)
 		if err != nil {
 			return nil, nil, fmt.Errorf("failed to read sp certificate file: %w", err)
 		}
@@ -353,7 +353,7 @@ func (s *SAMLService) loadSPCredentials() (*x509.Certificate, *rsa.PrivateKey, e
 
 	// Load private key
 	if s.config.SPKeyFile != "" {
-		keyData, err = os.ReadFile(s.config.SPKeyFile)
+		keyData, err = readSAMLCredentialFile(s.config.SPKeyFile)
 		if err != nil {
 			return nil, nil, fmt.Errorf("failed to read sp private key file: %w", err)
 		}
@@ -404,6 +404,26 @@ func (s *SAMLService) loadSPCredentials() (*x509.Certificate, *rsa.PrivateKey, e
 	return cert, key, nil
 }
 
+func readSAMLCredentialFile(rawPath string) ([]byte, error) {
+	cleaned := filepath.Clean(strings.TrimSpace(rawPath))
+	if cleaned == "" {
+		return nil, errors.New("credential file path is empty")
+	}
+
+	absolute, err := filepath.Abs(cleaned)
+	if err != nil {
+		return nil, fmt.Errorf("resolve credential file path: %w", err)
+	}
+	info, err := os.Stat(absolute)
+	if err != nil {
+		return nil, err
+	}
+	if !info.Mode().IsRegular() {
+		return nil, fmt.Errorf("credential file must be a regular file")
+	}
+	return os.ReadFile(absolute)
+}
+
 // MakeAuthRequest creates a SAML AuthnRequest and returns the redirect URL
 func (s *SAMLService) MakeAuthRequest(relayState string) (string, error) {
 	s.mu.RLock()

internal/api/saml_service_additional_test.go

@@ -85,6 +85,7 @@ func TestSAMLServiceIdentifiers(t *testing.T) {
 }
 
 func TestRefreshMetadata_Success(t *testing.T) {
+	allowSSOLoopbackFetchForTest(t)
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusOK)
 		w.Write([]byte(`<?xml version="1.0"?>
@@ -111,6 +112,7 @@ func TestRefreshMetadata_Success(t *testing.T) {
 }
 
 func TestFetchIDPMetadataFromURL_NonOK(t *testing.T) {
+	allowSSOLoopbackFetchForTest(t)
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusInternalServerError)
 	}))

internal/api/saml_service_test.go

@@ -194,6 +194,7 @@ func TestSAMLServiceBasicFlows(t *testing.T) {
 }
 
 func TestFetchMetadataFromURL(t *testing.T) {
+	allowSSOLoopbackFetchForTest(t)
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusOK)
 		w.Write([]byte(`<?xml version="1.0"?>

internal/api/sso_handlers.go

@@ -2,7 +2,6 @@ package api
 
 import (
 	"context"
-	"crypto/tls"
 	"crypto/x509"
 	"encoding/base64"
 	"encoding/json"
@@ -74,6 +73,12 @@ func validateURL(urlStr string, allowedSchemes []string) bool {
 	return false
 }
 
+func validateSSOFetchURL(ctx context.Context, rawURL string) (*url.URL, error) {
+	return validateOutboundFetchURL(ctx, rawURL, []string{"https", "http"}, outboundURLOptions{
+		allowPrivateIPs: true,
+	})
+}
+
 // SSOProviderResponse represents an SSO provider for API responses
 type SSOProviderResponse struct {
 	ID          string `json:"id"`
@@ -794,8 +799,17 @@ func (r *Router) testOIDCConnection(ctx context.Context, cfg *OIDCTestConfig) SS
 	// Fetch OIDC discovery document
 	discoveryURL := strings.TrimRight(cfg.IssuerURL, "/") + "/.well-known/openid-configuration"
 
+	validatedDiscoveryURL, err := validateSSOFetchURL(ctx, discoveryURL)
+	if err != nil {
+		return SSOTestResponse{
+			Success: false,
+			Message: "Discovery URL failed security validation",
+			Error:   err.Error(),
+		}
+	}
+
 	httpClient := newTestHTTPClient()
-	req, err := http.NewRequestWithContext(ctx, http.MethodGet, discoveryURL, nil)
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, validatedDiscoveryURL.String(), nil)
 	if err != nil {
 		return SSOTestResponse{
 			Success: false,
@@ -1018,18 +1032,18 @@ func (r *Router) handleMetadataPreview(w http.ResponseWriter, req *http.Request)
 // ============================================================================
 
 func newTestHTTPClient() *http.Client {
-	transport := http.DefaultTransport.(*http.Transport).Clone()
-	transport.TLSClientConfig = &tls.Config{
-		MinVersion: tls.VersionTLS12,
-	}
-	return &http.Client{
-		Transport: transport,
-		Timeout:   testConnectionTimeout,
-	}
+	return newRestrictedOutboundHTTPClient(testConnectionTimeout, outboundURLOptions{
+		allowPrivateIPs: true,
+	})
 }
 
 func fetchSAMLMetadataFromURL(ctx context.Context, client *http.Client, metadataURL string) ([]byte, *saml.EntityDescriptor, error) {
-	req, err := http.NewRequestWithContext(ctx, http.MethodGet, metadataURL, nil)
+	validatedURL, err := validateSSOFetchURL(ctx, metadataURL)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, validatedURL.String(), nil)
 	if err != nil {
 		return nil, nil, err
 	}

internal/api/sso_handlers_test.go

@@ -25,6 +25,15 @@ func setTestIP(req *http.Request) {
 	req.RemoteAddr = ip + ":12345"
 }
 
+func allowSSOLoopbackFetchForTest(t *testing.T) {
+	t.Helper()
+	prev := allowLoopbackOutboundFetch
+	allowLoopbackOutboundFetch = true
+	t.Cleanup(func() {
+		allowLoopbackOutboundFetch = prev
+	})
+}
+
 // Sample SAML metadata for testing
 const testSAMLMetadata = `<?xml version="1.0" encoding="UTF-8"?>
 <EntityDescriptor xmlns="urn:oasis:names:tc:SAML:2.0:metadata" entityID="https://idp.example.com">
@@ -53,6 +62,7 @@ const testOIDCDiscovery = `{
 }`
 
 func TestHandleTestSSOProvider_SAMLSuccess(t *testing.T) {
+	allowSSOLoopbackFetchForTest(t)
 	// Create mock SAML metadata server
 	metadataServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "application/xml")
@@ -138,6 +148,7 @@ func TestHandleTestSSOProvider_SAMLMetadataXML(t *testing.T) {
 }
 
 func TestHandleTestSSOProvider_SAMLFetchError(t *testing.T) {
+	allowSSOLoopbackFetchForTest(t)
 	// Server that returns 500
 	errorServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusInternalServerError)
@@ -204,6 +215,7 @@ func TestHandleTestSSOProvider_SAMLInvalidXML(t *testing.T) {
 }
 
 func TestHandleTestSSOProvider_OIDCSuccess(t *testing.T) {
+	allowSSOLoopbackFetchForTest(t)
 	// Create mock OIDC discovery server
 	discoveryServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if r.URL.Path == "/.well-known/openid-configuration" {
@@ -254,6 +266,7 @@ func TestHandleTestSSOProvider_OIDCSuccess(t *testing.T) {
 }
 
 func TestHandleTestSSOProvider_OIDCFetchError(t *testing.T) {
+	allowSSOLoopbackFetchForTest(t)
 	reqBody := SSOTestRequest{
 		Type: "oidc",
 		OIDC: &OIDCTestConfig{
@@ -360,6 +373,7 @@ func TestHandleTestSSOProvider_MissingConfig(t *testing.T) {
 }
 
 func TestHandleMetadataPreview_Success(t *testing.T) {
+	allowSSOLoopbackFetchForTest(t)
 	// Create mock metadata server
 	metadataServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "application/xml")
@@ -457,6 +471,7 @@ func TestHandleMetadataPreview_InvalidURL(t *testing.T) {
 }
 
 func TestHandleMetadataPreview_FetchError(t *testing.T) {
+	allowSSOLoopbackFetchForTest(t)
 	reqBody := MetadataPreviewRequest{
 		Type:        "saml",
 		MetadataURL: "http://localhost:99999/metadata",

internal/notifications/notifications.go

@@ -2963,6 +2963,18 @@ func (n *NotificationManager) ValidateWebhookURL(webhookURL string) error {
 	return nil
 }
 
+func (n *NotificationManager) validatedWebhookRequestURL(webhookURL string) (*url.URL, error) {
+	if err := n.ValidateWebhookURL(webhookURL); err != nil {
+		return nil, err
+	}
+	parsed, err := url.Parse(webhookURL)
+	if err != nil {
+		return nil, fmt.Errorf("invalid URL format: %w", err)
+	}
+	parsed.Fragment = ""
+	return parsed, nil
+}
+
 // isPrivateIP checks if an IP address is in a private range
 func isPrivateIP(ip net.IP) bool {
 	// Private IPv4 ranges

internal/notifications/webhook_enhanced.go

@@ -407,7 +407,12 @@ func (n *NotificationManager) sendWebhookOnceWithResponse(webhook EnhancedWebhoo
 		method = "POST"
 	}
 
-	req, err := http.NewRequest(method, webhook.URL, bytes.NewBuffer(payload))
+	validatedURL, err := n.validatedWebhookRequestURL(webhook.URL)
+	if err != nil {
+		return nil, fmt.Errorf("webhook URL validation failed: %w", err)
+	}
+
+	req, err := http.NewRequest(method, validatedURL.String(), bytes.NewBuffer(payload))
 	if err != nil {
 		return nil, fmt.Errorf("failed to create request: %w", err)
 	}
@@ -562,7 +567,12 @@ func (n *NotificationManager) TestEnhancedWebhook(webhook EnhancedWebhookConfig)
 		}
 	}
 
-	req, err := http.NewRequest(method, webhookURL, bytes.NewBuffer(payload))
+	validatedURL, err := n.validatedWebhookRequestURL(webhookURL)
+	if err != nil {
+		return 0, "", fmt.Errorf("webhook URL validation failed: %w", err)
+	}
+
+	req, err := http.NewRequest(method, validatedURL.String(), bytes.NewBuffer(payload))
 	if err != nil {
 		return 0, "", fmt.Errorf("failed to create request: %w", err)
 	}