MG-2069 - Remove relation requirement from entity unassignment (#2130)

Signed-off-by: WashingtonKK <[email protected]>
absmach · Apr 23, 2024 · 07635ad · 07635ad
1 parent 3aa674f
commit 07635ad
Show file tree

Hide file tree

Showing 11 changed files with 74 additions and 48 deletions.
diff --git a/api/openapi/auth.yml b/api/openapi/auth.yml
@@ -286,7 +286,7 @@ paths:
       security:
         - bearerAuth: []
       responses:
-        "200":
+        "204":
           description: Users successfully unassigned from domain.
         "400":
           description: Failed due to malformed domain's ID.

diff --git a/api/openapi/users.yml b/api/openapi/users.yml
@@ -1787,6 +1787,10 @@ components:
                 type: string
                 format: email
                 description: User email.
+              host:
+                type: string
+                example: examplehost
+                description: Email host.
 
     PasswordReset:
       description: Password reset request data, new password and token that is appended on password reset link received in email.
@@ -1799,11 +1803,13 @@ components:
                 type: string
                 format: password
                 description: New password.
+                example: 12345678
                 minimum: 8
               confirm_password:
                 type: string
                 format: password
                 description: New confirmation password.
+                example: 12345678
                 minimum: 8
               token:
                 type: string

diff --git a/auth/api/http/domains/endpoint_test.go b/auth/api/http/domains/endpoint_test.go
@@ -1163,15 +1163,6 @@ func TestUnassignDomainUsers(t *testing.T) {
 			status:      http.StatusBadRequest,
 			err:         apiutil.ErrValidation,
 		},
-		{
-			desc:        "unassign domain users with empty relation",
-			data:        fmt.Sprintf(`{"relation": "%s", "user_ids" : ["%s", "%s"]}`, "", validID, validID),
-			domainID:    domain.ID,
-			contentType: contentType,
-			token:       validToken,
-			status:      http.StatusBadRequest,
-			err:         apiutil.ErrValidation,
-		},
 	}
 
 	for _, tc := range cases {

diff --git a/auth/api/http/domains/requests.go b/auth/api/http/domains/requests.go
@@ -205,10 +205,6 @@ func (req unassignUsersReq) validate() error {
 		return apiutil.ErrMalformedPolicy
 	}
 
-	if req.Relation == "" {
-		return apiutil.ErrMalformedPolicy
-	}
-
 	return nil
 }
 

