moresandeep commented on code in PR #560:
URL: https://github.com/apache/knox/pull/560#discussion_r853988760
##########
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/JWTAccessTokenAssertionFilter.java:
##########
@@ -68,6 +70,10 @@ public void init( FilterConfig filterConfig ) throws
ServletException {
GatewayServices services = (GatewayServices)
filterConfig.getServletContext().getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE);
authority = services.getService(ServiceType.TOKEN_SERVICE);
sr = services.getService(ServiceType.SERVICE_REGISTRY_SERVICE);
+
+ this.tokenIssuer =
filterConfig.getInitParameter(JWTAccessTokenAssertionFilter.ISSUER) != null
Review Comment:
We should check for a blank string too.
##########
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/JWTAccessTokenAssertionFilter.java:
##########
@@ -141,7 +147,13 @@ private String getAccessToken(final String principalName,
String serviceName, lo
JWT token;
try {
- final JWTokenAttributes jwtAttributes = new
JWTokenAttributesBuilder().setUserName(principalName).setAudiences(serviceName).setAlgorithm(signatureAlgorithm).setExpires(expires).build();
+ final JWTokenAttributes jwtAttributes = new JWTokenAttributesBuilder()
Review Comment:
Thanks :)
##########
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/JWTAuthCodeAssertionFilter.java:
##########
@@ -55,6 +57,10 @@ public void init( FilterConfig filterConfig ) throws
ServletException {
GatewayServices services = (GatewayServices)
filterConfig.getServletContext().getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE);
authority = services.getService(ServiceType.TOKEN_SERVICE);
sr = services.getService(ServiceType.SERVICE_REGISTRY_SERVICE);
+
+ this.tokenIssuer =
filterConfig.getInitParameter(JWTAccessTokenAssertionFilter.ISSUER) != null
+ ?
filterConfig.getInitParameter(JWTAccessTokenAssertionFilter.ISSUER)
Review Comment:
As per RFC https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.1
issuer is a `principal` and not principals but could there be cases where we
might need to support multiple issuers? if not we are golden. Just wanted to
raise this.
##########
gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java:
##########
@@ -242,6 +243,9 @@ public void init() throws AliasServiceException,
ServiceLifecycleException, KeyL
? true
: Boolean.parseBoolean(includeGroupsInTokenAllowedParam);
+ this.tokenIssuer = context.getInitParameter(KNOX_TOKEN_ISSUER) != null
Review Comment:
Check for blank string.
##########
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/JWTAuthCodeAssertionFilter.java:
##########
@@ -42,6 +43,7 @@ public class JWTAuthCodeAssertionFilter extends
AbstractIdentityAssertionFilter
private JWTokenAuthority authority;
private ServiceRegistry sr;
+ private String tokenIssuer;
@Override
public void init( FilterConfig filterConfig ) throws ServletException {
Review Comment:
Not related to this change but we have some code duplication here with
`JWTAccessTokenAssertionFilter` We can prolly combine it into a common base
class. Not required here but would be nice.
##########
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/JWTAuthCodeAssertionFilter.java:
##########
@@ -55,6 +57,10 @@ public void init( FilterConfig filterConfig ) throws
ServletException {
GatewayServices services = (GatewayServices)
filterConfig.getServletContext().getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE);
authority = services.getService(ServiceType.TOKEN_SERVICE);
sr = services.getService(ServiceType.SERVICE_REGISTRY_SERVICE);
+
+ this.tokenIssuer =
filterConfig.getInitParameter(JWTAccessTokenAssertionFilter.ISSUER) != null
Review Comment:
We should check for blank 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
