Skip to content

Commit

Permalink
MF-1317 - Configurable regexp rule for password (#1355)
Browse files Browse the repository at this point in the history
* read and validate regex envar

Signed-off-by: Ivan Milosevic <iva@blokovi.com>

* pass regexp to user/api

Signed-off-by: Ivan Milosevic <iva@blokovi.com>

* resolve conflicts

Signed-off-by: Ivan Milosevic <iva@blokovi.com>

* use exported regexp variable

Signed-off-by: Ivan Milosevic <iva@blokovi.com>

* move password validation from users package

Signed-off-by: Ivan Milosevic <iva@blokovi.com>

* remove dead code

Signed-off-by: Ivan Milosevic <iva@blokovi.com>

* add password change request

Signed-off-by: Ivan Milosevic <iva@blokovi.com>

* move regexp from api to users package

Signed-off-by: Ivan Milosevic <iva@blokovi.com>

* fix tests

Signed-off-by: Ivan Milosevic <iva@blokovi.com>

* remove commented code

Signed-off-by: Ivan Milosevic <iva@blokovi.com>

* add regexp as field in userService, remove it as user exported global var

Signed-off-by: Ivan Milosevic <iva@blokovi.com>

* add passwd validation in service

Signed-off-by: Ivan Milosevic <iva@blokovi.com>

* Add psswd validation for change password in service

Signed-off-by: Ivan Milosevic <iva@blokovi.com>

* add password validation in password reset

Signed-off-by: Ivan Milosevic <iva@blokovi.com>

* Remove password validation from user validation test

Signed-off-by: Ivan Milosevic <iva@blokovi.com>

* Replace email and passwords in test with constants

Signed-off-by: Ivan Milosevic <iva@blokovi.com>

* compile error not fail silently

Signed-off-by: Ivan Milosevic <iva@blokovi.com>

* fix tempate path

Signed-off-by: Ivan Milosevic <iva@blokovi.com>
  • Loading branch information
blokovi committed Mar 1, 2021
1 parent e334569 commit 7bcaa32
Show file tree
Hide file tree
Showing 11 changed files with 69 additions and 35 deletions.
1 change: 1 addition & 0 deletions .env
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 11 additions & 1 deletion cmd/users/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"net/http"
"os"
"os/signal"
"regexp"
"strconv"
"syscall"
"time"
Expand Down Expand Up @@ -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
Expand All @@ -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"
Expand Down Expand Up @@ -123,6 +126,7 @@ type config struct {
authTimeout time.Duration
adminEmail string
adminPassword string
passRegex *regexp.Regexp
}

func main() {
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -213,6 +222,7 @@ func loadConfig() config {
authTimeout: authTimeout,
adminEmail: mainflux.Env(envAdminEmail, defAdminEmail),
adminPassword: mainflux.Env(envAdminPassword, defAdminPassword),
passRegex: passRegex,
}

}
Expand Down Expand Up @@ -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,
Expand Down
7 changes: 6 additions & 1 deletion pkg/sdk/go/users_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"net/http"
"net/http/httptest"
"regexp"
"testing"

"github.com/mainflux/mainflux"
Expand All @@ -24,6 +25,10 @@ const (
invalidEmail = "userexample.com"
)

var (
passRegex = regexp.MustCompile("^.{8,}$")
)

func newUserService() users.Service {
usersRepo := mocks.NewUserRepository()
groupsRepo := mocks.NewGroupRepository()
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion scripts/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
35 changes: 26 additions & 9 deletions users/api/endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"io/ioutil"
"net/http"
"net/http/httptest"
"regexp"
"strings"
"testing"

Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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},
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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},
Expand Down
6 changes: 1 addition & 5 deletions users/api/requests.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
)

const (
minPassLen = 8
maxNameSize = 1024
)

Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 2 additions & 0 deletions users/api/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
16 changes: 15 additions & 1 deletion users/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -164,24 +167,29 @@ 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,
hasher: hasher,
auth: auth,
email: m,
idProvider: idp,
passRegex: passRegex,
}
}

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)
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down
10 changes: 6 additions & 4 deletions users/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package users_test
import (
"context"
"fmt"
"regexp"
"testing"

"github.com/mainflux/mainflux"
Expand All @@ -27,6 +28,7 @@ var (
groupName = "Mainflux"

idProvider = uuid.New()
passRegex = regexp.MustCompile("^.{8,}$")
)

func newService() users.Service {
Expand All @@ -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) {
Expand All @@ -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,
},
}

Expand Down
6 changes: 0 additions & 6 deletions users/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
)

const (
minPassLen = 8
maxLocalLen = 64
maxDomainLen = 255
maxTLDLen = 24 // longest TLD currently in existence
Expand Down Expand Up @@ -46,11 +45,6 @@ func (u User) Validate() error {
if !isEmail(u.Email) {
return ErrMalformedEntity
}

if len(u.Password) < minPassLen {
return ErrMalformedEntity
}

return nil
}

Expand Down
Loading

0 comments on commit 7bcaa32

Please sign in to comment.