[TT-14494] improve error logging for JWKS URL handling

[NurayAhmadova]

Description

Fixed vague error logging (e.g., "illegal base64 data") for JWKS failures. Previously, generic errors made it difficult to distinguish between misconfiguration, network outages, or upstream content issues. The Gateway now specifically classifies these errors—identifying invalid Base64 configuration, network resolution failures, and invalid JSON content—to make debugging faster and more actionable.

Related Issue

TT-14494

Motivation and Context

Generic error messages hindered the diagnosis of authentication issues involving JWKS. This change ensures logs clearly distinguish between misconfiguration, network outages, and upstream content issues, providing immediate, actionable feedback to administrators.

How This Has Been Tested

Refactored and added new table-driven unit tests in TestJWTMiddleware_ErrorLogging and TestGatewayLogJWKError to verify:

• Configuration: Validation of jwt_source / jwks_uri for valid Base64.
• Network: Correct logging of DNS resolution failures and timeouts.
• Content: Handling of reachable endpoints returning invalid JSON or empty bodies.
• Verified that both the error type matches and the log message contains the specific root cause string.

Screenshots (if appropriate)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

• I ensured that the documentation is up to date
• I explained why this PR updates go.mod in detail with reasoning why it's required
• I would like a code coverage CI quality gate exception and have explained why

Ticket Details

TT-14494

Status	In Code Review
Summary	Unclear Gateway error logs when handling JWKS URLs

Generated at: 2025-12-17 05:40:11

Ticket Details

TT-14494

Status	In Code Review
Summary	Unclear Gateway error logs when handling JWKS URLs

Generated at: 2025-12-17 11:33:27

[probelabs]

This pull request introduces a centralized helper function, logJWKError, to improve the clarity and consistency of error logging for JSON Web Key Set (JWKS) URL handling. This change provides more specific and actionable error messages by categorizing failures into network/resolution issues (e.g., DNS errors, unreachable hosts) and content/parsing issues (e.g., malformed JSON). This enhances the debuggability of the gateway when interacting with external identity providers.

Files Changed Analysis

The core of this change is the new logJWKError function in gateway/log_helpers.go and its comprehensive tests in gateway/log_helpers_test.go. The existing JWT (mw_jwt.go) and External OAuth (mw_external_oauth.go) middlewares have been refactored to use this new helper, replacing generic error messages with categorized, more informative logs. A significant portion of the changes involves adding extensive test cases to verify the new logging behavior across a wide range of failure scenarios.

Architecture & Impact Assessment

• What this PR accomplishes: It standardizes and improves the observability of issues related to fetching and parsing JWKS. Operators can now more easily distinguish between network connectivity problems and issues with the JWKS content itself.
• Key technical changes introduced: A new centralized logging function, logJWKError, is introduced. This function uses modern Go error handling (errors.As, errors.Is) to classify errors based on their type. Existing middlewares are updated to use this function.
• Affected system components: The primary components affected are the JWT and External OAuth middlewares within the Tyk Gateway. The impact is confined to the logging subsystem, improving diagnostics without altering core request processing logic.

