Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review and Feedback #1

Open
jgrandja opened this issue Jun 30, 2017 · 13 comments
Open

Review and Feedback #1

jgrandja opened this issue Jun 30, 2017 · 13 comments

Comments

@jgrandja
Copy link

Hi Karthik,

Instead of providing a specialized version of BearerTokenExtractor that does the swap of opaque to Jwt token, I would recommend supplying a custom ResourceServerTokenServices that would encapsulate all that logic you currently have in BearerTokenExtractor.

The responsibility of a ResourceServerTokenServices is to convert/translate the String representation of a token to either a OAuth2Authentication or an OAuth2AccessToken.

So in ResourceServerConfig.configure(ResourceServerSecurityConfigurer) configure this ResourceServerSecurityConfigurer.tokenServices(customResourceServerTokenServices).

NOTE: Your custom implementation of ResourceServerTokenServices would need a reference to JwtTokenStore so it knows how to convert a Jwt token to a OAuth2Authentication or an OAuth2AccessToken.

If you want to dig deeper in the stack to understand how the request flows, here is the sequence:

  1. On the Resource Server end of things, OAuth2AuthenticationProcessingFilter gets called first.
  2. OAuth2AuthenticationProcessingFilter uses the default TokenExtractor ( BearerTokenExtractor) to get the token from the Authorization header and return it in a PreAuthenticatedAuthenticationToken.
  3. OAuth2AuthenticationManager.authenticate() is then called passing in the PreAuthenticatedAuthenticationToken.
  4. OAuth2AuthenticationManager will leverage the ResourceServerTokenServices you configured via ResourceServerSecurityConfigurer.tokenServices(customResourceServerTokenServices).
  5. The custom implementation of ResourceServerTokenServices will convert the Jwt token to an OAuth2Authentication via OAuth2Authentication loadAuthentication(String accessToken). This is where the JwtTokenStore.readAuthentication(String) will get leveraged for the translation. If the token is opaque/reference then call the identity service to swap for the Jwt and continue with the flow as above using the JwtTokenStore for translation.

Take a look at RemoteTokenServices which implements ResourceServerTokenServices. This might be of use for you.

The other thing I noticed is the use of @PreAuthorize on your Rest endpoints:

public class SampleRestController {

