From 9eac615dbe695f7dadb3eeb7240f2ab4266649f7 Mon Sep 17 00:00:00 2001 From: Guilherme Cassolato Date: Fri, 21 Aug 2026 11:20:20 +0200 Subject: [PATCH 1/5] Limit response body read in outbound HTTP evaluators Add `maxResponseBytes` field to the AuthConfig CRD for all evaluators that make outbound HTTP requests: generic HTTP metadata, callbacks, OPA external policy, JWT/OIDC discovery, OAuth2 token introspection, OIDC UserInfo, and UMA. When set, response bodies are capped via io.LimitReader before reading/decoding, preventing unbounded memory consumption from unexpectedly large responses. For third-party libraries (go-oidc) that read response bodies internally, a maxResponseBytesRoundTripper limits bodies at the HTTP transport level. Also refactors NewClient to use a functional options pattern (WithTimeout, WithTracing, WithMaxResponseBytes) replacing the previous NewClientWithTracing/NewClientWithTracingAndMaxResponseBytes functions. Co-Authored-By: Claude Opus 4.6 Signed-off-by: Guilherme Cassolato --- api/v1beta3/auth_config_types.go | 45 ++++++++ api/v1beta3/zz_generated.deepcopy.go | 25 ++++ controllers/auth_config_controller.go | 39 +++++-- docs/features.md | 29 +++++ .../authorino.kuadrant.io_authconfigs.yaml | 70 ++++++++++++ install/manifests.yaml | 70 ++++++++++++ pkg/evaluators/authorization/opa.go | 16 ++- pkg/evaluators/authorization/opa_test.go | 62 ++++++++++ pkg/evaluators/identity/jwt.go | 30 +++-- pkg/evaluators/identity/oauth2.go | 12 +- pkg/evaluators/identity/oauth2_test.go | 48 ++++++++ pkg/evaluators/metadata/generic_http.go | 15 ++- pkg/evaluators/metadata/generic_http_test.go | 108 ++++++++++++++++++ pkg/evaluators/metadata/uma.go | 43 +++---- pkg/evaluators/metadata/uma_test.go | 44 +++++++ pkg/evaluators/metadata/user_info.go | 14 ++- pkg/evaluators/metadata/user_info_test.go | 32 +++++- pkg/http/request.go | 99 +++++++++++----- pkg/http/request_test.go | 74 +++++++++++- pkg/json/json.go | 15 ++- pkg/oauth2/client_credentials.go | 2 +- 21 files changed, 796 insertions(+), 96 deletions(-) diff --git a/api/v1beta3/auth_config_types.go b/api/v1beta3/auth_config_types.go index c2499cd63..a703a8ac0 100644 --- a/api/v1beta3/auth_config_types.go +++ b/api/v1beta3/auth_config_types.go @@ -396,6 +396,15 @@ type JwtAuthenticationSpec struct { // +optional // +kubebuilder:validation:Minimum:=0 Timeout *int `json:"timeout,omitempty"` + + // Maximum number of bytes to read from OIDC discovery and JWKS HTTP response bodies. + // When set, response body readers will be limited to this size, preventing + // unbounded memory consumption from unexpectedly large responses. + // If the response exceeds this limit, the truncation will cause a decode error. + // If omitted or set to 0, no limit is applied. + // +optional + // +kubebuilder:validation:Minimum:=1 + MaxResponseBytes *int64 `json:"maxResponseBytes,omitempty"` } // Settings to perform the OAuth2 token introspection request. @@ -420,6 +429,15 @@ type OAuth2TokenIntrospectionSpec struct { // +optional // +kubebuilder:validation:Minimum:=0 Timeout *int `json:"timeout,omitempty"` + + // Maximum number of bytes to read from the token introspection HTTP response body. + // When set, the response body reader will be limited to this size, preventing + // unbounded memory consumption from unexpectedly large responses. + // If the response is JSON and exceeds this limit, the truncation will cause a decode error. + // If omitted or set to 0, no limit is applied. + // +optional + // +kubebuilder:validation:Minimum:=1 + MaxResponseBytes *int64 `json:"maxResponseBytes,omitempty"` } // Parameters of the Kubernetes TokenReview request @@ -600,6 +618,15 @@ type HttpEndpointSpec struct { // +optional // +kubebuilder:validation:Minimum:=0 Timeout *int `json:"timeout,omitempty"` + + // Maximum number of bytes to read from the HTTP response body. + // When set, the response body reader will be limited to this size, preventing + // unbounded memory consumption from unexpectedly large responses. + // If the response is JSON and exceeds this limit, the truncation will cause a decode error. + // If omitted or set to 0, no limit is applied. + // +optional + // +kubebuilder:validation:Minimum:=1 + MaxResponseBytes *int64 `json:"maxResponseBytes,omitempty"` } // +kubebuilder:validation:Enum:=GET;POST;PUT;PATCH;DELETE;HEAD;OPTIONS;CONNECT;TRACE @@ -666,6 +693,15 @@ type UserInfoMetadataSpec struct { // +optional // +kubebuilder:validation:Minimum:=0 Timeout *int `json:"timeout,omitempty"` + + // Maximum number of bytes to read from the UserInfo HTTP response body. + // When set, the response body reader will be limited to this size, preventing + // unbounded memory consumption from unexpectedly large responses. + // If the response exceeds this limit, the truncation will cause a decode error. + // If omitted or set to 0, no limit is applied. + // +optional + // +kubebuilder:validation:Minimum:=1 + MaxResponseBytes *int64 `json:"maxResponseBytes,omitempty"` } // Settings of the User-Managed Access (UMA) source of resource data. @@ -686,6 +722,15 @@ type UmaMetadataSpec struct { // +optional // +kubebuilder:validation:Minimum:=0 Timeout *int `json:"timeout,omitempty"` + + // Maximum number of bytes to read from UMA HTTP response bodies (discovery, PAT, resource queries). + // When set, response body readers will be limited to this size, preventing + // unbounded memory consumption from unexpectedly large responses. + // If the response exceeds this limit, the truncation will cause a decode error. + // If omitted or set to 0, no limit is applied. + // +optional + // +kubebuilder:validation:Minimum:=1 + MaxResponseBytes *int64 `json:"maxResponseBytes,omitempty"` } // +kubebuilder:validation:XValidation:rule="has(self.patternMatching) ? !(has(self.opa) || has(self.kubernetesSubjectAccessReview) || has(self.spicedb)) : has(self.opa) ? !(has(self.kubernetesSubjectAccessReview) || has(self.spicedb)) : has(self.kubernetesSubjectAccessReview) != has(self.spicedb)",message="Use exactly one of: patternMatching, opa, kubernetesSubjectAccessReview, spicedb" diff --git a/api/v1beta3/zz_generated.deepcopy.go b/api/v1beta3/zz_generated.deepcopy.go index abae85c9a..c88bd3683 100644 --- a/api/v1beta3/zz_generated.deepcopy.go +++ b/api/v1beta3/zz_generated.deepcopy.go @@ -722,6 +722,11 @@ func (in *HttpEndpointSpec) DeepCopyInto(out *HttpEndpointSpec) { *out = new(int) **out = **in } + if in.MaxResponseBytes != nil { + in, out := &in.MaxResponseBytes, &out.MaxResponseBytes + *out = new(int64) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HttpEndpointSpec. @@ -764,6 +769,11 @@ func (in *JwtAuthenticationSpec) DeepCopyInto(out *JwtAuthenticationSpec) { *out = new(int) **out = **in } + if in.MaxResponseBytes != nil { + in, out := &in.MaxResponseBytes, &out.MaxResponseBytes + *out = new(int64) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new JwtAuthenticationSpec. @@ -996,6 +1006,11 @@ func (in *OAuth2TokenIntrospectionSpec) DeepCopyInto(out *OAuth2TokenIntrospecti *out = new(int) **out = **in } + if in.MaxResponseBytes != nil { + in, out := &in.MaxResponseBytes, &out.MaxResponseBytes + *out = new(int64) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OAuth2TokenIntrospectionSpec. @@ -1296,6 +1311,11 @@ func (in *UmaMetadataSpec) DeepCopyInto(out *UmaMetadataSpec) { *out = new(int) **out = **in } + if in.MaxResponseBytes != nil { + in, out := &in.MaxResponseBytes, &out.MaxResponseBytes + *out = new(int64) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new UmaMetadataSpec. @@ -1332,6 +1352,11 @@ func (in *UserInfoMetadataSpec) DeepCopyInto(out *UserInfoMetadataSpec) { *out = new(int) **out = **in } + if in.MaxResponseBytes != nil { + in, out := &in.MaxResponseBytes, &out.MaxResponseBytes + *out = new(int64) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new UserInfoMetadataSpec. diff --git a/controllers/auth_config_controller.go b/controllers/auth_config_controller.go index 888f9ebf8..c16c16b0e 100644 --- a/controllers/auth_config_controller.go +++ b/controllers/auth_config_controller.go @@ -375,15 +375,22 @@ func (r *AuthConfigReconciler) translateAuthConfig(ctx context.Context, authConf authCred, ) oauth2.Timeout = oauth2Identity.Timeout + if oauth2Identity.MaxResponseBytes != nil { + oauth2.MaxResponseBytes = *oauth2Identity.MaxResponseBytes + } translatedIdentity.OAuth2 = oauth2 // oidc case api.JwtAuthentication: var jwtVerifier identity_evaluators.JWTVerifier + var jwtMaxResponseBytes int64 + if identity.Jwt.MaxResponseBytes != nil { + jwtMaxResponseBytes = *identity.Jwt.MaxResponseBytes + } if identity.Jwt.IssuerUrl != "" { - jwtVerifier = identity_evaluators.NewOIDCProviderVerifier(ctx, identity.Jwt.IssuerUrl, identity.Jwt.TTL, identity.Jwt.Timeout) + jwtVerifier = identity_evaluators.NewOIDCProviderVerifier(ctx, identity.Jwt.IssuerUrl, identity.Jwt.TTL, identity.Jwt.Timeout, jwtMaxResponseBytes) } else if identity.Jwt.JwksUrl != "" { - jwtVerifier = identity_evaluators.NewJwksVerifier(ctx, identity.Jwt.JwksUrl, identity.Jwt.Timeout) + jwtVerifier = identity_evaluators.NewJwksVerifier(ctx, identity.Jwt.JwksUrl, identity.Jwt.Timeout, jwtMaxResponseBytes) } else { return nil, fmt.Errorf("missing issuerUrl or jwksUrl for JWT authentication method") // should never happen if properly validated at the API level } @@ -510,6 +517,9 @@ func (r *AuthConfigReconciler) translateAuthConfig(ctx context.Context, authConf return nil, err } else { uma.Timeout = metadata.Uma.Timeout + if metadata.Uma.MaxResponseBytes != nil { + uma.MaxResponseBytes = *metadata.Uma.MaxResponseBytes + } translatedMetadata.UMA = uma } @@ -527,6 +537,9 @@ func (r *AuthConfigReconciler) translateAuthConfig(ctx context.Context, authConf } userInfo := metadata_evaluators.NewUserInfo(openIdConfigStore, metadata.UserInfo.UserInfoUrl) userInfo.Timeout = metadata.UserInfo.Timeout + if metadata.UserInfo.MaxResponseBytes != nil { + userInfo.MaxResponseBytes = *metadata.UserInfo.MaxResponseBytes + } translatedMetadata.UserInfo = userInfo // generic http @@ -609,12 +622,18 @@ func (r *AuthConfigReconciler) translateAuthConfig(ctx context.Context, authConf sharedSecret = string(secret.Data[externalRegistry.SharedSecret.Key]) } + var opaMaxResponseBytes int64 + if externalRegistry.MaxResponseBytes != nil { + opaMaxResponseBytes = *externalRegistry.MaxResponseBytes + } + externalSource = &authorization_evaluators.OPAExternalSource{ - Endpoint: externalRegistry.Url, - SharedSecret: sharedSecret, - AuthCredentials: newAuthCredential(externalRegistry.Credentials), - TTL: externalRegistry.TTL, - Timeout: externalRegistry.Timeout, + Endpoint: externalRegistry.Url, + SharedSecret: sharedSecret, + AuthCredentials: newAuthCredential(externalRegistry.Credentials), + TTL: externalRegistry.TTL, + Timeout: externalRegistry.Timeout, + MaxResponseBytes: opaMaxResponseBytes, } } @@ -1221,6 +1240,11 @@ func (r *AuthConfigReconciler) buildGenericHttpEvaluator(ctx context.Context, ht } } + var maxResponseBytes int64 + if http.MaxResponseBytes != nil { + maxResponseBytes = *http.MaxResponseBytes + } + ev := &metadata_evaluators.GenericHttp{ Endpoint: http.Url, DynamicEndpoint: dynamicEndpoint, @@ -1233,6 +1257,7 @@ func (r *AuthConfigReconciler) buildGenericHttpEvaluator(ctx context.Context, ht OAuth2: oauth2ClientCredentialsConfig, OAuth2TokenForceFetch: oauth2TokenForceFetch, Timeout: http.Timeout, + MaxResponseBytes: maxResponseBytes, } if sharedSecret != "" || oauth2ClientCredentialsConfig != nil { diff --git a/docs/features.md b/docs/features.md index d99c185ef..cb16d1272 100644 --- a/docs/features.md +++ b/docs/features.md @@ -421,6 +421,35 @@ In both cases, the location where the secret (long-lived or OAuth2 access token) Custom headers can be set with the `headers` field. Nevertheless, headers such as `Content-Type` and `Authorization` (or eventual custom header used for carrying the authentication secret, set instead via the `credentials` option) will be superseded by the respective values defined for the fields `contentType` and `sharedSecretRef`. +#### Limiting response size (`maxResponseBytes`) + +To protect Authorino from memory exhaustion caused by unexpectedly large HTTP responses from external services, set the `maxResponseBytes` field to cap the number of bytes read from the response body. + +```yaml +spec: + metadata: + my-metadata: + http: + url: https://external-service/metadata + maxResponseBytes: 65536 # 64 KiB +``` + +The same field is available in: +- `spec.callbacks.*.http` — HTTP callbacks +- `spec.authorization.*.opa.externalPolicy` — external OPA policy bundles +- `spec.authentication.*.jwt` — OIDC discovery and JWKS fetching +- `spec.authentication.*.oauth2Introspection` — OAuth2 token introspection +- `spec.metadata.*.userInfo` — OIDC UserInfo +- `spec.metadata.*.uma` — UMA resource registry (applies to discovery, PAT requests, and resource queries) + +> **Important:** When a response exceeds the limit, the body is truncated to the specified size. For `application/json` responses, this truncation produces malformed JSON that will cause a decode error. As a result, the evaluator will fail and the corresponding piece of the auth pipeline will not be available. +> +> Depending on how downstream authorization policies handle missing metadata, the outcome can vary: +> - **Policies that deny access when metadata is absent**: the request will be denied. This is the safer default. +> - **Policies that fall back to granting access on the absence of metadata**: truncation may cause the policy to inadvertently allow a request that should have been denied. Review your authorization policies to ensure they do not assume missing metadata means "allowed." +> +> When `maxResponseBytes` is omitted or set to `0`, no limit is applied and the full response body is read. + ### OIDC UserInfo ([`metadata.userInfo`](https://pkg.go.dev/github.com/kuadrant/authorino/api/v1beta3?utm_source=gopls#UserInfoMetadataSpec)) Online fetching of OpenID Connect (OIDC) UserInfo data (phase ii of the Authorino [Auth Pipeline](./architecture.md#the-auth-pipeline-aka-enforcing-protection-in-request-time)), associated with an OIDC identity source configured and resolved in phase (i). diff --git a/install/crd/authorino.kuadrant.io_authconfigs.yaml b/install/crd/authorino.kuadrant.io_authconfigs.yaml index b0b449f10..26bffc5d3 100644 --- a/install/crd/authorino.kuadrant.io_authconfigs.yaml +++ b/install/crd/authorino.kuadrant.io_authconfigs.yaml @@ -257,6 +257,16 @@ spec: The JSON Web Keys (JWK) obtained from this endpoint are automatically cached and the caching updated whenever the kid of a JWT does not match any of the cached JWKs (https://openid.net/specs/openid-connect-core-1_0.html#RotateSigKeys). One of: jwksUrl, issuerUrl type: string + maxResponseBytes: + description: |- + Maximum number of bytes to read from OIDC discovery and JWKS HTTP response bodies. + When set, response body readers will be limited to this size, preventing + unbounded memory consumption from unexpectedly large responses. + If the response exceeds this limit, the truncation will cause a decode error. + If omitted or set to 0, no limit is applied. + format: int64 + minimum: 1 + type: integer timeout: description: |- Timeout for OIDC discovery and JWK fetching HTTP requests, in milliseconds. @@ -319,6 +329,16 @@ spec: IMPORTANT: Ensure this URL points to a trusted OAuth2 server. If this value can be influenced by user input, you may be vulnerable to Server-Side Request Forgery (SSRF) attacks. type: string + maxResponseBytes: + description: |- + Maximum number of bytes to read from the token introspection HTTP response body. + When set, the response body reader will be limited to this size, preventing + unbounded memory consumption from unexpectedly large responses. + If the response is JSON and exceeds this limit, the truncation will cause a decode error. + If omitted or set to 0, no limit is applied. + format: int64 + minimum: 1 + type: integer timeout: description: |- Timeout for the token introspection HTTP request, in milliseconds. @@ -907,6 +927,16 @@ spec: type: object description: Custom headers in the HTTP request. type: object + maxResponseBytes: + description: |- + Maximum number of bytes to read from the HTTP response body. + When set, the response body reader will be limited to this size, preventing + unbounded memory consumption from unexpectedly large responses. + If the response is JSON and exceeds this limit, the truncation will cause a decode error. + If omitted or set to 0, no limit is applied. + format: int64 + minimum: 1 + type: integer method: default: GET description: |- @@ -1472,6 +1502,16 @@ spec: type: object description: Custom headers in the HTTP request. type: object + maxResponseBytes: + description: |- + Maximum number of bytes to read from the HTTP response body. + When set, the response body reader will be limited to this size, preventing + unbounded memory consumption from unexpectedly large responses. + If the response is JSON and exceeds this limit, the truncation will cause a decode error. + If omitted or set to 0, no limit is applied. + format: int64 + minimum: 1 + type: integer method: default: GET description: |- @@ -1836,6 +1876,16 @@ spec: type: object description: Custom headers in the HTTP request. type: object + maxResponseBytes: + description: |- + Maximum number of bytes to read from the HTTP response body. + When set, the response body reader will be limited to this size, preventing + unbounded memory consumption from unexpectedly large responses. + If the response is JSON and exceeds this limit, the truncation will cause a decode error. + If omitted or set to 0, no limit is applied. + format: int64 + minimum: 1 + type: integer method: default: GET description: |- @@ -2001,6 +2051,16 @@ spec: IMPORTANT: Ensure this URL points to a trusted UMA server. If this value can be influenced by user input, you may be vulnerable to Server-Side Request Forgery (SSRF) attacks. type: string + maxResponseBytes: + description: |- + Maximum number of bytes to read from UMA HTTP response bodies (discovery, PAT, resource queries). + When set, response body readers will be limited to this size, preventing + unbounded memory consumption from unexpectedly large responses. + If the response exceeds this limit, the truncation will cause a decode error. + If omitted or set to 0, no limit is applied. + format: int64 + minimum: 1 + type: integer timeout: description: |- Timeout for UMA HTTP requests (discovery, PAT, resource queries), in milliseconds. @@ -2021,6 +2081,16 @@ spec: Name of an OIDC JWT authentication rule whose obtained configuration includes an "userinfo_endpoint" claim. One of: identitySource, userInfoUrl type: string + maxResponseBytes: + description: |- + Maximum number of bytes to read from the UserInfo HTTP response body. + When set, the response body reader will be limited to this size, preventing + unbounded memory consumption from unexpectedly large responses. + If the response exceeds this limit, the truncation will cause a decode error. + If omitted or set to 0, no limit is applied. + format: int64 + minimum: 1 + type: integer timeout: description: |- Timeout for the UserInfo HTTP request, in milliseconds. diff --git a/install/manifests.yaml b/install/manifests.yaml index 07722f270..68354cdc4 100644 --- a/install/manifests.yaml +++ b/install/manifests.yaml @@ -292,6 +292,16 @@ spec: The JSON Web Keys (JWK) obtained from this endpoint are automatically cached and the caching updated whenever the kid of a JWT does not match any of the cached JWKs (https://openid.net/specs/openid-connect-core-1_0.html#RotateSigKeys). One of: jwksUrl, issuerUrl type: string + maxResponseBytes: + description: |- + Maximum number of bytes to read from OIDC discovery and JWKS HTTP response bodies. + When set, response body readers will be limited to this size, preventing + unbounded memory consumption from unexpectedly large responses. + If the response exceeds this limit, the truncation will cause a decode error. + If omitted or set to 0, no limit is applied. + format: int64 + minimum: 1 + type: integer timeout: description: |- Timeout for OIDC discovery and JWK fetching HTTP requests, in milliseconds. @@ -354,6 +364,16 @@ spec: IMPORTANT: Ensure this URL points to a trusted OAuth2 server. If this value can be influenced by user input, you may be vulnerable to Server-Side Request Forgery (SSRF) attacks. type: string + maxResponseBytes: + description: |- + Maximum number of bytes to read from the token introspection HTTP response body. + When set, the response body reader will be limited to this size, preventing + unbounded memory consumption from unexpectedly large responses. + If the response is JSON and exceeds this limit, the truncation will cause a decode error. + If omitted or set to 0, no limit is applied. + format: int64 + minimum: 1 + type: integer timeout: description: |- Timeout for the token introspection HTTP request, in milliseconds. @@ -966,6 +986,16 @@ spec: type: object description: Custom headers in the HTTP request. type: object + maxResponseBytes: + description: |- + Maximum number of bytes to read from the HTTP response body. + When set, the response body reader will be limited to this size, preventing + unbounded memory consumption from unexpectedly large responses. + If the response is JSON and exceeds this limit, the truncation will cause a decode error. + If omitted or set to 0, no limit is applied. + format: int64 + minimum: 1 + type: integer method: default: GET description: |- @@ -1579,6 +1609,16 @@ spec: type: object description: Custom headers in the HTTP request. type: object + maxResponseBytes: + description: |- + Maximum number of bytes to read from the HTTP response body. + When set, the response body reader will be limited to this size, preventing + unbounded memory consumption from unexpectedly large responses. + If the response is JSON and exceeds this limit, the truncation will cause a decode error. + If omitted or set to 0, no limit is applied. + format: int64 + minimum: 1 + type: integer method: default: GET description: |- @@ -1943,6 +1983,16 @@ spec: type: object description: Custom headers in the HTTP request. type: object + maxResponseBytes: + description: |- + Maximum number of bytes to read from the HTTP response body. + When set, the response body reader will be limited to this size, preventing + unbounded memory consumption from unexpectedly large responses. + If the response is JSON and exceeds this limit, the truncation will cause a decode error. + If omitted or set to 0, no limit is applied. + format: int64 + minimum: 1 + type: integer method: default: GET description: |- @@ -2108,6 +2158,16 @@ spec: IMPORTANT: Ensure this URL points to a trusted UMA server. If this value can be influenced by user input, you may be vulnerable to Server-Side Request Forgery (SSRF) attacks. type: string + maxResponseBytes: + description: |- + Maximum number of bytes to read from UMA HTTP response bodies (discovery, PAT, resource queries). + When set, response body readers will be limited to this size, preventing + unbounded memory consumption from unexpectedly large responses. + If the response exceeds this limit, the truncation will cause a decode error. + If omitted or set to 0, no limit is applied. + format: int64 + minimum: 1 + type: integer timeout: description: |- Timeout for UMA HTTP requests (discovery, PAT, resource queries), in milliseconds. @@ -2128,6 +2188,16 @@ spec: Name of an OIDC JWT authentication rule whose obtained configuration includes an "userinfo_endpoint" claim. One of: identitySource, userInfoUrl type: string + maxResponseBytes: + description: |- + Maximum number of bytes to read from the UserInfo HTTP response body. + When set, the response body reader will be limited to this size, preventing + unbounded memory consumption from unexpectedly large responses. + If the response exceeds this limit, the truncation will cause a decode error. + If omitted or set to 0, no limit is applied. + format: int64 + minimum: 1 + type: integer timeout: description: |- Timeout for the UserInfo HTTP request, in milliseconds. diff --git a/pkg/evaluators/authorization/opa.go b/pkg/evaluators/authorization/opa.go index 3f3f0597c..278429f9b 100644 --- a/pkg/evaluators/authorization/opa.go +++ b/pkg/evaluators/authorization/opa.go @@ -229,9 +229,10 @@ type OPAExternalSource struct { Endpoint string SharedSecret string auth.AuthCredentials - TTL int - Timeout *int - refresher workers.Worker + TTL int + Timeout *int + MaxResponseBytes int64 + refresher workers.Worker } func (ext *OPAExternalSource) downloadRegoDataFromUrl(ctx context.Context) (string, error) { @@ -256,14 +257,19 @@ func (ext *OPAExternalSource) downloadRegoDataFromUrl(ctx context.Context) (stri // Use the caller's context for tracing (so traces are linked), but the request uses Background context otel.GetTextMapPropagator().Inject(ctx, otel_propagation.HeaderCarrier(req.Header)) - if resp, err := httputil.NewClient(ext.Timeout).Do(req); err != nil { + if resp, err := httputil.NewClient(httputil.WithTimeout(ext.Timeout)).Do(req); err != nil { return "", fmt.Errorf("failed to fetch Rego config: %v", err) } else { defer func(Body io.ReadCloser) { _ = Body.Close() }(resp.Body) - body, err := io.ReadAll(resp.Body) + var bodyReader io.Reader = resp.Body + if ext.MaxResponseBytes > 0 { + bodyReader = io.LimitReader(resp.Body, ext.MaxResponseBytes) + } + + body, err := io.ReadAll(bodyReader) if err != nil { return "", fmt.Errorf("unable to read response body: %v", err) } diff --git a/pkg/evaluators/authorization/opa_test.go b/pkg/evaluators/authorization/opa_test.go index a4bed8049..6dbfd73c0 100644 --- a/pkg/evaluators/authorization/opa_test.go +++ b/pkg/evaluators/authorization/opa_test.go @@ -242,6 +242,68 @@ func TestOPANonBooleanAllowed(t *testing.T) { assert.ErrorContains(t, err, "Unauthorized") } +func TestOPAExternalUrlMaxResponseBytes(t *testing.T) { + t.Run("within limit", func(t *testing.T) { + extHttpMetadataServer := httptest.NewHttpServerMock(opaExtHttpServerMockAddr, map[string]httptest.HttpServerMockResponseFunc{ + "/rego": func() httptest.HttpServerMockResponse { + return httptest.HttpServerMockResponse{Status: 200, Body: opaInlineRegoV1DataMock} + }, + }) + defer extHttpMetadataServer.Close() + + externalSource := &OPAExternalSource{ + Endpoint: "http://" + opaExtHttpServerMockAddr + "/rego", + AuthCredentials: auth.NewAuthCredential("", ""), + MaxResponseBytes: int64(len(opaInlineRegoV1DataMock) + 100), + } + + opa, err := NewOPAAuthorization("test-opa-maxbytes-ok", "", externalSource, false, opaParser.RegoV1, 0, context.TODO()) + + assert.NilError(t, err) + assertOPAAuthorization(t, opa) + }) + + t.Run("exceeds limit truncates and causes error", func(t *testing.T) { + extHttpMetadataServer := httptest.NewHttpServerMock(opaExtHttpServerMockAddr, map[string]httptest.HttpServerMockResponseFunc{ + "/rego": func() httptest.HttpServerMockResponse { + return httptest.HttpServerMockResponse{Status: 200, Body: opaInlineRegoV1DataMock} + }, + }) + defer extHttpMetadataServer.Close() + + externalSource := &OPAExternalSource{ + Endpoint: "http://" + opaExtHttpServerMockAddr + "/rego", + AuthCredentials: auth.NewAuthCredential("", ""), + MaxResponseBytes: 10, + } + + opa, err := NewOPAAuthorization("test-opa-maxbytes-truncated", "", externalSource, false, opaParser.RegoV1, 0, context.TODO()) + + assert.Assert(t, err != nil, "expected error due to truncated Rego policy") + assert.Assert(t, opa == nil) + }) + + t.Run("zero means no limit", func(t *testing.T) { + extHttpMetadataServer := httptest.NewHttpServerMock(opaExtHttpServerMockAddr, map[string]httptest.HttpServerMockResponseFunc{ + "/rego": func() httptest.HttpServerMockResponse { + return httptest.HttpServerMockResponse{Status: 200, Body: opaInlineRegoV1DataMock} + }, + }) + defer extHttpMetadataServer.Close() + + externalSource := &OPAExternalSource{ + Endpoint: "http://" + opaExtHttpServerMockAddr + "/rego", + AuthCredentials: auth.NewAuthCredential("", ""), + MaxResponseBytes: 0, + } + + opa, err := NewOPAAuthorization("test-opa-maxbytes-nolimit", "", externalSource, false, opaParser.RegoV1, 0, context.TODO()) + + assert.NilError(t, err) + assertOPAAuthorization(t, opa) + }) +} + func assertOPAAuthorization(t *testing.T, opa *OPA) { ctrl := gomock.NewController(t) defer ctrl.Finish() diff --git a/pkg/evaluators/identity/jwt.go b/pkg/evaluators/identity/jwt.go index d4384c6f8..c28952d15 100644 --- a/pkg/evaluators/identity/jwt.go +++ b/pkg/evaluators/identity/jwt.go @@ -92,18 +92,24 @@ type JWTVerifier interface { } type oidcProviderVerifier struct { - issuerUrl string - timeout *int + issuerUrl string + timeout *int + maxResponseBytes int64 mu sync.RWMutex provider *oidc.Provider refresher workers.Worker } -func NewOIDCProviderVerifier(ctx gocontext.Context, issuerUrl string, ttl int, timeout *int) JWTVerifier { +func NewOIDCProviderVerifier(ctx gocontext.Context, issuerUrl string, ttl int, timeout *int, maxResponseBytes ...int64) JWTVerifier { + var maxBytes int64 + if len(maxResponseBytes) > 0 { + maxBytes = maxResponseBytes[0] + } v := &oidcProviderVerifier{ - issuerUrl: issuerUrl, - timeout: timeout, + issuerUrl: issuerUrl, + timeout: timeout, + maxResponseBytes: maxBytes, } ctxWithLogger := log.IntoContext(ctx, log.FromContext(ctx).WithName("jwt")) v.getOpenIdProvider(ctxWithLogger, false) @@ -170,10 +176,10 @@ func (v *oidcProviderVerifier) getOpenIdProvider(ctx gocontext.Context, force bo defer v.mu.Unlock() if v.provider == nil || force { - // Create HTTP client with timeout and trace propagation. + // Create HTTP client with timeout, trace propagation, and optional response body size limit. // Use Background context for request lifecycle (to avoid cancellation from reconciliation), // but propagate trace context from caller's ctx for observability. - httpClient := httputil.NewClientWithTracing(ctx, v.timeout) + httpClient := httputil.NewClient(httputil.WithTimeout(v.timeout), httputil.WithTracing(ctx), httputil.WithMaxResponseBytes(v.maxResponseBytes)) discoveryCtx := oidc.ClientContext(gocontext.Background(), httpClient) if provider, err := oidc.NewProvider(discoveryCtx, v.issuerUrl); err != nil { @@ -203,11 +209,15 @@ type jwksVerifier struct { jwks oidc.KeySet } -func NewJwksVerifier(ctx gocontext.Context, jwksUrl string, timeout *int) JWTVerifier { - // Create HTTP client with timeout and trace propagation. +func NewJwksVerifier(ctx gocontext.Context, jwksUrl string, timeout *int, maxResponseBytes ...int64) JWTVerifier { + // Create HTTP client with timeout, trace propagation, and optional response body size limit. // Use Background context for request lifecycle (to avoid cancellation from reconciliation), // but propagate trace context from caller's ctx for observability. - httpClient := httputil.NewClientWithTracing(ctx, timeout) + var maxBytes int64 + if len(maxResponseBytes) > 0 { + maxBytes = maxResponseBytes[0] + } + httpClient := httputil.NewClient(httputil.WithTimeout(timeout), httputil.WithTracing(ctx), httputil.WithMaxResponseBytes(maxBytes)) jwkCtx := oidc.ClientContext(gocontext.Background(), httpClient) return &jwksVerifier{ diff --git a/pkg/evaluators/identity/oauth2.go b/pkg/evaluators/identity/oauth2.go index d25348731..9b18d3300 100644 --- a/pkg/evaluators/identity/oauth2.go +++ b/pkg/evaluators/identity/oauth2.go @@ -26,6 +26,7 @@ type OAuth2 struct { ClientID string `yaml:"clientId"` ClientSecret string `yaml:"clientSecret"` Timeout *int + MaxResponseBytes int64 } func NewOAuth2Identity(tokenIntrospectionUrl string, tokenTypeHint string, clientID string, clientSecret string, creds auth.AuthCredentials) *OAuth2 { @@ -77,7 +78,7 @@ func (oauth *OAuth2) Call(pipeline auth.AuthPipeline, ctx gocontext.Context) (in otel.GetTextMapPropagator().Inject(ctx, otel_propagation.HeaderCarrier(req.Header)) - resp, err := httputil.NewClient(oauth.Timeout).Do(req) + resp, err := httputil.NewClient(httputil.WithTimeout(oauth.Timeout)).Do(req) if err != nil { return nil, err } @@ -85,6 +86,11 @@ func (oauth *OAuth2) Call(pipeline auth.AuthPipeline, ctx gocontext.Context) (in _ = Body.Close() }(resp.Body) + var bodyReader io.Reader = resp.Body + if oauth.MaxResponseBytes > 0 { + bodyReader = io.LimitReader(resp.Body, oauth.MaxResponseBytes) + } + // a non-200 response (e.g. invalid client credentials) is not a valid introspection // result and may not carry the RFC 7662 "active" field; treat it as an error instead // of attempting to parse it as a successful introspection response. @@ -92,14 +98,14 @@ func (oauth *OAuth2) Call(pipeline auth.AuthPipeline, ctx gocontext.Context) (in // the X-Ext-Auth-Reason header) and logged; the response body may contain data from the // auth server and is only logged at debug level. if resp.StatusCode != http.StatusOK { - body, _ := io.ReadAll(resp.Body) + body, _ := io.ReadAll(bodyReader) log.FromContext(ctx).WithName("oauth2").V(1).Info("token introspection request failed", "status", resp.Status, "response", string(body)) return nil, fmt.Errorf("token introspection request failed: %s", resp.Status) } // parse the response var claims map[string]interface{} - if err := json.NewDecoder(resp.Body).Decode(&claims); err != nil { + if err := json.NewDecoder(bodyReader).Decode(&claims); err != nil { return nil, err } else { active, ok := claims["active"].(bool) diff --git a/pkg/evaluators/identity/oauth2_test.go b/pkg/evaluators/identity/oauth2_test.go index d1e385fed..01198dd30 100644 --- a/pkg/evaluators/identity/oauth2_test.go +++ b/pkg/evaluators/identity/oauth2_test.go @@ -3,6 +3,7 @@ package identity import ( "context" "fmt" + "strings" "testing" mock_auth "github.com/kuadrant/authorino/pkg/auth/mocks" @@ -76,6 +77,53 @@ func TestOAuth2Call(t *testing.T) { } } +func TestOAuth2MaxResponseBytes(t *testing.T) { + largeClaims := `{ "active": true, "sub": "user123", "extra": "` + strings.Repeat("x", 1000) + `" }` + + authServer := httptest.NewHttpServerMock(oauthServerHost, map[string]httptest.HttpServerMockResponseFunc{ + "/introspect-large": func() httptest.HttpServerMockResponse { + return httptest.HttpServerMockResponse{Status: 200, Body: largeClaims} + }, + }) + defer authServer.Close() + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + authCredMock := mock_auth.NewMockAuthCredentials(ctrl) + authCredMock.EXPECT().GetCredentialsFromAuthReq(gomock.Any()).Return("oauth-opaque-token", nil).AnyTimes() + + pipelineMock := mock_auth.NewMockAuthPipeline(ctrl) + pipelineMock.EXPECT().GetHttp().Return(nil).AnyTimes() + + ctx := context.Background() + + t.Run("within limit", func(t *testing.T) { + oauthEvaluator := NewOAuth2Identity(fmt.Sprintf("http://%v/introspect-large", oauthServerHost), "access_token", "client-id", "client-secret", authCredMock) + oauthEvaluator.MaxResponseBytes = int64(len(largeClaims) + 100) + obj, err := oauthEvaluator.Call(pipelineMock, ctx) + assert.NilError(t, err) + claims := obj.(map[string]interface{}) + assert.Assert(t, claims["active"]) + }) + + t.Run("exceeds limit causes decode error", func(t *testing.T) { + oauthEvaluator := NewOAuth2Identity(fmt.Sprintf("http://%v/introspect-large", oauthServerHost), "access_token", "client-id", "client-secret", authCredMock) + oauthEvaluator.MaxResponseBytes = 50 + _, err := oauthEvaluator.Call(pipelineMock, ctx) + assert.Assert(t, err != nil) + }) + + t.Run("zero means no limit", func(t *testing.T) { + oauthEvaluator := NewOAuth2Identity(fmt.Sprintf("http://%v/introspect-large", oauthServerHost), "access_token", "client-id", "client-secret", authCredMock) + oauthEvaluator.MaxResponseBytes = 0 + obj, err := oauthEvaluator.Call(pipelineMock, ctx) + assert.NilError(t, err) + claims := obj.(map[string]interface{}) + assert.Assert(t, claims["active"]) + }) +} + func TestDefaultTokenTypeHint(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() diff --git a/pkg/evaluators/metadata/generic_http.go b/pkg/evaluators/metadata/generic_http.go index a8f860d9e..50bb76ad7 100644 --- a/pkg/evaluators/metadata/generic_http.go +++ b/pkg/evaluators/metadata/generic_http.go @@ -34,6 +34,7 @@ type GenericHttp struct { OAuth2 *oauth2.ClientCredentials OAuth2TokenForceFetch bool Timeout *int + MaxResponseBytes int64 auth.AuthCredentials } @@ -59,7 +60,7 @@ func (h *GenericHttp) Call(pipeline auth.AuthPipeline, ctx gocontext.Context) (i return nil, err } - resp, err := httputil.NewClient(h.Timeout).Do(req) + resp, err := httputil.NewClient(httputil.WithTimeout(h.Timeout)).Do(req) if err != nil { return nil, err } @@ -67,9 +68,14 @@ func (h *GenericHttp) Call(pipeline auth.AuthPipeline, ctx gocontext.Context) (i _ = Body.Close() }(resp.Body) + var bodyReader io.Reader = resp.Body + if h.MaxResponseBytes > 0 { + bodyReader = io.LimitReader(resp.Body, h.MaxResponseBytes) + } + // parse the response as json if strings.Contains(strings.Join(resp.Header["Content-Type"], ";"), "application/json") { - decoder := gojson.NewDecoder(resp.Body) + decoder := gojson.NewDecoder(bodyReader) var elements []map[string]interface{} @@ -94,10 +100,7 @@ func (h *GenericHttp) Call(pipeline auth.AuthPipeline, ctx gocontext.Context) (i } // parse the response as text - defer func(Body io.ReadCloser) { - _ = Body.Close() - }(resp.Body) - str, err := io.ReadAll(resp.Body) + str, err := io.ReadAll(bodyReader) if err != nil { return nil, err } diff --git a/pkg/evaluators/metadata/generic_http_test.go b/pkg/evaluators/metadata/generic_http_test.go index b3e42efe8..a756e114c 100644 --- a/pkg/evaluators/metadata/generic_http_test.go +++ b/pkg/evaluators/metadata/generic_http_test.go @@ -6,6 +6,7 @@ import ( "fmt" "net/http" gohttptest "net/http/httptest" + "strings" "testing" mock_auth "github.com/kuadrant/authorino/pkg/auth/mocks" @@ -435,6 +436,113 @@ func TestWithOAuth2AuthenticationWithoutTokenCache(t *testing.T) { assert.Equal(t, objJSON["foo"], "bar") } +func TestGenericHttpMaxResponseBytesJSON(t *testing.T) { + largeJSON := `{"key":"` + strings.Repeat("x", 1000) + `"}` + + extHttpMetadataServer := httptest.NewHttpServerMock(testHttpMetadataServerHost, map[string]httptest.HttpServerMockResponseFunc{ + "/metadata": httptest.NewHttpServerMockResponseFuncJSON(largeJSON), + }) + defer extHttpMetadataServer.Close() + + ctx := context.TODO() + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + endpoint := "http://" + testHttpMetadataServerHost + "/metadata" + + t.Run("within limit", func(t *testing.T) { + pipelineMock := mock_auth.NewMockAuthPipeline(ctrl) + pipelineMock.EXPECT().GetAuthorizationJSON().Return(genericHttpAuthDataMock()) + + metadata := &GenericHttp{ + Endpoint: endpoint, + Method: "GET", + MaxResponseBytes: int64(len(largeJSON) + 100), + } + + obj, err := metadata.Call(pipelineMock, ctx) + assert.NilError(t, err) + objJSON := obj.(map[string]interface{}) + assert.Equal(t, len(objJSON["key"].(string)), 1000) + }) + + t.Run("exceeds limit truncates and causes decode error", func(t *testing.T) { + pipelineMock := mock_auth.NewMockAuthPipeline(ctrl) + pipelineMock.EXPECT().GetAuthorizationJSON().Return(genericHttpAuthDataMock()) + + metadata := &GenericHttp{ + Endpoint: endpoint, + Method: "GET", + MaxResponseBytes: 10, + } + + _, err := metadata.Call(pipelineMock, ctx) + assert.Assert(t, err != nil, "expected error due to truncated JSON") + }) + + t.Run("zero means no limit", func(t *testing.T) { + pipelineMock := mock_auth.NewMockAuthPipeline(ctrl) + pipelineMock.EXPECT().GetAuthorizationJSON().Return(genericHttpAuthDataMock()) + + metadata := &GenericHttp{ + Endpoint: endpoint, + Method: "GET", + MaxResponseBytes: 0, + } + + obj, err := metadata.Call(pipelineMock, ctx) + assert.NilError(t, err) + objJSON := obj.(map[string]interface{}) + assert.Equal(t, len(objJSON["key"].(string)), 1000) + }) +} + +func TestGenericHttpMaxResponseBytesPlainText(t *testing.T) { + largeBody := strings.Repeat("A", 500) + + extHttpMetadataServer := httptest.NewHttpServerMock(testHttpMetadataServerHost, map[string]httptest.HttpServerMockResponseFunc{ + "/metadata": httptest.NewHttpServerMockResponseFuncPlain(largeBody), + }) + defer extHttpMetadataServer.Close() + + ctx := context.TODO() + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + endpoint := "http://" + testHttpMetadataServerHost + "/metadata" + + t.Run("within limit", func(t *testing.T) { + pipelineMock := mock_auth.NewMockAuthPipeline(ctrl) + pipelineMock.EXPECT().GetAuthorizationJSON().Return(genericHttpAuthDataMock()) + + metadata := &GenericHttp{ + Endpoint: endpoint, + Method: "GET", + MaxResponseBytes: 1000, + } + + obj, err := metadata.Call(pipelineMock, ctx) + assert.NilError(t, err) + assert.Equal(t, obj, largeBody) + }) + + t.Run("exceeds limit truncates response", func(t *testing.T) { + pipelineMock := mock_auth.NewMockAuthPipeline(ctrl) + pipelineMock.EXPECT().GetAuthorizationJSON().Return(genericHttpAuthDataMock()) + + metadata := &GenericHttp{ + Endpoint: endpoint, + Method: "GET", + MaxResponseBytes: 100, + } + + obj, err := metadata.Call(pipelineMock, ctx) + assert.NilError(t, err) + assert.Equal(t, len(obj.(string)), 100) + assert.Equal(t, obj, strings.Repeat("A", 100)) + }) +} + func genericHttpAuthDataMock() string { type mockIdentityObject struct { User string `json:"user"` diff --git a/pkg/evaluators/metadata/uma.go b/pkg/evaluators/metadata/uma.go index c37f97405..81229a27e 100644 --- a/pkg/evaluators/metadata/uma.go +++ b/pkg/evaluators/metadata/uma.go @@ -38,19 +38,19 @@ func (provider *Provider) GetTokenURL() string { return provider.tokenURL } -func (provider *Provider) GetResourcesByURI(uri string, pat PAT, ctx gocontext.Context, timeout *int) ([]interface{}, error) { +func (provider *Provider) GetResourcesByURI(uri string, pat PAT, ctx gocontext.Context, timeout *int, maxResponseBytes int64) ([]interface{}, error) { if err := context.CheckContext(ctx); err != nil { return nil, err } - resourceIDs, err := provider.queryResourcesByURI(uri, pat, ctx, timeout) + resourceIDs, err := provider.queryResourcesByURI(uri, pat, ctx, timeout, maxResponseBytes) if err != nil { return nil, err } - return provider.getResourcesByIDs(resourceIDs, pat, ctx, timeout) + return provider.getResourcesByIDs(resourceIDs, pat, ctx, timeout, maxResponseBytes) } -func (provider *Provider) queryResourcesByURI(uri string, pat PAT, ctx gocontext.Context, timeout *int) ([]string, error) { +func (provider *Provider) queryResourcesByURI(uri string, pat PAT, ctx gocontext.Context, timeout *int, maxResponseBytes int64) ([]string, error) { if err := context.CheckContext(ctx); err != nil { return nil, err } @@ -61,14 +61,14 @@ func (provider *Provider) queryResourcesByURI(uri string, pat PAT, ctx gocontext log.FromContext(ctx).V(1).Info("querying resources by uri", "url", queryResourcesURL.String()) var resourceIDs []string - if err := pat.Get(queryResourcesURL.String(), ctx, &resourceIDs, timeout); err != nil { + if err := pat.Get(queryResourcesURL.String(), ctx, &resourceIDs, timeout, maxResponseBytes); err != nil { return nil, err } else { return resourceIDs, nil } } -func (provider *Provider) getResourcesByIDs(resourceIDs []string, pat PAT, ctx gocontext.Context, timeout *int) ([]interface{}, error) { +func (provider *Provider) getResourcesByIDs(resourceIDs []string, pat PAT, ctx gocontext.Context, timeout *int, maxResponseBytes int64) ([]interface{}, error) { if err := context.CheckContext(ctx); err != nil { return nil, err } @@ -82,7 +82,7 @@ func (provider *Provider) getResourcesByIDs(resourceIDs []string, pat PAT, ctx g go func(id string) { defer waitGroup.Done() - if data, err := provider.getResourceByID(id, pat, ctx, timeout); err == nil { + if data, err := provider.getResourceByID(id, pat, ctx, timeout, maxResponseBytes); err == nil { buf <- data } }(resourceID) @@ -98,7 +98,7 @@ func (provider *Provider) getResourcesByIDs(resourceIDs []string, pat PAT, ctx g return resourceData, nil } -func (provider *Provider) getResourceByID(resourceID string, pat PAT, ctx gocontext.Context, timeout *int) (interface{}, error) { +func (provider *Provider) getResourceByID(resourceID string, pat PAT, ctx gocontext.Context, timeout *int, maxResponseBytes int64) (interface{}, error) { if err := context.CheckContext(ctx); err != nil { return nil, err } @@ -109,7 +109,7 @@ func (provider *Provider) getResourceByID(resourceID string, pat PAT, ctx gocont log.FromContext(ctx).V(1).Info("getting resource data", "url", resourceURL.String()) var data interface{} - if err := pat.Get(resourceURL.String(), ctx, &data, timeout); err != nil { + if err := pat.Get(resourceURL.String(), ctx, &data, timeout, maxResponseBytes); err != nil { return nil, err } return data, nil @@ -123,7 +123,7 @@ func (pat *PAT) String() string { return pat.AccessToken } -func (pat *PAT) Get(rawurl string, ctx gocontext.Context, v interface{}, timeout *int) error { +func (pat *PAT) Get(rawurl string, ctx gocontext.Context, v interface{}, timeout *int, maxResponseBytes ...int64) error { if err := context.CheckContext(ctx); err != nil { return err } @@ -138,7 +138,7 @@ func (pat *PAT) Get(rawurl string, ctx gocontext.Context, v interface{}, timeout otel.GetTextMapPropagator().Inject(ctx, otel_propagation.HeaderCarrier(req.Header)) // get the response - resp, err := httputil.NewClient(timeout).Do(req) + resp, err := httputil.NewClient(httputil.WithTimeout(timeout)).Do(req) if err != nil { return err } @@ -146,7 +146,7 @@ func (pat *PAT) Get(rawurl string, ctx gocontext.Context, v interface{}, timeout _ = body.Close() }(resp.Body) - return json.UnmashalJSONResponse(resp, &v, nil) + return json.UnmashalJSONResponse(resp, &v, nil, maxResponseBytes...) } func NewUMAMetadata(ctx gocontext.Context, endpoint string, clientID string, clientSecret string) (*UMA, error) { @@ -163,10 +163,11 @@ func NewUMAMetadata(ctx gocontext.Context, endpoint string, clientID string, cli } type UMA struct { - Endpoint string `yaml:"endpoint,omitempty"` - ClientID string `yaml:"client_id"` - ClientSecret string `yaml:"client_secret"` - Timeout *int + Endpoint string `yaml:"endpoint,omitempty"` + ClientID string `yaml:"client_id"` + ClientSecret string `yaml:"client_secret"` + Timeout *int + MaxResponseBytes int64 provider *Provider } @@ -185,7 +186,7 @@ func (uma *UMA) discover(ctx gocontext.Context) error { otel.GetTextMapPropagator().Inject(ctx, otel_propagation.HeaderCarrier(req.Header)) - if resp, err := httputil.NewClient(uma.Timeout).Do(req); err != nil { + if resp, err := httputil.NewClient(httputil.WithTimeout(uma.Timeout)).Do(req); err != nil { return fmt.Errorf("failed to fetch uma config: %v", err) } else { defer func(body io.ReadCloser) { @@ -194,7 +195,7 @@ func (uma *UMA) discover(ctx gocontext.Context) error { var p providerJSON var rawClaims []byte - if err = json.UnmashalJSONResponse(resp, &p, &rawClaims); err != nil { + if err = json.UnmashalJSONResponse(resp, &p, &rawClaims, uma.MaxResponseBytes); err != nil { return fmt.Errorf("failed to decode uma provider discovery object: %v", err) } @@ -225,7 +226,7 @@ func (uma *UMA) Call(pipeline auth.AuthPipeline, parentCtx gocontext.Context) (i // get resource data uri := pipeline.GetHttp().GetPath() - resourceData, err := uma.provider.GetResourcesByURI(uri, pat, ctx, uma.Timeout) + resourceData, err := uma.provider.GetResourcesByURI(uri, pat, ctx, uma.Timeout, uma.MaxResponseBytes) if err != nil { return nil, err @@ -262,7 +263,7 @@ func (uma *UMA) requestPAT(ctx gocontext.Context, pat *PAT) error { otel.GetTextMapPropagator().Inject(ctx, otel_propagation.HeaderCarrier(req.Header)) // get the response - resp, err := httputil.NewClient(uma.Timeout).Do(req) + resp, err := httputil.NewClient(httputil.WithTimeout(uma.Timeout)).Do(req) if err != nil { return err } @@ -271,7 +272,7 @@ func (uma *UMA) requestPAT(ctx gocontext.Context, pat *PAT) error { }(resp.Body) // parse the pat - if err := json.UnmashalJSONResponse(resp, pat, nil); err != nil { + if err := json.UnmashalJSONResponse(resp, pat, nil, uma.MaxResponseBytes); err != nil { return fmt.Errorf("failed to decode uma pat: %v", err) } diff --git a/pkg/evaluators/metadata/uma_test.go b/pkg/evaluators/metadata/uma_test.go index 38878f134..9440af145 100644 --- a/pkg/evaluators/metadata/uma_test.go +++ b/pkg/evaluators/metadata/uma_test.go @@ -86,3 +86,47 @@ func TestUMACall(t *testing.T) { assert.Equal(t, "["+resourceData+"]", string(data)) assert.NilError(t, err) } + +func TestUMAMaxResponseBytesDiscovery(t *testing.T) { + httpServer := httptest.NewHttpServerMock(umaServerHost, map[string]httptest.HttpServerMockResponseFunc{ + "/uma/.well-known/uma2-configuration": func() httptest.HttpServerMockResponse { + return httptest.HttpServerMockResponse{Status: 200, Body: umaWellKnownConfig} + }, + }) + defer httpServer.Close() + + t.Run("within limit", func(t *testing.T) { + uma := &UMA{ + Endpoint: umaIssuer, + ClientID: "client-id", + ClientSecret: "client-secret", + MaxResponseBytes: int64(len(umaWellKnownConfig) + 100), + } + err := uma.discover(context.TODO()) + assert.NilError(t, err) + assert.Equal(t, umaIssuer, uma.provider.issuer) + }) + + t.Run("exceeds limit causes decode error", func(t *testing.T) { + uma := &UMA{ + Endpoint: umaIssuer, + ClientID: "client-id", + ClientSecret: "client-secret", + MaxResponseBytes: 20, + } + err := uma.discover(context.TODO()) + assert.ErrorContains(t, err, "failed to decode uma provider discovery object") + }) + + t.Run("zero means no limit", func(t *testing.T) { + uma := &UMA{ + Endpoint: umaIssuer, + ClientID: "client-id", + ClientSecret: "client-secret", + MaxResponseBytes: 0, + } + err := uma.discover(context.TODO()) + assert.NilError(t, err) + assert.Equal(t, umaIssuer, uma.provider.issuer) + }) +} diff --git a/pkg/evaluators/metadata/user_info.go b/pkg/evaluators/metadata/user_info.go index 1e2ab7c12..14ba2a1ed 100644 --- a/pkg/evaluators/metadata/user_info.go +++ b/pkg/evaluators/metadata/user_info.go @@ -19,6 +19,7 @@ type UserInfo struct { OpenIdConfig auth.OpenIdConfigStore UserInfoEndpoint string Timeout *int + MaxResponseBytes int64 } func NewUserInfo(openIdConfigStore auth.OpenIdConfigStore, userInfoEndpoint string) *UserInfo { @@ -71,10 +72,10 @@ func (u *UserInfo) Call(pipeline auth.AuthPipeline, parentCtx gocontext.Context) } // fetch user info - return fetchUserInfo(userInfoEndpoint, accessToken, u.Timeout, ctx) + return fetchUserInfo(userInfoEndpoint, accessToken, u.Timeout, u.MaxResponseBytes, ctx) } -func fetchUserInfo(userInfoEndpoint string, accessToken string, timeout *int, ctx gocontext.Context) (interface{}, error) { +func fetchUserInfo(userInfoEndpoint string, accessToken string, timeout *int, maxResponseBytes int64, ctx gocontext.Context) (interface{}, error) { if err := context.CheckContext(ctx); err != nil { return nil, err } @@ -89,7 +90,7 @@ func fetchUserInfo(userInfoEndpoint string, accessToken string, timeout *int, ct otel.GetTextMapPropagator().Inject(ctx, otel_propagation.HeaderCarrier(req.Header)) - resp, err := httputil.NewClient(timeout).Do(req) + resp, err := httputil.NewClient(httputil.WithTimeout(timeout)).Do(req) if err != nil { return nil, err } @@ -97,9 +98,14 @@ func fetchUserInfo(userInfoEndpoint string, accessToken string, timeout *int, ct _ = Body.Close() }(resp.Body) + var bodyReader io.Reader = resp.Body + if maxResponseBytes > 0 { + bodyReader = io.LimitReader(resp.Body, maxResponseBytes) + } + // parse the response var claims map[string]interface{} - err = json.NewDecoder(resp.Body).Decode(&claims) + err = json.NewDecoder(bodyReader).Decode(&claims) if err != nil { return nil, err } diff --git a/pkg/evaluators/metadata/user_info_test.go b/pkg/evaluators/metadata/user_info_test.go index 46d5cce8e..881b7f011 100644 --- a/pkg/evaluators/metadata/user_info_test.go +++ b/pkg/evaluators/metadata/user_info_test.go @@ -16,8 +16,9 @@ import ( ) const ( - authServerHost string = "127.0.0.1:9002" - userInfoClaims string = `{ "sub": "831707be-ef07-4d63-b427-4216309e9897" }` + authServerHost string = "127.0.0.1:9002" + userInfoClaims string = `{ "sub": "831707be-ef07-4d63-b427-4216309e9897" }` + largeUserInfoClaims string = `{ "sub": "831707be-ef07-4d63-b427-4216309e9897", "extra": "` + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + `" }` ) var wellKnownOIDCConfig string = fmt.Sprintf(`{ @@ -61,6 +62,9 @@ func TestMain(m *testing.M) { "/userinfo": func() httptest.HttpServerMockResponse { return httptest.HttpServerMockResponse{Status: 200, Body: userInfoClaims} }, + "/userinfo-large": func() httptest.HttpServerMockResponse { + return httptest.HttpServerMockResponse{Status: 200, Body: largeUserInfoClaims} + }, }) defer authServer.Close() os.Exit(m.Run()) @@ -114,3 +118,27 @@ func TestUserInfoMissingOIDCConfig(t *testing.T) { _, err := ta.userInfo.Call(ta.pipelineMock, ta.ctx) assert.Error(t, err, "missing openid connect configuration") } + +func TestFetchUserInfoMaxResponseBytes(t *testing.T) { + ctx := context.TODO() + endpoint := fmt.Sprintf("http://%s/userinfo-large", authServerHost) + + t.Run("within limit", func(t *testing.T) { + obj, err := fetchUserInfo(endpoint, "token", nil, int64(len(largeUserInfoClaims)+100), ctx) + assert.NilError(t, err) + claims := obj.(map[string]interface{}) + assert.Equal(t, "831707be-ef07-4d63-b427-4216309e9897", claims["sub"]) + }) + + t.Run("exceeds limit causes decode error", func(t *testing.T) { + _, err := fetchUserInfo(endpoint, "token", nil, 30, ctx) + assert.Assert(t, err != nil) + }) + + t.Run("zero means no limit", func(t *testing.T) { + obj, err := fetchUserInfo(endpoint, "token", nil, 0, ctx) + assert.NilError(t, err) + claims := obj.(map[string]interface{}) + assert.Equal(t, "831707be-ef07-4d63-b427-4216309e9897", claims["sub"]) + }) +} diff --git a/pkg/http/request.go b/pkg/http/request.go index 9cfb171c8..b0e89a5e2 100644 --- a/pkg/http/request.go +++ b/pkg/http/request.go @@ -173,42 +173,85 @@ func (t *tracingRoundTripper) RoundTrip(req *http.Request) (*http.Response, erro return t.base.RoundTrip(req) } -// NewClient creates an HTTP client with the specified timeout. -// If timeoutMs is nil or negative, defaults to 5000ms (5 seconds). -// If timeoutMs is 0, no timeout is set (matching Go's http.Client convention). -// If timeoutMs is positive, uses that value as the timeout in milliseconds. -func NewClient(timeoutMs *int) *http.Client { - timeout := 5000 * time.Millisecond // default +type clientOptions struct { + timeoutMs *int + tracingCtx context.Context + maxResponseBytes int64 +} + +// Option configures an HTTP client created by NewClient. +type Option func(*clientOptions) + +// WithTimeout sets the client timeout in milliseconds. +// If nil or negative, defaults to 5000ms. If 0, no timeout is set. +func WithTimeout(timeoutMs *int) Option { + return func(o *clientOptions) { o.timeoutMs = timeoutMs } +} + +// WithTracing enables OpenTelemetry trace propagation on every outbound request. +func WithTracing(ctx context.Context) Option { + return func(o *clientOptions) { o.tracingCtx = ctx } +} + +// WithMaxResponseBytes limits every response body to the given number of bytes +// at the transport level, preventing unbounded memory consumption. +func WithMaxResponseBytes(n int64) Option { + return func(o *clientOptions) { o.maxResponseBytes = n } +} - if timeoutMs != nil && *timeoutMs >= 0 { - timeout = time.Duration(*timeoutMs) * time.Millisecond +// NewClient creates an HTTP client with the given options. +// Without options the client uses the default 5 000 ms timeout. +func NewClient(opts ...Option) *http.Client { + o := &clientOptions{} + for _, fn := range opts { + fn(o) } - return &http.Client{ - Timeout: timeout, + timeout := 5000 * time.Millisecond + if o.timeoutMs != nil && *o.timeoutMs >= 0 { + timeout = time.Duration(*o.timeoutMs) * time.Millisecond } -} -// NewClientWithTracing creates an HTTP client with the specified timeout and trace propagation. -// The trace context from ctx will be injected into all outbound HTTP requests made by this client. -// This is useful for instrumenting HTTP clients used by third-party libraries that create requests -// internally (e.g., go-oidc for OIDC discovery and JWK fetching). -// -// The ctx parameter is used only for trace propagation, not for request cancellation. -// Callers should use context.Background() or a non-cancellable context for the HTTP request lifecycle. -func NewClientWithTracing(ctx context.Context, timeoutMs *int) *http.Client { - baseClient := NewClient(timeoutMs) + client := &http.Client{Timeout: timeout} - // Wrap the transport with trace injection - baseTransport := baseClient.Transport - if baseTransport == nil { - baseTransport = http.DefaultTransport + if o.tracingCtx != nil { + base := client.Transport + if base == nil { + base = http.DefaultTransport + } + var transport http.RoundTripper = &tracingRoundTripper{base: base, ctx: o.tracingCtx} + if o.maxResponseBytes > 0 { + transport = &maxResponseBytesRoundTripper{base: transport, maxBytes: o.maxResponseBytes} + } + client.Transport = transport } - baseClient.Transport = &tracingRoundTripper{ - base: baseTransport, - ctx: ctx, + return client +} + +// maxResponseBytesRoundTripper wraps an http.RoundTripper and limits the size of response bodies. +// This prevents unbounded memory consumption when the HTTP client is used by third-party libraries +// whose response body reads cannot be controlled directly. +type maxResponseBytesRoundTripper struct { + base http.RoundTripper + maxBytes int64 +} + +func (rt *maxResponseBytesRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + resp, err := rt.base.RoundTrip(req) + if err != nil { + return nil, err } + resp.Body = &limitedReadCloser{ + Reader: io.LimitReader(resp.Body, rt.maxBytes), + Closer: resp.Body, + } + return resp, nil +} - return baseClient +// limitedReadCloser combines a limited io.Reader with the original io.Closer, +// so that reads are bounded while the underlying body is still properly closed. +type limitedReadCloser struct { + io.Reader + io.Closer } diff --git a/pkg/http/request_test.go b/pkg/http/request_test.go index f566ac144..a6ed4dfde 100644 --- a/pkg/http/request_test.go +++ b/pkg/http/request_test.go @@ -2,6 +2,7 @@ package http import ( "context" + "io" "net/http" "net/http/httptest" "strings" @@ -332,7 +333,7 @@ func TestNewClient(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - client := NewClient(tt.timeout) + client := NewClient(WithTimeout(tt.timeout)) if client == nil { t.Error("NewClient() returned nil") return @@ -628,7 +629,7 @@ func TestNewRequestWithCredentials(t *testing.T) { } } -func TestNewClientWithTracing(t *testing.T) { +func TestNewClientWithTracingOption(t *testing.T) { // Set up a real tracer and propagator for this test tp := sdktrace.NewTracerProvider() tracer := tp.Tracer("test") @@ -653,7 +654,7 @@ func TestNewClientWithTracing(t *testing.T) { // Create a client with tracing timeout := 5000 - client := NewClientWithTracing(ctx, &timeout) + client := NewClient(WithTimeout(&timeout), WithTracing(ctx)) // Make a request using this client req, err := http.NewRequest("GET", server.URL, nil) @@ -686,7 +687,7 @@ func TestNewClientWithTracing(t *testing.T) { } } -func TestNewClientWithTracing_TimeoutConfiguration(t *testing.T) { +func TestNewClientWithTracingOption_TimeoutConfiguration(t *testing.T) { ctx := context.Background() tests := []struct { @@ -713,7 +714,7 @@ func TestNewClientWithTracing_TimeoutConfiguration(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - client := NewClientWithTracing(ctx, tt.timeoutMs) + client := NewClient(WithTimeout(tt.timeoutMs), WithTracing(ctx)) gotMs := int(client.Timeout.Milliseconds()) if gotMs != tt.wantMs { @@ -732,6 +733,69 @@ func TestNewClientWithTracing_TimeoutConfiguration(t *testing.T) { } } +func TestMaxResponseBytesRoundTripper(t *testing.T) { + t.Run("limits response body", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("this is a long response body that exceeds the limit")) + })) + defer server.Close() + + client := NewClient(WithTracing(context.Background()), WithMaxResponseBytes(10)) + resp, err := client.Get(server.URL) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatalf("unexpected read error: %v", err) + } + if len(body) != 10 { + t.Errorf("expected body length 10, got %d", len(body)) + } + if string(body) != "this is a " { + t.Errorf("expected truncated body %q, got %q", "this is a ", string(body)) + } + }) + + t.Run("zero means no limit", func(t *testing.T) { + fullBody := strings.Repeat("x", 10000) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(fullBody)) + })) + defer server.Close() + + client := NewClient(WithTracing(context.Background()), WithMaxResponseBytes(0)) + resp, err := client.Get(server.URL) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatalf("unexpected read error: %v", err) + } + if len(body) != 10000 { + t.Errorf("expected full body length 10000, got %d", len(body)) + } + }) + + t.Run("transport wraps tracing", func(t *testing.T) { + client := NewClient(WithTracing(context.Background()), WithMaxResponseBytes(100)) + rt, ok := client.Transport.(*maxResponseBytesRoundTripper) + if !ok { + t.Fatal("expected Transport to be *maxResponseBytesRoundTripper") + } + if _, ok := rt.base.(*tracingRoundTripper); !ok { + t.Error("expected inner transport to be *tracingRoundTripper") + } + }) +} + func ptr(i int) *int { return &i } diff --git a/pkg/json/json.go b/pkg/json/json.go index 320621a9a..fdc2ecc6f 100644 --- a/pkg/json/json.go +++ b/pkg/json/json.go @@ -66,11 +66,18 @@ func (v *JSONValue) IsTemplate() bool { return len(curlyBracesForModifiersRegex.FindAllStringSubmatch(v.Pattern, -1)) != len(allCurlyBracesRegex.FindAllStringSubmatch(v.Pattern, -1)) } -// UnmashalJSONResponse unmarshalls a generic HTTP response body into a JSON structure -// Pass optionally a pointer to a byte array to get the raw body of the response object written back -func UnmashalJSONResponse(resp *http.Response, v interface{}, b *[]byte) error { +// UnmashalJSONResponse unmarshalls a generic HTTP response body into a JSON structure. +// Pass optionally a pointer to a byte array to get the raw body of the response object written back. +// When maxResponseBytes > 0, the response body reader is limited to that size to prevent +// unbounded memory consumption from large responses. +func UnmashalJSONResponse(resp *http.Response, v interface{}, b *[]byte, maxResponseBytes ...int64) error { + var bodyReader io.Reader = resp.Body + if len(maxResponseBytes) > 0 && maxResponseBytes[0] > 0 { + bodyReader = io.LimitReader(resp.Body, maxResponseBytes[0]) + } + // read response body - body, err := io.ReadAll(resp.Body) + body, err := io.ReadAll(bodyReader) if err != nil { return fmt.Errorf("unable to read response body: %v", err) } diff --git a/pkg/oauth2/client_credentials.go b/pkg/oauth2/client_credentials.go index 21211c1b5..6a4f8523c 100644 --- a/pkg/oauth2/client_credentials.go +++ b/pkg/oauth2/client_credentials.go @@ -47,7 +47,7 @@ func (c *ClientCredentials) ClientCredentialsToken(ctx context.Context, force bo // Inject custom HTTP client with timeout into context // The oauth2 library will use this client for token requests - httpClient := httputil.NewClient(c.Timeout) + httpClient := httputil.NewClient(httputil.WithTimeout(c.Timeout)) ctx = context.WithValue(ctx, gooauth2.HTTPClient, httpClient) token, err := c.Token(ctx) From fbee1d0781b6b2b367938eda5f49df32349b239d Mon Sep 17 00:00:00 2001 From: Guilherme Cassolato Date: Fri, 21 Aug 2026 11:46:45 +0200 Subject: [PATCH 2/5] Fix lint: check resp.Body.Close error in tests Co-Authored-By: Claude Opus 4.6 Signed-off-by: Guilherme Cassolato --- pkg/http/request_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/http/request_test.go b/pkg/http/request_test.go index a6ed4dfde..c8ec13ad1 100644 --- a/pkg/http/request_test.go +++ b/pkg/http/request_test.go @@ -746,7 +746,7 @@ func TestMaxResponseBytesRoundTripper(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() body, err := io.ReadAll(resp.Body) if err != nil { @@ -773,7 +773,7 @@ func TestMaxResponseBytesRoundTripper(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() body, err := io.ReadAll(resp.Body) if err != nil { From 169a1c573690f212e2cf0fdfe2d13f87fff42f4e Mon Sep 17 00:00:00 2001 From: Guilherme Cassolato Date: Fri, 21 Aug 2026 12:00:31 +0200 Subject: [PATCH 3/5] Address review feedback on maxResponseBytes - Detect OPA policy truncation before compilation to prevent silently evaluating an incomplete policy - Accept maxResponseBytes in UMA constructor so discovery requests are capped from the start - Make WithMaxResponseBytes work independently of WithTracing - Use http.NewRequestWithContext in tests to satisfy noctx linter - Fix docs wording: "can cause" instead of "will cause", drop invalid "set to 0" phrasing since Minimum:=1 prevents it Co-Authored-By: Claude Opus 4.6 Signed-off-by: Guilherme Cassolato --- controllers/auth_config_controller.go | 8 ++++--- docs/features.md | 4 ++-- pkg/evaluators/authorization/opa.go | 4 ++++ pkg/evaluators/authorization/opa_test.go | 27 +++++++++++++++++++++++- pkg/evaluators/metadata/uma.go | 9 ++++---- pkg/evaluators/metadata/uma_test.go | 6 +++--- pkg/http/request.go | 9 ++++---- pkg/http/request_test.go | 12 +++++++++-- 8 files changed, 59 insertions(+), 20 deletions(-) diff --git a/controllers/auth_config_controller.go b/controllers/auth_config_controller.go index c16c16b0e..ed174e99d 100644 --- a/controllers/auth_config_controller.go +++ b/controllers/auth_config_controller.go @@ -506,20 +506,22 @@ func (r *AuthConfigReconciler) translateAuthConfig(ctx context.Context, authConf return nil, err // TODO: Review this error, perhaps we don't need to return an error, just reenqueue. } + var umaMaxResponseBytes int64 + if metadata.Uma.MaxResponseBytes != nil { + umaMaxResponseBytes = *metadata.Uma.MaxResponseBytes + } if uma, err := metadata_evaluators.NewUMAMetadata( ctx, metadata.Uma.Endpoint, string(secret.Data["clientID"]), string(secret.Data["clientSecret"]), + umaMaxResponseBytes, ); err != nil { span.RecordError(err) span.SetStatus(codes.Error, "failed to create UMA metadata evaluator") return nil, err } else { uma.Timeout = metadata.Uma.Timeout - if metadata.Uma.MaxResponseBytes != nil { - uma.MaxResponseBytes = *metadata.Uma.MaxResponseBytes - } translatedMetadata.UMA = uma } diff --git a/docs/features.md b/docs/features.md index cb16d1272..a78e8e757 100644 --- a/docs/features.md +++ b/docs/features.md @@ -442,13 +442,13 @@ The same field is available in: - `spec.metadata.*.userInfo` — OIDC UserInfo - `spec.metadata.*.uma` — UMA resource registry (applies to discovery, PAT requests, and resource queries) -> **Important:** When a response exceeds the limit, the body is truncated to the specified size. For `application/json` responses, this truncation produces malformed JSON that will cause a decode error. As a result, the evaluator will fail and the corresponding piece of the auth pipeline will not be available. +> **Important:** When a response exceeds the limit, the body is truncated to the specified size. For `application/json` responses, this truncation produces malformed JSON that can cause a decode error. As a result, the evaluator will fail and the corresponding piece of the auth pipeline will not be available. > > Depending on how downstream authorization policies handle missing metadata, the outcome can vary: > - **Policies that deny access when metadata is absent**: the request will be denied. This is the safer default. > - **Policies that fall back to granting access on the absence of metadata**: truncation may cause the policy to inadvertently allow a request that should have been denied. Review your authorization policies to ensure they do not assume missing metadata means "allowed." > -> When `maxResponseBytes` is omitted or set to `0`, no limit is applied and the full response body is read. +> When `maxResponseBytes` is omitted, no limit is applied and the full response body is read. ### OIDC UserInfo ([`metadata.userInfo`](https://pkg.go.dev/github.com/kuadrant/authorino/api/v1beta3?utm_source=gopls#UserInfoMetadataSpec)) diff --git a/pkg/evaluators/authorization/opa.go b/pkg/evaluators/authorization/opa.go index 278429f9b..3862ef9b8 100644 --- a/pkg/evaluators/authorization/opa.go +++ b/pkg/evaluators/authorization/opa.go @@ -274,6 +274,10 @@ func (ext *OPAExternalSource) downloadRegoDataFromUrl(ctx context.Context) (stri return "", fmt.Errorf("unable to read response body: %v", err) } + if ext.MaxResponseBytes > 0 && int64(len(body)) >= ext.MaxResponseBytes { + return "", fmt.Errorf("response body truncated at %d bytes; refusing to compile a potentially incomplete policy", ext.MaxResponseBytes) + } + if resp.StatusCode != http.StatusOK { return "", fmt.Errorf("%s: %s", resp.Status, body) } diff --git a/pkg/evaluators/authorization/opa_test.go b/pkg/evaluators/authorization/opa_test.go index 6dbfd73c0..bc5969f38 100644 --- a/pkg/evaluators/authorization/opa_test.go +++ b/pkg/evaluators/authorization/opa_test.go @@ -279,7 +279,32 @@ func TestOPAExternalUrlMaxResponseBytes(t *testing.T) { opa, err := NewOPAAuthorization("test-opa-maxbytes-truncated", "", externalSource, false, opaParser.RegoV1, 0, context.TODO()) - assert.Assert(t, err != nil, "expected error due to truncated Rego policy") + assert.ErrorContains(t, err, "response body truncated") + assert.Assert(t, opa == nil) + }) + + t.Run("truncated valid rego prefix is rejected", func(t *testing.T) { + // The full policy has two rules; truncating to the first rule yields valid Rego + // that compiles fine but silently drops the deny logic. The truncation check must + // catch this before compilation. + fullPolicy := `allow := true +deny := true` + extHttpMetadataServer := httptest.NewHttpServerMock(opaExtHttpServerMockAddr, map[string]httptest.HttpServerMockResponseFunc{ + "/rego": func() httptest.HttpServerMockResponse { + return httptest.HttpServerMockResponse{Status: 200, Body: fullPolicy} + }, + }) + defer extHttpMetadataServer.Close() + + externalSource := &OPAExternalSource{ + Endpoint: "http://" + opaExtHttpServerMockAddr + "/rego", + AuthCredentials: auth.NewAuthCredential("", ""), + MaxResponseBytes: int64(len("allow := true\n")), + } + + opa, err := NewOPAAuthorization("test-opa-maxbytes-valid-prefix", "", externalSource, false, opaParser.RegoV1, 0, context.TODO()) + + assert.ErrorContains(t, err, "response body truncated") assert.Assert(t, opa == nil) }) diff --git a/pkg/evaluators/metadata/uma.go b/pkg/evaluators/metadata/uma.go index 81229a27e..6b00aed50 100644 --- a/pkg/evaluators/metadata/uma.go +++ b/pkg/evaluators/metadata/uma.go @@ -149,11 +149,12 @@ func (pat *PAT) Get(rawurl string, ctx gocontext.Context, v interface{}, timeout return json.UnmashalJSONResponse(resp, &v, nil, maxResponseBytes...) } -func NewUMAMetadata(ctx gocontext.Context, endpoint string, clientID string, clientSecret string) (*UMA, error) { +func NewUMAMetadata(ctx gocontext.Context, endpoint string, clientID string, clientSecret string, maxResponseBytes int64) (*UMA, error) { uma := &UMA{ - Endpoint: endpoint, - ClientID: clientID, - ClientSecret: clientSecret, + Endpoint: endpoint, + ClientID: clientID, + ClientSecret: clientSecret, + MaxResponseBytes: maxResponseBytes, } if err := uma.discover(ctx); err != nil { return nil, err diff --git a/pkg/evaluators/metadata/uma_test.go b/pkg/evaluators/metadata/uma_test.go index 9440af145..2279bf4f5 100644 --- a/pkg/evaluators/metadata/uma_test.go +++ b/pkg/evaluators/metadata/uma_test.go @@ -37,7 +37,7 @@ func TestNewUMAMetadata(t *testing.T) { }) defer httpServer.Close() - uma, err := NewUMAMetadata(context.TODO(), umaIssuer, "client-id", "client-secret") + uma, err := NewUMAMetadata(context.TODO(), umaIssuer, "client-id", "client-secret", 0) assert.NilError(t, err) assert.Equal(t, umaIssuer, uma.provider.issuer) @@ -49,7 +49,7 @@ func TestUMAMetadataFailToDecodeConfig(t *testing.T) { }) defer httpServer.Close() - uma, err := NewUMAMetadata(context.TODO(), umaIssuer, "client-id", "client-secret") + uma, err := NewUMAMetadata(context.TODO(), umaIssuer, "client-id", "client-secret", 0) assert.ErrorContains(t, err, "failed to decode uma provider discovery object") assert.Check(t, uma == nil) @@ -78,7 +78,7 @@ func TestUMACall(t *testing.T) { request := &envoy_auth.AttributeContext_HttpRequest{Path: "/someresource"} pipelineMock.EXPECT().GetHttp().Return(request) - uma, _ := NewUMAMetadata(context.TODO(), umaIssuer, "client-id", "client-secret") + uma, _ := NewUMAMetadata(context.TODO(), umaIssuer, "client-id", "client-secret", 0) obj, err := uma.Call(pipelineMock, context.TODO()) diff --git a/pkg/http/request.go b/pkg/http/request.go index b0e89a5e2..74437ab49 100644 --- a/pkg/http/request.go +++ b/pkg/http/request.go @@ -214,12 +214,11 @@ func NewClient(opts ...Option) *http.Client { client := &http.Client{Timeout: timeout} - if o.tracingCtx != nil { - base := client.Transport - if base == nil { - base = http.DefaultTransport + if o.tracingCtx != nil || o.maxResponseBytes > 0 { + transport := http.DefaultTransport + if o.tracingCtx != nil { + transport = &tracingRoundTripper{base: transport, ctx: o.tracingCtx} } - var transport http.RoundTripper = &tracingRoundTripper{base: base, ctx: o.tracingCtx} if o.maxResponseBytes > 0 { transport = &maxResponseBytesRoundTripper{base: transport, maxBytes: o.maxResponseBytes} } diff --git a/pkg/http/request_test.go b/pkg/http/request_test.go index c8ec13ad1..4c48a3d8e 100644 --- a/pkg/http/request_test.go +++ b/pkg/http/request_test.go @@ -742,7 +742,11 @@ func TestMaxResponseBytesRoundTripper(t *testing.T) { defer server.Close() client := NewClient(WithTracing(context.Background()), WithMaxResponseBytes(10)) - resp, err := client.Get(server.URL) + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, server.URL, nil) + if err != nil { + t.Fatalf("unexpected error creating request: %v", err) + } + resp, err := client.Do(req) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -769,7 +773,11 @@ func TestMaxResponseBytesRoundTripper(t *testing.T) { defer server.Close() client := NewClient(WithTracing(context.Background()), WithMaxResponseBytes(0)) - resp, err := client.Get(server.URL) + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, server.URL, nil) + if err != nil { + t.Fatalf("unexpected error creating request: %v", err) + } + resp, err := client.Do(req) if err != nil { t.Fatalf("unexpected error: %v", err) } From 1dbde3904c2340fee6dabf113ed953a68a289664 Mon Sep 17 00:00:00 2001 From: Guilherme Cassolato Date: Fri, 21 Aug 2026 12:35:35 +0200 Subject: [PATCH 4/5] Fix API field comments: drop invalid "or set to 0" phrasing Minimum:=1 prevents setting 0, so the comment should say "If omitted, no limit is applied" without mentioning 0. Co-Authored-By: Claude Opus 4.6 Signed-off-by: Guilherme Cassolato --- api/v1beta3/auth_config_types.go | 10 +++++----- install/crd/authorino.kuadrant.io_authconfigs.yaml | 14 +++++++------- install/manifests.yaml | 14 +++++++------- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/api/v1beta3/auth_config_types.go b/api/v1beta3/auth_config_types.go index a703a8ac0..f1991f248 100644 --- a/api/v1beta3/auth_config_types.go +++ b/api/v1beta3/auth_config_types.go @@ -401,7 +401,7 @@ type JwtAuthenticationSpec struct { // When set, response body readers will be limited to this size, preventing // unbounded memory consumption from unexpectedly large responses. // If the response exceeds this limit, the truncation will cause a decode error. - // If omitted or set to 0, no limit is applied. + // If omitted, no limit is applied. // +optional // +kubebuilder:validation:Minimum:=1 MaxResponseBytes *int64 `json:"maxResponseBytes,omitempty"` @@ -434,7 +434,7 @@ type OAuth2TokenIntrospectionSpec struct { // When set, the response body reader will be limited to this size, preventing // unbounded memory consumption from unexpectedly large responses. // If the response is JSON and exceeds this limit, the truncation will cause a decode error. - // If omitted or set to 0, no limit is applied. + // If omitted, no limit is applied. // +optional // +kubebuilder:validation:Minimum:=1 MaxResponseBytes *int64 `json:"maxResponseBytes,omitempty"` @@ -623,7 +623,7 @@ type HttpEndpointSpec struct { // When set, the response body reader will be limited to this size, preventing // unbounded memory consumption from unexpectedly large responses. // If the response is JSON and exceeds this limit, the truncation will cause a decode error. - // If omitted or set to 0, no limit is applied. + // If omitted, no limit is applied. // +optional // +kubebuilder:validation:Minimum:=1 MaxResponseBytes *int64 `json:"maxResponseBytes,omitempty"` @@ -698,7 +698,7 @@ type UserInfoMetadataSpec struct { // When set, the response body reader will be limited to this size, preventing // unbounded memory consumption from unexpectedly large responses. // If the response exceeds this limit, the truncation will cause a decode error. - // If omitted or set to 0, no limit is applied. + // If omitted, no limit is applied. // +optional // +kubebuilder:validation:Minimum:=1 MaxResponseBytes *int64 `json:"maxResponseBytes,omitempty"` @@ -727,7 +727,7 @@ type UmaMetadataSpec struct { // When set, response body readers will be limited to this size, preventing // unbounded memory consumption from unexpectedly large responses. // If the response exceeds this limit, the truncation will cause a decode error. - // If omitted or set to 0, no limit is applied. + // If omitted, no limit is applied. // +optional // +kubebuilder:validation:Minimum:=1 MaxResponseBytes *int64 `json:"maxResponseBytes,omitempty"` diff --git a/install/crd/authorino.kuadrant.io_authconfigs.yaml b/install/crd/authorino.kuadrant.io_authconfigs.yaml index 26bffc5d3..948e05f7a 100644 --- a/install/crd/authorino.kuadrant.io_authconfigs.yaml +++ b/install/crd/authorino.kuadrant.io_authconfigs.yaml @@ -263,7 +263,7 @@ spec: When set, response body readers will be limited to this size, preventing unbounded memory consumption from unexpectedly large responses. If the response exceeds this limit, the truncation will cause a decode error. - If omitted or set to 0, no limit is applied. + If omitted, no limit is applied. format: int64 minimum: 1 type: integer @@ -335,7 +335,7 @@ spec: When set, the response body reader will be limited to this size, preventing unbounded memory consumption from unexpectedly large responses. If the response is JSON and exceeds this limit, the truncation will cause a decode error. - If omitted or set to 0, no limit is applied. + If omitted, no limit is applied. format: int64 minimum: 1 type: integer @@ -933,7 +933,7 @@ spec: When set, the response body reader will be limited to this size, preventing unbounded memory consumption from unexpectedly large responses. If the response is JSON and exceeds this limit, the truncation will cause a decode error. - If omitted or set to 0, no limit is applied. + If omitted, no limit is applied. format: int64 minimum: 1 type: integer @@ -1508,7 +1508,7 @@ spec: When set, the response body reader will be limited to this size, preventing unbounded memory consumption from unexpectedly large responses. If the response is JSON and exceeds this limit, the truncation will cause a decode error. - If omitted or set to 0, no limit is applied. + If omitted, no limit is applied. format: int64 minimum: 1 type: integer @@ -1882,7 +1882,7 @@ spec: When set, the response body reader will be limited to this size, preventing unbounded memory consumption from unexpectedly large responses. If the response is JSON and exceeds this limit, the truncation will cause a decode error. - If omitted or set to 0, no limit is applied. + If omitted, no limit is applied. format: int64 minimum: 1 type: integer @@ -2057,7 +2057,7 @@ spec: When set, response body readers will be limited to this size, preventing unbounded memory consumption from unexpectedly large responses. If the response exceeds this limit, the truncation will cause a decode error. - If omitted or set to 0, no limit is applied. + If omitted, no limit is applied. format: int64 minimum: 1 type: integer @@ -2087,7 +2087,7 @@ spec: When set, the response body reader will be limited to this size, preventing unbounded memory consumption from unexpectedly large responses. If the response exceeds this limit, the truncation will cause a decode error. - If omitted or set to 0, no limit is applied. + If omitted, no limit is applied. format: int64 minimum: 1 type: integer diff --git a/install/manifests.yaml b/install/manifests.yaml index 68354cdc4..a3d599471 100644 --- a/install/manifests.yaml +++ b/install/manifests.yaml @@ -298,7 +298,7 @@ spec: When set, response body readers will be limited to this size, preventing unbounded memory consumption from unexpectedly large responses. If the response exceeds this limit, the truncation will cause a decode error. - If omitted or set to 0, no limit is applied. + If omitted, no limit is applied. format: int64 minimum: 1 type: integer @@ -370,7 +370,7 @@ spec: When set, the response body reader will be limited to this size, preventing unbounded memory consumption from unexpectedly large responses. If the response is JSON and exceeds this limit, the truncation will cause a decode error. - If omitted or set to 0, no limit is applied. + If omitted, no limit is applied. format: int64 minimum: 1 type: integer @@ -992,7 +992,7 @@ spec: When set, the response body reader will be limited to this size, preventing unbounded memory consumption from unexpectedly large responses. If the response is JSON and exceeds this limit, the truncation will cause a decode error. - If omitted or set to 0, no limit is applied. + If omitted, no limit is applied. format: int64 minimum: 1 type: integer @@ -1615,7 +1615,7 @@ spec: When set, the response body reader will be limited to this size, preventing unbounded memory consumption from unexpectedly large responses. If the response is JSON and exceeds this limit, the truncation will cause a decode error. - If omitted or set to 0, no limit is applied. + If omitted, no limit is applied. format: int64 minimum: 1 type: integer @@ -1989,7 +1989,7 @@ spec: When set, the response body reader will be limited to this size, preventing unbounded memory consumption from unexpectedly large responses. If the response is JSON and exceeds this limit, the truncation will cause a decode error. - If omitted or set to 0, no limit is applied. + If omitted, no limit is applied. format: int64 minimum: 1 type: integer @@ -2164,7 +2164,7 @@ spec: When set, response body readers will be limited to this size, preventing unbounded memory consumption from unexpectedly large responses. If the response exceeds this limit, the truncation will cause a decode error. - If omitted or set to 0, no limit is applied. + If omitted, no limit is applied. format: int64 minimum: 1 type: integer @@ -2194,7 +2194,7 @@ spec: When set, the response body reader will be limited to this size, preventing unbounded memory consumption from unexpectedly large responses. If the response exceeds this limit, the truncation will cause a decode error. - If omitted or set to 0, no limit is applied. + If omitted, no limit is applied. format: int64 minimum: 1 type: integer From 9d0ce475d7641f7c675132651c44d8da8ebfb22d Mon Sep 17 00:00:00 2001 From: Guilherme Cassolato Date: Fri, 21 Aug 2026 13:35:11 +0200 Subject: [PATCH 5/5] Pass timeout to UMA constructor; add no-tracing limit test The UMA constructor now receives timeout so discovery honours the configured value instead of always falling back to the default. Also adds test coverage for WithMaxResponseBytes without WithTracing. Co-Authored-By: Claude Opus 4.6 Signed-off-by: Guilherme Cassolato --- controllers/auth_config_controller.go | 2 +- pkg/evaluators/metadata/uma.go | 3 ++- pkg/evaluators/metadata/uma_test.go | 6 +++--- pkg/http/request_test.go | 27 +++++++++++++++++++++++++++ 4 files changed, 33 insertions(+), 5 deletions(-) diff --git a/controllers/auth_config_controller.go b/controllers/auth_config_controller.go index ed174e99d..604aae1d3 100644 --- a/controllers/auth_config_controller.go +++ b/controllers/auth_config_controller.go @@ -515,13 +515,13 @@ func (r *AuthConfigReconciler) translateAuthConfig(ctx context.Context, authConf metadata.Uma.Endpoint, string(secret.Data["clientID"]), string(secret.Data["clientSecret"]), + metadata.Uma.Timeout, umaMaxResponseBytes, ); err != nil { span.RecordError(err) span.SetStatus(codes.Error, "failed to create UMA metadata evaluator") return nil, err } else { - uma.Timeout = metadata.Uma.Timeout translatedMetadata.UMA = uma } diff --git a/pkg/evaluators/metadata/uma.go b/pkg/evaluators/metadata/uma.go index 6b00aed50..931257200 100644 --- a/pkg/evaluators/metadata/uma.go +++ b/pkg/evaluators/metadata/uma.go @@ -149,11 +149,12 @@ func (pat *PAT) Get(rawurl string, ctx gocontext.Context, v interface{}, timeout return json.UnmashalJSONResponse(resp, &v, nil, maxResponseBytes...) } -func NewUMAMetadata(ctx gocontext.Context, endpoint string, clientID string, clientSecret string, maxResponseBytes int64) (*UMA, error) { +func NewUMAMetadata(ctx gocontext.Context, endpoint string, clientID string, clientSecret string, timeout *int, maxResponseBytes int64) (*UMA, error) { uma := &UMA{ Endpoint: endpoint, ClientID: clientID, ClientSecret: clientSecret, + Timeout: timeout, MaxResponseBytes: maxResponseBytes, } if err := uma.discover(ctx); err != nil { diff --git a/pkg/evaluators/metadata/uma_test.go b/pkg/evaluators/metadata/uma_test.go index 2279bf4f5..9590e717c 100644 --- a/pkg/evaluators/metadata/uma_test.go +++ b/pkg/evaluators/metadata/uma_test.go @@ -37,7 +37,7 @@ func TestNewUMAMetadata(t *testing.T) { }) defer httpServer.Close() - uma, err := NewUMAMetadata(context.TODO(), umaIssuer, "client-id", "client-secret", 0) + uma, err := NewUMAMetadata(context.TODO(), umaIssuer, "client-id", "client-secret", nil, 0) assert.NilError(t, err) assert.Equal(t, umaIssuer, uma.provider.issuer) @@ -49,7 +49,7 @@ func TestUMAMetadataFailToDecodeConfig(t *testing.T) { }) defer httpServer.Close() - uma, err := NewUMAMetadata(context.TODO(), umaIssuer, "client-id", "client-secret", 0) + uma, err := NewUMAMetadata(context.TODO(), umaIssuer, "client-id", "client-secret", nil, 0) assert.ErrorContains(t, err, "failed to decode uma provider discovery object") assert.Check(t, uma == nil) @@ -78,7 +78,7 @@ func TestUMACall(t *testing.T) { request := &envoy_auth.AttributeContext_HttpRequest{Path: "/someresource"} pipelineMock.EXPECT().GetHttp().Return(request) - uma, _ := NewUMAMetadata(context.TODO(), umaIssuer, "client-id", "client-secret", 0) + uma, _ := NewUMAMetadata(context.TODO(), umaIssuer, "client-id", "client-secret", nil, 0) obj, err := uma.Call(pipelineMock, context.TODO()) diff --git a/pkg/http/request_test.go b/pkg/http/request_test.go index 4c48a3d8e..44b2ce1e5 100644 --- a/pkg/http/request_test.go +++ b/pkg/http/request_test.go @@ -802,6 +802,33 @@ func TestMaxResponseBytesRoundTripper(t *testing.T) { t.Error("expected inner transport to be *tracingRoundTripper") } }) + + t.Run("limits without tracing", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("this is a long response body that exceeds the limit")) + })) + defer server.Close() + + client := NewClient(WithMaxResponseBytes(10)) + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, server.URL, nil) + if err != nil { + t.Fatalf("unexpected error creating request: %v", err) + } + resp, err := client.Do(req) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + defer func() { _ = resp.Body.Close() }() + + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatalf("unexpected read error: %v", err) + } + if len(body) != 10 { + t.Errorf("expected body length 10, got %d", len(body)) + } + }) } func ptr(i int) *int {