cmcfarlen opened a new issue, #13607:
URL: https://github.com/apache/trafficserver/issues/13607
# access_control: implement the `scope` claim so a token authorizes only its
own resources
## Summary
The `access_control` plugin parses a `scope` claim out of the access token
and exposes it, but
never compares it to anything. `AccessToken::validate()` checks syntax,
required claims, the
HMAC signature, and the timing claims, then returns — it takes no
request/resource argument and
ends with a placeholder:
`plugins/experimental/access_control/access_control.cc:64`
```cpp
/** @todo: validate scope eventually */
return _state;
```
The consequence is that any token valid for a remap rule is valid for every
resource that rule
covers. There is no way to mint a token that is accepted for one path and
refused for another.
This is currently documented behavior rather than a defect — the admin guide
says of the
`scope` claim (`doc/admin-guide/plugins/access_control.en.rst:240`):
> `scope` for scope_, `optional`, ignored by the current version of the
plugin, still not
> finalized (more applications and their use cases need to be studied to
finalize the format)
This issue is a request to finish the feature, and a proposal for what
"finalized" could mean.
## Current state
Everything except the comparison is already in place:
| Piece | Location |
|---|---|
| `scope` parsed from the token | `access_control.cc:223` (`_scope =
value;`) |
| `getScope()` accessor | `access_control.h:157` |
| `INVALID_SCOPE`, `OUT_OF_SCOPE` states | `access_control.h:101-102` |
| `--invalid-scope-status-code` option (default 403) | `config.cc:176`,
`config.h:48` |
| That status wired into a response | `plugin.cc:187` |
`getScope()` has no caller outside `unit_tests/test_access_control.cc:67`,
and the member is
itself annotated `still @todo` (`access_control.h:205`).
The request path *is* available at the point where the token is checked.
`TSRemapDoRemap()` reads it (`plugin.cc:588`) and passes it to
`config->_uriPathScope.matchAll()` (`plugin.cc:600`), but that classifier
only decides
*whether* to enforce access control for a path — an admin-side allow/deny
list — not *which*
resources a given token may reach. On a match it calls
`enforceAccessControl()`
(`plugin.cc:604`), which validates the token (`plugin.cc:527`) without the
path being involved.
## Why it is worth finishing
The plugin already treats the token's `sub` claim as a *target audience*
rather than a
resource: the docs describe it as "an application specific audience (role,
group of users,
etc)", the worked example uses values like `frogs-in-a-well` and
`fish-in-a-sea`, and audience
separation is achieved through the cache key (via the `cachekey` plugin)
rather than through
authorization. That model is deliberate and works.
What is missing is any per-resource granularity for deployments that want
it. An operator who
wants "this token is good for `/reports/2026/` and nothing else" has no
mechanism, and the
presence of a `scope` claim plus an `--invalid-scope-status-code` option
reasonably suggests
one exists. Implementing `scope` closes that gap and makes the option
meaningful.
It also removes a small footgun: an operator can set
`--invalid-scope-status-code` today and
get no behavior from it.
## What implementing it requires
More than a bounds check, which is presumably why it was deferred:
1. **A matching semantic.** Exact match, path-prefix, or pattern? Prefix is
the obvious
default for a CDN path namespace, but it needs to be stated, and prefix
matching has to be
defined on normalized path segments so that `/reports2/` is not accepted
by a `/reports`
scope.
2. **Behavior when `scope` is absent.** Tokens in the field today have no
`scope`. Absent must
keep meaning "no restriction", or every existing token breaks. That
argues for the check
being opt-in per remap rule, or keyed off the claim's presence.
3. **What the scope is matched against.** The remapped path, the pre-remap
path, or the path
plus host? These differ once remap rules rewrite the URL, and the answer
determines whether
a scope is portable across rules.
4. **Plumbing.** `validate()` needs the resource passed in, or the scope
check needs to move to
a separate call that `enforceAccessControl()` makes after `validate()`
succeeds. The former
changes a public-ish signature within the plugin; the latter keeps
`validate()` purely about
token integrity, which may be the cleaner split.
5. **The format itself**, which is what the docs call out as unfinished — a
single path, a list,
or something structured.
`INVALID_SCOPE`/`OUT_OF_SCOPE` and the configured status code give the
failure path somewhere to
land, so the reporting side needs no new design.
## Suggested scope for a first change
A minimal, backward-compatible version:
- Add a scope check that runs after `validate()` succeeds, taking the
request path.
- Treat an absent or empty `scope` claim as unrestricted, so existing tokens
are unaffected.
- Match as a path-prefix on normalized segments.
- Return `OUT_OF_SCOPE` on failure so `--invalid-scope-status-code` applies.
- Document the matching rule and the absent-claim behavior, replacing the
"ignored by the
current version" note.
- Extend `unit_tests/test_access_control.cc`, which already covers
`getScope()` parsing, to
cover the comparison.
Anything richer than a prefix — lists, patterns, host-qualified scopes —
could follow once the
format question is settled.
## Notes
- The plugin is experimental and off by default
(`BUILD_EXPERIMENTAL_PLUGINS`), so this changes
nothing for a stock build.
- Line references are against `master` at `d01f3caf6a`.
--
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]
