From 315819094588cd46881cc70c4e82ea5104264023 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Sun, 4 Feb 2024 12:09:47 +0100 Subject: [PATCH] WebClient: respect second factor requirements enforced at group level Fixes #1506 Signed-off-by: Nicola Murino --- go.mod | 2 +- go.sum | 4 +- internal/httpd/api_mfa.go | 8 +-- internal/httpd/httpd_test.go | 101 +++++++++++++++++++++++++++++++++++ internal/httpd/webclient.go | 2 +- 5 files changed, 109 insertions(+), 8 deletions(-) diff --git a/go.mod b/go.mod index ecf1e50a..65e49e02 100644 --- a/go.mod +++ b/go.mod @@ -36,7 +36,7 @@ require ( github.com/hashicorp/go-hclog v1.6.2 github.com/hashicorp/go-plugin v1.6.0 github.com/hashicorp/go-retryablehttp v0.7.5 - github.com/jackc/pgx/v5 v5.5.2 + github.com/jackc/pgx/v5 v5.5.3 github.com/jlaffaye/ftp v0.0.0-20201112195030-9aae4d151126 github.com/klauspost/compress v1.17.5 github.com/lestrrat-go/jwx/v2 v2.0.19 diff --git a/go.sum b/go.sum index b8ab4221..1b8e9aac 100644 --- a/go.sum +++ b/go.sum @@ -235,8 +235,8 @@ github.com/jackc/pgpassfile v1.0.0 h1:/6Hmqy13Ss2zCq62VdNG8tM1wchn8zjSGOBJ6icpsI github.com/jackc/pgpassfile v1.0.0/go.mod h1:CEx0iS5ambNFdcRtxPj5JhEz+xB6uRky5eyVu/W2HEg= github.com/jackc/pgservicefile v0.0.0-20231201235250-de7065d80cb9 h1:L0QtFUgDarD7Fpv9jeVMgy/+Ec0mtnmYuImjTz6dtDA= github.com/jackc/pgservicefile v0.0.0-20231201235250-de7065d80cb9/go.mod h1:5TJZWKEWniPve33vlWYSoGYefn3gLQRzjfDlhSJ9ZKM= -github.com/jackc/pgx/v5 v5.5.2 h1:iLlpgp4Cp/gC9Xuscl7lFL1PhhW+ZLtXZcrfCt4C3tA= -github.com/jackc/pgx/v5 v5.5.2/go.mod h1:ez9gk+OAat140fv9ErkZDYFWmXLfV+++K0uAOiwgm1A= +github.com/jackc/pgx/v5 v5.5.3 h1:Ces6/M3wbDXYpM8JyyPD57ivTtJACFZJd885pdIaV2s= +github.com/jackc/pgx/v5 v5.5.3/go.mod h1:ez9gk+OAat140fv9ErkZDYFWmXLfV+++K0uAOiwgm1A= github.com/jackc/puddle/v2 v2.2.1 h1:RhxXJtFG022u4ibrCSMSiu5aOq1i77R3OHKNJj77OAk= github.com/jackc/puddle/v2 v2.2.1/go.mod h1:vriiEXHvEE654aYKXXjOvZM39qJ0q+azkZFrfEOc3H4= github.com/jhump/protoreflect v1.15.1 h1:HUMERORf3I3ZdX05WaQ6MIpd/NJ434hTp5YiKgfCL6c= diff --git a/internal/httpd/api_mfa.go b/internal/httpd/api_mfa.go index e2928226..fbd5a361 100644 --- a/internal/httpd/api_mfa.go +++ b/internal/httpd/api_mfa.go @@ -260,7 +260,7 @@ func getNewRecoveryCode() string { } func saveUserTOTPConfig(username string, r *http.Request, recoveryCodes []dataprovider.RecoveryCode) error { - user, err := dataprovider.UserExists(username, "") + user, userMerged, err := dataprovider.GetUserVariants(username, "") if err != nil { return err } @@ -270,13 +270,13 @@ func saveUserTOTPConfig(username string, r *http.Request, recoveryCodes []datapr if err != nil { return util.NewValidationError(fmt.Sprintf("unable to decode JSON body: %v", err)) } - if !user.Filters.TOTPConfig.Enabled && len(user.Filters.TwoFactorAuthProtocols) > 0 { + if !user.Filters.TOTPConfig.Enabled && len(userMerged.Filters.TwoFactorAuthProtocols) > 0 { return util.NewValidationError("two-factor authentication must be enabled") } - for _, p := range user.Filters.TwoFactorAuthProtocols { + for _, p := range userMerged.Filters.TwoFactorAuthProtocols { if !util.Contains(user.Filters.TOTPConfig.Protocols, p) { return util.NewValidationError(fmt.Sprintf("totp: the following protocols are required: %q", - strings.Join(user.Filters.TwoFactorAuthProtocols, ", "))) + strings.Join(userMerged.Filters.TwoFactorAuthProtocols, ", "))) } } if user.Filters.TOTPConfig.Secret == nil || !user.Filters.TOTPConfig.Secret.IsPlain() { diff --git a/internal/httpd/httpd_test.go b/internal/httpd/httpd_test.go index 1cd59d1b..36bebfba 100644 --- a/internal/httpd/httpd_test.go +++ b/internal/httpd/httpd_test.go @@ -3401,6 +3401,107 @@ func TestTwoFactorRequirements(t *testing.T) { assert.NoError(t, err) } +func TestTwoFactorRequirementsGroupLevel(t *testing.T) { + g := getTestGroup() + g.UserSettings.Filters.TwoFactorAuthProtocols = []string{common.ProtocolHTTP, common.ProtocolFTP} + group, _, err := httpdtest.AddGroup(g, http.StatusCreated) + assert.NoError(t, err) + u := getTestUser() + u.Groups = []sdk.GroupMapping{ + { + Name: group.Name, + Type: sdk.GroupTypePrimary, + }, + } + user, _, err := httpdtest.AddUser(u, http.StatusCreated) + assert.NoError(t, err) + + token, err := getJWTAPIUserTokenFromTestServer(defaultUsername, defaultPassword) + assert.NoError(t, err) + webToken, err := getJWTWebClientTokenFromTestServer(defaultUsername, defaultPassword) + assert.NoError(t, err) + + req, err := http.NewRequest(http.MethodGet, webClientFilesPath, nil) + assert.NoError(t, err) + req.RequestURI = webClientFilesPath + setJWTCookieForReq(req, webToken) + rr := executeRequest(req) + checkResponseCode(t, http.StatusForbidden, rr) + assert.Contains(t, rr.Body.String(), util.I18nError2FARequired) + + req, err = http.NewRequest(http.MethodGet, userDirsPath, nil) + assert.NoError(t, err) + setBearerForReq(req, token) + rr = executeRequest(req) + checkResponseCode(t, http.StatusForbidden, rr) + assert.Contains(t, rr.Body.String(), "Two-factor authentication requirements not met, please configure two-factor authentication for the following protocols") + + configName, key, _, err := mfa.GenerateTOTPSecret(mfa.GetAvailableTOTPConfigNames()[0], user.Username) + assert.NoError(t, err) + userTOTPConfig := dataprovider.UserTOTPConfig{ + Enabled: true, + ConfigName: configName, + Secret: kms.NewPlainSecret(key.Secret()), + Protocols: []string{common.ProtocolHTTP}, + } + asJSON, err := json.Marshal(userTOTPConfig) + assert.NoError(t, err) + req, err = http.NewRequest(http.MethodPost, userTOTPSavePath, bytes.NewBuffer(asJSON)) + assert.NoError(t, err) + setBearerForReq(req, token) + rr = executeRequest(req) + checkResponseCode(t, http.StatusBadRequest, rr) + assert.Contains(t, rr.Body.String(), "the following protocols are required") + + userTOTPConfig = dataprovider.UserTOTPConfig{ + Enabled: true, + ConfigName: configName, + Secret: kms.NewPlainSecret(key.Secret()), + Protocols: []string{common.ProtocolFTP, common.ProtocolHTTP}, + } + asJSON, err = json.Marshal(userTOTPConfig) + assert.NoError(t, err) + req, err = http.NewRequest(http.MethodPost, userTOTPSavePath, bytes.NewBuffer(asJSON)) + assert.NoError(t, err) + setBearerForReq(req, token) + rr = executeRequest(req) + checkResponseCode(t, http.StatusOK, rr) + + // now get new tokens and check that the two factor requirements are now met + passcode, err := generateTOTPPasscode(key.Secret()) + assert.NoError(t, err) + req, err = http.NewRequest(http.MethodGet, fmt.Sprintf("%v%v", httpBaseURL, userTokenPath), nil) + assert.NoError(t, err) + req.Header.Set("X-SFTPGO-OTP", passcode) + req.SetBasicAuth(defaultUsername, defaultPassword) + resp, err := httpclient.GetHTTPClient().Do(req) + assert.NoError(t, err) + assert.Equal(t, http.StatusOK, resp.StatusCode) + responseHolder := make(map[string]any) + err = render.DecodeJSON(resp.Body, &responseHolder) + assert.NoError(t, err) + userToken := responseHolder["access_token"].(string) + assert.NotEmpty(t, userToken) + err = resp.Body.Close() + assert.NoError(t, err) + + req, err = http.NewRequest(http.MethodGet, fmt.Sprintf("%v%v", httpBaseURL, userDirsPath), nil) + assert.NoError(t, err) + setBearerForReq(req, userToken) + resp, err = httpclient.GetHTTPClient().Do(req) + assert.NoError(t, err) + assert.Equal(t, http.StatusOK, resp.StatusCode) + err = resp.Body.Close() + assert.NoError(t, err) + + _, err = httpdtest.RemoveUser(user, http.StatusOK) + assert.NoError(t, err) + err = os.RemoveAll(user.GetHomeDir()) + assert.NoError(t, err) + _, err = httpdtest.RemoveGroup(group, http.StatusOK) + assert.NoError(t, err) +} + func TestLoginUserAPITOTP(t *testing.T) { user, _, err := httpdtest.AddUser(getTestUser(), http.StatusCreated) assert.NoError(t, err) diff --git a/internal/httpd/webclient.go b/internal/httpd/webclient.go index a08b4266..2b540bd2 100644 --- a/internal/httpd/webclient.go +++ b/internal/httpd/webclient.go @@ -663,7 +663,7 @@ func (s *httpdServer) renderClientMFAPage(w http.ResponseWriter, r *http.Request RecCodesURL: webClientRecoveryCodesPath, Protocols: dataprovider.MFAProtocols, } - user, err := dataprovider.UserExists(data.LoggedUser.Username, "") + user, err := dataprovider.GetUserWithGroupSettings(data.LoggedUser.Username, "") if err != nil { s.renderClientInternalServerErrorPage(w, r, err) return