Add validation tests for user entity

See merge request !572

Co-authored-by: Neil-Jocelyn Schark <neil-jocelyn.schark@stud.h-da.de>

controller/northbound/server/user.go

@@ -3,6 +3,7 @@ package server
 import (
 	"context"
 	"encoding/base64"
+	"errors"
 	"fmt"
 	"time"
 
@@ -20,6 +21,7 @@ import (
 	log "github.com/sirupsen/logrus"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
+	"google.golang.org/protobuf/reflect/protoreflect"
 
 	"golang.org/x/crypto/argon2"
 )
@@ -41,14 +43,33 @@ func NewUserServer(jwtManager *rbac.JWTManager, userService rbacInterfaces.UserS
 	}
 }
 
+func (r UserServer) checkForValidationErrors(request protoreflect.ProtoMessage) error {
+	err := r.protoValidator.Validate(request)
+	if err != nil {
+		var valErr *protovalidate.ValidationError
+
+		if ok := errors.As(err, &valErr); ok {
+			protoErr := valErr.ToProto()
+			grpcError, _ := status.New(codes.Aborted, "Validation failed").WithDetails(protoErr)
+
+			return grpcError.Err()
+		}
+
+		return status.Errorf(codes.Aborted, "%v", err)
+	}
+
+	return nil
+}
+
 // CreateUsers creates new users, can be 1 or more.
 func (u UserServer) CreateUsers(ctx context.Context, request *apb.CreateUsersRequest) (*apb.CreateUsersResponse, error) {
 	labels := prometheus.Labels{"service": "auth", "rpc": "post"}
 	start := metrics.StartHook(labels, grpcRequestsTotal)
 	defer metrics.FinishHook(labels, start, grpcRequestDurationSecondsTotal, grpcRequestDurationSeconds)
 
-	if err := u.protoValidator.Validate(request); err != nil {
-		return nil, status.Errorf(codes.Aborted, "%v", err)
+	err := u.checkForValidationErrors(request)
+	if err != nil {
+		return nil, err
 	}
 
 	for _, user := range request.User {
@@ -90,8 +111,9 @@ func (u UserServer) GetUser(ctx context.Context, request *apb.GetUserRequest) (*
 	start := metrics.StartHook(labels, grpcRequestsTotal)
 	defer metrics.FinishHook(labels, start, grpcRequestDurationSecondsTotal, grpcRequestDurationSeconds)
 
-	if err := u.protoValidator.Validate(request); err != nil {
-		return nil, status.Errorf(codes.Aborted, "%v", err)
+	err := u.checkForValidationErrors(request)
+	if err != nil {
+		return nil, err
 	}
 
 	userID, err := uuid.Parse(request.Id)
@@ -125,8 +147,9 @@ func (u UserServer) GetUsers(ctx context.Context, request *apb.GetUsersRequest)
 	start := metrics.StartHook(labels, grpcRequestsTotal)
 	defer metrics.FinishHook(labels, start, grpcRequestDurationSecondsTotal, grpcRequestDurationSeconds)
 
-	if err := u.protoValidator.Validate(request); err != nil {
-		return nil, status.Errorf(codes.Aborted, "%v", err)
+	err := u.checkForValidationErrors(request)
+	if err != nil {
+		return nil, err
 	}
 
 	userList, err := u.userService.GetAll()
@@ -158,8 +181,9 @@ func (u UserServer) UpdateUsers(ctx context.Context, request *apb.UpdateUsersReq
 	start := metrics.StartHook(labels, grpcRequestsTotal)
 	defer metrics.FinishHook(labels, start, grpcRequestDurationSecondsTotal, grpcRequestDurationSeconds)
 
-	if err := u.protoValidator.Validate(request); err != nil {
-		return nil, status.Errorf(codes.Aborted, "%v", err)
+	err := u.checkForValidationErrors(request)
+	if err != nil {
+		return nil, err
 	}
 
 	for _, user := range request.User {
@@ -198,8 +222,9 @@ func (u UserServer) DeleteUsers(ctx context.Context, request *apb.DeleteUsersReq
 	start := metrics.StartHook(labels, grpcRequestsTotal)
 	defer metrics.FinishHook(labels, start, grpcRequestDurationSecondsTotal, grpcRequestDurationSeconds)
 
-	if err := u.protoValidator.Validate(request); err != nil {
-		return nil, status.Errorf(codes.Aborted, "%v", err)
+	err := u.checkForValidationErrors(request)
+	if err != nil {
+		return nil, err
 	}
 
 	for _, user := range request.Username {

controller/northbound/server/user_test.go

@@ -2,10 +2,10 @@ package server
 
 import (
 	"context"
-	"fmt"
 	"testing"
 	"time"
 
+	"buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate"
 	"code.fbi.h-da.de/danet/gosdn/api/go/gosdn/conflict"
 	apb "code.fbi.h-da.de/danet/gosdn/api/go/gosdn/rbac"
 	eventservice "code.fbi.h-da.de/danet/gosdn/controller/eventService"
@@ -44,9 +44,10 @@ func TestUser_CreateUsers(t *testing.T) {
 		request *apb.CreateUsersRequest
 	}
 	tests := []struct {
-		name    string
-		args    args
-		wantErr bool
+		name             string
+		args             args
+		wantErr          bool
+		validationErrors []*validate.Violation
 	}{
 		{
 			name: "default create users",
@@ -87,6 +88,38 @@ func TestUser_CreateUsers(t *testing.T) {
 				},
 			},
 			wantErr: true,
+			validationErrors: []*validate.Violation{
+				{
+					FieldPath:    "user[0].password",
+					ConstraintId: "string.min_len",
+					Message:      "value length must be at least 5 characters",
+				}},
+		},
+		{
+			name: "create users with too short username should fail",
+			args: args{
+				ctx: context.TODO(),
+				request: &apb.CreateUsersRequest{
+					User: []*apb.User{
+						{
+							Name:     "a",
+							Roles:    map[string]string{pndID: "userTestRole"},
+							Password: "password",
+							Token:    "",
+							Metadata: &conflict.Metadata{
+								ResourceVersion: 0,
+							},
+						},
+					},
+				},
+			},
+			wantErr: true,
+			validationErrors: []*validate.Violation{
+				{
+					FieldPath:    "user[0].name",
+					ConstraintId: "string.min_len",
+					Message:      "value length must be at least 3 characters",
+				}},
 		},
 	}
 	for _, tt := range tests {
@@ -97,7 +130,10 @@ func TestUser_CreateUsers(t *testing.T) {
 				t.Errorf("User.CreateUsers() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
-			fmt.Printf("err %+v,want no error\n", err)
+
+			if tt.wantErr {
+				assertValidationErrors(t, err, tt.validationErrors)
+			}
 		})
 	}
 }
@@ -109,10 +145,11 @@ func TestUser_GetUser(t *testing.T) {
 		request *apb.GetUserRequest
 	}
 	tests := []struct {
-		name    string
-		args    args
-		want    *apb.GetUserResponse
-		wantErr bool
+		name             string
+		args             args
+		want             *apb.GetUserResponse
+		wantErr          bool
+		validationErrors []*validate.Violation
 	}{
 		{
 			name: "default get user",
@@ -140,6 +177,24 @@ func TestUser_GetUser(t *testing.T) {
 			want:    nil,
 			wantErr: true,
 		},
+		{
+			name: "fail get user due to missing name",
+			args: args{
+				ctx: context.TODO(),
+				request: &apb.GetUserRequest{
+					Name: "",
+					Id:   uuid.Nil.String(),
+				},
+			},
+			want:    nil,
+			wantErr: true,
+			validationErrors: []*validate.Violation{
+				{
+					FieldPath:    "name",
+					ConstraintId: "required",
+					Message:      "value is required",
+				}},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -150,6 +205,10 @@ func TestUser_GetUser(t *testing.T) {
 				return
 			}
 
+			if tt.wantErr {
+				assertValidationErrors(t, err, tt.validationErrors)
+			}
+
 			if got != nil {
 				if got.User.Name != tt.want.User.Name || got.User.Id != tt.want.User.Id {
 					t.Errorf("User.GetUser() = %v, want %v", got, tt.want)
@@ -230,10 +289,11 @@ func TestUser_UpdateUsers(t *testing.T) {
 		request *apb.UpdateUsersRequest
 	}
 	tests := []struct {
-		name    string
-		args    args
-		want    *apb.UpdateUsersResponse
-		wantErr bool
+		name             string
+		args             args
+		want             *apb.UpdateUsersResponse
+		wantErr          bool
+		validationErrors []*validate.Violation
 	}{
 		{
 			name: "default update user",
@@ -266,7 +326,7 @@ func TestUser_UpdateUsers(t *testing.T) {
 			wantErr: true,
 		},
 		{
-			name: "update user without name should fail",
+			name: "update user without name should fail validation",
 			args: args{ctx: context.TODO(),
 				request: &apb.UpdateUsersRequest{User: []*apb.UpdateUser{
 					{
@@ -279,6 +339,13 @@ func TestUser_UpdateUsers(t *testing.T) {
 			},
 			want:    nil,
 			wantErr: true,
+			validationErrors: []*validate.Violation{
+				{
+					FieldPath:    "name",
+					ConstraintId: "required",
+					Message:      "value is required",
+				},
+			},
 		},
 	}
 
@@ -290,6 +357,10 @@ func TestUser_UpdateUsers(t *testing.T) {
 				t.Errorf("User.UpdateUsers() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
+
+			if tt.wantErr {
+				assertValidationErrors(t, err, tt.validationErrors)
+			}
 		})
 	}
 }
@@ -300,10 +371,11 @@ func TestUser_DeleteUsers(t *testing.T) {
 		request *apb.DeleteUsersRequest
 	}
 	tests := []struct {
-		name    string
-		args    args
-		want    *apb.DeleteUsersResponse
-		wantErr bool
+		name             string
+		args             args
+		want             *apb.DeleteUsersResponse
+		wantErr          bool
+		validationErrors []*validate.Violation
 	}{
 		{
 			name: "default delete users",
@@ -314,13 +386,28 @@ func TestUser_DeleteUsers(t *testing.T) {
 			wantErr: false,
 		},
 		{
-			name: "error delete users",
+			name: "error delete users for non existing user",
 			args: args{ctx: context.TODO(),
 				request: &apb.DeleteUsersRequest{Username: []string{"no user"}},
 			},
 			want:    &apb.DeleteUsersResponse{},
 			wantErr: true,
 		},
+		{
+			name: "error delete users due to missing name",
+			args: args{ctx: context.TODO(),
+				request: &apb.DeleteUsersRequest{Username: []string{""}},
+			},
+			want:    &apb.DeleteUsersResponse{},
+			wantErr: true,
+			validationErrors: []*validate.Violation{
+				{
+					FieldPath:    "name",
+					ConstraintId: "required",
+					Message:      "value is required",
+				},
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -330,6 +417,10 @@ func TestUser_DeleteUsers(t *testing.T) {
 				t.Errorf("User.DeleteUsers() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
+
+			if tt.wantErr {
+				assertValidationErrors(t, err, tt.validationErrors)
+			}
 		})
 	}
 }