From 5eace9fb21fce78e8e05e1b4eba8e47143f88c49 Mon Sep 17 00:00:00 2001
From: Dominik Frantisek Bucik <bucik@ics.muni.cz>
Date: Wed, 12 Jan 2022 15:25:46 +0100
Subject: [PATCH] =?UTF-8?q?fix:=20=F0=9F=90=9B=20Fix=20missing=20sub=20in?=
 =?UTF-8?q?=20ClaimSourceProduceContext?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

bug caused some claims to not generate correctly (i.e. GA4GH passports
could not call the remote APIs due to missing user identifier extracted
from "sub" claim)
---
 .../Ga4ghPassportAndVisaClaimSource.java      |  1 +
 .../userInfo/PerunUserInfoCacheLoader.java    | 29 ++++++++++++++-----
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/ga4gh/Ga4ghPassportAndVisaClaimSource.java b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/ga4gh/Ga4ghPassportAndVisaClaimSource.java
index d3b722553..13307dc79 100644
--- a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/ga4gh/Ga4ghPassportAndVisaClaimSource.java
+++ b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/ga4gh/Ga4ghPassportAndVisaClaimSource.java
@@ -162,6 +162,7 @@ public abstract class Ga4ghPassportAndVisaClaimSource extends ClaimSource {
                                          ArrayNode passport,
                                          Set<String> linkedIdentities)
     {
+        log.debug("GA4GH: {}", uriVariables);
         JsonNode response = callHttpJsonAPI(repo, uriVariables);
         if (response != null) {
             JsonNode visas = response.path(GA4GH_CLAIM);
diff --git a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/userInfo/PerunUserInfoCacheLoader.java b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/userInfo/PerunUserInfoCacheLoader.java
index 715d231ef..02183e99f 100644
--- a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/userInfo/PerunUserInfoCacheLoader.java
+++ b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/userInfo/PerunUserInfoCacheLoader.java
@@ -60,10 +60,11 @@ public class PerunUserInfoCacheLoader extends CacheLoader<UserInfoCacheKey, User
         long perunUserId = key.getUserId();
         Set<String> attributes = constructAttributes(key.getScopes());
         Map<String, PerunAttributeValue> userAttributeValues = fetchUserAttributes(perunUserId, attributes);
+        String sub = extractSub(userAttributeValues, perunUserId, false);
 
         ClaimSourceProduceContext.ClaimSourceProduceContextBuilder builder = ClaimSourceProduceContext.builder()
                 .perunUserId(perunUserId)
-                .sub(ui.getSub())
+                .sub(sub)
                 .attrValues(userAttributeValues)
                 .scopes(key.getScopes())
                 .client(key.getClient())
@@ -97,11 +98,9 @@ public class PerunUserInfoCacheLoader extends CacheLoader<UserInfoCacheKey, User
     }
 
     private Set<String> constructAttributes(Set<String> requestedScopes) {
-        Set<String> attributes = new HashSet<>();
+        // always try to fetch sub, as it might be needed in further claims i.e. GA4GH processing
+        Set<String> attributes = new HashSet<>(openidMappings.getAttrNames());
         if (requestedScopes != null && !requestedScopes.isEmpty()) {
-            if (requestedScopes.contains(OPENID)) {
-                attributes.addAll(openidMappings.getAttrNames());
-            }
             if (requestedScopes.contains(PROFILE)) {
                 attributes.addAll(profileMappings.getAttrNames());
             }
@@ -182,17 +181,31 @@ public class PerunUserInfoCacheLoader extends CacheLoader<UserInfoCacheKey, User
 
     private void processOpenid(Map<String, PerunAttributeValue> userAttributeValues, long perunUserId,
                                PerunUserInfo ui) {
+        ui.setSub(extractSub(userAttributeValues, perunUserId, true));
+        ui.setId(perunUserId);
+    }
+
+    private String extractSub(Map<String, PerunAttributeValue> userAttributeValues, long perunUserId, boolean failOnNoSub) {
         JsonNode subJson = extractJsonValue(openidMappings.getSub(), userAttributeValues);
         if (subJson != null && !subJson.isNull() && StringUtils.hasText(subJson.asText())) {
+            String sub = subJson.asText();
             if (subModifiers != null) {
                 subJson = modifyClaims(subModifiers, subJson);
-                if (subJson.asText() == null || !StringUtils.hasText(subJson.asText())) {
+                if (failOnNoSub && (subJson.asText() == null || !StringUtils.hasText(subJson.asText()))) {
                     throw new RuntimeException("Sub has no value after modification for username " + perunUserId);
+                } else {
+                    sub = subJson.asText();
                 }
             }
-            ui.setSub(subJson.asText());
+            if (sub != null && StringUtils.hasText(sub)) {
+                return sub;
+            }
+        }
+        if (failOnNoSub) {
+            throw new RuntimeException("Sub has no value for username " + perunUserId);
+        } else {
+            return null;
         }
-        ui.setId(perunUserId);
     }
 
     private void processProfile(Map<String, PerunAttributeValue> userAttributeValues, PerunUserInfo ui) {