Skip to content

Commit 3835157

Browse files
tylerbensontylerbenson
authored
Merge pull request #577 from DataDog/tyler/spring-auth
Set user.principal in a way spring security can be covered
2 parents 31b2e0f + 1064e35 commit 3835157

9 files changed

Lines changed: 137 additions & 23 deletions

File tree

‎dd-java-agent/instrumentation/jetty-8/src/main/java/datadog/trace/instrumentation/jetty8/JettyHandlerAdvice.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public static Scope startSpan(
5252
Tags.HTTP_URL.set(span, req.getRequestURL().toString());
5353
span.setTag(DDTags.RESOURCE_NAME, resourceName);
5454
if (req.getUserPrincipal() != null) {
55-
span.setTag("user.principal", req.getUserPrincipal().getName());
55+
span.setTag(DDTags.USER_NAME, req.getUserPrincipal().getName());
5656
}
5757
return scope;
5858
}

‎dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import io.opentracing.propagation.Format;
1212
import io.opentracing.tag.Tags;
1313
import io.opentracing.util.GlobalTracer;
14+
import java.security.Principal;
1415
import java.util.Collections;
1516
import javax.servlet.ServletRequest;
1617
import javax.servlet.http.HttpServletRequest;
@@ -49,16 +50,24 @@ public static Scope startSpan(
4950
if (scope instanceof TraceScope) {
5051
((TraceScope) scope).setAsyncPropagation(true);
5152
}
52-
53-
if (httpServletRequest.getUserPrincipal() != null) {
54-
scope.span().setTag("user.principal", httpServletRequest.getUserPrincipal().getName());
55-
}
5653
return scope;
5754
}
5855

