You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@knox.apache.org by GitBox <gi...@apache.org> on 2022/09/07 12:45:41 UTC
[GitHub] [knox] smolnar82 opened a new pull request, #631: KNOX-2800 - Knox token revocation should also work for impersonated tokens
smolnar82 opened a new pull request, #631:
URL: https://github.com/apache/knox/pull/631
## What changes were proposed in this pull request?
When `knox.token.user.limit.exceeded.action` is set to `REMOVE_OLDEST`, Knox tries to revoke the oldest tokens of the given user for who the token was created. It's true for impersonated tokens too.
However, to be able to revoke a token one of the following conditions should be true:
- the revoker is either configured using the `knox.token.renewer.whitelist` parameter
- or, the revoker should remove its own token (see KNOX-2664)
The second condition was not updated though when the token-impersonation was implemented. From now on Knox considers a token an `own` token if either the `userName` metadata or the `createdBy` metadata matches the revoker subject (prior to this change we only considered the `userName` metadata).
## How was this patch tested?
Added a new unit test case to prevent us from regression as well as executed manual testing as follows:
- configured Knox this way:
- `gateway.knox.token.limit.per.user = 5` (gateway-site.xml)
- `knox.token.user.limit.exceeded.action = REMOVE_OLDEST` (homepage.xml)
- ` knox.token.proxyuser.guest.users = bob` (homepage.xml)
- ` knox.token.proxyuser.guest.hosts = *` (homepage.xml)
- logged into the Knox Home page as `guest`
- generated 6 tokens for user `bob` on the Token Generation UI
- confirmed that only 5 tokens were created for `bob`
<img width="1685" alt="Screenshot 2022-09-07 at 14 44 49" src="https://user-images.githubusercontent.com/34065904/188881401-c89bea58-7714-47a6-bc09-bccf512c0065.png">
```
2022-09-07 14:26:45,257 53f0bba2-a632-4f87-bbb9-466fee64c8f8 WARN knox.gateway (IdentityAsserterHttpServletRequestWrapper.java:scrubOfExistingPrincipalParams(212)) - Possible identity spoofing attempt - impersonation parameter removed: doAs
2022-09-07 14:26:45,258 53f0bba2-a632-4f87-bbb9-466fee64c8f8 ERROR service.knoxtoken (TokenResource.java:getAuthenticationToken(767)) - Unable to get token for user bob: token limit exceeded
2022-09-07 14:26:45,259 53f0bba2-a632-4f87-bbb9-466fee64c8f8 INFO service.knoxtoken (TokenResource.java:getAuthenticationToken(773)) - Revoking bob's oldest token d07cdeb9...72d5bca353c4 ...
2022-09-07 14:26:45,259 53f0bba2-a632-4f87-bbb9-466fee64c8f8 INFO token.state (AliasBasedTokenStateService.java:removeTokens(437)) - Removing token state aliases
2022-09-07 14:26:45,270 53f0bba2-a632-4f87-bbb9-466fee64c8f8 INFO token.state (AliasBasedTokenStateService.java:removeTokens(444)) - Removed token state aliases for d07cdeb9...72d5bca353c4
2022-09-07 14:26:45,270 53f0bba2-a632-4f87-bbb9-466fee64c8f8 INFO service.knoxtoken (TokenResource.java:revoke(591)) - Knox Token service (homepage) revoked token d07cde...a353c4 (d07cdeb9...72d5bca353c4) (renewer=guest)
2022-09-07 14:26:45,273 53f0bba2-a632-4f87-bbb9-466fee64c8f8 INFO service.knoxtoken (TokenResource.java:getAuthenticationToken(817)) - Knox Token service (homepage) issued token eyJqa3...3wSCSw (4329fcd2...7adf1dfda2c3)
2022-09-07 14:26:48,103 INFO token.state (AliasBasedTokenStateService.java:persistTokenState(242)) - Creating token state aliases
2022-09-07 14:26:48,426 INFO token.state (AliasBasedTokenStateService.java:persistTokenState(251)) - Created token state aliases for 4329fcd2...7adf1dfda2c3
```
--
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: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] smolnar82 commented on a diff in pull request #631: KNOX-2800 - Knox token revocation should also work for impersonated tokens
Posted by GitBox <gi...@apache.org>.
smolnar82 commented on code in PR #631:
URL: https://github.com/apache/knox/pull/631#discussion_r964838187
##########
gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java:
##########
@@ -770,7 +771,11 @@ private Response getAuthenticationToken() {
// userTokens is an ordered collection (by issue time) -> the first element is the oldest one
final String oldestTokenId = userTokens.iterator().next().getTokenId();
log.generalInfoMessage(String.format(Locale.getDefault(), "Revoking %s's oldest token %s ...", userName, Tokens.getTokenIDDisplayText(oldestTokenId)));
- revoke(oldestTokenId);
+ final Response revocationResponse = revoke(oldestTokenId);
+ if (Response.Status.OK.getStatusCode() != revocationResponse.getStatus()) {
+ return Response.status(Response.Status.BAD_REQUEST)
Review Comment:
Yes, that's right. We moved on, and that was a bug. This is happening only when the token limit is exceeded -> if the oldest token could not be removed for whatever reason, the new token must not be created.
--
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: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] smolnar82 merged pull request #631: KNOX-2800 - Knox token revocation should also work for impersonated tokens
Posted by GitBox <gi...@apache.org>.
smolnar82 merged PR #631:
URL: https://github.com/apache/knox/pull/631
--
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: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] smolnar82 commented on pull request #631: KNOX-2800 - Knox token revocation should also work for impersonated tokens
Posted by GitBox <gi...@apache.org>.
smolnar82 commented on PR #631:
URL: https://github.com/apache/knox/pull/631#issuecomment-1239343678
Cc. @MrtnBalazs
--
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: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] zeroflag commented on a diff in pull request #631: KNOX-2800 - Knox token revocation should also work for impersonated tokens
Posted by GitBox <gi...@apache.org>.
zeroflag commented on code in PR #631:
URL: https://github.com/apache/knox/pull/631#discussion_r964821436
##########
gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java:
##########
@@ -770,7 +771,11 @@ private Response getAuthenticationToken() {
// userTokens is an ordered collection (by issue time) -> the first element is the oldest one
final String oldestTokenId = userTokens.iterator().next().getTokenId();
log.generalInfoMessage(String.format(Locale.getDefault(), "Revoking %s's oldest token %s ...", userName, Tokens.getTokenIDDisplayText(oldestTokenId)));
- revoke(oldestTokenId);
+ final Response revocationResponse = revoke(oldestTokenId);
+ if (Response.Status.OK.getStatusCode() != revocationResponse.getStatus()) {
+ return Response.status(Response.Status.BAD_REQUEST)
Review Comment:
Why do we need to return with the failure at this point? As far as I see earlier we used to move on when the revoke was unsuccessful.
--
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: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org