Skip to content

[TT-16013] Gateway panics when validating JWT claims #7530

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
NurayAhmadova wants to merge 13 commits into from
Closed

[TT-16013] Gateway panics when validating JWT claims #7530

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
c58fdc3
[TT-16013] fix bug gateway panics when validating jwt claims
NurayAhmadova Nov 13, 2025
fea175e
[TT-16013] fix bug gateway panics when validating jwt claims - visor …
NurayAhmadova Nov 14, 2025
cb26375
TT-16013] fix bug gateway panics when validating jwt claims - fix tests
NurayAhmadova Nov 14, 2025
97b8fc3
[TT-16013] fix bug gateway panics when validating jwt claims - fix te…
NurayAhmadova Nov 17, 2025
6bdefd7
[TT-16013] fix bug gateway panics when validating jwt claims - fix vi…
NurayAhmadova Nov 17, 2025
1809dd6
[TT-16013] fix bug gateway panics when validating jwt claims - fix vi…
NurayAhmadova Nov 18, 2025
0f18da2
[TT-16013] fix bug gateway panics when validating jwt claims - fix vi…
NurayAhmadova Nov 19, 2025
0b4a04c
wip
NurayAhmadova Nov 24, 2025
f14f4e5
[TT-16013] fix bug gateway panics when validating jwt claims - wip
NurayAhmadova Nov 25, 2025
c8b0efe
some additional methods
shults Nov 25, 2025
c042d35
wip
NurayAhmadova Nov 26, 2025
296dd3f
some test fixes
shults Nov 26, 2025
65bd416
try to fix it
shults Nov 26, 2025
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
327 changes: 272 additions & 55 deletions apidef/oas/authentication.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,18 @@

import (
"encoding/json"
"errors"
"fmt"
"iter"
"reflect"
"sort"
"time"

"github.com/TykTechnologies/tyk/apidef"
"github.com/getkin/kin-openapi/openapi3"
clone "github.com/huandu/go-clone/generic"
"github.com/mitchellh/mapstructure"

"github.com/TykTechnologies/tyk/apidef"
"gopkg.in/yaml.v3"
)

// SecurityProcessingMode constants define how multiple security requirements are processed
Expand All @@ -26,80 +29,80 @@
func ValidateSecurityProcessingMode(mode string) bool {
return mode == "" || mode == SecurityProcessingModeLegacy || mode == SecurityProcessingModeCompliant
}

// GetDefaultSecurityProcessingMode returns the default security processing mode.
func GetDefaultSecurityProcessingMode() string {
return SecurityProcessingModeLegacy
}

