concord-db, concord-server: cache external app user mapping in database (#1244)

benbroadaway · web-flow · commit 65b6b2a0427d · 2025-12-29T08:02:57.000-06:00
diff --git a/server/db/src/main/resources/com/walmartlabs/concord/server/db/liquibase.xml b/server/db/src/main/resources/com/walmartlabs/concord/server/db/liquibase.xml
@@ -119,4 +119,5 @@
     <include file="v2.22.0.xml" relativeToChangelogFile="true"/>
     <include file="v2.23.0.xml" relativeToChangelogFile="true"/>
     <include file="v2.31.0.xml" relativeToChangelogFile="true"/>
+    <include file="v2.35.0.xml" relativeToChangelogFile="true"/>
 </databaseChangeLog>
diff --git a/server/db/src/main/resources/com/walmartlabs/concord/server/db/v2.35.0.xml b/server/db/src/main/resources/com/walmartlabs/concord/server/db/v2.35.0.xml
@@ -0,0 +1,24 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<databaseChangeLog
+        xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+        xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-3.3.xsd">
+
+    <changeSet id="2510000" author="benjamin.broadaway@walmart.com">
+        <createTable tableName="EXTERNAL_APP_USERS">
+            <column name="EXTERNAL_USER_ID" type="varchar(512)">
+                <constraints primaryKey="true" nullable="false"/>
+            </column>
+            <column name="USER_ID" type="uuid">
+                <constraints nullable="false"/>
+            </column>
+        </createTable>
+
+        <addForeignKeyConstraint baseTableName="EXTERNAL_APP_USERS"
+                                 baseColumnNames="USER_ID"
+                                 constraintName="FK_EXTERNAL_APP_USERS_USER_ID"
+                                 referencedTableName="USERS"
+                                 referencedColumnNames="USER_ID"
+                                 onDelete="CASCADE"/>
+    </changeSet>
+</databaseChangeLog>
diff --git a/server/dist/src/main/resources/concord-server.conf b/server/dist/src/main/resources/concord-server.conf
@@ -516,7 +516,10 @@ concord-server {
         useSenderLdapDn = true
 
         # if enabled, use the payload's 'sender.email' to find the initiator
-        userSenderEmail = false
+        useSenderEmail = false
+
+        # if enabled, stores a mapping of github user id -> concord user id in the DB
+        enableExternalUserIdMappingCache = false
 
         # Number of webhook sender email lookups to cache
         senderEmailCacheSize = 1024
diff --git a/server/impl/src/main/java/com/walmartlabs/concord/server/cfg/GithubConfiguration.java b/server/impl/src/main/java/com/walmartlabs/concord/server/cfg/GithubConfiguration.java
@@ -41,8 +41,8 @@ public class GithubConfiguration implements GitHubAppInstallationConfig {
     private boolean useSenderLdapDn;
 
     @Inject
-    @Config("github.userSenderEmail")
-    private boolean userSenderEmail;
+    @Config("github.useSenderEmail")
+    private boolean useSenderEmail;
 
     @Inject
     @Config("github.senderEmailCacheSize")
@@ -52,6 +52,10 @@ public class GithubConfiguration implements GitHubAppInstallationConfig {
     @Config("github.senderEmailCacheDuration")
     private Duration senderEmailCacheDuration;
 
+    @Inject
+    @Config("github.enableExternalUserIdMappingCache")
+    private boolean enableExternalUserIdMappingCache;
+
     @Inject
     @Config("github.logEvents")
     private boolean logEvents;
@@ -81,7 +85,11 @@ public boolean isUseSenderLdapDn() {
     }
 
     public boolean isUseSenderEmail() {
-        return userSenderEmail;
+        return useSenderEmail;
+    }
+
+    public boolean isEnableExternalUserIdMappingCache() {
+        return enableExternalUserIdMappingCache;
     }
 
     public long senderEmailCacheSize() {
diff --git a/server/impl/src/main/java/com/walmartlabs/concord/server/events/GithubEventResource.java b/server/impl/src/main/java/com/walmartlabs/concord/server/events/GithubEventResource.java
@@ -84,6 +84,9 @@
 import static com.walmartlabs.concord.common.MemoSupplier.memo;
 import static com.walmartlabs.concord.server.events.github.Constants.COMMIT_ID_KEY;
 import static com.walmartlabs.concord.server.events.github.Constants.EVENT_SOURCE;
+import static com.walmartlabs.concord.server.events.github.Constants.NODE_ID_KEY;
+import static com.walmartlabs.concord.server.events.github.Constants.SENDER_KEY;
+import static com.walmartlabs.concord.server.events.github.Constants.URL_KEY;
 
 /**
  * Handles external GitHub events.
@@ -295,8 +298,15 @@ public GithubEventInitiatorSupplier(UserManager userManager, LdapManager ldapMan
 
         @Override
         public UserEntry get() {
+            // GitHub user -> Concord user mapping may already exist in DB
+            UserEntry fromDbMapping = findUserInDbMapping();
+
+            if (fromDbMapping != null) {
+                return fromDbMapping;
+            }
+
+            // don't try to match against payload sender's ldap_dn or email if they aren't present
             if (!githubCfg.isUseSenderLdapDn() && !githubCfg.isUseSenderEmail()) {
-                // don't try to match against payload sender's ldap_dn or email
                 return fallback.get();
             }
 
@@ -305,6 +315,7 @@ public UserEntry get() {
             if (githubCfg.isUseSenderLdapDn()) {
                 UserEntry fromDn = findSenderDnInLdap();
                 if (fromDn != null) {
+                    addUserDbMapping(fromDn);
                     return fromDn;
                 }
             }
@@ -313,6 +324,7 @@ public UserEntry get() {
             if (githubCfg.isUseSenderEmail()) {
                 UserEntry fromEmail = findSenderEmailInLdap();
                 if (fromEmail != null) {
+                    addUserDbMapping(fromEmail);
                     return fromEmail;
                 }
             }
@@ -321,6 +333,49 @@ public UserEntry get() {
             return fallback.get();
         }
 
+        private UserEntry findUserInDbMapping() {
+            if (!githubCfg.isEnableExternalUserIdMappingCache()) {
+                return null;
+            }
+
+            String senderUrl = payload.getUrl(SENDER_KEY);
+            String senderNodeId = payload.getNodeId(SENDER_KEY);
+            String externalId = formatExternalId(senderUrl, senderNodeId);
+
+            if (externalId == null) {
+                return null;
+            }
+            return userManager.getUserFromExternalMapping(externalId).orElse(null);
+        }
+
+        private void addUserDbMapping(UserEntry user) {
+            if (!githubCfg.isEnableExternalUserIdMappingCache()) {
+                return;
+            }
+
+            String senderUrl = payload.getUrl(SENDER_KEY);
+            String senderNodeId = payload.getNodeId(SENDER_KEY);
+            String externalId = formatExternalId(senderUrl, senderNodeId);
+
+            if (externalId == null) {
+                return;
+            }
+
+            userManager.createExternalUserMapping(user.getId(), externalId);
+        }
+
+        private static String formatExternalId(String url, String userId) {
+            if (url == null || userId == null) {
+                return null;
+            }
+
+            if (url.isBlank() || userId.isBlank()) {
+                return null;
+            }
+
+            return String.format("github_%s=%s,%s=%s", URL_KEY, url, NODE_ID_KEY, userId);
+        }
+
         private UserEntry findSenderDnInLdap() {
             String ldapDn = payload.getSenderLdapDn();
             if (ldapDn == null || ldapDn.isBlank()) {
diff --git a/server/impl/src/main/java/com/walmartlabs/concord/server/events/github/Constants.java b/server/impl/src/main/java/com/walmartlabs/concord/server/events/github/Constants.java
@@ -43,7 +43,9 @@ public final class Constants {
     public static final String QUERY_PARAMS_KEY = "queryParams";
     public static final String BASE_KEY = "base";
     public static final String HEAD_KEY = "head";
+    public static final String NODE_ID_KEY = "node_id";
     public static final String REF_KEY = "ref";
+    public static final String URL_KEY = "url";
 
     public static final String GITHUB_ORG_KEY = "githubOrg";
     public static final String GITHUB_REPO_KEY = "githubRepo";
diff --git a/server/impl/src/main/java/com/walmartlabs/concord/server/events/github/Payload.java b/server/impl/src/main/java/com/walmartlabs/concord/server/events/github/Payload.java
@@ -210,6 +210,19 @@ public boolean hasPullRequestEntry() {
         return raw().containsKey(PULL_REQUEST_EVENT);
     }
 
+    public String getUrl(String attribute) {
+        Map<String, Object> m = MapUtils.getMap(raw(), attribute, Map.of());
+        return MapUtils.getString(m, URL_KEY);
+    }
+
+    /**
+     * @return graphql node id for the given attribute
+     */
+    public String getNodeId(String attribute) {
+        Map<String, Object> m = MapUtils.getMap(raw(), attribute, Map.of());
+        return MapUtils.getString(m, NODE_ID_KEY);
+    }
+
     public String getPullRequestBaseUrl() {
         Map<String, Object> pullRequest = getPullRequestAttribute(this.raw());
         return getPullRequestCloneUrl(pullRequest, BASE_KEY);
diff --git a/server/impl/src/main/java/com/walmartlabs/concord/server/user/UserDao.java b/server/impl/src/main/java/com/walmartlabs/concord/server/user/UserDao.java
@@ -160,6 +160,44 @@ public UserEntry get(UUID id) {
         return getUserInfo(tx, r);
     }
 
+    public UserEntry getUserFromExternalMapping(String externalId) {
+        DSLContext tx = dsl();
+
+        Record9<UUID, String, String, String, String, String, Boolean, Boolean, OffsetDateTime> r =
+                tx.select(USERS.USER_ID, USERS.USER_TYPE, USERS.USERNAME, USERS.DOMAIN, USERS.DISPLAY_NAME, USERS.USER_EMAIL, USERS.IS_DISABLED, USERS.IS_PERMANENTLY_DISABLED, USERS.DISABLED_DATE)
+                        .from(USERS, EXTERNAL_APP_USERS)
+                        .where(USERS.USER_ID.eq(EXTERNAL_APP_USERS.USER_ID))
+                        .and(EXTERNAL_APP_USERS.EXTERNAL_USER_ID.eq(externalId))
+                        .fetchOne();
+
+        if (r == null) {
+            return null;
+        }
+
+        return getUserInfo(tx, r);
+    }
+
+    /**
+     * Creates a mapping from an external user to an existing Concord user. The
+     * external identifier may not be a singular, exact, ID for the system. The
+     * context(s) creating and retrieving the mapping must understand the construction
+     * of the identifier. Typically, it should be a formatting along the lines
+     * of <code>{external_system} + {actual_user_id}</code> to avoid clashing of
+     * IDs that are unique to the system but potentially duplicated across other
+     * external systems that have been integrated (e.g. multiple GitHub instances).
+     * @param userId existing Concord user ID
+     * @param externalId identifier for user in an external system
+     */
+    public void createExternalUserMapping(UUID userId, String externalId) {
+        tx(tx -> tx.insertInto(EXTERNAL_APP_USERS)
+                .columns(EXTERNAL_APP_USERS.EXTERNAL_USER_ID, EXTERNAL_APP_USERS.USER_ID)
+                .values(externalId, userId)
+                .onConflict(EXTERNAL_APP_USERS.EXTERNAL_USER_ID)
+                .doUpdate()
+                .set(EXTERNAL_APP_USERS.USER_ID, userId)
+                .execute());
+    }
+
     /**
      * Returns the ID of the specified user.
      * Note that {@code username} and {@code domain} are case-insensitive and forced to lower case.
diff --git a/server/impl/src/main/java/com/walmartlabs/concord/server/user/UserManager.java b/server/impl/src/main/java/com/walmartlabs/concord/server/user/UserManager.java
@@ -108,6 +108,22 @@ public Optional<UUID> getId(String username, String userDomain, UserType type) {
         return Optional.ofNullable(userDao.getId(username, userDomain, type));
     }
 
+    public Optional<UserEntry> getUserFromExternalMapping(String externalId) {
+        if (externalId == null) {
+            return Optional.empty();
+        }
+
+        return Optional.ofNullable(userDao.getUserFromExternalMapping(externalId));
+    }
+
+    public void createExternalUserMapping(UUID concordUserId, String externalId) {
+        if (externalId == null) {
+            return;
+        }
+
+        userDao.createExternalUserMapping(concordUserId, externalId);
+    }
+
     public Optional<UserEntry> update(UUID userId, String displayName, String email, UserType userType, boolean isDisabled, Set<String> roles) {
         UserEntry prevEntry = userDao.get(userId);
         if (prevEntry == null) {
