Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve end-user message when a technical error occurs in a external auth hook. #1749

Open
Mathieu-COSYNS opened this issue Sep 6, 2024 · 0 comments
Labels
suggestion Feature suggestion

Comments

@Mathieu-COSYNS
Copy link

Is your feature request related to a problem? Please describe.

When the external auth hook doesn't work properly (eg. hook is an http link to a server not working, hook return an invalid json user configuration, ...) the error message in the ui say "Invalid credentials, please retry" which is misleading for the users. They credentials may be valid but for some reason the hook fail. Thinking that their credentials are invalid they will retry, retry and retry again until they gave up or go insane.

image

Describe the solution you'd like

Show a message like a 500 error message instead of invalid credentials error message.

For example in this piece of web client code :

user, err := dataprovider.CheckUserAndPass(username, password, ipAddr, protocol)
if err != nil {
updateLoginMetrics(&user, dataprovider.LoginMethodPassword, ipAddr, err)
s.renderClientLoginPage(w, r,
util.NewI18nError(dataprovider.ErrInvalidCredentials, util.I18nErrorInvalidCredentials))
return
}
connectionID := fmt.Sprintf("%v_%v", protocol, xid.New().String())

When dataprovider.CheckUserAndPass(...) return an error different than dataprovider.ErrInvalidCredentials instead of creating a new error with util.I18nErrorInvalidCredentials create a new error with util.I18nError500Message.

The example above only showcase a solution for the web client but this should also be considered for the web admin, and other protocols.

Describe alternatives you've considered

I have submitted the issue as a feature request because it doesn't break anything but it might as well be a bug.

What are you using SFTPGo for?

Private user, home usecase (home backup/VPS)

Additional context

Step to reproduce:

  1. Run docker run --rm -p 8080:8080 -e SFTPGO_DATA_PROVIDER__EXTERNAL_AUTH_HOOK=http://localhost -d "drakkan/sftpgo" without having anything running on http://localhost.
  2. After setting up the first admin account go directly to the webclient and try to login with any credentials. You will receive the message "Invalid credentials, please retry" but you also will see in the logs an entry with the message: error getting external auth hook HTTP response: Post \"http://localhost\": dial tcp [::1]:80: connect: connection refused witch is the real reason why you couldn't login and not the fact that you credentials where invalid.
@Mathieu-COSYNS Mathieu-COSYNS added the suggestion Feature suggestion label Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
suggestion Feature suggestion
Projects
None yet
Development

No branches or pull requests

1 participant