MFA: allow recovery codes only if two-factor auth is enabled

Fixes #965

Signed-off-by: Nicola Murino <nicola.murino@gmail.com>

go.mod

@@ -9,14 +9,14 @@ require (
 	github.com/GehirnInc/crypt v0.0.0-20200316065508-bb7000b8a962
 	github.com/alexedwards/argon2id v0.0.0-20211130144151-3585854a6387
 	github.com/aws/aws-sdk-go-v2 v1.16.12
-	github.com/aws/aws-sdk-go-v2/config v1.17.2
+	github.com/aws/aws-sdk-go-v2/config v1.17.3
-	github.com/aws/aws-sdk-go-v2/credentials v1.12.15
+	github.com/aws/aws-sdk-go-v2/credentials v1.12.16
 	github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.12.13
-	github.com/aws/aws-sdk-go-v2/feature/s3/manager v1.11.28
+	github.com/aws/aws-sdk-go-v2/feature/s3/manager v1.11.29
-	github.com/aws/aws-sdk-go-v2/service/marketplacemetering v1.13.14
+	github.com/aws/aws-sdk-go-v2/service/marketplacemetering v1.13.15
-	github.com/aws/aws-sdk-go-v2/service/s3 v1.27.6
+	github.com/aws/aws-sdk-go-v2/service/s3 v1.27.7
-	github.com/aws/aws-sdk-go-v2/service/secretsmanager v1.15.19
+	github.com/aws/aws-sdk-go-v2/service/secretsmanager v1.15.20
-	github.com/aws/aws-sdk-go-v2/service/sts v1.16.14
+	github.com/aws/aws-sdk-go-v2/service/sts v1.16.15
 	github.com/cockroachdb/cockroach-go/v2 v2.2.15
 	github.com/coreos/go-oidc/v3 v3.2.0
 	github.com/eikenb/pipeat v0.0.0-20210730190139-06b3e6902001
@@ -65,7 +65,7 @@ require (
 	go.etcd.io/bbolt v1.3.6
 	go.uber.org/automaxprocs v1.5.1
 	gocloud.dev v0.26.0
-	golang.org/x/crypto v0.0.0-20220826181053-bd7e27e6170d
+	golang.org/x/crypto v0.0.0-20220829220503-c86fa9a7ed90
 	golang.org/x/net v0.0.0-20220826154423-83b083e8dc8b
 	golang.org/x/oauth2 v0.0.0-20220822191816-0ebed06d0094
 	golang.org/x/sys v0.0.0-20220829200755-d48e67d00261
@@ -89,7 +89,7 @@ require (
 	github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.1.14 // indirect
 	github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.9.13 // indirect
 	github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.13.13 // indirect
-	github.com/aws/aws-sdk-go-v2/service/sso v1.11.18 // indirect
+	github.com/aws/aws-sdk-go-v2/service/sso v1.11.19 // indirect
 	github.com/aws/aws-sdk-go-v2/service/ssooidc v1.13.1 // indirect
 	github.com/aws/smithy-go v1.13.0 // indirect
 	github.com/beorn7/perks v1.0.1 // indirect
@@ -168,6 +168,6 @@ require (
 replace (
 	github.com/jlaffaye/ftp => github.com/drakkan/ftp v0.0.0-20201114075148-9b9adce499a9
 	github.com/pkg/sftp => github.com/drakkan/sftp v0.0.0-20220716075551-51a5aa4e044d
-	golang.org/x/crypto => github.com/drakkan/crypto v0.0.0-20220828084757-61f5262cc94f
+	golang.org/x/crypto => github.com/drakkan/crypto v0.0.0-20220831070132-e3c36f2ab82b
 	golang.org/x/net => github.com/drakkan/net v0.0.0-20220828084259-1562d1fb0fc5
 )

go.sum

@@ -148,17 +148,17 @@ github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.4.1/go.mod h1:n8Bs1ElDD
 github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.4.5 h1:7A1nDFvkVlBmMa69QMLkw/m/DDHm6PUluIYK61aQoOY=
 github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.4.5/go.mod h1:DnlOnWR2YuzMXNSHHNuoklObUE3SwWlcRTGL/zL+Aj8=
 github.com/aws/aws-sdk-go-v2/config v1.15.3/go.mod h1:9YL3v07Xc/ohTsxFXzan9ZpFpdTOFl4X65BAKYaz8jg=
-github.com/aws/aws-sdk-go-v2/config v1.17.2 h1:V96WPd2a1H/MXGZjk4zto+KpYnwZI2kdIdy/cI8kYnQ=
+github.com/aws/aws-sdk-go-v2/config v1.17.3 h1:s1As/fiVMmM3CObC4GcSaSbkhm88S6a5qn8St3wgal0=
-github.com/aws/aws-sdk-go-v2/config v1.17.2/go.mod h1:jumS/AMwul4WaG8vyXsF6kUndG9zndR+yfYBwl4i9ds=
+github.com/aws/aws-sdk-go-v2/config v1.17.3/go.mod h1:tRGUOfk9Rrf6UCJm5qDlL9AizSsgvteuKX4qajAV3pU=
 github.com/aws/aws-sdk-go-v2/credentials v1.11.2/go.mod h1:j8YsY9TXTm31k4eFhspiQicfXPLZ0gYXA50i4gxPE8g=
-github.com/aws/aws-sdk-go-v2/credentials v1.12.15 h1:6DONxG9cR3pAuISj1Irh5u2SRqCfIJwyHNyDDes7SZw=
+github.com/aws/aws-sdk-go-v2/credentials v1.12.16 h1:HXczS88Pg36j8dq0KSjtHBPFs8gdRyBSS1hueeG/rxA=
-github.com/aws/aws-sdk-go-v2/credentials v1.12.15/go.mod h1:41zTC6U/78fUD7ZCa5NymTJANDjfqySg5YEAYVFl2Ic=
+github.com/aws/aws-sdk-go-v2/credentials v1.12.16/go.mod h1:eLJ+j1lwQdHJ0c56tRoDWcgss1e/laVmvW2AaOicuAw=
 github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.12.3/go.mod h1:uk1vhHHERfSVCUnqSqz8O48LBYDSC+k6brng09jcMOk=
 github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.12.13 h1:+uferi8SUDZtMloCDt24Zenyy/i71C/ua5mjUCpbpN0=
 github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.12.13/go.mod h1:y0eXmsNBFIVjUE8ZBjES8myOHlMsXDz7qGT93+MVdjk=
 github.com/aws/aws-sdk-go-v2/feature/s3/manager v1.11.3/go.mod h1:0dHuD2HZZSiwfJSy1FO5bX1hQ1TxVV1QXXjpn3XUE44=
-github.com/aws/aws-sdk-go-v2/feature/s3/manager v1.11.28 h1:9aD3yJFiaU2MIs34XY/CjKFs//cZPdtGiGT2sm3XG6c=
+github.com/aws/aws-sdk-go-v2/feature/s3/manager v1.11.29 h1:VKi/79iKGaZ9pJTSuj/gNlzJdFczcGcsw9NDAT7I+hY=
-github.com/aws/aws-sdk-go-v2/feature/s3/manager v1.11.28/go.mod h1:hV8r7xrO3IghGtC87aX0JiJsN2zJhuzFExLSmgZ+7ek=
+github.com/aws/aws-sdk-go-v2/feature/s3/manager v1.11.29/go.mod h1:ge60sLiMug/7ubLIbRyM9zNv5fR99ZzR+staDaM7+Tw=
 github.com/aws/aws-sdk-go-v2/internal/configsources v1.1.9/go.mod h1:AnVH5pvai0pAF4lXRq0bmhbes1u9R8wTE+g+183bZNM=
 github.com/aws/aws-sdk-go-v2/internal/configsources v1.1.19 h1:gC5mudiFrWGhzcdoWj1iCGUfrzCpQG0MQIQf0CXFFQQ=
 github.com/aws/aws-sdk-go-v2/internal/configsources v1.1.19/go.mod h1:llxE6bwUZhuCas0K7qGiu5OgMis3N7kdWtFSxoHmJ7E=
@@ -183,25 +183,25 @@ github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.13.3/go.mod h1:Bm/v2Ia
 github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.13.13 h1:h1equp9qdWANft5cmtDUditRlALvE7tuaHs2RdSbsQg=
 github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.13.13/go.mod h1:3RA7cs1uHkbV3f6tMYy7u0OfkyVckZBM70wUS4h1MDk=
 github.com/aws/aws-sdk-go-v2/service/kms v1.16.3/go.mod h1:QuiHPBqlOFCi4LqdSskYYAWpQlx3PKmohy+rE2F+o5g=
-github.com/aws/aws-sdk-go-v2/service/marketplacemetering v1.13.14 h1:z0DStfYeB7FVRU5rCPKngvEFdGtz13L2g48KFHxkIsA=
+github.com/aws/aws-sdk-go-v2/service/marketplacemetering v1.13.15 h1:ek8ACOAGvDWRm1kFCcj22soNkkLFh4WPBFv7BdWqebs=
-github.com/aws/aws-sdk-go-v2/service/marketplacemetering v1.13.14/go.mod h1:Zf+Tf40dskiGdwVJU2HIgln1vtnQF8QpsguBsbI5Uq8=
+github.com/aws/aws-sdk-go-v2/service/marketplacemetering v1.13.15/go.mod h1:Zf+Tf40dskiGdwVJU2HIgln1vtnQF8QpsguBsbI5Uq8=
 github.com/aws/aws-sdk-go-v2/service/s3 v1.26.3/go.mod h1:g1qvDuRsJY+XghsV6zg00Z4KJ7DtFFCx8fJD2a491Ak=
-github.com/aws/aws-sdk-go-v2/service/s3 v1.27.6 h1:4+iM+rBAcS9zZwQ2+Y57GgC41cRe6C8khdYfqaUT2k0=
+github.com/aws/aws-sdk-go-v2/service/s3 v1.27.7 h1:BlxqVULzNS7udJIwZBJdL8NNcLbSwgXv/WRJCVUaMm8=
-github.com/aws/aws-sdk-go-v2/service/s3 v1.27.6/go.mod h1:orjy5IRgBQnh9EI/lMW7YGF6eYk6re8HPFbL66a2DSo=
+github.com/aws/aws-sdk-go-v2/service/s3 v1.27.7/go.mod h1:orjy5IRgBQnh9EI/lMW7YGF6eYk6re8HPFbL66a2DSo=
 github.com/aws/aws-sdk-go-v2/service/secretsmanager v1.15.4/go.mod h1:PJc8s+lxyU8rrre0/4a0pn2wgwiDvOEzoOjcJUBr67o=
-github.com/aws/aws-sdk-go-v2/service/secretsmanager v1.15.19 h1:uzk2J2iR959Rp78X9BWxyuk5pWw4P5s1FO1rGnvN4k0=
+github.com/aws/aws-sdk-go-v2/service/secretsmanager v1.15.20 h1:j41VjMJNc5T9AWkLf/FdVtR46st2PZYB/6xoBBY2/8Q=
-github.com/aws/aws-sdk-go-v2/service/secretsmanager v1.15.19/go.mod h1:F2AUfGEOcxpOTzo/+Bur5PrtsvnhVQQbd4CGfPicOpw=
+github.com/aws/aws-sdk-go-v2/service/secretsmanager v1.15.20/go.mod h1:F2AUfGEOcxpOTzo/+Bur5PrtsvnhVQQbd4CGfPicOpw=
 github.com/aws/aws-sdk-go-v2/service/sns v1.17.4/go.mod h1:kElt+uCcXxcqFyc+bQqZPFD9DME/eC6oHBXvFzQ9Bcw=
 github.com/aws/aws-sdk-go-v2/service/sqs v1.18.3/go.mod h1:skmQo0UPvsjsuYYSYMVmrPc1HWCbHUJyrCEp+ZaLzqM=
 github.com/aws/aws-sdk-go-v2/service/ssm v1.24.1/go.mod h1:NR/xoKjdbRJ+qx0pMR4mI+N/H1I1ynHwXnO6FowXJc0=
 github.com/aws/aws-sdk-go-v2/service/sso v1.11.3/go.mod h1:7UQ/e69kU7LDPtY40OyoHYgRmgfGM4mgsLYtcObdveU=
-github.com/aws/aws-sdk-go-v2/service/sso v1.11.18 h1:gTn1a/FbcOXK5LQS88dD5k+PKwyjVvhAEEwyN4c6eW8=
+github.com/aws/aws-sdk-go-v2/service/sso v1.11.19 h1:WdCwfJmu23XiIDeZwclSyAorQe916M3LeHd53xqBjfA=
-github.com/aws/aws-sdk-go-v2/service/sso v1.11.18/go.mod h1:ytmEi5+qwcSNcV2pVA8PIb1DnKT/0Bu/K4nfJHwoM6c=
+github.com/aws/aws-sdk-go-v2/service/sso v1.11.19/go.mod h1:ytmEi5+qwcSNcV2pVA8PIb1DnKT/0Bu/K4nfJHwoM6c=
 github.com/aws/aws-sdk-go-v2/service/ssooidc v1.13.1 h1:p48IfndYbRk3iDsoQAmVXdCKEM5+7Y50JAPikjwk8gI=
 github.com/aws/aws-sdk-go-v2/service/ssooidc v1.13.1/go.mod h1:NY+G+8PW0ISyJ7/6t5mgOe6qpJiwZa9Jix05WPscJjg=
 github.com/aws/aws-sdk-go-v2/service/sts v1.16.3/go.mod h1:bfBj0iVmsUyUg4weDB4NxktD9rDGeKSVWnjTnwbx9b8=
-github.com/aws/aws-sdk-go-v2/service/sts v1.16.14 h1:7kxso8VZLQ86Jg27QRBw6fjrQhQ8CMNMZ7SB0w7RQiA=
+github.com/aws/aws-sdk-go-v2/service/sts v1.16.15 h1:ApuR2BK9vf5/XXsImHBBsYJ6aUhmUhBHnZMPyhJo1jQ=
-github.com/aws/aws-sdk-go-v2/service/sts v1.16.14/go.mod h1:Y+BUV19q3OmQVqNUlbZ40zVi3NM6Biuxwkx/qdSD/CY=
+github.com/aws/aws-sdk-go-v2/service/sts v1.16.15/go.mod h1:Y+BUV19q3OmQVqNUlbZ40zVi3NM6Biuxwkx/qdSD/CY=
 github.com/aws/smithy-go v1.11.2/go.mod h1:3xHYmszWVx2c0kIwQeEVf9uSm4fYZt67FBJnwub1bgM=
 github.com/aws/smithy-go v1.13.0 h1:YfyEmSJLo7fAv8FbuDK4R8F9aAmi9DZ88Zb/KJJmUl0=
 github.com/aws/smithy-go v1.13.0/go.mod h1:Tg+OJXh4MB2R/uN61Ko2f6hTZwB/ZYGOtib8J3gBHzA=
@@ -262,8 +262,8 @@ github.com/dimchansky/utfbom v1.1.0/go.mod h1:rO41eb7gLfo8SF1jd9F8HplJm1Fewwi4mQ
 github.com/dimchansky/utfbom v1.1.1/go.mod h1:SxdoEBH5qIqFocHMyGOXVAybYJdr71b1Q/j0mACtrfE=
 github.com/dnaeon/go-vcr v1.2.0 h1:zHCHvJYTMh1N7xnV7zf1m1GPBF9Ad0Jk/whtQ1663qI=
 github.com/dnaeon/go-vcr v1.2.0/go.mod h1:R4UdLID7HZT3taECzJs4YgbbH6PIGXB6W/sc5OLb6RQ=
-github.com/drakkan/crypto v0.0.0-20220828084757-61f5262cc94f h1:fFnBNoP0CQJZcdDSV35Wjf7aeZE6AOW0WoC3XqIgzVY=
+github.com/drakkan/crypto v0.0.0-20220831070132-e3c36f2ab82b h1:kCNBtUFKfhiUaE1ZMgh83pXFVP2ZIHktwV15lmgD0Ok=
-github.com/drakkan/crypto v0.0.0-20220828084757-61f5262cc94f/go.mod h1:SiM6ypd8Xu1xldObYtbDztuUU7xUzMnUULfphXFZmro=
+github.com/drakkan/crypto v0.0.0-20220831070132-e3c36f2ab82b/go.mod h1:SiM6ypd8Xu1xldObYtbDztuUU7xUzMnUULfphXFZmro=
 github.com/drakkan/ftp v0.0.0-20201114075148-9b9adce499a9 h1:LPH1dEblAOO/LoG7yHPMtBLXhQmjaga91/DDjWk9jWA=
 github.com/drakkan/ftp v0.0.0-20201114075148-9b9adce499a9/go.mod h1:2lmrmq866uF2tnje75wQHzmPXhmSWUt7Gyx2vgK1RCU=
 github.com/drakkan/net v0.0.0-20220828084259-1562d1fb0fc5 h1:+sVMXrU1DiQLNDgz1KvybqHEzRf8KuX5xQW8fpii6rI=

internal/common/common.go

@@ -843,6 +843,9 @@ func (conns *ActiveConnections) Remove(connectionID string) {
 			conns.connections[lastIdx] = nil
 			conns.connections = conns.connections[:lastIdx]
 			conns.removeUserConnection(conn.GetUsername())
+			metric.UpdateActiveConnectionsSize(lastIdx)
+			logger.Debug(conn.GetProtocol(), conn.GetID(), "connection removed, local address %#v, remote address %#v close fs error: %v, num open connections: %v",
+				conn.GetLocalAddress(), conn.GetRemoteAddress(), err, lastIdx)
 			if conn.GetProtocol() == ProtocolFTP && conn.GetUsername() == "" {
 				ip := util.GetIPFromRemoteAddress(conn.GetRemoteAddress())
 				logger.ConnectionFailedLog("", ip, dataprovider.LoginMethodNoAuthTryed, conn.GetProtocol(),
@@ -852,9 +855,6 @@ func (conns *ActiveConnections) Remove(connectionID string) {
 				dataprovider.ExecutePostLoginHook(&dataprovider.User{}, dataprovider.LoginMethodNoAuthTryed, ip,
 					conn.GetProtocol(), dataprovider.ErrNoAuthTryed)
 			}
-			metric.UpdateActiveConnectionsSize(lastIdx)
-			logger.Debug(conn.GetProtocol(), conn.GetID(), "connection removed, local address %#v, remote address %#v close fs error: %v, num open connections: %v",
-				conn.GetLocalAddress(), conn.GetRemoteAddress(), err, lastIdx)
 			Config.checkPostDisconnectHook(conn.GetRemoteAddress(), conn.GetProtocol(), conn.GetUsername(),
 				conn.GetID(), conn.GetConnectionTime())
 			return

internal/httpd/api_mfa.go

@@ -15,6 +15,7 @@
 package httpd
 
 import (
+	"errors"
 	"fmt"
 	"net/http"
 	"strings"
@@ -27,6 +28,10 @@ import (
 	"github.com/drakkan/sftpgo/v2/internal/util"
 )
 
+var (
+	errRecoveryCodeForbidden = errors.New("recovery codes are not available with two-factor authentication disabled")
+)
+
 type generateTOTPRequest struct {
 	ConfigName string `json:"config_name"`
 }
@@ -152,6 +157,10 @@ func getRecoveryCodes(w http.ResponseWriter, r *http.Request) {
 			sendAPIResponse(w, r, err, "", getRespStatus(err))
 			return
 		}
+		if !user.Filters.TOTPConfig.Enabled {
+			sendAPIResponse(w, r, errRecoveryCodeForbidden, "", http.StatusForbidden)
+			return
+		}
 		accountRecoveryCodes = user.Filters.RecoveryCodes
 	} else {
 		admin, err := dataprovider.AdminExists(claims.Username)
@@ -159,6 +168,10 @@ func getRecoveryCodes(w http.ResponseWriter, r *http.Request) {
 			sendAPIResponse(w, r, err, "", getRespStatus(err))
 			return
 		}
+		if !admin.Filters.TOTPConfig.Enabled {
+			sendAPIResponse(w, r, errRecoveryCodeForbidden, "", http.StatusForbidden)
+			return
+		}
 		accountRecoveryCodes = admin.Filters.RecoveryCodes
 	}
 
@@ -195,6 +208,10 @@ func generateRecoveryCodes(w http.ResponseWriter, r *http.Request) {
 			sendAPIResponse(w, r, err, "", getRespStatus(err))
 			return
 		}
+		if !user.Filters.TOTPConfig.Enabled {
+			sendAPIResponse(w, r, errRecoveryCodeForbidden, "", http.StatusForbidden)
+			return
+		}
 		user.Filters.RecoveryCodes = accountRecoveryCodes
 		if err := dataprovider.UpdateUser(&user, dataprovider.ActionExecutorSelf, util.GetIPFromRemoteAddress(r.RemoteAddr)); err != nil {
 			sendAPIResponse(w, r, err, "", getRespStatus(err))
@@ -206,6 +223,10 @@ func generateRecoveryCodes(w http.ResponseWriter, r *http.Request) {
 			sendAPIResponse(w, r, err, "", getRespStatus(err))
 			return
 		}
+		if !admin.Filters.TOTPConfig.Enabled {
+			sendAPIResponse(w, r, errRecoveryCodeForbidden, "", http.StatusForbidden)
+			return
+		}
 		admin.Filters.RecoveryCodes = accountRecoveryCodes
 		if err := dataprovider.UpdateAdmin(&admin, dataprovider.ActionExecutorSelf, util.GetIPFromRemoteAddress(r.RemoteAddr)); err != nil {
 			sendAPIResponse(w, r, err, "", getRespStatus(err))
@@ -243,8 +264,12 @@ func saveUserTOTPConfig(username string, r *http.Request, recoveryCodes []datapr
 	if user.Filters.TOTPConfig.Secret == nil || !user.Filters.TOTPConfig.Secret.IsPlain() {
 		user.Filters.TOTPConfig.Secret = currentTOTPSecret
 	}
-	if user.CountUnusedRecoveryCodes() < 5 && user.Filters.TOTPConfig.Enabled {
+	if user.Filters.TOTPConfig.Enabled {
-		user.Filters.RecoveryCodes = recoveryCodes
+		if user.CountUnusedRecoveryCodes() < 5 && user.Filters.TOTPConfig.Enabled {
+			user.Filters.RecoveryCodes = recoveryCodes
+		}
+	} else {
+		user.Filters.RecoveryCodes = nil
 	}
 	return dataprovider.UpdateUser(&user, dataprovider.ActionExecutorSelf, util.GetIPFromRemoteAddress(r.RemoteAddr))
 }
@@ -260,8 +285,12 @@ func saveAdminTOTPConfig(username string, r *http.Request, recoveryCodes []datap
 	if err != nil {
 		return util.NewValidationError(fmt.Sprintf("unable to decode JSON body: %v", err))
 	}
-	if admin.CountUnusedRecoveryCodes() < 5 && admin.Filters.TOTPConfig.Enabled {
+	if admin.Filters.TOTPConfig.Enabled {
-		admin.Filters.RecoveryCodes = recoveryCodes
+		if admin.CountUnusedRecoveryCodes() < 5 && admin.Filters.TOTPConfig.Enabled {
+			admin.Filters.RecoveryCodes = recoveryCodes
+		}
+	} else {
+		admin.Filters.RecoveryCodes = nil
 	}
 	if admin.Filters.TOTPConfig.Secret == nil || !admin.Filters.TOTPConfig.Secret.IsPlain() {
 		admin.Filters.TOTPConfig.Secret = currentTOTPSecret

internal/httpd/httpd_test.go

@@ -2307,25 +2307,17 @@ func TestPermMFADisabled(t *testing.T) {
 	rr = executeRequest(req)
 	checkResponseCode(t, http.StatusOK, rr)
 
-	user.Filters.RecoveryCodes = []dataprovider.RecoveryCode{
-		{
-			Secret: kms.NewPlainSecret(util.GenerateUniqueID()),
-		},
-	}
-	user, resp, err = httpdtest.UpdateUser(user, http.StatusOK, "")
-	assert.NoError(t, err, string(resp))
-	assert.Contains(t, user.Filters.WebClient, sdk.WebClientMFADisabled)
-	assert.Len(t, user.Filters.RecoveryCodes, 12)
-
 	req, err = http.NewRequest(http.MethodGet, user2FARecoveryCodesPath, nil)
 	assert.NoError(t, err)
 	setBearerForReq(req, token)
 	rr = executeRequest(req)
-	checkResponseCode(t, http.StatusOK, rr)
+	checkResponseCode(t, http.StatusForbidden, rr)
-	var recCodes []recoveryCode
+
-	err = json.Unmarshal(rr.Body.Bytes(), &recCodes)
+	req, err = http.NewRequest(http.MethodPost, user2FARecoveryCodesPath, nil)
 	assert.NoError(t, err)
-	assert.Len(t, recCodes, 12)
+	setBearerForReq(req, token)
+	rr = executeRequest(req)
+	checkResponseCode(t, http.StatusForbidden, rr)
 
 	_, err = httpdtest.RemoveUser(user, http.StatusOK)
 	assert.NoError(t, err)
@@ -2538,6 +2530,11 @@ func TestLoginAdminAPITOTP(t *testing.T) {
 	rr := executeRequest(req)
 	checkResponseCode(t, http.StatusOK, rr)
 
+	admin, _, err = httpdtest.GetAdminByUsername(admin.Username, http.StatusOK)
+	assert.NoError(t, err)
+	assert.True(t, admin.Filters.TOTPConfig.Enabled)
+	assert.Len(t, admin.Filters.RecoveryCodes, 12)
+
 	req, err = http.NewRequest(http.MethodGet, fmt.Sprintf("%v%v", httpBaseURL, tokenPath), nil)
 	assert.NoError(t, err)
 	req.SetBasicAuth(altAdminUsername, altAdminPassword)
@@ -2582,6 +2579,46 @@ func TestLoginAdminAPITOTP(t *testing.T) {
 	assert.Equal(t, http.StatusOK, resp.StatusCode)
 	err = resp.Body.Close()
 	assert.NoError(t, err)
+	// get/set recovery codes
+	req, err = http.NewRequest(http.MethodGet, admin2FARecoveryCodesPath, nil)
+	assert.NoError(t, err)
+	setBearerForReq(req, altToken)
+	rr = executeRequest(req)
+	checkResponseCode(t, http.StatusOK, rr)
+
+	req, err = http.NewRequest(http.MethodPost, admin2FARecoveryCodesPath, nil)
+	assert.NoError(t, err)
+	setBearerForReq(req, altToken)
+	rr = executeRequest(req)
+	checkResponseCode(t, http.StatusOK, rr)
+
+	// disable two-factor auth
+	saveReq := make(map[string]bool)
+	saveReq["enabled"] = false
+	asJSON, err = json.Marshal(saveReq)
+	assert.NoError(t, err)
+	req, err = http.NewRequest(http.MethodPost, adminTOTPSavePath, bytes.NewBuffer(asJSON))
+	assert.NoError(t, err)
+	setBearerForReq(req, altToken)
+	rr = executeRequest(req)
+	checkResponseCode(t, http.StatusOK, rr)
+
+	admin, _, err = httpdtest.GetAdminByUsername(admin.Username, http.StatusOK)
+	assert.NoError(t, err)
+	assert.False(t, admin.Filters.TOTPConfig.Enabled)
+	assert.Len(t, admin.Filters.RecoveryCodes, 0)
+	// get/set recovery codes will not work
+	req, err = http.NewRequest(http.MethodGet, admin2FARecoveryCodesPath, nil)
+	assert.NoError(t, err)
+	setBearerForReq(req, altToken)
+	rr = executeRequest(req)
+	checkResponseCode(t, http.StatusForbidden, rr)
+
+	req, err = http.NewRequest(http.MethodPost, admin2FARecoveryCodesPath, nil)
+	assert.NoError(t, err)
+	setBearerForReq(req, altToken)
+	rr = executeRequest(req)
+	checkResponseCode(t, http.StatusForbidden, rr)
 
 	_, err = httpdtest.RemoveAdmin(admin, http.StatusOK)
 	assert.NoError(t, err)
@@ -5225,6 +5262,62 @@ func TestCloseConnectionAfterUserUpdateDelete(t *testing.T) {
 	assert.Len(t, common.Connections.GetStats(), 0)
 }
 
+func TestAdminGenerateRecoveryCodesSaveError(t *testing.T) {
+	err := dataprovider.Close()
+	assert.NoError(t, err)
+	err = config.LoadConfig(configDir, "")
+	assert.NoError(t, err)
+	providerConf := config.GetProviderConf()
+	providerConf.NamingRules = 7
+	err = dataprovider.Initialize(providerConf, configDir, true)
+	assert.NoError(t, err)
+
+	a := getTestAdmin()
+	a.Username = "adMiN@example.com "
+	admin, _, err := httpdtest.AddAdmin(a, http.StatusCreated)
+	assert.NoError(t, err)
+	configName, _, secret, _, err := mfa.GenerateTOTPSecret(mfa.GetAvailableTOTPConfigNames()[0], admin.Username)
+	assert.NoError(t, err)
+	admin.Filters.TOTPConfig = dataprovider.AdminTOTPConfig{
+		Enabled:    true,
+		ConfigName: configName,
+		Secret:     kms.NewPlainSecret(secret),
+	}
+	admin.Password = defaultTokenAuthPass
+	err = dataprovider.UpdateAdmin(&admin, "", "")
+	assert.NoError(t, err)
+	admin, _, err = httpdtest.GetAdminByUsername(a.Username, http.StatusOK)
+	assert.NoError(t, err)
+	assert.True(t, admin.Filters.TOTPConfig.Enabled)
+
+	passcode, err := generateTOTPPasscode(secret)
+	assert.NoError(t, err)
+	adminAPIToken, err := getJWTAPITokenFromTestServerWithPasscode(a.Username, defaultTokenAuthPass, passcode)
+	assert.NoError(t, err)
+	assert.NotEmpty(t, adminAPIToken)
+
+	err = dataprovider.Close()
+	assert.NoError(t, err)
+	err = config.LoadConfig(configDir, "")
+	assert.NoError(t, err)
+	providerConf = config.GetProviderConf()
+	providerConf.BackupsPath = backupsPath
+	err = dataprovider.Initialize(providerConf, configDir, true)
+	assert.NoError(t, err)
+	if config.GetProviderConf().Driver == dataprovider.MemoryDataProviderName {
+		return
+	}
+	req, err := http.NewRequest(http.MethodPost, admin2FARecoveryCodesPath, nil)
+	assert.NoError(t, err)
+	setBearerForReq(req, adminAPIToken)
+	rr := executeRequest(req)
+	checkResponseCode(t, http.StatusBadRequest, rr)
+	assert.Contains(t, rr.Body.String(), "the following characters are allowed")
+
+	_, err = httpdtest.RemoveAdmin(admin, http.StatusOK)
+	assert.NoError(t, err)
+}
+
 func TestNamingRules(t *testing.T) {
 	smtpCfg := smtp.Config{
 		Host:          "127.0.0.1",
@@ -5248,12 +5341,24 @@ func TestNamingRules(t *testing.T) {
 	user, _, err := httpdtest.AddUser(u, http.StatusCreated)
 	assert.NoError(t, err)
 	assert.Equal(t, "user@user.me", user.Username)
+	configName, _, secret, _, err := mfa.GenerateTOTPSecret(mfa.GetAvailableTOTPConfigNames()[0], user.Username)
+	assert.NoError(t, err)
+	user.Filters.TOTPConfig = dataprovider.UserTOTPConfig{
+		Enabled:    true,
+		ConfigName: configName,
+		Secret:     kms.NewPlainSecret(secret),
+		Protocols:  []string{common.ProtocolSSH},
+	}
+	user.Password = u.Password
+	err = dataprovider.UpdateUser(&user, "", "")
+	assert.NoError(t, err)
 	user.Username = u.Username
 	user.AdditionalInfo = "info"
 	user, _, err = httpdtest.UpdateUser(user, http.StatusOK, "")
 	assert.NoError(t, err)
 	user, _, err = httpdtest.GetUserByUsername(u.Username, http.StatusOK)
 	assert.NoError(t, err)
+	assert.True(t, user.Filters.TOTPConfig.Enabled)
 
 	a := getTestAdmin()
 	a.Username = "admiN@example.com "
@@ -5403,12 +5508,6 @@ func TestNamingRules(t *testing.T) {
 	checkResponseCode(t, http.StatusBadRequest, rr)
 	assert.Contains(t, rr.Body.String(), "the following characters are allowed")
 
-	req, err = http.NewRequest(http.MethodPost, admin2FARecoveryCodesPath, nil)
-	assert.NoError(t, err)
-	setBearerForReq(req, adminAPIToken)
-	rr = executeRequest(req)
-	checkResponseCode(t, http.StatusBadRequest, rr)
-	assert.Contains(t, rr.Body.String(), "the following characters are allowed")
 	// test admin reset password
 	form = make(url.Values)
 	form.Set("username", admin.Username)
@@ -7610,23 +7709,24 @@ func TestAdminTOTP(t *testing.T) {
 	assert.NoError(t, err, string(resp))
 	assert.True(t, admin.Filters.TOTPConfig.Enabled)
 	assert.Len(t, admin.Filters.RecoveryCodes, 12)
-	// if we use token we should get no recovery codes
+	// if we use token we cannot get or generate recovery codes
 	req, err = http.NewRequest(http.MethodGet, admin2FARecoveryCodesPath, nil)
 	assert.NoError(t, err)
 	setBearerForReq(req, token)
 	rr = executeRequest(req)
-	checkResponseCode(t, http.StatusOK, rr)
+	checkResponseCode(t, http.StatusForbidden, rr)
-	var recCodes []recoveryCode
+	req, err = http.NewRequest(http.MethodPost, admin2FARecoveryCodesPath, nil)
-	err = json.Unmarshal(rr.Body.Bytes(), &recCodes)
 	assert.NoError(t, err)
-	assert.Len(t, recCodes, 0)
+	setBearerForReq(req, token)
+	rr = executeRequest(req)
+	checkResponseCode(t, http.StatusForbidden, rr)
 	// now the same but with altToken
 	req, err = http.NewRequest(http.MethodGet, admin2FARecoveryCodesPath, nil)
 	assert.NoError(t, err)
 	setBearerForReq(req, altToken)
 	rr = executeRequest(req)
 	checkResponseCode(t, http.StatusOK, rr)
-	recCodes = nil
+	var recCodes []recoveryCode
 	err = json.Unmarshal(rr.Body.Bytes(), &recCodes)
 	assert.NoError(t, err)
 	assert.Len(t, recCodes, 12)
@@ -20947,8 +21047,15 @@ func setJWTCookieForReq(req *http.Request, jwtToken string) {
 }
 
 func getJWTAPITokenFromTestServer(username, password string) (string, error) {
+	return getJWTAPITokenFromTestServerWithPasscode(username, password, "")
+}
+
+func getJWTAPITokenFromTestServerWithPasscode(username, password, passcode string) (string, error) {
 	req, _ := http.NewRequest(http.MethodGet, tokenPath, nil)
 	req.SetBasicAuth(username, password)
+	if passcode != "" {
+		req.Header.Set("X-SFTPGO-OTP", passcode)
+	}
 	rr := executeRequest(req)
 	if rr.Code != http.StatusOK {
 		return "", fmt.Errorf("unexpected  status code %v", rr.Code)

templates/webadmin/mfa.html

@@ -81,6 +81,7 @@ along with this program.  If not, see <https://www.gnu.org/licenses/>.
     </div>
 </div>
 
+{{if .TOTPConfig.Enabled }}
 <div class="card shadow mb-4">
     <div class="card-header py-3">
         <h6 class="m-0 font-weight-bold text-primary">Recovery codes</h6>
@@ -117,6 +118,7 @@ along with this program.  If not, see <https://www.gnu.org/licenses/>.
     </div>
 </div>
 {{end}}
+{{end}}
 
 {{define "dialog"}}
 <div class="modal fade" id="disableTOTPModal" tabindex="-1" role="dialog" aria-labelledby="disableTOTPModalLabel"
@@ -344,7 +346,7 @@ along with this program.  If not, see <https://www.gnu.org/licenses/>.
                 $('#errorRecCodesMsg').show();
                 setTimeout(function () {
                     $('#errorRecCodesMsg').hide();
-                }, 5000);
+                }, 8000);
             }
         });
     }
@@ -388,7 +390,7 @@ along with this program.  If not, see <https://www.gnu.org/licenses/>.
                 $('#errorRecCodesMsg').show();
                 setTimeout(function () {
                     $('#errorRecCodesMsg').hide();
-                }, 5000);
+                }, 8000);
             }
         });
     }

templates/webclient/mfa.html

@@ -106,6 +106,7 @@ along with this program.  If not, see <https://www.gnu.org/licenses/>.
     </div>
 </div>
 
+{{if .TOTPConfig.Enabled }}
 <div class="card shadow mb-4">
     <div class="card-header py-3">
         <h6 class="m-0 font-weight-bold text-primary">Recovery codes</h6>
@@ -142,6 +143,7 @@ along with this program.  If not, see <https://www.gnu.org/licenses/>.
     </div>
 </div>
 {{end}}
+{{end}}
 
 {{define "dialog"}}
 <div class="modal fade" id="disableTOTPModal" tabindex="-1" role="dialog" aria-labelledby="disableTOTPModalLabel"
@@ -416,7 +418,7 @@ along with this program.  If not, see <https://www.gnu.org/licenses/>.
                 $('#errorRecCodesMsg').show();
                 setTimeout(function () {
                     $('#errorRecCodesMsg').hide();
-                }, 5000);
+                }, 8000);
             }
         });
     }
@@ -460,7 +462,7 @@ along with this program.  If not, see <https://www.gnu.org/licenses/>.
                 $('#errorRecCodesMsg').show();
                 setTimeout(function () {
                     $('#errorRecCodesMsg').hide();
-                }, 5000);
+                }, 8000);
             }
         });
     }