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
9 changes: 7 additions & 2 deletions gateway/mw_jwt.go
Original file line number Diff line number Diff line change
Expand Up @@ -1593,16 +1593,21 @@
return "", ErrNoSuitableUserIDClaimFound
}

func (gw *Gateway) invalidateJWKSCacheForAPIID(w http.ResponseWriter, r *http.Request) {
apiID := mux.Vars(r)["apiID"]
func invalidateJWKSCacheByAPIID(apiID string) {
raw, ok := JWKCaches.Load(apiID)
if ok {
jwkCache, interfaceOK := raw.(cache.Repository)
if !interfaceOK {
panic("JWKCache must implement cache.Repository")
}

Check warning on line 1602 in gateway/mw_jwt.go

View check run for this annotation

probelabs / Visor: architecture

architecture Issue

The use of `panic` on a failed type assertion can cause the entire gateway process to crash. While a type mismatch in the `JWKCaches` map indicates a severe programming error, a panic is an overly aggressive failure mode for a production gateway. A single API's misconfigured cache could lead to a denial of service for all APIs handled by the gateway instance. A more resilient design would be to log a critical error and return, thereby isolating the fault to the specific API and preventing a system-wide outage.
Raw output
Replace the `panic` with a critical error log. This ensures that the programming error is recorded for developers to fix, but it prevents the gateway from crashing, improving the overall stability and resilience of the system. For example: `mainLog.Errorf("Value in JWKCache is not of type cache.Repository for API ID: %s", apiID)`.
jwkCache.Flush()
mainLog.Debugf("JWKS cache for API: %s has been flushed", apiID)
}
}

func (gw *Gateway) invalidateJWKSCacheForAPIID(w http.ResponseWriter, r *http.Request) {
apiID := mux.Vars(r)["apiID"]
invalidateJWKSCacheByAPIID(apiID)
// Cache invalidation is idempotent: calling it ensures the key is absent,
// regardless of whether it was cached before or not.
doJSONWrite(w, http.StatusOK, apiOk("cache invalidated"))
Expand Down
45 changes: 40 additions & 5 deletions gateway/mw_jwt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,16 @@
"testing"
"time"

"github.com/TykTechnologies/tyk/apidef"

Check failure on line 15 in gateway/mw_jwt_test.go

View workflow job for this annotation

GitHub Actions / lint

File is not properly formatted (gci)
"github.com/TykTechnologies/tyk/apidef/oas"
"github.com/TykTechnologies/tyk/config"
tyktime "github.com/TykTechnologies/tyk/internal/time"
"github.com/TykTechnologies/tyk/test"
"github.com/TykTechnologies/tyk/user"
"github.com/go-jose/go-jose/v3"
"github.com/golang-jwt/jwt/v4"
"github.com/stretchr/testify/assert"

"github.com/TykTechnologies/tyk/apidef"
"github.com/TykTechnologies/tyk/config"
"github.com/TykTechnologies/tyk/test"
"github.com/TykTechnologies/tyk/user"
"github.com/stretchr/testify/require"

"github.com/TykTechnologies/tyk/internal/cache"
"github.com/TykTechnologies/tyk/internal/uuid"
Expand Down Expand Up @@ -2425,6 +2425,41 @@
assert.Equal(t, 0, jwkCache.Count())
}

func Test_NoticeInvalidateJWKSCacheForAPI(t *testing.T) {
ts := StartTest(nil)
defer ts.Close()

spec, jwtToken := ts.prepareJWTSessionRSAWithEncodedJWKWithAPIID(uuid.NewHex())

authHeaders := map[string]string{"authorization": jwtToken}

spec.JWTSource = testHttpJWK
ts.Gw.LoadAPI(spec)

// Fill the JWKS cache
_, err := ts.Run(t, test.TestCase{
Headers: authHeaders, Code: http.StatusOK,
})
assert.Nil(t, err)

// The previous request should have filled the cache with some entries
jwkCache := loadOrCreateJWKCacheByApiID(spec.APIID)
assert.True(t, jwkCache.Count() > 0)

// Emit event
n := Notification{
Command: NoticeInvalidateJWKSCacheForAPI,
Payload: spec.APIID,
Gw: ts.Gw,
}
ts.Gw.MainNotifier.Notify(n)

require.Eventuallyf(t, func() bool {
jwkCache = loadOrCreateJWKCacheByApiID(spec.APIID)
return jwkCache.Count() == 0
}, 5*time.Second, 100*time.Millisecond, "JWKS cache could not be flushed")
}

func TestJWTSessionRSAWithEncodedJWK(t *testing.T) {
ts := StartTest(nil)
defer ts.Close()
Expand Down
7 changes: 5 additions & 2 deletions gateway/redis_signals.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@
KeySpaceUpdateNotification NotificationCommand = "KeySpaceUpdateNotification"
OAuthPurgeLapsedTokens NotificationCommand = "OAuthPurgeLapsedTokens"
// NoticeDeleteAPICache is the command with which event is emitted from dashboard to invalidate cache for an API.
NoticeDeleteAPICache NotificationCommand = "DeleteAPICache"
NoticeUserKeyReset NotificationCommand = "UserKeyReset"
NoticeDeleteAPICache NotificationCommand = "DeleteAPICache"

Check warning on line 44 in gateway/redis_signals.go

View check run for this annotation

probelabs / Visor: dependency

architecture Issue

This change introduces a new Redis notification command, `NoticeInvalidateJWKSCacheForAPI`, which the gateway is now configured to consume. For this feature to be fully functional in a Multi-Data Center Bridge (MDCB) environment, a corresponding change is required in the publisher component (Tyk Dashboard, using tyk-sink) to send this command. Without the publisher-side implementation, this new cache-flushing capability will not be triggerable via MDCB.
Raw output
Ensure a corresponding ticket and pull request exist for the Tyk Dashboard/tyk-sink to implement the publishing of the `NoticeInvalidateJWKSCacheForAPI` command to complete the end-to-end feature.

Check warning on line 44 in gateway/redis_signals.go

View check run for this annotation

probelabs / Visor: connectivity

architecture Issue

The PR introduces a new Redis notification command `NoticeInvalidateJWKSCacheForAPI` for the gateway to consume. However, for this feature to be fully functional through the Multi-Data Center Bridge (MDCB), a corresponding change is required in the publisher component (e.g., Tyk Dashboard via tyk-sink) to send this command. Without the publisher-side implementation, this feature is incomplete, and the new listener logic will not be triggered in an MDCB environment.
Raw output
Verify that a corresponding pull request exists or is created for the Tyk Dashboard/tyk-sink to publish the `NoticeInvalidateJWKSCacheForAPI` command. The functionality is not complete without the sender-side implementation.
NoticeUserKeyReset NotificationCommand = "UserKeyReset"
NoticeInvalidateJWKSCacheForAPI NotificationCommand = "InvalidateJWKSCacheForAPI"
)

// Notification is a type that encodes a message published to a pub sub channel (shared between implementations)
Expand Down Expand Up @@ -159,6 +160,8 @@
if ok := gw.invalidateAPICache(notif.Payload); !ok {
log.WithError(err).Errorf("cache invalidation failed for: %s", notif.Payload)
}
case NoticeInvalidateJWKSCacheForAPI:

Check warning on line 163 in gateway/redis_signals.go

View check run for this annotation

probelabs / Visor: connectivity

security Issue

The introduction of the `NoticeInvalidateJWKSCacheForAPI` notification command allows for remote flushing of the JWKS cache via Redis pub/sub. If an attacker gains the ability to publish messages to the Tyk Redis instance, they could send a high volume of these notifications. This would force Tyk gateways to continuously flush their JWKS caches and re-fetch keys from upstream JWKS providers, leading to performance degradation and a potential Denial of Service (DoS) against both the gateway and the upstream identity provider.
Raw output
To enhance resilience, consider implementing rate-limiting within the gateway for this cache flush operation. This would prevent a flood of notifications for the same API ID from causing excessive upstream requests. Additionally, ensure that Redis instances are deployed in a trusted network with strict access controls.
invalidateJWKSCacheByAPIID(notif.Payload)
case NoticeUserKeyReset:
gw.handleUserKeyReset(notif.Payload)
default:
Expand Down
Loading