brusdev commented on a change in pull request #3849:
URL: https://github.com/apache/activemq-artemis/pull/3849#discussion_r751994117
##########
File path:
artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/OpenWireConnection.java
##########
@@ -768,7 +769,8 @@ public AMQConnectionContext initContext(ConnectionInfo
info) throws Exception {
}
private void createInternalSession(ConnectionInfo info) throws Exception {
- internalSession =
server.createSession(UUIDGenerator.getInstance().generateStringUUID(),
context.getUserName(), info.getPassword(), -1, this, true, false, false, false,
null, null, true, operationContext, protocolManager.getPrefixes(),
protocolManager.getSecurityDomain());
+ final String validatedUser = server.validateUser(context.getUserName(),
info.getPassword(), this, protocolManager.getSecurityDomain());
Review comment:
if OpenWireProtocolManager.validateUser could set a new field as
OpenWireConnection.validatedUser the OpenWireConnection.createInternalSession
shouldn't execute server.validateUser again
##########
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java
##########
@@ -1674,12 +1674,8 @@ public ServerSession createSession(final String name,
final boolean autoCreateQueues,
final OperationContext context,
final Map<SimpleString, RoutingType>
prefixes,
- final String securityDomain) throws
Exception {
- String validatedUser = "";
-
- if (securityStore != null) {
- validatedUser = securityStore.authenticate(username, password,
connection, securityDomain);
- }
+ final String securityDomain,
+ String validatedUser) throws Exception {
Review comment:
cool anticipating the authentication, if securityStore != null and
validatedUser == null we could call `securityStore.authenticate` or thwroing a
security exception
##########
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/BrokerBalancerControlImpl.java
##########
@@ -59,11 +59,10 @@ public BrokerBalancerControlImpl(final BrokerBalancer
balancer, final StorageMan
@Override
public CompositeData getTarget(String key) throws Exception {
- Target target = balancer.getTarget(key);
-
- if (target != null) {
+ Target.Result result = balancer.getTarget(key);
+ if (Target.Status.OK.compareTo(result.status) == 0) {
CompositeData connectorData = null;
- TransportConfiguration connector = target.getConnector();
+ TransportConfiguration connector = result.target.getConnector();
Review comment:
It looks a bit weird to me `Target.Result result...` and
`result.target...`, why an inner class Target.Result and not another class
TragetResult that includes Target and Status ?
##########
File path:
artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/amq/AMQSession.java
##########
@@ -133,7 +133,8 @@ public void initialize() {
// now
try {
- coreSession = server.createSession(name, username, password,
minLargeMessageSize, connection, true, false, false, false, null, this, true,
connection.getOperationContext(), protocolManager.getPrefixes(),
protocolManager.getSecurityDomain());
+ final String validatedUser = server.validateUser(username, password,
connection, protocolManager.getSecurityDomain());
Review comment:
if OpenWireProtocolManager.validateUser could set a new field as
OpenWireConnection.validatedUser the AMQSession.initialize shouldn't execute
server.validateUser again
--
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]