graph TD
    A[Error during JWKS fetch/parse] --> B{logJWKError};
    B --> C{Is it a Content/JSON error?};
    C -- Yes --> D[Log \"Invalid JWKS retrieved from endpoint\"];
    C -- No --> E{Is it a Network error?};
    E -- Yes --> F[Log \"JWKS endpoint resolution failed\"];
    E -- No --> G[Log \"Failed to fetch or decode JWKs\"];

Loading

Scope Discovery & Context Expansion

The changes are well-contained within the gateway package, primarily affecting the JWT and External OAuth middlewares where JWKS URLs are processed. The new logJWKError helper establishes a reusable pattern for categorized error logging. The refactoring correctly targets the primary functions responsible for fetching JWKS data, such as getSecretFromURL and getSecretFromJWKURL, and also covers initialization and legacy fallback mechanisms. The provided diff and extensive test files offer sufficient context to understand the scope and impact.

Metadata

• Review Effort: 3 / 5
• Primary Label: enhancement

---

Powered by Visor from Probelabs

Last updated: 2025-12-17T05:43:11.608Z | Triggered by: pr_updated | Commit: 4246b5b

💡 TIP: You can chat with Visor using /visor ask <your question>

[probelabs]

Security Issues (4)

Severity	Location	Issue
🔴 Critical	gateway/mw_external_oauth.go:220	The `jwtValidation.Source` field, which can contain a base64-encoded secret for JWT signing (e.g., for HMAC algorithms), is logged in cleartext if base64 decoding fails. This can expose sensitive credentials in the logs. 💡 Suggestion Avoid logging the raw `JWTSource`. Instead, log a generic error message or a sanitized version of the source (e.g., its type or length). k.Logger().WithError(err).Error("JWKS source decode failed: input is not a valid base64 string")
🔴 Critical	gateway/mw_jwt.go:245	The `cachedAPIDef.JWTSource` field, which can contain a base64-encoded secret for JWT signing (e.g., for HMAC algorithms), is logged in cleartext if base64 decoding fails. This can expose sensitive credentials in the logs. 💡 Suggestion Avoid logging the raw `JWTSource`. Instead, log a generic error message or a sanitized version of the source (e.g., its type or length). k.Logger().WithError(err).Error("JWKS source decode failed: input is not a valid base64 string")
🔴 Critical	gateway/mw_jwt.go:344	The `config.JWTSource` field, which can contain a base64-encoded secret for JWT signing (e.g., for HMAC algorithms), is logged in cleartext if base64 decoding fails. This can expose sensitive credentials in the logs. 💡 Suggestion Avoid logging the raw `JWTSource`. Instead, log a generic error message or a sanitized version of the source (e.g., its type or length). k.Logger().WithError(err).Error("JWKS source decode failed: input is not a valid base64 string")
🟡 Warning	gateway/log_helpers.go:113-116	The `jwkURL` is logged directly in multiple error messages within the `logJWKError` function. URLs, especially for authentication-related endpoints like JWKS, can sometimes contain sensitive information such as access tokens in query parameters. Logging the full URL could expose this information. 💡 Suggestion Sanitize the URL before logging it. Parse the URL and log only the scheme, host, and path, omitting the query string. Alternatively, implement a filtering mechanism to remove known sensitive query parameters. Example: parsedURL, parseErr := url.Parse(jwkURL) sanitizedURL := jwkURL if parseErr == nil { parsedURL.RawQuery = "" parsedURL.Fragment = "" sanitizedURL = parsedURL.String() } logger.WithError(err).Errorf("JWKS endpoint resolution failed: invalid or unreachable host %s", sanitizedURL)

Architecture Issues (1)

Severity	Location	Issue
🟡 Warning	gateway/mw_jwt.go:273	The error object `err` is logged twice in this block. The new helper function `logJWKError` is called on line 272, which logs the error with its details at the ERROR level. Immediately after, on line 273, `k.Logger().WithError(err).Info(...)` is called, which logs the same error object again with an informational message. This creates redundant log entries and is inconsistent with a similar change on line 281 where `WithError(err)` was correctly removed from the informational log. 💡 Suggestion Remove `WithError(err)` from the `.Info()` log call to avoid duplication, as the error has already been logged by `logJWKError`. This will make the logging pattern consistent throughout the function. 🔧 Suggested Fix k.Logger().Info("Failed to decode JWKs body with factory client. Trying x5c PEM fallback.")

✅ Performance Check Passed

No performance issues found – changes LGTM.

Quality Issues (1)

Severity	Location	Issue
🟡 Warning	gateway/mw_jwt_test.go:3960-3963	A test case modifies the package-level function variable `GetJWK` for mocking but does not restore its original value. This creates a leaky mock that can affect other tests in the package, leading to flaky or incorrect test results. 💡 Suggestion Restore the original value of `GetJWK` after the test completes. The preferred method is to use `t.Cleanup` to ensure restoration even if the test panics. This ensures test isolation and prevents side effects between tests. Example using t.Cleanup: // In the setup function, which would need to accept `t *testing.T` originalGetJWK := GetJWK t.Cleanup(func() { GetJWK = originalGetJWK }) GetJWK = func(_ string, _ bool) (*jose.JSONWebKeySet, error) { return nil, errors.New("factory client failed") }

---

Powered by Visor from Probelabs

Last updated: 2025-12-17T05:43:14.251Z | Triggered by: pr_updated | Commit: 4246b5b

💡 TIP: You can chat with Visor using /visor ask <your question>

[github-actions]

API Changes

no api changes detected

[NurayAhmadova]

Notes for the reviewers:

1. Please take a look at the TODO left in the getSecretFromURL() method and let me know if that log should stay or be removed.
2. Currently sonarqube is complaining about these TODO, once the questions are resolved it will be deleted.
3. Visor Error in gateway/mw_jwt.go:101 -> if you look at the history of the comments, visor was complaining about there being an error level log where the execution continues, now it's suggesting to bring it back. Note that if it is brought back, it prints twice in some cases, which could lead to log pollution. Please advise
4. Visor security-related comments are being ignored after a conversation with the Tyk team, log sanitization is omitted from the scope of this PR.

[shults]

Looks good to me.
Except if statements in tests. They seem to be redundant.
Tests look like to be very similar, maybe you can extract common code?

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
94.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[probelabs]

⚠️ Cherry-pick encountered conflicts. A draft PR was created: #7650