feat(access_control): Implement scope validation for per-resource authorization by eveniota · Pull Request #13708 · apache/trafficserver

eveniota · 2026-09-19T09:31:24Z

Summary

The access_control plugin previously parsed and exposed a scope claim and supported the --invalid-scope-status-code configuration option, but AccessToken::validate() never evaluated the scope against incoming request paths. Consequently, any valid token authorized access to every resource covered by a remap rule, and setting --invalid-scope-status-code produced no effect.
This PR finishes the feature by implementing path-prefix scope validation on normalized path segments, providing per-resource granularity while remaining fully backward compatible with existing tokens.

Design Approach (per #13607 suggestion)

This PR implements the minimal backward-compatible approach:

Path-prefix matching on normalized segments
Absent/empty scope = unrestricted (backward compatible)
Scope check in separate function after token validation

These choices are open to feedback; see Design Decisions section below.

Key Requirements & Design Decisions

Clean Separation of Concerns (Plumbing):
- Kept AccessToken::validate() focused purely on token cryptographic integrity, semantics, and timing.
- Implemented scope comparison as a separate helper function, validateScope(requestPath, scope), called in enforceAccessControl() immediately after token->validate() succeeds.
- When out of scope, the transaction state is set to OUT_OF_SCOPE, correctly triggering --invalid-scope-status-code (default: 403) and suppressing subject header extraction.
Matching Semantics (Normalized Path Segments):
- Matching is performed as a path-prefix on normalized segments.
- Handles trailing slashes, redundant slashes, and ATS's TSUrlPathGet() format (which omits leading slashes).
- Enforces directory segment boundaries so that a token scoped to /reports/ (or /reports) authorizes /reports/2026/ but strictly rejects sibling paths like /reports2/ or /reports_backup.
Backward Compatibility (Absent / Empty Scope):
- An absent or empty scope claim is treated as unrestricted. Existing tokens in the field continue to function without disruption.
Resource Granularity vs Target Audience (sub claim):
- Retains the existing model where sub represents target audience (e.g. frogs-in-a-well), while adding the missing per-resource granularity (e.g. scope="/reports/2026/").

What Changed

Core Matching Logic (access_control.cc, access_control.h):
- Declared and implemented validateScope(StringView requestPath, StringView scope).
- Added helper normalizePath() to ensure leading slashes, collapse consecutive slashes, and normalize trailing slashes.
- Updated @todo comments regarding scope validation.
Transaction Enforcement (plugin.cc):
- Inside enforceAccessControl(), extracted the request path via TSUrlPathGet() and validated it against token->getScope().
- On failure, sets data->_vaState = OUT_OF_SCOPE and invokes handleInvalidToken().
Unit Tests (test_access_control.cc):
- Added Catch2 test cases covering:
  - Empty scope (unrestricted / backward compatibility)
  - Exact match (/reports $\rightarrow$ /reports)
  - Subpath matches (/reports $\rightarrow$ /reports/2026/annual.pdf)
  - Sibling segment boundary enforcement (/reports vs /reports2 $\rightarrow$ rejected)
  - Inverted hierarchy / parent path requests with child scope $\rightarrow$ rejected
  - Normalization edge cases (trailing slashes, redundant slashes, root scope "/")
  - Token builder & parser integration with addScope() / getScope()
- Documentation (access_control.en.rst):
  - Replaced the note stating scope is "ignored by the current version of the plugin" with full documentation of the matching semantics, segment boundary rules, and status code behavior.

Future Work

Pattern matching (e.g., /reports/{year}/)
Scope lists or disjunctions
Host-qualified scopes (e.g., example.com:/reports/)
Query parameter scoping

These can be added in follow-ups once the core prefix matching is proven.

Verification

Catch2 unit tests pass: ./build/plugins/experimental/access_control/unit_tests/test_access_control
Code formatted via cmake --build build -t format

Add Catch2 unit tests for validateScope()

Copilot

Copilot review overview

🟡 Changes recommended

Documentation and API wording review comments remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2

Open (2)

Document unrestricted scopes and segment-boundary matching semantics · New
Correct API documentation to say paths fall within scope · New

What changed in this PR

Implements per-resource path-scope validation for the experimental access_control plugin.

Changes:

Adds normalized path-prefix matching with backward-compatible empty scopes.
Enforces scope checks during authorization.
Adds unit tests and updates documentation.

File	Description
`plugins/experimental/access_control/unit_tests/test_access_control.cc`	Scope matching and integration tests
`plugins/experimental/access_control/plugin.cc`	Scope enforcement integration
`plugins/experimental/access_control/access_control.h`	Scope validation API
`plugins/experimental/access_control/access_control.cc`	Path normalization and validation
`doc/admin-guide/plugins/access_control.en.rst`	Scope behavior documentation

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+ * Validates whether a request path fails within the scope claim of an access token.
+ * Matching is performed on normalized path segments. An empty or absent scope is
+ * treated as unrestricted (returns true).


Clarify scope usage in access control documentation. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🟡 Changes recommended

Address the traversal vulnerability and add the requested enforcement-path test coverage.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 1

Open (2)

Canonicalize dot segments before scope prefix validation · New
Correct API documentation to say paths fall within scope

Resolved since last review (1)

Document unrestricted scopes and segment-boundary matching semantics

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 2 · 1

Open (3)

This normalization decodes dot bytes but leaves encoded path separators untouched. For example,… · New
Canonicalize dot segments before scope prefix validation
Correct API documentation to say paths fall within scope

segments

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 3 · 1

Open (4)

String is an alias for std::string, but std::string::starts_with() is only available since… · New
This normalization decodes dot bytes but leaves encoded path separators untouched. For example,…
Canonicalize dot segments before scope prefix validation
Correct API documentation to say paths fall within scope

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🟡 Changes recommended

Critical path-decoding and scope-canonicalization issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 3 · 1

Open (4)

URL decoder reads past incomplete percent escapes · New
Plus signs incorrectly decoded as spaces in URL paths · New
Scope traversal normalization broadens authorization · New
Correct API documentation to say paths fall within scope

Resolved since last review (3)

String is an alias for std::string, but std::string::starts_with() is only available since…
This normalization decodes dot bytes but leaves encoded path separators untouched. For example,…
Canonicalize dot segments before scope prefix validation

Previously missed (1)

In code that hasn't changed since last review

Add integration coverage for scoped access enforcement

plugins/experimental/access_control/plugin.cc:539

The new enforcement branch is only exercised through direct validateScope() unit calls; the existing access-control AuTest does not send a scoped token through enforceAccessControl() or verify the configured out-of-scope status and subject-header suppression. Add an integration case for an in-scope and out-of-scope request so this transaction-level plumbing is protected.

+  }
+
+  String decoded(path.size(), '\0');
+  size_t decodedLen = urlDecode(path.data(), path.size(), decoded.data(), decoded.size());


