moresandeep commented on code in PR #1043: URL: https://github.com/apache/knox/pull/1043#discussion_r2136088088
########## gateway-provider-identity-assertion-common/src/main/java/org/apache/knox/gateway/identityasserter/common/filter/CommonIdentityAssertionFilter.java: ########## @@ -63,285 +64,304 @@ import org.apache.knox.gateway.util.HttpExceptionUtils; public class CommonIdentityAssertionFilter extends AbstractIdentityAssertionFilter { - private static final IdentityAsserterMessages LOG = MessagesFactory.get(IdentityAsserterMessages.class); - - public static final String VIRTUAL_GROUP_MAPPING_PREFIX = "group.mapping."; - public static final String GROUP_PRINCIPAL_MAPPING = "group.principal.mapping"; - public static final String PRINCIPAL_MAPPING = "principal.mapping"; - public static final String ADVANCED_PRINCIPAL_MAPPING = "expression.principal.mapping"; - private static final String PRINCIPAL_PARAM = "user.name"; - private static final String DOAS_PRINCIPAL_PARAM = "doAs"; - static final String IMPERSONATION_ENABLED_PARAM = AuthFilterUtils.PROXYUSER_PREFIX + ".impersonation.enabled"; - - private SimplePrincipalMapper mapper = new SimplePrincipalMapper(); - private final Parser parser = new Parser(); - private VirtualGroupMapper virtualGroupMapper; - /* List of all default and configured impersonation params */ - protected final List<String> impersonationParamsList = new ArrayList<>(); - protected boolean impersonationEnabled; - private AbstractSyntaxTree expressionPrincipalMapping; - private String topologyName; - - @Override - public void init(FilterConfig filterConfig) throws ServletException { - topologyName = (String) filterConfig.getServletContext().getAttribute(GatewayServices.GATEWAY_CLUSTER_ATTRIBUTE); - String principalMapping = filterConfig.getInitParameter(PRINCIPAL_MAPPING); - if (principalMapping == null || principalMapping.isEmpty()) { - principalMapping = filterConfig.getServletContext().getInitParameter(PRINCIPAL_MAPPING); - } - String groupPrincipalMapping = filterConfig.getInitParameter(GROUP_PRINCIPAL_MAPPING); - if (groupPrincipalMapping == null || groupPrincipalMapping.isEmpty()) { - groupPrincipalMapping = filterConfig.getServletContext().getInitParameter(GROUP_PRINCIPAL_MAPPING); + private static final IdentityAsserterMessages LOG = MessagesFactory.get(IdentityAsserterMessages.class); + + public static final String VIRTUAL_GROUP_MAPPING_PREFIX = "group.mapping."; + public static final String GROUP_PRINCIPAL_MAPPING = "group.principal.mapping"; + public static final String PRINCIPAL_MAPPING = "principal.mapping"; + public static final String ADVANCED_PRINCIPAL_MAPPING = "expression.principal.mapping"; + private static final String PRINCIPAL_PARAM = "user.name"; + private static final String DOAS_PRINCIPAL_PARAM = "doAs"; + static final String IMPERSONATION_ENABLED_PARAM = AuthFilterUtils.PROXYUSER_PREFIX + ".impersonation.enabled"; + + private SimplePrincipalMapper mapper = new SimplePrincipalMapper(); + private final Parser parser = new Parser(); + private VirtualGroupMapper virtualGroupMapper; + /* List of all default and configured impersonation params */ + protected final List<String> impersonationParamsList = new ArrayList<>(); + protected boolean impersonationEnabled; + private AbstractSyntaxTree expressionPrincipalMapping; + private String topologyName; + private boolean hasProxyGroupParams; + + @Override + public void init(FilterConfig filterConfig) throws ServletException { + topologyName = (String) filterConfig.getServletContext().getAttribute(GatewayServices.GATEWAY_CLUSTER_ATTRIBUTE); + String principalMapping = filterConfig.getInitParameter(PRINCIPAL_MAPPING); + if (principalMapping == null || principalMapping.isEmpty()) { + principalMapping = filterConfig.getServletContext().getInitParameter(PRINCIPAL_MAPPING); + } + String groupPrincipalMapping = filterConfig.getInitParameter(GROUP_PRINCIPAL_MAPPING); + if (groupPrincipalMapping == null || groupPrincipalMapping.isEmpty()) { + groupPrincipalMapping = filterConfig.getServletContext().getInitParameter(GROUP_PRINCIPAL_MAPPING); + } + if (principalMapping != null && !principalMapping.isEmpty() || groupPrincipalMapping != null && !groupPrincipalMapping.isEmpty()) { Review Comment: Good point, the do behave the same (the existing statement and the example you mentioned) this is because `&&` has higher precedence than `||` in Java [1]. Style wise i prefer the one you suggested but this is not part of my changes this is how it was before the change, not sure why Github is flagging this as a new change though. If you prefer i change it let me know! [1] https://stackoverflow.com/questions/4436669/how-to-prove-the-precedence-of-and-by-coding-in-java -- 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-unsubscr...@knox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org