fix: fetch user email error log (#48)

goto · May 17, 2024 · cf4b97f · cf4b97f
1 parent dc1b30b
commit cf4b97f
Show file tree

Hide file tree

Showing 21 changed files with 190 additions and 30 deletions.
diff --git a/core/action/service.go b/core/action/service.go
@@ -46,7 +46,7 @@ func (s Service) Get(ctx context.Context, id string) (Action, error) {
 func (s Service) Create(ctx context.Context, action Action) (Action, error) {
 	currentUser, err := s.userService.FetchCurrentUser(ctx)
 	if err != nil {
-		return Action{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error())
+		return Action{}, err
 	}
 
 	newAction, err := s.repository.Create(ctx, action)
@@ -73,7 +73,7 @@ func (s Service) List(ctx context.Context) ([]Action, error) {
 func (s Service) Update(ctx context.Context, id string, action Action) (Action, error) {
 	currentUser, err := s.userService.FetchCurrentUser(ctx)
 	if err != nil {
-		return Action{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error())
+		return Action{}, err
 	}
 
 	updatedAction, err := s.repository.Update(ctx, Action{

diff --git a/core/group/service.go b/core/group/service.go
@@ -59,7 +59,7 @@ func NewService(logger log.Logger, repository Repository, relationService Relati
 func (s Service) Create(ctx context.Context, grp Group) (Group, error) {
 	currentUser, err := s.userService.FetchCurrentUser(ctx)
 	if err != nil {
-		return Group{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error())
+		return Group{}, err
 	}
 
 	newGroup, err := s.repository.Create(ctx, grp)
@@ -105,7 +105,7 @@ func (s Service) List(ctx context.Context, flt Filter) ([]Group, error) {
 func (s Service) Update(ctx context.Context, grp Group) (Group, error) {
 	currentUser, err := s.userService.FetchCurrentUser(ctx)
 	if err != nil {
-		return Group{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error())
+		return Group{}, err
 	}
 
 	if strings.TrimSpace(grp.ID) != "" {

diff --git a/core/namespace/service.go b/core/namespace/service.go
@@ -46,7 +46,7 @@ func (s Service) Get(ctx context.Context, id string) (Namespace, error) {
 func (s Service) Create(ctx context.Context, ns Namespace) (Namespace, error) {
 	currentUser, err := s.userService.FetchCurrentUser(ctx)
 	if err != nil {
-		return Namespace{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error())
+		return Namespace{}, err
 	}
 
 	newNamespace, err := s.repository.Create(ctx, ns)
@@ -73,7 +73,7 @@ func (s Service) List(ctx context.Context) ([]Namespace, error) {
 func (s Service) Update(ctx context.Context, ns Namespace) (Namespace, error) {
 	currentUser, err := s.userService.FetchCurrentUser(ctx)
 	if err != nil {
-		return Namespace{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error())
+		return Namespace{}, err
 	}
 
 	updatedNamespace, err := s.repository.Update(ctx, ns)

diff --git a/core/organization/service.go b/core/organization/service.go
@@ -64,7 +64,7 @@ func (s Service) Get(ctx context.Context, idOrSlug string) (Organization, error)
 func (s Service) Create(ctx context.Context, org Organization) (Organization, error) {
 	currentUser, err := s.userService.FetchCurrentUser(ctx)
 	if err != nil {
-		return Organization{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error())
+		return Organization{}, err
 	}
 
 	newOrg, err := s.repository.Create(ctx, Organization{
@@ -99,7 +99,7 @@ func (s Service) List(ctx context.Context) ([]Organization, error) {
 func (s Service) Update(ctx context.Context, org Organization) (Organization, error) {
 	currentUser, err := s.userService.FetchCurrentUser(ctx)
 	if err != nil {
-		return Organization{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error())
+		return Organization{}, err
 	}
 
 	var updatedOrg Organization

diff --git a/core/policy/service.go b/core/policy/service.go
@@ -50,7 +50,7 @@ func (s Service) List(ctx context.Context) ([]Policy, error) {
 func (s Service) Create(ctx context.Context, policy Policy) ([]Policy, error) {
 	currentUser, err := s.userService.FetchCurrentUser(ctx)
 	if err != nil {
-		return []Policy{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error())
+		return []Policy{}, err
 	}
 
 	policyId, err := s.repository.Create(ctx, policy)
@@ -77,7 +77,7 @@ func (s Service) Create(ctx context.Context, policy Policy) ([]Policy, error) {
 func (s Service) Update(ctx context.Context, pol Policy) ([]Policy, error) {
 	currentUser, err := s.userService.FetchCurrentUser(ctx)
 	if err != nil {
-		return []Policy{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error())
+		return []Policy{}, err
 	}
 
 	policyId, err := s.repository.Update(ctx, pol)

diff --git a/core/project/service.go b/core/project/service.go
@@ -65,7 +65,7 @@ func (s Service) Get(ctx context.Context, idOrSlug string) (Project, error) {
 func (s Service) Create(ctx context.Context, prj Project) (Project, error) {
 	currentUser, err := s.userService.FetchCurrentUser(ctx)
 	if err != nil {
-		return Project{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error())
+		return Project{}, err
 	}
 
 	newProject, err := s.repository.Create(ctx, Project{
@@ -101,7 +101,7 @@ func (s Service) List(ctx context.Context) ([]Project, error) {
 func (s Service) Update(ctx context.Context, prj Project) (Project, error) {
 	currentUser, err := s.userService.FetchCurrentUser(ctx)
 	if err != nil {
-		return Project{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error())
+		return Project{}, err
 	}
 
 	if prj.ID != "" {

diff --git a/core/relation/service.go b/core/relation/service.go
@@ -50,7 +50,7 @@ func (s Service) Get(ctx context.Context, id string) (RelationV2, error) {
 func (s Service) Create(ctx context.Context, rel RelationV2) (RelationV2, error) {
 	currentUser, err := s.userService.FetchCurrentUser(ctx)
 	if err != nil {
-		return RelationV2{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error())
+		return RelationV2{}, err
 	}
 
 	createdRelation, err := s.repository.Create(ctx, rel)
@@ -151,7 +151,7 @@ func (s Service) CheckPermission(ctx context.Context, usr user.User, resourceNS
 func (s Service) DeleteSubjectRelations(ctx context.Context, resourceType, optionalResourceID string) error {
 	currentUser, err := s.userService.FetchCurrentUser(ctx)
 	if err != nil {
-		return fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error())
+		return err
 	}
 
 	err = s.authzRepository.DeleteSubjectRelations(ctx, resourceType, optionalResourceID)

diff --git a/core/resource/service.go b/core/resource/service.go
@@ -84,7 +84,7 @@ func (s Service) Get(ctx context.Context, id string) (Resource, error) {
 func (s Service) Create(ctx context.Context, res Resource) (Resource, error) {
 	currentUser, err := s.userService.FetchCurrentUser(ctx)
 	if err != nil {
-		return Resource{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error())
+		return Resource{}, err
 	}
 
 	urn := res.CreateURN()
@@ -153,7 +153,7 @@ func (s Service) List(ctx context.Context, flt Filter) (PagedResources, error) {
 func (s Service) Update(ctx context.Context, id string, resource Resource) (Resource, error) {
 	currentUser, err := s.userService.FetchCurrentUser(ctx)
 	if err != nil {
-		return Resource{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error())
+		return Resource{}, err
 	}
 
 	// TODO there should be an update logic like create here

diff --git a/core/role/service.go b/core/role/service.go
@@ -42,7 +42,7 @@ func NewService(logger log.Logger, repository Repository, userService UserServic
 func (s Service) Create(ctx context.Context, toCreate Role) (Role, error) {
 	currentUser, err := s.userService.FetchCurrentUser(ctx)
 	if err != nil {
-		return Role{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error())
+		return Role{}, err
 	}
 
 	roleID, err := s.repository.Create(ctx, toCreate)
@@ -78,7 +78,7 @@ func (s Service) List(ctx context.Context) ([]Role, error) {
 func (s Service) Update(ctx context.Context, toUpdate Role) (Role, error) {
 	currentUser, err := s.userService.FetchCurrentUser(ctx)
 	if err != nil {
-		return Role{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error())
+		return Role{}, err
 	}
 
 	roleID, err := s.repository.Update(ctx, toUpdate)

diff --git a/core/user/service.go b/core/user/service.go
@@ -57,7 +57,7 @@ func (s Service) GetByEmail(ctx context.Context, email string) (User, error) {
 func (s Service) Create(ctx context.Context, user User) (User, error) {
 	currentUser, err := s.FetchCurrentUser(ctx)
 	if err != nil {
-		return User{}, fmt.Errorf("%w: %s", ErrInvalidEmail, err.Error())
+		return User{}, err
 	}
 
 	newUser, err := s.repository.Create(ctx, User{
@@ -84,7 +84,7 @@ func (s Service) Create(ctx context.Context, user User) (User, error) {
 func (s Service) CreateMetadataKey(ctx context.Context, key UserMetadataKey) (UserMetadataKey, error) {
 	currentUser, err := s.FetchCurrentUser(ctx)
 	if err != nil {
-		return UserMetadataKey{}, fmt.Errorf("%w: %s", ErrInvalidEmail, err.Error())
+		return UserMetadataKey{}, err
 	}
 
 	newUserMetadataKey, err := s.repository.CreateMetadataKey(ctx, UserMetadataKey{
@@ -122,7 +122,7 @@ func (s Service) List(ctx context.Context, flt Filter) (PagedUsers, error) {
 func (s Service) UpdateByID(ctx context.Context, toUpdate User) (User, error) {
 	currentUser, err := s.FetchCurrentUser(ctx)
 	if err != nil {
-		return User{}, fmt.Errorf("%w: %s", ErrInvalidEmail, err.Error())
+		return User{}, err
 	}
 
 	updatedUser, err := s.repository.UpdateByID(ctx, User{
@@ -150,7 +150,7 @@ func (s Service) UpdateByID(ctx context.Context, toUpdate User) (User, error) {
 func (s Service) UpdateByEmail(ctx context.Context, toUpdate User) (User, error) {
 	currentUser, err := s.FetchCurrentUser(ctx)
 	if err != nil {
-		return User{}, fmt.Errorf("%w: %s", ErrInvalidEmail, err.Error())
+		return User{}, err
 	}
 
 	updatedUser, err := s.repository.UpdateByEmail(ctx, User{
@@ -187,7 +187,12 @@ func (s Service) FetchCurrentUser(ctx context.Context) (User, error) {
 
 	fetchedUser, err := s.repository.GetByEmail(ctx, email)
 	if err != nil {
-		return User{}, err
+		switch err {
+		case ErrNotExist:
+			return User{}, fmt.Errorf("%w for email %s", ErrInvalidEmail, email)
+		default:
+			return User{}, err
+		}
 	}
 
 	return fetchedUser, nil

diff --git a/core/user/service_test.go b/core/user/service_test.go
@@ -10,10 +10,10 @@ import (
 
 	"github.com/goto/shield/core/activity"
 	"github.com/goto/shield/core/user"
+	"github.com/goto/shield/pkg/logger"
 	shieldlogger "github.com/goto/shield/pkg/logger"
 
 	"github.com/goto/shield/core/mocks"
-	"github.com/goto/shield/pkg/logger"
 )
 
 func TestService_Create(t *testing.T) {
@@ -82,6 +82,100 @@ func TestService_Create(t *testing.T) {
 	}
 }
 
+func TestService_CreateMetadataKey(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name        string
+		email       string
+		metadataKey user.UserMetadataKey
+		setup       func(t *testing.T) *user.Service
+		want        user.UserMetadataKey
+		wantErr     error
+	}{
+		{
+			name:  "CreateMetadataKey",
+			email: "[email protected]",
+			metadataKey: user.UserMetadataKey{
+				Key:         "test-key",
+				Description: "description for test-key",
+			},
+			setup: func(t *testing.T) *user.Service {
+				t.Helper()
+				repository := &mocks.Repository{}
+				activityService := &mocks.ActivityService{}
+				logger := shieldlogger.InitLogger(logger.Config{})
+				repository.EXPECT().
+					GetByEmail(mock.Anything, "[email protected]").
+					Return(user.User{}, nil)
+				repository.EXPECT().
+					CreateMetadataKey(mock.Anything, user.UserMetadataKey{
+						Key:         "test-key",
+						Description: "description for test-key"}).
+					Return(user.UserMetadataKey{
+						Key:         "test-key",
+						Description: "description for test-key"}, nil).Once()
+				activityService.EXPECT().
+					Log(mock.Anything, "user_metadata_key.create", activity.Actor{}, user.UserMetadataKeyLogData{
+						Entity:      "user_metadata_key",
+						Key:         "test-key",
+						Description: "description for test-key",
+					}).Return(nil).Once()
+				return user.NewService(logger, repository, activityService)
+			},
+			want: user.UserMetadataKey{
+				Key:         "test-key",
+				Description: "description for test-key",
+			},
+		},
+		{
+			name:  "CreateMetadataKeyError",
+			email: "[email protected]",
+			metadataKey: user.UserMetadataKey{
+				Key:         "test-key",
+				Description: "description for test-key",
+			},
+			setup: func(t *testing.T) *user.Service {
+				t.Helper()
+				logger := shieldlogger.InitLogger(logger.Config{})
+				activityService := &mocks.ActivityService{}
+				repository := &mocks.Repository{}
+				repository.EXPECT().
+					GetByEmail(mock.Anything, "[email protected]").
+					Return(user.User{}, nil)
+				repository.EXPECT().
+					CreateMetadataKey(mock.Anything, user.UserMetadataKey{
+						Key:         "test-key",
+						Description: "description for test-key",
+					}).
+					Return(user.UserMetadataKey{}, user.ErrConflict).Once()
+				return user.NewService(logger, repository, activityService)
+			},
+			wantErr: user.ErrConflict,
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			svc := tt.setup(t)
+
+			assert.NotNil(t, svc)
+
+			ctx := user.SetContextWithEmail(context.TODO(), "[email protected]")
+			got, err := svc.CreateMetadataKey(ctx, tt.metadataKey)
+			if tt.wantErr != nil {
+				assert.Error(t, err)
+				assert.True(t, errors.Is(err, tt.wantErr))
+			} else {
+				assert.NoError(t, err)
+			}
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
+
 func TestService_List(t *testing.T) {
 	t.Parallel()
 

diff --git a/internal/api/v1beta1/action.go b/internal/api/v1beta1/action.go
@@ -6,6 +6,7 @@ import (
 
 	"github.com/goto/shield/core/action"
 	"github.com/goto/shield/core/namespace"
+	"github.com/goto/shield/core/user"
 	shieldv1beta1 "github.com/goto/shield/proto/v1beta1"
 	grpczap "github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap"
 	"google.golang.org/grpc/codes"
@@ -59,6 +60,9 @@ func (h Handler) CreateAction(ctx context.Context, request *shieldv1beta1.Create
 			errors.Is(err, action.ErrInvalidDetail),
 			errors.Is(err, action.ErrInvalidID):
 			return nil, grpcBadBodyError
+		case errors.Is(err, user.ErrInvalidEmail),
+			errors.Is(err, user.ErrMissingEmail):
+			return nil, grpcUnauthenticated
 		default:
 			return nil, grpcInternalServerError
 		}
@@ -114,6 +118,9 @@ func (h Handler) UpdateAction(ctx context.Context, request *shieldv1beta1.Update
 		case errors.Is(err, namespace.ErrNotExist),
 			errors.Is(err, action.ErrInvalidDetail):
 			return nil, grpcBadBodyError
+		case errors.Is(err, user.ErrInvalidEmail),
+			errors.Is(err, user.ErrMissingEmail):
+			return nil, grpcUnauthenticated
 		default:
 			return nil, grpcInternalServerError
 		}

diff --git a/internal/api/v1beta1/group.go b/internal/api/v1beta1/group.go
@@ -94,7 +94,8 @@ func (h Handler) CreateGroup(ctx context.Context, request *shieldv1beta1.CreateG
 			return nil, grpcConflictError
 		case errors.Is(err, group.ErrInvalidDetail), errors.Is(err, organization.ErrNotExist), errors.Is(err, organization.ErrInvalidUUID):
 			return nil, grpcBadBodyError
-		case errors.Is(err, user.ErrInvalidEmail):
+		case errors.Is(err, user.ErrInvalidEmail),
+			errors.Is(err, user.ErrMissingEmail):
 			return nil, grpcUnauthenticated
 		default:
 			return nil, grpcInternalServerError
@@ -183,6 +184,9 @@ func (h Handler) UpdateGroup(ctx context.Context, request *shieldv1beta1.UpdateG
 			errors.Is(err, organization.ErrInvalidUUID),
 			errors.Is(err, organization.ErrNotExist):
 			return nil, grpcBadBodyError
+		case errors.Is(err, user.ErrInvalidEmail),
+			errors.Is(err, user.ErrMissingEmail):
+			return nil, grpcUnauthenticated
 		default:
 			return nil, grpcInternalServerError
 		}