fix(keycloak-oauth-policy): detect simple types and avoid access errors
When pre-indexing the claims fields via reflection, ensure we do not try to traverse into
basic/internal Java objects such as String, as this causes access errors and is not useful anyway.
MODIFY
keycloak-oauth-policy/src/main/java/io/apiman/plugins/keycloak_oauth_policy/ClaimLookup.java
keycloak-oauth-policy/src/main/java/io/apiman/plugins/keycloak_oauth_policy/ClaimLookup.java
package io.apiman.plugins.keycloak_oauth_policy;
+
+import io.apiman.common.logging.ApimanLoggerFactory;
+import io.apiman.common.logging.IApimanLogger;
+
import java.lang.reflect.Field;
import java.util.ArrayDeque;
import java.util.ArrayList;
@@ -32,22 +36,45 @@ import org.keycloak.representations.JsonWebToken;
import com.fasterxml.jackson.annotation.JsonProperty;
/**
+ * Use introspection to look up values in an AccessToken.
+ *
* @author Marc Savy {@literal <msavy@redhat.com>}
*/
@SuppressWarnings("nls")
public class ClaimLookup {
private static final Map<String, List<Field>> STANDARD_CLAIMS_FIELD_MAP = new LinkedHashMap<>();
+ private static final IApimanLogger LOGGER = ApimanLoggerFactory.getLogger(ClaimLookup.class);
static {
Class<?> clazz = AccessToken.class;
do {
- getProperties(clazz, "", new ArrayDeque<Field>());
+ getProperties(clazz, "", new ArrayDeque<>());
} while ((clazz = clazz.getSuperclass()) != null);
// Legacy mappings, to ensure old configs keep working
STANDARD_CLAIMS_FIELD_MAP.put("username", STANDARD_CLAIMS_FIELD_MAP.get(IDToken.PREFERRED_USERNAME));
STANDARD_CLAIMS_FIELD_MAP.put("subject", STANDARD_CLAIMS_FIELD_MAP.get("sub"));
}
+ /**
+ * Get a claim by name.
+ *
+ * @param token token to retrieve claim from
+ * @param claim the claim (field key)
+ * @return string representation of claim
+ */
+ public static <T extends AccessToken> String getClaim(T token, String claim) {
+ if (claim == null || token == null) {
+ return null;
+ }
+ // Get the standard claim field, if available
+ if (STANDARD_CLAIMS_FIELD_MAP.containsKey(claim)) {
+ return callClaimChain(token, STANDARD_CLAIMS_FIELD_MAP.get(claim));
+ } else { // Otherwise, look up 'other claims'
+ Object otherClaim = getOtherClaimValue(token, claim);
+ return otherClaim == null ? null : otherClaim.toString();
+ }
+ }
+
private static void getProperties(Class<?> klazz, String path, Deque<Field> fieldChain) {
for (Field f : klazz.getDeclaredFields()) {
f.setAccessible(true);
@@ -55,9 +82,10 @@ public class ClaimLookup {
if (jsonProperty != null) {
fieldChain.push(f);
// If the inspected type has nested @JsonProperty annotations, we need to inspect it
- if (hasJsonPropertyAnnotation(f)) {
+ if (!terminalType(f) && hasJsonPropertyAnnotation(f)) {
getProperties(f.getType(), f.getName() + ".", fieldChain); // Add "." when traversing into new object.
- } else { // Otherwise, just assume it's simple as the best we can do is #toString
+ } else {
+ // Otherwise, just assume it's simple as the best we can do is #toString
List<Field> fieldList = new ArrayList<>(fieldChain);
Collections.reverse(fieldList);
STANDARD_CLAIMS_FIELD_MAP.put(path + jsonProperty.value(), fieldList);
@@ -67,45 +95,33 @@ public class ClaimLookup {
}
}
+ private static boolean terminalType(Field f) {
+ return f.getType().isPrimitive()
+ || f.getType().isArray()
+ || f.getType().getCanonicalName().startsWith("java.lang");
+ }
+
private static boolean hasJsonPropertyAnnotation(Field f) {
for (Field g : f.getType().getDeclaredFields()) {
g.setAccessible(true);
- if (g.getAnnotation(JsonProperty.class) != null)
+ if (g.getAnnotation(JsonProperty.class) != null) {
return true;
}
- return false;
- }
-
- /**
- *
- * @param token token to retrieve claim from
- * @param claim the claim (field key)
- * @return string representaion of claim
- */
- public static String getClaim(IDToken token, String claim) {
- if (claim == null || token == null)
- return null;
- // Get the standard claim field, if available
- if (STANDARD_CLAIMS_FIELD_MAP.containsKey(claim)) {
- return callClaimChain(token, STANDARD_CLAIMS_FIELD_MAP.get(claim));
- } else { // Otherwise look up 'other claims'
- Object otherClaim = getOtherClaimValue(token, claim);
- return otherClaim == null ? null : otherClaim.toString();
}
+ return false;
}
private static String callClaimChain(Object rootObject, List<Field> list) {
try {
Object candidate = rootObject;
for (Field f : list) {
- if ((candidate = f.get(candidate)) == null)
+ if ((candidate = f.get(candidate)) == null) {
break;
}
+ }
return (candidate == null) ? null : candidate.toString();
} catch (IllegalArgumentException | IllegalAccessException e) {
- // TODO Use logger. These exceptions shouldn't occur, but if it somehow does happen we need to know.
- System.err.println("Unexpected error looking up token field: " + e); //$NON-NLS-1$
- e.printStackTrace();
+ LOGGER.error(e, "Unexpected error looking up token field: ", e.getMessage());
}
return null;
}
@@ -119,8 +135,9 @@ public class ClaimLookup {
return jsonObject.get(split[i]);
} else {
Object val = jsonObject.get(split[i]);
- if (!(val instanceof Map))
+ if (!(val instanceof Map)) {
return null;
+ }
jsonObject = (Map<String, Object>) val;
}
}

MODIFY
keycloak-oauth-policy/src/main/java/io/apiman/plugins/keycloak_oauth_policy/KeycloakOauthPolicy.java
keycloak-oauth-policy/src/main/java/io/apiman/plugins/keycloak_oauth_policy/KeycloakOauthPolicy.java
*/
package io.apiman.plugins.keycloak_oauth_policy;
+import io.apiman.common.logging.ApimanLoggerFactory;
+import io.apiman.common.logging.IApimanLogger;
import io.apiman.gateway.engine.async.IAsyncResult;
import io.apiman.gateway.engine.async.IAsyncResultHandler;
import io.apiman.gateway.engine.beans.ApiRequest;
@@ -50,6 +52,7 @@ import java.util.Collections;
*/
public class KeycloakOauthPolicy extends AbstractMappedPolicy<KeycloakOauthConfigBean> {
+ private static final IApimanLogger LOGGER = ApimanLoggerFactory.getLogger(KeycloakOauthPolicy.class);
private static final String AUTHORIZATION_KEY = "Authorization"; //$NON-NLS-1$
private static final String ACCESS_TOKEN_QUERY_KEY = "access_token"; //$NON-NLS-1$
private static final String BEARER = "Bearer "; //$NON-NLS-1$
@@ -91,15 +94,12 @@ public class KeycloakOauthPolicy extends AbstractMappedPolicy<KeycloakOauthConfi
} else if (doTokenAuth(successStatus, request, context, config, chain, rawToken).getValue()) {
// Transport security check
if (config.getRequireTransportSecurity() && !request.isTransportSecure()) {
- // If we've detected a situation where we should blacklist a
- // token
+ // If we've detected a situation where we should blacklist a token
if (config.getBlacklistUnsafeTokens()) {
- blacklistToken(context, rawToken, new IAsyncResultHandler<Void>() {
- @Override
- public void handle(IAsyncResult<Void> result) {
+ blacklistToken(context, rawToken, (IAsyncResult<Void> result) -> {
if (result.isError()) {
- // TODO log the error (need a policy logger)
- }
+ LOGGER.error("An error occurred when blacklisting a token",
+ result.getError());
}
});
}
@@ -110,9 +110,7 @@ public class KeycloakOauthPolicy extends AbstractMappedPolicy<KeycloakOauthConfi
// If enabled we check against the blacklist
if (config.getBlacklistUnsafeTokens()) {
- isBlacklistedToken(context, rawToken, new IAsyncResultHandler<Boolean>() {
- @Override
- public void handle(IAsyncResult<Boolean> result) {
+ isBlacklistedToken(context, rawToken, (IAsyncResult<Boolean> result) -> {
if (result.isError()) {
throwError(successStatus, chain, result.getError());
} else if (result.getResult()) {
@@ -120,7 +118,6 @@ public class KeycloakOauthPolicy extends AbstractMappedPolicy<KeycloakOauthConfi
} else {
chain.doApply(request);
}
- }
});
} else {
if (successStatus.getValue()) {

MODIFY
keycloak-oauth-policy/src/test/java/io/apiman/plugins/keycloak_oauth_policy/KeycloakOauthPolicyLegacyTest.java
keycloak-oauth-policy/src/test/java/io/apiman/plugins/keycloak_oauth_policy/KeycloakOauthPolicyLegacyTest.java
@@ -128,15 +128,6 @@ public class KeycloakOauthPolicyLegacyTest {
willReturn(new InMemorySharedStateComponent());
}
- private String generateAndSerializeToken() throws CertificateEncodingException, IOException {
- token.notBefore(Time.currentTime() - 100);
-
- config.setRealm("apiman-realm");
- config.setRealmCertificateString(certificateAsPem(idpCertificates[0]));
-
- return new JWSBuilder().jsonContent(token).rsa256(idpPair.getPrivate());
- }
-
@Test
public void subjectToSub() throws CertificateEncodingException, IOException {
ForwardAuthInfo authInfo = new ForwardAuthInfo();
@@ -185,4 +176,13 @@ public class KeycloakOauthPolicyLegacyTest {
}
return sw.toString();
}
+
+ private String generateAndSerializeToken() throws CertificateEncodingException, IOException {
+ token.notBefore(Time.currentTime() - 100);
+
+ config.setRealm("apiman-realm");
+ config.setRealmCertificateString(certificateAsPem(idpCertificates[0]));
+
+ return new JWSBuilder().jsonContent(token).rsa256(idpPair.getPrivate());
+ }
}

fix(keycloak-oauth-policy): detect simple types and avoid access errors

When pre-indexing the claims fields via reflection, ensure we do not try to traverse into
basic/internal Java objects such as String, as this causes access errors and is not useful anyway.