cmcfarlen commented on code in PR #13708:
URL: https://github.com/apache/trafficserver/pull/13708#discussion_r4353510659
##########
plugins/experimental/access_control/plugin.cc:
##########
@@ -529,12 +526,31 @@ enforceAccessControl(TSHttpTxn txnp, TSRemapRequestInfo
*rri, AccessControlConfi
remapStatus =
handleInvalidToken(txnp, data, reject,
accessTokenStateToHttpStatus(data->_vaState, config), data->_vaState);
} else {
- /* Valid token, if configured extract the token subject to a header,
- * only if we can trust it - token is valid to prevent using it by
mistake */
- if (!config->_extrSubHdrName.empty()) {
- String sub(token->getSubject());
- setHeader(rri->requestBufp, rri->requestHdrp,
config->_extrSubHdrName.c_str(), config->_extrSubHdrName.size(),
- sub.c_str(), sub.size());
+ bool inScope = true;
+ if (config->_enforceScope) {
+ int pathLen = 0;
+ const char *path = TSUrlPathGet(rri->requestBufp,
rri->requestUrl, &pathLen);
Review Comment:
By the time a remap plugin runs, `rri->requestUrl` has already been
rewritten to the map target (`url_rewrite_remap_request()` runs before the
first plugin), so scope is matched against the origin-side path, not the path
the client sent. With `map /public/ https://origin/internal/`, a token has to
be scoped to `/internal/...`.
I think that's the right behavior: it's what the origin sees, and the
include/exclude URI path matching already works this way. But token issuers
need to know it, so please say so in the `scope` docs.
##########
doc/release-notes/upgrading.en.rst:
##########
@@ -49,6 +49,13 @@ Reaching a single metric by name is ``lookup()``.
Spans handed out unnamed slots that only ``rename()`` could name, and
``rename()`` mutated a name that the lock free readers hand out views of.
+Plugins
+-------
+
+access_control
+~~~~~~~~~~~~~~
+
+The :ref:`admin-plugins-access_control` plugin now supports path-prefix scope
enforcement using the ``scope`` claim in access tokens. Scope enforcement is
disabled by default for backward compatibility with deployments issuing tokens
with non-path scopes. To enable scope enforcement, pass
``--enforce-scope=true`` as a plugin parameter.
Review Comment:
This needs a blank line after it. Without one, docutils folds the following
`traffic_ctl Changes` title and its underline into this paragraph without a
warning, so the section heading disappears from the rendered page and Docs CI
doesn't catch it.
--
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]
