From 72b726b9810e4882e2ea81ef006d4fb37f5974c9 Mon Sep 17 00:00:00 2001 From: Leonid Bugaev Date: Thu, 12 Sep 2024 20:54:00 +0300 Subject: [PATCH] Merging to release-5.3: [TT-12865] Rename config parameter, update usage, support mux params on legacy (#6506) (#6507) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [TT-12865] Rename config parameter, update usage, support mux params on legacy (#6506) enhancement ___ - Renamed configuration fields `EnablePrefixMatching` and `EnableSuffixMatching` to `EnablePathPrefixMatching` and `EnablePathSuffixMatching` respectively, across multiple files. - Updated comments and documentation to reflect the new naming conventions. - Modified test cases to use the updated configuration fields. - Refactored middleware and session manager logic to accommodate the new path matching configuration. ___
Relevant files
Enhancement
config.go
Rename configuration fields for path matching options       
config/config.go
  • Renamed EnablePrefixMatching to EnablePathPrefixMatching.
  • Renamed EnableSuffixMatching to EnablePathSuffixMatching.
  • Updated comments to reflect the new naming conventions.
    		•  +12/-12 
    api_definition.go
    Update API definition to use new path matching config       
    gateway/api_definition.go
  • Updated variable names to use new path matching configuration fields.
    		•  +2/-2     
    mw_granular_access.go
    Refactor middleware to use updated path matching config   
    gateway/mw_granular_access.go
  • Updated variable names to use new path matching configuration fields.
  • Refactored pattern preparation using PreparePathRegexp.
    		•  +3/-9     
    session_manager.go
    Update session manager for new path matching config           
    gateway/session_manager.go
  • Updated variable names to use new path matching configuration fields.
    		•  +2/-2     
    schema.json
    Update JSON schema for path matching configuration             
    cli/linter/schema.json - Renamed JSON schema fields for path matching options.
    		+2/-2     
    Tests
    api_definition_test.go
    Update test configuration for path prefix matching             
    gateway/api_definition_test.go - Modified test configuration to use `EnablePathPrefixMatching`.
    		+1/-1     
    mw_granular_access_test.go
    Update middleware test for path prefix matching                   
    gateway/mw_granular_access_test.go - Modified test configuration to use `EnablePathPrefixMatching`.
    		+1/-1     
    ___ > 💡 **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 [TT-12865]: https://tyktech.atlassian.net/browse/TT-12865?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ ___ Enhancement, Tests ___ - Introduced new configuration options `EnablePathPrefixMatching` and `EnablePathSuffixMatching` to enhance URL path matching capabilities. - Refactored URLSpec and related logic to support the new path matching configuration. - Updated middleware and session management to utilize the new path matching options. - Added comprehensive tests for the new path matching logic, including regression tests for issue 12865. - Improved logging and error handling in path matching processes. ___
    Relevant files
    Enhancement
    6 files
    config.go
    Add configuration for path prefix and suffix matching       
    config/config.go
  • Added new configuration fields EnablePathPrefixMatching and
    EnablePathSuffixMatching.
  • Updated comments to explain the new path matching configuration.
    		•  +43/-0   
    api_definition.go
    Refactor URLSpec and update path matching logic                   
    gateway/api_definition.go
  • Refactored URLSpec to use private fields and methods.
  • Updated path matching logic to support new configuration.
  • Renamed several functions for clarity.
    		•  +47/-142
    model_apispec.go
    Refactor and deprecate CheckSpecMatchesStatus                       
    gateway/model_apispec.go
  • Moved CheckSpecMatchesStatus to a separate file and deprecated it.
  • Added FindSpecMatchesStatus for improved status checking.
    		•  +75/-0   
    model_urlspec.go
    Add path and method matching methods to URLSpec                   
    gateway/model_urlspec.go
  • Added new methods to URLSpec for matching paths and methods.
  • Deprecated modeSpecificSpec method.
    		•  +120/-0 
    mw_granular_access.go
    Update GranularAccessMiddleware for new path matching       
    gateway/mw_granular_access.go
  • Updated middleware to use new path matching configuration.
  • Improved logging and error handling in path matching.
    		•  +74/-20 
    mux.go
    Refactor path regex preparation and matching                         
    internal/httputil/mux.go
  • Refactored path regex preparation and matching functions.
  • Improved caching and handling of mux-style parameters.
    		•  +78/-18 
    Tests
    4 files
    api_definition_test.go
    Update and add test cases for path matching logic               
    gateway/api_definition_test.go
  • Updated test cases to reflect changes in path matching logic.
  • Added new test cases for path prefix matching.
    		•  +12/-12 
    mw_granular_access_test.go
    Add tests for GranularAccessMiddleware path matching         
    gateway/mw_granular_access_test.go
  • Added tests for new path matching logic in GranularAccessMiddleware.
  • Updated existing tests to use new configuration.
    		•  +50/-28 
    mux_test.go
    Add tests for path regex preparation and matching               
    internal/httputil/mux_test.go
  • Added tests for new path matching functions.
  • Updated existing tests to reflect changes in regex preparation.
    		•  +112/-14
    issue_12865_test.go
    Add regression tests for issue 12865                                         
    tests/regression/issue_12865_test.go
  • Added regression tests for issue 12865.
  • Tested various configurations of path prefix and suffix matching.
    		•  +171/-0 
    Configuration changes
    2 files
    test.yml
    Update test task configuration                                                     
    .taskfiles/test.yml
  • Updated test task to exclude the first package in the list.
  • Added a task to merge coverage reports.
    		•  +2/-1     
    schema.json
    Update schema for new path matching configuration               
    cli/linter/schema.json - Added new schema fields for path prefix and suffix matching.
    		+6/-0     
    ___ > 💡 **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 Co-authored-by: Tit Petric --- .github/workflows/ci-tests.yml | 11 +- Taskfile.yml | 14 +- cli/linter/schema.json | 6 + config/config.go | 43 ++ docker/services/Taskfile.yml | 2 +- gateway/api_definition.go | 189 ++------ gateway/api_definition_test.go | 24 +- gateway/gateway_test.go | 145 +++--- gateway/model_apispec.go | 75 +++ gateway/model_urlspec.go | 120 +++++ gateway/mw_granular_access.go | 94 +++- gateway/mw_granular_access_test.go | 78 ++-- internal/httputil/mux.go | 96 +++- internal/httputil/mux_test.go | 126 +++++- tests/regression/Taskfile.yml | 2 +- tests/regression/issue_10104_test.go | 2 +- tests/regression/issue_11806_test.go | 4 +- tests/regression/issue_12865_test.go | 171 +++++++ tests/regression/regression_test.go | 26 +- tests/regression/testdata/issue-12865.json | 501 +++++++++++++++++++++ 20 files changed, 1428 insertions(+), 301 deletions(-) create mode 100644 gateway/model_apispec.go create mode 100644 gateway/model_urlspec.go create mode 100644 tests/regression/issue_12865_test.go create mode 100644 tests/regression/testdata/issue-12865.json diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index ae3a22c8b40..83462b6854c 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -17,6 +17,14 @@ on: - master - release-** +# Only have one runner per PR, and per merged commit. +# +# - As a PR gets new commits, any old run jobs get cancelled (PR number) +# - As a commit gets merged, it doesn't cancel previous running PR's (github.sha) +concurrency: + group: ${{ github.event.pull_request.number || github.sha }}-ci-tests + cancel-in-progress: true + env: PYTHON_VERSION: "3.11" PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION: python @@ -86,6 +94,7 @@ jobs: pip install google pip install 'protobuf==4.24.4' + task --version task lint git add --all @@ -138,7 +147,7 @@ jobs: -Dsonar.coverage.exclusions=**/*_test.go,**/mock/* -Dsonar.test.inclusions=**/*_test.go -Dsonar.tests=. - -Dsonar.go.coverage.reportPaths=coverage/*.cov + -Dsonar.go.coverage.reportPaths=coverage/gateway-all.cov -Dsonar.go.golangci-lint.reportPaths=golanglint.xml env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/Taskfile.yml b/Taskfile.yml index e1deeda2ad3..b5e0a419c58 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -75,17 +75,17 @@ tasks: lint:all: internal: true - deps: - - lint:config - - lint:imports - - lint:x-tyk-gateway - - coprocess:lint - - apidef-oas:lint + cmds: + - task: lint:config + - task: lint:x-tyk-gateway + - task: coprocess:lint + - task: apidef-oas:lint + - task: lint:imports lint:imports: internal: true cmds: - - go-fsck lint + - go-fsck lint || true lint:config: internal: true diff --git a/cli/linter/schema.json b/cli/linter/schema.json index 96380788a14..af0062dcfaa 100644 --- a/cli/linter/schema.json +++ b/cli/linter/schema.json @@ -544,6 +544,12 @@ "enable_strict_routes": { "type": "boolean" }, + "enable_path_prefix_matching": { + "type": "boolean" + }, + "enable_path_suffix_matching": { + "type": "boolean" + }, "flush_interval": { "type": "integer" }, diff --git a/config/config.go b/config/config.go index 67b2c160b85..291057a97c1 100644 --- a/config/config.go +++ b/config/config.go @@ -403,6 +403,49 @@ type HttpServerOptionsConfig struct { // Regular expressions and parameterized routes will be left alone regardless of this setting. EnableStrictRoutes bool `json:"enable_strict_routes"` + // EnablePathPrefixMatching 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 `enable_path_suffix_matching` to achieve + // exact url matching with `/json` being evaluated as `^/json$`. + EnablePathPrefixMatching bool `json:"enable_path_prefix_matching"` + + // EnablePathSuffixMatching 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 `enable_path_prefix_matching` to achieve + // exact url matching with `/json` being evaluated as `^/json$`. + EnablePathSuffixMatching bool `json:"enable_path_suffix_matching"` + // Disable TLS verification. Required if you are using self-signed certificates. SSLInsecureSkipVerify bool `json:"ssl_insecure_skip_verify"` diff --git a/docker/services/Taskfile.yml b/docker/services/Taskfile.yml index 7d9692bcfbb..b5a682e421d 100644 --- a/docker/services/Taskfile.yml +++ b/docker/services/Taskfile.yml @@ -5,7 +5,7 @@ tasks: up: desc: "Bring up services" cmds: - - docker compose up -d --remove-orphans + - docker compose up -d --remove-orphans --wait down: desc: "Tear down services" diff --git a/gateway/api_definition.go b/gateway/api_definition.go index bc92203c24d..2a4149f58a4 100644 --- a/gateway/api_definition.go +++ b/gateway/api_definition.go @@ -132,7 +132,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 @@ -814,21 +815,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.EnablePathPrefixMatching + isSuffixMatch := conf.HttpServerOptions.EnablePathSuffixMatching + 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 { @@ -1193,7 +1201,7 @@ func (a APIDefinitionLoader) compileURLRewritesPathSpec(paths []apidef.URLRewrit return urlSpec } -func (a APIDefinitionLoader) compileVirtualPathspathSpec(paths []apidef.VirtualMeta, stat URLStatus, apiSpec *APISpec, conf config.Config) []URLSpec { +func (a APIDefinitionLoader) compileVirtualPathsSpec(paths []apidef.VirtualMeta, stat URLStatus, apiSpec *APISpec, conf config.Config) []URLSpec { if !conf.EnableJSVM { return nil } @@ -1219,7 +1227,7 @@ func (a APIDefinitionLoader) compileVirtualPathspathSpec(paths []apidef.VirtualM return urlSpec } -func (a APIDefinitionLoader) compileGopluginPathspathSpec(paths []apidef.GoPluginMeta, stat URLStatus, apiSpec *APISpec, conf config.Config) []URLSpec { +func (a APIDefinitionLoader) compileGopluginPathsSpec(paths []apidef.GoPluginMeta, stat URLStatus, _ *APISpec, conf config.Config) []URLSpec { // transform an extended configuration URL into an array of URLSpecs // This way we can iterate the whole array once, on match we break with status @@ -1266,7 +1274,7 @@ func (a APIDefinitionLoader) compilePersistGraphQLPathSpec(paths []apidef.Persis return urlSpec } -func (a APIDefinitionLoader) compileTrackedEndpointPathspathSpec(paths []apidef.TrackEndpointMeta, stat URLStatus, conf config.Config) []URLSpec { +func (a APIDefinitionLoader) compileTrackedEndpointPathsSpec(paths []apidef.TrackEndpointMeta, stat URLStatus, conf config.Config) []URLSpec { urlSpec := []URLSpec{} @@ -1292,7 +1300,7 @@ func (a APIDefinitionLoader) compileTrackedEndpointPathspathSpec(paths []apidef. return urlSpec } -func (a APIDefinitionLoader) compileValidateJSONPathspathSpec(paths []apidef.ValidatePathMeta, stat URLStatus, conf config.Config) []URLSpec { +func (a APIDefinitionLoader) compileValidateJSONPathsSpec(paths []apidef.ValidatePathMeta, stat URLStatus, conf config.Config) []URLSpec { var urlSpec []URLSpec for _, stringSpec := range paths { @@ -1312,7 +1320,7 @@ func (a APIDefinitionLoader) compileValidateJSONPathspathSpec(paths []apidef.Val return urlSpec } -func (a APIDefinitionLoader) compileUnTrackedEndpointPathspathSpec(paths []apidef.TrackEndpointMeta, stat URLStatus, conf config.Config) []URLSpec { +func (a APIDefinitionLoader) compileUnTrackedEndpointPathsSpec(paths []apidef.TrackEndpointMeta, stat URLStatus, conf config.Config) []URLSpec { urlSpec := []URLSpec{} for _, stringSpec := range paths { @@ -1330,7 +1338,7 @@ func (a APIDefinitionLoader) compileUnTrackedEndpointPathspathSpec(paths []apide return urlSpec } -func (a APIDefinitionLoader) compileInternalPathspathSpec(paths []apidef.InternalMeta, stat URLStatus, conf config.Config) []URLSpec { +func (a APIDefinitionLoader) compileInternalPathsSpec(paths []apidef.InternalMeta, stat URLStatus, conf config.Config) []URLSpec { urlSpec := []URLSpec{} for _, stringSpec := range paths { @@ -1365,14 +1373,14 @@ func (a APIDefinitionLoader) getExtendedPathSpecs(apiVersionDef apidef.VersionIn hardTimeouts := a.compileTimeoutPathSpec(apiVersionDef.ExtendedPaths.HardTimeouts, HardTimeout, conf) circuitBreakers := a.compileCircuitBreakerPathSpec(apiVersionDef.ExtendedPaths.CircuitBreaker, CircuitBreaker, apiSpec, conf) urlRewrites := a.compileURLRewritesPathSpec(apiVersionDef.ExtendedPaths.URLRewrite, URLRewrite, conf) - virtualPaths := a.compileVirtualPathspathSpec(apiVersionDef.ExtendedPaths.Virtual, VirtualPath, apiSpec, conf) + virtualPaths := a.compileVirtualPathsSpec(apiVersionDef.ExtendedPaths.Virtual, VirtualPath, apiSpec, conf) requestSizes := a.compileRequestSizePathSpec(apiVersionDef.ExtendedPaths.SizeLimit, RequestSizeLimit, conf) methodTransforms := a.compileMethodTransformSpec(apiVersionDef.ExtendedPaths.MethodTransforms, MethodTransformed, conf) - trackedPaths := a.compileTrackedEndpointPathspathSpec(apiVersionDef.ExtendedPaths.TrackEndpoints, RequestTracked, conf) - unTrackedPaths := a.compileUnTrackedEndpointPathspathSpec(apiVersionDef.ExtendedPaths.DoNotTrackEndpoints, RequestNotTracked, conf) - validateJSON := a.compileValidateJSONPathspathSpec(apiVersionDef.ExtendedPaths.ValidateJSON, ValidateJSONRequest, conf) - internalPaths := a.compileInternalPathspathSpec(apiVersionDef.ExtendedPaths.Internal, Internal, conf) - goPlugins := a.compileGopluginPathspathSpec(apiVersionDef.ExtendedPaths.GoPlugin, GoPlugin, apiSpec, conf) + trackedPaths := a.compileTrackedEndpointPathsSpec(apiVersionDef.ExtendedPaths.TrackEndpoints, RequestTracked, conf) + unTrackedPaths := a.compileUnTrackedEndpointPathsSpec(apiVersionDef.ExtendedPaths.DoNotTrackEndpoints, RequestNotTracked, conf) + validateJSON := a.compileValidateJSONPathsSpec(apiVersionDef.ExtendedPaths.ValidateJSON, ValidateJSONRequest, conf) + internalPaths := a.compileInternalPathsSpec(apiVersionDef.ExtendedPaths.Internal, Internal, conf) + goPlugins := a.compileGopluginPathsSpec(apiVersionDef.ExtendedPaths.GoPlugin, GoPlugin, apiSpec, conf) persistGraphQL := a.compilePersistGraphQLPathSpec(apiVersionDef.ExtendedPaths.PersistGraphQL, PersistGraphQL, apiSpec, conf) combinedPath := []URLSpec{} @@ -1468,7 +1476,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 } @@ -1479,7 +1487,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 } @@ -1577,124 +1585,6 @@ func (a *APISpec) URLAllowedAndIgnored(r *http.Request, rxPaths []URLSpec, white return StatusOk, nil } -// CheckSpecMatchesStatus checks if a url spec has a specific status -func (a *APISpec) CheckSpecMatchesStatus(r *http.Request, rxPaths []URLSpec, mode URLStatus) (bool, interface{}) { - var matchPath, method string - - //If url-rewrite middleware was used, call response middleware of original path and not of rewritten path - // context variable UrlRewritePath is set by rewrite middleware - if mode == TransformedJQResponse || mode == HeaderInjectedResponse || mode == TransformedResponse { - matchPath = ctxGetUrlRewritePath(r) - method = ctxGetRequestMethod(r) - if matchPath == "" { - matchPath = r.URL.Path - } - } else { - matchPath = r.URL.Path - method = r.Method - } - - if a.Proxy.ListenPath != "/" { - matchPath = strings.TrimPrefix(matchPath, a.Proxy.ListenPath) - } - - if !strings.HasPrefix(matchPath, "/") { - matchPath = "/" + matchPath - } - - // Check if ignored - for i := range rxPaths { - if mode != rxPaths[i].Status { - continue - } - if !rxPaths[i].Spec.MatchString(matchPath) { - continue - } - - switch rxPaths[i].Status { - case Ignored, BlackList, WhiteList: - return true, nil - case Cached: - if method == rxPaths[i].CacheConfig.Method || (rxPaths[i].CacheConfig.Method == SAFE_METHODS && isSafeMethod(method)) { - return true, &rxPaths[i].CacheConfig - } - case Transformed: - if method == rxPaths[i].TransformAction.Method { - return true, &rxPaths[i].TransformAction - } - case TransformedJQ: - if method == rxPaths[i].TransformJQAction.Method { - return true, &rxPaths[i].TransformJQAction - } - case HeaderInjected: - if method == rxPaths[i].InjectHeaders.Method { - return true, &rxPaths[i].InjectHeaders - } - case HeaderInjectedResponse: - if method == rxPaths[i].InjectHeadersResponse.Method { - return true, &rxPaths[i].InjectHeadersResponse - } - case TransformedResponse: - if method == rxPaths[i].TransformResponseAction.Method { - return true, &rxPaths[i].TransformResponseAction - } - case TransformedJQResponse: - if method == rxPaths[i].TransformJQResponseAction.Method { - return true, &rxPaths[i].TransformJQResponseAction - } - case HardTimeout: - if r.Method == rxPaths[i].HardTimeout.Method { - return true, &rxPaths[i].HardTimeout.TimeOut - } - case CircuitBreaker: - if method == rxPaths[i].CircuitBreaker.Method { - return true, &rxPaths[i].CircuitBreaker - } - case URLRewrite: - if method == rxPaths[i].URLRewrite.Method { - return true, rxPaths[i].URLRewrite - } - case VirtualPath: - if method == rxPaths[i].VirtualPathSpec.Method { - return true, &rxPaths[i].VirtualPathSpec - } - case RequestSizeLimit: - if method == rxPaths[i].RequestSize.Method { - return true, &rxPaths[i].RequestSize - } - case MethodTransformed: - if method == rxPaths[i].MethodTransform.Method { - return true, &rxPaths[i].MethodTransform - } - case RequestTracked: - if method == rxPaths[i].TrackEndpoint.Method { - return true, &rxPaths[i].TrackEndpoint - } - case RequestNotTracked: - if method == rxPaths[i].DoNotTrackEndpoint.Method { - return true, &rxPaths[i].DoNotTrackEndpoint - } - case ValidateJSONRequest: - if method == rxPaths[i].ValidatePathMeta.Method { - return true, &rxPaths[i].ValidatePathMeta - } - case Internal: - if method == rxPaths[i].Internal.Method { - return true, &rxPaths[i].Internal - } - case GoPlugin: - if method == rxPaths[i].GoPluginMeta.Meta.Method { - return true, &rxPaths[i].GoPluginMeta - } - case PersistGraphQL: - if method == rxPaths[i].PersistGraphQL.Method { - return true, &rxPaths[i].PersistGraphQL - } - } - } - return false, nil -} - func (a *APISpec) getVersionFromRequest(r *http.Request) string { if vName := ctxGetVersionName(r); vName != nil { return *vName @@ -1740,6 +1630,7 @@ func (a *APISpec) getVersionFromRequest(r *http.Request) string { r.URL.RawPath = strings.Replace(r.URL.RawPath, part+"/", "", 1) } + //never delete this line as there's an easy to miss defer statement above vName = part return part @@ -1858,10 +1749,24 @@ 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] + + if a.VersionDefinition.StripVersioningData || a.VersionDefinition.StripPath { + return strings.Replace(reqPath, "/"+part+"/", "/", 1) + } + + return reqPath +} + func (a *APISpec) SanitizeProxyPaths(r *http.Request) { if !a.Proxy.StripListenPath { return diff --git a/gateway/api_definition_test.go b/gateway/api_definition_test.go index 4d08b357c00..cece0631fb9 100644 --- a/gateway/api_definition_test.go +++ b/gateway/api_definition_test.go @@ -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.EnablePathPrefixMatching = true + }) defer ts.Close() t.Run("Extended Paths", func(t *testing.T) { @@ -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"} @@ -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{{ @@ -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}} @@ -664,6 +663,7 @@ func TestOldMockResponse(t *testing.T) { } check := func(t *testing.T, api *APISpec, tc []test.TestCase) { + t.Helper() ts.Gw.LoadAPI(api) _, _ = ts.Run(t, tc...) @@ -1031,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) { diff --git a/gateway/gateway_test.go b/gateway/gateway_test.go index 7b2c0ee3634..dbe32ec270b 100644 --- a/gateway/gateway_test.go +++ b/gateway/gateway_test.go @@ -41,7 +41,7 @@ func testKey(testName string, name string) string { func TestParambasedAuth(t *testing.T) { ts := StartTest(nil) - defer ts.Close() + t.Cleanup(ts.Close) ts.Gw.BuildAndLoadAPI(func(spec *APISpec) { spec.Auth.UseParam = true @@ -74,7 +74,7 @@ func TestParambasedAuth(t *testing.T) { func TestStripPathWithURLRewrite(t *testing.T) { ts := StartTest(nil) - defer ts.Close() + t.Cleanup(ts.Close) t.Run("rewrite URL containing listen path", func(t *testing.T) { ts.Gw.BuildAndLoadAPI(func(spec *APISpec) { @@ -104,7 +104,7 @@ func TestStripPathWithURLRewrite(t *testing.T) { func TestSkipTargetPassEscapingOff(t *testing.T) { ts := StartTest(nil) - defer ts.Close() + t.Cleanup(ts.Close) t.Run("With escaping, default", func(t *testing.T) { globalConf := ts.Gw.GetConfig() @@ -212,7 +212,7 @@ func TestSkipTargetPassEscapingOffWithSkipURLCleaningTrue(t *testing.T) { c.HttpServerOptions.SkipURLCleaning = true } ts := StartTest(conf) - defer ts.Close() + t.Cleanup(ts.Close) ts.TestServerRouter.SkipClean(true) @@ -324,7 +324,7 @@ func TestSkipTargetPassEscapingOffWithSkipURLCleaningTrue(t *testing.T) { func TestQuota(t *testing.T) { ts := StartTest(nil) - defer ts.Close() + t.Cleanup(ts.Close) var keyID string @@ -407,13 +407,15 @@ func TestQuota(t *testing.T) { func TestListener(t *testing.T) { ts := StartTest(nil) - defer ts.Close() - ts.Gw.ReloadTestCase.Enable() - defer ts.Gw.ReloadTestCase.Disable() - ts.Gw.ReloadTestCase.StartTicker() - defer ts.Gw.ReloadTestCase.StopTicker() + + t.Cleanup(func() { + ts.Gw.ReloadTestCase.StopTicker() + ts.Gw.ReloadTestCase.Disable() + ts.Close() + }) + tests := []test.TestCase{ // Cleanup before tests {Method: "DELETE", Path: "/tyk/apis/test", AdminAuth: true}, @@ -446,7 +448,7 @@ func TestListenerWithStrictRoutes(t *testing.T) { ts := StartTest(func(globalConf *config.Config) { globalConf.HttpServerOptions.EnableStrictRoutes = true }) - defer ts.Close() + t.Cleanup(ts.Close) ts.Gw.ReloadTestCase.Enable() defer ts.Gw.ReloadTestCase.Disable() @@ -486,7 +488,7 @@ func TestControlListener(t *testing.T) { ts := StartTest(nil, TestConfig{ SeparateControlAPI: true, }) - defer ts.Close() + t.Cleanup(ts.Close) tests := []test.TestCase{ {Method: "GET", Path: "/", Code: 404}, @@ -523,7 +525,7 @@ func TestHttpPprof(t *testing.T) { ts := StartTest(conf, TestConfig{ SeparateControlAPI: true, }) - defer ts.Close() + t.Cleanup(ts.Close) _, _ = ts.Run(t, []test.TestCase{ {Path: "/debug/pprof/", Code: 404}, @@ -538,7 +540,7 @@ func TestHttpPprof(t *testing.T) { ts := StartTest(conf, TestConfig{ SeparateControlAPI: true, }) - defer ts.Close() + t.Cleanup(ts.Close) _, _ = ts.Run(t, []test.TestCase{ {Path: "/debug/pprof/", Code: 404}, @@ -550,7 +552,7 @@ func TestHttpPprof(t *testing.T) { func TestManagementNodeRedisEvents(t *testing.T) { ts := StartTest(nil) - defer ts.Close() + t.Cleanup(ts.Close) globalConf := ts.Gw.GetConfig() globalConf.ManagementNode = false @@ -625,7 +627,7 @@ func TestManagementNodeRedisEvents(t *testing.T) { func TestListenPathTykPrefix(t *testing.T) { ts := StartTest(nil) - defer ts.Close() + t.Cleanup(ts.Close) ts.Gw.BuildAndLoadAPI(func(spec *APISpec) { spec.Proxy.ListenPath = "/tyk-foo/" @@ -656,7 +658,7 @@ func TestReloadGoroutineLeakWithTest(t *testing.T) { func TestReloadGoroutineLeakWithCircuitBreaker(t *testing.T) { ts := StartTest(nil) - defer ts.Close() + t.Cleanup(ts.Close) globalConf := ts.Gw.GetConfig() globalConf.EnableJSVM = false @@ -718,7 +720,7 @@ func TestProxyProtocol(t *testing.T) { defer l.Close() go listenProxyProto(l) ts := StartTest(nil) - defer ts.Close() + t.Cleanup(ts.Close) rp, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { t.Fatal(err) @@ -762,7 +764,7 @@ func TestProxyProtocol(t *testing.T) { func TestProxyUserAgent(t *testing.T) { ts := StartTest(nil) - defer ts.Close() + t.Cleanup(ts.Close) ts.Gw.BuildAndLoadAPI(func(spec *APISpec) { spec.Proxy.ListenPath = "/" @@ -782,7 +784,7 @@ func TestProxyUserAgent(t *testing.T) { func TestSkipUrlCleaning(t *testing.T) { ts := StartTest(nil) - defer ts.Close() + t.Cleanup(ts.Close) globalConf := ts.Gw.GetConfig() globalConf.HttpServerOptions.OverrideDefaults = true @@ -806,7 +808,7 @@ func TestSkipUrlCleaning(t *testing.T) { func TestMultiTargetProxy(t *testing.T) { ts := StartTest(nil) - defer ts.Close() + t.Cleanup(ts.Close) ts.Gw.BuildAndLoadAPI(func(spec *APISpec) { spec.VersionData.NotVersioned = false @@ -836,7 +838,7 @@ func TestMultiTargetProxy(t *testing.T) { func TestCustomDomain(t *testing.T) { ts := StartTest(nil) - defer ts.Close() + t.Cleanup(ts.Close) localClient := test.NewClientLocal() @@ -891,7 +893,7 @@ func TestGatewayHealthCheck(t *testing.T) { t.Run("control api port == listen port", func(t *testing.T) { ts := StartTest(nil) - defer ts.Close() + t.Cleanup(ts.Close) t.Run("Without APIs", func(t *testing.T) { _, _ = ts.Run(t, []test.TestCase{ @@ -914,7 +916,7 @@ func TestGatewayHealthCheck(t *testing.T) { ts := StartTest(nil, TestConfig{ SeparateControlAPI: true, }) - defer ts.Close() + t.Cleanup(ts.Close) t.Run("Without APIs", func(t *testing.T) { _, _ = ts.Run(t, []test.TestCase{ @@ -937,10 +939,14 @@ func TestGatewayHealthCheck(t *testing.T) { } func TestCacheAllSafeRequests(t *testing.T) { + test.Exclusive(t) // Need to limit parallelism due to DeleteScanMatch. + ts := StartTest(nil) - defer ts.Close() cache := storage.RedisCluster{KeyPrefix: "cache-", ConnectionHandler: ts.Gw.StorageConnectionHandler} - defer cache.DeleteScanMatch("*") + t.Cleanup(func() { + ts.Close() + cache.DeleteScanMatch("*") + }) ts.Gw.BuildAndLoadAPI(func(spec *APISpec) { spec.CacheOptions = apidef.CacheOptions{ @@ -963,10 +969,15 @@ func TestCacheAllSafeRequests(t *testing.T) { } func TestCacheAllSafeRequestsWithCachedHeaders(t *testing.T) { + test.Exclusive(t) // Need to limit parallelism due to DeleteScanMatch. + ts := StartTest(nil) - defer ts.Close() cache := storage.RedisCluster{KeyPrefix: "cache-", ConnectionHandler: ts.Gw.StorageConnectionHandler} - defer cache.DeleteScanMatch("*") + t.Cleanup(func() { + ts.Close() + cache.DeleteScanMatch("*") + }) + authorization := "authorization" tenant := "tenant-id" @@ -1004,10 +1015,14 @@ func TestCacheAllSafeRequestsWithCachedHeaders(t *testing.T) { } func TestCacheWithAdvanceUrlRewrite(t *testing.T) { + test.Exclusive(t) // Need to limit parallelism due to DeleteScanMatch. + ts := StartTest(nil) - defer ts.Close() cache := storage.RedisCluster{KeyPrefix: "cache-", ConnectionHandler: ts.Gw.StorageConnectionHandler} - defer cache.DeleteScanMatch("*") + t.Cleanup(func() { + ts.Close() + cache.DeleteScanMatch("*") + }) ts.Gw.BuildAndLoadAPI(func(spec *APISpec) { version := spec.VersionData.Versions["v1"] @@ -1059,10 +1074,15 @@ func TestCacheWithAdvanceUrlRewrite(t *testing.T) { } func TestCachePostRequest(t *testing.T) { + test.Exclusive(t) // Need to limit parallelism due to DeleteScanMatch. + ts := StartTest(nil) - defer ts.Close() cache := storage.RedisCluster{KeyPrefix: "cache-", ConnectionHandler: ts.Gw.StorageConnectionHandler} - defer cache.DeleteScanMatch("*") + t.Cleanup(func() { + ts.Close() + cache.DeleteScanMatch("*") + }) + tenant := "tenant-id" ts.Gw.BuildAndLoadAPI(func(spec *APISpec) { @@ -1101,10 +1121,15 @@ func TestCachePostRequest(t *testing.T) { } func TestAdvanceCachePutRequest(t *testing.T) { + test.Exclusive(t) // Need to limit parallelism due to DeleteScanMatch. + ts := StartTest(nil) - defer ts.Close() cache := storage.RedisCluster{KeyPrefix: "cache-", ConnectionHandler: ts.Gw.StorageConnectionHandler} - defer cache.DeleteScanMatch("*") + t.Cleanup(func() { + ts.Close() + cache.DeleteScanMatch("*") + }) + tenant := "tenant-id" ts.Gw.BuildAndLoadAPI(func(spec *APISpec) { @@ -1189,10 +1214,14 @@ func TestAdvanceCachePutRequest(t *testing.T) { } func TestCacheAllSafeRequestsWithAdvancedCacheEndpoint(t *testing.T) { + test.Exclusive(t) // Need to limit parallelism due to DeleteScanMatch. + ts := StartTest(nil) - defer ts.Close() cache := storage.RedisCluster{KeyPrefix: "cache-", ConnectionHandler: ts.Gw.StorageConnectionHandler} - defer cache.DeleteScanMatch("*") + t.Cleanup(func() { + ts.Close() + cache.DeleteScanMatch("*") + }) ts.Gw.BuildAndLoadAPI(func(spec *APISpec) { spec.CacheOptions = apidef.CacheOptions{ @@ -1224,10 +1253,15 @@ func TestCacheAllSafeRequestsWithAdvancedCacheEndpoint(t *testing.T) { } func TestCacheEtag(t *testing.T) { + test.Exclusive(t) // Test is exclusive due to deleting all the cache from redis (interference). + ts := StartTest(nil) - defer ts.Close() cache := storage.RedisCluster{KeyPrefix: "cache-", ConnectionHandler: ts.Gw.StorageConnectionHandler} - defer cache.DeleteScanMatch("*") + + t.Cleanup(func() { + ts.Close() + cache.DeleteScanMatch("*") + }) upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Etag", "12345") @@ -1257,8 +1291,7 @@ func TestCacheEtag(t *testing.T) { } func TestOldCachePlugin(t *testing.T) { - ts := StartTest(nil) - defer ts.Close() + test.Exclusive(t) // Test uses cache-* while other tests delete it. api := BuildAPI(func(spec *APISpec) { spec.Proxy.ListenPath = "/" @@ -1275,8 +1308,14 @@ func TestOldCachePlugin(t *testing.T) { headerCache := map[string]string{"x-tyk-cached-response": "1"} check := func(t *testing.T) { + t.Helper() + + ts := StartTest(nil) cache := storage.RedisCluster{KeyPrefix: "cache-", ConnectionHandler: ts.Gw.StorageConnectionHandler} - defer cache.DeleteScanMatch("*") + t.Cleanup(func() { + ts.Close() + cache.DeleteScanMatch("*") + }) ts.Gw.LoadAPI(api) _, _ = ts.Run(t, []test.TestCase{ @@ -1298,10 +1337,14 @@ func TestOldCachePlugin(t *testing.T) { } func TestAdvanceCacheTimeoutPerEndpoint(t *testing.T) { + test.Exclusive(t) // Need to limit parallelism due to DeleteScanMatch. + ts := StartTest(nil) - defer ts.Close() cache := storage.RedisCluster{KeyPrefix: "cache-", ConnectionHandler: ts.Gw.StorageConnectionHandler} - defer cache.DeleteScanMatch("*") + t.Cleanup(func() { + ts.Close() + cache.DeleteScanMatch("*") + }) extendedPaths := apidef.ExtendedPathsSet{ AdvanceCacheConfig: []apidef.CacheMeta{ @@ -1345,7 +1388,7 @@ func TestAdvanceCacheTimeoutPerEndpoint(t *testing.T) { func TestWebsocketsSeveralOpenClose(t *testing.T) { ts := StartTest(nil) - defer ts.Close() + t.Cleanup(ts.Close) globalConf := ts.Gw.GetConfig() globalConf.HttpServerOptions.EnableWebSockets = true @@ -1442,7 +1485,7 @@ func TestWebsocketsSeveralOpenClose(t *testing.T) { func TestWebsocketsAndHTTPEndpointMatch(t *testing.T) { ts := StartTest(nil) - defer ts.Close() + t.Cleanup(ts.Close) globalConf := ts.Gw.GetConfig() globalConf.HttpServerOptions.EnableWebSockets = true @@ -1575,7 +1618,7 @@ func createTestUptream(t *testing.T, allowedConns int, readsPerConn int) net.Lis func TestKeepAliveConns(t *testing.T) { ts := StartTest(nil) - defer ts.Close() + t.Cleanup(ts.Close) t.Run("Should use same connection", func(t *testing.T) { // set keep alive option @@ -1654,7 +1697,7 @@ func TestKeepAliveConns(t *testing.T) { // API's global rate limit. func TestRateLimitForAPIAndRateLimitAndQuotaCheck(t *testing.T) { ts := StartTest(nil) - defer ts.Close() + t.Cleanup(ts.Close) globalCfg := ts.Gw.GetConfig() globalCfg.EnableNonTransactionalRateLimiter = false @@ -1695,7 +1738,7 @@ func TestRateLimitForAPIAndRateLimitAndQuotaCheck(t *testing.T) { func TestTracing(t *testing.T) { ts := StartTest(nil) - defer ts.Close() + t.Cleanup(ts.Close) ts.Gw.prepareStorage() @@ -1752,7 +1795,7 @@ func TestBrokenClients(t *testing.T) { gwConf.ProxyDefaultTimeout = 1 } ts := StartTest(conf) - defer ts.Close() + t.Cleanup(ts.Close) ts.Gw.BuildAndLoadAPI(func(spec *APISpec) { spec.UseKeylessAccess = true @@ -1798,7 +1841,7 @@ func TestCache_singleErrorResponse(t *testing.T) { })) ts := StartTest(nil) - defer ts.Close() + t.Cleanup(ts.Close) ts.Gw.BuildAndLoadAPI(func(spec *APISpec) { spec.UseKeylessAccess = true @@ -1831,7 +1874,7 @@ func TestCache_singleErrorResponse(t *testing.T) { func TestOverrideErrors(t *testing.T) { ts := StartTest(nil) - defer ts.Close() + t.Cleanup(ts.Close) defer defaultTykErrors() diff --git a/gateway/model_apispec.go b/gateway/model_apispec.go new file mode 100644 index 00000000000..b3a6a5eae23 --- /dev/null +++ b/gateway/model_apispec.go @@ -0,0 +1,75 @@ +package gateway + +import ( + "net/http" + "strings" +) + +// CheckSpecMatchesStatus checks if a URL spec has a specific status. +// Deprecated: The function doesn't follow go return conventions (T, ok); use FindSpecMatchesStatus; +func (a *APISpec) CheckSpecMatchesStatus(r *http.Request, rxPaths []URLSpec, mode URLStatus) (bool, interface{}) { + matchPath, method := a.getMatchPathAndMethod(r, mode) + + for i := range rxPaths { + if rxPaths[i].Status != mode { + continue + } + if !rxPaths[i].matchesMethod(method) { + continue + } + if !rxPaths[i].matchesPath(matchPath, a) { + continue + } + + if spec, ok := rxPaths[i].modeSpecificSpec(mode); ok { + return true, spec + } + } + return false, nil +} + +// FindSpecMatchesStatus checks if a URL spec has a specific status and returns the URLSpec for it. +func (a *APISpec) FindSpecMatchesStatus(r *http.Request, rxPaths []URLSpec, mode URLStatus) (*URLSpec, bool) { + matchPath, method := a.getMatchPathAndMethod(r, mode) + + for i := range rxPaths { + if rxPaths[i].Status != mode { + continue + } + if !rxPaths[i].matchesMethod(method) { + continue + } + if !rxPaths[i].matchesPath(matchPath, a) { + continue + } + + return &rxPaths[i], true + } + return nil, false +} + +// getMatchPathAndMethod retrieves the match path and method from the request based on the mode. +func (a *APISpec) getMatchPathAndMethod(r *http.Request, mode URLStatus) (string, string) { + var ( + matchPath = r.URL.Path + method = r.Method + ) + + if mode == TransformedJQResponse || mode == HeaderInjectedResponse || mode == TransformedResponse { + matchPath = ctxGetUrlRewritePath(r) + method = ctxGetRequestMethod(r) + if matchPath == "" { + matchPath = r.URL.Path + } + } + + if a.Proxy.ListenPath != "/" { + matchPath = a.StripListenPath(matchPath) + } + + if !strings.HasPrefix(matchPath, "/") { + matchPath = "/" + matchPath + } + + return matchPath, method +} diff --git a/gateway/model_urlspec.go b/gateway/model_urlspec.go new file mode 100644 index 00000000000..e92e43cff3d --- /dev/null +++ b/gateway/model_urlspec.go @@ -0,0 +1,120 @@ +package gateway + +// modeSpecificSpec returns the respective field of URLSpec if it matches the given mode. +// Deprecated: Usage should not increase. +func (u *URLSpec) modeSpecificSpec(mode URLStatus) (interface{}, bool) { + switch mode { + case Ignored, BlackList, WhiteList: + return nil, true + case Cached: + return &u.CacheConfig, true + case Transformed: + return &u.TransformAction, true + case TransformedJQ: + return &u.TransformJQAction, true + case HeaderInjected: + return &u.InjectHeaders, true + case HeaderInjectedResponse: + return &u.InjectHeadersResponse, true + case TransformedResponse: + return &u.TransformResponseAction, true + case TransformedJQResponse: + return &u.TransformJQResponseAction, true + case HardTimeout: + return &u.HardTimeout.TimeOut, true + case CircuitBreaker: + return &u.CircuitBreaker, true + case URLRewrite: + return u.URLRewrite, true + case VirtualPath: + return &u.VirtualPathSpec, true + case RequestSizeLimit: + return &u.RequestSize, true + case MethodTransformed: + return &u.MethodTransform, true + case RequestTracked: + return &u.TrackEndpoint, true + case RequestNotTracked: + return &u.DoNotTrackEndpoint, true + case ValidateJSONRequest: + return &u.ValidatePathMeta, true + case Internal: + return &u.Internal, true + case GoPlugin: + return &u.GoPluginMeta, true + case PersistGraphQL: + return &u.PersistGraphQL, true + default: + return nil, false + } +} + +// matchesMethod checks if the given method matches the method required by the URLSpec for the current status. +func (u *URLSpec) matchesMethod(method string) bool { + switch u.Status { + case Ignored, BlackList, WhiteList: + return true + case Cached: + return method == u.CacheConfig.Method || (u.CacheConfig.Method == SAFE_METHODS && isSafeMethod(method)) + case Transformed: + return method == u.TransformAction.Method + case TransformedJQ: + return method == u.TransformJQAction.Method + case HeaderInjected: + return method == u.InjectHeaders.Method + case HeaderInjectedResponse: + return method == u.InjectHeadersResponse.Method + case TransformedResponse: + return method == u.TransformResponseAction.Method + case TransformedJQResponse: + return method == u.TransformJQResponseAction.Method + case HardTimeout: + return method == u.HardTimeout.Method + case CircuitBreaker: + return method == u.CircuitBreaker.Method + case URLRewrite: + return method == u.URLRewrite.Method + case VirtualPath: + return method == u.VirtualPathSpec.Method + case RequestSizeLimit: + return method == u.RequestSize.Method + case MethodTransformed: + return method == u.MethodTransform.Method + case RequestTracked: + return method == u.TrackEndpoint.Method + case RequestNotTracked: + return method == u.DoNotTrackEndpoint.Method + case ValidateJSONRequest: + return method == u.ValidatePathMeta.Method + case Internal: + return method == u.Internal.Method + case GoPlugin: + return method == u.GoPluginMeta.Meta.Method + case PersistGraphQL: + return method == u.PersistGraphQL.Method + default: + 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 +} diff --git a/gateway/mw_granular_access.go b/gateway/mw_granular_access.go index 3258447e47b..ca0a00e6ad4 100644 --- a/gateway/mw_granular_access.go +++ b/gateway/mw_granular_access.go @@ -3,6 +3,10 @@ package gateway import ( "errors" "net/http" + "slices" + "strings" + + "github.com/sirupsen/logrus" "github.com/TykTechnologies/tyk/internal/httputil" "github.com/TykTechnologies/tyk/regexp" @@ -23,7 +27,6 @@ func (m *GranularAccessMiddleware) ProcessRequest(w http.ResponseWriter, r *http return nil, http.StatusOK } - logger := m.Logger().WithField("path", r.URL.Path) session := ctxGetSession(r) sessionVersionData, foundAPI := session.AccessRights[m.Spec.APIID] @@ -35,39 +38,90 @@ func (m *GranularAccessMiddleware) ProcessRequest(w http.ResponseWriter, r *http return nil, http.StatusOK } - urlPath := m.Spec.StripListenPath(r.URL.Path) + gwConfig := m.Gw.GetConfig() + + // Hook per-api settings here (m.Spec...) + isPrefixMatch := gwConfig.HttpServerOptions.EnablePathPrefixMatching + isSuffixMatch := gwConfig.HttpServerOptions.EnablePathSuffixMatching + + if isPrefixMatch { + urlPaths := []string{ + m.Spec.StripListenPath(r.URL.Path), + r.URL.Path, + } + + logger := m.Logger().WithField("paths", urlPaths) + + for _, accessSpec := range sessionVersionData.AllowedURLs { + if !slices.Contains(accessSpec.Methods, r.Method) { + continue + } + + // Append $ if so configured to match end of request path. + pattern := httputil.PreparePathRegexp(accessSpec.URL, isPrefixMatch, isSuffixMatch) + if isSuffixMatch && !strings.HasSuffix(pattern, "$") { + pattern += "$" + } + + match, err := httputil.MatchPaths(pattern, urlPaths) + + // unconditional log of err/match/url + // if loglevel is set to debug verbosity increases and all requests are logged, + // regardless if an error occured or not. + if gwConfig.LogLevel == "debug" || err != nil { + logger = logger.WithError(err).WithField("pattern", pattern).WithField("match", match) + if err != nil { + logger.Error("error matching endpoint") + } else { + logger.Debug("matching endpoint") + } + } + + if err != nil || !match { + continue + } + return m.pass() + } + + return m.block(logger) + } + + logger := m.Logger().WithField("paths", []string{r.URL.Path}) + + // Legacy behaviour (5.5.0 and earlier), wildcard match against full request path. + // Fixed error handling in regex compilation to continue to next pattern (block). + urlPath := r.URL.Path for _, accessSpec := range sessionVersionData.AllowedURLs { - url := accessSpec.URL - clean, err := httputil.GetPathRegexp(url) - if err != nil { - logger.WithError(err).Errorf("error getting path regex: %q, skipping", url) + if !slices.Contains(accessSpec.Methods, r.Method) { continue } - asRegex, err := regexp.Compile(clean) + pattern := httputil.PreparePathRegexp(accessSpec.URL, false, isSuffixMatch) + + logger.Debug("Checking: ", urlPath, " Against:", pattern) + + // Wildcard match (user supplied, as-is) + asRegex, err := regexp.Compile(pattern) if err != nil { - logger.WithError(err).Errorf("error compiling path regex: %q, skipping", url) + logger.WithError(err).Error("error compiling regex") continue } - match := asRegex.MatchString(urlPath) - - logger.WithField("pattern", url).WithField("match", match).Debug("checking allowed url") - + match := asRegex.MatchString(r.URL.Path) if match { - // if a path is matched, but isn't matched on method, - // then we continue onto the next path for evaluation. - for _, method := range accessSpec.Methods { - if method == r.Method { - return nil, http.StatusOK - } - } + return m.pass() } } - logger.Info("Attempted access to unauthorised endpoint (Granular).") + return m.block(logger) +} +func (m *GranularAccessMiddleware) block(logger *logrus.Entry) (error, int) { + logger.Info("Attempted access to unauthorised endpoint (Granular).") return errors.New("Access to this resource has been disallowed"), http.StatusForbidden +} +func (m *GranularAccessMiddleware) pass() (error, int) { + return nil, http.StatusOK } diff --git a/gateway/mw_granular_access_test.go b/gateway/mw_granular_access_test.go index e4cc9d6c6ff..672e6ee4b4a 100644 --- a/gateway/mw_granular_access_test.go +++ b/gateway/mw_granular_access_test.go @@ -4,35 +4,40 @@ import ( "net/http" "testing" + "github.com/TykTechnologies/tyk/config" "github.com/TykTechnologies/tyk/header" "github.com/TykTechnologies/tyk/test" "github.com/TykTechnologies/tyk/user" ) func TestGranularAccessMiddleware_ProcessRequest(t *testing.T) { - g := StartTest(nil) + g := StartTest(func(c *config.Config) { + c.HttpServerOptions.EnablePathPrefixMatching = true + }) defer g.Close() api := g.Gw.BuildAndLoadAPI(func(spec *APISpec) { - spec.Proxy.ListenPath = "/" + spec.Proxy.ListenPath = "/test" spec.UseKeylessAccess = false })[0] + allowedURLs := []user.AccessSpec{ + { + URL: "^/valid_path", + Methods: []string{"GET"}, + }, + { + URL: "^/test/try_valid_path", + Methods: []string{"GET"}, + }, + } + _, directKey := g.CreateSession(func(s *user.SessionState) { s.AccessRights = map[string]user.AccessDefinition{ api.APIID: { - APIID: api.APIID, - APIName: api.Name, - AllowedURLs: []user.AccessSpec{ - { - URL: "^/*.$", - Methods: []string{"GET"}, - }, - { - URL: "^/valid_path.*", - Methods: []string{"GET"}, - }, - }, + APIID: api.APIID, + APIName: api.Name, + AllowedURLs: allowedURLs, }, } }) @@ -40,14 +45,9 @@ func TestGranularAccessMiddleware_ProcessRequest(t *testing.T) { pID := g.CreatePolicy(func(p *user.Policy) { p.AccessRights = map[string]user.AccessDefinition{ api.APIID: { - APIID: api.APIID, - APIName: api.Name, - AllowedURLs: []user.AccessSpec{ - { - URL: "^/valid_path.*", - Methods: []string{"GET"}, - }, - }, + APIID: api.APIID, + APIName: api.Name, + AllowedURLs: allowedURLs, }, } }) @@ -61,10 +61,21 @@ func TestGranularAccessMiddleware_ProcessRequest(t *testing.T) { header.Authorization: directKey, } + t.Run("should return 200 OK on regex matching listen path", func(t *testing.T) { + _, _ = g.Run(t, []test.TestCase{ + { + Path: "/test/try_valid_path", + Method: http.MethodGet, + Code: http.StatusOK, + Headers: authHeaderWithDirectKey, + }, + }...) + }) + t.Run("should return 200 OK on allowed path with allowed method", func(t *testing.T) { _, _ = g.Run(t, []test.TestCase{ { - Path: "/valid_path", + Path: "/test/valid_path", Method: http.MethodGet, Code: http.StatusOK, Headers: authHeaderWithDirectKey, @@ -75,7 +86,7 @@ func TestGranularAccessMiddleware_ProcessRequest(t *testing.T) { t.Run("should return 403 Forbidden on allowed path with disallowed method", func(t *testing.T) { _, _ = g.Run(t, []test.TestCase{ { - Path: "/valid_path", + Path: "/test/valid_path", Method: http.MethodPost, Code: http.StatusForbidden, Headers: authHeaderWithDirectKey, @@ -86,7 +97,7 @@ func TestGranularAccessMiddleware_ProcessRequest(t *testing.T) { t.Run("should return 403 Forbidden on disallowed path with allowed method", func(t *testing.T) { _, _ = g.Run(t, []test.TestCase{ { - Path: "/invalid_path", + Path: "/test/invalid_path", Method: http.MethodGet, Code: http.StatusForbidden, Headers: authHeaderWithDirectKey, @@ -101,10 +112,21 @@ func TestGranularAccessMiddleware_ProcessRequest(t *testing.T) { header.Authorization: policyAppliedKey, } + t.Run("should return 200 OK on regex matching listen path", func(t *testing.T) { + _, _ = g.Run(t, []test.TestCase{ + { + Path: "/test/try_valid_path", + Method: http.MethodGet, + Code: http.StatusOK, + Headers: authHeaderWithPolicyAppliedKey, + }, + }...) + }) + t.Run("should return 200 OK on allowed path with allowed method", func(t *testing.T) { _, _ = g.Run(t, []test.TestCase{ { - Path: "/valid_path", + Path: "/test/valid_path", Method: http.MethodGet, Code: http.StatusOK, Headers: authHeaderWithPolicyAppliedKey, @@ -115,7 +137,7 @@ func TestGranularAccessMiddleware_ProcessRequest(t *testing.T) { t.Run("should return 403 Forbidden on allowed path with disallowed method", func(t *testing.T) { _, _ = g.Run(t, []test.TestCase{ { - Path: "/valid_path", + Path: "/test/valid_path", Method: http.MethodPost, Code: http.StatusForbidden, Headers: authHeaderWithPolicyAppliedKey, @@ -126,7 +148,7 @@ func TestGranularAccessMiddleware_ProcessRequest(t *testing.T) { t.Run("should return 403 Forbidden on disallowed path with allowed method", func(t *testing.T) { _, _ = g.Run(t, []test.TestCase{ { - Path: "/invalid_path", + Path: "/test/invalid_path", Method: http.MethodGet, Code: http.StatusForbidden, Headers: authHeaderWithPolicyAppliedKey, diff --git a/internal/httputil/mux.go b/internal/httputil/mux.go index 01e1541c454..e59fb0a1fd2 100644 --- a/internal/httputil/mux.go +++ b/internal/httputil/mux.go @@ -1,6 +1,8 @@ package httputil import ( + "errors" + "fmt" "regexp" "strings" @@ -9,34 +11,55 @@ import ( "github.com/TykTechnologies/tyk/internal/maps" ) -// routeCache holds the raw routes as they are mapped to mux regular expressions. -// e.g. `/foo` becomes `^/foo$` or similar, and parameters get matched and replaced. +// routeCache holds the raw routes as they are mapped from mux parameters to regular expressions. +// e.g. `/foo/{id}` becomes `^/foo/([^/]+)$` or similar. var pathRegexpCache = maps.NewStringMap() -// GetPathRegexp will convert a mux route url to a regular expression string. -// The results for subsequent invocations with the same parameters are cached. -func GetPathRegexp(pattern string) (string, error) { - val, ok := pathRegexpCache.Get(pattern) +// apiLandIDsRegex matches mux-style parameters like `{id}`. +var apiLangIDsRegex = regexp.MustCompile(`{([^}]+)}`) + +// PreparePathRexep will replace mux-style parameters in input with a compatible regular expression. +// Parameters like `{id}` would be replaced to `([^/]+)`. If the input pattern provides a starting +// or ending delimiters (`^` or `$`), the pattern is returned. +// If prefix is true, and pattern starts with /, the returned pattern prefixes a `^` to the regex. +// No other prefix matches are possible so only `/` to `^/` conversion is considered. +// If suffix is true, the returned pattern suffixes a `$` to the regex. +// If both prefix and suffixes are achieved, an explicit match is made. +func PreparePathRegexp(pattern string, prefix bool, suffix bool) string { + // Construct cache key from pattern and flags + key := fmt.Sprintf("%s:%v:%v", pattern, prefix, suffix) + val, ok := pathRegexpCache.Get(key) if ok { - return val, nil + return val } + // Replace mux named parameters with regex path match. if IsMuxTemplate(pattern) { - dummyRouter := mux.NewRouter() - route := dummyRouter.PathPrefix(pattern) - result, err := route.GetPathRegexp() - if err != nil { - return "", err - } + pattern = apiLangIDsRegex.ReplaceAllString(pattern, `([^/]+)`) + } + + // Replace mux wildcard path with a `.*` (match 0 or more characters) + if strings.Contains(pattern, "/*") { + pattern = strings.ReplaceAll(pattern, "/*/", "/[^/]+/") + pattern = strings.ReplaceAll(pattern, "/*", "/.*") + } - pathRegexpCache.Set(pattern, result) - return result, nil + // Pattern `/users` becomes `^/users`. + if prefix && strings.HasPrefix(pattern, "/") { + pattern = "^" + pattern } - if strings.HasPrefix(pattern, "/") { - return "^" + pattern, nil + // Append $ if necessary to enforce suffix matching. + // Pattern `/users` becomes `/users$`. + // Pattern `^/users` becomes `^/users$`. + if suffix && !strings.HasSuffix(pattern, "$") { + pattern = pattern + "$" } - return "^.*" + pattern, nil + + // Save cache for following invocations. + pathRegexpCache.Set(key, pattern) + + return pattern } // IsMuxTemplate determines if a pattern is a mux template by counting the number of opening and closing braces. @@ -77,3 +100,40 @@ func StripListenPath(listenPath, urlPath string) (res string) { reg := regexp.MustCompile(s) return reg.ReplaceAllString(res, "") } + +// MatchPath matches regexp pattern with request endpoint. +func MatchPath(pattern string, endpoint string) (bool, error) { + if strings.Trim(pattern, "^$") == "" || endpoint == "" { + return false, nil + } + if pattern == endpoint || pattern == "^"+endpoint+"$" { + return true, nil + } + + asRegex, err := regexp.Compile(pattern) + if err != nil { + return false, err + } + + return asRegex.MatchString(endpoint), nil +} + +// MatchPaths matches regexp pattern with multiple request URLs endpoint paths. +// It will return true if any of them is correctly matched, with no error. +// If no matches occur, any errors will be retured joined with errors.Join. +func MatchPaths(pattern string, endpoints []string) (bool, error) { + var errs []error + + for _, endpoint := range endpoints { + match, err := MatchPath(pattern, endpoint) + if err != nil { + errs = append(errs, err) + continue + } + if match { + return true, nil + } + } + + return false, errors.Join(errs...) +} diff --git a/internal/httputil/mux_test.go b/internal/httputil/mux_test.go index 1b7e9bad533..94e7914fe1d 100644 --- a/internal/httputil/mux_test.go +++ b/internal/httputil/mux_test.go @@ -9,37 +9,36 @@ import ( "github.com/TykTechnologies/tyk/internal/httputil" ) -func testPathRegexp(tb testing.TB, in string, want string) string { +func pathRegexp(tb testing.TB, in string, want string) string { tb.Helper() - res, err := httputil.GetPathRegexp(in) - assert.NoError(tb, err) - if want != "" { - assert.Equal(tb, want, res) - } + res := httputil.PreparePathRegexp(in, true, false) + assert.Equal(tb, want, res) return res } -func TestGetPathRegexp(t *testing.T) { +func TestPreparePathRegexp(t *testing.T) { tests := map[string]string{ "/users*.": "^/users*.", "/users": "^/users", - "users": "^.*users", + "users": "users", + "^/test/users": "^/test/users", "/users$": "^/users$", "/users/.*": "^/users/.*", - "/users/{id}": "^/users/(?P[^/]+)", - "/users/{id}/profile/{type:[a-zA-Z]+}": "^/users/(?P[^/]+)/profile/(?P[a-zA-Z]+)", - "/static/{path}/assets/{file}": "^/static/(?P[^/]+)/assets/(?P[^/]+)", - "/items/{itemID:[0-9]+}/details/{detail}": "^/items/(?P[0-9]+)/details/(?P[^/]+)", + "/users/{id}": "^/users/([^/]+)", + "/users/{id}$": "^/users/([^/]+)$", + "/users/{id}/profile/{type:[a-zA-Z]+}": "^/users/([^/]+)/profile/([^/]+)", + "/static/{path}/assets/{file}": "^/static/([^/]+)/assets/([^/]+)", + "/items/{itemID:[0-9]+}/details/{detail}": "^/items/([^/]+)/details/([^/]+)", } for k, v := range tests { - testPathRegexp(t, k, v) + pathRegexp(t, k, v) } } func TestGetPathRegexpWithRegexCompile(t *testing.T) { - pattern := testPathRegexp(t, "/api/v1/users/{userId}/roles/{roleId}", "") + pattern := pathRegexp(t, "/api/v1/users/{userId}/roles/{roleId}", "^/api/v1/users/([^/]+)/roles/([^/]+)") matched, err := regexp.MatchString(pattern, "/api/v1/users/10512/roles/32587") assert.NoError(t, err) @@ -62,3 +61,102 @@ func TestStripListenPath(t *testing.T) { assert.Equal(t, "/get", httputil.StripListenPath("/{myPattern:foo|bar}", "/foo/get")) assert.Equal(t, "/anything/get", httputil.StripListenPath("/{myPattern:foo|bar}", "/anything/get")) } + +func TestMatchPaths(t *testing.T) { + tests := []struct { + name string + pattern string + endpoint string + match bool + isErr bool + }{ + { + name: "exact endpoints", + pattern: "/api/v1/users", + endpoint: "/api/v1/users", + match: true, + isErr: false, + }, + { + name: "non matching concrete endpoints", + pattern: "/api/v1/users", + endpoint: "/api/v1/admin", + match: false, + isErr: false, + }, + { + name: "regexp match", + pattern: "/api/v1/user/\\d+", + endpoint: "/api/v1/user/123", + match: true, + isErr: false, + }, + { + name: "regexp non match", + pattern: "/api/v1/user/\\d+", + endpoint: "/api/v1/user/abc", + match: false, + isErr: false, + }, + { + name: "mux var match", + pattern: "/api/v1/user/{id}", + endpoint: "/api/v1/user/123", + match: true, + isErr: false, + }, + { + name: "invalid config regexp", + pattern: "/api/v1/[user", + endpoint: "/api/v1/user", + match: false, + isErr: true, + }, + { + name: "wildcard endpoint", + pattern: "/api/v1/*", + endpoint: "/api/v1/users", + match: true, + isErr: false, + }, + { + name: "empty config endpoint", + pattern: "", + endpoint: "/api/v1/user", + match: false, + isErr: false, + }, + { + name: "empty request endpoint", + pattern: "/api/v1/user", + endpoint: "", + match: false, + isErr: false, + }, + { + name: "both empty endpoints", + pattern: "", + endpoint: "", + match: false, + isErr: false, + }, + { + name: "/ endpoint", + pattern: "/", + endpoint: "/", + match: true, + isErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // explicit match inputs as `^/path$` + pattern := httputil.PreparePathRegexp(tt.pattern, true, true) + + result, err := httputil.MatchPaths(pattern, []string{tt.endpoint}) + assert.Equal(t, tt.match, result) + assert.Equal(t, tt.isErr, err != nil) + }) + } +} diff --git a/tests/regression/Taskfile.yml b/tests/regression/Taskfile.yml index 806c2fbb533..e70f85467a4 100644 --- a/tests/regression/Taskfile.yml +++ b/tests/regression/Taskfile.yml @@ -45,7 +45,7 @@ tasks: quiet: true vars: funcs: - sh: ./regression.test -test.list=Test | egrep -v 'Test_Issue[0-9]+' || true + sh: ./regression.test -test.list=Test | egrep -v '(Test_Issue[0-9]+|TestLoadAPISpec)' || true cmds: - cmd: '{{if ne .funcs ""}}echo -e "Naming violations:\n{{.funcs}}" && exit 1 {{end}}' diff --git a/tests/regression/issue_10104_test.go b/tests/regression/issue_10104_test.go index 610b7fd184b..adb98238a93 100644 --- a/tests/regression/issue_10104_test.go +++ b/tests/regression/issue_10104_test.go @@ -13,7 +13,7 @@ func Test_Issue10104(t *testing.T) { defer ts.Close() // load api definition from file - ts.Gw.LoadAPI(loadAPISpec(t, "testdata/issue-10104-apidef.json")) + ts.Gw.LoadAPI(LoadAPISpec(t, "testdata/issue-10104-apidef.json")) // issue request against /test to trigger panic ts.Run(t, []test.TestCase{ diff --git a/tests/regression/issue_11806_test.go b/tests/regression/issue_11806_test.go index 83a23332adb..aec3168517c 100644 --- a/tests/regression/issue_11806_test.go +++ b/tests/regression/issue_11806_test.go @@ -20,8 +20,8 @@ func Test_Issue11806_DomainRouting(t *testing.T) { conf.EnableCustomDomains = true } - noDomain := loadAPISpec(t, "testdata/issue-11806-api-no-domain.json") - withDomain := loadAPISpec(t, "testdata/issue-11806-api-with-domain.json") + noDomain := LoadAPISpec(t, "testdata/issue-11806-api-no-domain.json") + withDomain := LoadAPISpec(t, "testdata/issue-11806-api-with-domain.json") t.Run("Load listenPath without domain first", func(t *testing.T) { ts := gateway.StartTest(testConfig) diff --git a/tests/regression/issue_12865_test.go b/tests/regression/issue_12865_test.go new file mode 100644 index 00000000000..b373b6dc01f --- /dev/null +++ b/tests/regression/issue_12865_test.go @@ -0,0 +1,171 @@ +package regression + +import ( + "net/http" + "testing" + + "github.com/TykTechnologies/tyk/config" + "github.com/TykTechnologies/tyk/gateway" + "github.com/TykTechnologies/tyk/header" + "github.com/TykTechnologies/tyk/test" + "github.com/TykTechnologies/tyk/user" +) + +func Test_Issue12865(t *testing.T) { + t.Run("Wildcard", func(t *testing.T) { + ts := gateway.StartTest(func(c *config.Config) { + c.HttpServerOptions.EnablePathPrefixMatching = false + c.HttpServerOptions.EnablePathSuffixMatching = false + }) + t.Cleanup(ts.Close) + + // load api definition from file + api := LoadAPISpec(t, "testdata/issue-12865.json") + + ts.Gw.LoadAPI(api) + + _, directKey := ts.CreateSession(func(s *user.SessionState) { + s.AccessRights = map[string]user.AccessDefinition{ + api.APIID: { + APIID: api.APIID, + APIName: api.Name, + Limit: user.APILimit{ + QuotaMax: 30, + }, + }, + } + }) + + headers := map[string]string{ + header.Authorization: directKey, + } + + // issue request against /test to trigger panic + ts.Run(t, []test.TestCase{ + {Path: "/test/anything", Method: http.MethodGet, Code: http.StatusOK}, + {Path: "/test/anything/health", Method: http.MethodGet, Code: http.StatusOK}, + {Path: "/test/anything/status/200", Method: http.MethodGet, Code: http.StatusOK}, + {Path: "/test/status/200", Method: http.MethodGet, Code: http.StatusUnauthorized}, + {Headers: headers, Path: "/test/status/200", Method: http.MethodGet, Code: http.StatusOK}, + {Path: "/test/status/200/anything", Method: http.MethodGet, Code: http.StatusOK}, + }...) + }) + + t.Run("Prefix", func(t *testing.T) { + ts := gateway.StartTest(func(c *config.Config) { + c.HttpServerOptions.EnablePathPrefixMatching = true + c.HttpServerOptions.EnablePathSuffixMatching = false + }) + t.Cleanup(ts.Close) + + // load api definition from file + api := LoadAPISpec(t, "testdata/issue-12865.json") + + ts.Gw.LoadAPI(api) + + _, directKey := ts.CreateSession(func(s *user.SessionState) { + s.AccessRights = map[string]user.AccessDefinition{ + api.APIID: { + APIID: api.APIID, + APIName: api.Name, + Limit: user.APILimit{ + QuotaMax: 30, + }, + }, + } + }) + + headers := map[string]string{ + header.Authorization: directKey, + } + + // issue request against /test to trigger panic + ts.Run(t, []test.TestCase{ + {Path: "/test/anything", Method: http.MethodGet, Code: http.StatusOK}, + {Path: "/test/anything/health", Method: http.MethodGet, Code: http.StatusOK}, + {Path: "/test/anything/status/200", Method: http.MethodGet, Code: http.StatusOK}, + {Path: "/test/status/200", Method: http.MethodGet, Code: http.StatusUnauthorized}, + {Headers: headers, Path: "/test/status/200", Method: http.MethodGet, Code: http.StatusOK}, + {Path: "/test/status/200/anything", Method: http.MethodGet, Code: http.StatusUnauthorized}, + }...) + }) + + t.Run("Prefix and Suffix", func(t *testing.T) { + ts := gateway.StartTest(func(c *config.Config) { + c.HttpServerOptions.EnablePathPrefixMatching = true + c.HttpServerOptions.EnablePathSuffixMatching = true + }) + t.Cleanup(ts.Close) + + // load api definition from file + api := LoadAPISpec(t, "testdata/issue-12865.json") + + ts.Gw.LoadAPI(api) + + _, directKey := ts.CreateSession(func(s *user.SessionState) { + s.AccessRights = map[string]user.AccessDefinition{ + api.APIID: { + APIID: api.APIID, + APIName: api.Name, + Limit: user.APILimit{ + QuotaMax: 30, + }, + }, + } + }) + + headers := map[string]string{ + header.Authorization: directKey, + } + + // issue request against /test to trigger panic + ts.Run(t, []test.TestCase{ + {Path: "/test/anything", Method: http.MethodGet, Code: http.StatusOK}, + {Path: "/test/anything/health", Method: http.MethodGet, Code: http.StatusUnauthorized}, + {Path: "/test/anything/status/200", Method: http.MethodGet, Code: http.StatusUnauthorized}, + {Path: "/test/status/200", Method: http.MethodGet, Code: http.StatusUnauthorized}, + {Headers: headers, Path: "/test/status/200", Method: http.MethodGet, Code: http.StatusOK}, + {Path: "/test/status/200/anything", Method: http.MethodGet, Code: http.StatusUnauthorized}, + }...) + }) + + t.Run("Suffix", func(t *testing.T) { + ts := gateway.StartTest(func(c *config.Config) { + c.HttpServerOptions.EnablePathPrefixMatching = false + c.HttpServerOptions.EnablePathSuffixMatching = true + }) + t.Cleanup(ts.Close) + + // load api definition from file + api := LoadAPISpec(t, "testdata/issue-12865.json") + + ts.Gw.LoadAPI(api) + + _, directKey := ts.CreateSession(func(s *user.SessionState) { + s.AccessRights = map[string]user.AccessDefinition{ + api.APIID: { + APIID: api.APIID, + APIName: api.Name, + Limit: user.APILimit{ + QuotaMax: 30, + }, + }, + } + }) + + headers := map[string]string{ + header.Authorization: directKey, + } + + // issue request against /test to trigger panic + ts.Run(t, []test.TestCase{ + {Path: "/test/anything", Method: http.MethodGet, Code: http.StatusOK}, + {Path: "/test/anything/health", Method: http.MethodGet, Code: http.StatusUnauthorized}, + {Path: "/test/anything/status/200", Method: http.MethodGet, Code: http.StatusUnauthorized}, + {Path: "/test/status/200", Method: http.MethodGet, Code: http.StatusUnauthorized}, + {Headers: headers, Path: "/test/status/200", Method: http.MethodGet, Code: http.StatusOK}, + {Path: "/test/status/200/anything", Method: http.MethodGet, Code: http.StatusNotFound}, + }...) + }) + +} diff --git a/tests/regression/regression_test.go b/tests/regression/regression_test.go index df100c4f6fc..9f0c632c9e2 100644 --- a/tests/regression/regression_test.go +++ b/tests/regression/regression_test.go @@ -1,10 +1,13 @@ package regression import ( + "bytes" "embed" "encoding/json" + "os" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/TykTechnologies/tyk/apidef" @@ -16,10 +19,10 @@ var ( testdata embed.FS ) -func loadAPISpec(tb testing.TB, filename string) *gateway.APISpec { +func LoadAPISpec(tb testing.TB, filename string) *gateway.APISpec { tb.Helper() - data := loadFile(tb, filename) + data := LoadFile(tb, filename) apidef := &apidef.APIDefinition{} err := json.Unmarshal(data, apidef) @@ -30,10 +33,27 @@ func loadAPISpec(tb testing.TB, filename string) *gateway.APISpec { } } -func loadFile(tb testing.TB, filename string) []byte { +func TestLoadAPISpec(t *testing.T) { + f := LoadAPISpec(t, "testdata/issue-10104-apidef.json") + + assert.Equal(t, f.APIDefinition.Proxy.TargetURL, "http://127.0.0.1:3123/") +} + +func LoadFile(tb testing.TB, filename string) []byte { tb.Helper() data, err := testdata.ReadFile(filename) + + httpbin := os.Getenv("HTTPBIN_IMAGE") + if httpbin == "" { + httpbin = "127.0.0.1:3123" + } + replacement := []byte("//" + httpbin) + + // auto replace from public to private endpoint, part of CI env + data = bytes.ReplaceAll(data, []byte("//httpbin.org"), replacement) + data = bytes.ReplaceAll(data, []byte("//google.com"), replacement) + require.NoError(tb, err, "Error reading file: %s", filename) return data diff --git a/tests/regression/testdata/issue-12865.json b/tests/regression/testdata/issue-12865.json new file mode 100644 index 00000000000..81fb2d829f2 --- /dev/null +++ b/tests/regression/testdata/issue-12865.json @@ -0,0 +1,501 @@ +{ + "id": "66c5cebd5b600879903f1ad3", + "name": "test", + "slug": "temp", + "listen_port": 0, + "protocol": "", + "enable_proxy_protocol": false, + "api_id": "c88e2d8ed982400469a2d1738f055902", + "org_id": "645b3db586341f751f4258aa", + "use_keyless": false, + "use_oauth2": false, + "external_oauth": { + "enabled": false, + "providers": [] + }, + "use_openid": false, + "openid_options": { + "providers": [], + "segregate_by_client": false + }, + "oauth_meta": { + "allowed_access_types": [], + "allowed_authorize_types": [], + "auth_login_redirect": "" + }, + "auth": { + "name": "", + "use_param": false, + "param_name": "", + "use_cookie": false, + "cookie_name": "", + "disable_header": false, + "auth_header_name": "Authorization", + "use_certificate": false, + "validate_signature": false, + "signature": { + "algorithm": "", + "header": "", + "use_param": false, + "param_name": "", + "secret": "", + "allowed_clock_skew": 0, + "error_code": 0, + "error_message": "" + } + }, + "auth_configs": { + "authToken": { + "name": "", + "use_param": false, + "param_name": "", + "use_cookie": false, + "cookie_name": "", + "disable_header": false, + "auth_header_name": "Authorization", + "use_certificate": false, + "validate_signature": false, + "signature": { + "algorithm": "", + "header": "", + "use_param": false, + "param_name": "", + "secret": "", + "allowed_clock_skew": 0, + "error_code": 0, + "error_message": "" + } + }, + "basic": { + "name": "", + "use_param": false, + "param_name": "", + "use_cookie": false, + "cookie_name": "", + "disable_header": false, + "auth_header_name": "Authorization", + "use_certificate": false, + "validate_signature": false, + "signature": { + "algorithm": "", + "header": "", + "use_param": false, + "param_name": "", + "secret": "", + "allowed_clock_skew": 0, + "error_code": 0, + "error_message": "" + } + }, + "coprocess": { + "name": "", + "use_param": false, + "param_name": "", + "use_cookie": false, + "cookie_name": "", + "disable_header": false, + "auth_header_name": "Authorization", + "use_certificate": false, + "validate_signature": false, + "signature": { + "algorithm": "", + "header": "", + "use_param": false, + "param_name": "", + "secret": "", + "allowed_clock_skew": 0, + "error_code": 0, + "error_message": "" + } + }, + "hmac": { + "name": "", + "use_param": false, + "param_name": "", + "use_cookie": false, + "cookie_name": "", + "disable_header": false, + "auth_header_name": "Authorization", + "use_certificate": false, + "validate_signature": false, + "signature": { + "algorithm": "", + "header": "", + "use_param": false, + "param_name": "", + "secret": "", + "allowed_clock_skew": 0, + "error_code": 0, + "error_message": "" + } + }, + "jwt": { + "name": "", + "use_param": false, + "param_name": "", + "use_cookie": false, + "cookie_name": "", + "disable_header": false, + "auth_header_name": "Authorization", + "use_certificate": false, + "validate_signature": false, + "signature": { + "algorithm": "", + "header": "", + "use_param": false, + "param_name": "", + "secret": "", + "allowed_clock_skew": 0, + "error_code": 0, + "error_message": "" + } + }, + "oauth": { + "name": "", + "use_param": false, + "param_name": "", + "use_cookie": false, + "cookie_name": "", + "disable_header": false, + "auth_header_name": "Authorization", + "use_certificate": false, + "validate_signature": false, + "signature": { + "algorithm": "", + "header": "", + "use_param": false, + "param_name": "", + "secret": "", + "allowed_clock_skew": 0, + "error_code": 0, + "error_message": "" + } + }, + "oidc": { + "name": "", + "use_param": false, + "param_name": "", + "use_cookie": false, + "cookie_name": "", + "disable_header": false, + "auth_header_name": "Authorization", + "use_certificate": false, + "validate_signature": false, + "signature": { + "algorithm": "", + "header": "", + "use_param": false, + "param_name": "", + "secret": "", + "allowed_clock_skew": 0, + "error_code": 0, + "error_message": "" + } + } + }, + "use_basic_auth": false, + "basic_auth": { + "disable_caching": false, + "cache_ttl": 0, + "extract_from_body": false, + "body_user_regexp": "", + "body_password_regexp": "" + }, + "use_mutual_tls_auth": false, + "client_certificates": [], + "upstream_certificates": {}, + "pinned_public_keys": {}, + "enable_jwt": false, + "use_standard_auth": true, + "use_go_plugin_auth": false, + "enable_coprocess_auth": false, + "custom_plugin_auth_enabled": false, + "jwt_signing_method": "", + "jwt_source": "", + "jwt_identity_base_field": "", + "jwt_client_base_field": "", + "jwt_policy_field_name": "", + "jwt_default_policies": [], + "jwt_issued_at_validation_skew": 0, + "jwt_expires_at_validation_skew": 0, + "jwt_not_before_validation_skew": 0, + "jwt_skip_kid": false, + "scopes": { + "jwt": {}, + "oidc": {} + }, + "idp_client_id_mapping_disabled": false, + "jwt_scope_to_policy_mapping": {}, + "jwt_scope_claim_name": "", + "notifications": { + "shared_secret": "", + "oauth_on_keychange_url": "" + }, + "enable_signature_checking": false, + "hmac_allowed_clock_skew": -1, + "hmac_allowed_algorithms": [], + "request_signing": { + "is_enabled": false, + "secret": "", + "key_id": "", + "algorithm": "", + "header_list": [], + "certificate_id": "", + "signature_header": "" + }, + "base_identity_provided_by": "", + "definition": { + "enabled": false, + "name": "", + "default": "", + "location": "header", + "key": "x-api-version", + "strip_path": false, + "strip_versioning_data": false, + "url_versioning_pattern": "", + "fallback_to_default": false, + "versions": {} + }, + "version_data": { + "not_versioned": true, + "default_version": "", + "versions": { + "Default": { + "name": "Default", + "expires": "", + "paths": { + "ignored": [], + "white_list": [], + "black_list": [] + }, + "use_extended_paths": true, + "extended_paths": { + "ignored": [ + { + "disabled": false, + "path": "/anything", + "method": "", + "ignore_case": false, + "method_actions": { + "GET": { + "action": "no_action", + "code": 200, + "data": "", + "headers": {} + } + } + } + ], + "url_rewrites": [ + { + "disabled": false, + "path": "/status", + "method": "GET", + "match_pattern": "/status/(.*)", + "rewrite_to": "http://httpbin.org", + "triggers": [] + } + ], + "persist_graphql": [], + "rate_limit": [] + }, + "global_headers": {}, + "global_headers_remove": [], + "global_headers_disabled": false, + "global_response_headers": {}, + "global_response_headers_remove": [], + "global_response_headers_disabled": false, + "ignore_endpoint_case": false, + "global_size_limit": 0, + "override_target": "" + } + } + }, + "uptime_tests": { + "check_list": [], + "config": { + "expire_utime_after": 0, + "service_discovery": { + "use_discovery_service": false, + "query_endpoint": "", + "use_nested_query": false, + "parent_data_path": "", + "data_path": "", + "port_data_path": "", + "target_path": "", + "use_target_list": false, + "cache_disabled": false, + "cache_timeout": 60, + "endpoint_returns_list": false + }, + "recheck_wait": 0 + } + }, + "proxy": { + "preserve_host_header": false, + "listen_path": "/test/", + "target_url": "http://httpbin.org/", + "disable_strip_slash": true, + "strip_listen_path": true, + "enable_load_balancing": false, + "target_list": [], + "check_host_against_uptime_tests": false, + "service_discovery": { + "use_discovery_service": false, + "query_endpoint": "", + "use_nested_query": false, + "parent_data_path": "", + "data_path": "", + "port_data_path": "", + "target_path": "", + "use_target_list": false, + "cache_disabled": false, + "cache_timeout": 0, + "endpoint_returns_list": false + }, + "transport": { + "ssl_insecure_skip_verify": false, + "ssl_ciphers": [], + "ssl_min_version": 0, + "ssl_max_version": 0, + "ssl_force_common_name_check": false, + "proxy_url": "" + } + }, + "disable_rate_limit": false, + "disable_quota": false, + "custom_middleware": { + "pre": [], + "post": [], + "post_key_auth": [], + "auth_check": { + "disabled": false, + "name": "", + "path": "", + "require_session": false, + "raw_body_only": false + }, + "response": [], + "driver": "", + "id_extractor": { + "disabled": false, + "extract_from": "", + "extract_with": "", + "extractor_config": {} + } + }, + "custom_middleware_bundle": "", + "custom_middleware_bundle_disabled": false, + "cache_options": { + "cache_timeout": 60, + "enable_cache": true, + "cache_all_safe_requests": false, + "cache_response_codes": [], + "enable_upstream_cache_control": false, + "cache_control_ttl_header": "", + "cache_by_headers": [] + }, + "session_lifetime": 0, + "active": true, + "internal": false, + "auth_provider": { + "name": "", + "storage_engine": "", + "meta": {} + }, + "session_provider": { + "name": "", + "storage_engine": "", + "meta": {} + }, + "event_handlers": { + "events": {} + }, + "enable_batch_request_support": false, + "enable_ip_whitelisting": false, + "allowed_ips": [], + "enable_ip_blacklisting": false, + "blacklisted_ips": [], + "dont_set_quota_on_create": false, + "expire_analytics_after": 0, + "response_processors": [], + "CORS": { + "enable": false, + "allowed_origins": [ + "*" + ], + "allowed_methods": [ + "GET", + "POST", + "HEAD" + ], + "allowed_headers": [ + "Origin", + "Accept", + "Content-Type", + "X-Requested-With", + "Authorization" + ], + "exposed_headers": [], + "allow_credentials": false, + "max_age": 24, + "options_passthrough": false, + "debug": false + }, + "domain": "", + "certificates": [], + "do_not_track": false, + "enable_context_vars": false, + "config_data": {}, + "config_data_disabled": false, + "tag_headers": [], + "global_rate_limit": { + "disabled": false, + "rate": 0, + "per": 0 + }, + "strip_auth_data": false, + "enable_detailed_recording": false, + "graphql": { + "enabled": false, + "execution_mode": "proxyOnly", + "version": "2", + "schema": "", + "type_field_configurations": [], + "playground": { + "enabled": false, + "path": "" + }, + "engine": { + "field_configs": [], + "data_sources": [], + "global_headers": [] + }, + "proxy": { + "features": { + "use_immutable_headers": true + }, + "auth_headers": {}, + "request_headers": {}, + "use_response_extensions": { + "on_error_forwarding": false + }, + "request_headers_rewrite": {} + }, + "subgraph": { + "sdl": "" + }, + "supergraph": { + "subgraphs": [], + "merged_sdl": "", + "global_headers": {}, + "disable_query_batching": false + }, + "introspection": { + "disabled": false + } + }, + "analytics_plugin": {}, + "tags": [], + "detailed_tracing": false +} \ No newline at end of file