NOISSUE - Remove redundant relation check

[WashingtonKK]

Signed-off-by: WashingtonKK [email protected]

What type of PR is this?

This is a refactor because it removes a redundant relation check when unassigning users from a domain.

What does this do?

Removes a redundant relation check when unassigning users from a domain.

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

No issue

Have you included tests for your changes?

No, the changes are covered by existing tests.

Did you document any new/modified feature?

No, the changes are already documented.

Notes

[arvindh123]

@WashingtonKK

We should not allow a user to have mutiple relationship with domain.

And we also should not allow SuperAdmin to create new relationship with domain .

So i suggesting below

Modification for addPolicyPreCondition in auth/spicedb/policies.go

func (pa *policyAgent) addPolicyPreCondition(ctx context.Context, pr auth.PolicyReq) ([]*v1.Precondition, error) {
	// Checks are required for following  ( -> means adding)
	// 1.) user -> group (both user groups and channels)
	// 2.) user -> thing
	// 3.) group -> group (both for adding parent_group and channels)
	// 4.) group (channel) -> thing
	// 5.) user -> domain

	switch {
	// 1.) user -> group (both user groups and channels)
	// Checks :
	// - USER with ANY RELATION to DOMAIN
	// - GROUP with DOMAIN RELATION to DOMAIN
	case pr.SubjectType == auth.UserType && pr.ObjectType == auth.GroupType:
		return pa.userGroupPreConditions(ctx, pr)

	// 2.) user -> thing
	// Checks :
	// - USER with ANY RELATION to DOMAIN
	// - THING with DOMAIN RELATION to DOMAIN
	case pr.SubjectType == auth.UserType && pr.ObjectType == auth.ThingType:
		return pa.userThingPreConditions(ctx, pr)

	// 3.) group -> group (both for adding parent_group and channels)
	// Checks :
	// - CHILD_GROUP with out PARENT_GROUP RELATION with any GROUP
	case pr.SubjectType == auth.GroupType && pr.ObjectType == auth.GroupType:
		return groupPreConditions(pr)

	// 4.) group (channel) -> thing
	// Checks :
	// - GROUP (channel) with DOMAIN RELATION to DOMAIN
	// - NO GROUP should not have PARENT_GROUP RELATION with GROUP (channel)
	// - THING with DOMAIN RELATION to DOMAIN
	case pr.SubjectType == auth.GroupType && pr.ObjectType == auth.ThingType:
		return channelThingPreCondition(pr)

	// 5.) user -> domain
	// Checks :
	// - User is doesn't have any relation with domain
	case pr.SubjectType == auth.UserType && pr.ObjectType == auth.DomainType:
		return pa.userDomainPreConditions(ctx, pr)

	// Check thing and group not belongs to other domain before adding to domain
	case pr.SubjectType == auth.DomainType && pr.Relation == auth.DomainRelation && (pr.ObjectType == auth.ThingType || pr.ObjectType == auth.GroupType):
		preconds := []*v1.Precondition{
			{
				Operation: v1.Precondition_OPERATION_MUST_NOT_MATCH,
				Filter: &v1.RelationshipFilter{
					ResourceType:       pr.ObjectType,
					OptionalResourceId: pr.Object,
					OptionalRelation:   auth.DomainRelation,
					OptionalSubjectFilter: &v1.SubjectFilter{
						SubjectType: auth.DomainType,
					},
				},
			},
		}
		return preconds, nil
	}
	return nil, nil
}

Addition of new function in in auth/spicedb/policies.go