	@ResponseBody
	@RequestMapping("/")
	@PreAuthorize("isAuthenticated() && #oauth2.hasScope('write') && @sampleSecurityService.hasPermission(authentication)")
	public String a(@RequestParam(required = false, name = "useFeign") boolean useFeign,
			OAuth2Authentication authentication) {

@PreAuthorize is typically used for method level security beyond the web-tier, specifically, the service or data tier.

For the web tier, you would configure your authorization rules as follows:

public class ResourceServerConfig extends ResourceServerConfigurerAdapter {

	private final TokenStore tokenStore;

	private final RestTemplate exchangeRestTemplate;

	/**
	 * Configure the access rules for securing the resources.
	 */
	@Override
	public void configure(HttpSecurity http) throws Exception {
		http
				.cors()
				.and()
				.authorizeRequests()
					.antMatchers("/**").access("isAuthenticated() && #oauth2.hasScope('write') && @sampleSecurityService.hasPermission(authentication)");
	}
@jgrandja
Copy link
Author

Look into and see if Wso2 implements the JSON Web Key spec.

If it does, then you can leverage the JwkTokenStore which provides the capability of verifying the JWS (Jwt signature) using the public key(s) exposed by the JwkSet endpoint.

alwaysastudent pushed a commit that referenced this issue Jun 30, 2017
Addresses one of the feedbacks from #1
@alwaysastudent
Copy link
Owner

alwaysastudent commented Jun 30, 2017

Hi @jgrandja - Appreciate your valuable feedback. I have incorporated one of the suggestions already and I have a question for the other.

  • I removed the JwtTokenStore from ResourceServerSecurityConfigurer and added a custom ResourceServerTokenServices into that as per your your suggestion, it works and it makes perfect sense. Commit 8ed096e ; I was using a custom BearerTokenExtractor since I had JwtTokenStore configured in with the ResourceServerSecurityConfigurer.tokenStore(). And I'll get an exception on entry within the OAuth2AuthenticationProcessingFilter, I use anything other than a JWT token. Note - I may still have to have a custom BearerTokenExtractor since we carry this opaque token using a cookie on our website, or I have a better option ?

  • I see you are suggesting we should be defining the auth rules, at the filter level as a part of ResourceServerConfigurerAdapter and not in the RestController methods. But a RestController endpoint can have multiple resources with various different access control policies, and we can multiple such controllers within an application. Doesn't it get a bit gnarly to maintain these rules on a central config that will be detached from the function ? Please check the 3 resources under UserServiceRestController where I have different access rules given in a declarative manner. I don't know if it is subjective, but it looks a bit more intuitive at the resource level, than in a central ResourceServerConfigurerAdapter. Kindly advise on this.

  • I will check about the Jwk spec and if it has been supported on our Identity provider.

Thanks again for helping.

alwaysastudent pushed a commit that referenced this issue Jun 30, 2017
Incorporating feedback #1
@jgrandja
Copy link
Author

jgrandja commented Jul 4, 2017

You don't need a custom TokenExtractor, the BearerTokenExtractor which is set as the default in OAuth2AuthenticationProcessingFilter should be sufficient.

Your custom ResourceServerTokenServices should be able to handle both the opaque (reference) token as well as the Jwt token.

As far as defining the auth rules...if you define them at the endpoints using @PreAuthorize then you are NOT using web security, instead you are using method security. A very important thing to be aware of here is that the authorization check will proceed through the springSecurityFilterChain onto the DispatcherServlet and just before the endpoint service handler to an AOP method advice. You really want the springSecurityFilterChain to apply the authorization check and either allow/disallow the request to the DispatcherServlet and eventually to the endpoint handler.

HttpSecurity was specifically designed for web security. So if you want to protect you web endpoints than you use HttpSecurity. As an example, using your UserServiceRestController, the authorization rules in ResourceServerConfig would be as follows:

  public void configure(HttpSecurity http) throws Exception {
    http
            .cors()
            .and()
            .authorizeRequests()
              .mvcMatchers("/user/{guid}").access("(hasRole('USER') || hasRole('CS')) && #oauth2.hasScope('read')")
              .mvcMatchers("/user/create").access("hasRole('WEB') && #oauth2.hasScope('write')")
              .mvcMatchers("/user/update/{guid}").access("#oauth2.hasScope('write') && (hasRole('CS') || @userServiceSecurity.doesGuidMatch(authentication, #guid))")
              .anyRequest().authenticated();
  }

Here are some resources that you can read on how Authorization is implemented in Spring Security:

http://docs.spring.io/spring-security/site/docs/5.0.0.M2/reference/htmlsingle/#jc-authorize-requests
http://docs.spring.io/spring-security/site/docs/5.0.0.M2/reference/htmlsingle/#authorization

@alwaysastudent
Copy link
Owner

alwaysastudent commented Jul 4, 2017

Hmm, I just want to mention that we also use a Cookie to carry the opaque (reference) token information. i.e - we can expect the header Authorization: Bearer OR a Cookie to have the token's value. Sorry I am bringing this up again. In those situation, should't we use a custom TokenExtractor to extract this value out from the cookie ?

Thank you so much for explaining the differences between method security versus the web security. The examples are so handy and useful ! We will use the HttpSecurity for protecting the resources, it makes a lot of sense. But I still feel It would have been so cool, if we had a way to declaratively perform all these web security, using annotations that would implicitly register something like a ResourceServerConfigurer :) Since it is not possible, do you think it is a good practice to have each RestController extend ResourceServerConfigurerAdapter and have their own specific HttpSecurity configuration ? I saw we can have multiple ResourceServerConfigurer defined, that where my thought came from to have each RestController be a ResourceServerConfigurer itself.

I have another question, which is slightly off topic but if you can give any sort of advise from your experience, that would be awesome. In our applications we deal with a lot of service to service calls. Calls come from internet into one service which call another service and that would in turn call others and so on. All the services would need to share a particular request context and behave in a uniform manner (OAuth2, i18N, A/B testing profile etc). In our legacy system we achieve this using headers, and we establish the context by propagation/relay of these headers across the services. When we move to a cloud native platform, what might be an efficient way to achieve this, other than having to rely on some custom code to propagate these custom headers across ? Since this situation is not specifically related to security, it is not clean to use JWT as a carrier for all of these information either. Do you have any advise on this ?

@jgrandja
Copy link
Author

jgrandja commented Jul 5, 2017

Yes, if the reference token is contained in a Cookie then you would use TokenExtractor to get access to the HttpServletRequest for extraction.

Having each RestController extend ResourceServerConfigurerAdapter is not a good idea from a design standpoint. RestController is a Controller it is not a ResourceServerConfigurerAdapter if you look at it from a logical standpoint. No to mention, there may be side affects if you were to use this approach.

Agreed that JWT is not meant to be used to propogate request context information so I don't suggest that approach. What I have seen on a number of occasions is a combination of a distributed cache that synchronizes with a local cache to the service. Each service has a local cache that is synchronized from the remote distributed cache. The local cache promotes latency since the service will get data from the local cache rather then making a network call to the distributed cache. However, there may be cache misses at times which at point a network call will happen but the goal is to prevent these network calls to the distributed cache in most cases. It's a matter of setting up caches in an efficient manner. There is a lot more to talk about with this type of design within the overall architecture but at least it's something to go on. I would suggest reaching out to the Platform Architect on the account as I'm sure they will be able to advise further on this.

@alwaysastudent
Copy link
Owner

alwaysastudent commented Jul 6, 2017

@jgrandja - Thank you for the review and feedback. I think we are at a good spot now and feel a lot comfortable. I will reach back again if there are any questions.

Regarding the request context propagation, if you are using some cache are you implying something like a session store ? and the session id be getting propagated ?

@alwaysastudent
Copy link
Owner

btw, the customization for ResourceServerTokenServices seems like causing an undesirable side effect.

Please check the code spring-security-oauth/OAuth2AuthenticationManager.java#L103 where you would see the auth object returned from the tokenServices.loadAuthentication is being mutated with the details from the method argument authentication. [auth.setDetails(authentication.getDetails());]

This cause the opaque token to be relayed, rather than the inferred JWT. Since the OAuth2RestTemplate and Feign uses OAuth2ClientContext.getAccessToken which pulls out this opaque token details. Check the request scope bean configuration for OAuth2ClientContext

https://github.com/spring-projects/spring-boot/blob/master/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/client/OAuth2RestOperationsConfiguration.java#L146

@alwaysastudent
Copy link
Owner

@jgrandja - any suggestions on the above finding ?

@jgrandja
Copy link
Author

jgrandja commented Jul 7, 2017

Interesting - regarding the Boot auto-config RequestScopedConfiguration. I'm not sure why the logic is the way it is - using OAuth2AuthenticationDetails.getTokenValue() to set the AccessToken to use.

The best way around this is to ensure this auto-config class doesn't kick in. RequestScopedConfiguration has this annotation defined @ConditionalOnMissingBean(OAuth2ClientConfiguration.class)

This means if you have configured OAuth2ClientConfiguration in the context than RequestScopedConfiguration won't kick in.

@EnableOAuth2Client will import OAuth2ClientConfiguration so using this annotation will ensure the Boot auto-config backs off. Try this and hopefully that will work for you.

Regarding the request context propagation, no I wasn't suggesting a session store or session id propagation. I'm specifically talking about a X-Request-ID header, for example, that get's created at the API Gateway service and is propogated in request headers downstream to all service. Each service would use the X-Request-ID header to lookup request context info in the cache. This type of thing is difficult to explain in writing so I would again suggest reaching out to the Platform Architect on the account to discuss. This is a fairly common pattern so I'm sure he/she will be able to help.

@alwaysastudent
Copy link
Owner

alwaysastudent commented Jul 21, 2017

It seems @EnableOAuth2Client/OAuth2ClientConfiguration is not a right approach. It is used for creating clients that would require to acquire a fresh access token. This won't relay the already existing one that is there in the context.

@alwaysastudent
Copy link
Owner

@jgrandja - do you have any suggestions ?

If I use @EnableOAuth2Client, the oauth2ClientContext is initialized on a session-scope without any access token set.

Check OAuth2RestOperationsConfiguration.SessionScopedConfiguration and OAuth2ClientConfiguration

Due to the above set up, if I make a call outbound with an OAuth2RestTemplate or with a Feign client (which implicitly would engage OAuth2FeignRequestInterceptor), they will not get the current access token from the oauth2ClientContext and will try to acquire a new access token using the accessTokenProvider.obtainAccessToken.

Check OAuth2RestTemplate and OAuth2FeignRequestInterceptor

For example, the call stack forOAuth2RestTemplate would look like this

org.springframework.security.oauth2.client.token.AccessTokenProviderChain.obtainAccessToken(OAuth2ProtectedResourceDetails, AccessTokenRequest)
org.springframework.security.oauth2.client.OAuth2RestTemplate.acquireAccessToken(OAuth2ClientContext)
org.springframework.security.oauth2.client.OAuth2RestTemplate.getAccessToken()
org.springframework.security.oauth2.client.OAuth2RestTemplate.createRequest(URI, HttpMethod)
org.springframework.web.client.RestTemplate.doExecute(URI, HttpMethod, RequestCallback, ResponseExtractor<T>)

Note - If I don't have the @EnableOAuth2Client annotation, OAuth2RestOperationsConfiguration.RequestScopedConfiguration would kick in which sets up the current access token within the oauth2ClientContext,

Check OAuth2RestOperationsConfiguration.RequestScopedConfiguration

@jgrandja
Copy link
Author

Yes, the @EnableOAuth2Client is for configuring a new OAuth2 client which will obtain a new access token if it doesn't already have one. I don't recall suggesting this for relaying an existing access token?

I'm not familiar with the Feign client but the best place to have questions answered time efficiently is via Gitter and by joining the appropriate room, for example, the Spring Cloud room.

@alwaysastudent
Copy link
Owner

Hi @jgrandja, thank you for the feedback.

Yes, the @EnableOAuth2Client is for configuring a new OAuth2 client which will obtain a new access token if it doesn't already have one

This is the catch, I am not seeing this behavior. The behavior it induces on OAuth2RestTemplate is, it always tries to obtain a new access token since it does not set the current access token on oauth2ClientContext

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants