From dc1b30baffd747276b0ed73361e7020232f3a049 Mon Sep 17 00:00:00 2001
From: Ishan Arya <ishanarya0@gmail.com>
Date: Fri, 17 May 2024 11:48:27 +0530
Subject: [PATCH] fix: relation creation API (#49)

---
 core/resource/service.go              | 17 +++++++--------
 internal/api/v1beta1/relation.go      |  5 +++++
 internal/api/v1beta1/relation_test.go | 31 +++++++++++++++++++++++++--
 3 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/core/resource/service.go b/core/resource/service.go
index b92069b1a..10ddd5bff 100644
--- a/core/resource/service.go
+++ b/core/resource/service.go
@@ -227,8 +227,8 @@ func (s Service) CheckAuthz(ctx context.Context, res Resource, act action.Action
 	isSystemNS := namespace.IsSystemNamespaceID(res.NamespaceID)
 	fetchedResource := res
 
-	if isSystemNS {
-		if !uuid.IsValid(res.Name) {
+	if !uuid.IsValid(res.Name) {
+		if isSystemNS {
 			switch res.NamespaceID {
 			case namespace.DefinitionProject.ID:
 				project, err := s.projectService.Get(ctx, res.Name)
@@ -249,15 +249,14 @@ func (s Service) CheckAuthz(ctx context.Context, res Resource, act action.Action
 				}
 				res.Name = group.ID
 			}
-		}
-		fetchedResource.Idxa = res.Name
-	} else {
-		fetchedResource, err = s.repository.GetByNamespace(ctx, res.Name, res.NamespaceID)
-		if err != nil {
-			return false, ErrNotExist
+		} else {
+			fetchedResource, err = s.repository.GetByNamespace(ctx, res.Name, res.NamespaceID)
+			if err != nil {
+				return false, ErrNotExist
+			}
 		}
 	}
-
+	fetchedResource.Idxa = res.Name
 	fetchedResourceNS := namespace.Namespace{ID: fetchedResource.NamespaceID}
 	return s.relationService.CheckPermission(ctx, currentUser, fetchedResourceNS, fetchedResource.Idxa, act)
 }
diff --git a/internal/api/v1beta1/relation.go b/internal/api/v1beta1/relation.go
index d06413218..41aaf08e0 100644
--- a/internal/api/v1beta1/relation.go
+++ b/internal/api/v1beta1/relation.go
@@ -13,6 +13,7 @@ import (
 
 	"github.com/goto/shield/core/relation"
 	errpkg "github.com/goto/shield/pkg/errors"
+	"github.com/goto/shield/pkg/uuid"
 	shieldv1beta1 "github.com/goto/shield/proto/v1beta1"
 	grpczap "github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap"
 	"google.golang.org/grpc/codes"
@@ -60,6 +61,10 @@ func (h Handler) CreateRelation(ctx context.Context, request *shieldv1beta1.Crea
 		return nil, grpcBadBodyError
 	}
 
+	if !uuid.IsValid(request.GetBody().GetObjectId()) {
+		return nil, grpcBadBodyError
+	}
+
 	principal, subjectID := extractSubjectFromPrincipal(request.GetBody().GetSubject())
 
 	result, err := h.resourceService.CheckAuthz(ctx, resource.Resource{
diff --git a/internal/api/v1beta1/relation_test.go b/internal/api/v1beta1/relation_test.go
index c62074b7c..b6844c967 100644
--- a/internal/api/v1beta1/relation_test.go
+++ b/internal/api/v1beta1/relation_test.go
@@ -26,14 +26,27 @@ var (
 			RoleID:    "role1",
 		},
 		Object: relation.Object{
-			ID:          "object-id",
+			ID:          "5e70ba45-dc63-4152-9d72-27cbc34d9d13",
+			NamespaceID: "ns2",
+		},
+	}
+
+	testRelationV2Invalid = relation.RelationV2{
+		ID: "relation-id-1",
+		Subject: relation.Subject{
+			ID:        "subject-id",
+			Namespace: "ns1",
+			RoleID:    "role1",
+		},
+		Object: relation.Object{
+			ID:          "non-uuid",
 			NamespaceID: "ns2",
 		},
 	}
 
 	testRelationPB = &shieldv1beta1.Relation{
 		Id:              "relation-id-1",
-		ObjectId:        "object-id",
+		ObjectId:        "5e70ba45-dc63-4152-9d72-27cbc34d9d13",
 		ObjectNamespace: "ns2",
 		Subject:         "ns1:subject-id",
 		RoleName:        "role1",
@@ -111,6 +124,20 @@ func TestHandler_CreateRelation(t *testing.T) {
 			want:    nil,
 			wantErr: grpcInternalServerError,
 		},
+		{
+			name:  "should return bad body error if object id is not uuid",
+			setup: func(rs *mocks.RelationService, res *mocks.ResourceService) {},
+			request: &shieldv1beta1.CreateRelationRequest{
+				Body: &shieldv1beta1.RelationRequestBody{
+					ObjectId:        testRelationV2Invalid.Object.ID,
+					ObjectNamespace: testRelationV2Invalid.Object.NamespaceID,
+					Subject:         generateSubject(testRelationV2Invalid.Subject.ID, testRelationV2Invalid.Subject.Namespace),
+					RoleName:        testRelationV2Invalid.Subject.RoleID,
+				},
+			},
+			want:    nil,
+			wantErr: grpcBadBodyError,
+		},
 		{
 			name: "should return permision denied error if resource service's CheckAuthz function returns false",
 			setup: func(rs *mocks.RelationService, res *mocks.ResourceService) {