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

SEC-2713: Support authorization rules by SimpMessageType #2936

Closed
spring-issuemaster opened this issue Sep 5, 2014 · 8 comments
Closed

SEC-2713: Support authorization rules by SimpMessageType #2936

spring-issuemaster opened this issue Sep 5, 2014 · 8 comments

Comments

@spring-issuemaster
Copy link

@spring-issuemaster spring-issuemaster commented Sep 5, 2014

Sergi Almar (Migrated from SEC-2713) said:

The initial WebSocket security only allows protecting a destination without taking into account the message type. When using a subprotocol like STOMP subscriptions and messages should be protected independently: a user may be allowed to subscribe to a destination in order to receive messages, but not allowed to send messages to that destination.

@spring-issuemaster

This comment has been minimized.

Copy link
Author

@spring-issuemaster spring-issuemaster commented Sep 5, 2014

Rob Winch said:

salmar Thanks for the feedback! I think providing some concrete use cases of what you are looking for would be helpful to ensure we resolve this issue appropriately. From what you are describing it sounds as though you are looking for something like this:


@Configuration
public class WebSocketSecurityConfig extends AbstractSecurityWebSocketMessageBrokerConfigurer {

    @Override
    protected void configure(MessageSecurityMetadataSourceRegistry messages) {
        messages
                .destinationMatchers(SimpMessageType.SUBSCRIBE, "/messages").permitAll()
                .destinationMatchers(SimpMessageType.MESSAGE, "/messages").authenticated()
                .anyMessage().hasRole("USER");

    }

    // avoid processing outbound channel
    public void configureClientOutboundChannel(ChannelRegistration registration) {}
}

I'm wondering what other attributes you feel would be useful matching on out of the box. For example, STOMP has the command header too.

For now you can work around this pretty easily using something like this:


@Configuration
public class WebSocketSecurityConfig extends AbstractSecurityWebSocketMessageBrokerConfigurer {

    @Override
    protected void configure(MessageSecurityMetadataSourceRegistry messages) {
        messages
            .matchers(new TypedDestinationMatcher(SimpMessageType.CONNECT,"/topic/messages")).permitAll()
            .matchers(new TypedDestinationMatcher(SimpMessageType.MESSAGE,"/topic/messages")).authenticated()
                .anyMessage().hasRole("USER");

    }

    // avoid processing outbound channel
    public void configureClientOutboundChannel(ChannelRegistration registration) {}
}

public class TypedDestinationMatcher implements MessageMatcher<Object> {
    private SimpMessageType type;
    private MessageMatcher<Object> delegate;

    public TypedDestinationMatcher(SimpMessageType type, String pattern) {
        this.type = type;
        this.delegate = new SimpDestinationMessageMatcher(pattern);
    }

    @Override
    public boolean matches(Message<? extends Object> message) {
        SimpMessageType actualType = SimpMessageHeaderAccessor.getMessageType(message.getHeaders());
        return actualType == type && this.delegate.matches(message);
    }

}

cc [~rstoya05-aop]

@spring-issuemaster

This comment has been minimized.

Copy link
Author

@spring-issuemaster spring-issuemaster commented Sep 5, 2014

zyro said:

that no other matcher is shipped ootb else than the SimpDestinationMessageMatcher does not mean it does not work i would say.

maybe sth. along the following lines can do the job as well?

 @Override
protected void configure(MessageSecurityMetadataSourceRegistry messages) {
    messages
        .destinationMatchers("/subscriptions/**")
        .matchers(m -> SimpMessageHeaderAccessor.wrap(m).getMessageType().equals(SimpMessageType.SUBSCRIBE))
        .hasRole("SUBSCRIBER")
        .destinationMatchers("/messages/**")
        .matchers(m -> SimpMessageHeaderAccessor.wrap(m).getMessageType().equals(SimpMessageType.MESSAGE))
        .hasRole("MESSENGER");
}

@rwinch: this was no comment in reply to yours. just had kinda the same timing ;)

@spring-issuemaster

This comment has been minimized.

Copy link
Author

@spring-issuemaster spring-issuemaster commented Sep 5, 2014

Rob Winch said:

zyro No problem. I have real life "race conditions" all the time :)

I do think that salmar has a point that it should be easier. In fact, I had debated doing this in the initial iteration but due to time constraints left it out.

@spring-issuemaster

This comment has been minimized.

Copy link
Author

@spring-issuemaster spring-issuemaster commented Sep 5, 2014

zyro said:

absolutely no objections against .typeMatchers(SimpMessageType...) :)

@spring-issuemaster

This comment has been minimized.

Copy link
Author

@spring-issuemaster spring-issuemaster commented Sep 5, 2014

Sergi Almar said:

Awesome Rob, I think something in this direction will make things easier. The use case I'm thinking of is a destination where messages are not user generated, users just subscribe to the destination to receive notifications but they shouldn't be able to send fake messages to the channel. Imagine a simple chat where the system generates notifications when a user joins / leaves the room, if anyone could send messages to the channel, fake information could be introduced and sent to all other users (this is just a really simple example but could be a security hole if user can generate random fake messages in these situations).

Rob, do you think we can have it before SpringOne?

@spring-issuemaster

This comment has been minimized.

Copy link
Author

@spring-issuemaster spring-issuemaster commented Sep 5, 2014

Rob Winch said:

Sergi,

What are your thoughts on other types of attributes that should be able to be matched?

Rob, do you think we can have it before SpringOne?

Such an enhancement will also clean up my demo some too, but.... I don't have time to do an actual release. I don't have time to think through the syntax that I would like for the final release at the moment. Can the workaround be sufficient for now and you can mention that improved matching support will be provided in the next milestone?

@spring-issuemaster

This comment has been minimized.

Copy link
Author

@spring-issuemaster spring-issuemaster commented Sep 19, 2014

Rob Winch said:

I have changed the matching to support the type. It is also better aligned with the HTTP matching by changing the name from destinationMatchers to antMatchers. For example:

@Override
protected void configureInbound(MessageSecurityMetadataSourceRegistry messages) {
    messages
        .typeMatchers(SimpMessageType.CONNECT).permitAll()
        .antMatchers(SimpMessageType.MESSAGE, "/topic/**","/queue/**").denyAll()
        .anyMessage().hasRole("USER");
}

NOTE the method name has changed to configureInbound and configureOutbound as part of SEC-2704

@spring-issuemaster

This comment has been minimized.

Copy link
Author

@spring-issuemaster spring-issuemaster commented Sep 19, 2014

zyro said:

plain and simply awesome. thx a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.