Set user.principal in a way spring security can be covered
Improve tests.
MODIFY
dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java
dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java
@@ -11,6 +11,7 @@ import io.opentracing.SpanContext;
import io.opentracing.propagation.Format;
import io.opentracing.tag.Tags;
import io.opentracing.util.GlobalTracer;
+import java.security.Principal;
import java.util.Collections;
import javax.servlet.ServletRequest;
import javax.servlet.http.HttpServletRequest;
@@ -49,16 +50,24 @@ public class Servlet2Advice {
if (scope instanceof TraceScope) {
((TraceScope) scope).setAsyncPropagation(true);
}
-
- if (httpServletRequest.getUserPrincipal() != null) {
- scope.span().setTag("user.principal", httpServletRequest.getUserPrincipal().getName());
- }
return scope;
}
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void stopSpan(
- @Advice.Enter final Scope scope, @Advice.Thrown final Throwable throwable) {
+ @Advice.Enter final Scope scope,
+ @Advice.Argument(0) final ServletRequest req,
+ @Advice.Thrown final Throwable throwable) {
+ // Set user.principal regardless of who created this span.
+ final Span currentSpan = GlobalTracer.get().activeSpan();
+ if (currentSpan != null) {
+ if (req instanceof HttpServletRequest) {
+ final Principal principal = ((HttpServletRequest) req).getUserPrincipal();
+ if (principal != null) {
+ currentSpan.setTag("user.principal", principal.getName());
+ }
+ }
+ }
if (scope != null) {
final Span span = scope.span();

MODIFY
dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java
dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java
@@ -11,6 +11,7 @@ import io.opentracing.SpanContext;
import io.opentracing.propagation.Format;
import io.opentracing.tag.Tags;
import io.opentracing.util.GlobalTracer;
+import java.security.Principal;
import java.util.Collections;
import java.util.concurrent.atomic.AtomicBoolean;
import javax.servlet.ServletRequest;
@@ -52,10 +53,6 @@ public class Servlet3Advice {
if (scope instanceof TraceScope) {
((TraceScope) scope).setAsyncPropagation(true);
}
-
- if (httpServletRequest.getUserPrincipal() != null) {
- scope.span().setTag("user.principal", httpServletRequest.getUserPrincipal().getName());
- }
return scope;
}
@@ -65,6 +62,16 @@ public class Servlet3Advice {
@Advice.Argument(1) final ServletResponse response,
@Advice.Enter final Scope scope,
@Advice.Thrown final Throwable throwable) {
+ // Set user.principal regardless of who created this span.
+ final Span currentSpan = GlobalTracer.get().activeSpan();
+ if (currentSpan != null) {
+ if (request instanceof HttpServletRequest) {
+ final Principal principal = ((HttpServletRequest) request).getUserPrincipal();
+ if (principal != null) {
+ currentSpan.setTag("user.principal", principal.getName());
+ }
+ }
+ }
if (scope != null) {
if (request instanceof HttpServletRequest && response instanceof HttpServletResponse) {

MODIFY
dd-java-agent/instrumentation/spring-web/spring-web.gradle
dd-java-agent/instrumentation/spring-web/spring-web.gradle
@@ -22,7 +22,9 @@ dependencies {
annotationProcessor deps.autoservice
implementation deps.autoservice
- testCompile project(':dd-java-agent:testing')
+ testCompile(project(':dd-java-agent:testing')){
+ exclude(module: 'jetty-server') // incompatable servlet api
+ }
// Include servlet instrumentation for verifying the tomcat requests
testCompile project(':dd-java-agent:instrumentation:servlet-3')
@@ -32,8 +34,7 @@ dependencies {
testCompile group: 'org.spockframework', name: 'spock-spring', version: '1.1-groovy-2.4'
- testCompile group: 'org.springframework', name: 'spring-web', version: '4.3.14.RELEASE'
- testCompile group: 'org.springframework', name: 'spring-webmvc', version: '4.3.14.RELEASE'
- testCompile group: 'org.springframework.boot', name: 'spring-boot-starter-test', version: '1.5.10.RELEASE'
- testCompile group: 'org.springframework.boot', name: 'spring-boot-starter-tomcat', version: '1.5.10.RELEASE'
+ testCompile group: 'org.springframework.boot', name: 'spring-boot-starter-test', version: '1.5.+'
+ testCompile group: 'org.springframework.boot', name: 'spring-boot-starter-web', version: '1.5.+'
+ testCompile group: 'org.springframework.boot', name: 'spring-boot-starter-security', version: '1.5.+'
}

MODIFY
dd-java-agent/instrumentation/spring-web/src/test/groovy/test/SpringBootBasedTest.groovy
dd-java-agent/instrumentation/spring-web/src/test/groovy/test/SpringBootBasedTest.groovy
@@ -13,6 +13,9 @@ import org.springframework.boot.test.web.client.TestRestTemplate
import org.springframework.web.bind.MethodArgumentNotValidException
import org.springframework.web.util.NestedServletException
+import static test.Application.PASS
+import static test.Application.USER
+
@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT)
class SpringBootBasedTest extends AgentTestRunner {
@@ -25,7 +28,8 @@ class SpringBootBasedTest extends AgentTestRunner {
def "valid response"() {
expect:
port != 0
- restTemplate.getForObject("http://localhost:$port/", String) == "Hello World"
+ restTemplate.withBasicAuth(USER, PASS)
+ .getForObject("http://localhost:$port/", String) == "Hello World"
and:
assertTraces(1) {
@@ -44,6 +48,7 @@ class SpringBootBasedTest extends AgentTestRunner {
"span.origin.type" "org.apache.catalina.core.ApplicationFilterChain"
"component" "java-web-servlet"
"http.status_code" 200
+ "user.principal" USER
defaultTags()
}
}
@@ -54,7 +59,8 @@ class SpringBootBasedTest extends AgentTestRunner {
def "generates spans"() {
expect:
- restTemplate.getForObject("http://localhost:$port/param/asdf1234/", String) == "Hello asdf1234"
+ restTemplate.withBasicAuth(USER, PASS)
+ .getForObject("http://localhost:$port/param/asdf1234/", String) == "Hello asdf1234"
assertTraces(1) {
trace(0, 2) {
@@ -72,6 +78,7 @@ class SpringBootBasedTest extends AgentTestRunner {
"span.origin.type" "org.apache.catalina.core.ApplicationFilterChain"
"component" "java-web-servlet"
"http.status_code" 200
+ "user.principal" USER
defaultTags()
}
}
@@ -80,9 +87,61 @@ class SpringBootBasedTest extends AgentTestRunner {
}
}
+ def "missing auth"() {
+ setup:
+ def resp = restTemplate.getForObject("http://localhost:$port/param/asdf1234/", Map)
+
+ expect:
+ resp["status"] == 401
+ resp["error"] == "Unauthorized"
+
+ assertTraces(2) {
+ trace(0, 1) {
+ span(0) {
+ operationName "servlet.request"
+ resourceName "GET /param/?/"
+ spanType DDSpanTypes.WEB_SERVLET
+ parent()
+ errored false
+ tags {
+ "http.url" "http://localhost:$port/param/asdf1234/"
+ "http.method" "GET"
+ "span.kind" "server"
+ "span.type" "web"
+ "span.origin.type" "org.apache.catalina.core.ApplicationFilterChain"
+ "component" "java-web-servlet"
+ "http.status_code" 401
+ defaultTags()
+ }
+ }
+ }
+ trace(1, 2) {
+ span(0) {
+ operationName "servlet.request"
+ resourceName "GET /error"
+ spanType DDSpanTypes.WEB_SERVLET
+ parent()
+ errored false
+ tags {
+ "http.url" "http://localhost:$port/error"
+ "http.method" "GET"
+ "span.kind" "server"
+ "span.type" "web"
+ "span.origin.type" "org.apache.catalina.core.ApplicationFilterChain"
+ "component" "java-web-servlet"
+ "http.status_code" 401
+ defaultTags()
+ }
+ }
+ controllerSpan(it, 1, "BasicErrorController.error")
+ }
+ }
+ }
+
def "generates 404 spans"() {
setup:
- def response = restTemplate.getForObject("http://localhost:$port/invalid", Map)
+ def response = restTemplate.withBasicAuth(USER, PASS)
+ .getForObject("http://localhost:$port/invalid", Map)
expect:
response.get("status") == 404
@@ -104,6 +163,7 @@ class SpringBootBasedTest extends AgentTestRunner {
"span.origin.type" "org.apache.catalina.core.ApplicationFilterChain"
"component" "java-web-servlet"
"http.status_code" 404
+ "user.principal" USER
defaultTags()
}
}
@@ -134,7 +194,8 @@ class SpringBootBasedTest extends AgentTestRunner {
def "generates error spans"() {
setup:
- def response = restTemplate.getForObject("http://localhost:$port/error/qwerty/", Map)
+ def response = restTemplate.withBasicAuth(USER, PASS)
+ .getForObject("http://localhost:$port/error/qwerty/", Map)
expect:
response.get("status") == 500
@@ -158,6 +219,7 @@ class SpringBootBasedTest extends AgentTestRunner {
"span.origin.type" "org.apache.catalina.core.ApplicationFilterChain"
"component" "java-web-servlet"
"http.status_code" 500
+ "user.principal" USER
errorTags NestedServletException, "Request processing failed; nested exception is java.lang.RuntimeException: qwerty"
defaultTags()
}
@@ -190,7 +252,8 @@ class SpringBootBasedTest extends AgentTestRunner {
def "validated form"() {
expect:
- restTemplate.postForObject("http://localhost:$port/validated", new TestForm("bob", 20), String) == "Hello bob Person(Name: bob, Age: 20)"
+ restTemplate.withBasicAuth(USER, PASS)
+ .postForObject("http://localhost:$port/validated", new TestForm("bob", 20), String) == "Hello bob Person(Name: bob, Age: 20)"
assertTraces(1) {
trace(0, 2) {
@@ -208,6 +271,7 @@ class SpringBootBasedTest extends AgentTestRunner {
"span.origin.type" "org.apache.catalina.core.ApplicationFilterChain"
"component" "java-web-servlet"
"http.status_code" 200
+ "user.principal" USER
defaultTags()
}
}
@@ -218,7 +282,8 @@ class SpringBootBasedTest extends AgentTestRunner {
def "invalid form"() {
setup:
- def response = restTemplate.postForObject("http://localhost:$port/validated", new TestForm("bill", 5), Map, Map)
+ def response = restTemplate.withBasicAuth(USER, PASS)
+ .postForObject("http://localhost:$port/validated", new TestForm("bill", 5), Map, Map)
expect:
response.get("status") == 400
@@ -242,6 +307,7 @@ class SpringBootBasedTest extends AgentTestRunner {
"span.origin.type" "org.apache.catalina.core.ApplicationFilterChain"
"component" "java-web-servlet"
"http.status_code" 400
+ "user.principal" USER
"error" false
"error.msg" String
"error.type" MethodArgumentNotValidException.name

MODIFY
dd-java-agent/instrumentation/spring-web/src/test/java/test/Application.java
dd-java-agent/instrumentation/spring-web/src/test/java/test/Application.java
@@ -2,11 +2,38 @@ package test;
import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.security.config.annotation.web.builders.HttpSecurity;
+import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
+import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
+import org.springframework.security.core.userdetails.User;
+import org.springframework.security.core.userdetails.UserDetailsService;
+import org.springframework.security.provisioning.InMemoryUserDetailsManager;
@SpringBootApplication
public class Application {
+ public static final String USER = "username";
+ public static final String PASS = "password";
+ private static final String ROLE = "USER";
public static void main(final String[] args) {
SpringApplication.run(Application.class, args);
}
+
+ @Configuration
+ @EnableWebSecurity
+ public class WebSecurityConfig extends WebSecurityConfigurerAdapter {
+ @Override
+ protected void configure(final HttpSecurity http) throws Exception {
+ http.csrf().disable().authorizeRequests().anyRequest().authenticated().and().httpBasic();
+ }
+
+ @Bean
+ @Override
+ public UserDetailsService userDetailsService() {
+ return new InMemoryUserDetailsManager(
+ User.withUsername(USER).password(PASS).roles(ROLE).build());
+ }
+ }
}

Set user.principal in a way spring security can be covered

Improve tests.