eveniota commented on PR #13708:
URL: https://github.com/apache/trafficserver/pull/13708#issuecomment-6159380790
@cmcfarlen Thanks for the thorough review. I have addressed all the feedback
in the latest update:
### 1. Path Validation & Origin Forwarding
• Changed normalizePath() to reject request paths containing dot segments
(. or .., raw or percent-encoded) rather than resolving them. This eliminates
authorization mismatches against origin servers (e.g. ..;) since ATS forwards
the raw path.
• Removing dot-segment resolution also eliminated the std::vector<size_t>
allocation per request and the missing <vector> include nit.
### 2. Root Scope (/) Shortcut
• Moved the normScope == "/" shortcut check before requestPath
normalization. Tokens scoped to / now pass immediately, allowing paths with
percent-encoded characters like %2F or literal %.
### 3. Backward Compatibility & Upgrade (--enforce-scope)
• Added an opt-in flag --enforce-scope (default: false) to avoid breaking
existing deployments that issue tokens with non-path scope values (e.g.,
scope=read ).
• Documented --enforce-scope in
doc/admin-guide/plugins/access_control.en.rst.
• Added an upgrade notice under ATS v11.x in
doc/release-notes/upgrading.en.rst.
### 4. Status Code Mapping
• Mapped INVALID_REQUEST_PATH to OUT_OF_SCOPE so requests with
unacceptable paths return --invalid-scope-status-code (403 by default) as
documented, rather than INVALID_SYNTAX.
### 5. Cleanup
• Removed the redundant validateScope() wrapper function. Unit tests now
invoke validateScopeDetailed() directly and assert exact enum results
(IN_SCOPE, OUT_OF_SCOPE, INVALID_REQUEST_PATH, INVALID_SCOPE).
### 6. AuTest & Replay Fixes
• Switched --extract-subject-to-header from @TokenSubject to TokenSubject
in both the test remap config and access_control.replay.yaml (avoiding YAML
scalar parsing issues with @ and the ATS MIME.cc origin-forwarding suppression
of @-
prefixed headers).
• Updated transaction 4 to check TokenSubject on proxy-request (removing
the check on client proxy-response).
• Updated transaction 5 to verify TokenSubject: { as: absent } on
proxy-request for an out-of-scope request.
All unit tests pass (127 assertions in 18 test cases) and formatting
(cmake --build build -t format) has been run.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
