cshannon commented on code in PR #2461:
URL: https://github.com/apache/activemq/pull/2461#discussion_r4249282603
##########
activemq-broker/src/main/java/org/apache/activemq/broker/TransportConnector.java:
##########
@@ -22,6 +22,7 @@
import java.util.ArrayList;
import java.util.LinkedList;
import java.util.List;
+import java.util.Locale;
Review Comment:
This import looks like a mistake?
##########
activemq-broker/src/main/java/org/apache/activemq/broker/TransportConnection.java:
##########
@@ -1480,9 +1481,26 @@ public Response processBrokerInfo(BrokerInfo info)
throws IOException {
setDuplexNetworkConnectorId(duplexNetworkConnectorId);
}
+ // A connection that authenticated as an ordinary client and only now
declares
+ // itself a network connection must have been permitted to do so.
Bridges normally
+ // send BrokerInfo first, in which case authentication already saw the
flag and
+ // there are no connection states here yet.
+ var connectionStates = listConnectionStates();
Review Comment:
In general it doesn't really seem like we should be doing any kind of
authentication checks here and I think it makes sense to move this and treat
each connection attempt independently.
The proper place to do the checking is here:
https://github.com/apache/activemq/blob/d9093d38bf4f157947d8d16012be2b6f1868f5b6/activemq-broker/src/main/java/org/apache/activemq/broker/TransportConnection.java#L866
After the addConnection() executes to handle the jaas authenticaiton logic
there is that line which process addNetworkConnection(). These checks should be
done either before the addNetworkConnection() call or just moved into that
method first so that we throw the exception and cleanup before continuing.
I don't think it makes sense to loop over all states, I think we can just
allow it as long as the current connection attempt is ok (there's usually only
1 state anyways)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
