Skip to content

[TT-16132] Granular access control conditions on query parameters, headers and body - #8652

Merged
mativm02 merged 7 commits into
masterfrom
TT-16132/granular-access-conditions
Sep 7, 2026
Merged

[TT-16132] Granular access control conditions on query parameters, headers and body#8652
mativm02 merged 7 commits into
masterfrom
TT-16132/granular-access-conditions

Conversation

@mativm02

@mativm02 mativm02 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Description

Takes the TT-17881 spike PoC to something shippable. The first two commits are the PoC as it stands on #8623 (draft); the third is the production work.

Conditions are validated where they are accepted. Access conditions fail closed — a condition the Gateway cannot evaluate denies every request the entry would otherwise have granted. user.AccessCondition.Validate reports those cases (an unreadable match_rx, a condition that configures no options, an unrecognised on), and the key and policy APIs reject them at write time. The rules live in the user package rather than in the Gateway so the Dashboard can apply exactly the same ones, instead of the two drifting apart on what counts as valid.

The CoProcess bridge carries conditions. The Protocol Buffer session carried only the URL and the methods, so a custom auth plugin's session came back without its conditions. A spec that loses its conditions grants strictly more than the one that went out, so this is a correctness problem rather than a missing feature. AccessSpec now carries them in both directions.

🚨 A pre-existing bug fixed while in there. TykSessionState allocated the allowed-URL slice by length and then appended to it, padding every access definition with as many zero-valued specs as it had real ones. An empty URL is an empty regex, which matches every path — so those padding entries handed a custom-auth session the whole API, regardless of what its allowed_urls said. This predates conditions entirely and affects any gRPC, Python or Lua custom auth plugin that returns allowed_urls. It is fixed and covered by a test here, but it deserves its own ticket and a look from whoever owns security response; it should not ship quietly inside a feature PR.

copyAllowedURLs dropped conditions too. It has no production caller today, but a copy that silently widens access is a trap for whoever wires it up next.

Two questions the refinement left open, answered here

How a condition says "this parameter must not be supplied." An empty match_rx places no constraint on the value, so reversing it can only be about presence:

{ "query_val_matches": { "persnbr": { "reverse": true } } }

No new API surface, and the PoC's {"match_rx": ".*", "reverse": true} still works. Happy to revisit if reviewers prefer an explicit field — but it has to be settled before release, since customers will write policies against whatever ships.

An unconditional and a conditional entry for the same endpoint. The unconditional one grants access. allowed_urls is a list of grants and grants are additive, so combining policies can never tighten what another policy already allowed. Pinned by a test in both orderings rather than left to the reader.

Notes for review

  • The evaluator borrows apidef.RoutingTriggerOptions as its configuration shape, which couples an access decision to a type maintained for URL Rewrite. An option the evaluator did not know about would be accepted, appear to constrain the request, and then be ignored — access somebody thought they had restricted. A test now fails if that type gains a field.
  • checkTriggerOptions (commit 1) is a behaviour-preserving extraction from mw_url_rewrite.go. It is no longer needed by this feature, since the evaluator is standalone. Happy to split it into its own PR to keep the security-relevant change small — say the word.
  • coprocess_session_state.pb.go had struct fields its own descriptor did not declare, so the checked-in bindings had drifted from the .proto. Regenerating made it consistent, which accounts for most of that diff.
  • The Python, Ruby, C++ and Java bindings are deliberately not regenerated. They are already stale by an unrelated release, and regenerating with a protoc newer than the one they were built with risks breaking plugin runtimes. proto3 preserves unknown fields, so conditions survive a plugin round trip regardless — plugins just cannot read them yet. This wants a separate pass with a pinned toolchain.

Related Issue

https://tyktech.atlassian.net/browse/TT-16132 (spike: https://tyktech.atlassian.net/browse/TT-17881)

Supersedes the draft PoC in #8623 — that one can be closed once this is reviewed.

