diff --git a/api/managementpb/role/role.pb.go b/api/managementpb/role/role.pb.go index 608b6f93cb..2ec5f9dbf5 100644 --- a/api/managementpb/role/role.pb.go +++ b/api/managementpb/role/role.pb.go @@ -249,6 +249,8 @@ type DeleteRoleRequest struct { unknownFields protoimpl.UnknownFields RoleId uint32 `protobuf:"varint,1,opt,name=role_id,json=roleId,proto3" json:"role_id,omitempty"` + // Role ID to be used as a replacement for the role. Additional logic applies. + ReplacementRoleId uint32 `protobuf:"varint,2,opt,name=replacement_role_id,json=replacementRoleId,proto3" json:"replacement_role_id,omitempty"` } func (x *DeleteRoleRequest) Reset() { @@ -290,6 +292,13 @@ func (x *DeleteRoleRequest) GetRoleId() uint32 { return 0 } +func (x *DeleteRoleRequest) GetReplacementRoleId() uint32 { + if x != nil { + return x.ReplacementRoleId + } + return 0 +} + type DeleteRoleResponse struct { state protoimpl.MessageState sizeCache protoimpl.SizeCache @@ -814,10 +823,13 @@ var file_managementpb_role_role_proto_rawDesc = []byte{ 0x65, 0x73, 0x63, 0x72, 0x69, 0x70, 0x74, 0x69, 0x6f, 0x6e, 0x18, 0x04, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0b, 0x64, 0x65, 0x73, 0x63, 0x72, 0x69, 0x70, 0x74, 0x69, 0x6f, 0x6e, 0x22, 0x14, 0x0a, 0x12, 0x55, 0x70, 0x64, 0x61, 0x74, 0x65, 0x52, 0x6f, 0x6c, 0x65, 0x52, 0x65, 0x73, 0x70, 0x6f, - 0x6e, 0x73, 0x65, 0x22, 0x34, 0x0a, 0x11, 0x44, 0x65, 0x6c, 0x65, 0x74, 0x65, 0x52, 0x6f, 0x6c, + 0x6e, 0x73, 0x65, 0x22, 0x64, 0x0a, 0x11, 0x44, 0x65, 0x6c, 0x65, 0x74, 0x65, 0x52, 0x6f, 0x6c, 0x65, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x12, 0x1f, 0x0a, 0x07, 0x72, 0x6f, 0x6c, 0x65, 0x5f, 0x69, 0x64, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0d, 0x42, 0x06, 0xe2, 0xdf, 0x1f, 0x02, 0x10, - 0x00, 0x52, 0x06, 0x72, 0x6f, 0x6c, 0x65, 0x49, 0x64, 0x22, 0x14, 0x0a, 0x12, 0x44, 0x65, 0x6c, + 0x00, 0x52, 0x06, 0x72, 0x6f, 0x6c, 0x65, 0x49, 0x64, 0x12, 0x2e, 0x0a, 0x13, 0x72, 0x65, 0x70, + 0x6c, 0x61, 0x63, 0x65, 0x6d, 0x65, 0x6e, 0x74, 0x5f, 0x72, 0x6f, 0x6c, 0x65, 0x5f, 0x69, 0x64, + 0x18, 0x02, 0x20, 0x01, 0x28, 0x0d, 0x52, 0x11, 0x72, 0x65, 0x70, 0x6c, 0x61, 0x63, 0x65, 0x6d, + 0x65, 0x6e, 0x74, 0x52, 0x6f, 0x6c, 0x65, 0x49, 0x64, 0x22, 0x14, 0x0a, 0x12, 0x44, 0x65, 0x6c, 0x65, 0x74, 0x65, 0x52, 0x6f, 0x6c, 0x65, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x22, 0x31, 0x0a, 0x0e, 0x47, 0x65, 0x74, 0x52, 0x6f, 0x6c, 0x65, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x12, 0x1f, 0x0a, 0x07, 0x72, 0x6f, 0x6c, 0x65, 0x5f, 0x69, 0x64, 0x18, 0x01, 0x20, 0x01, diff --git a/api/managementpb/role/role.proto b/api/managementpb/role/role.proto index edadccff6f..c6c210aab0 100644 --- a/api/managementpb/role/role.proto +++ b/api/managementpb/role/role.proto @@ -44,6 +44,8 @@ message DeleteRoleRequest { int_gt: 0 } ]; + // Role ID to be used as a replacement for the role. Additional logic applies. + uint32 replacement_role_id = 2; } message DeleteRoleResponse {} diff --git a/managed/models/role_helpers.go b/managed/models/role_helpers.go index 99c25352de..9abcd2ab21 100644 --- a/managed/models/role_helpers.go +++ b/managed/models/role_helpers.go @@ -27,8 +27,6 @@ import ( var ( // ErrRoleNotFound is returned when a role is not found. ErrRoleNotFound = fmt.Errorf("RoleNotFound") - // ErrRoleIsAssigned is returned when a role is assigned to a user and cannot be removed. - ErrRoleIsAssigned = fmt.Errorf("RoleIsAssigned") // ErrRoleIsDefaultRole is returned when trying to delete a default role. ErrRoleIsDefaultRole = fmt.Errorf("RoleIsDefaultRole") ) @@ -81,7 +79,7 @@ func AssignDefaultRole(tx *reform.TX, userID int) error { } // DeleteRole deletes a role, if possible. -func DeleteRole(tx *reform.TX, roleID int) error { +func DeleteRole(tx *reform.TX, roleID, replacementRoleID int) error { q := tx.Querier var role Role @@ -89,6 +87,7 @@ func DeleteRole(tx *reform.TX, roleID int) error { return err } + // Check if it's the default role. settings, err := GetSettings(tx) if err != nil { return err @@ -98,15 +97,10 @@ func DeleteRole(tx *reform.TX, roleID int) error { return ErrRoleIsDefaultRole } - s, err := q.FindOneFrom(UserRolesView, "role_id", roleID) - if err != nil && !errors.As(err, &reform.ErrNoRows) { + if err := replaceRole(tx, roleID, replacementRoleID); err != nil { return err } - if s != nil { - return ErrRoleIsAssigned - } - if err := q.Delete(&role); err != nil { if errors.As(err, &reform.ErrNoRows) { return ErrRoleNotFound @@ -118,6 +112,72 @@ func DeleteRole(tx *reform.TX, roleID int) error { return nil } +func replaceRole(tx *reform.TX, roleID, newRoleID int) error { + q := tx.Querier + + // If no new role is assigned, we remove all role assignements. + if newRoleID == 0 { + _, err := q.DeleteFrom(UserRolesView, "WHERE role_id = $1", roleID) + return err + } + + if err := FindAndLockRole(tx, newRoleID, &Role{}); err != nil { + return err + } + + // Check if the role is the last role for a user and apply special logic if it is. + structs, err := q.FindAllFrom(UserRolesView, "role_id", roleID) + if err != nil { + return err + } + + for _, s := range structs { + ur, ok := s.(*UserRoles) + if !ok { + return fmt.Errorf("invalid data structure in user roles for role ID %d. Found %+v", roleID, s) + } + + roleStructs, err := q.SelectAllFrom(UserRolesView, "WHERE user_id = $1 FOR UPDATE", ur.UserID) + if err != nil { + return err + } + + // If there are more than 1 roles, we remove the role without a replacement. + if len(roleStructs) > 1 { + _, err := q.DeleteFrom(UserRolesView, "WHERE user_id = $1 AND role_id = $2", ur.UserID, roleID) + if err != nil { + return err + } + + continue + } + + // The removed role is the last one. We replace it with a new role. + _, err = q.Exec(` + UPDATE user_roles + SET + role_id = $1 + WHERE + user_id = $2 AND + role_id = $3 AND + NOT EXISTS( + SELECT 1 + FROM user_roles ur + WHERE + ur.user_id = $2 AND + ur.role_id = $1 + )`, + newRoleID, + ur.UserID, + roleID) + if err != nil { + return err + } + } + + return nil +} + // FindAndLockRole retrieves a role by ID and locks it for update. func FindAndLockRole(tx *reform.TX, roleID int, role *Role) error { err := tx.Querier.SelectOneTo(role, "WHERE id = $1 FOR UPDATE", roleID) diff --git a/managed/models/role_helpers_test.go b/managed/models/role_helpers_test.go index 3044418e0a..17394c4ff6 100644 --- a/managed/models/role_helpers_test.go +++ b/managed/models/role_helpers_test.go @@ -27,6 +27,11 @@ import ( "github.com/percona/pmm/managed/utils/testdb" ) +const ( + roleATitle = "Role A" + roleBTitle = "Role B" +) + //nolint:paralleltest func TestRoleHelpers(t *testing.T) { sqlDB := testdb.Open(t, models.SkipFixtures, nil) @@ -72,7 +77,7 @@ func TestRoleHelpers(t *testing.T) { }) //nolint:paralleltest - t.Run("shall throw on role not found", func(t *testing.T) { + t.Run("shall throw on assigning non-existent role", func(t *testing.T) { tx, teardown := setup(t) defer teardown(t) @@ -81,7 +86,7 @@ func TestRoleHelpers(t *testing.T) { }) //nolint:paralleltest - t.Run("shall create new user", func(t *testing.T) { + t.Run("shall create a new user on role assign", func(t *testing.T) { tx, teardown := setup(t) defer teardown(t) @@ -94,27 +99,109 @@ func TestRoleHelpers(t *testing.T) { }) //nolint:paralleltest - t.Run("shall delete role", func(t *testing.T) { - tx, teardown := setup(t) - defer teardown(t) - - var role models.Role - role.Title = "Role A" - require.NoError(t, models.CreateRole(tx.Querier, &role)) - require.NoError(t, models.DeleteRole(tx, int(role.ID))) + t.Run("unassigned role", func(t *testing.T) { + //nolint:paralleltest + t.Run("shall delete role with no replacement", func(t *testing.T) { + tx, teardown := setup(t) + defer teardown(t) + + var role models.Role + role.Title = roleATitle + require.NoError(t, models.CreateRole(tx.Querier, &role)) + require.NoError(t, models.DeleteRole(tx, int(role.ID), 0)) + }) + + //nolint:paralleltest + t.Run("shall delete role with replacement", func(t *testing.T) { + tx, teardown := setup(t) + defer teardown(t) + + var roleA, roleB models.Role + roleA.Title = roleATitle + roleB.Title = roleBTitle + require.NoError(t, models.CreateRole(tx.Querier, &roleA)) + require.NoError(t, models.CreateRole(tx.Querier, &roleB)) + require.NoError(t, models.DeleteRole(tx, int(roleA.ID), int(roleB.ID))) + }) }) //nolint:paralleltest - t.Run("shall not delete if role is assigned", func(t *testing.T) { - tx, teardown := setup(t) - defer teardown(t) - - var role models.Role - require.NoError(t, models.CreateRole(tx.Querier, &role)) - require.NoError(t, models.AssignRoles(tx, 24, []int{int(role.ID)})) + t.Run("single role assigned", func(t *testing.T) { + //nolint:paralleltest + t.Run("shall delete role with no replacement", func(t *testing.T) { + tx, teardown := setup(t) + defer teardown(t) + + var role models.Role + role.Title = roleATitle + require.NoError(t, models.CreateRole(tx.Querier, &role)) + require.NoError(t, models.AssignRoles(tx, userID, []int{int(role.ID)})) + require.NoError(t, models.DeleteRole(tx, int(role.ID), 0)) + + roles, err := models.GetUserRoles(tx.Querier, userID) + require.NoError(t, err) + require.Equal(t, len(roles), 0) + }) + + //nolint:paralleltest + t.Run("shall delete role with replacement", func(t *testing.T) { + tx, teardown := setup(t) + defer teardown(t) + + var roleA, roleB models.Role + roleA.Title = roleATitle + roleB.Title = roleBTitle + require.NoError(t, models.CreateRole(tx.Querier, &roleA)) + require.NoError(t, models.CreateRole(tx.Querier, &roleB)) + require.NoError(t, models.AssignRoles(tx, userID, []int{int(roleA.ID)})) + require.NoError(t, models.DeleteRole(tx, int(roleA.ID), int(roleB.ID))) + + roles, err := models.GetUserRoles(tx.Querier, userID) + require.NoError(t, err) + require.Equal(t, len(roles), 1) + require.Equal(t, roles[0].ID, roleB.ID) + }) + }) - err := models.DeleteRole(tx, int(role.ID)) - require.Equal(t, err, models.ErrRoleIsAssigned) + //nolint:paralleltest + t.Run("multiple roles assigned", func(t *testing.T) { + //nolint:paralleltest + t.Run("shall delete role with no replacement", func(t *testing.T) { + tx, teardown := setup(t) + defer teardown(t) + + var roleA, roleB models.Role + roleA.Title = roleATitle + roleB.Title = roleBTitle + require.NoError(t, models.CreateRole(tx.Querier, &roleA)) + require.NoError(t, models.CreateRole(tx.Querier, &roleB)) + require.NoError(t, models.AssignRoles(tx, userID, []int{int(roleA.ID), int(roleB.ID)})) + require.NoError(t, models.DeleteRole(tx, int(roleA.ID), 0)) + + roles, err := models.GetUserRoles(tx.Querier, userID) + require.NoError(t, err) + require.Equal(t, len(roles), 1) + require.Equal(t, roles[0].ID, roleB.ID) + }) + + //nolint:paralleltest + t.Run("shall delete role with replacement", func(t *testing.T) { + tx, teardown := setup(t) + defer teardown(t) + + var roleA, roleB models.Role + roleA.Title = roleATitle + roleB.Title = roleBTitle + require.NoError(t, models.CreateRole(tx.Querier, &roleA)) + require.NoError(t, models.CreateRole(tx.Querier, &roleB)) + require.NoError(t, models.AssignRoles(tx, userID, []int{int(roleA.ID), int(roleB.ID)})) + require.NoError(t, models.DeleteRole(tx, int(roleA.ID), int(roleB.ID))) + + roles, err := models.GetUserRoles(tx.Querier, userID) + require.NoError(t, err) + require.Equal(t, len(roles), 1) + require.Equal(t, roles[0].ID, roleB.ID) + }) }) //nolint:paralleltest @@ -127,7 +214,7 @@ func TestRoleHelpers(t *testing.T) { require.NoError(t, models.AssignRoles(tx, 24, []int{int(role.ID)})) require.NoError(t, models.ChangeDefaultRole(tx, int(role.ID))) - err := models.DeleteRole(tx, int(role.ID)) + err := models.DeleteRole(tx, int(role.ID), 0) require.Equal(t, err, models.ErrRoleIsDefaultRole) }) @@ -137,8 +224,8 @@ func TestRoleHelpers(t *testing.T) { defer teardown(t) var roleA, roleB models.Role - roleA.Title = "Role A" - roleB.Title = "Role B" + roleA.Title = roleATitle + roleB.Title = roleBTitle require.NoError(t, models.CreateRole(tx.Querier, &roleA)) require.NoError(t, models.CreateRole(tx.Querier, &roleB)) diff --git a/managed/services/management/role.go b/managed/services/management/role.go index 2670e1d1b0..43de627e6a 100644 --- a/managed/services/management/role.go +++ b/managed/services/management/role.go @@ -91,7 +91,7 @@ func (r *RoleService) UpdateRole(_ context.Context, req *rolev1beta1.UpdateRoleR //nolint:unparam func (r *RoleService) DeleteRole(_ context.Context, req *rolev1beta1.DeleteRoleRequest) (*rolev1beta1.DeleteRoleResponse, error) { errTx := r.db.InTransaction(func(tx *reform.TX) error { - if err := models.DeleteRole(tx, int(req.RoleId)); err != nil { + if err := models.DeleteRole(tx, int(req.RoleId), int(req.ReplacementRoleId)); err != nil { if errors.Is(err, models.ErrRoleNotFound) { return status.Errorf(codes.NotFound, "Role not found") } diff --git a/update/ansible/playbook/tasks/update.yml b/update/ansible/playbook/tasks/update.yml index 486e767178..d5d2990801 100644 --- a/update/ansible/playbook/tasks/update.yml +++ b/update/ansible/playbook/tasks/update.yml @@ -14,6 +14,7 @@ - dbaas-tools - pmm2-client - pmm-dump + - vmproxy pre_tasks: - name: detect /srv/pmm-distribution stat: