0

In this tutorial the author uses a global variable for the RestTemplate in a @Controller. For an incoming request, he extracts the Bearer token out of the request and adds an interceptor that adds the token to the outgoing requests of the RestTemplate.

I think, there might be a race condition. A request of a second user might get the interceptor from a first user and therefore authenticates as the first user.

According to Okta, they tested the code and did not experience any race condition. Is there some mechanism in Spring that ensures this?

  • the only thing i can see which seems bad is that he adds a new interceptor in each request, which will just add on and add on and add on, since the RestTemplate i presume is a singleton – Toerktumlare Feb 18 '21 at 23:39
  • so i presume each of his requests will contain about 1 gazillion Authentication headers since it adds one header for each request. – Toerktumlare Feb 18 '21 at 23:46
  • Yes, this is true as well but furthermore, since the RestTemplate is a kind of singelton, one request will also have the interceptors of other requests, right? Therefore, my requests will have tokens from other users? Or are the interceptors bound to a request? Which I don't believe. – Christof Tinnes Feb 19 '21 at 09:35
  • a request can have multiple of the same header, which means its all about how the implementation on the receiving side is written, if it uses all headers, usually people ony take the first header, but i would say, his solution is bad. – Toerktumlare Feb 19 '21 at 09:50
  • But from a security point of view it is already dangerous that wrong tokens are transmitted. Image it is a GET request for a banking account. Could lead to see the account of someone else :-D But the race condition isn't there you mean? So imagine the other side would always take the first header: I think still, it could happen that a user gets the wrong token, because of a race condition. – Christof Tinnes Feb 19 '21 at 20:17
  • if its thread safe, im going to answer "probably" not, but more than that i dont want to say until one does some load tests. I would at least not approve that code in a production evironment. – Toerktumlare Feb 19 '21 at 20:35

1 Answers1

0

RestTemplate itself is thread safe.

The code in question:

restTemplate.getInterceptors().add(getBearerTokenInterceptor(accessToken.getTokenValue()));

... does seem like it could have issues. I'm not sure since I haven't tried to see if it is with unit or integration tests.

From the getInterceptors() Javadoc:

/**
 * Get the request interceptors that this accessor uses.
 * <p>The returned {@link List} is active and may be modified. Note,
 * however, that the interceptors will not be resorted according to their
 * {@linkplain AnnotationAwareOrderComparator#sort(List) order} before the
 * {@link ClientHttpRequestFactory} is built.
 */

You could try using setInterceptors() instead, since it replaces the interceptors.

Matt Raible
  • 8,187
  • 9
  • 61
  • 120