MG - 1529 - User Removal

[rodneyosodo]

What type of PR is this?

This is a feature because it adds user removal

What does this do?

This PR adds a new method to delete a user with a given ID in the repository package. The implementation of this method uses an SQL query to delete the client from the database.

Additionally, an extra method has been added to the gRPC auth service to handle entity deletion. This is useful because it reduces network trips back and forth from the client microservices to the auth microservice. Test cases have been added for the delete method to ensure its functionality.

Which issue(s) does this PR fix/relate to?

• Resolves Add user removal and/or blocking feature #1529

Have you included tests for your changes?

Yes, I have included tests for my changes.

Did you document any new/modified feature?

Yes, I have updated the documentation for the new feature.

Notes

[arvindh123]

This below snippet will delete all Thing, Group, Channel relation related to user id within the domain

Suggested change

	policy := PolicyReq{
	Subject: EncodeDomainUserID(domain.ID, id),
	SubjectType: UserType,
	Object: domain.ID,
	ObjectType: DomainType,
	}
	if err := svc.agent.DeletePolicy(ctx, policy); err != nil {
	return err
	}
	policy := PolicyReq{
	Subject: EncodeDomainUserID(domain.ID, id),
	SubjectType: UserType,
	ObjectType: ThingType,
	}
	if err := svc.agent.DeletePolicy(ctx, policy); err != nil {
	return err
	}
	policy = PolicyReq{
	Subject: EncodeDomainUserID(domain.ID, id),
	SubjectType: UserType,
	ObjectType: GroupType,
	}
	if err := svc.agent.DeletePolicy(ctx, policy); err != nil {
	return err
	}
	policy = PolicyReq{
	Subject: EncodeDomainUserID(domain.ID, id),
	SubjectType: UserType,
	Object: domain.ID,
	ObjectType: DomainType,
	}
	if err := svc.agent.DeletePolicy(ctx, policy); err != nil {
	return err
	}

[arvindh123]

There is no need to remove policy between user_id and things/groups, because,There will be no subject just with user id in spicedb.
User's Subject id in spicesdb will be always combination <domain_id>_<user_id>

Suggested change

	req := PolicyReq{
	Subject: id,
	SubjectType: UserType,
	ObjectType: ThingType,
	}
	if err := svc.agent.DeletePolicy(ctx, req); err != nil {
	return err
	}
	req.ObjectType = GroupType
	if err := svc.agent.DeletePolicy(ctx, req); err != nil {
	return err
	}
	req.ObjectType = DomainType
	if err := svc.agent.DeletePolicy(ctx, req); err != nil {
	return err
	}
	req := PolicyReq{
	Subject: id,
	SubjectType: UserType,
	ObjectType: PlatformType,
	req.Object = MagistralaObject
	}
	if err := svc.agent.DeletePolicy(ctx, req); err != nil {
	return err
	}

[rodneyosodo]

If we leave it spiceDB will see it as a valid policy and will not clean up

[arvindh123]

Hello @rodneyosodo left some comments for delete policies in auth service.

[arvindh123]

@rodneyosodo @dborovcanin

For Delete users , Instead of deleting the user policy & removing from repo on Delete request,

I propose some thing like below:

On Delete request , change the state of user to delete in repo for the user.
Then we can delete the user with a background event handler / task written in GO , not using postgres extension, these handler will remove policies and user from repo

This gives advantage like,

• If user present in large domains and owns large entities , if we do on Delete request it will take time return response , the requesting doing the delete process
• During the delete , if any error happens, remain policy stay on spiceDB, So still user have access to them.

We can even have configuration for background user removal handler to start the removal operation after certain period. Example, background handler will remove after 30 days when user status changes to delete

We will have new challenges on this approach,
When user state changes to delete, If user signup again back to platform before the background user removal handler starts its operation to remove the user, then user might not allowed to signup.
Because in users Database we have unique contraint for email and username.

To overcome this issue, when user while chaning the user state delete, at the same time, we can make the email and username field in database empty and move the existing value to some other column like email_before_delete and username_before_delete . This helps to keep the record of username and email.

[arvindh123]

Suggested change

	case GroupType:
	// Remove policy of child groups
	req := PolicyReq{
	SubjectType: GroupType,
	Subject: id,
	ObjectType: GroupType,
	}
	if err := svc.DeletePolicy(ctx, req); err != nil {
	return err
	}
	// Remove policy of things
	req.ObjectType = ThingType
	if err := svc.DeletePolicy(ctx, req); err != nil {
	return err
	}
	// Remove policy from domain
	req.SubjectType = DomainType
	req.ObjectType = GroupType
	if err := svc.DeletePolicy(ctx, req); err != nil {
	return err
	}
	// Remove policy of users
	req.SubjectType = UserType
	if err := svc.DeletePolicy(ctx, req); err != nil {
	return err
	}
	case GroupType:
	// Remove policies where group is subject and object is another child group.
	req := PolicyReq{
	SubjectType: GroupType,
	Subject: id,
	ObjectType: GroupType,
	}
	if err := svc.DeletePolicy(ctx, req); err != nil {
	return err
	}
	// Remove policies where group is subject and object is thing.
	req.ObjectType = ThingType
	if err := svc.DeletePolicy(ctx, req); err != nil {
	return err
	}
	// Remove all policies where group is object.
	req = PolicyReq{
	Object: id,
	ObjectType: GroupType,
	}
	if err := svc.DeletePolicy(ctx, req); err != nil {
	return err
	}

The above suggestin will work , if we have changes in svc.agent.DeletePolicy show below

func (pa *policyAgent) DeletePolicy(ctx context.Context, pr auth.PolicyReq) error {
	req := &v1.DeleteRelationshipsRequest{
		RelationshipFilter: &v1.RelationshipFilter{
			ResourceType:       pr.ObjectType,
			OptionalResourceId: pr.Object,
		},
	}

	if pr.Relation != "" {
		req.RelationshipFilter.OptionalRelation = pr.Relation
	}

	if pr.SubjectType != "" {
		req.RelationshipFilter.OptionalSubjectFilter = &v1.SubjectFilter{
			SubjectType: pr.SubjectType,
		}
		if pr.Subject != "" {
			req.RelationshipFilter.OptionalSubjectFilter.OptionalSubjectId = pr.Subject
		}
		if pr.SubjectRelation != "" {
			req.RelationshipFilter.OptionalSubjectFilter.OptionalRelation = &v1.SubjectFilter_RelationFilter{
				Relation: pr.SubjectRelation,
			}
		}
	}

	if _, err := pa.permissionClient.DeleteRelationships(ctx, req); err != nil {
		return errors.Wrap(errRemovePolicies, handleSpicedbError(err))
	}
	return nil
}

[rodneyosodo]

This automatically deletes the other group entities

[arvindh123]

DeleteStatus can be highest status , If we want to hide the Deleted users from List with status=all http://localhost/users?status=all,
If DeleteStatus becmes higher status, then superadmin can get deleted user by explicitly like status=deleted

[rodneyosodo]

Highest status meaning 0 or n?

[arvindh123]

n