Skip to content

Commit bfcadfd

Browse files
authored
concord-server: option to look up GH webhook event sender by email (#1243)
1 parent 2c20318 commit bfcadfd

File tree

7 files changed

+294
-8
lines changed

7 files changed

+294
-8
lines changed

server/dist/src/main/resources/concord-server.conf

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -515,6 +515,15 @@ concord-server {
515515
# if enabled use the payload's 'sender.ldap_dn' to find the initiator
516516
useSenderLdapDn = true
517517

518+
# if enabled, use the payload's 'sender.email' to find the initiator
519+
userSenderEmail = false
520+
521+
# Number of webhook sender email lookups to cache
522+
senderEmailCacheSize = 1024
523+
524+
# Duration of time to keep sender email lookups cached
525+
senderEmailCacheDuration = "3 hours"
526+
518527
# save external events into the audit log
519528
logEvents = true
520529

server/impl/src/main/java/com/walmartlabs/concord/server/cfg/GithubConfiguration.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,18 @@ public class GithubConfiguration implements GitHubAppInstallationConfig {
4040
@Config("github.useSenderLdapDn")
4141
private boolean useSenderLdapDn;
4242

43+
@Inject
44+
@Config("github.userSenderEmail")
45+
private boolean userSenderEmail;
46+
47+
@Inject
48+
@Config("github.senderEmailCacheSize")
49+
private long senderEmailCacheSize;
50+
51+
@Inject
52+
@Config("github.senderEmailCacheDuration")
53+
private Duration senderEmailCacheDuration;
54+
4355
@Inject
4456
@Config("github.logEvents")
4557
private boolean logEvents;
@@ -68,6 +80,18 @@ public boolean isUseSenderLdapDn() {
6880
return useSenderLdapDn;
6981
}
7082

83+
public boolean isUseSenderEmail() {
84+
return userSenderEmail;
85+
}
86+
87+
public long senderEmailCacheSize() {
88+
return senderEmailCacheSize;
89+
}
90+
91+
public Duration senderEmailCacheDuration() {
92+
return senderEmailCacheDuration;
93+
}
94+
7195
public boolean isLogEvents() {
7296
return logEvents;
7397
}

server/impl/src/main/java/com/walmartlabs/concord/server/events/GithubEventResource.java

Lines changed: 191 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,18 @@
2020
* =====
2121
*/
2222

23+
import com.codahale.metrics.Gauge;
2324
import com.codahale.metrics.Histogram;
2425
import com.codahale.metrics.MetricRegistry;
2526
import com.fasterxml.jackson.databind.ObjectMapper;
27+
import com.google.common.cache.CacheBuilder;
28+
import com.google.common.cache.CacheLoader;
29+
import com.google.common.cache.LoadingCache;
30+
import com.walmartlabs.concord.common.AuthTokenProvider;
2631
import com.walmartlabs.concord.common.ConfigurationUtils;
32+
import com.walmartlabs.concord.common.ExternalAuthToken;
33+
import com.walmartlabs.concord.common.ObjectMapperProvider;
34+
import com.walmartlabs.concord.common.cfg.MappingAuthConfig;
2735
import com.walmartlabs.concord.runtime.v2.model.GithubTriggerExclusiveMode;
2836
import com.walmartlabs.concord.sdk.Constants;
2937
import com.walmartlabs.concord.sdk.MapUtils;
@@ -32,6 +40,7 @@
3240
import com.walmartlabs.concord.server.audit.AuditObject;
3341
import com.walmartlabs.concord.server.cfg.GithubConfiguration;
3442
import com.walmartlabs.concord.server.events.github.GithubTriggerProcessor;
43+
import com.walmartlabs.concord.server.events.github.GithubUtils;
3544
import com.walmartlabs.concord.server.events.github.Payload;
3645
import com.walmartlabs.concord.server.org.triggers.TriggerEntry;
3746
import com.walmartlabs.concord.server.org.triggers.TriggerUtils;
@@ -54,12 +63,22 @@
5463
import org.slf4j.Logger;
5564
import org.slf4j.LoggerFactory;
5665

66+
import javax.annotation.Nonnull;
5767
import javax.inject.Inject;
5868
import javax.ws.rs.*;
5969
import javax.ws.rs.core.Context;
6070
import javax.ws.rs.core.MediaType;
6171
import javax.ws.rs.core.UriInfo;
72+
import java.io.IOException;
73+
import java.io.InputStream;
74+
import java.net.URI;
75+
import java.net.http.HttpClient;
76+
import java.net.http.HttpRequest;
77+
import java.net.http.HttpResponse;
78+
import java.net.http.HttpResponse.BodyHandlers;
6279
import java.util.*;
80+
import java.util.concurrent.ConcurrentHashMap;
81+
import java.util.concurrent.ExecutionException;
6382
import java.util.function.Supplier;
6483

6584
import static com.walmartlabs.concord.common.MemoSupplier.memo;
@@ -71,14 +90,16 @@
7190
* Uses a custom authentication mechanism,
7291
* see {@link com.walmartlabs.concord.server.security.GithubAuthenticatingFilter}.
7392
* <p>
74-
* See also https://developer.github.com/webhooks/
93+
* See also <a href="https://developer.github.com/webhooks/">developer.github.com/webhooks</a>
7594
*/
7695
@Path("/events/github")
7796
@Tag(name = "GitHub Events")
7897
public class GithubEventResource implements Resource {
7998

8099
private static final Logger log = LoggerFactory.getLogger(GithubEventResource.class);
81100

101+
private static final String ERROR_USER_EMAIL_LOOKUP = "Error looking up user info {}: {}";
102+
82103
private final GithubConfiguration githubCfg;
83104
private final TriggerProcessExecutor executor;
84105
private final AuditLog auditLog;
@@ -87,6 +108,8 @@ public class GithubEventResource implements Resource {
87108
private final LdapManager ldapManager;
88109
private final TriggerEventInitiatorResolver initiatorResolver;
89110
private final Histogram startedProcessesPerEvent;
111+
private final Map<String, Long> rateLimitGauges;
112+
private final LoadingCache<EmailCacheKey, Optional<String>> ghUserEmailCache;
90113

91114
@Inject
92115
public GithubEventResource(GithubConfiguration githubCfg,
@@ -96,7 +119,9 @@ public GithubEventResource(GithubConfiguration githubCfg,
96119
UserManager userManager,
97120
LdapManager ldapManager,
98121
TriggerEventInitiatorResolver initiatorResolver,
99-
MetricRegistry metricRegistry) {
122+
MetricRegistry metricRegistry,
123+
AuthTokenProvider authTokenProvider,
124+
ObjectMapperProvider objectMapperProvider) {
100125

101126
this.githubCfg = githubCfg;
102127
this.executor = executor;
@@ -106,6 +131,18 @@ public GithubEventResource(GithubConfiguration githubCfg,
106131
this.ldapManager = ldapManager;
107132
this.initiatorResolver = initiatorResolver;
108133
this.startedProcessesPerEvent = metricRegistry.histogram("started-processes-per-github-event");
134+
this.rateLimitGauges = new ConcurrentHashMap<>(githubCfg.getAuthConfigs().size());
135+
this.ghUserEmailCache = CacheBuilder.newBuilder()
136+
.expireAfterWrite(githubCfg.senderEmailCacheDuration())
137+
.maximumSize(githubCfg.senderEmailCacheSize())
138+
.concurrencyLevel(32)
139+
.recordStats()
140+
.build(new EmailCacheLoader(githubCfg, rateLimitGauges, authTokenProvider, objectMapperProvider.get()));
141+
142+
for (MappingAuthConfig c : githubCfg.getAuthConfigs()) {
143+
Gauge<Long> rateLimitGauge = () -> rateLimitGauges.getOrDefault(c.id(), -1L);
144+
metricRegistry.gauge("github-rate-limit-" + c.id(), () -> rateLimitGauge);
145+
}
109146
}
110147

111148
@POST
@@ -258,22 +295,65 @@ public GithubEventInitiatorSupplier(UserManager userManager, LdapManager ldapMan
258295

259296
@Override
260297
public UserEntry get() {
261-
if (!githubCfg.isUseSenderLdapDn()) {
298+
if (!githubCfg.isUseSenderLdapDn() && !githubCfg.isUseSenderEmail()) {
299+
// don't try to match against payload sender's ldap_dn or email
262300
return fallback.get();
263301
}
264302

303+
// only LDAP users are supported in GitHub triggers
304+
// ideally, match against exact LDAP DN (requires GitHub to be integrated with LDAP)
305+
if (githubCfg.isUseSenderLdapDn()) {
306+
UserEntry fromDn = findSenderDnInLdap();
307+
if (fromDn != null) {
308+
return fromDn;
309+
}
310+
}
311+
312+
// alternatively, user email may work (e.g. from SSO provider which has upstream LDAP source)
313+
if (githubCfg.isUseSenderEmail()) {
314+
UserEntry fromEmail = findSenderEmailInLdap();
315+
if (fromEmail != null) {
316+
return fromEmail;
317+
}
318+
}
319+
320+
log.warn("getOrCreateUserEntry ['{}'] -> can't determine the sender's 'ldap_dn' or 'email', falling back to 'login'", payload);
321+
return fallback.get();
322+
}
323+
324+
private UserEntry findSenderDnInLdap() {
265325
String ldapDn = payload.getSenderLdapDn();
266-
if (ldapDn == null || ldapDn.trim().isEmpty()) {
267-
log.warn("getOrCreateUserEntry ['{}'] -> can't determine the sender's 'ldap_dn', falling back to 'login'", payload);
268-
return fallback.get();
326+
if (ldapDn == null || ldapDn.isBlank()) {
327+
return null;
269328
}
270329

271-
// only LDAP users are supported in GitHub triggers
272330
try {
273331
LdapPrincipal p = ldapManager.getPrincipalByDn(ldapDn);
332+
274333
if (p == null) {
275334
log.warn("getOrCreateUserEntry ['{}'] -> can't find user by ldap DN ({})", payload, ldapDn);
276-
return fallback.get();
335+
return null;
336+
}
337+
338+
return userManager.getOrCreate(p.getUsername(), p.getDomain(), UserType.LDAP)
339+
.orElseThrow(() -> new ConcordApplicationException("User not found: " + p.getUsername()));
340+
} catch (Exception e) {
341+
throw new RuntimeException(e);
342+
}
343+
}
344+
345+
private UserEntry findSenderEmailInLdap() {
346+
String email = getEmail();
347+
if (email == null || email.isBlank()) {
348+
return null;
349+
}
350+
351+
try {
352+
LdapPrincipal p = ldapManager.getPrincipalByMail(email);
353+
354+
if (p == null) {
355+
log.warn("getOrCreateUserEntry ['{}'] -> can't find user by ldap mail ({})", payload, email);
356+
return null;
277357
}
278358

279359
return userManager.getOrCreate(p.getUsername(), p.getDomain(), UserType.LDAP)
@@ -282,5 +362,108 @@ public UserEntry get() {
282362
throw new RuntimeException(e);
283363
}
284364
}
365+
366+
private String getEmail() {
367+
URI repoUrl = GithubUtils.getRepoCloneUrl(payload);
368+
URI userUrl = GithubUtils.getSenderUrl(payload);
369+
370+
try {
371+
return ghUserEmailCache.get(new EmailCacheKey(repoUrl, userUrl))
372+
.orElse(null);
373+
} catch (ExecutionException ee) {
374+
Throwable t = ee.getCause();
375+
log.warn(ERROR_USER_EMAIL_LOOKUP, userUrl, t.getMessage());
376+
}
377+
378+
return null;
379+
}
380+
}
381+
382+
private static class UserLookupException extends Exception {
383+
public UserLookupException(String message) {
384+
super(message);
385+
}
386+
}
387+
388+
private record GitHubUser(String email) {
389+
}
390+
391+
private record EmailCacheKey(@Nonnull URI repoUrl, @Nonnull URI userUrl) {
392+
393+
@Override
394+
public boolean equals(Object o) {
395+
if (o == null || getClass() != o.getClass()) return false;
396+
397+
EmailCacheKey that = (EmailCacheKey) o;
398+
// userUrl is sufficient for equality for caching--it will point to
399+
// the same user regardless of repoUrl.
400+
// repoUrl is only necessary to acquire token, not for caching.
401+
return userUrl().equals(that.userUrl());
402+
}
403+
404+
@Override
405+
public int hashCode() {
406+
return userUrl().hashCode();
407+
}
408+
}
409+
410+
private static class EmailCacheLoader extends CacheLoader<EmailCacheKey, Optional<String>> {
411+
412+
private final Map<String, Long> rateLimitGauges;
413+
private final AuthTokenProvider authTokenProvider;
414+
private final ObjectMapper objectMapper;
415+
private final HttpClient httpClient;
416+
417+
public EmailCacheLoader(GithubConfiguration githubCfg,
418+
Map<String, Long> rateLimitGauges,
419+
AuthTokenProvider authTokenProvider,
420+
ObjectMapper objectMapper) {
421+
422+
this.rateLimitGauges = rateLimitGauges;
423+
this.authTokenProvider = authTokenProvider;
424+
this.objectMapper = objectMapper;
425+
this.httpClient = HttpClient.newBuilder()
426+
.connectTimeout(githubCfg.getHttpClientTimeout())
427+
.build();
428+
}
429+
430+
@Override
431+
public @Nonnull Optional<String> load(@Nonnull EmailCacheKey key) throws Exception {
432+
URI repoUrl = key.repoUrl();
433+
URI userUrl = key.userUrl();
434+
435+
Optional<ExternalAuthToken> t = authTokenProvider.getToken(repoUrl, null);
436+
437+
if (t.isEmpty()) {
438+
return Optional.empty();
439+
}
440+
441+
HttpRequest req = HttpRequest.newBuilder(userUrl)
442+
.GET()
443+
.header("Authorization", "Bearer " + t.get().token())
444+
.build();
445+
446+
HttpResponse<InputStream> resp = httpClient.send(req, BodyHandlers.ofInputStream());
447+
448+
rateLimitGauges.put(t.get().authId(), resp.headers()
449+
.firstValueAsLong("X-RateLimit-Remaining")
450+
.orElse(-1L));
451+
452+
if (resp.statusCode() != 200) {
453+
throw new UserLookupException("Non-200 response [: " + resp.statusCode() + "]: " + readBody(resp));
454+
}
455+
456+
GitHubUser m = objectMapper.readValue(resp.body(), GitHubUser.class);
457+
458+
return Optional.ofNullable(m.email());
459+
}
460+
461+
private static String readBody(HttpResponse<InputStream> resp) {
462+
try (InputStream is = resp.body()) {
463+
return new String(is.readAllBytes());
464+
} catch (IOException e) {
465+
return "error reading response body: " + e.getMessage();
466+
}
467+
}
285468
}
286469
}

server/impl/src/main/java/com/walmartlabs/concord/server/events/github/GithubUtils.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,11 @@
2121
*/
2222

2323

24+
import com.walmartlabs.concord.common.ConfigurationUtils;
2425
import com.walmartlabs.concord.sdk.MapUtils;
2526
import com.walmartlabs.concord.server.org.triggers.TriggerEntry;
2627

28+
import java.net.URI;
2729
import java.util.Objects;
2830
import java.util.regex.Matcher;
2931
import java.util.regex.Pattern;
@@ -97,6 +99,27 @@ public static boolean ignoreEmptyPush(TriggerEntry triggerEntry) {
9799
return MapUtils.getBoolean(triggerEntry.getCfg(), Constants.IGNORE_EMPTY_PUSH_KEY, true);
98100
}
99101

102+
public static URI getSenderUrl(Payload p) {
103+
Object rawUrl = ConfigurationUtils.get(p.raw(), "sender", "url");
104+
105+
if (rawUrl instanceof String url) {
106+
return URI.create(url);
107+
} else {
108+
throw new IllegalArgumentException("Invalid url info url: " + rawUrl);
109+
}
110+
111+
}
112+
113+
public static URI getRepoCloneUrl(Payload p) {
114+
Object rawUrl = ConfigurationUtils.get(p.raw(), "repository", "clone_url");
115+
116+
if (rawUrl instanceof String url) {
117+
return URI.create(url);
118+
} else {
119+
throw new IllegalArgumentException("Invalid repository clone url: " + rawUrl);
120+
}
121+
}
122+
100123
private static String getRepoPath(String repoUrl) {
101124
// tests support
102125
if (repoUrl.startsWith("/")) {

0 commit comments

Comments
 (0)