diff --git a/auth/postgres/domains.go b/auth/postgres/domains.go
@@ -370,16 +370,19 @@ func (repo domainRepo) DeletePolicies(ctx context.Context, pcs ...auth.Policy) (
 
 	for _, pc := range pcs {
 		q := `
-			DELETE FROM
-				policies
-			WHERE
-				subject_type = :subject_type
-				AND subject_id = :subject_id
-				AND subject_relation = :subject_relation
-				AND relation = :relation
-				AND object_type = :object_type
-				AND object_id = :object_id
-			;`
+		DELETE FROM
+			policies
+		WHERE
+			subject_type = :subject_type
+			AND subject_id = :subject_id
+			AND subject_relation = :subject_relation
+			AND object_type = :object_type
+			AND object_id = :object_id
+		`
+		if pc.Relation != "" {
+			q = fmt.Sprintf("%s AND relation = :relation", q)
+		}
+		q = fmt.Sprintf("%s;", q)
 
 		dbpc := toDBPolicy(pc)
 		row, err := tx.NamedQuery(q, dbpc)

diff --git a/auth/postgres/domains_test.go b/auth/postgres/domains_test.go
@@ -83,6 +83,17 @@ func TestDeletePolicyCopy(t *testing.T) {
 			},
 			err: nil,
 		},
+		{
+			desc: "delete a  policy with empty relation",
+			pc: auth.Policy{
+				SubjectType: "unknown",
+				SubjectID:   "unknown",
+				Relation:    "",
+				ObjectType:  "unknown",
+				ObjectID:    "unknown",
+			},
+			err: nil,
+		},
 	}
 
 	for _, tc := range cases {

diff --git a/auth/service.go b/auth/service.go
@@ -718,31 +718,57 @@ func (svc service) AssignUsers(ctx context.Context, token, id string, userIds []
 }
 
 func (svc service) UnassignUsers(ctx context.Context, token, id string, userIds []string, relation string) error {
-	if err := svc.Authorize(ctx, PolicyReq{
+	pr := PolicyReq{
 		Subject:     token,
 		SubjectType: UserType,
 		SubjectKind: TokenKind,
 		Object:      id,
 		ObjectType:  DomainType,
 		Permission:  SharePermission,
-	}); err != nil {
+	}
+	if err := svc.Authorize(ctx, pr); err != nil {
 		return err
 	}
 
-	if err := svc.Authorize(ctx, PolicyReq{
-		Subject:     token,
-		SubjectType: UserType,
-		SubjectKind: TokenKind,
-		Object:      id,
-		ObjectType:  DomainType,
-		Permission:  SwitchToPermission(relation),
-	}); err != nil {
-		return err
+	if relation != "" {
+		pr.Permission = SwitchToPermission(relation)
+		if err := svc.Authorize(ctx, pr); err != nil {
+			return err
+		}
+
+		if err := svc.removeDomainPolicies(ctx, id, relation, userIds...); err != nil {
+			return errors.Wrap(errRemovePolicies, err)
+		}
+		return nil
+	}
+	pr.Permission = AdminPermission
+	if err := svc.Authorize(ctx, pr); err != nil {
+		// User is not admin.
+		var ids []string
+		for _, userID := range userIds {
+			if err := svc.Authorize(ctx, PolicyReq{
+				Subject:     userID,
+				SubjectType: UserType,
+				SubjectKind: UsersKind,
+				Permission:  AdminPermission,
+				Object:      id,
+				ObjectType:  DomainType,
+			}); err != nil {
+				// Append all non-admins to ids.
+				ids = append(ids, userID)
+			}
+		}
+
+		userIds = ids
 	}
 
-	if err := svc.removeDomainPolicies(ctx, id, relation, userIds...); err != nil {
-		return errors.Wrap(errRemovePolicies, err)
+	for _, rel := range []string{MemberRelation, ViewerRelation, EditorRelation} {
+		// Remove only non-admins.
+		if err := svc.removeDomainPolicies(ctx, id, rel, userIds...); err != nil {
+			return errors.Wrap(errRemovePolicies, err)
+		}
 	}
+
 	return nil
 }
 
@@ -911,8 +937,8 @@ func (svc service) removeDomainPolicies(ctx context.Context, domainID, relation
 	if err := svc.agent.DeletePolicies(ctx, prs); err != nil {
 		return errors.Wrap(errRemovePolicies, err)
 	}
-	err = svc.domains.DeletePolicies(ctx, pcs...)
-	if err != nil {
+
+	if err = svc.domains.DeletePolicies(ctx, pcs...); err != nil {
 		return errors.Wrap(errRemovePolicies, err)
 	}
 	return err

diff --git a/things/api/http/requests.go b/things/api/http/requests.go
@@ -245,10 +245,6 @@ func (req unassignUsersRequest) validate() error {
 		return apiutil.ErrBearerToken
 	}
 
-	if req.Relation == "" {
-		return apiutil.ErrMissingRelation
-	}
-
 	if req.groupID == "" {
 		return apiutil.ErrMissingID
 	}

diff --git a/things/api/http/requests_test.go b/things/api/http/requests_test.go
@@ -603,7 +603,7 @@ func TestUnassignUsersRequestValidate(t *testing.T) {
 				UserIDs:  []string{validID},
 				Relation: "",
 			},
-			err: apiutil.ErrMissingRelation,
+			err: nil,
 		},
 	}
 	for _, c := range cases {

diff --git a/users/api/requests.go b/users/api/requests.go
@@ -330,9 +330,6 @@ func (req unassignUsersReq) validate() error {
 		return apiutil.ErrMissingID
 	}
 
-	if req.Relation == "" {
-		return apiutil.ErrMissingRelation
-	}
 	if len(req.UserIDs) == 0 {
 		return apiutil.ErrEmptyList
 	}

diff --git a/users/api/requests_test.go b/users/api/requests_test.go
@@ -816,7 +816,7 @@ func TestUnassignUsersRequestValidate(t *testing.T) {
 				UserIDs:  []string{validID},
 				Relation: "",
 			},
-			err: apiutil.ErrMissingRelation,
+			err: nil,
 		},
 	}
 	for _, c := range cases {