Companion Dashboard PR: TykTechnologies/tyk-analytics#6170
Dashboard UI is tracked separately in TT-18101.

Motivation and Context

Tyk decides access on path and method alone. A single endpoint such as /connections means "my own connections" when called bare, and "somebody else's" when called with a persnbr parameter — the same URL and method, two very different levels of access.

The customer cannot express "the public application may call this endpoint, but only without persnbr", so they maintain four endpoints for one operation and redirect them internally back onto the real one — a workaround that hits TT-6699, where the query parameters get dropped in transit. Apigee and Azure API Management both support this today.

How This Has Been Tested

Unit tests for the evaluator and the validator; end-to-end tests through a running Gateway for the customer scenario (bare request allowed, ?persnbr= / ?agreenbr= / ?account= refused, unrelated parameters still allowed), for grant precedence in both orderings, and for a stored uncompilable pattern denying rather than erroring.

Because the middleware runs after authentication against the normalised session, conditions are auth-agnostic by construction. That is also exactly the kind of argument that stops being true without anyone noticing, so there is now a test running identical conditions through an auth-token session and a JWT-derived one and asserting the same allow/refuse results.

URL Rewrite, policy, key-handler, CoProcess and session suites all pass; the earlier manual PoC verification (25 end-to-end scenarios across auth token and JWT) still holds.

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 — written and ready on a docs branch, held back until the API shape is agreed here. Release notes still need the 5.16 section, which does not exist yet.
  • I explained why this PR updates go.mod in detail with reasoning why it's required — it does not.
  • I would like a code coverage CI quality gate exception and have explained why

Ticket Details

TT-16132
Status Merge
Summary Conditional Granular Access Control for keys and policies

Generated at: 2026-09-07 12:02:58

@github-actions

github-actions Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

🎯 Recommended Merge Targets

Based on JIRA ticket TT-16132: Conditional Granular Access Control for keys and policies

Fix Version: Tyk 5.16.0

⚠️ Warning: Expected release branches not found in repository

Required:

  • master - No matching release branches found. Fix will be included in future releases.

📋 Workflow

  1. Merge this PR to master first

@probelabs

probelabs Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

This pull request introduces a significant enhancement to Tyk's authorization capabilities by enabling granular access control conditions based on request headers, query parameters, and the request body. This allows for more sophisticated security policies, such as restricting access to an endpoint based on the presence or absence of a specific parameter.

Key changes include:

  • New Access Conditions: The AccessSpec in policies and keys now supports a list of Conditions. Each condition can require that all or any of a set of rules match the incoming request.
  • Fail-Closed Evaluation: A new evaluation engine is added to the GranularAccessMiddleware. It is designed to be "fail-closed," meaning any misconfigured or un-evaluatable condition (e.g., an invalid regular expression) will result in access being denied.
  • API Validation: The gateway's management API now validates access conditions when keys or policies are created or updated. This prevents the storage of invalid rules that would cause runtime issues. The validation logic is centralized in the user package for reusability (e.g., by the Tyk Dashboard).
  • CoProcess Bridge Update: The Protocol Buffers definition for the session state has been extended to include access conditions. This ensures that custom authentication plugins (gRPC, Python, Lua) can receive and return these conditions, preventing policies from being inadvertently weakened.
  • Critical Bug Fix: A pre-existing security vulnerability in the CoProcess bridge has been fixed. Previously, incorrect slice allocation could grant a custom-auth session full API access, regardless of its allowed_urls. This fix is included in the PR.
  • Policy Merging: The logic for merging multiple policies has been updated to be condition-aware, ensuring that grants are combined correctly without unintentionally restricting access.

Files Changed Analysis

