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/12 12:44:30 UTC
[GitHub] [knox] zeroflag opened a new pull request, #554: KNOX-2731 Allow group membership information to be included in issued JWTs
zeroflag opened a new pull request, #554:
URL: https://github.com/apache/knox/pull/554
## What changes were proposed in this pull request?
The KnoxToken service should allow for the option to have group membership information for the authenticated user included in the issued JWT.
## How was this patch tested?
Manually
--
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] lmccay commented on a diff in pull request #554: KNOX-2731 Allow group membership information to be included in issued JWTs
Posted by GitBox <gi...@apache.org>.
lmccay commented on code in PR #554:
URL: https://github.com/apache/knox/pull/554#discussion_r848827981
##########
gateway-spi/src/main/java/org/apache/knox/gateway/services/security/token/JWTokenAttributesBuilder.java:
##########
@@ -32,6 +34,9 @@ public class JWTokenAttributesBuilder {
private boolean managed;
private String jku;
private String type;
+ private Set<String> groups;
+ private String kid;
+ private String issuer = "KNOXSSO";
Review Comment:
I thought we made this configurable at some point. That change would likely be better as a separate PR though.
--
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 #554: KNOX-2731 Allow group membership information to be included in issued JWTs
Posted by GitBox <gi...@apache.org>.
pzampino commented on code in PR #554:
URL: https://github.com/apache/knox/pull/554#discussion_r848773721
##########
gateway-release/home/conf/topologies/homepage.xml:
##########
@@ -113,6 +113,10 @@
<name>knox.token.proxyuser.admin.hosts</name>
<value>*</value>
</param>
+ <param>
+ <name>knox.token.include.groups</name>
+ <value>false</value>
Review Comment:
Does the param need to be added here? It should simply default to false.
##########
gateway-spi/src/main/java/org/apache/knox/gateway/services/security/token/JWTokenAttributesBuilder.java:
##########
@@ -32,6 +34,9 @@ public class JWTokenAttributesBuilder {
private boolean managed;
private String jku;
private String type;
+ private Set<String> groups;
+ private String kid;
+ private String issuer = "KNOXSSO";
Review Comment:
I think this was like this before, but should the issuer really be KNOXSSO in ALL cases?
##########
gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java:
##########
@@ -228,6 +235,9 @@ public void init() throws AliasServiceException, ServiceLifecycleException, KeyL
}
}
+ String shouldIncludeGroupsParam = context.getInitParameter(TOKEN_INCLUDE_GROUPS_IN_JWT);
+ shouldIncludeGroups = shouldIncludeGroupsParam == null ? false : Boolean.parseBoolean(shouldIncludeGroupsParam);
Review Comment:
It looks like this is defaulting to false, and the explicit false in the homepage topology should be unnecessary.
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DefaultTokenAuthorityService.java:
##########
@@ -104,26 +103,15 @@ public void setAliasService(AliasService as) {
@Override
public JWT issueToken(JWTokenAttributes jwtAttributes) throws TokenServiceException {
- String[] claimArray = new String[6];
- claimArray[0] = "KNOXSSO";
- claimArray[1] = jwtAttributes.getUserName();
- claimArray[2] = null;
- if (jwtAttributes.getExpires() == -1) {
- claimArray[3] = null;
- }
- else {
- claimArray[3] = String.valueOf(jwtAttributes.getExpires());
- }
final String algorithm = jwtAttributes.getAlgorithm();
if(SUPPORTED_HMAC_SIG_ALGS.contains(algorithm)) {
- claimArray[4] = null;
- claimArray[5] = null;
+ jwtAttributes.setKid(null);
Review Comment:
Do these need to be explicitly set to null, or could this code be simplified?
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DefaultTokenAuthorityService.java:
##########
@@ -104,26 +103,15 @@ public void setAliasService(AliasService as) {
@Override
public JWT issueToken(JWTokenAttributes jwtAttributes) throws TokenServiceException {
- String[] claimArray = new String[6];
Review Comment:
JWTokenAttributes has replaced the claimsArray throughout?
##########
gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java:
##########
@@ -106,6 +111,7 @@ public class TokenResource {
private static final String TOKEN_TTL_PARAM = "knox.token.ttl";
private static final String TOKEN_TYPE_PARAM = "knox.token.type";
private static final String TOKEN_AUDIENCES_PARAM = "knox.token.audiences";
+ private static final String TOKEN_INCLUDE_GROUPS_IN_JWT = "knox.token.include.groups";
Review Comment:
MINOR: It may be good to define a TOKEN_PARAM_PREFIX="knox.token.", and reference this from the other config property name values to avoid accidental variations across these properties. I realize it was this way before your changes, but while you're here, it may be worth doing.
For example:
private static final String TOKEN_PARAM_PREFIX = "knox.token.";
private static final String TOKEN_TTLE_PARAM = TOKEN_PARAM_PREFIX + "ttl";
private static final String TOKEN_TYPE_PARAM = TOKEN_PARAM_PREFIX + "type";
private static final String TOKEN_AUDIENCES_PARAM = TOKEN_PARAM_PREFIX + "audiences";
##########
gateway-server/src/test/java/org/apache/knox/gateway/services/token/impl/DefaultTokenStateServiceTest.java:
##########
@@ -369,16 +370,7 @@ protected TokenStateService createTokenStateService() throws Exception {
/* create a test JWT token */
protected JWT getJWTToken(final long expiry) {
- String[] claims = new String[6];
- claims[0] = "KNOXSSO";
- claims[1] = "john.doe@example.com";
- claims[2] = "https://login.example.com";
- if(expiry > 0) {
- claims[3] = Long.toString(expiry);
- }
- claims[4] = "E0LDZulQ0XE_otJ5aoQtQu-RnXv8hU-M9U4dD7vDioA";
- claims[5] = null;
- JWT token = new JWTToken("RS256", claims);
+ JWT token = new JWTToken(new JWTokenAttributesBuilder().setExpires(expiry).setAlgorithm("RS256").build());
Review Comment:
This test is slightly different in that the test token has differing contents. Has the coverage of this test changed as a result? Or, does it only reflect the change in handling of claims, whereas now, previously necessary claims are no longer necessary?
##########
gateway-service-knoxtoken/src/test/java/org/apache/knox/gateway/service/knoxtoken/TokenServiceResourceTest.java:
##########
@@ -1396,8 +1444,8 @@ private Subject createTestSubject(final String username) {
return s;
}
- private static Map<String, String> parseJSONResponse(final String response) throws IOException {
- return (new ObjectMapper()).readValue(response, new TypeReference<Map<String, String>>(){});
+ private static Map<String, Object> parseJSONResponse(final String response) throws IOException {
Review Comment:
Why was this changed to return a Map of Object values? What is the response including that isn't a String?
--
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 #554: KNOX-2731 Allow group membership information to be included in issued JWTs
Posted by GitBox <gi...@apache.org>.
zeroflag commented on code in PR #554:
URL: https://github.com/apache/knox/pull/554#discussion_r849242035
##########
gateway-spi/src/main/java/org/apache/knox/gateway/services/security/token/JWTokenAttributesBuilder.java:
##########
@@ -32,6 +34,9 @@ public class JWTokenAttributesBuilder {
private boolean managed;
private String jku;
private String type;
+ private Set<String> groups;
+ private String kid;
+ private String issuer = "KNOXSSO";
Review Comment:
We have a `jwt.expected.issuer` config param which is used in `validateToken`. But the it seems the issuer used by the `issueToken` has always been `KNOXSSO`
--
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 #554: KNOX-2731 Allow group membership information to be included in issued JWTs
Posted by GitBox <gi...@apache.org>.
pzampino commented on code in PR #554:
URL: https://github.com/apache/knox/pull/554#discussion_r849455075
##########
gateway-spi/src/main/java/org/apache/knox/gateway/services/security/token/JWTokenAttributesBuilder.java:
##########
@@ -32,6 +34,9 @@ public class JWTokenAttributesBuilder {
private boolean managed;
private String jku;
private String type;
+ private Set<String> groups;
+ private String kid;
+ private String issuer = "KNOXSSO";
Review Comment:
I've created [KNOX-2732](https://issues.apache.org/jira/browse/KNOX-2732) to address this.
--
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 #554: KNOX-2731 Allow group membership information to be included in issued JWTs
Posted by GitBox <gi...@apache.org>.
zeroflag commented on code in PR #554:
URL: https://github.com/apache/knox/pull/554#discussion_r849659142
##########
gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java:
##########
@@ -144,6 +144,7 @@ public class TokenResource {
private static final String TARGET_ENDPOINT_PULIC_CERT_PEM = TOKEN_PARAM_PREFIX + "target.endpoint.cert.pem";
static final String QUERY_PARAMETER_DOAS = "doAs";
static final String PROXYUSER_PREFIX = TOKEN_PARAM_PREFIX + "proxyuser";
+ public static final String KNOX_TOKEN_INCLUDE_GROUPS = "knox.token.include.groups";
Review Comment:
I fixed it.
--
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 #554: KNOX-2731 Allow group membership information to be included in issued JWTs
Posted by GitBox <gi...@apache.org>.
smolnar82 merged PR #554:
URL: https://github.com/apache/knox/pull/554
--
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 #554: KNOX-2731 Allow group membership information to be included in issued JWTs
Posted by GitBox <gi...@apache.org>.
pzampino commented on code in PR #554:
URL: https://github.com/apache/knox/pull/554#discussion_r849450326
##########
gateway-spi/src/main/java/org/apache/knox/gateway/services/security/token/JWTokenAttributesBuilder.java:
##########
@@ -32,6 +34,9 @@ public class JWTokenAttributesBuilder {
private boolean managed;
private String jku;
private String type;
+ private Set<String> groups;
+ private String kid;
+ private String issuer = "KNOXSSO";
Review Comment:
I agree that it need not be part of this PR, but I think it should be a configurable value.
--
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 #554: KNOX-2731 Allow group membership information to be included in issued JWTs
Posted by GitBox <gi...@apache.org>.
zeroflag commented on code in PR #554:
URL: https://github.com/apache/knox/pull/554#discussion_r849245117
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DefaultTokenAuthorityService.java:
##########
@@ -104,26 +103,15 @@ public void setAliasService(AliasService as) {
@Override
public JWT issueToken(JWTokenAttributes jwtAttributes) throws TokenServiceException {
- String[] claimArray = new String[6];
- claimArray[0] = "KNOXSSO";
- claimArray[1] = jwtAttributes.getUserName();
- claimArray[2] = null;
- if (jwtAttributes.getExpires() == -1) {
- claimArray[3] = null;
- }
- else {
- claimArray[3] = String.valueOf(jwtAttributes.getExpires());
- }
final String algorithm = jwtAttributes.getAlgorithm();
if(SUPPORTED_HMAC_SIG_ALGS.contains(algorithm)) {
- claimArray[4] = null;
- claimArray[5] = null;
+ jwtAttributes.setKid(null);
Review Comment:
It follows the existing behaviour. @moresandeep do you remember why we need to null out the the jku + kid when the sign algorithm is "HS256", "HS384" or "HS512" ?
--
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 #554: KNOX-2731 Allow group membership information to be included in issued JWTs
Posted by GitBox <gi...@apache.org>.
zeroflag commented on code in PR #554:
URL: https://github.com/apache/knox/pull/554#discussion_r849266857
##########
gateway-server/src/test/java/org/apache/knox/gateway/services/token/impl/DefaultTokenStateServiceTest.java:
##########
@@ -369,16 +370,7 @@ protected TokenStateService createTokenStateService() throws Exception {
/* create a test JWT token */
protected JWT getJWTToken(final long expiry) {
- String[] claims = new String[6];
- claims[0] = "KNOXSSO";
- claims[1] = "john.doe@example.com";
- claims[2] = "https://login.example.com";
- if(expiry > 0) {
- claims[3] = Long.toString(expiry);
- }
- claims[4] = "E0LDZulQ0XE_otJ5aoQtQu-RnXv8hU-M9U4dD7vDioA";
- claims[5] = null;
- JWT token = new JWTToken("RS256", claims);
+ JWT token = new JWTToken(new JWTokenAttributesBuilder().setExpires(expiry).setAlgorithm("RS256").build());
Review Comment:
Putting it back or removing it makes no difference. These attributes are not checked in this test. `JWTTokenTest` covers the attribute checks.
--
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 #554: KNOX-2731 Allow group membership information to be included in issued JWTs
Posted by GitBox <gi...@apache.org>.
pzampino commented on code in PR #554:
URL: https://github.com/apache/knox/pull/554#discussion_r849698798
##########
gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java:
##########
@@ -827,6 +837,10 @@ private Response getAuthenticationToken() {
return Response.ok().entity("{ \"Unable to acquire token.\" }").build();
}
+ private boolean shouldIncludeGroups() {
+ return Boolean.parseBoolean(request.getParameter(KNOX_TOKEN_INCLUDE_GROUPS));
Review Comment:
Ok, cool.
--
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 #554: KNOX-2731 Allow group membership information to be included in issued JWTs
Posted by GitBox <gi...@apache.org>.
pzampino commented on code in PR #554:
URL: https://github.com/apache/knox/pull/554#discussion_r849646292
##########
gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java:
##########
@@ -827,6 +837,10 @@ private Response getAuthenticationToken() {
return Response.ok().entity("{ \"Unable to acquire token.\" }").build();
}
+ private boolean shouldIncludeGroups() {
+ return Boolean.parseBoolean(request.getParameter(KNOX_TOKEN_INCLUDE_GROUPS));
Review Comment:
What happens if someone specifies a non-boolean value for the request param? No one SHOULD do that, but what happens if they do?
##########
gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java:
##########
@@ -144,6 +144,7 @@ public class TokenResource {
private static final String TARGET_ENDPOINT_PULIC_CERT_PEM = TOKEN_PARAM_PREFIX + "target.endpoint.cert.pem";
static final String QUERY_PARAMETER_DOAS = "doAs";
static final String PROXYUSER_PREFIX = TOKEN_PARAM_PREFIX + "proxyuser";
+ public static final String KNOX_TOKEN_INCLUDE_GROUPS = "knox.token.include.groups";
Review Comment:
nit: Can/should TOKEN_PARAM_PREFIX be used here?
--
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 #554: KNOX-2731 Allow group membership information to be included in issued JWTs
Posted by GitBox <gi...@apache.org>.
zeroflag commented on code in PR #554:
URL: https://github.com/apache/knox/pull/554#discussion_r849218331
##########
gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java:
##########
@@ -106,6 +111,7 @@ public class TokenResource {
private static final String TOKEN_TTL_PARAM = "knox.token.ttl";
private static final String TOKEN_TYPE_PARAM = "knox.token.type";
private static final String TOKEN_AUDIENCES_PARAM = "knox.token.audiences";
+ private static final String TOKEN_INCLUDE_GROUPS_IN_JWT = "knox.token.include.groups";
Review Comment:
Ok, good idea, I'll change this.
--
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 #554: KNOX-2731 Allow group membership information to be included in issued JWTs
Posted by GitBox <gi...@apache.org>.
zeroflag commented on code in PR #554:
URL: https://github.com/apache/knox/pull/554#discussion_r849217236
##########
gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java:
##########
@@ -228,6 +235,9 @@ public void init() throws AliasServiceException, ServiceLifecycleException, KeyL
}
}
+ String shouldIncludeGroupsParam = context.getInitParameter(TOKEN_INCLUDE_GROUPS_IN_JWT);
+ shouldIncludeGroups = shouldIncludeGroupsParam == null ? false : Boolean.parseBoolean(shouldIncludeGroupsParam);
Review Comment:
We can remove it from there. It only serves as "documentation" of a new config parameter.
--
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 #554: KNOX-2731 Allow group membership information to be included in issued JWTs
Posted by GitBox <gi...@apache.org>.
zeroflag commented on code in PR #554:
URL: https://github.com/apache/knox/pull/554#discussion_r849205220
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DefaultTokenAuthorityService.java:
##########
@@ -104,26 +103,15 @@ public void setAliasService(AliasService as) {
@Override
public JWT issueToken(JWTokenAttributes jwtAttributes) throws TokenServiceException {
- String[] claimArray = new String[6];
Review Comment:
Yes, I discussed it with @smolnar82 and we decided to remove the claimsArray, and use the JWTTokenAttributes instead.
--
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 #554: KNOX-2731 Allow group membership information to be included in issued JWTs
Posted by GitBox <gi...@apache.org>.
pzampino commented on code in PR #554:
URL: https://github.com/apache/knox/pull/554#discussion_r849704522
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DefaultTokenAuthorityService.java:
##########
@@ -104,26 +103,15 @@ public void setAliasService(AliasService as) {
@Override
public JWT issueToken(JWTokenAttributes jwtAttributes) throws TokenServiceException {
- String[] claimArray = new String[6];
- claimArray[0] = "KNOXSSO";
- claimArray[1] = jwtAttributes.getUserName();
- claimArray[2] = null;
- if (jwtAttributes.getExpires() == -1) {
- claimArray[3] = null;
- }
- else {
- claimArray[3] = String.valueOf(jwtAttributes.getExpires());
- }
final String algorithm = jwtAttributes.getAlgorithm();
if(SUPPORTED_HMAC_SIG_ALGS.contains(algorithm)) {
- claimArray[4] = null;
- claimArray[5] = null;
+ jwtAttributes.setKid(null);
Review Comment:
I would expect them to be null by default. Are they getting initialized, and then cleared here at a later time?
I guess it's not that big of an issue, but it would simplify the code a little. Let's worry about it later.
--
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 #554: KNOX-2731 Allow group membership information to be included in issued JWTs
Posted by GitBox <gi...@apache.org>.
zeroflag commented on code in PR #554:
URL: https://github.com/apache/knox/pull/554#discussion_r849203844
##########
gateway-release/home/conf/topologies/homepage.xml:
##########
@@ -113,6 +113,10 @@
<name>knox.token.proxyuser.admin.hosts</name>
<value>*</value>
</param>
+ <param>
+ <name>knox.token.include.groups</name>
+ <value>false</value>
Review Comment:
Yes, the default is false so it is not needed. It was added here just to show the user that this option now available (it is easier to turn it on if we already have this param in the config).
--
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 #554: KNOX-2731 Allow group membership information to be included in issued JWTs
Posted by GitBox <gi...@apache.org>.
zeroflag commented on code in PR #554:
URL: https://github.com/apache/knox/pull/554#discussion_r849217749
##########
gateway-service-knoxtoken/src/test/java/org/apache/knox/gateway/service/knoxtoken/TokenServiceResourceTest.java:
##########
@@ -1396,8 +1444,8 @@ private Subject createTestSubject(final String username) {
return s;
}
- private static Map<String, String> parseJSONResponse(final String response) throws IOException {
- return (new ObjectMapper()).readValue(response, new TypeReference<Map<String, String>>(){});
+ private static Map<String, Object> parseJSONResponse(final String response) throws IOException {
Review Comment:
Now the map can contain a Set value for the groups.
--
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 #554: KNOX-2731 Allow group membership information to be included in issued JWTs
Posted by GitBox <gi...@apache.org>.
zeroflag commented on code in PR #554:
URL: https://github.com/apache/knox/pull/554#discussion_r849654484
##########
gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java:
##########
@@ -827,6 +837,10 @@ private Response getAuthenticationToken() {
return Response.ok().entity("{ \"Unable to acquire token.\" }").build();
}
+ private boolean shouldIncludeGroups() {
+ return Boolean.parseBoolean(request.getParameter(KNOX_TOKEN_INCLUDE_GROUPS));
Review Comment:
It will default to false. The parseBoolean is implemented a follows:
```java
public static boolean parseBoolean(String s) {
return ((s != null) && s.equalsIgnoreCase("true"));
}
```
--
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