Skip to content

Add DestinationPathPatternMessageMatcher human reviewer #21

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

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

Add DestinationPathPatternMessageMatcher human reviewer #21

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.
*
* 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) {
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) {
return destinationPathPatterns(null, patterns);
Copy link

Choose a reason for hiding this comment

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

issue (complexity): Consider creating a helper method to encapsulate the common matcher-creation logic, reducing code duplication and improving maintainability by avoiding nearly identical code paths in different methods, such as in destinationPathPatterns.

Consider extracting the common matcher‐creation logic into a helper method rather than repeating nearly identical code paths in different methods. For example, you could create a private method that builds a matcher based on the provided pattern and type, and then have your builder methods call that helper. This would reduce duplication and ease maintenance.

Example Suggestion:

private MessageMatcher<Object> createDestinationMatcher(String pattern, SimpMessageType type, boolean useHttpPathSeparator) {
    if (MessageMatcherFactory.usesPathPatterns()) {
        // For new behavior using path patterns
        return new LazySimpDestinationPatternMessageMatcher(pattern, type, useHttpPathSeparator);
    }
    // Fallback to legacy matcher
    return new LazySimpDestinationMessageMatcher(pattern, type);
}

Then, in your builder methods (like destinationPathPatterns), replace the loop with:

private Builder.Constraint destinationPathPatterns(SimpMessageType type, String... patterns) {
    List<MessageMatcher<?>> matchers = new ArrayList<>(patterns.length);
    for (String pattern : patterns) {
        matchers.add(createDestinationMatcher(pattern, type, this.useHttpPathSeparator));
    }
    return new Builder.Constraint(matchers);
}

This consolidation preserves existing behavior while reducing conditional branching and duplicated code paths.

}

/**
* 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() {
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
*/
@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> {

private final Supplier<DestinationPathPatternMessageMatcher> delegate;

private LazySimpDestinationPatternMessageMatcher(String pattern, SimpMessageType type,
boolean useHttpPathSeparator) {
this.delegate = SingletonSupplier.of(() -> {
DestinationPathPatternMessageMatcher.Builder builder = (useHttpPathSeparator)
? DestinationPathPatternMessageMatcher.withDefaults()
: 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