+  }
+
+  String decoded(path.size(), '\0');
+  size_t decodedLen = urlDecode(path.data(), path.size(), decoded.data(), decoded.size());


+  String normRequestPath = normalizePath(requestPath);
+  String normScope       = normalizePath(scope);
+  if (normScope == "/") {
+    return true;


eveniota added 4 commits September 19, 2026 13:24

scope-validation: Add validateScope() method with path-prefix matching

dccae8a

tests: Add scope validation test cases

6198e2c

Add Catch2 unit tests for validateScope()

scope-validation: Integrate scope check into enforceAccessControl()

eba1cde

docs: Document scope claim behaviour and matching semantics

82a94ad

Copilot AI lite review requested due to automatic review settings September 19, 2026 09:31

Copilot started reviewing on behalf of eveniota September 19, 2026 09:31 View session

Copilot AI reviewed Sep 19, 2026

View reviewed changes

Update scope description for access control

5bdf062

Clarify scope usage in access control documentation. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings September 21, 2026 11:28

Copilot started reviewing on behalf of eveniota September 21, 2026 11:34 View session

Copilot AI reviewed Sep 21, 2026

View reviewed changes

Comment thread plugins/experimental/access_control/access_control.cc Outdated

JosiahWI assigned eveniota Sep 21, 2026

JosiahWI added the access_control label Sep 21, 2026

JosiahWI added this to the 11.0.0 milestone Sep 21, 2026

scope-validation: Canonicalize dot segments and escaped traversal

8c0fddf

Copilot AI review requested due to automatic review settings September 21, 2026 14:54

Copilot AI reviewed Sep 21, 2026

View reviewed changes

Comment thread plugins/experimental/access_control/access_control.cc Outdated

Copilot started reviewing on behalf of eveniota September 21, 2026 15:57 View session

scope-validation: Canonicalize path separators and dot

275fb1f

segments

Copilot AI review requested due to automatic review settings September 21, 2026 16:20

Copilot AI reviewed Sep 21, 2026

View reviewed changes

Comment thread plugins/experimental/access_control/access_control.cc Outdated

Copilot started reviewing on behalf of eveniota September 21, 2026 16:54 View session

bryancall requested a review from cmcfarlen September 21, 2026 22:05

Refactor access control path comparison logic

d203a81

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings September 22, 2026 02:29

Copilot started reviewing on behalf of eveniota September 22, 2026 02:29 View session

Copilot AI reviewed Sep 22, 2026

View reviewed changes

Navigation Menu

feat(access_control): Implement scope validation for per-resource authorization - #13708

feat(access_control): Implement scope validation for per-resource authorization#13708
eveniota wants to merge 8 commits into
apache:masterfrom
eveniota:access-control/scope-claim-validation

eveniota commented Sep 19, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

eveniota commented Sep 19, 2026

Summary

Design Approach (per #13607 suggestion)

Key Requirements & Design Decisions

What Changed

Future Work

Verification

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants