Fix compatibility problem with aliyun and netease mail as authentication source

[darren]

Close #27043

[darren]

@lunny @puni9869

I have modified the original issue title and make changes to this pull request, can you re-review this?
this is the original code that handles smtp auth error

gitea/services/auth/source/smtp/source_authenticate.go

Lines 47 to 55 in 684ab40

	if (ok && tperr.Code == 535) ||
	strings.Contains(err.Error(), "Username and Password not accepted") {
	return nil, user_model.ErrUserNotExist{Name: userName}
	}
	if (ok && tperr.Code == 534) ||
	strings.Contains(err.Error(), "Application-specific password required") {
	return nil, user_model.ErrUserNotExist{Name: userName}
	}
	return nil, err

it only handles specific textproto error, so it may have compatibility with different email providers. and the actual textproto error is hidden from the log.

with this pr, when auth fails it shows error in log like this(aliyun mail)

2023/09/16 10:13:01 ...ers/web/auth/auth.go:206:SignInPost() [I] Failed authentication attempt for xxx@xxx.com from 127.0.0.1:11919: invalid argument
526 Authentication failure[0]

[puni9869]

Will take a look tomorrow.

[lunny]

It's not easy if we have no accounts of that two platforms.

[darren]

It's not easy if we have no accounts of that two platforms.

Actually you don't have to have accounts of these email providers, Just setup an SMTP authentication source with smtp.qiye.aliyun.com as host. And do login with any email account, the 500 error will show up.

[yp05327]

If I understand correctly, 526 error is caused by DNS error?
https://www.alibabacloud.com/help/en/direct-mail/user-guide/summary-of-common-email-return-codes-and-solutions

Maybe this one:
https://www.alibabacloud.com/help/en/alibaba-mail/latest/526-authentication-failure
From this doc, 526 error is not only caused by incorrect username/password error, but also incorrect server address.

ps: What I want to say is that, this is not a normal error code for smtp authentication error, so we need docs to explain why we need to do this.

[darren]

If I understand correctly, 526 error is caused by DNS error? https://www.alibabacloud.com/help/en/direct-mail/user-guide/summary-of-common-email-return-codes-and-solutions

Maybe this one: https://www.alibabacloud.com/help/en/alibaba-mail/latest/526-authentication-failure

What I want to say is that, this is not a normal error code for smtp authentication error, so we need docs to explain why we need to do this.

As I have commented in the PR:

	// when authentication via smtp fails, wraps ErrInvalidArgument
	// with the original textproto.Error as the cause,
	// so it will show username_password_incorrect to the user
	// while log the original error so that admin can check.
	// see: routers/web/auth/auth.go SiginPost

Is that clear enough to you ?

What this PR does is that instead of showing 500 to end user when such errors occurs, just show login failure to user and let admin to check the log to pinpoint the actual reason while not hiding it.

It's not perfect though, but still be better.

[yp05327]

I prefer adjusting the logic in UserSignIn and the error handling logic in SignInPost, as not all errors mean incorrect username/password.
For example, do not always show 500 error page, just show error info to users, like there's an unexpected error occurred during the login process, please contact the admin.

And maybe not related, the error handling logic for UserSignIn is checking ErrNotExist to detect whether user is existed, but it not only contains user not found error, but also some other errors, e.g. host not found.
Then you can see this, a white screen with nothing.

[yp05327]

@lunny
maybe backport label should be changed? This PR has been moved to 1.22 milestone.