// Authentication contains configuration about the authentication methods and security policies applied to requests.
type Authentication struct {
// Enabled makes the API protected when one of the authentication modes is enabled.
//
// Tyk classic API definition: `!use_keyless`.
Enabled bool `bson:"enabled" json:"enabled"` // required

// StripAuthorizationData ensures that any security tokens used for accessing APIs are stripped and not passed to the upstream.
//
// Tyk classic API definition: `strip_auth_data`.
StripAuthorizationData bool `bson:"stripAuthorizationData,omitempty" json:"stripAuthorizationData,omitempty"`

// BaseIdentityProvider enables the use of multiple authentication mechanisms.
// It provides the session object that determines access control, rate limits and usage quotas.

Check failure on line 51 in apidef/oas/authentication.go

View check run for this annotation

probelabs / Visor: architecture

architecture Issue 

The refactoring of `SecuritySchemes` into a struct fails to address the original race condition. The new struct lacks any synchronization primitives (like a `sync.RWMutex`), and operations on it are not atomic. Concurrent access to a shared `OAS` object will still lead to race conditions like lost updates, and potentially the original `concurrent map writes` panic if the internal map is accessed concurrently. The PR's primary goal of fixing the concurrency issue is not met.
Raw output 
To ensure thread safety, add a `sync.RWMutex` to the `SecuritySchemes` struct. Protect all reads and writes to the internal `container` map with the mutex. This typically involves changing methods to use pointer receivers (`func (ss *SecuritySchemes) ...`) and modifying the map in-place under lock protection, which is a more idiomatic and robust solution for concurrent map access in Go.
//
// It should be set to one of the following:
//
// - `auth_token`
// - `hmac_key`
// - `basic_auth_user`
// - `jwt_claim`
// - `oidc_user`
// - `oauth_key`
// - `custom_auth`
//
// Tyk classic API definition: `base_identity_provided_by`.
BaseIdentityProvider apidef.AuthTypeEnum `bson:"baseIdentityProvider,omitempty" json:"baseIdentityProvider,omitempty"`

// HMAC contains the configurations related to HMAC authentication mode.
//
// Tyk classic API definition: `auth_configs["hmac"]`
HMAC *HMAC `bson:"hmac,omitempty" json:"hmac,omitempty"`

// OIDC contains the configurations related to OIDC authentication mode.
//
// Tyk classic API definition: `auth_configs["oidc"]`
OIDC *OIDC `bson:"oidc,omitempty" json:"oidc,omitempty"`

// Custom contains the configurations related to Custom authentication mode.
//
// Tyk classic API definition: `auth_configs["coprocess"]`
Custom *CustomPluginAuthentication `bson:"custom,omitempty" json:"custom,omitempty"`

// SecuritySchemes contains security schemes definitions.
SecuritySchemes SecuritySchemes `bson:"securitySchemes,omitempty" json:"securitySchemes,omitempty"`

// CustomKeyLifetime contains configuration for the maximum retention period for access tokens.
CustomKeyLifetime *CustomKeyLifetime `bson:"customKeyLifetime,omitempty" json:"customKeyLifetime,omitempty"`

// SecurityProcessingMode controls how Tyk will process the OpenAPI `security` field if multiple security requirement objects are declared.
// - "legacy" (default): Only the first security requirement object will be processed; uses BaseIdentityProvider to create the session object.
// - "compliant": All security requirement objects will be processed, request will be authorized if any of these are validated; the origin for the session object will be determined dynamically based on the validated security requirement.
SecurityProcessingMode string `bson:"securityProcessingMode,omitempty" json:"securityProcessingMode,omitempty"`

// Security is an extension to the OpenAPI security field and is used when securityProcessingMode is set to "compliant".
// This can be used to combine any declared securitySchemes including Tyk proprietary auth methods.
Security [][]string `bson:"security,omitempty" json:"security,omitempty"`
}

