diff --git a/config/options_test.go b/config/options_test.go index e54db2343..989841f02 100644 --- a/config/options_test.go +++ b/config/options_test.go @@ -17,7 +17,7 @@ import ( "github.com/stretchr/testify/require" ) -var cmpOptIgnoreUnexported = cmpopts.IgnoreUnexported(Options{}) +var cmpOptIgnoreUnexported = cmpopts.IgnoreUnexported(Options{}, Policy{}) func Test_Validate(t *testing.T) { t.Parallel() diff --git a/config/policy.go b/config/policy.go index 4de6de579..da461e20e 100644 --- a/config/policy.go +++ b/config/policy.go @@ -44,9 +44,10 @@ type Policy struct { Source *StringURL `yaml:",omitempty" json:"source,omitempty" hash:"ignore"` // Additional route matching options - Prefix string `mapstructure:"prefix" yaml:"prefix,omitempty" json:"prefix,omitempty"` - Path string `mapstructure:"path" yaml:"path,omitempty" json:"path,omitempty"` - Regex string `mapstructure:"regex" yaml:"regex,omitempty" json:"regex,omitempty"` + Prefix string `mapstructure:"prefix" yaml:"prefix,omitempty" json:"prefix,omitempty"` + Path string `mapstructure:"path" yaml:"path,omitempty" json:"path,omitempty"` + Regex string `mapstructure:"regex" yaml:"regex,omitempty" json:"regex,omitempty"` + compiledRegex *regexp.Regexp // Path Rewrite Options PrefixRewrite string `mapstructure:"prefix_rewrite" yaml:"prefix_rewrite,omitempty" json:"prefix_rewrite,omitempty"` @@ -496,6 +497,17 @@ func (p *Policy) Validate() error { return fmt.Errorf("config: only prefix_rewrite or regex_rewrite_pattern can be specified, but not both") } + if p.Regex != "" { + rawRE := p.Regex + if !strings.HasPrefix(rawRE, "^") { + rawRE = "^" + rawRE + } + if !strings.HasSuffix(rawRE, "$") { + rawRE += "$" + } + p.compiledRegex, _ = regexp.Compile(rawRE) + } + return nil } @@ -564,9 +576,8 @@ func (p *Policy) Matches(requestURL url.URL) bool { } } - if p.Regex != "" { - re, err := regexp.Compile(p.Regex) - if err == nil && !re.MatchString(requestURL.String()) { + if p.compiledRegex != nil { + if !p.compiledRegex.MatchString(requestURL.Path) { return false } } diff --git a/config/policy_test.go b/config/policy_test.go index b441b3bc9..cb9e7969e 100644 --- a/config/policy_test.go +++ b/config/policy_test.go @@ -6,10 +6,12 @@ import ( "testing" envoy_config_cluster_v3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3" - "github.com/golang/protobuf/proto" "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" + + "github.com/pomerium/pomerium/internal/urlutil" ) func Test_PolicyValidate(t *testing.T) { @@ -233,3 +235,38 @@ func TestPolicy_FromToPb(t *testing.T) { assert.Equal(t, p.Redirect.HTTPSRedirect, policyFromProto.Redirect.HTTPSRedirect) }) } + +func TestPolicy_Matches(t *testing.T) { + t.Run("full", func(t *testing.T) { + p := &Policy{ + From: "https://www.example.com", + To: mustParseWeightedURLs(t, "https://localhost"), + Regex: `/foo`, + } + assert.NoError(t, p.Validate()) + + assert.False(t, p.Matches(urlutil.MustParseAndValidateURL(`https://www.example.com/foo/bar`)), + "regex should only match full string") + }) + t.Run("issue2952", func(t *testing.T) { + p := &Policy{ + From: "https://www.example.com", + To: mustParseWeightedURLs(t, "https://localhost"), + Regex: `^\/foo\/bar\/[0-9a-f]\/{0,1}$`, + } + assert.NoError(t, p.Validate()) + + assert.True(t, p.Matches(urlutil.MustParseAndValidateURL(`https://www.example.com/foo/bar/0`))) + }) + t.Run("issue2592-test2", func(t *testing.T) { + p := &Policy{ + From: "https://www.example.com", + To: mustParseWeightedURLs(t, "https://localhost"), + Regex: `/admin/.*`, + } + assert.NoError(t, p.Validate()) + + assert.True(t, p.Matches(urlutil.MustParseAndValidateURL(`https://www.example.com/admin/foo`))) + assert.True(t, p.Matches(urlutil.MustParseAndValidateURL(`https://www.example.com/admin/bar`))) + }) +} diff --git a/internal/urlutil/url.go b/internal/urlutil/url.go index 87a873d72..0edecd794 100644 --- a/internal/urlutil/url.go +++ b/internal/urlutil/url.go @@ -50,6 +50,16 @@ func ParseAndValidateURL(rawurl string) (*url.URL, error) { return u, nil } +// MustParseAndValidateURL parses the URL via ParseAndValidateURL but panics if there is an error. +// (useful for testing) +func MustParseAndValidateURL(rawURL string) url.URL { + u, err := ParseAndValidateURL(rawURL) + if err != nil { + panic(err) + } + return *u +} + // ValidateURL wraps standard library's default url.Parse because // it's much more lenient about what type of urls it accepts than pomerium. func ValidateURL(u *url.URL) error {