Password bypass

[JessyJP]

1 We don't need the boolean found variable. Directly return when we find the IP. It's more efficient and we skip the rest of the check on long IP lists. 2 Second, we can introduce an unconditional authentication case. When the case is met we add the device for the next check and return.

• When the password is empty should always mean unconditional login. Currently, we can't make the password empty within Quelea, but at the very least we can do it externally.
• If we have an additional property like a boolean for example remote.control.require.authentication, then: (QueleaProperties.get().getRemoteControlPassword().length() == 0) could be: (!QueleaProperties.get().getRemoteControlRequireAuthentication() || QueleaProperties.get().getRemoteControlPassword().length() == 0)
  3. I feel like having an extra switch is not as elegant as just having an empty password so hence the edit && false) {//The empty password check disabled by default

I feel the first commit is a must! I think in that case the second commit makes sense given the first. I mean it's a presentation software so at least we should be able to remove the password even it's via notepad at the very least.

Weird I can't open the preference pannel in debug mode and the database doesn't show!

---------- Another option

Now if it's really really important to not have an empty password we can add a checkbox next to the password edit field. The checkbox will be connected to remote.control.require.authentication Then have boolean getRemoteControlRequireAuthentication() and setRemoteControlRequireAuthentication(boolean isRequired). I can do a third commit with the checkbox but I think it's silly to add extra stuff, so as is, is fine.

---------- Also

Also '&& false) {//The empty password check disabled by default ' becomes && getRemoteControlRequireAuthentication() ) {//The empty password check is only enabled when authentication is required Could do it like that too. In that case, enable/disable actually works on the empty check rather than the authentication itself. That's still a valid way of doing it.

[JessyJP]

• Well obviously, but I don't imagine you actually want to remove this code right now. I know you like to be careful about changes so I try to be noninvasive as possible. Also while testing around you might want to switch it off & on. Ah, and if an extra property switch is added like a checkbox, that && false is where it will go. I would just put a comment to say why it's disabled ( I think i did) but for any reason, you might want to resit that later.

I mean, most people use a dedicated network and probably don't bother changing the password.

• passwordless authentication? Yes! As I said, I think currently this is the most noninvasive option. It retains everything as is while adding the option if the user wants to remove the password. The password remains there by default when the program is installed. Everything's the same. *(Mutating state in a method starting is also bad form from a style perspective.) - Hmmm, I do see your point about the is type functions, but in this case, I am not so sure it's a bad idea.
  1. isLoggedOn() - doesn't have to necessarily imply security. The IP still needs to be logged which comes back to the idea about a logger https://github.com/quelea-projection/Quelea/issues/597.
  2. It can also return unconditionally true which will work but then who will add the device?
  3. But, let's say it's not an elegant way of doing it. That means another function and another check. isLoggedOn() || checkPasswordDisabled(String ip) ... or isLoggedOn() || logRemoteActicity(String ip, String action/handlerID) ...

something like this :?

private boolean checkPasswordDisabled(){
  boolean isDisabledState = (QueleaApp.get().getPass().len() == 0) || QueleaApp.get().getSomeCheckboxForDisableedAuthenticationValue();

  if (isDisabledState) {
  addDevice(ip);
  }

  return isDisabledState;
}

or

private logRemoteActicity(String ip, String action/handlerID){
  boolean isDisabledState = (QueleaApp.get().getPass().len() == 0) || QueleaApp.get().getSomeCheckboxForDisableedAuthenticationValue();

  if (isDisabledState && (//is not in the device list)) {
  addDevice(ip);
  }

  addToRemoteHandlerLog(ip, action);// Some function to  track who did what

  return isDisabledState;
}

4. I wanted to avoid calling one more function or condition every time there was an incoming message.
5. Or we can rename the function to validateLogin() or something.

Well either way works for me, but then with option 2, all login checks would have to be replaced. Ok, "replace all" in notepadd++ would do but yeah that's the other reason. Also the adding of the device occurs only if that very specific condition is met. Either way you are the main chef and you say what goes in or out of the source soup.