If the auth_overrides_username setting is enabled, and a username is used by more than two users on the SSO provider site, users other than the first user to register on Discourse with the username will have the digit that’s appended to their username changed when they logout and log back into Discourse. For example, if there are three users with the username bob on the SSO provider site and the Discourse user bob1 logs into Discourse via SSO, they are likely to have their Discourse username updated to bob3.

This is an edge case, but we were handling it correctly in the past. A part of my changes in #14531 broke that handling. Turned out, this edge case was the reason why we needed the allowed_username = nil parameter in UserNameSuggester.suggest. I removed this parameter in that PR and introduced the regression.

In this PR, I've returned the second parameter to UserNameSuggester.suggest. I've named it current_username this time, in my opinion the previous name allowed_username was a bit confusing:

# the second parameter is confusing, 
# why call username suggester at all if we already know an allowed username?
# we can just use the allowed username
def suggest(name_or_email, allowed_username = nil)
...
end

Also, I've added test cases that explicitly document this edge case.

I was thinking of going even further, but I'm on the fence here. We can remove the suggest method and create two new methods:

module UserNameSuggester
  def self.suggest_for_new_user(name_or_email)
  end

  def self.suggest_for_existent_user(name_or_email,  current_username)
  end
end

This way, clients will always be enforced to

1. Decide consciously if they need a suggestion for a new user or for an old one
2. Always pass the current username for existent users

Without passing the current username for existent users, we'll always be seeing the same bug. It would be fine to enforce it.