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

WebSocket matchers ignore parameters #4469

Closed
karayv opened this issue Jul 25, 2017 · 7 comments
Closed

WebSocket matchers ignore parameters #4469

karayv opened this issue Jul 25, 2017 · 7 comments
Assignees
Labels
in: messaging An issue in spring-security-messaging type: enhancement A general enhancement
Milestone

Comments

@karayv
Copy link

karayv commented Jul 25, 2017

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

@karayv
Copy link
Author

karayv commented Jul 26, 2017

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);
    }

@neuhalje
Copy link

neuhalje commented Nov 2, 2018

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);
    }
  }

}

@rwinch
Copy link
Member

rwinch commented Nov 8, 2018

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

@rwinch rwinch added in: messaging An issue in spring-security-messaging status: ideal-for-contribution An issue that we actively are looking for someone to help us with labels Nov 8, 2018
@dbuos
Copy link
Contributor

dbuos commented Nov 14, 2018

@rwinch I'd like to take this one

@rwinch
Copy link
Member

rwinch commented Nov 15, 2018

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

@rwinch rwinch removed the status: ideal-for-contribution An issue that we actively are looking for someone to help us with label Nov 15, 2018
@dbuos
Copy link
Contributor

dbuos commented Nov 18, 2018

@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.

dbuos added a commit to dbuos/spring-security that referenced this issue Nov 21, 2018
Extract path variables expressed in SimpDestinationMessageMatcher's pattern.

Issue: spring-projectsgh-4469
@rwinch rwinch self-assigned this Jun 7, 2019
@rwinch rwinch added the type: enhancement A general enhancement label Jun 7, 2019
@rwinch rwinch added this to the 5.2.0.M3 milestone Jun 7, 2019
rwinch pushed a commit that referenced this issue Jun 7, 2019
Extract path variables expressed in SimpDestinationMessageMatcher's pattern.

Issue: gh-4469
rwinch added a commit that referenced this issue Jun 7, 2019
@rwinch
Copy link
Member

rwinch commented Jun 7, 2019

Fixed via gh-6110

@rwinch rwinch closed this as completed Jun 7, 2019
rwinch added a commit that referenced this issue Jun 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: messaging An issue in spring-security-messaging type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants