From edda14368a6d99ecf220e8b6572881c3f1db1a01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leonard=20Br=C3=BCnings?= Date: Mon, 10 Jul 2017 20:28:06 +0200 Subject: [PATCH] Only log mismatch warnings, if values are actually defined and differ Change-Id: I89f233191ff146660e658da7cd68a90b8a99ae54 --- .../request/ConnectOAuth2RequestFactory.java | 38 ++++++++++++++----- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/openid-connect-server/src/main/java/org/mitre/openid/connect/request/ConnectOAuth2RequestFactory.java b/openid-connect-server/src/main/java/org/mitre/openid/connect/request/ConnectOAuth2RequestFactory.java index a71aa6c6f..4502cf296 100644 --- a/openid-connect-server/src/main/java/org/mitre/openid/connect/request/ConnectOAuth2RequestFactory.java +++ b/openid-connect-server/src/main/java/org/mitre/openid/connect/request/ConnectOAuth2RequestFactory.java @@ -279,7 +279,7 @@ public class ConnectOAuth2RequestFactory extends DefaultOAuth2RequestFactory { Set responseTypes = OAuth2Utils.parseParameterList(claims.getStringClaim(RESPONSE_TYPE)); if (!responseTypes.isEmpty()) { - if (!responseTypes.equals(request.getResponseTypes())) { + if (definedButDifferent(responseTypes, request.getResponseTypes())) { logger.info("Mismatch between request object and regular parameter for response_type, using request object"); } request.setResponseTypes(responseTypes); @@ -287,7 +287,7 @@ public class ConnectOAuth2RequestFactory extends DefaultOAuth2RequestFactory { String redirectUri = claims.getStringClaim(REDIRECT_URI); if (redirectUri != null) { - if (!redirectUri.equals(request.getRedirectUri())) { + if (definedButDifferent(redirectUri, request.getRedirectUri())) { logger.info("Mismatch between request object and regular parameter for redirect_uri, using request object"); } request.setRedirectUri(redirectUri); @@ -295,7 +295,7 @@ public class ConnectOAuth2RequestFactory extends DefaultOAuth2RequestFactory { String state = claims.getStringClaim(STATE); if(state != null) { - if (!state.equals(request.getState())) { + if (definedButDifferent(state, request.getState())) { logger.info("Mismatch between request object and regular parameter for state, using request object"); } request.setState(state); @@ -303,7 +303,7 @@ public class ConnectOAuth2RequestFactory extends DefaultOAuth2RequestFactory { String nonce = claims.getStringClaim(NONCE); if(nonce != null) { - if (!nonce.equals(request.getExtensions().get(NONCE))) { + if (definedButDifferent(nonce, (String) request.getExtensions().get(NONCE))) { logger.info("Mismatch between request object and regular parameter for nonce, using request object"); } request.getExtensions().put(NONCE, nonce); @@ -311,7 +311,7 @@ public class ConnectOAuth2RequestFactory extends DefaultOAuth2RequestFactory { String display = claims.getStringClaim(DISPLAY); if (display != null) { - if (!display.equals(request.getExtensions().get(DISPLAY))) { + if (definedButDifferent(display, (String) request.getExtensions().get(DISPLAY))) { logger.info("Mismatch between request object and regular parameter for display, using request object"); } request.getExtensions().put(DISPLAY, display); @@ -319,7 +319,7 @@ public class ConnectOAuth2RequestFactory extends DefaultOAuth2RequestFactory { String prompt = claims.getStringClaim(PROMPT); if (prompt != null) { - if (!prompt.equals(request.getExtensions().get(PROMPT))) { + if (definedButDifferent(prompt, (String) request.getExtensions().get(PROMPT))) { logger.info("Mismatch between request object and regular parameter for prompt, using request object"); } request.getExtensions().put(PROMPT, prompt); @@ -327,7 +327,7 @@ public class ConnectOAuth2RequestFactory extends DefaultOAuth2RequestFactory { Set scope = OAuth2Utils.parseParameterList(claims.getStringClaim(SCOPE)); if (!scope.isEmpty()) { - if (!scope.equals(request.getScope())) { + if (definedButDifferent(scope, request.getScope())) { logger.info("Mismatch between request object and regular parameter for scope, using request object"); } request.setScope(scope); @@ -336,7 +336,7 @@ public class ConnectOAuth2RequestFactory extends DefaultOAuth2RequestFactory { JsonObject claimRequest = parseClaimRequest(claims.getStringClaim(CLAIMS)); if (claimRequest != null) { Serializable claimExtension = request.getExtensions().get(CLAIMS); - if (claimExtension == null || !claimRequest.equals(parseClaimRequest(claimExtension.toString()))) { + if (claimExtension != null && !claimRequest.equals(parseClaimRequest(claimExtension.toString()))) { logger.info("Mismatch between request object and regular parameter for claims, using request object"); } // we save the string because the object might not be a Java Serializable, and we can parse it easily enough anyway @@ -345,7 +345,7 @@ public class ConnectOAuth2RequestFactory extends DefaultOAuth2RequestFactory { String loginHint = claims.getStringClaim(LOGIN_HINT); if (loginHint != null) { - if (!loginHint.equals(request.getExtensions().get(LOGIN_HINT))) { + if (definedButDifferent(loginHint, (String) request.getExtensions().get(LOGIN_HINT))) { logger.info("Mistmatch between request object and regular parameter for login_hint, using requst object"); } request.getExtensions().put(LOGIN_HINT, loginHint); @@ -356,6 +356,26 @@ public class ConnectOAuth2RequestFactory extends DefaultOAuth2RequestFactory { } } + /** + * Returns true if both objects are defined but different + * @param requestValue value from the request object + * @param request value from the authorization request directly + * @return true if both objects are defined but different, false otherwise + */ + private boolean definedButDifferent(String requestValue, String request) { + return request != null && !requestValue.equals(request); + } + + /** + * Returns true if both objects are defined but different + * @param requestValue value from the request object + * @param request value from the authorization request directly + * @return true if both objects are defined but different, false otherwise + */ + private boolean definedButDifferent(Set requestValue, Set request) { + return request != null && !request.isEmpty() && !requestValue.equals(request); + } + /** * @param claimRequestString * @return