From 3267a50ae3c05d0fe137dd43937e9317e8b7a546 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Wed, 31 Aug 2022 09:29:39 +0200 Subject: [PATCH] MFA: allow recovery codes only if two-factor auth is enabled Fixes #965 Signed-off-by: Nicola Murino --- go.mod | 20 ++--- go.sum | 36 ++++---- internal/common/common.go | 6 +- internal/httpd/api_mfa.go | 37 +++++++- internal/httpd/httpd_test.go | 159 +++++++++++++++++++++++++++++------ templates/webadmin/mfa.html | 6 +- templates/webclient/mfa.html | 6 +- 7 files changed, 205 insertions(+), 65 deletions(-) diff --git a/go.mod b/go.mod index 2716fe39..8f1ae6d6 100644 --- a/go.mod +++ b/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/credentials v1.12.15 + github.com/aws/aws-sdk-go-v2/config v1.17.3 + 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/service/marketplacemetering v1.13.14 - github.com/aws/aws-sdk-go-v2/service/s3 v1.27.6 - github.com/aws/aws-sdk-go-v2/service/secretsmanager v1.15.19 - github.com/aws/aws-sdk-go-v2/service/sts v1.16.14 + github.com/aws/aws-sdk-go-v2/feature/s3/manager v1.11.29 + github.com/aws/aws-sdk-go-v2/service/marketplacemetering v1.13.15 + github.com/aws/aws-sdk-go-v2/service/s3 v1.27.7 + github.com/aws/aws-sdk-go-v2/service/secretsmanager v1.15.20 + 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 ) diff --git a/go.sum b/go.sum index 8bbd969f..499305f6 100644 --- a/go.sum +++ b/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.2/go.mod h1:jumS/AMwul4WaG8vyXsF6kUndG9zndR+yfYBwl4i9ds= +github.com/aws/aws-sdk-go-v2/config v1.17.3 h1:s1As/fiVMmM3CObC4GcSaSbkhm88S6a5qn8St3wgal0= +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.15/go.mod h1:41zTC6U/78fUD7ZCa5NymTJANDjfqySg5YEAYVFl2Ic= +github.com/aws/aws-sdk-go-v2/credentials v1.12.16 h1:HXczS88Pg36j8dq0KSjtHBPFs8gdRyBSS1hueeG/rxA= +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.28/go.mod h1:hV8r7xrO3IghGtC87aX0JiJsN2zJhuzFExLSmgZ+7ek= +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.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.14/go.mod h1:Zf+Tf40dskiGdwVJU2HIgln1vtnQF8QpsguBsbI5Uq8= +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.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.6/go.mod h1:orjy5IRgBQnh9EI/lMW7YGF6eYk6re8HPFbL66a2DSo= +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.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.19/go.mod h1:F2AUfGEOcxpOTzo/+Bur5PrtsvnhVQQbd4CGfPicOpw= +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.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.18/go.mod h1:ytmEi5+qwcSNcV2pVA8PIb1DnKT/0Bu/K4nfJHwoM6c= +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.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.14/go.mod h1:Y+BUV19q3OmQVqNUlbZ40zVi3NM6Biuxwkx/qdSD/CY= +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.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-20220828084757-61f5262cc94f/go.mod h1:SiM6ypd8Xu1xldObYtbDztuUU7xUzMnUULfphXFZmro= +github.com/drakkan/crypto v0.0.0-20220831070132-e3c36f2ab82b h1:kCNBtUFKfhiUaE1ZMgh83pXFVP2ZIHktwV15lmgD0Ok= +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= diff --git a/internal/common/common.go b/internal/common/common.go index a23423a1..d71128a0 100644 --- a/internal/common/common.go +++ b/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 diff --git a/internal/httpd/api_mfa.go b/internal/httpd/api_mfa.go index bbab7418..cf11e990 100644 --- a/internal/httpd/api_mfa.go +++ b/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 { - user.Filters.RecoveryCodes = recoveryCodes + if user.Filters.TOTPConfig.Enabled { + 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 { - admin.Filters.RecoveryCodes = recoveryCodes + if admin.Filters.TOTPConfig.Enabled { + 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 diff --git a/internal/httpd/httpd_test.go b/internal/httpd/httpd_test.go index 79818e3d..d354d465 100644 --- a/internal/httpd/httpd_test.go +++ b/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) - var recCodes []recoveryCode - err = json.Unmarshal(rr.Body.Bytes(), &recCodes) + checkResponseCode(t, http.StatusForbidden, rr) + + 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) - var recCodes []recoveryCode - err = json.Unmarshal(rr.Body.Bytes(), &recCodes) + checkResponseCode(t, http.StatusForbidden, rr) + req, err = http.NewRequest(http.MethodPost, admin2FARecoveryCodesPath, nil) 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) diff --git a/templates/webadmin/mfa.html b/templates/webadmin/mfa.html index c72404f9..36dea6b9 100644 --- a/templates/webadmin/mfa.html +++ b/templates/webadmin/mfa.html @@ -81,6 +81,7 @@ along with this program. If not, see . +{{if .TOTPConfig.Enabled }}
Recovery codes
@@ -117,6 +118,7 @@ along with this program. If not, see .
{{end}} +{{end}} {{define "dialog"}} +{{if .TOTPConfig.Enabled }}
Recovery codes
@@ -142,6 +143,7 @@ along with this program. If not, see .
{{end}} +{{end}} {{define "dialog"}}