The changes are spread across several key areas of the gateway:

  • coprocess/: The protobuf schema (coprocess_session_state.proto) and generated Go code are updated to include AccessCondition and related structures, enabling them to be passed through the CoProcess gRPC bridge.
  • gateway/mw_granular_access_conditions.go (New): This new file contains the core logic for evaluating access conditions against an incoming HTTP request.
  • gateway/mw_granular_access.go: The existing middleware is updated to invoke the new condition evaluation logic.
  • user/access_conditions.go (New): This file centralizes the validation logic for access conditions, ensuring regex patterns are valid and the structure is sound before being saved.
  • gateway/api.go & gateway/policy.go: These files are modified to call the new validation logic from user/access_conditions.go at the API boundary for keys and policies.
  • gateway/coprocess_helpers.go: This file is updated to correctly translate conditions between the internal session state and the protobuf representation. It also contains the critical fix for the allowed_urls allocation bug.
  • internal/policy/util.go: The policy merging utility is updated to treat specs for the same URL but with different conditions as distinct, ensuring correct union of permissions.
  • user/session.go: The core AccessSpec struct is updated to include the new Conditions field.
  • *_test.go files: Extensive unit and end-to-end tests have been added to cover the new evaluator, validation rules, policy merging, and to pin down expected behaviors for various scenarios.

Architecture & Impact Assessment

  • What this PR accomplishes: It fundamentally enhances Tyk's authorization model, moving beyond simple path/method matching to a more flexible attribute-based access control (ABAC) system. This allows operators to implement much finer-grained security policies on a single API endpoint.
  • Key technical changes introduced:
    1. A new AccessCondition data structure is integrated into the user.AccessSpec.
    2. A dedicated, fail-closed condition evaluator is introduced in the request processing pipeline.
    3. Pre-emptive validation is added to the management API for keys and policies.
    4. The CoProcess data contract is extended for custom plugins.
    5. A critical security bug in CoProcess session handling is resolved.
  • Affected system components:
    • Authorization Engine: The core access control logic is directly modified.
    • Policy Management: The structure of policies and the logic for merging them are updated.
    • Management API: Endpoints for keys (/tyk/keys) and policies (/tyk/policies) have new validation rules.
    • CoProcess Plugins: The data contract is updated. Existing plugins will continue to function but will not be aware of the new conditions until they are updated.

Request Flow with Granular Access Conditions

graph TD
    A[Request In] --> B(Auth Middleware);
    B --> C{Session Normalized};
    C --> D(GranularAccessMiddleware);
    D --> E{URL & Method Match?};
    E -- Yes --> F{Access Conditions Exist?};
    E -- No --> G[403 Forbidden];
    F -- No --> H[200 OK / Allowed];
    F -- Yes --> I(Evaluate Conditions on Headers, Query, Body);
    I -- All Match --> H;
    I -- Any Fail --> G;
Loading

Scope Discovery & Context Expansion

  • Tyk Dashboard: This change necessitates a corresponding update in the Tyk Dashboard to provide a UI for managing these new conditions. The shared validation logic in the user package is designed to support this.
  • Custom Plugins (CoProcess): Developers of custom authentication plugins will need to regenerate their client code from the new .proto file to read or manipulate access conditions. The change is non-breaking for existing plugins, which will ignore the new fields.
  • Security: The PR fixes a significant pre-existing security vulnerability in the CoProcess bridge. This fix is a critical improvement and should be considered for backporting to relevant versions as it affects any user of gRPC, Python, or Lua custom auth plugins.
Metadata
  • Review Effort: 4 / 5
  • Primary Label: feature

Powered by Visor from Probelabs

Last updated: 2026-09-07T12:04:38.093Z | Triggered by: pr_updated | Commit: 7cdee18

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

@probelabs

probelabs Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Security Issues (1)

Severity Location Issue
🟡 Warning gateway/mw_granular_access_conditions.go:342
The `payload_matches` condition reads the entire request body into memory using `io.ReadAll` without an explicit size limit. This can lead to a Denial of Service (DoS) vulnerability if an attacker sends a request with a very large body, causing excessive memory consumption. The gateway's `max_request_body_size` setting, which mitigates this, is not enabled by default.
💡 SuggestionTo mitigate this risk, the documentation for this feature should strongly warn users to configure `max_request_body_size`. For defense-in-depth, consider using an `io.LimitReader` within `readRequestBody` to enforce a reasonable default maximum size (e.g., 10MB) when the global configuration is not set. This would prevent unbounded memory allocation by default.

Architecture Issues (1)

Severity Location Issue
🟡 Warning user/session.go:56
The `AccessCondition` struct directly uses `apidef.RoutingTriggerOptions`. While this is a pragmatic reuse of an existing data structure, it creates a tight coupling between the access control feature and the URL rewrite feature. A future change to `RoutingTriggerOptions` for URL rewriting purposes could inadvertently affect security-critical access control logic if not handled carefully.
💡 SuggestionTo mitigate the risk of this coupling, consider adding a prominent comment to the `apidef.RoutingTriggerOptions` struct definition itself, warning developers that it is used in a security-critical context by the granular access control middleware and that any changes must be reflected in the access condition validator and evaluator. The test `TestAccessConditions_EveryTriggerOptionIsEvaluated` is an excellent safeguard, and a comment would complement it by providing upfront context to developers before they make changes.

Performance Issues (2)

Severity Location Issue
🟡 Warning gateway/mw_granular_access_conditions.go:240
The `matchPayload` function reads the entire request body into memory to perform regex matching. Although the evaluation logic is optimized to perform this check last and only when necessary, it can lead to significant memory usage and latency for requests with large bodies, especially if the `max_request_body_size` gateway setting is not configured.
💡 SuggestionThis is an inherent trade-off for this feature. The implementation is robust in that it delays this expensive operation. Ensure that this behavior and its performance implications are clearly documented for operators, strongly recommending the use of `max_request_body_size` as a safeguard when using payload-based access conditions.
🟡 Warning internal/policy/util.go:49
The `allowedURLKey` function uses `json.Marshal` to generate a stable key for merging access specs with conditions. JSON marshaling relies on reflection and can be resource-intensive, potentially impacting the performance of policy merging, which occurs during session creation or updates. This could become a bottleneck for keys associated with many policies that make heavy use of granular conditions.
💡 SuggestionWhile the added benchmark is a good step to monitor this, consider exploring more performant serialization or hashing methods for generating the key if profiling indicates this is a hotspot. For example, a custom function that iterates the struct fields and writes to a hash function directly could avoid the overhead of reflection. For the initial implementation, the current approach is acceptable for its correctness and simplicity.

Security Issues (1)

Severity Location Issue
🟡 Warning gateway/mw_granular_access_conditions.go:342
The `payload_matches` condition reads the entire request body into memory using `io.ReadAll` without an explicit size limit. This can lead to a Denial of Service (DoS) vulnerability if an attacker sends a request with a very large body, causing excessive memory consumption. The gateway's `max_request_body_size` setting, which mitigates this, is not enabled by default.
💡 SuggestionTo mitigate this risk, the documentation for this feature should strongly warn users to configure `max_request_body_size`. For defense-in-depth, consider using an `io.LimitReader` within `readRequestBody` to enforce a reasonable default maximum size (e.g., 10MB) when the global configuration is not set. This would prevent unbounded memory allocation by default.
\n\n ### Architecture Issues (1)
Severity Location Issue
🟡 Warning user/session.go:56
The `AccessCondition` struct directly uses `apidef.RoutingTriggerOptions`. While this is a pragmatic reuse of an existing data structure, it creates a tight coupling between the access control feature and the URL rewrite feature. A future change to `RoutingTriggerOptions` for URL rewriting purposes could inadvertently affect security-critical access control logic if not handled carefully.
💡 SuggestionTo mitigate the risk of this coupling, consider adding a prominent comment to the `apidef.RoutingTriggerOptions` struct definition itself, warning developers that it is used in a security-critical context by the granular access control middleware and that any changes must be reflected in the access condition validator and evaluator. The test `TestAccessConditions_EveryTriggerOptionIsEvaluated` is an excellent safeguard, and a comment would complement it by providing upfront context to developers before they make changes.
\n\n \n\n

Quality Issues (1)

Severity Location Issue
🟡 Warning gateway/mw_granular_access_conditions.go:347
The `readRequestBody` function reads the entire request body into memory using `io.ReadAll` without an explicit size limit. If a global `max_request_body_size` is not configured for the gateway, this could allow an attacker to send a request with a very large body, potentially causing memory exhaustion and a denial-of-service. While the logic correctly avoids reading the body unless necessary, a request that satisfies all other conditions will still trigger this unbounded read for payload matching.
💡 SuggestionTo provide defense-in-depth, consider wrapping the request body with an `io.LimitedReader` before calling `io.ReadAll`. This would cap the amount of data read for payload matching to a safe limit (e.g., 1MB), protecting against resource exhaustion even if a global limit is not set.

Powered by Visor from Probelabs

Last updated: 2026-09-07T12:04:14.776Z | Triggered by: pr_updated | Commit: 7cdee18

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

@mativm02
mativm02 force-pushed the TT-16132/granular-access-conditions branch from 6582bd6 to 393191a Compare August 26, 2026 13:14
Kofo Okesola and others added 6 commits August 27, 2026 09:02
Access conditions reused the URL Rewrite trigger matcher, but the two want
opposite defaults. A rewrite trigger decides whether to apply a transformation,
so leaning towards firing is harmless; an access condition decides whether to
let a request through, so it has to lean the other way.

Borrowing that matcher meant a policy could not express the rule the customer
actually needs, and three ways to get more access than the policy granted:

  - "parameter must be absent" was inexpressible. Reverse was only evaluated
    when the parameter was present, so a public policy written with it also
    rejected the legitimate bare request.
  - matchers were counted per matching value rather than per configured name,
    so repeating one parameter satisfied a rule that required a different one.
  - only one supplied value had to match, so an extra value on a repeated
    parameter went unchecked.
  - an uncompilable reversed pattern granted access instead of denying it.

Evaluate conditions in a dedicated evaluator instead, with absence
expressible, every supplied value checked, matchers counted per name, and
anything unevaluable denied. mw_url_rewrite.go is left alone so existing
rewrite triggers are unaffected. Evaluation no longer records matches in the
request context data either, so it cannot disturb the trigger numbering a
later URL Rewrite relies on.

The same fixes apply to the header, path part, session meta and request
context matchers, which shared the defects.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Builds on the spike PoC (TT-17881) and closes the gaps that stood between
it and a shippable feature.

Validation. Access conditions fail closed, so a condition the Gateway
cannot evaluate denies every request the entry would otherwise grant.
user.AccessCondition.Validate reports those cases - an unreadable
match_rx, a condition configuring no options, an unrecognised "on" - and
the key and policy APIs reject them at write time. The rules live in the
user package so the Dashboard can apply exactly the same ones when it
accepts a key or policy, rather than the two drifting apart on what
counts as valid.

CoProcess bridge. The Protocol Buffer session carried only the URL and
the methods, so a custom auth plugin's session came back without its
conditions - a spec that loses its conditions grants strictly more than
the one that went out. AccessSpec now carries them in both directions.

While there: TykSessionState allocated the allowed URL slice by length
and then appended to it, padding every access definition with as many
zero-valued specs as it had real ones. An empty URL is an empty regex,
which matches every path, so those padding entries handed a custom-auth
session the whole API regardless of what its allowed_urls said. This
predates conditions and applies to any gRPC, Python or Lua custom auth
plugin that returns allowed_urls.

copyAllowedURLs dropped conditions too. It has no production caller
today, but a copy that silently widens access is a trap for whoever
wires it up next.

