collapsed two serialization functions into one

openid-connect-common/src/main/java/org/mitre/openid/connect/view/UserInfoSerializer.java

@@ -1,6 +1,6 @@
 package org.mitre.openid.connect.view;
 
-import java.util.List;
+import java.util.HashSet;
 import java.util.Map.Entry;
 import java.util.Set;
 
@@ -14,29 +14,6 @@ public class UserInfoSerializer {
 
 	private static ScopeClaimTranslationService translator = new ScopeClaimTranslationService();
 	
-	/**
-	 * Filter the UserInfo object by scope, using our ScopeClaimTranslationService to determine
-	 * which claims are allowed for each given scope.
-	 * 
-	 * @param ui the UserInfo to filter
-	 * @param scope the allowed scopes to filter by
-	 * @return the filtered JsonObject result
-	 */
-	public static JsonObject filterByScope(UserInfo ui, Set<String> scope) {
-
-		JsonObject uiJson = ui.toJson();
-		List<String> filteredClaims = translator.getClaimsForScopeSet(scope);
-		JsonObject result = new JsonObject();
-		
-		for (String claim : filteredClaims) {
-			if (uiJson.has(claim)) {
-				result.add(claim, uiJson.get(claim));
-			}
-		}
-		
-		return result;
-	}
-	
 	/**
 	 * Build a JSON response according to the request object received.
 	 * 
@@ -51,32 +28,43 @@ public class UserInfoSerializer {
 	 */
 	public static JsonObject toJsonFromRequestObj(UserInfo ui, Set<String> scope, JsonObject authorizedClaims, JsonObject requestedClaims) {
 
-		// Only proceed if we have both requested claims and authorized claims list. Otherwise just return
-		// the scope-filtered claim set.
-		if (requestedClaims == null || authorizedClaims == null) {
-			return filterByScope(ui, scope);
-		}
-		
 		// get the base object
 		JsonObject obj = ui.toJson();
 		
-		List<String> allowedByScope = translator.getClaimsForScopeSet(scope);
-		JsonObject userinfoAuthorized = authorizedClaims.getAsJsonObject().get("userinfo").getAsJsonObject();
-		JsonObject userinfoRequested = requestedClaims.getAsJsonObject().get("userinfo").getAsJsonObject();
+		Set<String> allowedByScope = translator.getClaimsForScopeSet(scope);
+		Set<String> authorizedByClaims = new HashSet<String>();
+		Set<String> requestedByClaims = new HashSet<String>();
 		
-		if (userinfoAuthorized == null || !userinfoAuthorized.isJsonObject()) {
-			return obj;
+		if (authorizedClaims != null) {
+			JsonObject userinfoAuthorized = authorizedClaims.getAsJsonObject().get("userinfo").getAsJsonObject();
+			for (Entry<String, JsonElement> entry : userinfoAuthorized.getAsJsonObject().entrySet()) {
+				authorizedByClaims.add(entry.getKey());
+			}
 		}
-
+		if (requestedClaims != null) {
+			JsonObject userinfoRequested = requestedClaims.getAsJsonObject().get("userinfo").getAsJsonObject();
+			for (Entry<String, JsonElement> entry : userinfoRequested.getAsJsonObject().entrySet()) {
+				requestedByClaims.add(entry.getKey());
+			}
+		}
+		
 		// Filter claims by performing a manual intersection of claims that are allowed by the given scope, requested, and authorized.
 		// We cannot use Sets.intersection() or similar because Entry<> objects will evaluate to being unequal if their values are
 		// different, whereas we are only interested in matching the Entry<>'s key values.
 		JsonObject result = new JsonObject();		
-		for (Entry<String, JsonElement> entry : userinfoAuthorized.getAsJsonObject().entrySet()) {
-			if (userinfoRequested.has(entry.getKey()) && allowedByScope.contains(entry.getKey())) {
-				result.add(entry.getKey(), entry.getValue());
+		for (Entry<String, JsonElement> entry : obj.entrySet()) {
+			
+			if (allowedByScope.contains(entry.getKey())
+					|| authorizedByClaims.contains(entry.getKey())) {
+				// it's allowed either by scope or by the authorized claims (either way is fine with us)
+				
+				if (requestedByClaims.isEmpty() || requestedByClaims.contains(entry.getKey())) {
+					// the requested claims are empty (so we allow all), or they're not empty and this claim was specifically asked for
+					result.add(entry.getKey(), entry.getValue());
+				} // otherwise there were specific claims requested and this wasn't one of them
 			}
 		}
+		
 		return result;
 	}
 }

openid-connect-server/src/main/java/org/mitre/openid/connect/view/UserInfoView.java

@@ -108,11 +108,7 @@ public class UserInfoView extends AbstractView {
 			if (model.get("requestedClaims") != null) {
 				requestedClaims = jsonParser.parse((String) model.get("requestedClaims")).getAsJsonObject();
 			}
-			if (authorizedClaims != null || requestedClaims != null) {
-				gson.toJson(UserInfoSerializer.toJsonFromRequestObj(userInfo, scope, authorizedClaims, requestedClaims), out);
-			} else {
-				gson.toJson(UserInfoSerializer.filterByScope(userInfo, scope), out);
-			}
+			gson.toJson(UserInfoSerializer.toJsonFromRequestObj(userInfo, scope, authorizedClaims, requestedClaims), out);
 
 		} catch (IOException e) {