Capture identity attributes (enduser.*) for applications using Spring Security

[philsttr]

Is your feature request related to a problem? Please describe.

General identity attributes (enduser.id, enduser.role, enduser.scope) are not captured for SERVER spans in applications using Spring Security OAuth2 Resource Server for Servlet or WebFlux based applications.

The existing servlet instrumentation currently captures the enduser.id in a couple places (here and here). However, in an Servlet-based application using Spring Security OAuth2 Resource Server, the request.getUserPrincipal() returns null at this level. It returns null here because Spring Security wraps that request object in a request wrapper at a higher level, and the user principal is only returned from the wrapper at the higher level. In other words, since the servlet instrumentation only has access to the lower level request, and not the higher level request wrapper, the user principal seen by the servlet instrumentation is null, and therefore the enduser.id is not captured.

In addition, there is no existing instrumentation for capturing the enduser.id in WebFlux applications.

Describe the solution you'd like

I would like general identity attributes (enduser.id, enduser.role, enduser.scope) to be captured for SERVER spans in applications using Spring Security OAuth2 Resource Server for Servlet and WebFlux based applications.

Describe alternatives you've considered

No response

Additional context

While the new instrumentation probably needs to be Spring Security specific, it does not necessarily need to be specific to OAuth2 Resource Servers authentication/authorization. i.e. The new instrumentation could probably work for any type of Spring Security based authentication/authorization.

[mateuszrzeszutek]

Hey @philsttr ,

I think this'd be a welcome addition 👍 We'd have to make the new instrumentation disabled by default because of the

Given the sensitive nature of this information, SDKs and exporters SHOULD drop these attributes by default and then provide a configuration parameter to turn on retention for use cases where the information is required and would not violate any policies or regulations.

fragment in the spec theough.

[philsttr]

Agreed. Probably should do that for the existing servlet instrumentation that sets enduser.id as well, but that's probably another issue.

[swar8080]

Maybe these attributes could be captured for most spring security use cases by instrumenting SecurityContextHolder#setContext? From there a list of GrantedAuthority are accessible, which is where roles and scopes are stored. The spring convention is that roles start with the prefix ROLE_, and the oauth2 resource server library stores scopes as granted authorities prefixed with SCOPE_ by default

One of the things i'm not sure about is this part of the convention:

It is expected this information would be propagated unchanged from node-to-node within the system using the Baggage mechanism. These attributes should not be used to record system-to-system authentication attributes.

For example, i've seen production queue consumer code that manually calls SecurityContextHolder#setContext. Including these attributes as baggage on HTTP requests by that queue consumer could be unexpected. So maybe more focused instrumentation is needed to avoid that, like on the specific spring security oauth2 resource server http filters.

[philsttr]

Filed #9740 for updating the existing servlet instrumentation to not capture enduser.id by default.

[philsttr]

Looking for advice on how to best implement this requirement..

Given the sensitive nature of this information, SDKs and exporters SHOULD drop these attributes by default and then provide a configuration parameter to turn on retention for use cases where the information is required and would not violate any policies or regulations.

Should this requirement be implemented by:

1. having each instrumentation that captures the endpoint.* attributes look at a common configuration value to determine if it should capture them, OR
2. have each instrumentation always capture them, but then have something later (the exporter?) filter them out based on configuration ?

Also looking for suggestions on what the configuration property should be named. If we go with option 1 (where instrumentations check a config property), then perhaps otel.instrumentation.common.capture-enduser.enabled=true/false

[mateuszrzeszutek]

IMO option 1 should be chosen as the solution here -- treat this attribute as an opt-in thing, and only collect it when the user explicitly configures the agent to do so.

If we go with option 1 (where instrumentations check a config property), then perhaps otel.instrumentation.common.capture-enduser.enabled=true/false

SGTM 👍

[philsttr]

For example, i've seen production queue consumer code that manually calls SecurityContextHolder#setContext. Including these attributes as baggage on HTTP requests by that queue consumer could be unexpected.

I agree. I have seen a lot of code other than inbound authentication code call SecurityContextHolder#setContext. So I think if we were to instrument that method, it would catch too many use cases outside of the scope of setting the enduser.* attributes for SERVER spans.

So maybe more focused instrumentation is needed to avoid that, like on the specific spring security oauth2 resource server http filters.

I agree with this. After looking through spring security code a bit, I think the authentication related filters would be the best place to capture the enduser.* attributes for SERVER spans. Here are the the locations that I have found to capture the enduser.* attributes for the various types of authentication:

Reactive:

• AuthenticationWebFilter.onAuthenticationSuccess
• OAuth2AuthorizationCodeGrantWebFilter.onAuthenticationSuccess

Servlet:

• AuthenticationFilter.successfulAuthentication
• AbstractPreAuthenticatedProcessingFilter.successfulAuthentication
• AbstractAuthenticationProcessingFilter.successfulAuthentication
• RememberMeAuthenticationFilter.onSuccessfulAuthentication
• BasicAuthenticationFilter.onSuccessfulAuthentication
• BearerTokenAuthenticationFilter.doFilterInternal (wrap FilterChain on entry with a FilterChain that captures the current Authentication)
• OAuth2AuthorizationCodeGrantFilter (wrap AuthenticationManager in constructor with an AuthenticationManager that captures returned Authentication)

[trask]

IMO option 1 should be chosen as the solution here -- treat this attribute as an opt-in thing, and only collect it when the user explicitly configures the agent to do so.

related: https://github.com/open-telemetry/semantic-conventions/issues/128#issuecomment-1775594259

The guidance to not generate PII by default is temporary, pending better enable/disable features in the SDK for full user control.

[philsttr]

I just filed https://github.com/spring-projects/spring-security/issues/14046 to see if the Spring Security team would consider adding a hook that could be used by instrumentation to capture these attributes, or to see if they would recommend a different approach.

[philsttr]

New approach, as suggested by the spring-security folks...

Add a filter into the spring security filter chain. The new filter is ordered after the filters that setup the spring security context. Therefore, the new filter can retrieve the Authentication and populate the enduser.* attributes.

More specifically

• Servlet apps: instrument org.springframework.security.config.annotation.web.builders.HttpSecurity.build() to add a filter before the AuthorizationFilter
• Reactive apps: instrument org.springframework.security.config.web.server.ServerHttpSecurity.build() to add a filter before the logout filter

[philsttr]

For reference: https://github.com/open-telemetry/opentelemetry-java-instrumentation/pull/9777 adds the OpenTelemetry instrumentation described above