diff --git a/openid-connect-common/src/main/java/org/mitre/openid/connect/view/UserInfoSerializer.java b/openid-connect-common/src/main/java/org/mitre/openid/connect/view/UserInfoSerializer.java index 19401a82c..375eb3628 100644 --- a/openid-connect-common/src/main/java/org/mitre/openid/connect/view/UserInfoSerializer.java +++ b/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 scope) { - - JsonObject uiJson = ui.toJson(); - List 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 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 allowedByScope = translator.getClaimsForScopeSet(scope); - JsonObject userinfoAuthorized = authorizedClaims.getAsJsonObject().get("userinfo").getAsJsonObject(); - JsonObject userinfoRequested = requestedClaims.getAsJsonObject().get("userinfo").getAsJsonObject(); + Set allowedByScope = translator.getClaimsForScopeSet(scope); + Set authorizedByClaims = new HashSet(); + Set requestedByClaims = new HashSet(); - if (userinfoAuthorized == null || !userinfoAuthorized.isJsonObject()) { - return obj; + if (authorizedClaims != null) { + JsonObject userinfoAuthorized = authorizedClaims.getAsJsonObject().get("userinfo").getAsJsonObject(); + for (Entry entry : userinfoAuthorized.getAsJsonObject().entrySet()) { + authorizedByClaims.add(entry.getKey()); + } } - + if (requestedClaims != null) { + JsonObject userinfoRequested = requestedClaims.getAsJsonObject().get("userinfo").getAsJsonObject(); + for (Entry 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 entry : userinfoAuthorized.getAsJsonObject().entrySet()) { - if (userinfoRequested.has(entry.getKey()) && allowedByScope.contains(entry.getKey())) { - result.add(entry.getKey(), entry.getValue()); + for (Entry 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; } } diff --git a/openid-connect-server/src/main/java/org/mitre/openid/connect/view/UserInfoView.java b/openid-connect-server/src/main/java/org/mitre/openid/connect/view/UserInfoView.java index a05a2a9b6..99ea8368b 100644 --- a/openid-connect-server/src/main/java/org/mitre/openid/connect/view/UserInfoView.java +++ b/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) {