NOISSUE - Add Secret Validation on Registration (absmach#2109)

Signed-off-by: Rodney Osodo <[email protected]>

cmd/users/main.go

@@ -186,7 +186,7 @@ func main() {
 	oauthProvider := googleoauth.NewProvider(oauthConfig, cfg.OAuthUIRedirectURL, cfg.OAuthUIErrorURL)
 
 	mux := chi.NewRouter()
-	httpSrv := httpserver.New(ctx, cancel, svcName, httpServerConfig, capi.MakeHandler(csvc, gsvc, mux, logger, cfg.InstanceID, oauthProvider), logger)
+	httpSrv := httpserver.New(ctx, cancel, svcName, httpServerConfig, capi.MakeHandler(csvc, gsvc, mux, logger, cfg.InstanceID, cfg.PassRegex, oauthProvider), logger)
 
 	if cfg.SendTelemetry {
 		chc := chclient.New(svcName, magistrala.Version, logger, cancel)
@@ -219,7 +219,7 @@ func newService(ctx context.Context, authClient magistrala.AuthServiceClient, db
 		logger.Error(fmt.Sprintf("failed to configure e-mailing util: %s", err.Error()))
 	}
 
-	csvc := users.NewService(cRepo, authClient, emailerClient, hsr, idp, c.PassRegex, c.SelfRegister)
+	csvc := users.NewService(cRepo, authClient, emailerClient, hsr, idp, c.SelfRegister)
 	gsvc := mggroups.NewService(gRepo, idp, authClient)
 
 	csvc, err = uevents.NewEventStoreMiddleware(ctx, csvc, c.ESURL)

internal/api/common.go

@@ -106,8 +106,7 @@ func EncodeError(_ context.Context, err error, w http.ResponseWriter) {
 
 	w.Header().Set("Content-Type", ContentType)
 	switch {
-	case errors.Contains(err, apiutil.ErrInvalidSecret),
-		errors.Contains(err, svcerr.ErrMalformedEntity),
+	case errors.Contains(err, svcerr.ErrMalformedEntity),
 		errors.Contains(err, errors.ErrMalformedEntity),
 		errors.Contains(err, apiutil.ErrMissingID),
 		errors.Contains(err, apiutil.ErrEmptyList),
@@ -121,7 +120,12 @@ func EncodeError(_ context.Context, err error, w http.ResponseWriter) {
 		errors.Contains(err, apiutil.ErrInvalidQueryParams),
 		errors.Contains(err, apiutil.ErrInvalidStatus),
 		errors.Contains(err, apiutil.ErrMissingRelation),
-		errors.Contains(err, apiutil.ErrValidation):
+		errors.Contains(err, apiutil.ErrValidation),
+		errors.Contains(err, apiutil.ErrMissingIdentity),
+		errors.Contains(err, apiutil.ErrMissingSecret),
+		errors.Contains(err, apiutil.ErrMissingPass),
+		errors.Contains(err, apiutil.ErrMissingConfPass),
+		errors.Contains(err, apiutil.ErrPasswordFormat):
 		w.WriteHeader(http.StatusBadRequest)
 	case errors.Contains(err, svcerr.ErrAuthentication),
 		errors.Contains(err, apiutil.ErrBearerToken):

internal/api/common_test.go

@@ -225,7 +225,7 @@ func TestEncodeError(t *testing.T) {
 		{
 			desc: "BadRequest",
 			errs: []error{
-				apiutil.ErrInvalidSecret,
+				apiutil.ErrMissingSecret,
 				svcerr.ErrMalformedEntity,
 				errors.ErrMalformedEntity,
 				apiutil.ErrMissingID,
@@ -240,7 +240,7 @@ func TestEncodeError(t *testing.T) {
 		{
 			desc: "BadRequest with validation error",
 			errs: []error{
-				errors.Wrap(apiutil.ErrValidation, apiutil.ErrInvalidSecret),
+				errors.Wrap(apiutil.ErrValidation, apiutil.ErrMissingSecret),
 				errors.Wrap(apiutil.ErrValidation, svcerr.ErrMalformedEntity),
 				errors.Wrap(apiutil.ErrValidation, errors.ErrMalformedEntity),
 				errors.Wrap(apiutil.ErrValidation, apiutil.ErrMissingID),

internal/apiutil/errors.go

@@ -132,6 +132,9 @@ var (
 	// ErrMissingSecret indicates missing secret.
 	ErrMissingSecret = errors.New("missing secret")
 
+	// ErrPasswordFormat indicates weak password.
+	ErrPasswordFormat = errors.New("password does not meet the requirements")
+
 	// ErrMissingOwner indicates missing entity owner.
 	ErrMissingOwner = errors.New("missing entity owner")
 
@@ -144,9 +147,6 @@ var (
 	// ErrMissingName indicates missing identity name.
 	ErrMissingName = errors.New("missing identity name")
 
-	// ErrInvalidSecret indicates invalid secret.
-	ErrInvalidSecret = errors.New("missing secret")
-
 	// ErrInvalidLevel indicates an invalid group level.
 	ErrInvalidLevel = errors.New("invalid group level (should be between 0 and 5)")
 

pkg/sdk/go/groups_test.go

@@ -37,14 +37,14 @@ func setupGroups() (*httptest.Server, *mocks.Repository, *authmocks.AuthClient)
 	grepo := new(mocks.Repository)
 
 	auth := new(authmocks.AuthClient)
-	csvc := users.NewService(crepo, auth, emailer, phasher, idProvider, passRegex, true)
+	csvc := users.NewService(crepo, auth, emailer, phasher, idProvider, true)
 	gsvc := groups.NewService(grepo, idProvider, auth)
 
 	logger := mglog.NewMock()
 	mux := chi.NewRouter()
 	provider := new(oauth2mocks.Provider)
 	provider.On("Name").Return("test")
-	api.MakeHandler(csvc, gsvc, mux, logger, "", provider)
+	api.MakeHandler(csvc, gsvc, mux, logger, "", passRegex, provider)
 
 	return httptest.NewServer(mux), grepo, auth
 }

pkg/sdk/go/things_test.go

@@ -892,8 +892,8 @@ func TestUpdateThingSecret(t *testing.T) {
 			newSecret: "newSecret",
 			token:     validToken,
 			response:  sdk.Thing{},
-			repoErr:   apiutil.ErrInvalidSecret,
-			err:       errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrUpdateEntity, apiutil.ErrInvalidSecret), http.StatusBadRequest),
+			repoErr:   apiutil.ErrMissingSecret,
+			err:       errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrUpdateEntity, apiutil.ErrMissingSecret), http.StatusBadRequest),
 		},
 	}
 	for _, tc := range cases {

pkg/sdk/go/tokens_test.go

@@ -62,7 +62,7 @@ func TestIssueToken(t *testing.T) {
 			desc:  "issue token for an empty user",
 			login: sdk.Login{},
 			token: &magistrala.Token{},
-			err:   errors.NewSDKErrorWithStatus(errors.Wrap(apiutil.ErrValidation, apiutil.ErrMissingIdentity), http.StatusInternalServerError),
+			err:   errors.NewSDKErrorWithStatus(errors.Wrap(apiutil.ErrValidation, apiutil.ErrMissingIdentity), http.StatusBadRequest),
 		},
 		{
 			desc:     "issue token for invalid identity",

pkg/sdk/go/users_test.go

@@ -43,14 +43,14 @@ func setupUsers() (*httptest.Server, *umocks.Repository, *gmocks.Repository, *au
 	gRepo := new(gmocks.Repository)
 
 	auth := new(authmocks.AuthClient)
-	csvc := users.NewService(crepo, auth, emailer, phasher, idProvider, passRegex, true)
+	csvc := users.NewService(crepo, auth, emailer, phasher, idProvider, true)
 	gsvc := groups.NewService(gRepo, idProvider, auth)
 
 	logger := mglog.NewMock()
 	mux := chi.NewRouter()
 	provider := new(oauth2mocks.Provider)
 	provider.On("Name").Return("test")
-	api.MakeHandler(csvc, gsvc, mux, logger, "", provider)
+	api.MakeHandler(csvc, gsvc, mux, logger, "", passRegex, provider)
 
 	return httptest.NewServer(mux), crepo, gRepo, auth
 }
@@ -62,7 +62,7 @@ func TestCreateClient(t *testing.T) {
 	user := sdk.User{
 		Name:        "clientname",
 		Tags:        []string{"tag1", "tag2"},
-		Credentials: sdk.Credentials{Identity: "[email protected]", Secret: "secret"},
+		Credentials: sdk.Credentials{Identity: "[email protected]", Secret: "12345678"},
 		Status:      mgclients.EnabledStatus.String(),
 	}
 	conf := sdk.Config{
@@ -96,7 +96,7 @@ func TestCreateClient(t *testing.T) {
 			client:   sdk.User{},
 			response: sdk.User{},
 			token:    token,
-			err:      errors.NewSDKErrorWithStatus(errors.Wrap(apiutil.ErrValidation, errors.ErrMalformedEntity), http.StatusBadRequest),
+			err:      errors.NewSDKErrorWithStatus(errors.Wrap(apiutil.ErrValidation, apiutil.ErrMissingIdentity), http.StatusBadRequest),
 		},
 		{
 			desc: "register a user that can't be marshalled",
@@ -135,7 +135,7 @@ func TestCreateClient(t *testing.T) {
 			},
 			response: sdk.User{},
 			token:    token,
-			err:      errors.NewSDKErrorWithStatus(errors.Wrap(apiutil.ErrValidation, errors.ErrMalformedEntity), http.StatusBadRequest),
+			err:      errors.NewSDKErrorWithStatus(errors.Wrap(apiutil.ErrValidation, apiutil.ErrMissingIdentity), http.StatusBadRequest),
 		},
 		{
 			desc: "register user with empty identity",
@@ -147,14 +147,7 @@ func TestCreateClient(t *testing.T) {
 			},
 			response: sdk.User{},
 			token:    token,
-			err:      errors.NewSDKErrorWithStatus(errors.Wrap(apiutil.ErrValidation, errors.ErrMalformedEntity), http.StatusBadRequest),
-		},
-		{
-			desc:     "register empty user",
-			client:   sdk.User{},
-			response: sdk.User{},
-			token:    token,
-			err:      errors.NewSDKErrorWithStatus(errors.Wrap(apiutil.ErrValidation, errors.ErrMalformedEntity), http.StatusBadRequest),
+			err:      errors.NewSDKErrorWithStatus(errors.Wrap(apiutil.ErrValidation, apiutil.ErrMissingIdentity), http.StatusBadRequest),
 		},
 		{
 			desc: "register user with every field defined",
@@ -828,7 +821,7 @@ func TestUpdateClientSecret(t *testing.T) {
 			newSecret: "newSecret",
 			token:     validToken,
 			response:  sdk.User{},
-			repoErr:   apiutil.ErrInvalidSecret,
+			repoErr:   apiutil.ErrMissingSecret,
 			err:       errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrNotFound, repoerr.ErrMissingSecret), http.StatusBadRequest),
 		},
 	}

users/api/clients.go

@@ -8,6 +8,7 @@ import (
 	"encoding/json"
 	"log/slog"
 	"net/http"
+	"regexp"
 	"strings"
 
 	"github.com/absmach/magistrala/auth"
@@ -22,8 +23,12 @@ import (
 	"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
 )
 
+var passRegex = regexp.MustCompile("^.{8,}$")
+
 // MakeHandler returns a HTTP handler for API endpoints.
-func clientsHandler(svc users.Service, r *chi.Mux, logger *slog.Logger, providers ...oauth2.Provider) http.Handler {
+func clientsHandler(svc users.Service, r *chi.Mux, logger *slog.Logger, pr *regexp.Regexp, providers ...oauth2.Provider) http.Handler {
+	passRegex = pr
+
 	opts := []kithttp.ServerOption{
 		kithttp.ServerErrorEncoder(apiutil.LoggingErrorEncoder(logger, api.EncodeError)),
 	}

users/api/endpoint_test.go

@@ -9,6 +9,7 @@ import (
 	"io"
 	"net/http"
 	"net/http/httptest"
+	"regexp"
 	"strings"
 	"testing"
 
@@ -44,11 +45,11 @@ var (
 		Metadata:    validCMetadata,
 		Status:      mgclients.EnabledStatus,
 	}
-	validToken        = "valid"
-	inValidToken      = "invalid"
-	inValid           = "invalid"
-	validID           = "d4ebb847-5d0e-4e46-bdd9-b6aceaaa3a22"
-	ErrPasswordFormat = errors.New("password does not meet the requirements")
+	validToken   = "valid"
+	inValidToken = "invalid"
+	inValid      = "invalid"
+	validID      = "d4ebb847-5d0e-4e46-bdd9-b6aceaaa3a22"
+	passRegex    = regexp.MustCompile("^.{8,}$")
 )
 
 const contentType = "application/json"
@@ -92,7 +93,7 @@ func newUsersServer() (*httptest.Server, *mocks.Service) {
 	mux := chi.NewRouter()
 	provider := new(oauth2mocks.Provider)
 	provider.On("Name").Return("test")
-	httpapi.MakeHandler(svc, gsvc, mux, logger, "", provider)
+	httpapi.MakeHandler(svc, gsvc, mux, logger, "", passRegex, provider)
 
 	return httptest.NewServer(mux), svc
 }
@@ -1166,8 +1167,8 @@ func TestPasswordReset(t *testing.T) {
 			data:        fmt.Sprintf(`{"token": "%s", "password": "%s", "confirm_password": "%s"}`, validToken, "weak", "weak"),
 			token:       validToken,
 			contentType: contentType,
-			status:      http.StatusInternalServerError,
-			err:         ErrPasswordFormat,
+			status:      http.StatusBadRequest,
+			err:         apiutil.ErrPasswordFormat,
 		},
 		{
 			desc:        "password reset with empty token",
@@ -1182,7 +1183,7 @@ func TestPasswordReset(t *testing.T) {
 			data:        fmt.Sprintf(`{"token": "%s", "password": "%s", "confirm_password": "%s"}`, validToken, "", ""),
 			token:       validToken,
 			contentType: contentType,
-			status:      http.StatusInternalServerError,
+			status:      http.StatusBadRequest,
 			err:         apiutil.ErrValidation,
 		},
 		{
@@ -1338,7 +1339,7 @@ func TestUpdateClientSecret(t *testing.T) {
 	}{
 		{
 			desc: "update user secret with valid token",
-			data: fmt.Sprintf(`{"secret": "%s"}`, "strongersecret"),
+			data: `{"old_secret": "strongersecret", "new_secret": "strongersecret"}`,
 			client: mgclients.Client{
 				ID: client.ID,
 				Credentials: mgclients.Credentials{
@@ -1353,7 +1354,7 @@ func TestUpdateClientSecret(t *testing.T) {
 		},
 		{
 			desc: "update user secret with empty token",
-			data: fmt.Sprintf(`{"secret": "%s"}`, "strongersecret"),
+			data: `{"old_secret": "strongersecret", "new_secret": "strongersecret"}`,
 			client: mgclients.Client{
 				ID: client.ID,
 				Credentials: mgclients.Credentials{
@@ -1368,7 +1369,7 @@ func TestUpdateClientSecret(t *testing.T) {
 		},
 		{
 			desc: "update user secret with invalid token",
-			data: fmt.Sprintf(`{"secret": "%s"}`, "strongersecret"),
+			data: `{"old_secret": "strongersecret", "new_secret": "strongersecret"}`,
 			client: mgclients.Client{
 				ID: client.ID,
 				Credentials: mgclients.Credentials{
@@ -1384,7 +1385,7 @@ func TestUpdateClientSecret(t *testing.T) {
 
 		{
 			desc: "update user secret with empty secret",
-			data: fmt.Sprintf(`{"secret": "%s"}`, ""),
+			data: `{"old_secret": "", "new_secret": "strongersecret"}`,
 			client: mgclients.Client{
 				ID: client.ID,
 				Credentials: mgclients.Credentials{
@@ -1395,11 +1396,11 @@ func TestUpdateClientSecret(t *testing.T) {
 			contentType: contentType,
 			token:       validToken,
 			status:      http.StatusBadRequest,
-			err:         apiutil.ErrBearerKey,
+			err:         apiutil.ErrMissingPass,
 		},
 		{
 			desc: "update user secret with invalid contentype",
-			data: fmt.Sprintf(`{"secret": "%s"}`, ""),
+			data: `{"old_secret": "strongersecret", "new_secret": "strongersecret"}`,
 			client: mgclients.Client{
 				ID: client.ID,
 				Credentials: mgclients.Credentials{
@@ -1479,7 +1480,7 @@ func TestIssueToken(t *testing.T) {
 			desc:        "issue token with empty identity",
 			data:        fmt.Sprintf(`{"identity": "%s", "secret": "%s", "domainID": "%s"}`, "", secret, validID),
 			contentType: contentType,
-			status:      http.StatusInternalServerError,
+			status:      http.StatusBadRequest,
 			err:         apiutil.ErrValidation,
 		},
 		{

users/api/requests.go

@@ -20,6 +20,15 @@ func (req createClientReq) validate() error {
 	if len(req.client.Name) > api.MaxNameSize {
 		return apiutil.ErrNameSize
 	}
+	if req.client.Credentials.Identity == "" {
+		return apiutil.ErrMissingIdentity
+	}
+	if req.client.Credentials.Secret == "" {
+		return apiutil.ErrMissingPass
+	}
+	if !passRegex.MatchString(req.client.Credentials.Secret) {
+		return apiutil.ErrPasswordFormat
+	}
 
 	return req.client.Validate()
 }
@@ -180,6 +189,12 @@ func (req updateClientSecretReq) validate() error {
 	if req.token == "" {
 		return apiutil.ErrBearerToken
 	}
+	if req.OldSecret == "" || req.NewSecret == "" {
+		return apiutil.ErrMissingPass
+	}
+	if !passRegex.MatchString(req.NewSecret) {
+		return apiutil.ErrPasswordFormat
+	}
 
 	return nil
 }
@@ -211,7 +226,7 @@ func (req loginClientReq) validate() error {
 		return apiutil.ErrMissingIdentity
 	}
 	if req.Secret == "" {
-		return apiutil.ErrMissingSecret
+		return apiutil.ErrMissingPass
 	}
 
 	return nil
@@ -265,6 +280,9 @@ func (req resetTokenReq) validate() error {
 	if req.Password != req.ConfPass {
 		return apiutil.ErrInvalidResetPass
 	}
+	if !passRegex.MatchString(req.ConfPass) {
+		return apiutil.ErrPasswordFormat
+	}
 
 	return nil
 }