fix: while updating role, do full refresh of permissions

Currently while updating role, frontier used to identify the deleted permissions from role and only remove them which are needed. One problem with this is if in case someone manually modifies such roles and the permissions gets out of sync, it will not try to correct them. Now it deletes all existing permissions and create the new requested on every update call. Signed-off-by: Kush Sharma <[email protected]>
raystack · Nov 6, 2024 · dc8d9bd · dc8d9bd
1 parent 6b4d111
commit dc8d9bd
Show file tree

Hide file tree

Showing 7 changed files with 43 additions and 51 deletions.
diff --git a/core/role/service.go b/core/role/service.go
@@ -97,42 +97,38 @@ func (s Service) createRolePermissionRelation(ctx context.Context, roleID string
 	return nil
 }
 
-func (s Service) deleteRolePermissionRelation(ctx context.Context, roleID string, permissions []string) error {
+func (s Service) deleteRolePermissionRelations(ctx context.Context, roleID string) error {
 	// delete relation between role and permissions
 	// for example for each permission:
 	// app/role:org_owner#organization_delete@app/user:*
 	// app/role:org_owner#organization_update@app/user:*
 	// this needs to be created for each type of principles
-	for _, perm := range permissions {
-		err := s.relationService.Delete(ctx, relation.Relation{
-			Object: relation.Object{
-				ID:        roleID,
-				Namespace: schema.RoleNamespace,
-			},
-			Subject: relation.Subject{
-				ID:        "*", // all principles who have role will have access
-				Namespace: schema.UserPrincipal,
-			},
-			RelationName: perm,
-		})
-		if err != nil {
-			return err
-		}
-		// do the same with service user
-		err = s.relationService.Delete(ctx, relation.Relation{
-			Object: relation.Object{
-				ID:        roleID,
-				Namespace: schema.RoleNamespace,
-			},
-			Subject: relation.Subject{
-				ID:        "*", // all principles who have role will have access
-				Namespace: schema.ServiceUserPrincipal,
-			},
-			RelationName: perm,
-		})
-		if err != nil {
-			return err
-		}
+	err := s.relationService.Delete(ctx, relation.Relation{
+		Object: relation.Object{
+			ID:        roleID,
+			Namespace: schema.RoleNamespace,
+		},
+		Subject: relation.Subject{
+			ID:        "*", // all principles who have role will have access
+			Namespace: schema.UserPrincipal,
+		},
+	})
+	if err != nil {
+		return err
+	}
+	// do the same with service user
+	err = s.relationService.Delete(ctx, relation.Relation{
+		Object: relation.Object{
+			ID:        roleID,
+			Namespace: schema.RoleNamespace,
+		},
+		Subject: relation.Subject{
+			ID:        "*", // all principles who have role will have access
+			Namespace: schema.ServiceUserPrincipal,
+		},
+	})
+	if err != nil {
+		return err
 	}
 	return nil
 }
@@ -165,16 +161,8 @@ func (s Service) Update(ctx context.Context, toUpdate Role) (Role, error) {
 		return Role{}, err
 	}
 
-	// figure out what to delete from permission relation
-	var permissionsToDelete []string
-	for _, perm := range existingRole.Permissions {
-		if !utils.Contains(toUpdate.Permissions, perm) {
-			permissionsToDelete = append(permissionsToDelete, perm)
-		}
-	}
-
-	// delete relation between role and permissions
-	if err := s.deleteRolePermissionRelation(ctx, existingRole.ID, permissionsToDelete); err != nil {
+	// delete all existing relation between role and permissions
+	if err := s.deleteRolePermissionRelations(ctx, existingRole.ID); err != nil {
 		return Role{}, err
 	}
 

diff --git a/internal/api/v1beta1/billing_checkout.go b/internal/api/v1beta1/billing_checkout.go
@@ -128,7 +128,7 @@ func (h Handler) CreateCheckout(ctx context.Context, request *frontierv1beta1.Cr
 	})
 	if err != nil {
 		if errors.Is(err, product.ErrPerSeatLimitReached) {
-			return nil, status.Errorf(codes.InvalidArgument, err.Error())
+			return nil, status.Errorf(codes.InvalidArgument, "%v", err)
 		}
 		return nil, err
 	}

diff --git a/internal/api/v1beta1/billing_customer.go b/internal/api/v1beta1/billing_customer.go
@@ -74,7 +74,7 @@ func (h Handler) CreateBillingAccount(ctx context.Context, request *frontierv1be
 	}, request.GetOffline())
 	if err != nil {
 		if errors.Is(err, customer.ErrActiveConflict) {
-			return nil, status.Errorf(codes.FailedPrecondition, err.Error())
+			return nil, status.Errorf(codes.FailedPrecondition, "%v", err)
 		}
 		return nil, err
 	}

diff --git a/internal/api/v1beta1/deleter.go b/internal/api/v1beta1/deleter.go
@@ -17,14 +17,14 @@ type CascadeDeleter interface {
 
 func (h Handler) DeleteProject(ctx context.Context, request *frontierv1beta1.DeleteProjectRequest) (*frontierv1beta1.DeleteProjectResponse, error) {
 	if err := h.deleterService.DeleteProject(ctx, request.GetId()); err != nil {
-		return nil, status.Errorf(codes.Internal, err.Error())
+		return nil, status.Errorf(codes.Internal, "%v", err)
 	}
 	return &frontierv1beta1.DeleteProjectResponse{}, nil
 }
 
 func (h Handler) DeleteOrganization(ctx context.Context, request *frontierv1beta1.DeleteOrganizationRequest) (*frontierv1beta1.DeleteOrganizationResponse, error) {
 	if err := h.deleterService.DeleteOrganization(ctx, request.GetId()); err != nil {
-		return nil, status.Errorf(codes.Internal, err.Error())
+		return nil, status.Errorf(codes.Internal, "%v", err)
 	}
 	return &frontierv1beta1.DeleteOrganizationResponse{}, nil
 }
diff --git a/internal/store/postgres/relation_repository.go b/internal/store/postgres/relation_repository.go
@@ -32,6 +32,8 @@ func (r RelationRepository) Upsert(ctx context.Context, relationToCreate relatio
 			"object_namespace_name":    relationToCreate.Object.Namespace,
 			"object_id":                relationToCreate.Object.ID,
 			"relation_name":            relationToCreate.RelationName,
+			"created_at":               goqu.L("now()"),
+			"updated_at":               goqu.L("now()"),
 		}).OnConflict(
 		goqu.DoUpdate("subject_namespace_name, subject_id, object_namespace_name, object_id, relation_name", goqu.Record{
 			"subject_namespace_name": relationToCreate.Subject.Namespace,

diff --git a/internal/store/postgres/relation_repository_test.go b/internal/store/postgres/relation_repository_test.go
@@ -209,7 +209,7 @@ func (s *RelationRepositoryTestSuite) TestUpsert() {
 				"ID",
 				"CreatedAt",
 				"UpdatedAt")) {
-				s.T().Fatalf(cmp.Diff(got, tc.ExpectedRelation))
+				s.T().Fatal(cmp.Diff(got, tc.ExpectedRelation))
 			}
 		})
 	}