func (pa *policyAgent) userDomainPreConditions(ctx context.Context, pr auth.PolicyReq) ([]*v1.Precondition, error) {
	var preconds []*v1.Precondition

	if err := pa.CheckPolicy(ctx, auth.PolicyReq{
		Subject:     pr.Subject,
		SubjectType: pr.SubjectType,
		Permission:  auth.AdminPermission,
		Object:      auth.MagistralaObject,
		ObjectType:  auth.PlatformType,
	}); err == nil {
		return preconds, fmt.Errorf("use already exists in domain")
	}

	// user should not have any relation with group
	preconds = append(preconds, &v1.Precondition{
		Operation: v1.Precondition_OPERATION_MUST_NOT_MATCH,
		Filter: &v1.RelationshipFilter{
			ResourceType:       auth.DomainType,
			OptionalResourceId: pr.Object,
			OptionalSubjectFilter: &v1.SubjectFilter{
				SubjectType:       auth.UserType,
				OptionalSubjectId: pr.Subject,
			},
		},
	})

	return preconds, nil
}

Modification of auth/postgres/init.go

// Copyright (c) Abstract Machines
// SPDX-License-Identifier: Apache-2.0

package postgres

import (
	_ "github.com/jackc/pgx/v5/stdlib" // required for SQL access
	migrate "github.com/rubenv/sql-migrate"
)

// Migration of Auth service.
func Migration() *migrate.MemoryMigrationSource {
	return &migrate.MemoryMigrationSource{
		Migrations: []*migrate.Migration{
			{
				Id: "auth_1",
				Up: []string{
					`CREATE TABLE IF NOT EXISTS keys (
                        id          VARCHAR(254) NOT NULL,
                        type        SMALLINT,
                        subject     VARCHAR(254) NOT NULL,
                        issuer_id   VARCHAR(254) NOT NULL,
                        issued_at   TIMESTAMP NOT NULL,
                        expires_at  TIMESTAMP,
                        PRIMARY KEY (id, issuer_id)
                    )`,

					`CREATE TABLE IF NOT EXISTS domains (
                        id          VARCHAR(36) PRIMARY KEY,
                        name        VARCHAR(254),
                        tags        TEXT[],
                        metadata    JSONB,
                        alias       VARCHAR(254) NULL UNIQUE,
                        created_at  TIMESTAMP,
                        updated_at  TIMESTAMP,
                        updated_by  VARCHAR(254),
                        created_by  VARCHAR(254),
                        status      SMALLINT NOT NULL DEFAULT 0 CHECK (status >= 0)
                    );`,
					`CREATE TABLE IF NOT EXISTS policies (
                        subject_type        VARCHAR(254) NOT NULL,
                        subject_id          VARCHAR(254) NOT NULL,
                        subject_relation    VARCHAR(254) NOT NULL,
                        relation            VARCHAR(254) NOT NULL,
                        object_type         VARCHAR(254) NOT NULL,
                        object_id           VARCHAR(254) NOT NULL,
                        CONSTRAINT unique_policy_constraint UNIQUE (subject_type, subject_id, subject_relation, object_type, object_id)
                    );`,
				},
				Down: []string{
					`DROP TABLE IF EXISTS keys`,
				},
			},
		},
	}
}

[WashingtonKK]

@arvindh123, yes - I think it is possible. Let me work on it, I will let you know in case I encounter any challenges.

[arvindh123]

let me explain this here case here for removing users from Domain

Example:

Doman name: domain_1

domain_1 Administrator: user_101 user_102

domain_1 Editor: user_1

domain_1 Viewer: user_2

domain_1 Member: user_3

user_1 (domain_1 Editor) should able to remove user_2 user_3 without giving relation in request JSON

{ 
    "user_ids": ["user_2", "user_3" ]
} 

if user_1 (domain_1 Editor) tries to remove user_2, user_3 , user_101 (domain administrator) , then the request should fails

{ 
    "user_ids": ["user_2", "user_3","user_100" ]
} 

[WashingtonKK]

if user_1 (domain_1 Editor) tries to remove user_2, user_3 , user_101 (domain administrator) , should it fail or just ignore the admin users and remove the other users?

{ 
    "user_ids": ["user_2", "user_3","user_100" ]
} 

[arvindh123]

if user_1 (domain_1 Editor) tries to remove user_2, user_3 , user_101 (domain administrator) , should it fail or just ignore the admin users and remove the other users?

{ 
    "user_ids": ["user_2", "user_3","user_100" ]
} 

It should fail.