From 01b666a78fb3c1cb449ba267561ff172a43a2624 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Fri, 14 Jun 2024 19:34:42 +0200 Subject: [PATCH] WebUIs: check login conditions before allowing password reset Signed-off-by: Nicola Murino --- internal/httpd/api_utils.go | 39 +++++++++++++++++---- internal/httpd/httpd_test.go | 68 ++++++++++++++++++++++++++++++++++-- internal/httpd/server.go | 2 +- 3 files changed, 99 insertions(+), 10 deletions(-) diff --git a/internal/httpd/api_utils.go b/internal/httpd/api_utils.go index 09c0f258..e25409e8 100644 --- a/internal/httpd/api_utils.go +++ b/internal/httpd/api_utils.go @@ -36,6 +36,7 @@ import ( "github.com/go-chi/chi/v5/middleware" "github.com/go-chi/render" "github.com/klauspost/compress/zip" + "github.com/rs/xid" "github.com/sftpgo/sdk/plugin/notifier" "github.com/drakkan/sftpgo/v2/internal/common" @@ -748,6 +749,31 @@ func checkHTTPClientUser(user *dataprovider.User, r *http.Request, connectionID return nil } +func getActiveAdmin(username, ipAddr string) (dataprovider.Admin, error) { + admin, err := dataprovider.AdminExists(username) + if err != nil { + return admin, err + } + if err := admin.CanLogin(ipAddr); err != nil { + return admin, util.NewRecordNotFoundError(fmt.Sprintf("admin %q cannot login: %v", username, err)) + } + return admin, nil +} + +func getActiveUser(username string, r *http.Request) (dataprovider.User, error) { + user, err := dataprovider.GetUserWithGroupSettings(username, "") + if err != nil { + return user, err + } + if err := user.CheckLoginConditions(); err != nil { + return user, util.NewRecordNotFoundError(fmt.Sprintf("user %q cannot login: %v", username, err)) + } + if err := checkHTTPClientUser(&user, r, xid.New().String(), false); err != nil { + return user, util.NewRecordNotFoundError(fmt.Sprintf("user %q cannot login: %v", username, err)) + } + return user, nil +} + func handleForgotPassword(r *http.Request, username string, isAdmin bool) error { var email, subject string var err error @@ -758,11 +784,11 @@ func handleForgotPassword(r *http.Request, username string, isAdmin bool) error return util.NewI18nError(util.NewValidationError("username is mandatory"), util.I18nErrorUsernameRequired) } if isAdmin { - admin, err = dataprovider.AdminExists(username) + admin, err = getActiveAdmin(username, util.GetIPFromRemoteAddress(r.RemoteAddr)) email = admin.Email subject = fmt.Sprintf("Email Verification Code for admin %q", username) } else { - user, err = dataprovider.GetUserWithGroupSettings(username, "") + user, err = getActiveUser(username, r) email = user.Email subject = fmt.Sprintf("Email Verification Code for user %q", username) if err == nil { @@ -777,8 +803,9 @@ func handleForgotPassword(r *http.Request, username string, isAdmin bool) error if err != nil { if errors.Is(err, util.ErrNotFound) { handleDefenderEventLoginFailed(util.GetIPFromRemoteAddress(r.RemoteAddr), err) //nolint:errcheck - logger.Debug(logSender, middleware.GetReqID(r.Context()), "username %q does not exists, reset password request silently ignored, is admin? %v", - username, isAdmin) + logger.Debug(logSender, middleware.GetReqID(r.Context()), + "username %q does not exists or cannot login, reset password request silently ignored, is admin? %t, err: %v", + username, isAdmin, err) return nil } return util.NewI18nError(util.NewGenericError("Error retrieving your account, please try again later"), util.I18nErrorGetUser) @@ -838,7 +865,7 @@ func handleResetPassword(r *http.Request, code, newPassword, confirmPassword str return &admin, &user, util.NewValidationError("invalid confirmation code") } if isAdmin { - admin, err = dataprovider.AdminExists(resetCode.Username) + admin, err = getActiveAdmin(resetCode.Username, ipAddr) if err != nil { return &admin, &user, util.NewValidationError("unable to associate the confirmation code with an existing admin") } @@ -851,7 +878,7 @@ func handleResetPassword(r *http.Request, code, newPassword, confirmPassword str err = resetCodesMgr.Delete(code) return &admin, &user, err } - user, err = dataprovider.GetUserWithGroupSettings(resetCode.Username, "") + user, err = getActiveUser(resetCode.Username, r) if err != nil { return &admin, &user, util.NewValidationError("Unable to associate the confirmation code with an existing user") } diff --git a/internal/httpd/httpd_test.go b/internal/httpd/httpd_test.go index f77fd1f9..7c14b8e1 100644 --- a/internal/httpd/httpd_test.go +++ b/internal/httpd/httpd_test.go @@ -25413,6 +25413,24 @@ func TestAdminForgotPassword(t *testing.T) { lastResetCode = "" form.Set("username", altAdminUsername) + // disable the admin + admin.Status = 0 + admin, _, err = httpdtest.UpdateAdmin(admin, http.StatusOK) + assert.NoError(t, err) + + req, err = http.NewRequest(http.MethodPost, webAdminForgotPwdPath, bytes.NewBuffer([]byte(form.Encode()))) + assert.NoError(t, err) + req.RemoteAddr = defaultRemoteAddr + setLoginCookie(req, loginCookie) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + rr = executeRequest(req) + assert.Equal(t, http.StatusFound, rr.Code) + assert.Len(t, lastResetCode, 0) + + admin.Status = 1 + admin, _, err = httpdtest.UpdateAdmin(admin, http.StatusOK) + assert.NoError(t, err) + req, err = http.NewRequest(http.MethodPost, webAdminForgotPwdPath, bytes.NewBuffer([]byte(form.Encode()))) assert.NoError(t, err) req.RemoteAddr = defaultRemoteAddr @@ -25451,7 +25469,10 @@ func TestAdminForgotPassword(t *testing.T) { rr = executeRequest(req) assert.Equal(t, http.StatusOK, rr.Code) assert.Contains(t, rr.Body.String(), util.I18nErrorChangePwdGeneric) - // ok + // disable the admin + admin.Status = 0 + admin, _, err = httpdtest.UpdateAdmin(admin, http.StatusOK) + assert.NoError(t, err) form.Set("code", lastResetCode) req, err = http.NewRequest(http.MethodPost, webAdminResetPwdPath, bytes.NewBuffer([]byte(form.Encode()))) assert.NoError(t, err) @@ -25459,6 +25480,19 @@ func TestAdminForgotPassword(t *testing.T) { setLoginCookie(req, loginCookie) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") rr = executeRequest(req) + assert.Equal(t, http.StatusOK, rr.Code) + assert.Contains(t, rr.Body.String(), util.I18nErrorChangePwdGeneric) + + admin.Status = 1 + admin, _, err = httpdtest.UpdateAdmin(admin, http.StatusOK) + assert.NoError(t, err) + // ok + req, err = http.NewRequest(http.MethodPost, webAdminResetPwdPath, bytes.NewBuffer([]byte(form.Encode()))) + assert.NoError(t, err) + req.RemoteAddr = defaultRemoteAddr + setLoginCookie(req, loginCookie) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + rr = executeRequest(req) assert.Equal(t, http.StatusFound, rr.Code) loginCookie, csrfToken, err = getCSRFTokenMock(webLoginPath, defaultRemoteAddr) @@ -25593,10 +25627,11 @@ func TestUserForgotPassword(t *testing.T) { rr = executeRequest(req) assert.Equal(t, http.StatusOK, rr.Code) assert.Contains(t, rr.Body.String(), util.I18nErrorPwdResetForbidded) + user.ExpirationDate = util.GetTimeAsMsSinceEpoch(time.Now().Add(-1 * time.Hour)) user.Filters.WebClient = []string{sdk.WebClientAPIKeyAuthChangeDisabled} user, _, err = httpdtest.UpdateUser(user, http.StatusOK, "") assert.NoError(t, err) - + // user is expired lastResetCode = "" req, err = http.NewRequest(http.MethodPost, webClientForgotPwdPath, bytes.NewBuffer([]byte(form.Encode()))) assert.NoError(t, err) @@ -25605,6 +25640,18 @@ func TestUserForgotPassword(t *testing.T) { req.Header.Set("Content-Type", "application/x-www-form-urlencoded") rr = executeRequest(req) assert.Equal(t, http.StatusFound, rr.Code) + assert.Len(t, lastResetCode, 0) + + user.ExpirationDate = util.GetTimeAsMsSinceEpoch(time.Now().Add(24 * time.Hour)) + user, _, err = httpdtest.UpdateUser(user, http.StatusOK, "") + assert.NoError(t, err) + req, err = http.NewRequest(http.MethodPost, webClientForgotPwdPath, bytes.NewBuffer([]byte(form.Encode()))) + assert.NoError(t, err) + req.RemoteAddr = defaultRemoteAddr + setLoginCookie(req, loginCookie) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + rr = executeRequest(req) + assert.Equal(t, http.StatusFound, rr.Code) assert.GreaterOrEqual(t, len(lastResetCode), 20) // no login token form = make(url.Values) @@ -25648,8 +25695,23 @@ func TestUserForgotPassword(t *testing.T) { rr = executeRequest(req) assert.Equal(t, http.StatusOK, rr.Code) assert.Contains(t, rr.Body.String(), util.I18nErrorChangePwdGeneric) - // ok + // Invalid login condition form.Set("code", lastResetCode) + user.Filters.DeniedProtocols = []string{common.ProtocolHTTP} + user, _, err = httpdtest.UpdateUser(user, http.StatusOK, "") + assert.NoError(t, err) + req, err = http.NewRequest(http.MethodPost, webClientResetPwdPath, bytes.NewBuffer([]byte(form.Encode()))) + assert.NoError(t, err) + req.RemoteAddr = defaultRemoteAddr + setLoginCookie(req, loginCookie) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + rr = executeRequest(req) + assert.Equal(t, http.StatusOK, rr.Code) + assert.Contains(t, rr.Body.String(), util.I18nErrorChangePwdGeneric) + // ok + user.Filters.DeniedProtocols = []string{common.ProtocolFTP} + user, _, err = httpdtest.UpdateUser(user, http.StatusOK, "") + assert.NoError(t, err) req, err = http.NewRequest(http.MethodPost, webClientResetPwdPath, bytes.NewBuffer([]byte(form.Encode()))) assert.NoError(t, err) req.RemoteAddr = defaultRemoteAddr diff --git a/internal/httpd/server.go b/internal/httpd/server.go index e8fe32e2..c8a7e3c9 100644 --- a/internal/httpd/server.go +++ b/internal/httpd/server.go @@ -308,7 +308,7 @@ func (s *httpdServer) handleWebClientPasswordResetPost(w http.ResponseWriter, r } connectionID := fmt.Sprintf("%v_%v", getProtocolFromRequest(r), xid.New().String()) if err := checkHTTPClientUser(user, r, connectionID, true); err != nil { - s.renderClientResetPwdPage(w, r, util.NewI18nError(err, util.I18nErrorDirList403)) + s.renderClientResetPwdPage(w, r, util.NewI18nError(err, util.I18nErrorLoginAfterReset)) return }