@@ -272,7 +272,7 @@ func (s *RelationRepositoryTestSuite) TestList() {
 				sort.Slice(tc.ExpectedRelations, func(i, j int) bool {
 					return tc.ExpectedRelations[i].RelationName < tc.ExpectedRelations[j].RelationName
 				})
-				s.T().Fatalf(cmp.Diff(got, tc.ExpectedRelations))
+				s.T().Fatal(cmp.Diff(got, tc.ExpectedRelations))
 			}
 		})
 	}

diff --git a/internal/store/postgres/user_repository.go b/internal/store/postgres/user_repository.go
@@ -114,10 +114,12 @@ func (r UserRepository) Create(ctx context.Context, usr user.User) (user.User, e
 	}
 
 	insertRow := goqu.Record{
-		"name":   strings.ToLower(usr.Name),
-		"email":  strings.ToLower(usr.Email),
-		"title":  usr.Title,
-		"avatar": usr.Avatar,
+		"name":       strings.ToLower(usr.Name),
+		"email":      strings.ToLower(usr.Email),
+		"title":      usr.Title,
+		"avatar":     usr.Avatar,
+		"created_at": goqu.L("now()"),
+		"updated_at": goqu.L("now()"),
 	}
 	if usr.Metadata != nil {
 		marshaledMetadata, err := json.Marshal(usr.Metadata)