Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fetch user email error log #48

Merged
merged 5 commits into from
May 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions core/action/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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{
Expand Down
4 changes: 2 additions & 2 deletions core/group/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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) != "" {
Expand Down
4 changes: 2 additions & 2 deletions core/namespace/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions core/organization/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions core/policy/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions core/project/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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 != "" {
Expand Down
4 changes: 2 additions & 2 deletions core/relation/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions core/resource/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions core/role/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
15 changes: 10 additions & 5 deletions core/user/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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
mabdh marked this conversation as resolved.
Show resolved Hide resolved
}
}

return fetchedUser, nil
Expand Down
96 changes: 95 additions & 1 deletion core/user/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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()

Expand Down
7 changes: 7 additions & 0 deletions internal/api/v1beta1/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
6 changes: 5 additions & 1 deletion internal/api/v1beta1/group.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
Loading
Loading