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/04/21 09:15:14 UTC
[GitHub] [knox] smolnar82 opened a new pull request, #562: KNOX-2734 - Passcode token is optional in TokenResource's response
smolnar82 opened a new pull request, #562:
URL: https://github.com/apache/knox/pull/562
## What changes were proposed in this pull request?
As described in [KNOX-2734](https://issues.apache.org/jira/browse/KNOX-2734), the passcode token should not be presented in the JSON response coming from TokenResource, when a Knox Token is generated, if:
- token state management is disabled
- the underlying token state backend stores the tokens in-memory only
The token generation UI should have been modified too: when there is no passcode tag in the JSON response there is no reason to show its label on the UI.
## How was this patch tested?
Added new JUnit tests to cover the new business logic.
Additionally, I did E2E testing:
- added KnoxToken service into the `sandbox` topology with `knox.token.exp.server-managed=false`
- configured the token state backend to `DefaultTokenStateService` in `gateway-site.xml`
- Left `knox.token.exp.server-managed=true` in the `homepage` topology
- Generated a token using the token generation page and confirmed the `Passcode Token` label was hidden
```
$ curl -iku admin:admin-password https://localhost:8443/gateway/sandbox/knoxtoken/api/v1/token
HTTP/1.1 200 OK
...
{"access_token":"eyJqa3...W7S7rAVtg","token_id":"bc6ae4b8-3064-4835-8601-5cfffa0cbc51","managed":"false","target_url":"proxy-token/","homepage_url":"homepage/home?profile=token&topologies=sandbox","endpoint_public_cert":"MIIDeDCCAmCgAwIBAgIIfjC1dY...etfIPYZ5yWVL7Q==","token_type":"Bearer","expires_in":1660896802064}
```
- Changed `knox.token.exp.server-managed` to `true` in the `sandbox` topology (please note, the token state backend is still `DefaultTokenStateService`
```
$ curl -iku admin:admin-password https://localhost:8443/gateway/sandbox/knoxtoken/api/v1/token
HTTP/1.1 200 OK
...
{"access_token":"eyJqa3UiOiJodH...Gfy157xezu3Q","token_id":"8fad76c1-147d-44f0-8d18-a067eea7d615","managed":"true","target_url":"proxy-token/","homepage_url":"homepage/home?profile=token&topologies=sandbox","endpoint_public_cert":"MIIDeDCCAmCgAwIBAgIIfj...aetfIPYZ5yWVL7Q==","token_type":"Bearer","expires_in":1660897040594}
```
- Set `gateway.service.tokenstate.impl` to `org.apache.knox.gateway.services.token.impl.AliasBasedTokenStateService` in `gateway-site.xml` and re-started Knox
```
{"access_token":"eyJqa3UiOiJodHRwczp...3zAiz5ygsEBuOVQ","token_id":"f5aaa081-5de2-40aa-8d6f-9961a93bf502","managed":"true","target_url":"proxy-token/","homepage_url":"homepage/home?profile=token&topologies=sandbox","endpoint_public_cert":"MIIDeDCCAmCg...kEFdn5aetfIPYZ5yWVL7Q==","token_type":"Bearer","expires_in":1660896828475,"passcode":"WmpWaFlXRXdPREV0TldSbE1pMDBNR0ZoTFRoa05tWXRPVGsyTVdFNU0ySm1OVEF5OjpZVFF6T1dRd05UVXROamcwWWkwME9HWTNMVGxqT1RBdE16WTBZMkUwTlRFMllXRTM="}
```
- Generated a token using the token generation page and confirmed the `Passcode Token` label was shown with the correct passcode
--
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] pzampino commented on a diff in pull request #562: KNOX-2734 - Passcode token is optional in TokenResource's response
Posted by GitBox <gi...@apache.org>.
pzampino commented on code in PR #562:
URL: https://github.com/apache/knox/pull/562#discussion_r855557121
##########
gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java:
##########
@@ -804,8 +804,11 @@ private Response getAuthenticationToken() {
if (endpointPublicCert != null) {
map.put(ENDPOINT_PUBLIC_CERT, endpointPublicCert);
}
+
final String passcode = UUID.randomUUID().toString();
- map.put(PASSCODE, generatePasscodeField(tokenId, passcode));
+ if (tokenStateService != null && !tokenStateService.storeTokensInMemoryOnly()) {
Review Comment:
This is where you could employ tokenStateService instanceof PersistentTokenStateService
##########
gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/TokenIDAsHTTPBasicCredsFederationFilterTest.java:
##########
@@ -372,6 +372,11 @@ public void start() throws ServiceLifecycleException {
public void stop() throws ServiceLifecycleException {
}
+ @Override
+ public boolean storeTokensInMemoryOnly() {
Review Comment:
Rather than adding this method to all the implementations, I tend to think of these things in terms of interfaces (e.g., PersistentTokenStateService).
Then, the referencing code can check (tokenStateService instanceof PersistentTokenStore).
Perhaps, the persistent implementations could extend an abstract PersistentTokenStateService class, which itself extends DefaultTokenStateService.
--
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 #562: KNOX-2734 - Passcode token is optional in TokenResource's response
Posted by GitBox <gi...@apache.org>.
smolnar82 commented on code in PR #562:
URL: https://github.com/apache/knox/pull/562#discussion_r855831070
##########
gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/TokenIDAsHTTPBasicCredsFederationFilterTest.java:
##########
@@ -372,6 +372,11 @@ public void start() throws ServiceLifecycleException {
public void stop() throws ServiceLifecycleException {
}
+ @Override
+ public boolean storeTokensInMemoryOnly() {
Review Comment:
This works too and is a good idea. I submitted a new PS.
--
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 #562: KNOX-2734 - Passcode token is optional in TokenResource's response
Posted by GitBox <gi...@apache.org>.
smolnar82 merged PR #562:
URL: https://github.com/apache/knox/pull/562
--
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