Spring-security: WebSocket matchers ignore parameters

Created on 25 Jul 2017  路  7Comments  路  Source: spring-projects/spring-security

Summary

I want to authorize topic subscription by topic name. .simpSubscribeDestMatchers("/topic/list/{location}/**") .access("@webSecurity.checkLocation(authentication,#location)")

Actual Behavior

My location parameter is not passed to webSecurity.checkLocation(). The method is called, but the parameter is null.

Expected Behavior

Correct, non-null, location parameter passed to webSecurity.checkLocation() method. According to the documentation this is possible for antMatchers
https://docs.spring.io/spring-security/site/docs/current/reference/htmlsingle/#el-access-web-path-variables

Please suggest workarounds if exist.

Configuration

@Configuration
public class WebSocketSecurityConfig extends AbstractSecurityWebSocketMessageBrokerConfigurer {

    @Override
    protected boolean sameOriginDisabled() {
        return true;
    }

    @Override
    protected void configureInbound(MessageSecurityMetadataSourceRegistry messages) {
        messages //
                .nullDestMatcher().authenticated() //
                .simpDestMatchers("/app/**").authenticated() //
                .simpSubscribeDestMatchers("/topic/list/{location}/**")
                      .access("@webSecurity.checkLocation(authentication,#location)") //
                .anyMessage().denyAll();
    }
}

Version

compile group: 'org.springframework.security.oauth', name: 'spring-security-oauth2', version: '2.1.1.RELEASE'

compile group: 'org.springframework.security', name: 'spring-security-messaging', version: '4.2.3.RELEASE'

Sample

messaging enhancement

Most helpful comment

@rwinch I made a PR with my first aproach to solve this issue, I'll be happy to know your opinion about my approach and to make any adjustments in the case that may be necessary.

All 7 comments

I found a workaround.

You can use implicit variable message:
.simpSubscribeDestMatchers("/topic/list/*") .access("@webSocketSecurity.checkLocationByMsg(authentication,message)")

And you need extra work to be done in the service:

@Component
public class WebSocketSecurity {

    @Autowired
    private WebSecurity webSecurity;

    public boolean checkLocationByMsg(Authentication authentication, Message<?> message) {

        StompHeaderAccessor sha = StompHeaderAccessor.wrap(message);
        String topic = sha.getDestination();
        String location = topic.replace(MyConstants.listTopicPrefix, "");

        return webSecurity.checkLocation(authentication, location);
    }

Here is my (even more hackish) approach to extract the path variables:

@Configuration
public class WebSocketSecurityConfig extends AbstractSecurityWebSocketMessageBrokerConfigurer {

  // see https://docs.spring.io/spring-security/site/docs/current/reference/htmlsingle/#websocket

  @Override
  protected void configureInbound(MessageSecurityMetadataSourceRegistry messages) {

    messages
        .nullDestMatcher().hasRole("USER")
        // BUG: Ant Style matchers do not return the value (https://github.com/spring-projects/spring-security/issues/4469)
        .simpSubscribeDestMatchers(SUBSCRIBE_TO_USER_EVENTS_PATTERN)
        .access("@webSecurity.checkUserCanSubscribeTo(authentication,message)")

        .simpSubscribeDestMatchers("/topic/*").hasRole("USER")
        .simpTypeMatchers(MESSAGE, SUBSCRIBE).denyAll()
        .anyMessage().denyAll();
  }

  @Bean
  public WebSecurity webSecurity() {
    return new WebSecurity();
  }

  /**
   * Checks that the current logged in user is allowed to subscribe  based on
   * paths.
   *
   * The path pattern "{userId}" is used to restrict access based on the current principal.
   *
   * E.g. the logged in user "user_a"  ...
   * * can subscribe to "/user/queue/accounts/user_a/events"
   * * cannot subscribe to "/user/queue/accounts/user_b/events"
   *
   * This class is for demonstration and an ugly workaround for https://github.com/spring-projects/spring-security/issues/4469
   */
  public static class WebSecurity {

    final static String SUBSCRIBE_TO_USER_EVENTS_PATTERN = "/user/queue/accounts/{userId}/events";

    private final static String USER_ID = "userId";

    public boolean checkUserCanSubscribeTo(Authentication authentication, Message<?> message) {

      final String targetedUserId = extractUserId(SUBSCRIBE_TO_USER_EVENTS_PATTERN, message);

      return isAuthenticatedAndInUserRole(authentication)
          && validDestinationUserForSubscribing((User) authentication.getPrincipal(),
          targetedUserId);
    }

    boolean validDestinationUserForSubscribing(User principal, String subscribeTo) {
      return principal != null && principal.getUsername().equalsIgnoreCase(subscribeTo);
    }

    private boolean isAuthenticatedAndInUserRole(Authentication authentication) {
      return authentication.isAuthenticated()
          && authentication.getAuthorities().stream()
          .anyMatch(authority -> "ROLE_USER".equals(authority.getAuthority()));
    }

    private String extractUserId(String pattern, Message<?> message) {
      StompHeaderAccessor sha = StompHeaderAccessor.wrap(message);
      String topic = sha.getDestination();

      final AntPathMatcher matcher = new AntPathMatcher("/");
      if (!matcher.match(pattern, topic)){
        return null;
      }
      final Map<String, String> uriTemplateVariables = matcher
          .extractUriTemplateVariables(pattern, topic);
      return uriTemplateVariables.get(USER_ID);
    }
  }

}

If anyone is interested in submitting a PR I'd be happy to help get this merged in

@rwinch I'd like to take this one

@Daniel69 Thanks for volunteering! The issue is all yours. If have any questions or need any help, please let me know!

@rwinch I made a PR with my first aproach to solve this issue, I'll be happy to know your opinion about my approach and to make any adjustments in the case that may be necessary.

Fixed via gh-6110

Was this page helpful?
0 / 5 - 0 ratings