Case insensitive logins plugin (code review)

[thedumbtechguy]

Hello again @jeremyevans. Pray all is well with you and your family.

I recently needed to implement case insensitive logins (sqlite) and I came up with a quick and dirty solution which works.
I just wanted to know if this approach is correct so I can solidify the implementation or improve.
I would appreciate your thoughts.

require "rodauth"

module Rodauth
  Feature.define(:case_insensitive_login, :CaseInsensitiveLogin) do
    def param(key)
      if [login_param, login_confirm_param].include?(key)
        super&.downcase
      else
        super
      end
    end

    def account_from_login(login)
      super(login&.downcase)
    end
  end
end

[jeremyevans]

param should always return a string (it calls param_or_nil and uses to_s on the result), so super&.downcase could just be super.downcase. I checked and account_from_login is only ever called with something that isn't param(login_param) if you are using the http_basic_auth feature.

I wonder if we should do this by default, or work on a different approach. PostgreSQL uses citext for case insensitive logins, and MySQL and MSSQL are case insensitive by default, so SQLite is the only supported database where you need to do something special for case insensitive logins. Apparently you can get case insensitive behavior in SQLite using COLLATE NOCASE, but it has drawbacks.

One issue with doing this by default is that downcasing automatically could break existing installations on SQLite. So that could only be changed in a major release. Until then, we would need to make it optional.

[jeremyevans]

I see that as a separate issue, because it doesn't involve the database. However, I agree with you that we should fix it. We should use casecmp for login vs login_confirm comparison.

I'm not in favor of forcing downcasing of login params by default. That should only be done as a last resort if the database will use a case-sensitive comparison. It's better to use a case-insensitive column, because it is case-preserving.

I'll try to work on this before the next release.

[jeremyevans]

OK, I think all issues have been addressed as of eabfa69 . If you have time, please review and let me know if anything was missed.

[thedumbtechguy]

@jeremyevans it might be my lack of understanding on how the your plugin system works, but I don't see where it is invoked. Other than that, it looks complete.

This is an aside to understand your thinking wrt to preserving case.
Does it matter if you want case insensitive behavior esp. since we are dealing with emails?
Is there any value to preserving the case it was entered in?

[jeremyevans]

The login vs login_confirm comparison is now case insensitive by default. Automatic downcasing of logins is done by explicitly loading the downcased_logins plugin.

Email addresses are technically case-sensitive, see RFC 5321 section 2.4: https://datatracker.ietf.org/doc/html/rfc5321#section-2.4 . Most servers will treat them in a case-insensitive matter, but it is valid for a server to treat them in a case-sensitive manner.

[thedumbtechguy]

TIL

Thank you