From 9855a0fe2db2d035856cb4b737aa046894cf31ef Mon Sep 17 00:00:00 2001 From: jub0bs Date: Thu, 29 Feb 2024 16:16:41 +0100 Subject: [PATCH] simplify error messages --- fcors_invalid_policies_test.go | 52 +++++++++++++++++----------------- internal/middleware.go | 33 ++++++++------------- internal/options.go | 11 +++---- 3 files changed, 42 insertions(+), 54 deletions(-) diff --git a/fcors_invalid_policies_test.go b/fcors_invalid_policies_test.go index dc4b0e9..cd84fa9 100644 --- a/fcors_invalid_policies_test.go +++ b/fcors_invalid_policies_test.go @@ -27,16 +27,16 @@ func TestInvalidPoliciesForAllowAccess(t *testing.T) { fcors.FromOrigins("http://example.com:6060"), risky.PrivateNetworkAccess(), }, - errorMsg: `fcors: insecure origin pattern "http://example.com:6060" requires option ` + - `risky.TolerateInsecureOrigins when Private-Network Access is enabled`, + errorMsg: `fcors: insecure origin patterns like "http://example.com:6060" ` + + `are by default prohibited when Private-Network Access is enabled`, }, { desc: "option PrivateNetworkAccessInNoCORSModeOnly is used and specified origin is insecure", options: []fcors.OptionAnon{ fcors.FromOrigins("http://example.com:6060"), risky.PrivateNetworkAccessInNoCORSModeOnly(), }, - errorMsg: `fcors: insecure origin pattern "http://example.com:6060" requires option ` + - `risky.TolerateInsecureOrigins when Private-Network Access is enabled`, + errorMsg: `fcors: insecure origin patterns like "http://example.com:6060" ` + + `are by default prohibited when Private-Network Access is enabled`, }, { desc: "specified origin's host is an invalid IP address", options: []fcors.OptionAnon{fcors.FromOrigins("http://[::1]1:6060")}, @@ -106,8 +106,8 @@ func TestInvalidPoliciesForAllowAccess(t *testing.T) { ), risky.PrivateNetworkAccess(), }, - errorMsg: `fcors: insecure origin patterns "http://example.com:6060", "http://*.example.com:6060" ` + - `require option risky.TolerateInsecureOrigins when Private-Network Access is enabled`, + errorMsg: `fcors: insecure origin patterns like "http://example.com:6060", "http://*.example.com:6060" ` + + `are by default prohibited when Private-Network Access is enabled`, }, { desc: "option PrivateNetworkAccessInNoCORSModeOnly is used and some origin patterns are insecure", options: []fcors.OptionAnon{ @@ -117,8 +117,8 @@ func TestInvalidPoliciesForAllowAccess(t *testing.T) { ), risky.PrivateNetworkAccessInNoCORSModeOnly(), }, - errorMsg: `fcors: insecure origin patterns "http://example.com:6060", "http://*.example.com:6060" ` + - `require option risky.TolerateInsecureOrigins when Private-Network Access is enabled`, + errorMsg: `fcors: insecure origin patterns like "http://example.com:6060", "http://*.example.com:6060" ` + + `are by default prohibited when Private-Network Access is enabled`, }, { desc: "specified base origin's host is an invalid IP address", options: []fcors.OptionAnon{fcors.FromOrigins("http://*.[::1]1:6060")}, @@ -179,8 +179,8 @@ func TestInvalidPoliciesForAllowAccess(t *testing.T) { }, { desc: "specified base origin's host is a public suffix", options: []fcors.OptionAnon{fcors.FromOrigins("https://*.github.io")}, - errorMsg: `fcors: origin patterns like "https://*.github.io" that allow arbitrary ` + - `subdomains of public suffix "github.io" are by default prohibited`, + errorMsg: `fcors: origin patterns like "https://*.github.io" that encompass ` + + `subdomains of a public suffix are by default prohibited`, }, { desc: "missing call to FromOrigins or FromAnyOrigin", options: []fcors.OptionAnon{fcors.WithAnyMethod()}, @@ -613,16 +613,16 @@ func TestInvalidPoliciesForAllowAccessWithCredentials(t *testing.T) { }, { desc: "specified origin is insecure", options: []fcors.Option{fcors.FromOrigins("http://example.com:6060")}, - errorMsg: `fcors: insecure origin pattern "http://example.com:6060" requires ` + - `option risky.TolerateInsecureOrigins when credentialed access is enabled`, + errorMsg: `fcors: insecure origin patterns like "http://example.com:6060" ` + + `are by default prohibited when credentialed access is enabled`, }, { desc: "option PrivateNetworkAccess is used and specified origin is insecure", options: []fcors.Option{ fcors.FromOrigins("http://example.com:6060"), risky.PrivateNetworkAccess(), }, - errorMsg: `fcors: insecure origin pattern "http://example.com:6060" requires option ` + - `risky.TolerateInsecureOrigins when credentialed access is enabled and/or ` + + errorMsg: `fcors: insecure origin patterns like "http://example.com:6060" ` + + `are by default prohibited when credentialed access is enabled and/or ` + `Private-Network Access is enabled`, }, { desc: "option PrivateNetworkAccessInNoCORSModeOnly is used and specified origin is insecure", @@ -630,8 +630,8 @@ func TestInvalidPoliciesForAllowAccessWithCredentials(t *testing.T) { fcors.FromOrigins("http://example.com:6060"), risky.PrivateNetworkAccessInNoCORSModeOnly(), }, - errorMsg: `fcors: insecure origin pattern "http://example.com:6060" requires option ` + - `risky.TolerateInsecureOrigins when credentialed access is enabled and/or ` + + errorMsg: `fcors: insecure origin patterns like "http://example.com:6060" ` + + `are by default prohibited when credentialed access is enabled and/or ` + `Private-Network Access is enabled`, }, { desc: "specified origin's host is an invalid IP address", @@ -696,8 +696,8 @@ func TestInvalidPoliciesForAllowAccessWithCredentials(t *testing.T) { }, { desc: "specified base origin is insecure", options: []fcors.Option{fcors.FromOrigins("http://*.example.com:6060")}, - errorMsg: `fcors: insecure origin pattern "http://*.example.com:6060" requires option ` + - `risky.TolerateInsecureOrigins when credentialed access is enabled`, + errorMsg: `fcors: insecure origin patterns like "http://*.example.com:6060" ` + + `are by default prohibited when credentialed access is enabled`, }, { desc: "option PrivateNetworkAccess is used and some origin patterns are insecure", options: []fcors.Option{ @@ -707,8 +707,8 @@ func TestInvalidPoliciesForAllowAccessWithCredentials(t *testing.T) { ), risky.PrivateNetworkAccess(), }, - errorMsg: `fcors: insecure origin patterns "http://example.com:6060", "http://*.example.com:6060" ` + - `require option risky.TolerateInsecureOrigins when credentialed access is enabled and/or ` + + errorMsg: `fcors: insecure origin patterns like "http://example.com:6060", "http://*.example.com:6060" ` + + `are by default prohibited when credentialed access is enabled and/or ` + `Private-Network Access is enabled`, }, { desc: "option PrivateNetworkAccessInNoCORSModeOnly is used and some origin patterns are insecure", @@ -719,8 +719,8 @@ func TestInvalidPoliciesForAllowAccessWithCredentials(t *testing.T) { ), risky.PrivateNetworkAccessInNoCORSModeOnly(), }, - errorMsg: `fcors: insecure origin patterns "http://example.com:6060", "http://*.example.com:6060" ` + - `require option risky.TolerateInsecureOrigins when credentialed access is enabled and/or ` + + errorMsg: `fcors: insecure origin patterns like "http://example.com:6060", "http://*.example.com:6060" ` + + `are by default prohibited when credentialed access is enabled and/or ` + `Private-Network Access is enabled`, }, { desc: "specified base origin's host is an invalid IP address", @@ -782,8 +782,8 @@ func TestInvalidPoliciesForAllowAccessWithCredentials(t *testing.T) { }, { desc: "specified base origin's host is a public suffix", options: []fcors.Option{fcors.FromOrigins("https://*.github.io")}, - errorMsg: `fcors: origin patterns like "https://*.github.io" that allow arbitrary ` + - `subdomains of public suffix "github.io" are by default prohibited`, + errorMsg: `fcors: origin patterns like "https://*.github.io" that encompass ` + + `subdomains of a public suffix are by default prohibited`, }, { desc: "missing call to FromOrigins", options: []fcors.Option{fcors.WithAnyMethod()}, @@ -1107,8 +1107,8 @@ func TestInvalidPoliciesForAllowAccessWithCredentials(t *testing.T) { `fcors: option WithRequestHeaders used multiple times`, `fcors: specified max-age value 86401 exceeds upper bound 86400`, `fcors: option MaxAgeInSeconds used multiple times`, - `fcors: insecure origin pattern "http://example.com" requires option ` + - `risky.TolerateInsecureOrigins when credentialed access is enabled`, + `fcors: insecure origin patterns like "http://example.com" ` + + `are by default prohibited when credentialed access is enabled`, }, "\n"), }, } diff --git a/internal/middleware.go b/internal/middleware.go index 0a931c0..d01be79 100644 --- a/internal/middleware.go +++ b/internal/middleware.go @@ -57,7 +57,7 @@ func init() { } type TempConfig struct { - PublicSuffixError error + PublicSuffixes []string InsecureOriginPatterns []string SingleNonWildcardOrigin string AllowedMethods util.Set[string] @@ -118,23 +118,9 @@ func (cfg *Config) validate() error { // which itself doesn't require risky.TolerateInsecureOrigins. var errorMsg strings.Builder var patterns = cfg.tmp.InsecureOriginPatterns - if len(cfg.tmp.InsecureOriginPatterns) == 1 { - errorMsg.WriteString("insecure origin pattern ") - errorMsg.WriteByte('"') - errorMsg.WriteString(patterns[0]) - errorMsg.WriteString(`" requires `) - } else { - errorMsg.WriteString("insecure origin patterns ") - for i := 0; i < len(patterns)-1; i++ { - errorMsg.WriteByte('"') - errorMsg.WriteString(patterns[i]) - errorMsg.WriteString(`", `) - } - errorMsg.WriteByte('"') - errorMsg.WriteString(patterns[len(patterns)-1]) - errorMsg.WriteString(`" require `) - } - errorMsg.WriteString("option risky." + optTIO + " when ") + errorMsg.WriteString(`insecure origin patterns like "`) + errorMsg.WriteString(strings.Join(patterns, `", "`)) + errorMsg.WriteString(`" are by default prohibited when `) if cfg.AllowCredentials { errorMsg.WriteString("credentialed access is enabled") } @@ -147,9 +133,14 @@ func (cfg *Config) validate() error { err := util.NewError(errorMsg.String()) errs = append(errs, err) } - if cfg.tmp.PublicSuffixError != nil && - !cfg.tmp.SkipPublicSuffixCheck { - errs = append(errs, cfg.tmp.PublicSuffixError) + if len(cfg.tmp.PublicSuffixes) > 0 && !cfg.tmp.SkipPublicSuffixCheck { + var errorMsg strings.Builder + errorMsg.WriteString(`origin patterns like "`) + errorMsg.WriteString(strings.Join(cfg.tmp.PublicSuffixes, `", "`)) + errorMsg.WriteString(`" that encompass subdomains of a public suffix`) + errorMsg.WriteString(" are by default prohibited") + err := util.NewError(errorMsg.String()) + errs = append(errs, err) } if cfg.tmp.FromOriginsCalled && cfg.AllowAnyOrigin { const msg = "incompatible options " + optFO + " and " + optFAO diff --git a/internal/options.go b/internal/options.go index 9ea18e9..dd3d898 100644 --- a/internal/options.go +++ b/internal/options.go @@ -89,7 +89,7 @@ func NewMiddleware[A applier](cred bool, one A, others ...A) (Middleware, error) func FromOrigins(one string, others ...string) Option { var ( setOfPatterns = make(util.Set[origin.Pattern]) - publicSuffixError error + publicSuffixes []string insecureOriginPatterns []string nonWildcardOrigin string ) @@ -105,11 +105,8 @@ func FromOrigins(one string, others ...string) Option { nonWildcardOrigin = raw } if pattern.Kind == origin.PatternKindSubdomains { - eTLD, isEffectiveTLD := pattern.HostIsEffectiveTLD() - if isEffectiveTLD && publicSuffixError == nil { - const tmpl = "origin patterns like %q that allow arbitrary " + - "subdomains of public suffix %q are by default prohibited" - publicSuffixError = util.Errorf(tmpl, raw, eTLD) + if _, isEffectiveTLD := pattern.HostIsEffectiveTLD(); isEffectiveTLD { + publicSuffixes = append(publicSuffixes, raw) } } setOfPatterns.Add(*pattern) @@ -131,7 +128,7 @@ func FromOrigins(one string, others ...string) Option { } cfg.tmp.FromOriginsCalled = true cfg.tmp.InsecureOriginPatterns = insecureOriginPatterns - cfg.tmp.PublicSuffixError = publicSuffixError + cfg.tmp.PublicSuffixes = publicSuffixes if len(errs) != 0 { return errors.Join(errs...) }