5956
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
6057
public static void stopSpan(
61-
@Advice.Enter final Scope scope, @Advice.Thrown final Throwable throwable) {
58+
@Advice.Enter final Scope scope,
59+
@Advice.Argument(0) final ServletRequest req,
60+
@Advice.Thrown final Throwable throwable) {
61+
// Set user.principal regardless of who created this span.
62+
final Span currentSpan = GlobalTracer.get().activeSpan();
63+
if (currentSpan != null) {
64+
if (req instanceof HttpServletRequest) {
65+
final Principal principal = ((HttpServletRequest) req).getUserPrincipal();
66+
if (principal != null) {
67+
currentSpan.setTag(DDTags.USER_NAME, principal.getName());
68+
}
69+
}
70+
}
6271

6372
if (scope != null) {
6473
final Span span = scope.span();

‎dd-java-agent/instrumentation/servlet-2/src/test/groovy/JettyServlet2Test.groovy‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import datadog.trace.agent.test.AgentTestRunner
22
import datadog.trace.agent.test.TestUtils
33
import datadog.trace.agent.test.utils.OkHttpUtils
44
import datadog.trace.api.DDSpanTypes
5+
import datadog.trace.api.DDTags
56
import okhttp3.Credentials
67
import okhttp3.Interceptor
78
import okhttp3.OkHttpClient
@@ -94,7 +95,7 @@ class JettyServlet2Test extends AgentTestRunner {
9495
"span.type" DDSpanTypes.WEB_SERVLET
9596
"servlet.context" "/ctx"
9697
if (auth) {
97-
"user.principal" "user"
98+
"$DDTags.USER_NAME" "user"
9899
}
99100
defaultTags(distributedTracing)
100101
}

‎dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import io.opentracing.propagation.Format;
1212
import io.opentracing.tag.Tags;
1313
import io.opentracing.util.GlobalTracer;
14+
import java.security.Principal;
1415
import java.util.Collections;
1516
import java.util.concurrent.atomic.AtomicBoolean;
1617
import javax.servlet.ServletRequest;
@@ -52,10 +53,6 @@ public static Scope startSpan(
5253
if (scope instanceof TraceScope) {
5354
((TraceScope) scope).setAsyncPropagation(true);
5455
}
55-
56-
if (httpServletRequest.getUserPrincipal() != null) {
57-
scope.span().setTag("user.principal", httpServletRequest.getUserPrincipal().getName());
58-
}
5956
return scope;
6057
}
6158

@@ -65,6 +62,16 @@ public static void stopSpan(
6562
@Advice.Argument(1) final ServletResponse response,
6663
@Advice.Enter final Scope scope,
6764
@Advice.Thrown final Throwable throwable) {
65+
// Set user.principal regardless of who created this span.
66+
final Span currentSpan = GlobalTracer.get().activeSpan();
67+
if (currentSpan != null) {
68+
if (request instanceof HttpServletRequest) {
69+
final Principal principal = ((HttpServletRequest) request).getUserPrincipal();
70+
if (principal != null) {
71+
currentSpan.setTag(DDTags.USER_NAME, principal.getName());
72+
}
73+
}
74+
}
6875

6976
if (scope != null) {
7077
if (request instanceof HttpServletRequest && response instanceof HttpServletResponse) {

‎dd-java-agent/instrumentation/servlet-3/src/test/groovy/JettyServlet3Test.groovy‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import datadog.trace.agent.test.AgentTestRunner
22
import datadog.trace.agent.test.TestUtils
33
import datadog.trace.agent.test.utils.OkHttpUtils
44
import datadog.trace.api.DDSpanTypes
5+
import datadog.trace.api.DDTags
56
import okhttp3.Credentials
67
import okhttp3.Interceptor
78
import okhttp3.OkHttpClient
@@ -101,7 +102,7 @@ class JettyServlet3Test extends AgentTestRunner {
101102
"span.type" DDSpanTypes.WEB_SERVLET
102103
"http.status_code" 200
103104
if (auth) {
104-
"user.principal" "user"
105+
"$DDTags.USER_NAME" "user"
105106
}
106107
defaultTags(distributedTracing)
107108
}

‎dd-java-agent/instrumentation/spring-web/spring-web.gradle‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ dependencies {
2222
annotationProcessor deps.autoservice
2323
implementation deps.autoservice
2424

25-
testCompile project(':dd-java-agent:testing')
25+
testCompile(project(':dd-java-agent:testing')){
26+
exclude(module: 'jetty-server') // incompatable servlet api
27+
}
2628

2729
// Include servlet instrumentation for verifying the tomcat requests
2830
testCompile project(':dd-java-agent:instrumentation:servlet-3')
@@ -32,8 +34,7 @@ dependencies {
3234

3335
testCompile group: 'org.spockframework', name: 'spock-spring', version: '1.1-groovy-2.4'
3436

35-
testCompile group: 'org.springframework', name: 'spring-web', version: '4.3.14.RELEASE'
36-
testCompile group: 'org.springframework', name: 'spring-webmvc', version: '4.3.14.RELEASE'
37-
testCompile group: 'org.springframework.boot', name: 'spring-boot-starter-test', version: '1.5.10.RELEASE'
38-
testCompile group: 'org.springframework.boot', name: 'spring-boot-starter-tomcat', version: '1.5.10.RELEASE'
37+
testCompile group: 'org.springframework.boot', name: 'spring-boot-starter-test', version: '1.5.17.RELEASE'
38+
testCompile group: 'org.springframework.boot', name: 'spring-boot-starter-web', version: '1.5.17.RELEASE'
39+
testCompile group: 'org.springframework.boot', name: 'spring-boot-starter-security', version: '1.5.17.RELEASE'
3940
}

‎dd-java-agent/instrumentation/spring-web/src/test/groovy/test/SpringBootBasedTest.groovy‎

Lines changed: 72 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ import org.springframework.boot.test.web.client.TestRestTemplate
1313
import org.springframework.web.bind.MethodArgumentNotValidException
1414
import org.springframework.web.util.NestedServletException
1515

16+
import static test.Application.PASS
17+
import static test.Application.USER
18+
1619
@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT)
1720
class SpringBootBasedTest extends AgentTestRunner {
1821

@@ -25,7 +28,8 @@ class SpringBootBasedTest extends AgentTestRunner {
2528
def "valid response"() {
2629
expect:
2730
port != 0
28-
restTemplate.getForObject("http://localhost:$port/", String) == "Hello World"
31+
restTemplate.withBasicAuth(USER, PASS)
32+
.getForObject("http://localhost:$port/", String) == "Hello World"
2933

3034
and:
3135
assertTraces(1) {
@@ -44,6 +48,7 @@ class SpringBootBasedTest extends AgentTestRunner {
4448
"span.origin.type" "org.apache.catalina.core.ApplicationFilterChain"
4549
"component" "java-web-servlet"
4650
"http.status_code" 200
51+
"$DDTags.USER_NAME" USER
4752
defaultTags()
4853
}
4954
}
@@ -54,7 +59,8 @@ class SpringBootBasedTest extends AgentTestRunner {
5459

5560
def "generates spans"() {
5661
expect:
57-
restTemplate.getForObject("http://localhost:$port/param/asdf1234/", String) == "Hello asdf1234"
62+
restTemplate.withBasicAuth(USER, PASS)
63+
.getForObject("http://localhost:$port/param/asdf1234/", String) == "Hello asdf1234"
5864

5965
assertTraces(1) {
6066
trace(0, 2) {
@@ -72,6 +78,7 @@ class SpringBootBasedTest extends AgentTestRunner {
7278
"span.origin.type" "org.apache.catalina.core.ApplicationFilterChain"
7379
"component" "java-web-servlet"
7480
"http.status_code" 200
81+
"$DDTags.USER_NAME" USER
7582
defaultTags()
7683
}
7784
}
@@ -80,9 +87,61 @@ class SpringBootBasedTest extends AgentTestRunner {
8087
}
8188
}
8289

90+
def "missing auth"() {
91+
setup:
92+
def resp = restTemplate.getForObject("http://localhost:$port/param/asdf1234/", Map)
93+
94+
expect:
95+
resp["status"] == 401
96+
resp["error"] == "Unauthorized"
97+
98+
assertTraces(2) {
99+
trace(0, 1) {
100+
span(0) {
101+
operationName "servlet.request"
102+
resourceName "GET /param/?/"
103+
spanType DDSpanTypes.WEB_SERVLET
104+
parent()
105+
errored false
106+
tags {
107+
"http.url" "http://localhost:$port/param/asdf1234/"
108+
"http.method" "GET"
109+
"span.kind" "server"
110+
"span.type" "web"
111+
"span.origin.type" "org.apache.catalina.core.ApplicationFilterChain"
112+
"component" "java-web-servlet"
113+
"http.status_code" 401
114+
defaultTags()
115+
}
116+
}
117+
}
118+
trace(1, 2) {
119+
span(0) {
120+
operationName "servlet.request"
121+
resourceName "GET /error"
122+
spanType DDSpanTypes.WEB_SERVLET
123+
parent()
124+
errored false
125+
tags {
126+
"http.url" "http://localhost:$port/error"
127+
"http.method" "GET"
128+
"span.kind" "server"
129+
"span.type" "web"
130+
"span.origin.type" "org.apache.catalina.core.ApplicationFilterChain"
131+
"component" "java-web-servlet"
132+
"http.status_code" 401
133+
defaultTags()
134+
}
135+
}
136+
controllerSpan(it, 1, "BasicErrorController.error")
137+
}
138+
}
139+
}
140+
83141
def "generates 404 spans"() {
84142
setup:
85-
def response = restTemplate.getForObject("http://localhost:$port/invalid", Map)
143+
def response = restTemplate.withBasicAuth(USER, PASS)
144+
.getForObject("http://localhost:$port/invalid", Map)
86145

87146
expect:
88147
response.get("status") == 404
@@ -104,6 +163,7 @@ class SpringBootBasedTest extends AgentTestRunner {
104163
"span.origin.type" "org.apache.catalina.core.ApplicationFilterChain"
105164
"component" "java-web-servlet"
106165
"http.status_code" 404
166+
"$DDTags.USER_NAME" USER
107167
defaultTags()
108168
}
109169
}
@@ -134,7 +194,8 @@ class SpringBootBasedTest extends AgentTestRunner {
134194

135195
def "generates error spans"() {
136196
setup:
137-
def response = restTemplate.getForObject("http://localhost:$port/error/qwerty/", Map)
197+
def response = restTemplate.withBasicAuth(USER, PASS)
198+
.getForObject("http://localhost:$port/error/qwerty/", Map)
138199

139200
expect:
140201
response.get("status") == 500
@@ -158,6 +219,7 @@ class SpringBootBasedTest extends AgentTestRunner {
158219
"span.origin.type" "org.apache.catalina.core.ApplicationFilterChain"
159220
"component" "java-web-servlet"
160221
"http.status_code" 500
222+
"$DDTags.USER_NAME" USER
161223
errorTags NestedServletException, "Request processing failed; nested exception is java.lang.RuntimeException: qwerty"
162224
defaultTags()
163225
}
@@ -190,7 +252,8 @@ class SpringBootBasedTest extends AgentTestRunner {
190252

191253
def "validated form"() {
192254
expect:
193-
restTemplate.postForObject("http://localhost:$port/validated", new TestForm("bob", 20), String) == "Hello bob Person(Name: bob, Age: 20)"
255+
restTemplate.withBasicAuth(USER, PASS)
256+
.postForObject("http://localhost:$port/validated", new TestForm("bob", 20), String) == "Hello bob Person(Name: bob, Age: 20)"
194257

195258
assertTraces(1) {
196259
trace(0, 2) {
@@ -208,6 +271,7 @@ class SpringBootBasedTest extends AgentTestRunner {
208271
"span.origin.type" "org.apache.catalina.core.ApplicationFilterChain"
209272
"component" "java-web-servlet"
210273
"http.status_code" 200
274+
"$DDTags.USER_NAME" USER
211275
defaultTags()
212276
}
213277
}
@@ -218,7 +282,8 @@ class SpringBootBasedTest extends AgentTestRunner {
218282

219283
def "invalid form"() {
220284
setup:
221-
def response = restTemplate.postForObject("http://localhost:$port/validated", new TestForm("bill", 5), Map, Map)
285+
def response = restTemplate.withBasicAuth(USER, PASS)
286+
.postForObject("http://localhost:$port/validated", new TestForm("bill", 5), Map, Map)
222287

223288
expect:
224289
response.get("status") == 400
@@ -242,6 +307,7 @@ class SpringBootBasedTest extends AgentTestRunner {
242307
"span.origin.type" "org.apache.catalina.core.ApplicationFilterChain"
243308
"component" "java-web-servlet"
244309
"http.status_code" 400
310+
"$DDTags.USER_NAME" USER
245311
"error" false
246312
"error.msg" String
247313
"error.type" MethodArgumentNotValidException.name

‎dd-java-agent/instrumentation/spring-web/src/test/java/test/Application.java‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,38 @@
22

33
import org.springframework.boot.SpringApplication;
44
import org.springframework.boot.autoconfigure.SpringBootApplication;
5+
import org.springframework.context.annotation.Bean;
6+
import org.springframework.context.annotation.Configuration;
7+
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
8+
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
9+
import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
10+
import org.springframework.security.core.userdetails.User;
11+
import org.springframework.security.core.userdetails.UserDetailsService;
12+
import org.springframework.security.provisioning.InMemoryUserDetailsManager;
513

614
@SpringBootApplication
715
public class Application {
16+
public static final String USER = "username";
17+
public static final String PASS = "password";
18+
private static final String ROLE = "USER";
819

920
public static void main(final String[] args) {
1021
SpringApplication.run(Application.class, args);
1122
}
23+
24+
@Configuration
25+
@EnableWebSecurity
26+
public class WebSecurityConfig extends WebSecurityConfigurerAdapter {
27+
@Override
28+
protected void configure(final HttpSecurity http) throws Exception {
29+
http.csrf().disable().authorizeRequests().anyRequest().authenticated().and().httpBasic();
30+
}
31+
32+
@Bean
33+
@Override
34+
public UserDetailsService userDetailsService() {
35+
return new InMemoryUserDetailsManager(
36+
User.withUsername(USER).password(PASS).roles(ROLE).build());
37+
}
38+
}
1239
}

‎dd-trace-api/src/main/java/datadog/trace/api/DDTags.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ public class DDTags {
88
public static final String THREAD_ID = "thread.id";
99
public static final String DB_STATEMENT = "sql.query";
1010

11+
public static final String USER_NAME = "user.principal";
12+
1113
public static final String ERROR_MSG = "error.msg"; // string representing the error message
1214
public static final String ERROR_TYPE = "error.type"; // string representing the type of the error
1315
public static final String ERROR_STACK = "error.stack"; // human readable version of the stack

0 commit comments

Comments
 (0)