From 7bcaa323d463afcc8154ccee550b27036827cff0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20Milo=C5=A1evi=C4=87?= Date: Mon, 1 Mar 2021 15:22:57 +0100 Subject: [PATCH] MF-1317 - Configurable regexp rule for password (#1355) * read and validate regex envar Signed-off-by: Ivan Milosevic * pass regexp to user/api Signed-off-by: Ivan Milosevic * resolve conflicts Signed-off-by: Ivan Milosevic * use exported regexp variable Signed-off-by: Ivan Milosevic * move password validation from users package Signed-off-by: Ivan Milosevic * remove dead code Signed-off-by: Ivan Milosevic * add password change request Signed-off-by: Ivan Milosevic * move regexp from api to users package Signed-off-by: Ivan Milosevic * fix tests Signed-off-by: Ivan Milosevic * remove commented code Signed-off-by: Ivan Milosevic * add regexp as field in userService, remove it as user exported global var Signed-off-by: Ivan Milosevic * add passwd validation in service Signed-off-by: Ivan Milosevic * Add psswd validation for change password in service Signed-off-by: Ivan Milosevic * add password validation in password reset Signed-off-by: Ivan Milosevic * Remove password validation from user validation test Signed-off-by: Ivan Milosevic * Replace email and passwords in test with constants Signed-off-by: Ivan Milosevic * compile error not fail silently Signed-off-by: Ivan Milosevic * fix tempate path Signed-off-by: Ivan Milosevic --- .env | 1 + cmd/users/main.go | 12 +++++++++++- pkg/sdk/go/users_test.go | 7 ++++++- scripts/run.sh | 2 +- users/api/endpoint_test.go | 35 ++++++++++++++++++++++++++--------- users/api/requests.go | 6 +----- users/api/transport.go | 2 ++ users/service.go | 16 +++++++++++++++- users/service_test.go | 10 ++++++---- users/users.go | 6 ------ users/users_test.go | 7 ------- 11 files changed, 69 insertions(+), 35 deletions(-) diff --git a/.env b/.env index 8162f629..785ddba8 100644 --- a/.env +++ b/.env @@ -46,6 +46,7 @@ MF_USERS_DB=users MF_USERS_ADMIN_EMAIL=admin@example.com MF_USERS_ADMIN_PASSWORD=12345678 MF_USERS_RESET_PWD_TEMPLATE=users.tmpl +MF_USERS_PASS_REGEX=^.{8,}$ ### Email utility MF_EMAIL_HOST=smtp.mailtrap.io diff --git a/cmd/users/main.go b/cmd/users/main.go index cbe8a33c..ddb48e93 100644 --- a/cmd/users/main.go +++ b/cmd/users/main.go @@ -12,6 +12,7 @@ import ( "net/http" "os" "os/signal" + "regexp" "strconv" "syscall" "time" @@ -63,6 +64,7 @@ const ( defEmailTemplate = "email.tmpl" defAdminEmail = "" defAdminPassword = "" + defPassRegex = "^.{8,}$" defAdminGroup = "mainflux" defTokenResetEndpoint = "/reset-request" // URL where user lands after click on the reset link from email @@ -89,6 +91,7 @@ const ( envAdminEmail = "MF_USERS_ADMIN_EMAIL" envAdminPassword = "MF_USERS_ADMIN_PASSWORD" + envPassRegex = "MF_USERS_PASS_REGEX" envEmailHost = "MF_EMAIL_HOST" envEmailPort = "MF_EMAIL_PORT" @@ -123,6 +126,7 @@ type config struct { authTimeout time.Duration adminEmail string adminPassword string + passRegex *regexp.Regexp } func main() { @@ -175,6 +179,11 @@ func loadConfig() config { log.Fatalf("Invalid value passed for %s\n", envAuthTLS) } + passRegex, err := regexp.Compile(mainflux.Env(envPassRegex, defPassRegex)) + if err != nil { + log.Fatalf("Invalid password validation rules %s\n", envPassRegex) + } + dbConfig := postgres.Config{ Host: mainflux.Env(envDBHost, defDBHost), Port: mainflux.Env(envDBPort, defDBPort), @@ -213,6 +222,7 @@ func loadConfig() config { authTimeout: authTimeout, adminEmail: mainflux.Env(envAdminEmail, defAdminEmail), adminPassword: mainflux.Env(envAdminPassword, defAdminPassword), + passRegex: passRegex, } } @@ -287,7 +297,7 @@ func newService(db *sqlx.DB, tracer opentracing.Tracer, auth mainflux.AuthServic idProvider := uuid.New() - svc := users.New(userRepo, groupRepo, hasher, auth, emailer, idProvider) + svc := users.New(userRepo, groupRepo, hasher, auth, emailer, idProvider, c.passRegex) svc = api.LoggingMiddleware(svc, logger) svc = api.MetricsMiddleware( svc, diff --git a/pkg/sdk/go/users_test.go b/pkg/sdk/go/users_test.go index 377dce95..c99a7498 100644 --- a/pkg/sdk/go/users_test.go +++ b/pkg/sdk/go/users_test.go @@ -8,6 +8,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "regexp" "testing" "github.com/mainflux/mainflux" @@ -24,6 +25,10 @@ const ( invalidEmail = "userexample.com" ) +var ( + passRegex = regexp.MustCompile("^.{8,}$") +) + func newUserService() users.Service { usersRepo := mocks.NewUserRepository() groupsRepo := mocks.NewGroupRepository() @@ -32,7 +37,7 @@ func newUserService() users.Service { emailer := mocks.NewEmailer() idProvider := uuid.New() - return users.New(usersRepo, groupsRepo, hasher, auth, emailer, idProvider) + return users.New(usersRepo, groupsRepo, hasher, auth, emailer, idProvider, passRegex) } func newUserServer(svc users.Service) *httptest.Server { diff --git a/scripts/run.sh b/scripts/run.sh index 9ffc7799..666920cd 100755 --- a/scripts/run.sh +++ b/scripts/run.sh @@ -38,7 +38,7 @@ done ### # Users ### -MF_USERS_LOG_LEVEL=info MF_EMAIL_TEMPLATE=../docker/users/emailer/templates/email.tmpl $BUILD_DIR/mainflux-users & +MF_USERS_LOG_LEVEL=info MF_EMAIL_TEMPLATE=../docker/templates/users.tmpl $BUILD_DIR/mainflux-users & ### # Things diff --git a/users/api/endpoint_test.go b/users/api/endpoint_test.go index 70238cfc..7c60c94a 100644 --- a/users/api/endpoint_test.go +++ b/users/api/endpoint_test.go @@ -11,6 +11,7 @@ import ( "io/ioutil" "net/http" "net/http/httptest" + "regexp" "strings" "testing" @@ -27,17 +28,22 @@ import ( const ( contentType = "application/json" + validEmail = "user@example.com" invalidEmail = "userexample.com" + validPass = "password" + invalidPass = "wrong" ) var ( - user = users.User{Email: "user@example.com", Password: "password"} + user = users.User{Email: validEmail, Password: validPass} notFoundRes = toJSON(errorRes{users.ErrUserNotFound.Error()}) unauthRes = toJSON(errorRes{users.ErrUnauthorizedAccess.Error()}) malformedRes = toJSON(errorRes{users.ErrMalformedEntity.Error()}) + weakPassword = toJSON(errorRes{users.ErrPasswordFormat.Error()}) unsupportedRes = toJSON(errorRes{api.ErrUnsupportedContentType.Error()}) failDecodeRes = toJSON(errorRes{api.ErrFailedDecode.Error()}) groupExists = toJSON(errorRes{users.ErrGroupConflict.Error()}) + passRegex = regexp.MustCompile("^.{8,}$") ) type testRequest struct { @@ -73,7 +79,7 @@ func newService() users.Service { email := mocks.NewEmailer() idProvider := uuid.New() - return users.New(usersRepo, groupRepo, hasher, auth, email, idProvider) + return users.New(usersRepo, groupRepo, hasher, auth, email, idProvider, passRegex) } func newServer(svc users.Service) *httptest.Server { @@ -93,7 +99,8 @@ func TestRegister(t *testing.T) { client := ts.Client() data := toJSON(user) - invalidData := toJSON(users.User{Email: invalidEmail, Password: "password"}) + invalidData := toJSON(users.User{Email: invalidEmail, Password: validPass}) + invalidPasswordData := toJSON(users.User{Email: validEmail, Password: invalidPass}) invalidFieldData := fmt.Sprintf(`{"email": "%s", "pass": "%s"}`, user.Email, user.Password) cases := []struct { @@ -105,6 +112,7 @@ func TestRegister(t *testing.T) { {"register new user", data, contentType, http.StatusCreated}, {"register existing user", data, contentType, http.StatusConflict}, {"register user with invalid email address", invalidData, contentType, http.StatusBadRequest}, + {"register user with weak password", invalidPasswordData, contentType, http.StatusBadRequest}, {"register user with invalid request format", "{", contentType, http.StatusBadRequest}, {"register user with empty JSON request", "{}", contentType, http.StatusBadRequest}, {"register user with empty request", "", contentType, http.StatusBadRequest}, @@ -138,15 +146,15 @@ func TestLogin(t *testing.T) { data := toJSON(user) invalidEmailData := toJSON(users.User{ Email: invalidEmail, - Password: "password", + Password: validPass, }) invalidData := toJSON(users.User{ - Email: "user@example.com", + Email: validEmail, Password: "invalid_password", }) nonexistentData := toJSON(users.User{ Email: "non-existentuser@example.com", - Password: "password", + Password: validPass, }) _, err := svc.Register(context.Background(), user) require.Nil(t, err, fmt.Sprintf("register user got unexpected error: %s", err)) @@ -235,7 +243,7 @@ func TestPasswordResetRequest(t *testing.T) { nonexistentData := toJSON(users.User{ Email: "non-existentuser@example.com", - Password: "password", + Password: validPass, }) expectedExisting := toJSON(struct { @@ -322,9 +330,12 @@ func TestPasswordReset(t *testing.T) { reqData.Token = token - reqData.ConfPass = "wrong" + reqData.ConfPass = invalidPass reqPassNoMatch := toJSON(reqData) + reqData.Password = invalidPass + reqPassWeak := toJSON(reqData) + cases := []struct { desc string req string @@ -340,6 +351,7 @@ func TestPasswordReset(t *testing.T) { {"password reset request with empty JSON request", "{}", contentType, http.StatusBadRequest, malformedRes, token}, {"password reset request with empty request", "", contentType, http.StatusBadRequest, failDecodeRes, token}, {"password reset request with missing content type", reqExisting, "", http.StatusUnsupportedMediaType, unsupportedRes, token}, + {"password reset with weak password", reqPassWeak, contentType, http.StatusBadRequest, weakPassword, token}, } for _, tc := range cases { @@ -395,9 +407,13 @@ func TestPasswordChange(t *testing.T) { reqNoExist := toJSON(reqData) - reqData.OldPassw = "wrong" + reqData.OldPassw = invalidPass reqWrongPass := toJSON(reqData) + reqData.OldPassw = user.Password + reqData.Password = invalidPass + reqWeakPass := toJSON(reqData) + resData.Msg = users.ErrUnauthorizedAccess.Error() cases := []struct { @@ -411,6 +427,7 @@ func TestPasswordChange(t *testing.T) { {"password change with valid token", dataResExisting, contentType, http.StatusCreated, expectedSuccess, token}, {"password change with invalid token", reqNoExist, contentType, http.StatusForbidden, unauthRes, ""}, {"password change with invalid old password", reqWrongPass, contentType, http.StatusForbidden, unauthRes, token}, + {"password change with invalid new password", reqWeakPass, contentType, http.StatusBadRequest, weakPassword, token}, {"password change with empty JSON request", "{}", contentType, http.StatusBadRequest, malformedRes, token}, {"password change empty request", "", contentType, http.StatusBadRequest, failDecodeRes, token}, {"password change missing content type", dataResExisting, "", http.StatusUnsupportedMediaType, unsupportedRes, token}, diff --git a/users/api/requests.go b/users/api/requests.go index 280972f1..59ca18c1 100644 --- a/users/api/requests.go +++ b/users/api/requests.go @@ -8,7 +8,6 @@ import ( ) const ( - minPassLen = 8 maxNameSize = 1024 ) @@ -100,11 +99,8 @@ func (req passwChangeReq) validate() error { if req.Token == "" { return users.ErrUnauthorizedAccess } - if len(req.Password) < minPassLen { - return users.ErrMalformedEntity - } if req.OldPassword == "" { - return users.ErrUnauthorizedAccess + return users.ErrMalformedEntity } return nil } diff --git a/users/api/transport.go b/users/api/transport.go index 086ee554..53acc6ed 100644 --- a/users/api/transport.go +++ b/users/api/transport.go @@ -432,6 +432,8 @@ func encodeError(_ context.Context, err error, w http.ResponseWriter) { w.WriteHeader(http.StatusBadRequest) case errors.Contains(errorVal, users.ErrRecoveryToken): w.WriteHeader(http.StatusNotFound) + case errors.Contains(errorVal, users.ErrPasswordFormat): + w.WriteHeader(http.StatusBadRequest) default: w.WriteHeader(http.StatusInternalServerError) } diff --git a/users/service.go b/users/service.go index 61c59b6d..dac7adfd 100644 --- a/users/service.go +++ b/users/service.go @@ -66,6 +66,9 @@ var ( // ErrAssignUserToGroup indicates an error in assigning user to a group. ErrAssignUserToGroup = errors.New("failed assigning user to a group") + + // ErrPasswordFormat indicates weak password. + ErrPasswordFormat = errors.New("password does not meet the requirements") ) // Service specifies an API that must be fullfiled by the domain service @@ -164,10 +167,11 @@ type usersService struct { email Emailer auth mainflux.AuthServiceClient idProvider mainflux.IDProvider + passRegex *regexp.Regexp } // New instantiates the users service implementation -func New(users UserRepository, groups GroupRepository, hasher Hasher, auth mainflux.AuthServiceClient, m Emailer, idp mainflux.IDProvider) Service { +func New(users UserRepository, groups GroupRepository, hasher Hasher, auth mainflux.AuthServiceClient, m Emailer, idp mainflux.IDProvider, passRegex *regexp.Regexp) Service { return &usersService{ users: users, groups: groups, @@ -175,6 +179,7 @@ func New(users UserRepository, groups GroupRepository, hasher Hasher, auth mainf auth: auth, email: m, idProvider: idp, + passRegex: passRegex, } } @@ -182,6 +187,9 @@ func (svc usersService) Register(ctx context.Context, user User) (string, error) if err := user.Validate(); err != nil { return "", err } + if !svc.passRegex.MatchString(user.Password) { + return "", ErrPasswordFormat + } hash, err := svc.hasher.Hash(user.Password) if err != nil { return "", errors.Wrap(ErrMalformedEntity, err) @@ -290,6 +298,9 @@ func (svc usersService) ResetPassword(ctx context.Context, resetToken, password if err != nil || u.Email == "" { return ErrUserNotFound } + if !svc.passRegex.MatchString(password) { + return ErrPasswordFormat + } password, err = svc.hasher.Hash(password) if err != nil { return err @@ -302,6 +313,9 @@ func (svc usersService) ChangePassword(ctx context.Context, authToken, password, if err != nil { return errors.Wrap(ErrUnauthorizedAccess, err) } + if !svc.passRegex.MatchString(password) { + return ErrPasswordFormat + } u := User{ Email: email, Password: oldPassword, diff --git a/users/service_test.go b/users/service_test.go index c316db3d..93b2197e 100644 --- a/users/service_test.go +++ b/users/service_test.go @@ -6,6 +6,7 @@ package users_test import ( "context" "fmt" + "regexp" "testing" "github.com/mainflux/mainflux" @@ -27,6 +28,7 @@ var ( groupName = "Mainflux" idProvider = uuid.New() + passRegex = regexp.MustCompile("^.{8,}$") ) func newService() users.Service { @@ -36,7 +38,7 @@ func newService() users.Service { auth := mocks.NewAuthService(map[string]string{user.Email: user.Email}) e := mocks.NewEmailer() - return users.New(userRepo, groupRepo, hasher, auth, e, idProvider) + return users.New(userRepo, groupRepo, hasher, auth, e, idProvider, passRegex) } func TestRegister(t *testing.T) { @@ -58,12 +60,12 @@ func TestRegister(t *testing.T) { err: users.ErrConflict, }, { - desc: "register new user with empty password", + desc: "register new user with weak password", user: users.User{ Email: user.Email, - Password: "", + Password: "weak", }, - err: users.ErrMalformedEntity, + err: users.ErrPasswordFormat, }, } diff --git a/users/users.go b/users/users.go index 7b90ace7..d7dedd94 100644 --- a/users/users.go +++ b/users/users.go @@ -13,7 +13,6 @@ import ( ) const ( - minPassLen = 8 maxLocalLen = 64 maxDomainLen = 255 maxTLDLen = 24 // longest TLD currently in existence @@ -46,11 +45,6 @@ func (u User) Validate() error { if !isEmail(u.Email) { return ErrMalformedEntity } - - if len(u.Password) < minPassLen { - return ErrMalformedEntity - } - return nil } diff --git a/users/users_test.go b/users/users_test.go index 5da4e3fc..b777f6aa 100644 --- a/users/users_test.go +++ b/users/users_test.go @@ -72,13 +72,6 @@ func TestValidate(t *testing.T) { }, err: users.ErrMalformedEntity, }, - "validate user with empty password": { - user: users.User{ - Email: email, - Password: "", - }, - err: users.ErrMalformedEntity, - }, "validate user with invalid email": { user: users.User{ Email: "userexample.com",