[
https://issues.apache.org/jira/browse/KNOX-3529?focusedWorklogId=1049314&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1049314
]
ASF GitHub Bot logged work on KNOX-3529:
----------------------------------------
Author: ASF GitHub Bot
Created on: 22/Oct/26 13:41
Start Date: 22/Oct/26 13:41
Worklog Time Spent: 10m
Work Description: smolnar82 opened a new pull request, #1467:
URL: https://github.com/apache/knox/pull/1467
[KNOX-3529](https://issues.apache.org/jira/browse/KNOX-3529) - Count the
per-user token limit per token type
## What changes were proposed in this pull request?
The token state is shared by the whole gateway, so
`knox.token.limit.per.user` used to count **all** server-managed tokens of a
user (except KnoxSSO cookies), no matter which API issued them. As a result, a
user's client credentials and API keys used up their JWT budget, and a
`CLIENTID` limit could reject new credentials because the user had JWTs from an
unrelated topology.
This PR makes each API count only its own token types:
- `KNOXTOKEN` (including the OAuth token endpoint and KnoxIDF): JWTs,
KnoxIDF authorization codes and refresh tokens
- `CLIENTID` (and KnoxIDF client registration): client credentials
- `APIKEY`: API keys
KnoxSSO cookies still never count. With `REMOVE_OLDEST`, the oldest token
**of the counted types** is removed.
The new service parameter `knox.token.limit.per.user.by.type.enabled`
controls this. It defaults to `true`; setting it to `false` restores the
previous behavior of counting all token types together. `CLIENTID` and `APIKEY`
take the usual prefixes (`clientid.` / `apikey.`).
Code changes:
- `TokenResource`: filters the user's tokens through
`getTokenTypesCountedTowardsLimit()`; `ClientCredentialsResource` and
`APIKeyResource` override it. The chosen mode is logged at startup.
- `TokenMetadata.getTokenType()`: new helper that returns the
`TokenMetadataType`. Tokens with no type or an unknown type are treated as JWTs.
- Docs: `config_knox_token.md`, `config_client_credentials.md`,
`config_apikey.md` and `knoxidf/configuration.md`.
**Behavior change on upgrade:** limits get looser, not stricter. For
example, a user can now have up to `knox.token.limit.per.user` JWTs **plus**
that many client credentials, where before the two together were capped at that
number. The docs include an upgrade note: deployments that relied on one
combined cap should lower the per-API limits or set
`knox.token.limit.per.user.by.type.enabled=false`.
## How was this patch tested?
- New unit tests in `TokenServiceResourceTest`:
- JWT limits ignore client credentials and API keys, both by default and
when explicitly enabled.
- Authorization codes and refresh tokens count towards the JWT limit.
- With `false`, all types count (the old behavior).
- `REMOVE_OLDEST` revokes the oldest token of the same type.
- The client credentials limit ignores JWTs by default and when enabled,
and counts them with `false`.
- New `TokenMetadataTest.testGetTokenType` covering missing, known and
unknown types.
- `mvn verify` passes for `gateway-spi`, `gateway-service-knoxtoken` and
`gateway-service-knoxidf`.
- The full Docker Compose integration suite passes locally (142 tests,
pylint 10.00/10).
## Integration Tests
New `test_token_limit_by_type.py` with two new topologies:
- `tokenlimitbytype.xml`: `CLIENTID`, `APIKEY` and `KNOXTOKEN`, each limited
to 2 per user, using the default (per-type) counting.
- `tokenlimit-combined.xml`: `KNOXTOKEN` limited to 2 with
`knox.token.limit.per.user.by.type.enabled=false`.
The token store is shared by all test files, so each test has `admin` mint
tokens on behalf of (`doAs`) a fresh random user and starts from zero tokens.
Tests:
- `test_jwt_is_issued_for_the_impersonated_user`: sanity check that the
token's `sub` is the `doAs` user
- `test_jwts_ignore_client_credentials_and_api_keys`
- `test_client_credentials_and_api_keys_ignore_jwts`
- `test_combined_counting_includes_client_credentials`: with `false`, client
credentials use up the JWT limit; the per-type topology still issues JWTs
```
tests-1 | ------------------------------------
tests-1 | Your code has been rated at 10.00/10
tests-1 |
tests-1 | Waiting for knox...
tests-1 | ============================= test session starts
==============================
tests-1 | platform linux
Issue Time Tracking
-------------------
Worklog Id: (was: 1049314)
Remaining Estimate: 0h
Time Spent: 10m
> Count the per-user token limit separately for each token type
> -------------------------------------------------------------
>
> Key: KNOX-3529
> URL: https://issues.apache.org/jira/browse/KNOX-3529
> Project: Apache Knox
> Issue Type: Improvement
> Components: JWT, Server
> Affects Versions: 2.0.0, 2.1.0, 3.0.0
> Reporter: Sandor Molnar
> Assignee: Sandor Molnar
> Priority: Major
> Fix For: 3.1.0
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> h3. Problem
> The per-user token limit ({{knox.token.limit.per.user}}) currently counts
> every token stored for a user, regardless of type, except KnoxSSO cookies
> ({{TokenResource#enforceTokenLimitsAsRequired}}). The token state service is
> shared across the whole gateway, so tokens issued by one service (or
> topology) count against the limits of all the others.
> This causes trouble when a client uses one token type to get another. Example:
> - Topology A runs the {{CLIENTID}} service with
> {{clientid.knox.token.limit.per.user=100}}. A user creates 10 client
> credentials there.
> - Topology B runs the {{KNOXTOKEN}} service with
> {{knox.token.limit.per.user=150}}. A client exchanges those credentials for
> access tokens via {{/oauth/v1/token}}.
> - Topology B can mint only 140 access tokens, because the 10 client
> credentials count against its limit too. API keys owned by the same user
> reduce it further.
> - With {{knox.token.user.limit.exceeded.action=REMOVE_OLDEST}} it's worse:
> the oldest tokens are usually the long-lived client credentials or API keys,
> so minting an access token revokes the credential that was used to get it.
> Client credentials and API keys are long-lived credentials. Access tokens
> issued by {{KNOXTOKEN}} are usually short-lived. Counting them together means
> neither limit means what an admin expects.
> h3. Proposal
> Count tokens only against the limit for their own type. Every stored token
> already records its type ({{TokenMetadata.TYPE}}: {{JWT}}, {{CLIENT_ID}},
> {{API_KEY}}, {{KNOXSSO_COOKIE}}, ...), and
> {{TokenStateService#getTokens(userName)}} already returns that metadata, so
> no storage change is needed.
> * {{TokenResource}} gets a hook that returns the token types its limit
> counts. The default is everything except {{KNOXSSO_COOKIE}}, {{CLIENT_ID}}
> and {{API_KEY}} (so {{JWT}}, {{AUTH_CODE}} and {{REFRESH_TOKEN}}).
> - {{ClientCredentialsResource}} counts only {{CLIENT_ID}} tokens, and
> {{APIKeyResource}} counts only {{API_KEY}} tokens.
> - {{enforceTokenLimitsAsRequired}} filters the user's tokens by that hook
> before comparing against the limit. {{REMOVE_OLDEST}} then revokes the oldest
> token of the same type only.
> - Tokens with no {{TYPE}} metadata (stored by older versions) are treated as
> {{JWT}}.
> This changes what the limit means. Counting each token type separately
> becomes the default, controlled by a new service parameter that defaults to
> {{true}}. Deployments that want to keep today's behavior, where all of a
> user's tokens count together, can set it to {{false}}:
> {code:xml}
> <param>
> <name>knox.token.limit.per.user.by.type.enabled</name>
> <value>false</value>
> </param>
> {code}
> For {{CLIENTID}} and {{APIKEY}}, the parameter takes the usual prefix
> ({{clientid.}} / {{apikey.}}).
> Note: existing deployments will allow more tokens per user after upgrading,
> because each API now has its own budget instead of sharing one combined cap.
> For example, a user could have up to {{knox.token.limit.per.user}} JWTs plus
> that many client credentials, where before the two together were capped at
> that number. This is documented as an upgrade note.
> h3. KnoxIDF tokens
> KnoxIDF auth codes ({{AUTH_CODE}}) and refresh tokens ({{REFRESH_TOKEN}})
> stay in the default count, together with {{JWT}}. They are short-lived and
> deleted on use (auth codes are consumed on redemption, refresh tokens on
> rotation), so they don't build up the way long-lived client credentials and
> API keys do. Excluding them would also leave them with no per-user limit at
> all.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
