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.

Sign up for GitHub

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

Add DestinationPathPatternMessageMatcher #16635

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Add DestinationPathPatternMessageMatcher #16635

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...ity/config/annotation/web/socket/WebSocketMessageBrokerSecurityConfigurationDocTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 the original author or authors.
* Copyright 2002-2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down
2 changes: 1 addition & 1 deletion ...curity/config/annotation/web/socket/WebSocketMessageBrokerSecurityConfigurationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2025 the original author or authors.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there are no other changes here, please go ahead and revert the copyright year so that this file is left out of the PR. Please do this for any other files like this one.

*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down
156 changes: 144 additions & 12 deletions ...ork/security/messaging/access/intercept/MessageMatcherDelegatingAuthorizationManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 the original author or authors.
* Copyright 2002-2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -17,6 +17,7 @@
package org.springframework.security.messaging.access.intercept;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.function.Supplier;
Expand All @@ -32,6 +33,7 @@
import org.springframework.security.authorization.AuthorizationDecision;
import org.springframework.security.authorization.AuthorizationManager;
import org.springframework.security.core.Authentication;
import org.springframework.security.messaging.util.matcher.DestinationPathPatternMessageMatcher;
import org.springframework.security.messaging.util.matcher.MessageMatcher;
import org.springframework.security.messaging.util.matcher.SimpDestinationMessageMatcher;
import org.springframework.security.messaging.util.matcher.SimpMessageTypeMatcher;
Expand Down Expand Up @@ -87,12 +89,11 @@ private MessageAuthorizationContext<?> authorizationContext(MessageMatcher<?> ma
if (!matcher.matches((Message) message)) {
return null;
}
if (matcher instanceof SimpDestinationMessageMatcher simp) {
return new MessageAuthorizationContext<>(message, simp.extractPathVariables(message));
if (matcher instanceof Builder.LazySimpDestinationMessageMatcher pathMatcher) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we make the change to MessageMatcher suggested in this PR, then this casting can all be removed.

return new MessageAuthorizationContext<>(message, pathMatcher.extractPathVariables(message));
}
if (matcher instanceof Builder.LazySimpDestinationMessageMatcher) {
Builder.LazySimpDestinationMessageMatcher path = (Builder.LazySimpDestinationMessageMatcher) matcher;
return new MessageAuthorizationContext<>(message, path.extractPathVariables(message));
if (matcher instanceof Builder.LazySimpDestinationPatternMessageMatcher pathMatcher) {
return new MessageAuthorizationContext<>(message, pathMatcher.extractPathVariables(message));
}
return new MessageAuthorizationContext<>(message);
}
Expand All @@ -112,8 +113,11 @@ public static final class Builder {

private final List<Entry<AuthorizationManager<MessageAuthorizationContext<?>>>> mappings = new ArrayList<>();

@Deprecated
private Supplier<PathMatcher> pathMatcher = AntPathMatcher::new;

private boolean useHttpPathSeparator = true;

public Builder() {
}

Expand All @@ -132,11 +136,11 @@ public Builder.Constraint anyMessage() {
* @return the Expression to associate
*/
public Builder.Constraint nullDestMatcher() {
return matchers(SimpDestinationMessageMatcher.NULL_DESTINATION_MATCHER);
return matchers(DestinationPathPatternMessageMatcher.NULL_DESTINATION_MATCHER);
}

/**
* Maps a {@link List} of {@link SimpDestinationMessageMatcher} instances.
* Maps a {@link List} of {@link SimpMessageTypeMatcher} instances.
* @param typesToMatch the {@link SimpMessageType} instance to match on
* @return the {@link Builder.Constraint} associated to the matchers.
*/
Expand All @@ -156,35 +160,90 @@ public Builder.Constraint simpTypeMatchers(SimpMessageType... typesToMatch) {
* @param patterns the patterns to create
* {@link org.springframework.security.messaging.util.matcher.SimpDestinationMessageMatcher}
* from.
* @deprecated use {@link #destinationPathPatterns(String...)}
*/
@Deprecated
public Builder.Constraint simpDestMatchers(String... patterns) {
return simpDestMatchers(null, patterns);
}

/**
* Allows the creation of a security {@link Constraint} applying to messages whose
* destinations match the provided {@code patterns}.
* <p>
* The matching of each pattern is performed by a
* {@link DestinationPathPatternMessageMatcher} instance that matches
* irrespectively of {@link SimpMessageType}. If no destination is found on the
* {@code Message}, then each {@code Matcher} returns false.
* </p>
* @param patterns the destination path patterns to which the security
* {@code Constraint} will be applicable
* @since 6.5
*/
public Builder.Constraint destinationPathPatterns(String... patterns) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is another way to opt-in, I'd prefer to leave the methods that take a String the same. The reason is that it still describes what the application is doing: matching a simp destination.

return destinationPathPatterns(null, patterns);
}

/**
* Maps a {@link List} of {@link SimpDestinationMessageMatcher} instances that
* match on {@code SimpMessageType.MESSAGE}. If no destination is found on the
* Message, then the Matcher returns false.
* @param patterns the patterns to create
* {@link org.springframework.security.messaging.util.matcher.SimpDestinationMessageMatcher}
* from.
* @deprecated use {@link #destinationPathPatterns(String...)}
*/
@Deprecated
public Builder.Constraint simpMessageDestMatchers(String... patterns) {
return simpDestMatchers(SimpMessageType.MESSAGE, patterns);
}

/**
* Allows the creation of a security {@link Constraint} applying to messages of
* the type {@code SimpMessageType.MESSAGE} whose destinations match the provided
* {@code patterns}.
* <p>
* The matching of each pattern is performed by a
* {@link DestinationPathPatternMessageMatcher}. If no destination is found on the
* {@code Message}, then each {@code Matcher} returns false.
* @param patterns the patterns to create
* {@link DestinationPathPatternMessageMatcher} from.
* @since 6.5
*/
public Builder.Constraint simpTypeMessageDestinationPatterns(String... patterns) {
return destinationPathPatterns(SimpMessageType.MESSAGE, patterns);
}

/**
* Maps a {@link List} of {@link SimpDestinationMessageMatcher} instances that
* match on {@code SimpMessageType.SUBSCRIBE}. If no destination is found on the
* Message, then the Matcher returns false.
* @param patterns the patterns to create
* {@link org.springframework.security.messaging.util.matcher.SimpDestinationMessageMatcher}
* from.
* @deprecated use {@link #simpTypeSubscribeDestinationPatterns(String...)}
*/
@Deprecated
public Builder.Constraint simpSubscribeDestMatchers(String... patterns) {
return simpDestMatchers(SimpMessageType.SUBSCRIBE, patterns);
}

/**
* Allows the creation of a security {@link Constraint} applying to messages of
* the type {@code SimpMessageType.SUBSCRIBE} whose destinations match the
* provided {@code patterns}.
* <p>
* The matching of each pattern is performed by a
* {@link DestinationPathPatternMessageMatcher}. If no destination is found on the
* {@code Message}, then each {@code Matcher} returns false.
* @param patterns the patterns to create
* {@link DestinationPathPatternMessageMatcher} from.
* @since 6.5
*/
public Builder.Constraint simpTypeSubscribeDestinationPatterns(String... patterns) {
return destinationPathPatterns(SimpMessageType.SUBSCRIBE, patterns);
}

/**
* Maps a {@link List} of {@link SimpDestinationMessageMatcher} instances. If no
* destination is found on the Message, then the Matcher returns false.
Expand All @@ -195,7 +254,9 @@ public Builder.Constraint simpSubscribeDestMatchers(String... patterns) {
* from.
* @return the {@link Builder.Constraint} that is associated to the
* {@link MessageMatcher}
* @deprecated use {@link #destinationPathPatterns(String...)}
*/
@Deprecated
private Builder.Constraint simpDestMatchers(SimpMessageType type, String... patterns) {
List<MessageMatcher<?>> matchers = new ArrayList<>(patterns.length);
for (String pattern : patterns) {
Expand All @@ -205,13 +266,52 @@ private Builder.Constraint simpDestMatchers(SimpMessageType type, String... patt
return new Builder.Constraint(matchers);
}

/**
* Allows the creation of a security {@link Constraint} applying to messages of
* the provided {@code type} whose destinations match the provided
* {@code patterns}.
* <p>
* The matching of each pattern is performed by a
* {@link DestinationPathPatternMessageMatcher}. If no destination is found on the
* {@code Message}, then each {@code Matcher} returns false.
* </p>
* @param type the {@link SimpMessageType} to match on. If null, the
* {@link SimpMessageType} is not considered for matching.
* @param patterns the patterns to create
* {@link DestinationPathPatternMessageMatcher} from.
* @return the {@link Builder.Constraint} that is associated to the
* {@link MessageMatcher}s
* @since 6.5
*/
private Builder.Constraint destinationPathPatterns(SimpMessageType type, String... patterns) {
List<MessageMatcher<?>> matchers = new ArrayList<>(patterns.length);
for (String pattern : patterns) {
MessageMatcher<Object> matcher = new LazySimpDestinationPatternMessageMatcher(pattern, type,
this.useHttpPathSeparator);
matchers.add(matcher);
}
return new Builder.Constraint(matchers);
}

/**
* Instruct this builder to match message destinations using the separator
* configured in
* {@link org.springframework.http.server.PathContainer.Options#MESSAGE_ROUTE}
*/
public Builder messageRouteSeparator() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably too specific for the DSL. The application in this case should constructor their own message matcher instance.

this.useHttpPathSeparator = false;
return this;
}

/**
* The {@link PathMatcher} to be used with the
* {@link Builder#simpDestMatchers(String...)}. The default is to use the default
* constructor of {@link AntPathMatcher}.
* @param pathMatcher the {@link PathMatcher} to use. Cannot be null.
* @return the {@link Builder} for further customization.
* @deprecated use {@link #messageRouteSeparator()} to alter the path separator
*/
@Deprecated
public Builder simpDestPathMatcher(PathMatcher pathMatcher) {
Assert.notNull(pathMatcher, "pathMatcher cannot be null");
this.pathMatcher = () -> pathMatcher;
Expand All @@ -224,7 +324,9 @@ public Builder simpDestPathMatcher(PathMatcher pathMatcher) {
* computation or lookup of the {@link PathMatcher}.
* @param pathMatcher the {@link PathMatcher} to use. Cannot be null.
* @return the {@link Builder} for further customization.
* @deprecated use {@link #messageRouteSeparator()} to alter the path separator
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead, let's begin with directing them to use #matchers to construct their own matcher.

*/
@Deprecated
public Builder simpDestPathMatcher(Supplier<PathMatcher> pathMatcher) {
Assert.notNull(pathMatcher, "pathMatcher cannot be null");
this.pathMatcher = pathMatcher;
Expand All @@ -240,9 +342,7 @@ public Builder simpDestPathMatcher(Supplier<PathMatcher> pathMatcher) {
*/
public Builder.Constraint matchers(MessageMatcher<?>... matchers) {
List<MessageMatcher<?>> builders = new ArrayList<>(matchers.length);
for (MessageMatcher<?> matcher : matchers) {
builders.add(matcher);
}
builders.addAll(Arrays.asList(matchers));
return new Builder.Constraint(builders);
}

Expand Down Expand Up @@ -381,6 +481,7 @@ public Builder access(AuthorizationManager<MessageAuthorizationContext<?>> autho

}

@Deprecated
private final class LazySimpDestinationMessageMatcher implements MessageMatcher<Object> {

private final Supplier<SimpDestinationMessageMatcher> delegate;
Expand Down Expand Up @@ -412,6 +513,37 @@ Map<String, String> extractPathVariables(Message<?> message) {

}

private static final class LazySimpDestinationPatternMessageMatcher implements MessageMatcher<Object> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing deferral work is there to address an eager instantiation issue caused by trying to resolve the Spring Websocket PathMatcher instance too early. When we aren't using PathMatcher, then this lazy-loading is likely unnecessary.


private final Supplier<DestinationPathPatternMessageMatcher> delegate;

private LazySimpDestinationPatternMessageMatcher(String pattern, SimpMessageType type,
boolean useHttpPathSeparator) {
this.delegate = SingletonSupplier.of(() -> {
DestinationPathPatternMessageMatcher.Builder builder = (useHttpPathSeparator)
? DestinationPathPatternMessageMatcher.withDefaults()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd recommend starting with withDefaults for now. I'm not certain that we want to support PathOptions.MESSAGE_ROUTE yet.

: DestinationPathPatternMessageMatcher.messageRoute();
if (type == null) {
return builder.matcher(pattern);
}
if (SimpMessageType.MESSAGE == type || SimpMessageType.SUBSCRIBE == type) {
return builder.messageType(type).matcher(pattern);
}
throw new IllegalStateException(type + " is not supported since it does not have a destination");
});
}

@Override
public boolean matches(Message<?> message) {
return this.delegate.get().matches(message);
}

Map<String, String> extractPathVariables(Message<?> message) {
return this.delegate.get().extractPathVariables(message);
}

}

}

private static final class Entry<T> {
Expand All @@ -420,7 +552,7 @@ private static final class Entry<T> {

private final T entry;

Entry(MessageMatcher requestMatcher, T entry) {
Entry(MessageMatcher<?> requestMatcher, T entry) {
this.messageMatcher = requestMatcher;
this.entry = entry;
}
Expand Down
Loading