refactor: add validation logic before removing role or permission

Author: SalmaElsoly

pkg/errors.go

@@ -4,6 +4,11 @@ var (
 	ErrAlreadyExists           = errorString("already exists")
 	ErrInvalidResourceOrAction = errorString("invalid resource or action")
 	ErrInvalidName             = errorString("invalid name")
+	ErrNotFound                = errorString("not found")
+	ErrRoleInUse               = errorString("role is in use and cannot be removed")
+	ErrPermissionInUse         = errorString("permission is in use and cannot be removed")
+	ErrDuplicateRole           = errorString("role with this name already exists")
+	ErrDuplicatePermission     = errorString("permission with this resource and action already exists")
 )
 
 type errorString string

pkg/rbac.go

@@ -43,6 +43,15 @@ func (r *RBAC) CreateRole(ctx context.Context, name, description string) error {
 	if n == "" {
 		return ErrInvalidName
 	}
+
+	exists, err := r.roleNameExists(ctx, n)
+	if err != nil {
+		return err
+	}
+	if exists {
+		return ErrDuplicateRole
+	}
+
 	role := Role{ID: uuid.New().String(), Name: n, Description: description}
 	return r.store.CreateRole(ctx, role)
 }
@@ -51,6 +60,19 @@ func (r *RBAC) RemoveRole(ctx context.Context, roleID string) error {
 	if err := validateUUIDs(roleID); err != nil {
 		return err
 	}
+
+	if _, err := r.store.GetRole(ctx, roleID); err != nil {
+		return ErrNotFound
+	}
+
+	inUse, err := r.isRoleInUse(ctx, roleID)
+	if err != nil {
+		return err
+	}
+	if inUse {
+		return ErrRoleInUse
+	}
+
 	return r.store.RemoveRole(ctx, roleID)
 }
 
@@ -60,6 +82,15 @@ func (r *RBAC) CreatePermission(ctx context.Context, resource, action string) er
 	if res == "" || a == "" {
 		return ErrInvalidResourceOrAction
 	}
+
+	exists, err := r.permissionExists(ctx, res, a)
+	if err != nil {
+		return err
+	}
+	if exists {
+		return ErrDuplicatePermission
+	}
+
 	p := Permission{ID: uuid.New().String(), Resource: res, Action: a}
 	return r.store.CreatePermission(ctx, p)
 }
@@ -68,20 +99,64 @@ func (r *RBAC) RemovePermission(ctx context.Context, permID string) error {
 	if err := validateUUIDs(permID); err != nil {
 		return err
 	}
+
+	if _, err := r.store.GetPermission(ctx, permID); err != nil {
+		return ErrNotFound
+	}
+
+	inUse, err := r.isPermissionInUse(ctx, permID)
+	if err != nil {
+		return err
+	}
+	if inUse {
+		return ErrPermissionInUse
+	}
+
 	return r.store.RemovePermission(ctx, permID)
 }
 
 func (r *RBAC) AssignRole(ctx context.Context, subjectID, roleID string) error {
 	if err := validateUUIDs(roleID); err != nil {
 		return err
 	}
+
+	if _, err := r.store.GetRole(ctx, roleID); err != nil {
+		return ErrNotFound
+	}
+
+	roles, err := r.store.ListSubjectRoles(ctx, subjectID)
+	if err != nil {
+		return err
+	}
+	for _, role := range roles {
+		if role.ID == roleID {
+			return ErrAlreadyExists
+		}
+	}
+
 	return r.store.AssignRole(ctx, subjectID, roleID)
 }
 
 func (r *RBAC) RevokeRole(ctx context.Context, subjectID, roleID string) error {
 	if err := validateUUIDs(roleID); err != nil {
 		return err
 	}
+
+	roles, err := r.store.ListSubjectRoles(ctx, subjectID)
+	if err != nil {
+		return err
+	}
+	found := false
+	for _, role := range roles {
+		if role.ID == roleID {
+			found = true
+			break
+		}
+	}
+	if !found {
+		return ErrNotFound
+	}
+
 	return r.store.RevokeRole(ctx, subjectID, roleID)
 }
 
