2

I have a service that requires to read a special http query parameter. Therefore I have to access the current HttpServletRequest somehow. As I cannot hand the request down as a parameter, I have to inject or read it somehow.

There are two possibilities: either get the request from RequestContextHolder, or directly inject HttpServletRequest. What is correct? Or maybe there is even a 3rd option?

@Service
public class MyUserDetailsService implements UserDetailsService {
    //TODO is that correct? scope?
    @Autowired
    private HttpServletRequest req;

    @Override
    public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException {
        HttpServletRequest req = ((ServletRequestAttributes) RequestContextHolder.currentRequestAttributes()).getRequest();
        req.getParameter("myparam");
   }
}

Is that threadsafe, as MyUserDetailsService is obviously as singleton, and HttpServletRequest is supposed to be @RequestScope?

membersound
  • 81,582
  • 193
  • 585
  • 1,120
  • 1
    why don't you read that query param in controller and pass it through service? – Ryuzaki L Jul 30 '19 at 13:52
  • Because `UserDetailsService` is a spring class, thus I cannot change method signatnure as it is likewise called by an internal spring service. – membersound Jul 30 '19 at 13:54
  • `public UserDetails loadUserByUsername(String username, HttpServletRequest request) throws UsernameNotFoundException ` – Zorglube Jul 30 '19 at 14:05
  • Don't... You now have bound your service layer to your web layer... While the dependency should be the other way around. What is it you need the parameter for? Looks like you are using the wrong component (`UserdetailsService` to achieve something). That being said, will it work, yes it will as the `HttpServletRequest` is an actual proxy to the current request for the executing thread and thus isolated. Should you do it, no you shouldn't. – M. Deinum Jul 30 '19 at 14:10
  • @M.Deinum I'm configuring the `AuthenticationManagerBuilder` with two `AuthenticationProvider`. One for database lookup, and one for a legacy file lookup. Each provider is a `DaoAuthenticationProvider` that uses a custom `UserDetailsService`, which is my strategy to either fetch the user from db, or from legacy file. But in case of the legacy service lookup, I need an additional parameter from the query to support the legacy lookup. That's why I have to get access to the `HttpServletRequest`, and cannot hand it down as parameter. – membersound Jul 30 '19 at 14:14
  • To avoid the dependency highlighted above create a ServletFilter which stores the value in a `ThreadLocal` which can then be retrieved via a static call in the service. Similar to what you have now with `RequestContextHolder` but avoids dependency on the Servlet API in your services. – Alan Hay Jul 30 '19 at 14:17
  • What do you need to additional parameter for? – M. Deinum Jul 30 '19 at 14:18
  • @AlanHay an interceptor won't work, as this executes in a filter chain, which is executed before the `DispatcherServlet` and thus before any `HandlerInterceptor` can do anything. – M. Deinum Jul 30 '19 at 14:19
  • ServletFilter would work though? – Alan Hay Jul 30 '19 at 14:20
  • @M.Deinum I need the additional parameter for querying the legacy file (where passwords are not simple user-pass combinations, but also depend on a 3rd parameter (that I have in the url). – membersound Jul 30 '19 at 14:32
  • Then simply extend the `WebAuthenticationDetailsSource` to add the needed parameter as detail to the `UsernamePasswordAuthentication` token and in your custom `AuthenticationProvider` (which shouldn't be a `DaoAuthenticationProvider`!) obtain this additional property from the `getDetails` method on the `Authentication` object. That way you have decoupled everything. – M. Deinum Jul 31 '19 at 05:48
  • Could you give an example on how `WebAuthenticationDetailsSource` connects to `UsernamePasswordAuthentication`? I could not find any connection it itself nor in its `WebAuthenticationDetails`. – membersound Jul 31 '19 at 09:15

3 Answers3

2

You can get HttpServletRequest in this way if you don't want to use @Autowired in the controller:

HttpServletRequest request = 
((ServletRequestAttributes) RequestContextHolder.currentRequestAttributes())
.getRequest();
august0490
  • 777
  • 1
  • 9
  • 15
0

I don't know what exactly are you train to do, but you need to know: With HttpServletRequest in a service class, you will have an error because servelet request and Spring Components like Services was in a different scope, and you can't access it when HttpServletRequest was not in the same thread. I have almost the same issue, in my case, I needed to get a user ID in JWT header and save to make some queries. So I created a filter in Spring Security chain and did use the SecurityContextHolder class. Here my code:

@Slf4j
public class JWTAuthenticationFilter extends GenericFilterBean {

    private String enviroment;
    private JWTAuthenticationService jwtAuthenticationService;

    public JWTAuthenticationFilter(String enviroment,
                                   JWTAuthenticationService jwtAuthenticationService) {
        this.enviroment = enviroment;
        this.jwtAuthenticationService = jwtAuthenticationService;
    }

    @Override
    public void doFilter(ServletRequest request, ServletResponse response, FilterChain filterChain) throws IOException, ServletException {

        Claims jwtClaims = this.jwtAuthenticationService.getJWTClaims((HttpServletRequest) request);

        try {
            this.jwtAuthenticationService.checkJWT(enviroment, jwtClaims);
        } catch (UnauthorizedException ex){
            log.error("Token JWT Inválido");
            ((HttpServletResponse) response).sendError(401, ex.getMessage());
            return;
        }

        Authentication authentication = this.jwtAuthenticationService.getAuthentication(jwtClaims);

        SecurityContextHolder.getContext().setAuthentication(authentication);
        filterChain.doFilter(request, response);
    }
}


@Service
public class AuthRequestService {

  public String getAuthorizationKey() {
    Claims claims = (Claims)SecurityContextHolder.getContext().getAuthentication().getPrincipal();
    return claims.get(KEY_CLAIM).toString();
  }

}

With this approach, I avoid the thread error because I'm using the current context that SecurityContextHolder manager.

Rodrigo Sene
  • 309
  • 1
  • 13
-1

Injecting it directly should work, but it will be Request Scope, not Session Scope.

Injecting the Request-scoped bean will result in a proxy being created. Accessing methods on that proxy from a thread that is not originating from the DispatcherServlet will result in an IllegalStateException being thrown, because there is no available request according to the proxy (in actuality there might be zero or many concurrent requests, but it's impossible to deterministically bind to one). However, when accessing the proxy's method from a thread that is actually part of an HTTP request, in your case probably coming in the Controller->Service, it will act just as if it was the current HttpServletRequest.

I'm not sure that the Service Layer is the right place to be binding HttpServletRequest as it might be possible that the service is access from other asynchronous contexts, although I'm not sure of your use-case. In any case, as long as your architecture is well defined and the class depending on the HttpServletRequest bean is only using it in a request thread/context, it will work fine.

So in summary, what you should do really depends on your use-case. It sounds like you will only be accessing it from within the DispatcherServlet and would thus be safe, but perhaps you should abstract (separate the concern of) the acquiring of the data to another class that provides it to the Service class.

rewolf
  • 5,561
  • 4
  • 40
  • 51
  • 1
    As `MyUserDetailsService` is probably a Singleton is this approach thread safe? (Not my down vote) This answer states yes however no supporting documentation quoted: https://stackoverflow.com/questions/48574780/autowired-httpservletrequest-vs-passing-as-parameter-best-practice/48575275 – Alan Hay Jul 30 '19 at 14:06
  • Requests are tied to threads. So if a thread that is accessing your service is doing so as part of a request, then this will be populated with the current request. If from another thread context, it will be null. – rewolf Jul 30 '19 at 15:44
  • MyUserDetailsService most likely is SIngleton scoped though. But there's another answer here that suggests it is thread safe. https://stackoverflow.com/questions/49680692/spring-mvc-can-i-autowire-httpservletrequest-in-restcontroller – Alan Hay Jul 30 '19 at 15:55
  • I've updated my answer. What do you mean by Thread-safe exactly? – rewolf Jul 31 '19 at 01:57