Precedence. A session holding both an unconditional and a conditional
entry for one endpoint is granted access by the unconditional one:
allowed_urls is a list of grants and grants are additive, so combining
policies can never tighten what another policy already allowed. Pinned
by a test in both orderings rather than left to the reader.

Absence. "This parameter must not be supplied" is spelled as an empty
match_rx with reverse set - the pattern says nothing about the value, so
reversing it can only be about presence. ".*" with reverse is equivalent
and still works.

The evaluator borrows apidef.RoutingTriggerOptions as its configuration
shape, which couples an access decision to a type maintained for URL
Rewrite. An option the evaluator does not know about would be accepted,
appear to constrain the request, and then be ignored - so a test now
fails if that type gains a field, rather than the drift surfacing as
access somebody thought they had restricted.

Also removes initTriggerOptions and initStringRegexMaps, left behind
when the evaluator stopped reusing the URL Rewrite matcher.
Option groups were all evaluated even when the outcome was already
decided. Under "all" that meant a request whose header or query
condition had already failed still had its entire body read into memory
for a payload match it could not benefit from.

Groups are ordered cheapest first with the payload last, so short
circuiting is what makes that ordering worth anything. Semantics are
unchanged - under "all" one unsatisfied group is the whole answer, and
under "any" the first satisfied one already returned early.

Reading the payload is unbounded, as it is in the URL Rewrite trigger
this borrows its configuration shape from, and the Gateway only caps
request bodies when max_request_body_size is set, which it is not by
default. Not reading the body at all when it cannot change the outcome
is the cheap half of that; a dedicated inspection limit would be a
config decision rather than a fix.

Also adds a benchmark for MergeAllowedURLs. Specs without conditions are
keyed by URL alone and never reach the JSON encoding, so existing
deployments pay nothing; specs with conditions cost roughly 0.6us each.
SonarCloud flagged three issues in new code, all in test files:
mw_granular_access_test.go had third-party imports sitting in the
standard library group, and two http.NewRequest calls discarded their
error.
SonarCloud flagged conditionMatch (21) and checkTriggerOptions (38)
against a limit of 15. Both had grown the same way: six near-identical
blocks, one per option group, each repeating the counting and
short-circuiting logic inline.

Both now describe their groups as data and share one loop over them, so
the counting rules are stated once instead of six times.

The two loops differ in one respect, which is now stated rather than
implied. conditionMatch stops as soon as the answer is settled, because
its last group reads the whole request body. checkTriggerOptions
deliberately does not: its checks record their matches in the request
context data for rewrite templates to interpolate, so skipping the
remaining options after one fails would change what a rewrite can
reference.

Also drops the lazily parsed query cache in conditionMatch. Only one
group ever read it, so it was never saving a second parse.

Visor separately reported a nil pointer dereference on the request
context matches. Reading from a nil map is defined in Go and yields the
zero value, so there is no panic - a request with no context data simply
supplies no values. Added a test for it: a plain rule cannot be
satisfied, a reversed one is, which is what access control needs.
@mativm02
mativm02 force-pushed the TT-16132/granular-access-conditions branch from 948b275 to d41283f Compare August 27, 2026 12:03
@kofoworola
kofoworola self-requested a review August 28, 2026 11:15
@mativm02
mativm02 enabled auto-merge (squash) September 7, 2026 12:02
@sentinelone-cnapp-eu1

Copy link
Copy Markdown

SentinelOne CNS Hardcoded Secret Detector
✅ Congratulations, your code is safe

SentinelOne CNS is a cloud-agnostic, agentless CSPM & CWPP solution that continuously detects and prevents vulnerabilities that have the highest probability of being exploited in Azure, AWS, Google Cloud, and Kubernetes.

@sonarqubecloud

sonarqubecloud Bot commented Sep 7, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
2 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

@mativm02
mativm02 merged commit a6a789f into master Sep 7, 2026
61 of 63 checks passed
@mativm02
mativm02 deleted the TT-16132/granular-access-conditions branch September 7, 2026 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants