Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions apidef/oas/servers_regeneration.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,29 @@
return nil
}

// GenerateTykServers generates and returns only Tyk-managed server URLs for an API.
// This is a convenience method that generates servers without modifying the OAS spec.
// Unlike RegenerateServers, this does not include user-defined servers and does not
// modify the OAS servers array.
func (s *OAS) GenerateTykServers(
apiData *apidef.APIDefinition,
baseAPI *apidef.APIDefinition,
config ServerRegenerationConfig,
versionName string,
) []*openapi3.Server {
serverInfos := generateTykServers(apiData, baseAPI, config, versionName)

servers := make([]*openapi3.Server, len(serverInfos))
for i, info := range serverInfos {
servers[i] = &openapi3.Server{
URL: info.url,
Description: info.description,
}
}

return servers
}

Check notice on line 119 in apidef/oas/servers_regeneration.go

View check run for this annotation

probelabs / Visor: architecture

architecture Issue

The `GenerateTykServers` function is implemented as a method on the `*OAS` struct but does not use the receiver `s`. This indicates it is a stateless, pure function. Defining it as a method can be misleading, as it implies an interaction with the `OAS` instance's state, which is not the case.
Raw output
To improve clarity and reflect its stateless nature, consider refactoring `GenerateTykServers` into a package-level function. This change would make its contract more explicit—it takes inputs and produces an output without side effects on an `OAS` object. This also simplifies its usage, as callers would no longer need to create a dummy `OAS` instance to invoke it.
// generateTykServers generates all Tyk-managed server URLs for an API.
func generateTykServers(
apiData *apidef.APIDefinition,
Expand Down Expand Up @@ -259,51 +282,51 @@
}

// buildServerURL constructs a server URL from protocol, host, and path.
func buildServerURL(protocol, host, listenPath string) string {
if !strings.HasPrefix(listenPath, "/") {
listenPath = "/" + listenPath
}

host = strings.TrimSuffix(host, "/")

// Clean the path to remove double slashes
listenPath = path.Clean(listenPath)

// Check if host already has a protocol (e.g., from edge endpoints)
if strings.HasPrefix(host, "http://") || strings.HasPrefix(host, "https://") {
return host + listenPath
}

return protocol + host + listenPath
}

// buildVersionedServerURL constructs a server URL for a versioned API
// based on the versioning method (URL path, query param, or header).
func buildVersionedServerURL(
protocol, host, listenPath string,
versionLocation, versionKey, versionName string,
) (string, string) {
baseURL := buildServerURL(protocol, host, listenPath)
var description string

switch versionLocation {
case "url":
fallthrough
default:
if !strings.HasSuffix(baseURL, "/") {
baseURL += "/"
}
baseURL += versionName

case "url-param":
baseURL += "?" + versionKey + "=" + versionName

case "header":
return baseURL, description
}

return baseURL, description
}

Check warning on line 329 in apidef/oas/servers_regeneration.go

View check run for this annotation

probelabs / Visor: security

security Issue

User-controllable parts of the URL path (`listenPath`, `versionName`) are not sanitized for path traversal elements (`..`). The use of `path.Clean` on `listenPath` in `buildServerURL` and direct concatenation of `versionName` in `buildVersionedServerURL` can lead to the generation of misleading URLs in the OpenAPI specification. A user with permissions to define an API could craft these values to make a server URL appear to be on a different path, for example `https://api.com/api/../admin` which resolves to `https://api.com/admin`.
Raw output
Sanitize user-provided URL path segments to prevent path traversal. Replace `path.Clean` with a function that only normalizes slashes. Before concatenating path segments like `versionName`, ensure they do not contain `..` sequences, for example by checking for and rejecting them, or by removing them. Using `net/url.URL.ResolveReference` is a more robust way to construct URLs.

// removeTykGeneratedURLs removes Tyk-generated server URLs from the servers list.
// It uses URL normalization for robust matching and preserves all other servers.
Expand Down
161 changes: 161 additions & 0 deletions apidef/oas/servers_regeneration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,167 @@
})
}

func TestOAS_GenerateTykServers(t *testing.T) {
t.Parallel()

t.Run("standard non-versioned API", func(t *testing.T) {
t.Parallel()

oas := &OAS{}
apiDef := &apidef.APIDefinition{
APIID: "test-api",
Proxy: apidef.ProxyConfig{
ListenPath: "/test",
},
}

config := ServerRegenerationConfig{
Protocol: "http://",
DefaultHost: "localhost:8080",
}

servers := oas.GenerateTykServers(apiDef, nil, config, "")

require.Len(t, servers, 1)
assert.Equal(t, "http://localhost:8080/test", servers[0].URL)
})

t.Run("versioned child API with URL path versioning", func(t *testing.T) {
t.Parallel()

oas := &OAS{}
baseAPI := &apidef.APIDefinition{
APIID: "base-id",
Proxy: apidef.ProxyConfig{
ListenPath: "/products",
},
VersionDefinition: apidef.VersionDefinition{
Enabled: true,
Location: "url",
Key: "version",
Versions: map[string]string{
"v1": "base-id",
"v2": "child-id",
},
},
}

childAPI := &apidef.APIDefinition{
APIID: "child-id",
Proxy: apidef.ProxyConfig{
ListenPath: "/products-v2",
},
VersionData: apidef.VersionData{
NotVersioned: false,
Versions: map[string]apidef.VersionInfo{
"v2": {},
},
},
}

config := ServerRegenerationConfig{
Protocol: "https://",
DefaultHost: "api.example.com",
}

servers := oas.GenerateTykServers(childAPI, baseAPI, config, "v2")

// Should have versioned URL
require.NotEmpty(t, servers)

Check warning on line 826 in apidef/oas/servers_regeneration_test.go

View check run for this annotation

probelabs / Visor: quality

style Issue

The assertion logic to find a specific URL within the generated servers is verbose. Using a loop with a boolean flag can be simplified for better readability and conciseness. A more idiomatic approach would be to collect the URLs into a slice and use `assert.Contains`.
Raw output
Refactor the URL check to be more idiomatic. Collect all server URLs into a slice and then use `assert.Contains` to check for the presence of the expected URL. This makes the test's intent clearer and reduces boilerplate code.

```go
	// Should have versioned URL
	require.NotEmpty(t, servers)

	urls := make([]string, len(servers))
	for i, server := range servers {
		urls[i] = server.URL
	}

	assert.Contains(t, urls, "https://api.example.com/products/v2", "Expected to find versioned URL https://api.example.com/products/v2")
```
found := false
for _, server := range servers {
if server.URL == "https://api.example.com/products/v2" {
found = true
break
}
}
assert.True(t, found, "Expected to find versioned URL https://api.example.com/products/v2")
})

t.Run("API with custom domain", func(t *testing.T) {
t.Parallel()

oas := &OAS{}
apiDef := &apidef.APIDefinition{
APIID: "test-api",
Domain: "custom.example.com",
Proxy: apidef.ProxyConfig{
ListenPath: "/api",
},
}

config := ServerRegenerationConfig{
Protocol: "https://",
DefaultHost: "localhost:8080",
}

servers := oas.GenerateTykServers(apiDef, nil, config, "")

require.Len(t, servers, 1)
assert.Equal(t, "https://custom.example.com/api", servers[0].URL)
})

t.Run("base API with versioning and fallback", func(t *testing.T) {
t.Parallel()

oas := &OAS{}
baseAPI := &apidef.APIDefinition{
APIID: "base-id",
Proxy: apidef.ProxyConfig{
ListenPath: "/products",
},
VersionDefinition: apidef.VersionDefinition{
Enabled: true,
Name: "v1",
Location: "url",
Default: "v1",
FallbackToDefault: true,
Versions: map[string]string{
"v1": "base-id",
},
},
}

config := ServerRegenerationConfig{
Protocol: "http://",
DefaultHost: "localhost:8080",
}

servers := oas.GenerateTykServers(baseAPI, nil, config, "")

// Should have both versioned URL and fallback URL
require.Len(t, servers, 2)
urls := []string{servers[0].URL, servers[1].URL}
assert.Contains(t, urls, "http://localhost:8080/products/v1")
assert.Contains(t, urls, "http://localhost:8080/products")
})

t.Run("returns openapi3.Server type not serverInfo", func(t *testing.T) {
t.Parallel()

oas := &OAS{}
apiDef := &apidef.APIDefinition{
APIID: "test-api",
Proxy: apidef.ProxyConfig{
ListenPath: "/test",
},
}

config := ServerRegenerationConfig{
Protocol: "http://",
DefaultHost: "localhost:8080",
}

servers := oas.GenerateTykServers(apiDef, nil, config, "")

// Verify it returns the correct type
require.NotNil(t, servers)
require.IsType(t, []*openapi3.Server{}, servers)
require.Len(t, servers, 1)
require.IsType(t, &openapi3.Server{}, servers[0])
})
}

func TestShouldUpdateChildAPIs(t *testing.T) {
t.Parallel()

Expand Down
Loading