Skip to content

Commit a8933e8

Browse files
manuel-alvarez-alvarezmanuel-alvarez-alvarez
committed
More fixes again
1 parent 2dad32b commit a8933e8

File tree

4 files changed

+24
-31
lines changed

4 files changed

+24
-31
lines changed

‎dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java

+13-11
Original file line numberDiff line numberDiff line change
@@ -288,13 +288,17 @@ private Flow<Void> onLoginEvent(
288288
} else {
289289
segment.setTagTop("appsec.events." + eventName + ".usr.id", user, true);
290290
}
291+
segment.setTagTop("_dd.appsec.user.collection_mode", mode.fullName());
291292
}
292293

293294
// update user span tags
294295
segment.setTagTop("appsec.events." + eventName + ".usr.login", user, true);
295296

296297
// update current context with new user login
297298
ctx.setUserLoginSource(mode);
299+
if (mode == SDK) {
300+
ctx.setUserIdSource(mode); // we are setting the usr.id through the SDK
301+
}
298302
final boolean newUserLogin = !user.equals(ctx.getUserLogin());
299303
if (!newUserLogin) {
300304
return NoopFlow.INSTANCE;
@@ -303,20 +307,20 @@ private Flow<Void> onLoginEvent(
303307

304308
// call waf if we have a new user login
305309
final List<Address<?>> addresses = new ArrayList<>(3);
310+
final MapDataBundle.Builder bundleBuilder = new MapDataBundle.Builder(CAPACITY_3_4);
306311
addresses.add(KnownAddresses.USER_LOGIN);
307-
addresses.add(KnownAddresses.USER_ID);
312+
bundleBuilder.add(KnownAddresses.USER_LOGIN, user);
313+
if (mode == SDK) {
314+
addresses.add(KnownAddresses.USER_ID);
315+
bundleBuilder.add(KnownAddresses.USER_ID, user);
316+
}
317+
// we don't support null values for the address so we use an invalid placeholder here
308318
if (sourceEvent == LoginEvent.LOGIN_SUCCESS) {
309319
addresses.add(KnownAddresses.LOGIN_SUCCESS);
320+
bundleBuilder.add(KnownAddresses.LOGIN_SUCCESS, "invalid");
310321
} else if (sourceEvent == LoginEvent.LOGIN_FAILURE) {
311322
addresses.add(KnownAddresses.LOGIN_FAILURE);
312-
}
313-
final MapDataBundle.Builder bundleBuilder =
314-
new MapDataBundle.Builder(addresses.size() == 2 ? CAPACITY_0_2 : CAPACITY_3_4);
315-
bundleBuilder.add(KnownAddresses.USER_ID, user);
316-
bundleBuilder.add(KnownAddresses.USER_LOGIN, user);
317-
if (addresses.size() == 3) {
318-
// we don't support null values for the address so we use an invalid placeholder here
319-
bundleBuilder.add(addresses.get(2), "invalid");
323+
bundleBuilder.add(KnownAddresses.LOGIN_FAILURE, "invalid");
320324
}
321325
final DataBundle bundle = bundleBuilder.build();
322326
final String subInfoKey =
@@ -348,8 +352,6 @@ private Flow<Void> onRequestSession(final RequestContext ctx_, final String sess
348352
}
349353
// unlikely that multiple threads will update the value at the same time
350354
ctx.setSessionId(sessionId);
351-
final TraceSegment segment = ctx_.getTraceSegment();
352-
segment.setTagTop("usr.session_id", sessionId);
353355
while (true) {
354356
DataSubscriberInfo subInfo = sessionIdSubInfo;
355357
if (subInfo == null) {

‎dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy

+11-4
Original file line numberDiff line numberDiff line change
@@ -1044,7 +1044,6 @@ class GatewayBridgeSpecification extends DDSpecification {
10441044
requestSessionCB.apply(ctx, sessionId)
10451045

10461046
then:
1047-
1 * traceSegment.setTagTop('usr.session_id', sessionId)
10481047
1 * eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >>
10491048
{ a, b, db, gw -> bundle = db; gatewayContext = gw; NoopFlow.INSTANCE }
10501049
bundle.get(KnownAddresses.SESSION_ID) == sessionId
@@ -1111,7 +1110,9 @@ class GatewayBridgeSpecification extends DDSpecification {
11111110
1 * traceSegment.setTagTop('asm.keep', true)
11121111
1 * traceSegment.setTagTop('_dd.p.appsec', true)
11131112
1 * eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> { a, b, DataBundle db, GatewayContext gw ->
1114-
assert db.get(KnownAddresses.USER_ID) == expectedUser
1113+
if (mode == SDK) {
1114+
assert db.get(KnownAddresses.USER_ID) == expectedUser
1115+
}
11151116
assert db.get(KnownAddresses.USER_LOGIN) == expectedUser
11161117
assert !gw.isTransient
11171118
return NoopFlow.INSTANCE
@@ -1148,7 +1149,9 @@ class GatewayBridgeSpecification extends DDSpecification {
11481149
1 * traceSegment.setTagTop('asm.keep', true)
11491150
1 * traceSegment.setTagTop('_dd.p.appsec', true)
11501151
1 * eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> { a, b, DataBundle db, GatewayContext gw ->
1151-
assert db.get(KnownAddresses.USER_ID) == expectedUser
1152+
if (mode == SDK) {
1153+
assert db.get(KnownAddresses.USER_ID) == expectedUser
1154+
}
11521155
assert db.get(KnownAddresses.USER_LOGIN) == expectedUser
11531156
assert db.get(KnownAddresses.LOGIN_SUCCESS) != null
11541157
assert !gw.isTransient
@@ -1187,7 +1190,9 @@ class GatewayBridgeSpecification extends DDSpecification {
11871190
1 * traceSegment.setTagTop('asm.keep', true)
11881191
1 * traceSegment.setTagTop('_dd.p.appsec', true)
11891192
1 * eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> { a, b, DataBundle db, GatewayContext gw ->
1190-
assert db.get(KnownAddresses.USER_ID) == expectedUser
1193+
if (mode == SDK) {
1194+
assert db.get(KnownAddresses.USER_ID) == expectedUser
1195+
}
11911196
assert db.get(KnownAddresses.USER_LOGIN) == expectedUser
11921197
assert db.get(KnownAddresses.LOGIN_FAILURE) != null
11931198
assert !gw.isTransient
@@ -1254,6 +1259,7 @@ class GatewayBridgeSpecification extends DDSpecification {
12541259
1 * traceSegment.setTagTop('appsec.events.users.login.success.usr.login', firstUser, true)
12551260
1 * traceSegment.setTagTop('usr.id', firstUser, false)
12561261
1 * traceSegment.setTagTop('_dd.appsec.events.users.login.success.sdk', true, true)
1262+
1 * traceSegment.setTagTop('_dd.appsec.user.collection_mode', 'sdk')
12571263
12581264
0 * traceSegment.setTagTop('_dd.appsec.usr.login', _)
12591265
0 * traceSegment.setTagTop('_dd.appsec.events.users.login.success.auto.mode', _, _)
@@ -1267,6 +1273,7 @@ class GatewayBridgeSpecification extends DDSpecification {
12671273
0 * traceSegment.setTagTop('appsec.events.users.login.success.usr.login', _, _)
12681274
0 * traceSegment.setTagTop('usr.id', _, _)
12691275
0 * traceSegment.setTagTop('_dd.appsec.events.users.login.success.sdk', _, _)
1276+
0 * traceSegment.setTagTop('_dd.appsec.user.collection_mode', _)
12701277
12711278
1 * traceSegment.setTagTop('_dd.appsec.usr.login', secondUser)
12721279
1 * traceSegment.setTagTop('_dd.appsec.events.users.login.success.auto.mode', IDENTIFICATION.fullName(), true)

‎dd-java-agent/instrumentation/spring-security-5/src/test/groovy/datadog/trace/instrumentation/springsecurity5/SpringBootBasedTest.groovy

-8
Original file line numberDiff line numberDiff line change
@@ -152,9 +152,7 @@ class SpringBootBasedTest extends AppSecHttpServerTest<ConfigurableApplicationCo
152152
response.body().string() == REGISTER.body
153153
!span.getTags().isEmpty()
154154
span.getTag('appsec.events.users.signup.usr.login') == 'admin'
155-
span.getTag('appsec.events.users.signup.usr.id') == 'admin'
156155
span.getTag('_dd.appsec.usr.login') == 'admin'
157-
span.getTag('_dd.appsec.usr.id') == 'admin'
158156
span.getTag('_dd.appsec.events.users.signup.auto.mode') == 'identification'
159157
span.getTag('appsec.events.users.signup.track') == true
160158
span.getTag('appsec.events.users.signup')['enabled'] == 'true'
@@ -181,9 +179,7 @@ class SpringBootBasedTest extends AppSecHttpServerTest<ConfigurableApplicationCo
181179
response.body().string() == LOGIN.body
182180
!span.getTags().isEmpty()
183181
span.getTag('appsec.events.users.login.failure.usr.login') == 'not_existing_user'
184-
span.getTag('appsec.events.users.login.failure.usr.id') == 'not_existing_user'
185182
span.getTag('_dd.appsec.usr.login') == 'not_existing_user'
186-
span.getTag('_dd.appsec.usr.id') == 'not_existing_user'
187183
span.getTag('_dd.appsec.events.users.login.failure.auto.mode') == 'identification'
188184
span.getTag('appsec.events.users.login.failure.track') == true
189185
span.getTag('appsec.events.users.login.failure.usr.exists') == false
@@ -208,9 +204,7 @@ class SpringBootBasedTest extends AppSecHttpServerTest<ConfigurableApplicationCo
208204
response.body().string() == LOGIN.body
209205
!span.getTags().isEmpty()
210206
span.getTag('appsec.events.users.login.failure.usr.login') == 'admin'
211-
span.getTag('appsec.events.users.login.failure.usr.id') == 'admin'
212207
span.getTag('_dd.appsec.usr.login') == 'admin'
213-
span.getTag('_dd.appsec.usr.id') == 'admin'
214208
span.getTag('_dd.appsec.events.users.login.failure.auto.mode') == 'identification'
215209
span.getTag('appsec.events.users.login.failure.track') == true
216210
// TODO: Ideally should be `false` but we have no reliable method to detect it it is just absent. See APPSEC-12765.
@@ -237,9 +231,7 @@ class SpringBootBasedTest extends AppSecHttpServerTest<ConfigurableApplicationCo
237231
response.body().string() == LOGIN.body
238232
!span.getTags().isEmpty()
239233
span.getTag('appsec.events.users.login.success.usr.login') == 'admin'
240-
span.getTag('appsec.events.users.login.success.usr.id') == 'admin'
241234
span.getTag('_dd.appsec.usr.login') == 'admin'
242-
span.getTag('_dd.appsec.usr.id') == 'admin'
243235
span.getTag('_dd.appsec.events.users.login.success.auto.mode') == 'identification'
244236
span.getTag('appsec.events.users.login.success.track') == true
245237
span.getTag('appsec.events.users.login.success')['credentialsNonExpired'] == 'true'

‎dd-java-agent/instrumentation/spring-security-6/src/test/groovy/datadog/trace/instrumentation/springsecurity6/SpringBootBasedTest.groovy

-8
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,7 @@ class SpringBootBasedTest extends AppSecHttpServerTest<ConfigurableApplicationCo
128128
response.body().string() == REGISTER.body
129129
!span.getTags().isEmpty()
130130
span.getTag('appsec.events.users.signup.usr.login') == 'admin'
131-
span.getTag('appsec.events.users.signup.usr.id') == 'admin'
132131
span.getTag('_dd.appsec.usr.login') == 'admin'
133-
span.getTag('_dd.appsec.usr.id') == 'admin'
134132
span.getTag('_dd.appsec.events.users.signup.auto.mode') == 'identification'
135133
span.getTag('appsec.events.users.signup.track') == true
136134
span.getTag('appsec.events.users.signup')['enabled'] == 'true'
@@ -157,9 +155,7 @@ class SpringBootBasedTest extends AppSecHttpServerTest<ConfigurableApplicationCo
157155
response.body().string() == LOGIN.body
158156
!span.getTags().isEmpty()
159157
span.getTag('appsec.events.users.login.failure.usr.login') == 'not_existing_user'
160-
span.getTag('appsec.events.users.login.failure.usr.id') == 'not_existing_user'
161158
span.getTag('_dd.appsec.usr.login') == 'not_existing_user'
162-
span.getTag('_dd.appsec.usr.id') == 'not_existing_user'
163159
span.getTag('_dd.appsec.events.users.login.failure.auto.mode') == 'identification'
164160
span.getTag('appsec.events.users.login.failure.track') == true
165161
span.getTag('appsec.events.users.login.failure.usr.exists') == false
@@ -184,9 +180,7 @@ class SpringBootBasedTest extends AppSecHttpServerTest<ConfigurableApplicationCo
184180
response.body().string() == LOGIN.body
185181
!span.getTags().isEmpty()
186182
span.getTag('appsec.events.users.login.failure.usr.login') == 'admin'
187-
span.getTag('appsec.events.users.login.failure.usr.id') == 'admin'
188183
span.getTag('_dd.appsec.usr.login') == 'admin'
189-
span.getTag('_dd.appsec.usr.id') == 'admin'
190184
span.getTag('_dd.appsec.events.users.login.failure.auto.mode') == 'identification'
191185
span.getTag('appsec.events.users.login.failure.track') == true
192186
// TODO: Ideally should be `false` but we have no reliable method to detect it it is just absent. See APPSEC-12765.
@@ -213,9 +207,7 @@ class SpringBootBasedTest extends AppSecHttpServerTest<ConfigurableApplicationCo
213207
response.body().string() == LOGIN.body
214208
!span.getTags().isEmpty()
215209
span.getTag('appsec.events.users.login.success.usr.login') == 'admin'
216-
span.getTag('appsec.events.users.login.success.usr.id') == 'admin'
217210
span.getTag('_dd.appsec.usr.login') == 'admin'
218-
span.getTag('_dd.appsec.usr.id') == 'admin'
219211
span.getTag('_dd.appsec.events.users.login.success.auto.mode') == 'identification'
220212
span.getTag('appsec.events.users.login.success.track') == true
221213
span.getTag('appsec.events.users.login.success')['credentialsNonExpired'] == 'true'

0 commit comments

Comments
 (0)