Skip to content

Commit

Permalink
[TT-12865] URL matching prefixes/explicit, regex support (#6475)
Browse files Browse the repository at this point in the history
### **User description**

https://tyktech.atlassian.net/browse/TT-12865

---

### **PR Type**
Bug fix, Tests


___

### **Description**
- Enhanced URL path matching to handle both stripped and full URL paths,
ensuring backward compatibility.
- Improved error handling and logging for regex matching in
`ProcessRequest`.
- Updated test cases to reflect changes in URL path handling and added
new tests for regex matching.
- Improved regex pattern handling by supporting patterns starting with
'^' in `GetPathRegexp`.


___



### **Changes walkthrough** 📝
<table><thead><tr><th></th><th align="left">Relevant
files</th></tr></thead><tbody><tr><td><strong>Bug
fix</strong></td><td><table>
<tr>
  <td>
    <details>
<summary><strong>mw_granular_access.go</strong><dd><code>Enhance URL
path matching for backward compatibility</code>&nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; </dd></summary>
<hr>

gateway/mw_granular_access.go

<li>Introduced handling for both stripped and full URL paths.<br> <li>
Added error handling for regex matching.<br> <li> Refactored logger
initialization.<br>


</details>


  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6475/files#diff-618f7d55751d572562a29506a13beba2da969436e974f8b51df7d9708c925436">+17/-8</a>&nbsp;
&nbsp; </td>

</tr>                    

<tr>
  <td>
    <details>
<summary><strong>mux.go</strong><dd><code>Improve regex pattern handling
in path matching</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

internal/httputil/mux.go

<li>Added handling for patterns starting with '^'.<br> <li> Ensured
backward compatibility with existing regex patterns.<br>


</details>


  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6475/files#diff-3d9ee5f5e946d72e6f2ae662ff03ee5253bbdc15203d2e4f6e9f46c13011ebf8">+3/-0</a>&nbsp;
&nbsp; &nbsp; </td>

</tr>                    
</table></td></tr><tr><td><strong>Tests</strong></td><td><table>
<tr>
  <td>
    <details>
<summary><strong>mw_granular_access_test.go</strong><dd><code>Update
tests for enhanced URL path matching</code>&nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
</dd></summary>
<hr>

gateway/mw_granular_access_test.go

<li>Updated test cases to include regex matching for listen paths.<br>
<li> Adjusted paths in test cases to reflect new listen path.<br> <li>
Added new test cases for regex matching.<br>


</details>


  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6475/files#diff-8e0d7cfef26688edd7d08334d955039dab5deb3caf860d29eff6d09894eaba20">+46/-27</a>&nbsp;
</td>

</tr>                    

<tr>
  <td>
    <details>
<summary><strong>mux_test.go</strong><dd><code>Add test for regex
pattern handling improvement</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

internal/httputil/mux_test.go

- Added test case for regex pattern starting with '^'.



</details>


  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6475/files#diff-8f7ce1891e221d7adb9e68f2e951f33edfbde2128187abb6e837ac01952d7888">+1/-0</a>&nbsp;
&nbsp; &nbsp; </td>

</tr>                    
</table></td></tr></tr></tbody></table>

___

> 💡 **PR-Agent usage**:
>Comment `/help` on the PR to get a list of all available PR-Agent tools
and their descriptions

---------

Co-authored-by: Tit Petric <tit@tyk.io>
  • Loading branch information
titpetric and Tit Petric authored Sep 12, 2024
1 parent 9091920 commit e2500c9
Show file tree
Hide file tree
Showing 16 changed files with 1,237 additions and 310 deletions.
6 changes: 6 additions & 0 deletions cli/linter/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,12 @@
"enable_strict_routes": {
"type": "boolean"
},
"enable_prefix_matching": {
"type": "boolean"
},
"enable_suffix_matching": {
"type": "boolean"
},
"flush_interval": {
"type": "integer"
},
Expand Down
43 changes: 43 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,49 @@ type HttpServerOptionsConfig struct {
// Regular expressions and parameterized routes will be left alone regardless of this setting.
EnableStrictRoutes bool `json:"enable_strict_routes"`

// EnablePrefixMatching changes the URL matching from wildcard mode to prefix mode.
// For example, `/json` matches `*/json*` by current default behaviour.
// If prefix matching is enabled, the match will be performed as a prefix match (`/json*`).
//
// The `/json` url would be matched as `^/json` against the following paths:
//
// - Full listen path and versioning URL (`/listen-path/v4/json`)
// - Stripped listen path URL (`/v4/json`)
// - Stripped version information (`/json`) - match.
//
// If versioning is disabled then the following URLs are considered:
//
// - Full listen path and endpoint (`/listen-path/json`)
// - Stripped listen path (`/json`) - match.
//
// For inputs that start with `/`, a prefix match is ensured by prepending
// the start of string `^` caret.
//
// For all other cases, the pattern remains unmodified.
//
// Combine this option with EnableSuffixMatching to achieve strict
// url matching with `/json` being evaluated as `^/json$`.
EnablePrefixMatching bool `json:"enable_prefix_matching"`

// EnableSuffixMatching changes the URL matching to match as a suffix.
// For example: `/json` is matched as `/json$` against the following paths:
//
// - Full listen path and versioning URL (`/listen-path/v4/json`)
// - Stripped listen path URL (`/v4/json`)
// - Stripped version information (`/json`) - match.
//
// If versioning is disabled then the following URLs are considered:
//
// - Full listen path and endpoint (`/listen-path/json`)
// - Stripped listen path (`/json`) - match.
//
// If the input pattern already ends with a `$` (`/json$`) then
// the pattern remains unmodified.
//
// Combine this option with EnablePrefixMatching to achieve strict
// url matching with `/json` being evaluated as `^/json$`.
EnableSuffixMatching bool `json:"enable_suffix_matching"`

// Disable TLS verification. Required if you are using self-signed certificates.
SSLInsecureSkipVerify bool `json:"ssl_insecure_skip_verify"`

Expand Down
56 changes: 44 additions & 12 deletions gateway/api_definition.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,8 @@ const (
// path is on any of the white, black or ignored lists. This is generated as part of the
// configuration init
type URLSpec struct {
Spec *regexp.Regexp
spec *regexp.Regexp

Status URLStatus
MethodActions map[string]apidef.EndpointMethodMeta
Whitelist apidef.EndPointMeta
Expand Down Expand Up @@ -817,21 +818,28 @@ func (a APIDefinitionLoader) getPathSpecs(apiVersionDef apidef.VersionInfo, conf
return combinedPath, len(whiteListPaths) > 0
}

// match mux tags, `{id}`.
var apiLangIDsRegex = regexp.MustCompile(`{([^}]+)}`)

func (a APIDefinitionLoader) generateRegex(stringSpec string, newSpec *URLSpec, specType URLStatus, conf config.Config) {
// replace mux named parameters with regex path match
asRegexStr := apiLangIDsRegex.ReplaceAllString(stringSpec, `([^/]+)`)
var (
pattern string
err error
)
// Hook per-api settings here via newSpec *URLSpec
isPrefixMatch := conf.HttpServerOptions.EnablePrefixMatching
isSuffixMatch := conf.HttpServerOptions.EnableSuffixMatching
isIgnoreCase := newSpec.IgnoreCase || conf.IgnoreEndpointCase

pattern = httputil.PreparePathRegexp(stringSpec, isPrefixMatch, isSuffixMatch)

// Case insensitive match
if newSpec.IgnoreCase || conf.IgnoreEndpointCase {
asRegexStr = "(?i)" + asRegexStr
if isIgnoreCase {
pattern = "(?i)" + pattern
}

asRegex, _ := regexp.Compile(asRegexStr)
asRegex, err := regexp.Compile(pattern)
log.WithError(err).Debugf("URLSpec: %s => %s type=%d", stringSpec, pattern, specType)

newSpec.Status = specType
newSpec.Spec = asRegex
newSpec.spec = asRegex
}

func (a APIDefinitionLoader) compilePathSpec(paths []string, specType URLStatus, conf config.Config) []URLSpec {
Expand Down Expand Up @@ -1493,7 +1501,7 @@ func (a *APISpec) getURLStatus(stat URLStatus) RequestStatus {
// URLAllowedAndIgnored checks if a url is allowed and ignored.
func (a *APISpec) URLAllowedAndIgnored(r *http.Request, rxPaths []URLSpec, whiteListStatus bool) (RequestStatus, interface{}) {
for i := range rxPaths {
if !rxPaths[i].Spec.MatchString(r.URL.Path) {
if !rxPaths[i].matchesPath(r.URL.Path, a) {
continue
}

Expand All @@ -1504,7 +1512,7 @@ func (a *APISpec) URLAllowedAndIgnored(r *http.Request, rxPaths []URLSpec, white

// Check if ignored
for i := range rxPaths {
if !rxPaths[i].Spec.MatchString(r.URL.Path) {
if !rxPaths[i].matchesPath(r.URL.Path, a) {
continue
}

Expand Down Expand Up @@ -1776,10 +1784,34 @@ func (a *APISpec) Version(r *http.Request) (*apidef.VersionInfo, RequestStatus)
return &version, StatusOk
}

// StripListenPath will strip the listen path from the URL, keeping version in tact.
func (a *APISpec) StripListenPath(reqPath string) string {
return httputil.StripListenPath(a.Proxy.ListenPath, reqPath)
}

// StripVersionPath will strip the version from the URL. The input URL
// should already have listen path stripped.
func (a *APISpec) StripVersionPath(reqPath string) string {
// First part of the url is the version fragment
part := strings.Split(strings.Trim(reqPath, "/"), "/")[0]

matchesUrlVersioningPattern := true
if a.VersionDefinition.UrlVersioningPattern != "" {
re, err := regexp.Compile(a.VersionDefinition.UrlVersioningPattern)
if err != nil {
log.Error("Error compiling versioning pattern: ", err)
} else {
matchesUrlVersioningPattern = re.Match([]byte(part))
}
}

if (a.VersionDefinition.StripVersioningData || a.VersionDefinition.StripPath) && matchesUrlVersioningPattern {
return strings.Replace(reqPath, "/"+part+"/", "/", 1)
}

return reqPath
}

func (a *APISpec) SanitizeProxyPaths(r *http.Request) {
if !a.Proxy.StripListenPath {
return
Expand Down
23 changes: 11 additions & 12 deletions gateway/api_definition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -412,13 +412,15 @@ func TestConflictingPaths(t *testing.T) {

ts.Run(t, []test.TestCase{
// Should ignore auth check
{Method: "POST", Path: "/customer-servicing/documents/metadata/purge", Code: http.StatusOK},
{Method: "GET", Path: "/customer-servicing/documents/metadata/{id}", Code: http.StatusOK},
{Method: "POST", Path: "/metadata/purge", Code: http.StatusOK},
{Method: "GET", Path: "/metadata/{id}", Code: http.StatusOK},
}...)
}

func TestIgnored(t *testing.T) {
ts := StartTest(nil)
ts := StartTest(func(c *config.Config) {
c.HttpServerOptions.EnablePrefixMatching = true
})
defer ts.Close()

t.Run("Extended Paths", func(t *testing.T) {
Expand All @@ -445,14 +447,13 @@ func TestIgnored(t *testing.T) {
{Path: "/ignored/literal", Code: http.StatusOK},
{Path: "/ignored/123/test", Code: http.StatusOK},
// Only GET is ignored
{Method: "POST", Path: "/ext/ignored/literal", Code: 401},
{Method: "POST", Path: "/ext/ignored/literal", Code: http.StatusUnauthorized},

{Path: "/", Code: 401},
{Path: "/", Code: http.StatusUnauthorized},
}...)
})

t.Run("Simple Paths", func(t *testing.T) {

ts.Gw.BuildAndLoadAPI(func(spec *APISpec) {
UpdateAPIVersion(spec, "v1", func(v *apidef.VersionInfo) {
v.Paths.Ignored = []string{"/ignored/literal", "/ignored/{id}/test"}
Expand All @@ -467,15 +468,14 @@ func TestIgnored(t *testing.T) {
// Should ignore auth check
{Path: "/ignored/literal", Code: http.StatusOK},
{Path: "/ignored/123/test", Code: http.StatusOK},
// All methods ignored
{Method: "POST", Path: "/ext/ignored/literal", Code: http.StatusOK},

{Path: "/", Code: 401},
{Method: "POST", Path: "/ext/ignored/literal", Code: http.StatusUnauthorized},

{Path: "/", Code: http.StatusUnauthorized},
}...)
})

t.Run("With URL rewrite", func(t *testing.T) {

ts.Gw.BuildAndLoadAPI(func(spec *APISpec) {
UpdateAPIVersion(spec, "v1", func(v *apidef.VersionInfo) {
v.ExtendedPaths.URLRewrite = []apidef.URLRewriteMeta{{
Expand Down Expand Up @@ -510,7 +510,6 @@ func TestIgnored(t *testing.T) {
})

t.Run("Case Sensitivity", func(t *testing.T) {

spec := BuildAPI(func(spec *APISpec) {
UpdateAPIVersion(spec, "v1", func(v *apidef.VersionInfo) {
v.ExtendedPaths.Ignored = []apidef.EndPointMeta{{Path: "/Foo", IgnoreCase: false}, {Path: "/bar", IgnoreCase: true}}
Expand Down Expand Up @@ -1032,7 +1031,7 @@ func (ts *Test) testPrepareDefaultVersion() (string, *APISpec) {
func TestGetVersionFromRequest(t *testing.T) {

versionInfo := apidef.VersionInfo{}
versionInfo.Paths.WhiteList = []string{"/foo"}
versionInfo.Paths.WhiteList = []string{"/foo", "/v3/foo"}
versionInfo.Paths.BlackList = []string{"/bar"}

t.Run("Header location", func(t *testing.T) {
Expand Down
10 changes: 5 additions & 5 deletions gateway/model_apispec.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ func (a *APISpec) CheckSpecMatchesStatus(r *http.Request, rxPaths []URLSpec, mod
if rxPaths[i].Status != mode {
continue
}
if !rxPaths[i].Spec.MatchString(matchPath) {
if !rxPaths[i].matchesMethod(method) {
continue
}
if !rxPaths[i].matchesMethod(method) {
if !rxPaths[i].matchesPath(matchPath, a) {
continue
}

Expand All @@ -36,10 +36,10 @@ func (a *APISpec) FindSpecMatchesStatus(r *http.Request, rxPaths []URLSpec, mode
if rxPaths[i].Status != mode {
continue
}
if !rxPaths[i].Spec.MatchString(matchPath) {
if !rxPaths[i].matchesMethod(method) {
continue
}
if !rxPaths[i].matchesMethod(method) {
if !rxPaths[i].matchesPath(matchPath, a) {
continue
}

Expand All @@ -64,7 +64,7 @@ func (a *APISpec) getMatchPathAndMethod(r *http.Request, mode URLStatus) (string
}

if a.Proxy.ListenPath != "/" {
matchPath = strings.TrimPrefix(matchPath, a.Proxy.ListenPath)
matchPath = a.StripListenPath(matchPath)
}

if !strings.HasPrefix(matchPath, "/") {
Expand Down
22 changes: 22 additions & 0 deletions gateway/model_urlspec.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,3 +98,25 @@ func (u *URLSpec) matchesMethod(method string) bool {
return false
}
}

// matchesPath takes the input string and matches it against an internal regex.
// it will match the regex against the clean URL with stripped listen path first,
// then it will match against the full URL including the listen path as provided.
// APISpec to provide URL sanitization of the input is passed along.
func (a *URLSpec) matchesPath(reqPath string, api *APISpec) bool {
clean := api.StripListenPath(reqPath)
noVersion := api.StripVersionPath(clean)
// match /users
if noVersion != clean && a.spec.MatchString(noVersion) {
return true
}
// match /v3/users
if a.spec.MatchString(clean) {
return true
}
// match /listenpath/v3/users
if a.spec.MatchString(reqPath) {
return true
}
return false
}
Loading

0 comments on commit e2500c9

Please sign in to comment.