TIdSocksServer and TIdEventSocksServer do not honor NeedsAuthentication property

[VA1DER]

Neither the TIdSocksServer nor the TIdEventSocksServer components honor the "NeedsAuthentication" property. If OnAuthenticate is not set, both components will refuse to allow connections.

Only setting OnAuthenticate will allow connections:

function TIdCustomSocksServer.DoAuthenticate(AContext: TIdSocksServerContext): Boolean;
begin
  Result := Assigned(OnAuthenticate);
  if Result then begin
    OnAuthenticate(AContext, Result);
  end;
end;

[rlebeau]

NeedsAuthentication is currently handled only for SOCKS v5, in TIdCustomSocksServer.HandleConnectV5():

• if NeedsAuthentication=False, the server responds to the client's initial handshake telling it that authentication is not required, and then the server reads the client's next request. DoAuthentication() is not called.
• if NeedsAuthentication=True, the server responds to the client's initial handshake asking for authentication, then reads in the client's credentials, and then calls DoAuthenticate() to validate the credentials. If OnAuthenticate is not assigned then authentication fails and the connection is closed, otherwise the server responds that authentication succeeded and reads the client's next request.

So, if you enable SOCKS v5, setting NeedsAuthentication=True requires you to also assign an OnAuthentication handler, whereas setting NeedsAuthentication=False makes OnAuthentication optional. This seems like expected behavior as designed.

In the case of SOCKS v4/a, on the other hand, a userid is part of the client's initial handshake, so OnAuthenticate is currently required to validate it. NeedsAuthentication is not looked at for SOCKS v4/a at this time. Is this what you are asking about? If so, then a simple fix would be to update TIdCustomSocksServer.HandleConnectV4() to look at NeedsAuthentication before calling DoAuthenticate().

[VA1DER]

Yes, it was for Socks 4/a. My project binds it to 127.0.0.1 since it's a local stream-over-http client. I don't need any authentication, and just assumed that NeedsAuthentication=False would handle that. It's a small thing to have an empty OnAuthenticate, but yes I would suggest implementing the change you propose.