@@ -91,14 +166,14 @@ func (r *RBAC) AddPermissionToRole(ctx context.Context, roleID, permID string) e
 	}
 	role, err := r.store.GetRole(ctx, roleID)
 	if err != nil {
-		return err
+		return ErrNotFound
 	}
 	if r.roleHasPermission(&role, permID) {
 		return ErrAlreadyExists
 	}
 	p, err := r.store.GetPermission(ctx, permID)
 	if err != nil {
-		return err
+		return ErrNotFound
 	}
 	role.Permissions = append(role.Permissions, p)
 	return r.store.UpdateRole(ctx, role)
@@ -110,8 +185,13 @@ func (r *RBAC) RemovePermissionFromRole(ctx context.Context, roleID, permID stri
 	}
 	role, err := r.store.GetRole(ctx, roleID)
 	if err != nil {
-		return err
+		return ErrNotFound
 	}
+
+	if !r.roleHasPermission(&role, permID) {
+		return ErrNotFound
+	}
+
 	role.Permissions = r.filterOutPermission(role.Permissions, permID)
 	return r.store.UpdateRole(ctx, role)
 }
@@ -132,6 +212,22 @@ func (r *RBAC) RevokeSubjectGrant(ctx context.Context, subjectID, grantID string
 	if err := validateUUIDs(grantID); err != nil {
 		return err
 	}
+
+	grants, err := r.store.ListSubjectGrants(ctx, subjectID)
+	if err != nil {
+		return err
+	}
+	found := false
+	for _, grant := range grants {
+		if grant.ID == grantID {
+			found = true
+			break
+		}
+	}
+	if !found {
+		return ErrNotFound
+	}
+
 	return r.store.RevokeSubjectGrant(ctx, subjectID, grantID)
 }
 
@@ -212,3 +308,70 @@ func validateUUIDs(ids ...string) error {
 	}
 	return nil
 }
+
+
+func (r *RBAC) roleNameExists(ctx context.Context, name string) (bool, error) {
+	roles, err := r.store.ListRoles(ctx)
+	if err != nil {
+		return false, err
+	}
+	for _, role := range roles {
+		if role.Name == name {
+			return true, nil
+		}
+	}
+	return false, nil
+}
+
+
+func (r *RBAC) permissionExists(ctx context.Context, resource, action string) (bool, error) {
+	perms, err := r.store.ListPermissions(ctx)
+	if err != nil {
+		return false, err
+	}
+	for _, p := range perms {
+		if p.Resource != resource {
+			continue
+		}
+		if p.Action != action {
+			continue
+		}
+		return true, nil
+	}
+	return false, nil
+}
+
+
+func (r *RBAC) isRoleInUse(ctx context.Context, roleID string) (bool, error) {
+	subjects, err := r.store.ListSubjects(ctx)
+	if err != nil {
+		return false, err
+	}
+	for _, subjectID := range subjects {
+		roles, err := r.store.ListSubjectRoles(ctx, subjectID)
+		if err != nil {
+			return false, err
+		}
+		for _, role := range roles {
+			if role.ID == roleID {
+				return true, nil
+			}
+		}
+	}
+	return false, nil
+}
+
+func (r *RBAC) isPermissionInUse(ctx context.Context, permID string) (bool, error) {
+	roles, err := r.store.ListRoles(ctx)
+	if err != nil {
+		return false, err
+	}
+	for _, role := range roles {
+		for _, p := range role.Permissions {
+			if p.ID == permID {
+				return true, nil
+			}
+		}
+	}
+	return false, nil
+}

pkg/store.go

@@ -19,6 +19,7 @@ type Store interface {
 	// Subject role bindings
 	AssignRole(ctx context.Context, subjectID, roleID string) error
 	RevokeRole(ctx context.Context, subjectID, roleID string) error
+	ListSubjects(ctx context.Context) ([]string, error)
 	ListSubjectRoles(ctx context.Context, subjectID string) ([]Role, error)
 
 	// Subject direct grants