Skip to content

refactor: rename service user variables #1802

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.

Sign up for GitHub

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

Open
RushikeshBhavsar3605 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

refactor: rename service user variables #1802

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
24 changes: 12 additions & 12 deletions core/serviceuser/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,15 +132,15 @@ func (s Service) GetByIDs(ctx context.Context, ids []string) ([]ServiceUser, err
}

func (s Service) ListByOrg(ctx context.Context, orgID string) ([]ServiceUser, error) {
userIDs, err := s.membershipService.ListPrincipalIDsByResource(ctx, orgID, schema.OrganizationNamespace, schema.ServiceUserPrincipal)
serviceUserIDs, err := s.membershipService.ListPrincipalIDsByResource(ctx, orgID, schema.OrganizationNamespace, schema.ServiceUserPrincipal)
if err != nil {
return nil, err
}
if len(userIDs) == 0 {
// no users
if len(serviceUserIDs) == 0 {
// no service users
return []ServiceUser{}, nil
}
return s.repo.GetByIDs(ctx, userIDs)
return s.repo.GetByIDs(ctx, serviceUserIDs)
}

func (s Service) Delete(ctx context.Context, id string) error {
Expand Down Expand Up @@ -456,7 +456,7 @@ func (s Service) FilterSudos(ctx context.Context, ids []string) ([]string, error

// Sudo add platform permissions to user
func (s Service) Sudo(ctx context.Context, id string, relationName string) error {
currentUser, err := s.Get(ctx, id)
serviceUser, err := s.Get(ctx, id)
if err != nil {
return err
}
Expand All @@ -471,7 +471,7 @@ func (s Service) Sudo(ctx context.Context, id string, relationName string) error
// Act on the exact relation, not the permission it grants: admin and member both
// grant `check`, so a permission check would skip adding member to an existing
// admin and break the admin->member downgrade. Safe to run again.
if ok, err := s.IsSudo(ctx, currentUser.ID, relationName); err != nil {
if ok, err := s.IsSudo(ctx, serviceUser.ID, relationName); err != nil {
return err
} else if ok {
return nil
Expand All @@ -483,7 +483,7 @@ func (s Service) Sudo(ctx context.Context, id string, relationName string) error
Namespace: schema.PlatformNamespace,
},
Subject: relation.Subject{
ID: currentUser.ID,
ID: serviceUser.ID,
Namespace: schema.ServiceUserPrincipal,
},
RelationName: relationName,
Expand All @@ -497,7 +497,7 @@ func (s Service) Sudo(ctx context.Context, id string, relationName string) error
if relationName == schema.MemberRelationName {
event = pkgAuditRecord.PlatformMemberAddedEvent
}
return s.recordPlatformAuditRecord(ctx, currentUser, event, relationName)
return s.recordPlatformAuditRecord(ctx, serviceUser, event, relationName)
}

// UnSudo removes a platform relation (admin or member) from a service user.
Expand All @@ -510,15 +510,15 @@ func (s Service) UnSudo(ctx context.Context, id, relationName string) error {
return fmt.Errorf("invalid relation name, possible options are: %s, %s", schema.MemberRelationName, schema.AdminRelationName)
}

currentUser, err := s.Get(ctx, id)
serviceUser, err := s.Get(ctx, id)
if err != nil {
return err
}

// Only act (and audit) when the specific relation actually exists, so the
// revoke event reflects a real state change. Checking the relation directly
// is precise for both admin and member.
present, err := s.IsSudo(ctx, currentUser.ID, relationName)
present, err := s.IsSudo(ctx, serviceUser.ID, relationName)
if err != nil {
return err
}
Expand All @@ -533,7 +533,7 @@ func (s Service) UnSudo(ctx context.Context, id, relationName string) error {
Namespace: schema.PlatformNamespace,
},
Subject: relation.Subject{
ID: currentUser.ID,
ID: serviceUser.ID,
Namespace: schema.ServiceUserPrincipal,
},
RelationName: relationName,
Expand All @@ -545,7 +545,7 @@ func (s Service) UnSudo(ctx context.Context, id, relationName string) error {
if relationName == schema.MemberRelationName {
event = pkgAuditRecord.PlatformMemberRemovedEvent
}
return s.recordPlatformAuditRecord(ctx, currentUser, event, relationName)
return s.recordPlatformAuditRecord(ctx, serviceUser, event, relationName)
}

// recordPlatformAuditRecord logs a platform admin/member grant or revoke on a service
Expand Down
16 changes: 8 additions & 8 deletions internal/api/v1beta1connect/project.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,23 +252,23 @@ func (h *ConnectHandler) ListProjectServiceUsers(ctx context.Context, request *c
}

suIDs := utils.Map(members, func(m membership.Member) string { return m.PrincipalID })
var users []serviceuser.ServiceUser
var serviceUsers []serviceuser.ServiceUser
if len(suIDs) > 0 {
users, err = h.serviceUserService.GetByIDs(ctx, suIDs)
serviceUsers, err = h.serviceUserService.GetByIDs(ctx, suIDs)
if err != nil {
return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("ListProjectServiceUsers.GetByIDs: project_id=%s: %w", prj.ID, err))
}
}

var transformedUsers []*frontierv1beta1.ServiceUser
var transformedServiceUsers []*frontierv1beta1.ServiceUser
rolePairPBs := []*frontierv1beta1.ListProjectServiceUsersResponse_RolePair{}
for _, a := range users {
u, err := transformServiceUserToPB(a)
for _, su := range serviceUsers {
serviceUserPB, err := transformServiceUserToPB(su)
if err != nil {
return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("ListProjectServiceUsers: entity_id=%s: %w", a.ID, err))
return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("ListProjectServiceUsers: entity_id=%s: %w", su.ID, err))
}

transformedUsers = append(transformedUsers, u)
transformedServiceUsers = append(transformedServiceUsers, serviceUserPB)
}

for _, m := range members {
Expand All @@ -289,7 +289,7 @@ func (h *ConnectHandler) ListProjectServiceUsers(ctx context.Context, request *c
}

return connect.NewResponse(&frontierv1beta1.ListProjectServiceUsersResponse{
Serviceusers: transformedUsers,
Serviceusers: transformedServiceUsers,
RolePairs: rolePairPBs,
}), nil
}
Expand Down
14 changes: 7 additions & 7 deletions internal/api/v1beta1connect/serviceuser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,25 +40,25 @@ func toJSONWebKey(keySet jwk.Set) (*JsonWebKeySet, error) {
}

func (h *ConnectHandler) ListServiceUsers(ctx context.Context, request *connect.Request[frontierv1beta1.ListServiceUsersRequest]) (*connect.Response[frontierv1beta1.ListServiceUsersResponse], error) {
var users []*frontierv1beta1.ServiceUser
usersList, err := h.serviceUserService.List(ctx, serviceuser.Filter{
var serviceUsers []*frontierv1beta1.ServiceUser
serviceUsersList, err := h.serviceUserService.List(ctx, serviceuser.Filter{
OrgID: request.Msg.GetOrgId(),
State: serviceuser.State(request.Msg.GetState()),
})
if err != nil {
return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("ListServiceUsers: org_id=%s state=%s: %w", request.Msg.GetOrgId(), request.Msg.GetState(), err))
}

for _, user := range usersList {
userPB, err := transformServiceUserToPB(user)
for _, su := range serviceUsersList {
serviceUserPB, err := transformServiceUserToPB(su)
if err != nil {
return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("ListServiceUsers: entity_id=%s: %w", user.ID, err))
return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("ListServiceUsers: entity_id=%s: %w", su.ID, err))
}
users = append(users, userPB)
serviceUsers = append(serviceUsers, serviceUserPB)
}

return connect.NewResponse(&frontierv1beta1.ListServiceUsersResponse{
Serviceusers: users,
Serviceusers: serviceUsers,
}), nil
}

Expand Down
16 changes: 8 additions & 8 deletions internal/store/postgres/serviceuser_repository.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,25 +206,25 @@ func (s ServiceUserRepository) GetByIDs(ctx context.Context, ids []string) ([]se
return nil, fmt.Errorf("%w: %s", errQuery, err)
}

var fetchedUsers []ServiceUser
var fetchedServiceUsers []ServiceUser
if err = s.dbc.WithTimeout(ctx, TABLE_SERVICEUSER, "Get", func(ctx context.Context) error {
return s.dbc.SelectContext(ctx, &fetchedUsers, query, params...)
return s.dbc.SelectContext(ctx, &fetchedServiceUsers, query, params...)
}); err != nil {
if errors.Is(err, sql.ErrNoRows) {
return nil, serviceuser.ErrNotExist
}
return nil, fmt.Errorf("%w: %s", errDB, err)
}

var transformedUsers []serviceuser.ServiceUser
for _, u := range fetchedUsers {
transformedUser, err := u.transform()
var transformedServiceUsers []serviceuser.ServiceUser
for _, su := range fetchedServiceUsers {
transformedServiceUser, err := su.transform()
if err != nil {
return nil, fmt.Errorf("failed to transform user: %w", err)
return nil, fmt.Errorf("failed to transform service user: %w", err)
}
transformedUsers = append(transformedUsers, transformedUser)
transformedServiceUsers = append(transformedServiceUsers, transformedServiceUser)
}
return transformedUsers, nil
return transformedServiceUsers, nil
}

// ListMissingOrgPolicy returns service users whose owning org has no matching
Expand Down
12 changes: 6 additions & 6 deletions test/e2e/regression/serviceusers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -783,8 +783,8 @@ func (s *ServiceUsersRegressionTestSuite) TestServiceUserAsPlatformMember() {
// check if we have su permissions by listing users
listUsersBeforeResp, err := s.testBench.AdminClient.ListPlatformUsers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.ListPlatformUsersRequest{}))
s.Assert().NoError(err)
s.Assert().False(utils.ContainsFunc(listUsersBeforeResp.Msg.GetServiceusers(), func(user *frontierv1beta1.ServiceUser) bool {
return user.GetId() == createServiceUserResp.Msg.GetServiceuser().GetId()
s.Assert().False(utils.ContainsFunc(listUsersBeforeResp.Msg.GetServiceusers(), func(serviceUser *frontierv1beta1.ServiceUser) bool {
return serviceUser.GetId() == createServiceUserResp.Msg.GetServiceuser().GetId()
}))

// make service user platform member
Expand All @@ -799,8 +799,8 @@ func (s *ServiceUsersRegressionTestSuite) TestServiceUserAsPlatformMember() {
s.Assert().NoError(err)
s.Assert().NotNil(listUsersResp)
s.Assert().Len(listUsersResp.Msg.GetUsers(), 1)
s.Assert().True(utils.ContainsFunc(listUsersResp.Msg.GetServiceusers(), func(user *frontierv1beta1.ServiceUser) bool {
return user.GetId() == createServiceUserResp.Msg.GetServiceuser().GetId()
s.Assert().True(utils.ContainsFunc(listUsersResp.Msg.GetServiceusers(), func(serviceUser *frontierv1beta1.ServiceUser) bool {
return serviceUser.GetId() == createServiceUserResp.Msg.GetServiceuser().GetId()
}))
})
s.Run("4. remove a service user in an org which was platform member", func() {
Expand Down Expand Up @@ -906,8 +906,8 @@ func (s *ServiceUsersRegressionTestSuite) TestServiceUserAsPlatformMember() {
listUsersResp, err := s.testBench.AdminClient.ListPlatformUsers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.ListPlatformUsersRequest{}))
s.Assert().NoError(err)
s.Assert().NotNil(listUsersResp)
s.Assert().True(utils.ContainsFunc(listUsersResp.Msg.GetServiceusers(), func(user *frontierv1beta1.ServiceUser) bool {
return user.GetId() == createServiceUserResp.Msg.GetServiceuser().GetId()
s.Assert().True(utils.ContainsFunc(listUsersResp.Msg.GetServiceusers(), func(serviceUser *frontierv1beta1.ServiceUser) bool {
return serviceUser.GetId() == createServiceUserResp.Msg.GetServiceuser().GetId()
}))

// superusers shouldn't be listed in non admin calls even if they have access
Expand Down