timoninmaxim commented on a change in pull request #8892:
URL: https://github.com/apache/ignite/pull/8892#discussion_r615554729
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/authentication/IgniteAuthenticationProcessor.java
##########
@@ -836,14 +792,11 @@ private void
onAuthenticateResponseMessage(UserAuthenticateResponseMessage msg)
* Local node joined to topology. Discovery cache is available but no
discovery custom message are received.
* Initial user set and initial user operation (received on join) are
processed here.
*/
- public void onLocalJoin() {
- if (coordinator() == null)
+ private void onLocalJoin() {
+ if (ctx.isDaemon() || ctx.clientDisconnected() || coordinator() ==
null)
Review comment:
I see, thanks.
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/authentication/IgniteAuthenticationProcessor.java
##########
@@ -677,8 +635,6 @@ private ClusterNode coordinator() {
U.warn(log, "Cannot find the server coordinator node. "
+ "Possible a client is started with
forceServerMode=true. " +
"Security warning: user authentication will be
disabled on the client.");
-
- isEnabled = false;
Review comment:
I see there 2 possible issues:
1. a misleading comment on the previous line, as user authentication isn't
disabled now explicitly;
2. as there no check for `isEnabled` in ioLsnr then for the case described
in the warning below (client is in forceServerMode=true,
cfg.isAuthenticationEnabled=true and there no other nodes) we can hang on this
chain:
`ioLsnr -> onAuthenticateRequestMessage -> authenticateOnServer ->
readyForAuthFut.get();`
Also for `discoLsnr` there can be NPE for `onNodeLeft -> coordinator().id()`.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
