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 2020/06/09 16:29:19 UTC
[GitHub] [knox] smolnar82 opened a new pull request #341: KNOX-2383 - Checking token expiration in cache should not depend on the validate flag
smolnar82 opened a new pull request #341:
URL: https://github.com/apache/knox/pull/341
## What changes were proposed in this pull request?
The issue was that `AliasBasedTokenStateService.getTokenExpiration(...)` only checked the in-memory cache for token expiration if `validate` flag was set to `false`. Therefore, if the client wanted to use the previoulsy acquired token before it got written out to the underlying keystore (`__gateway-credentials.jceks`) it received an `expired token` error because this method returned 0 (=nothing was found in either in the memory nor in the keystore).
The fix is as simple as making sure the `validate` flag should not affect the source of the token expiration (it should only validate if needed)
## How was this patch tested?
Updated and executed JUnit tests:
```
$ mvn clean -Dshellcheck=true -T1C verify -Prelease,package
...
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 18:27 min (Wall Clock)
[INFO] Finished at: 2020-06-09T11:27:21+02:00
[INFO] Final Memory: 432M/2223M
[INFO] ------------------------------------------------------------------------
```
In addition to uni testing the following manual test steps were executed:
1. built and redeployed Knox with my changes and set the following before I started Knox:
1. created a new topology called `tokenbased` with a JWT federation provider where I set `knox.token.exp.server-managed` to 'true'. I added the HDFSUI service here for my tests.
2. added `KNOXTOKEN` service to `sandbox` topology (also set `knox.token.exp.server-managed` to 'true')
3. set `gateway.knox.token.state.alias.persistence.interval` to `60` so that I had enough time to invoke the HDFSUI through `tokenbased` before the previously acquired Knox token got written out into the keystore
2. issued the following two commands:
```
curl -ivku guest:guest-password https://localhost:8443/gateway/sandbox/knoxtoken/api/v1/token
```
and
```
curl -ivk -H "Authorization: Bearer eyJhbGciOiJSUzI1NiJ9.eyJzd...1xCsdEgkP1Be-H-nbw" https://localhost:8443/gateway/tokenbased/hdfs?host=https://$HDFS_HOST:20102/
```
3. HDFS UI loaded properly
4. 50 seconds later I saw that the token got actually stored in the keystore:
```
2020-06-09 11:21:31,732 INFO token.state (AliasBasedTokenStateService.java:persistTokenState(111)) - Creating token state aliases
2020-06-09 11:21:31,936 INFO token.state (AliasBasedTokenStateService.java:persistTokenState(116)) - Created token state aliases for 41bd9151-4ca1-4a06-87e4-ca543412b1b0
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] pzampino commented on a change in pull request #341: KNOX-2383 - Checking token expiration in cache should not depend on the validate flag
Posted by GitBox <gi...@apache.org>.
pzampino commented on a change in pull request #341:
URL: https://github.com/apache/knox/pull/341#discussion_r437480353
##########
File path: gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/AliasBasedTokenStateService.java
##########
@@ -166,24 +166,21 @@ protected long getMaxLifetime(final String tokenId) {
@Override
public long getTokenExpiration(String tokenId, boolean validate) throws UnknownTokenException {
- long expiration = 0;
-
- if (!validate) {
- // If validation is not required, then check the in-memory collection first
- try {
- expiration = super.getTokenExpiration(tokenId, validate);
- return expiration;
- } catch (UnknownTokenException e) {
- // It's not in memory
- }
+ if (validate) {
Review comment:
I would keep this where it was previously, since the super will validate based on the in-memory store, and if it's not in-memory, only then do we want to validate the alias.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] smolnar82 merged pull request #341: KNOX-2383 - Checking token expiration in cache should not depend on the validate flag
Posted by GitBox <gi...@apache.org>.
smolnar82 merged pull request #341:
URL: https://github.com/apache/knox/pull/341
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] smolnar82 commented on a change in pull request #341: KNOX-2383 - Checking token expiration in cache should not depend on the validate flag
Posted by GitBox <gi...@apache.org>.
smolnar82 commented on a change in pull request #341:
URL: https://github.com/apache/knox/pull/341#discussion_r437484510
##########
File path: gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/AliasBasedTokenStateService.java
##########
@@ -166,24 +166,21 @@ protected long getMaxLifetime(final String tokenId) {
@Override
public long getTokenExpiration(String tokenId, boolean validate) throws UnknownTokenException {
- long expiration = 0;
-
- if (!validate) {
- // If validation is not required, then check the in-memory collection first
- try {
- expiration = super.getTokenExpiration(tokenId, validate);
- return expiration;
- } catch (UnknownTokenException e) {
- // It's not in memory
- }
+ if (validate) {
Review comment:
Fixed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org