fix(users): add secret validation on registration

Add secret validation on the service layer when registering users. Updated HTTP status codes and adjusted test cases for issue token and client creation validation with missing either identity or secret.

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

internal/api/common.go

@@ -121,7 +121,9 @@ 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):
 		w.WriteHeader(http.StatusBadRequest)
 	case errors.Contains(err, svcerr.ErrAuthentication),
 		errors.Contains(err, apiutil.ErrBearerToken):

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

@@ -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",

users/api/endpoint_test.go

@@ -1479,7 +1479,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,12 @@ 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.ErrMissingSecret
+	}
 
 	return req.client.Validate()
 }

users/api/requests_test.go

@@ -67,6 +67,34 @@ func TestCreateClientReqValidate(t *testing.T) {
 			},
 			err: apiutil.ErrNameSize,
 		},
+		{
+			desc: "missing identity in request",
+			req: createClientReq{
+				token: valid,
+				client: mgclients.Client{
+					ID:   validID,
+					Name: valid,
+					Credentials: mgclients.Credentials{
+						Secret: valid,
+					},
+				},
+			},
+			err: apiutil.ErrMissingIdentity,
+		},
+		{
+			desc: "missing secret in request",
+			req: createClientReq{
+				token: valid,
+				client: mgclients.Client{
+					ID:   validID,
+					Name: valid,
+					Credentials: mgclients.Credentials{
+						Identity: "[email protected]",
+					},
+				},
+			},
+			err: apiutil.ErrMissingSecret,
+		},
 	}
 	for _, tc := range cases {
 		err := tc.req.validate()

users/service.go

@@ -84,6 +84,9 @@ func (svc service) RegisterClient(ctx context.Context, token string, cli mgclien
 	}
 
 	if cli.Credentials.Secret != "" {
+		if !svc.passRegex.MatchString(cli.Credentials.Secret) {
+			return mgclients.Client{}, errors.Wrap(svcerr.ErrMalformedEntity, ErrPasswordFormat)
+		}
 		hash, err := svc.hasher.Hash(cli.Credentials.Secret)
 		if err != nil {
 			return mgclients.Client{}, errors.Wrap(repoerr.ErrMalformedEntity, err)
@@ -92,10 +95,10 @@ func (svc service) RegisterClient(ctx context.Context, token string, cli mgclien
 	}
 
 	if cli.Status != mgclients.DisabledStatus && cli.Status != mgclients.EnabledStatus {
-		return mgclients.Client{}, svcerr.ErrInvalidStatus
+		return mgclients.Client{}, errors.Wrap(svcerr.ErrMalformedEntity, svcerr.ErrInvalidStatus)
 	}
 	if cli.Role != mgclients.UserRole && cli.Role != mgclients.AdminRole {
-		return mgclients.Client{}, svcerr.ErrInvalidRole
+		return mgclients.Client{}, errors.Wrap(svcerr.ErrMalformedEntity, svcerr.ErrInvalidRole)
 	}
 	cli.ID = clientID
 	cli.CreatedAt = time.Now()
@@ -314,7 +317,7 @@ func (svc service) ResetSecret(ctx context.Context, resetToken, secret string) e
 		return repoerr.ErrNotFound
 	}
 	if !svc.passRegex.MatchString(secret) {
-		return ErrPasswordFormat
+		return errors.Wrap(svcerr.ErrMalformedEntity, ErrPasswordFormat)
 	}
 	secret, err = svc.hasher.Hash(secret)
 	if err != nil {
@@ -340,7 +343,7 @@ func (svc service) UpdateClientSecret(ctx context.Context, token, oldSecret, new
 		return mgclients.Client{}, err
 	}
 	if !svc.passRegex.MatchString(newSecret) {
-		return mgclients.Client{}, ErrPasswordFormat
+		return mgclients.Client{}, errors.Wrap(svcerr.ErrMalformedEntity, ErrPasswordFormat)
 	}
 	dbClient, err := svc.clients.RetrieveByID(ctx, id)
 	if err != nil {

users/service_test.go

@@ -174,7 +174,7 @@ func TestRegisterClient(t *testing.T) {
 			},
 			addPoliciesResponse:    &magistrala.AddPoliciesRes{Added: true},
 			deletePoliciesResponse: &magistrala.DeletePoliciesRes{Deleted: true},
-			err:                    nil,
+			err:                    errors.ErrMalformedEntity,
 		},
 		{
 			desc: " register a client with a secret that is too long",