// CustomKeyLifetime contains configuration for custom key retention.
type CustomKeyLifetime struct {
// Enabled enables custom maximum retention for keys for the API.
Enabled bool `bson:"enabled,omitempty" json:"enabled,omitempty"`
// Value configures the expiry interval for a Key.
// The value is a string that specifies the interval in a compact form,
// where hours, minutes and seconds are denoted by 'h', 'm' and 's' respectively.
// Multiple units can be combined to represent the duration.
//

Check warning on line 105 in apidef/oas/authentication.go

View check run for this annotation

probelabs / Visor: architecture

architecture Issue 

The chosen immutable value object pattern for `SecuritySchemes` introduces unnecessary complexity and has proven to be error-prone, as seen by the incorrect usage of the `delete` method. This pattern, which involves cloning the map on every mutation, also has potential performance overhead. A simpler, more idiomatic Go approach using a mutex-protected struct would be easier to understand, less susceptible to misuse, and would have correctly solved the underlying concurrency problem.
Raw output 
Re-evaluate the architectural decision to use an immutable data structure. Refactor `SecuritySchemes` to use a standard Go pattern for concurrent-safe maps: a struct containing the map and a `sync.RWMutex`. This would simplify the implementation and eliminate the need for callers to manage state by reassigning return values.
// Examples of valid shorthand notations:
// - "1h" : one hour
// - "20m" : twenty minutes
Expand Down Expand Up @@ -208,9 +211,9 @@

if a.OIDC != nil {
a.OIDC.ExtractTo(api)
}

if a.Custom == nil {

Check failure on line 216 in apidef/oas/authentication.go

View check run for this annotation

probelabs / Visor: quality

architecture Issue 

The refactoring of `SecuritySchemes` from a map to a struct does not solve the underlying concurrency issue. The new struct lacks synchronization primitives (e.g., a `sync.RWMutex`), making it unsafe for concurrent access. The original problem was a 'concurrent map writes' panic, and this implementation is still susceptible to race conditions when an object containing `SecuritySchemes` is modified by multiple goroutines.
Raw output 
To ensure thread safety, add a `sync.RWMutex` to the `SecuritySchemes` struct. Use a write lock (`mu.Lock()`) in methods that modify the internal map (like `Set` and `delete`) and a read lock (`mu.RLock()`) in methods that only read from it (like `Get`, `Len`, `Iter`). Consider using pointer receivers for mutating methods to modify the struct in-place, which is a more common and less error-prone pattern for stateful objects in Go than the immutable approach attempted here.

Check failure on line 216 in apidef/oas/authentication.go

View check run for this annotation

probelabs / Visor: quality

logic Issue 

The function `decodeScheme` is not implemented and returns a hardcoded error. This function is essential for `UnmarshalJSON` and `UnmarshalYAML` to work, as it's responsible for converting a raw map into a specific security scheme struct. As a result, any attempt to deserialize an API definition with security schemes from JSON or YAML will fail.
Raw output 
Implement the `decodeScheme` function. It should inspect the input `map[string]interface{}` to determine the type of security scheme (e.g., by checking keys like `type`, `scheme`, `bearerFormat`) and then use a library like `mapstructure` to decode the map into the corresponding struct pointer (e.g., `*JWT`, `*Token`).
a.Custom = &CustomPluginAuthentication{}
defer func() {
a.Custom = nil
Expand All @@ -221,84 +224,241 @@

if a.CustomKeyLifetime == nil {
a.CustomKeyLifetime = &CustomKeyLifetime{}
defer func() {
a.CustomKeyLifetime = nil
}()

Check failure on line 229 in apidef/oas/authentication.go

View check run for this annotation

probelabs / Visor: architecture

logic Issue 

The `decodeScheme` function, which is critical for deserializing security schemes from JSON or YAML, is not implemented and returns a hardcoded 'not implemented' error. This makes the new `SecuritySchemes` struct unusable for any configuration that needs to be loaded from a file or external source, as unmarshalling will always fail.
Raw output 
Implement the `decodeScheme` function. It should inspect the raw `interface{}` data to determine the concrete type of the security scheme (e.g., by checking for fields unique to JWT, Basic, Token, etc.) and then correctly unmarshal the data into the corresponding struct type.
}

Check failure on line 230 in apidef/oas/authentication.go

View check run for this annotation

probelabs / Visor: security

logic Issue 

The function `decodeScheme` is a stub that always returns a "not implemented" error. This function is called during JSON and YAML unmarshalling of `SecuritySchemes`. Consequently, any attempt to deserialize a configuration containing security schemes will fail. This is a denial-of-service vulnerability, as it will prevent the gateway from loading valid API definitions.
Raw output 
Implement the `decodeScheme` function to correctly identify the type of security scheme from the raw `interface{}` data (likely a `map[string]interface{}`) and decode it into the corresponding struct (e.g., `*Token`, `*JWT`, etc.). This will likely require inspecting fields to determine the scheme type, similar to the logic in the `importNative` function.

a.CustomKeyLifetime.ExtractTo(api)
}

// SecuritySchemes holds security scheme values, filled with Import().
type SecuritySchemes map[string]interface{}
// SecuritySchemes zero value is usable. Methods lazily initialize internal state.
// Recommendation: prefer NewSecuritySchemes for clarity and explicit initialization.
type SecuritySchemes struct {
container map[string]SecuritySchemeMarker
}

func (ss SecuritySchemes) Set(key string, value SecuritySchemeMarker) SecuritySchemes {
if reflect.TypeOf(value).Kind() != reflect.Pointer {
panic("value must be a pointer")
}

var res SecuritySchemes
res.container = clone.Clone(ss.container)
if res.container == nil {
res.container = map[string]SecuritySchemeMarker{}
}
res.container[key] = value

Check notice on line 251 in apidef/oas/authentication.go

View check run for this annotation

probelabs / Visor: performance

performance Issue 

The `Set` method performs a deep clone of the entire security schemes map on every call. When importing multiple security schemes in a loop (as done in `importAuthentication`), this results in repeatedly cloning a growing map, leading to quadratic complexity (O(n²)) in terms of elements copied. While the number of security schemes is typically small, this can become a performance bottleneck during API definition loading for APIs with many security schemes.
Raw output 
For bulk updates, such as in `importAuthentication`, consider using a builder pattern or creating a new map and setting it once at the end to avoid repeated cloning. Alternatively, if performance during API loading becomes a concern, replacing the immutability pattern with a `sync.RWMutex` to protect a mutable map would be more performant for writes.
return res
}

// Get retrieves a security scheme by name.
// It returns the stored value and true when present, otherwise (nil, false).
func (ss SecuritySchemes) Get(key string) (SecuritySchemeMarker, bool) {
if ss.container == nil {
return nil, false
}
value, ok := ss.container[key]
return value, ok
}

// GetVal retrieves a security scheme by name.
// It returns the stored value and true when present, otherwise nil.
func (ss SecuritySchemes) GetVal(key string) interface{} {
val, _ := ss.Get(key)
return val
}

// Delete removes a scheme from the receiver by name.
// It silently returns when the receiver or its container map is nil.
func (ss SecuritySchemes) delete(key string) SecuritySchemes {
var res SecuritySchemes
res.container = clone.Clone(ss.container)

Check notice on line 276 in apidef/oas/authentication.go

View check run for this annotation

probelabs / Visor: performance

performance Issue 

The `delete` method performs a deep clone of the entire security schemes map on every call. This is inefficient if multiple deletions are needed, as it repeatedly copies the map.
Raw output 
If bulk deletions are a possibility, provide a method to remove multiple keys at once to reduce the number of cloning operations. As with the `Set` method, if API loading performance is critical, consider using a mutex-guarded mutable map instead of an immutable one.

delete(res.container, key)
return res
}

// Len reports the number of registered security schemes.
// A nil receiver reports zero.
func (ss SecuritySchemes) Len() int {
return len(ss.container)
}

// Iter returns a snapshot under a short read lock to avoid holding locks during iteration.
// This allocates per call. Given the typical small number of schemes, the trade-off is acceptable.
func (ss SecuritySchemes) Iter() iter.Seq2[string, interface{}] {
return func(yield func(string, interface{}) bool) {
for k, v := range ss.container {
if !yield(k, v) {
return
}
}
}
}

// MarshalJSON implements json.Marshaler.
func (ss *SecuritySchemes) MarshalJSON() ([]byte, error) {
if ss.container == nil {
return []byte(`{}`), nil
}

return json.Marshal(ss.container)
}

// UnmarshalJSON implements json.Unmarshaler.
func (ss *SecuritySchemes) UnmarshalJSON(b []byte) error {
if string(b) == "null" {
ss.container = nil
return nil
}

tmp := make(map[string]interface{})
if err := json.Unmarshal(b, &tmp); err != nil {
return err
}

container := make(map[string]SecuritySchemeMarker)
for k, v := range tmp {
if decoded, err := decodeScheme(v); err != nil {
return err
} else {
container[k] = decoded
}
}

ss.container = container
return nil
}

// MarshalYAML makes SecuritySchemes YAML-friendly by exposing its container map.
func (ss *SecuritySchemes) MarshalYAML() (interface{}, error) {
if ss.container == nil {
return map[string]interface{}{}, nil
}

return clone.Clone(ss.container), nil
}

// UnmarshalYAML populates SecuritySchemes.container from YAML.
func (ss *SecuritySchemes) UnmarshalYAML(n *yaml.Node) error {
//todo remap keys in the existing map
if n.Tag == "!!null" {
ss.container = make(map[string]SecuritySchemeMarker)
return nil
}

var tmp map[string]interface{}
if err := n.Decode(&tmp); err != nil {
return err
}

if container, err := decodeSchemes(tmp); err != nil {
return err
} else {
ss.container = container
}

return nil
}

var _ yaml.Unmarshaler = (*SecuritySchemes)(nil)
var _ yaml.Marshaler = (*SecuritySchemes)(nil)
var _ json.Marshaler = (*SecuritySchemes)(nil)
var _ json.Unmarshaler = (*SecuritySchemes)(nil)

// SecurityScheme defines an Importer interface for security schemes.
type SecurityScheme interface {
Import(nativeSS *openapi3.SecurityScheme, enable bool)
}

// Import takes the openapi3.SecurityScheme as argument and applies it to the receiver. The
// SecuritySchemes receiver is a map, so modification of the receiver is enabled, regardless
// of the fact that the receiver isn't a pointer type. The map is a pointer type itself.
func (ss SecuritySchemes) Import(name string, nativeSS *openapi3.SecurityScheme, enable bool) error {
switch {
case nativeSS.Type == typeAPIKey:
token := &Token{}
if ss[name] == nil {
ss[name] = token
// loadForImport returns an existing *T if present,
// or builds a new *T hydrated from a map (if possible),
// or otherwise returns a zero-value *T.
func loadForImport[T SecuritySchemeMarker](ss *SecuritySchemes, name string) T {
var zero T

if ss == nil {
return zero
}

scheme, exists := ss.Get(name)
if !exists {
return zero
}

if typed, ok := scheme.(T); ok {
return typed
}

var v T
decoder, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{TagName: "json", Result: v})
if err != nil {
log.Debug("Initialization of the new decoder couldn't succeed")
return zero
}

err = decoder.Decode(scheme)
if err != nil {
log.Debug("Unmarshalling to struct couldn't succeed")
return zero
}

return v
}

func decodeSchemes(in map[string]interface{}) (map[string]SecuritySchemeMarker, error) {
out := make(map[string]SecuritySchemeMarker, len(in))

for k, v := range in {
if decoded, err := decodeScheme(v); err != nil {
return nil, err
} else {
if tokenVal, ok := ss[name].(*Token); ok {
token = tokenVal
} else {
toStructIfMap(ss[name], token)
}
out[k] = decoded
}
}

return out, nil
}

func decodeScheme(raw interface{}) (SecuritySchemeMarker, error) {
return nil, errors.New("not implemented")
}

func (ss SecuritySchemes) importNative(name string, nativeSS *openapi3.SecurityScheme, enable bool) (SecuritySchemes, error) {
self := ss

switch {
case nativeSS.Type == typeAPIKey:
token := loadForImport[*Token](&ss, name)
token.Enabled = &enable
case nativeSS.Type == typeHTTP && nativeSS.Scheme == schemeBearer && nativeSS.BearerFormat == bearerFormatJWT:
jwt := &JWT{}
if ss[name] == nil {
ss[name] = jwt
} else {
if jwtVal, ok := ss[name].(*JWT); ok {
jwt = jwtVal
} else {
toStructIfMap(ss[name], jwt)
}
}
self = self.Set(name, token)

case nativeSS.Type == typeHTTP &&
nativeSS.Scheme == schemeBearer &&
nativeSS.BearerFormat == bearerFormatJWT:

jwt := loadForImport[*JWT](&ss, name)
jwt.Import(enable)
case nativeSS.Type == typeHTTP && nativeSS.Scheme == schemeBasic:
basic := &Basic{}
if ss[name] == nil {
ss[name] = basic
} else {
if basicVal, ok := ss[name].(*Basic); ok {
basic = basicVal
} else {
toStructIfMap(ss[name], basic)
}
}
self = self.Set(name, jwt)

case nativeSS.Type == typeHTTP &&
nativeSS.Scheme == schemeBasic:

basic := loadForImport[*Basic](&ss, name)
basic.Import(enable)
case nativeSS.Type == typeOAuth2:
oauth := &OAuth{}
if ss[name] == nil {
ss[name] = oauth
} else {
if oauthVal, ok := ss[name].(*OAuth); ok {
oauth = oauthVal
} else {
toStructIfMap(ss[name], oauth)
}
}
self = self.Set(name, basic)

case nativeSS.Type == typeOAuth2:
oauth := loadForImport[*OAuth](&ss, name)
oauth.Import(enable)
self = self.Set(name, oauth)

default:
return fmt.Errorf(unsupportedSecuritySchemeFmt, name)
return SecuritySchemes{}, fmt.Errorf(unsupportedSecuritySchemeFmt, name)
}

return nil
return self, nil
}

func baseIdentityProviderPrecedence(authType apidef.AuthTypeEnum) int {
Expand All @@ -318,14 +478,14 @@

// GetBaseIdentityProvider returns the identity provider by precedence from SecuritySchemes.
func (ss SecuritySchemes) GetBaseIdentityProvider() (res apidef.AuthTypeEnum) {
if len(ss) < 2 {
if ss.Len() < 2 {
return
}

resBaseIdentityProvider := baseIdentityProviderPrecedence(apidef.AuthTypeNone)
res = apidef.OAuthKey

for _, scheme := range ss {
for _, scheme := range ss.Iter() {
if _, ok := scheme.(*Token); ok {
return apidef.AuthToken
}
Expand Down Expand Up @@ -576,6 +736,8 @@

// HMAC holds the configuration for the HMAC authentication mode.
type HMAC struct {
SecuritySchemeMarkerImpl

// Enabled activates the HMAC authentication mode.
//
// Tyk classic API definition: `enable_signature_checking`.
Expand Down Expand Up @@ -757,6 +919,7 @@

// CustomPluginAuthentication holds configuration for custom plugins.
type CustomPluginAuthentication struct {
SecuritySchemeMarkerImpl
// Enabled activates the CustomPluginAuthentication authentication mode.
//
// Tyk classic API definition: `enable_coprocess_auth`/`use_go_plugin_auth`.
Expand Down Expand Up @@ -1018,3 +1181,57 @@

id.Config.ExtractTo(api)
}

//
//var _ json.Marshaler = (*Scheme)(nil)
//var _ json.Unmarshaler = (*Scheme)(nil)
//var _ yaml.Marshaler = (*Scheme)(nil)
//var _ yaml.Unmarshaler = (*Scheme)(nil)
//
//type Scheme struct {
// src map[string]any
// mu sync.RWMutex
//
// jwt *JWT
// basic *Basic
//}
//
//func NewSchemeFromMap(in map[string]any) *Scheme {
// return &Scheme{
// src: in,
// }
//}
//
//func (a *Scheme) Jwt() *JWT {
// return a.jwt
//}
//
//func (a *Scheme) Basic() *Basic {
// return a.basic
//}
//
//func (a *Scheme) MarshalJSON() ([]byte, error) {
// a.mu.RLock()
// defer a.mu.RUnlock()
// return json.Marshal(a.src)
//}
//
//func (a *Scheme) UnmarshalJSON(bytes []byte) error {
// a.mu.Lock()
// defer a.mu.Unlock()
// return json.Unmarshal(bytes, &a.src)
//}
//
//func (a *Scheme) MarshalYAML() (interface{}, error) {
// a.mu.RLock()
// defer a.mu.RUnlock()
//
// return yaml.Marshal(a.src)
//}
//
//func (a *Scheme) UnmarshalYAML(value *yaml.Node) error {
// a.mu.Lock()
// defer a.mu.Unlock()
//
// return yaml.Unmarshal(value, &a.src)
//}
Loading
Loading