Fix #3532: Enable ownership tests for Team entity (#3567)

catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TeamRepository.java

@@ -13,11 +13,14 @@
 
 package org.openmetadata.catalog.jdbi3;
 
+import static org.openmetadata.catalog.Entity.FIELD_OWNER;
+import static org.openmetadata.catalog.Entity.TEAM;
 import static org.openmetadata.common.utils.CommonUtil.listOrEmpty;
 
 import com.fasterxml.jackson.core.JsonProcessingException;
 import java.io.IOException;
 import java.net.URI;
+import java.text.ParseException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.UUID;
@@ -32,18 +35,11 @@ import org.openmetadata.catalog.util.EntityUtil;
 import org.openmetadata.catalog.util.EntityUtil.Fields;
 
 public class TeamRepository extends EntityRepository<Team> {
-  static final Fields TEAM_UPDATE_FIELDS = new Fields(TeamResource.ALLOWED_FIELDS, "profile,users,defaultRoles");
+  static final Fields TEAM_UPDATE_FIELDS = new Fields(TeamResource.ALLOWED_FIELDS, "owner,profile,users,defaultRoles");
-  static final Fields TEAM_PATCH_FIELDS = new Fields(TeamResource.ALLOWED_FIELDS, "profile,users,defaultRoles");
+  static final Fields TEAM_PATCH_FIELDS = new Fields(TeamResource.ALLOWED_FIELDS, "owner,profile,users,defaultRoles");
 
   public TeamRepository(CollectionDAO dao) {
-    super(
+    super(TeamResource.COLLECTION_PATH, TEAM, Team.class, dao.teamDAO(), dao, TEAM_PATCH_FIELDS, TEAM_UPDATE_FIELDS);
-        TeamResource.COLLECTION_PATH,
-        Entity.TEAM,
-        Team.class,
-        dao.teamDAO(),
-        dao,
-        TEAM_PATCH_FIELDS,
-        TEAM_UPDATE_FIELDS);
   }
 
   public List<EntityReference> getEntityReferences(List<UUID> ids) {
@@ -58,13 +54,14 @@ public class TeamRepository extends EntityRepository<Team> {
   }
 
   @Override
-  public Team setFields(Team team, Fields fields) throws IOException {
+  public Team setFields(Team team, Fields fields) throws IOException, ParseException {
     if (!fields.contains("profile")) {
       team.setProfile(null); // Clear the profile attribute, if it was not requested
     }
     team.setUsers(fields.contains("users") ? getUsers(team) : null);
     team.setOwns(fields.contains("owns") ? getOwns(team) : null);
     team.setDefaultRoles(fields.contains("defaultRoles") ? getDefaultRoles(team) : null);
+    team.setOwner(fields.contains(FIELD_OWNER) ? getOwner(team) : null);
     return team;
   }
 
@@ -81,6 +78,8 @@ public class TeamRepository extends EntityRepository<Team> {
 
   @Override
   public void prepare(Team team) throws IOException {
+    // Check if owner is valid and set the relationship
+    EntityUtil.populateOwner(daoCollection.userDAO(), daoCollection.teamDAO(), team.getOwner()); // Validate owner
     validateUsers(team.getUsers());
     validateRoles(team.getDefaultRoles());
   }
@@ -88,25 +87,28 @@ public class TeamRepository extends EntityRepository<Team> {
   @Override
   public void storeEntity(Team team, boolean update) throws IOException {
     // Relationships and fields such as href are derived and not stored as part of json
+    EntityReference owner = team.getOwner();
     List<EntityReference> users = team.getUsers();
     List<EntityReference> defaultRoles = team.getDefaultRoles();
 
     // Don't store users, defaultRoles, href as JSON. Build it on the fly based on relationships
-    team.withUsers(null).withDefaultRoles(null).withHref(null);
+    team.withUsers(null).withDefaultRoles(null).withHref(null).withOwner(null);
 
     store(team.getId(), team, update);
 
     // Restore the relationships
-    team.withUsers(users).withDefaultRoles(defaultRoles);
+    team.withUsers(users).withDefaultRoles(defaultRoles).withOwner(owner);
   }
 
   @Override
   public void storeRelationships(Team team) {
+    // Add team owner relationship
+    setOwner(team.getId(), TEAM, team.getOwner());
     for (EntityReference user : listOrEmpty(team.getUsers())) {
-      addRelationship(team.getId(), user.getId(), Entity.TEAM, Entity.USER, Relationship.HAS);
+      addRelationship(team.getId(), user.getId(), TEAM, Entity.USER, Relationship.HAS);
     }
     for (EntityReference defaultRole : listOrEmpty(team.getDefaultRoles())) {
-      addRelationship(team.getId(), defaultRole.getId(), Entity.TEAM, Entity.ROLE, Relationship.HAS);
+      addRelationship(team.getId(), defaultRole.getId(), TEAM, Entity.ROLE, Relationship.HAS);
     }
   }
 
@@ -116,18 +118,18 @@ public class TeamRepository extends EntityRepository<Team> {
   }
 
   private List<EntityReference> getUsers(Team team) throws IOException {
-    List<String> userIds = findTo(team.getId(), Entity.TEAM, Relationship.HAS, Entity.USER);
+    List<String> userIds = findTo(team.getId(), TEAM, Relationship.HAS, Entity.USER);
     return EntityUtil.populateEntityReferences(userIds, Entity.USER);
   }
 
   private List<EntityReference> getOwns(Team team) throws IOException {
     // Compile entities owned by the team
     return EntityUtil.populateEntityReferences(
-        daoCollection.relationshipDAO().findTo(team.getId().toString(), Entity.TEAM, Relationship.OWNS.ordinal()));
+        daoCollection.relationshipDAO().findTo(team.getId().toString(), TEAM, Relationship.OWNS.ordinal()));
   }
 
   private List<EntityReference> getDefaultRoles(Team team) throws IOException {
-    List<String> defaultRoleIds = findTo(team.getId(), Entity.TEAM, Relationship.HAS, Entity.ROLE);
+    List<String> defaultRoleIds = findTo(team.getId(), TEAM, Relationship.HAS, Entity.ROLE);
     return EntityUtil.populateEntityReferences(defaultRoleIds, Entity.ROLE);
   }
 
@@ -163,6 +165,16 @@ public class TeamRepository extends EntityRepository<Team> {
       return entity.getDeleted();
     }
 
+    @Override
+    public void setOwner(EntityReference owner) {
+      entity.setOwner(owner);
+    }
+
+    @Override
+    public EntityReference getOwner() {
+      return entity.getOwner();
+    }
+
     @Override
     public String getFullyQualifiedName() {
       return entity.getName();
@@ -195,7 +207,7 @@ public class TeamRepository extends EntityRepository<Team> {
           .withName(getFullyQualifiedName())
           .withDescription(getDescription())
           .withDisplayName(getDisplayName())
-          .withType(Entity.TEAM)
+          .withType(TEAM)
           .withHref(getHref())
           .withDeleted(isDeleted());
     }
@@ -270,7 +282,7 @@ public class TeamRepository extends EntityRepository<Team> {
       List<EntityReference> origUsers = listOrEmpty(origTeam.getUsers());
       List<EntityReference> updatedUsers = listOrEmpty(updatedTeam.getUsers());
       updateToRelationships(
-          "users", Entity.TEAM, origTeam.getId(), Relationship.HAS, Entity.USER, origUsers, updatedUsers, false);
+          "users", TEAM, origTeam.getId(), Relationship.HAS, Entity.USER, origUsers, updatedUsers, false);
     }
 
     private void updateDefaultRoles(Team origTeam, Team updatedTeam) throws JsonProcessingException {
@@ -278,7 +290,7 @@ public class TeamRepository extends EntityRepository<Team> {
       List<EntityReference> updatedDefaultRoles = listOrEmpty(updatedTeam.getDefaultRoles());
       updateToRelationships(
           "defaultRoles",
-          Entity.TEAM,
+          TEAM,
           origTeam.getId(),
           Relationship.HAS,
           Entity.ROLE,

catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/teams/TeamResource.java

@@ -75,6 +75,7 @@ public class TeamResource extends EntityResource<Team, TeamRepository> {
 
   @Override
   public Team addHref(UriInfo uriInfo, Team team) {
+    Entity.withHref(uriInfo, team.getOwner());
     Entity.withHref(uriInfo, team.getUsers());
     Entity.withHref(uriInfo, team.getDefaultRoles());
     Entity.withHref(uriInfo, team.getOwns());
@@ -94,7 +95,7 @@ public class TeamResource extends EntityResource<Team, TeamRepository> {
     }
   }
 
-  static final String FIELDS = "profile,users,owns,defaultRoles";
+  static final String FIELDS = "owner,profile,users,owns,defaultRoles";
   public static final List<String> ALLOWED_FIELDS = Entity.getEntityFields(Team.class);
 
   @GET
@@ -294,7 +295,7 @@ public class TeamResource extends EntityResource<Team, TeamRepository> {
       @Context UriInfo uriInfo, @Context SecurityContext securityContext, @Valid CreateTeam ct)
       throws IOException, ParseException {
     Team team = getTeam(ct, securityContext);
-    SecurityUtil.checkAdminOrBotOrOwner(authorizer, securityContext, dao.getOriginalOwner(team));
+    SecurityUtil.checkAdminRoleOrPermissions(authorizer, securityContext, dao.getOriginalOwner(team));
     RestUtil.PutResponse<Team> response = dao.createOrUpdate(uriInfo, team);
     addHref(uriInfo, response.getEntity());
     return response.toResponse();
@@ -361,6 +362,7 @@ public class TeamResource extends EntityResource<Team, TeamRepository> {
         .withDisplayName(ct.getDisplayName())
         .withProfile(ct.getProfile())
         .withOwner(ct.getOwner())
+        .withIsJoinable(ct.getIsJoinable())
         .withUpdatedBy(securityContext.getUserPrincipal().getName())
         .withUpdatedAt(System.currentTimeMillis())
         .withUsers(dao.getEntityReferences(ct.getUsers()))

catalog-rest-service/src/main/java/org/openmetadata/catalog/security/DefaultAuthorizer.java

@@ -155,11 +155,8 @@ public class DefaultAuthorizer implements Authorizer {
 
       Object entity = Entity.getEntity(entityReference, new EntityUtil.Fields(List.of("tags", FIELD_OWNER)));
       EntityReference owner = Entity.getEntityInterface(entity).getOwner();
-      if (Entity.shouldHaveOwner(entityReference.getType()) && owner == null) {
+
-        // Entity does not have an owner.
+      if (Entity.shouldHaveOwner(entityReference.getType()) && owner != null && isOwnedByUser(user, owner)) {
-        return true;
-      }
-      if (Entity.shouldHaveOwner(entityReference.getType()) && isOwnedByUser(user, owner)) {
         // Entity is owned by the user.
         return true;
       }

catalog-rest-service/src/main/resources/json/data/policy/DataConsumerAccessControl.json

@@ -6,5 +6,30 @@
   "policyType": "AccessControl",
   "enabled": true,
   "deleted": false,
-  "rules": []
+  "rules": [
+    {
+      "name": "DataConsumerRoleAccessControlPolicy-UpdateDescription",
+      "userRoleAttr": "DataConsumer",
+      "operation": "UpdateDescription",
+      "allow": true,
+      "enabled": true,
+      "priority": 1000
+    },
+    {
+      "name": "DataConsumerRoleAccessControlPolicy-UpdateOwner",
+      "userRoleAttr": "DataConsumer",
+      "operation": "UpdateOwner",
+      "allow": true,
+      "enabled": true,
+      "priority": 1000
+    },
+    {
+      "name": "DataConsumerRoleAccessControlPolicy-UpdateTags",
+      "userRoleAttr": "DataConsumer",
+      "operation": "UpdateTags",
+      "allow": true,
+      "enabled": true,
+      "priority": 1000
+    }
+  ]
 }

catalog-rest-service/src/main/resources/json/schema/api/teams/createTeam.json

@@ -41,10 +41,10 @@
       "$ref": "../../type/entityReference.json",
       "default": null
     },
-    "joinable": {
+    "isJoinable": {
-      "description": "This field describes if the users can join a team without any permission. By default this is set to True.",
+      "description": "Can any user join this team during sign up? Value of true indicates yes, and false no.",
       "type": "boolean",
-      "default": "true"
+      "default": true
     }
   },
   "required": ["name"],

catalog-rest-service/src/main/resources/json/schema/entity/teams/team.json

@@ -61,6 +61,11 @@
       "$ref": "../../type/entityReference.json",
       "default": null
     },
+    "isJoinable": {
+      "description": "Can any user join this team during sign up? Value of true indicates yes, and false no.",
+      "type": "boolean",
+      "default": true
+    },
     "changeDescription": {
       "description": "Change that lead to this version of the entity.",
       "$ref": "../../type/entityHistory.json#/definitions/changeDescription"

catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/EntityResourceTest.java

@@ -262,15 +262,21 @@ public abstract class EntityResourceTest<T, K> extends CatalogApplicationTest {
       webhookResourceTest.startWebhookSubscription();
       webhookResourceTest.startWebhookEntitySubscriptions(entityType);
     }
-
+    RoleResourceTest roleResourceTest = new RoleResourceTest();
+    DATA_CONSUMER_ROLE =
+        roleResourceTest.getEntityByName(DATA_CONSUMER_ROLE_NAME, RoleResource.FIELDS, ADMIN_AUTH_HEADERS);
+    DATA_CONSUMER_ROLE_REFERENCE = new RoleEntityInterface(DATA_CONSUMER_ROLE).getEntityReference();
     UserResourceTest userResourceTest = new UserResourceTest();
-    USER1 = userResourceTest.createEntity(userResourceTest.createRequest(test), ADMIN_AUTH_HEADERS);
+    USER1 =
+        userResourceTest.createEntity(
+            userResourceTest.createRequest(test).withRoles(List.of(DATA_CONSUMER_ROLE.getId())), ADMIN_AUTH_HEADERS);
     USER_OWNER1 = new UserEntityInterface(USER1).getEntityReference();
 
-    USER2 = userResourceTest.createEntity(userResourceTest.createRequest(test, 1), ADMIN_AUTH_HEADERS);
+    USER2 =
+        userResourceTest.createEntity(
+            userResourceTest.createRequest(test, 1).withRoles(List.of(DATA_CONSUMER_ROLE.getId())), ADMIN_AUTH_HEADERS);
     USER_OWNER2 = new UserEntityInterface(USER2).getEntityReference();
 
-    RoleResourceTest roleResourceTest = new RoleResourceTest();
     DATA_STEWARD_ROLE =
         roleResourceTest.getEntityByName(DATA_STEWARD_ROLE_NAME, RoleResource.FIELDS, ADMIN_AUTH_HEADERS);
     DATA_STEWARD_ROLE_REFERENCE = new RoleEntityInterface(DATA_STEWARD_ROLE).getEntityReference();
@@ -280,9 +286,7 @@ public abstract class EntityResourceTest<T, K> extends CatalogApplicationTest {
                 .createRequest("user-data-steward", "", "", null)
                 .withRoles(List.of(DATA_STEWARD_ROLE.getId())),
             ADMIN_AUTH_HEADERS);
-    DATA_CONSUMER_ROLE =
+
-        roleResourceTest.getEntityByName(DATA_CONSUMER_ROLE_NAME, RoleResource.FIELDS, ADMIN_AUTH_HEADERS);
-    DATA_CONSUMER_ROLE_REFERENCE = new RoleEntityInterface(DATA_CONSUMER_ROLE).getEntityReference();
     USER_WITH_DATA_CONSUMER_ROLE =
         userResourceTest.createEntity(
             userResourceTest
@@ -1125,12 +1129,12 @@ public abstract class EntityResourceTest<T, K> extends CatalogApplicationTest {
 
     T entity = createEntity(createRequest(getEntityName(test), "description", null, null), ADMIN_AUTH_HEADERS);
 
-    // Anyone can update description on unowned entity.
+    // Admins, Owner or a User with policy can update the entity
     entity = patchEntityAndCheckAuthorization(getEntityInterface(entity), TestUtils.ADMIN_USER_NAME, false);
-    entity = patchEntityAndCheckAuthorization(getEntityInterface(entity), USER1.getName(), false);
     entity = patchEntityAndCheckAuthorization(getEntityInterface(entity), USER_WITH_DATA_STEWARD_ROLE.getName(), false);
     entity =
         patchEntityAndCheckAuthorization(getEntityInterface(entity), USER_WITH_DATA_CONSUMER_ROLE.getName(), false);
+    entity = patchEntityAndCheckAuthorization(getEntityInterface(entity), USER1.getName(), false);
 
     EntityInterface<T> entityInterface = getEntityInterface(entity);
 
@@ -1155,7 +1159,7 @@ public abstract class EntityResourceTest<T, K> extends CatalogApplicationTest {
     entity = patchEntityAndCheckAuthorization(getEntityInterface(entity), TestUtils.ADMIN_USER_NAME, false);
     entity = patchEntityAndCheckAuthorization(getEntityInterface(entity), USER1.getName(), false);
     entity = patchEntityAndCheckAuthorization(getEntityInterface(entity), USER_WITH_DATA_STEWARD_ROLE.getName(), false);
-    patchEntityAndCheckAuthorization(getEntityInterface(entity), USER_WITH_DATA_CONSUMER_ROLE.getName(), true);
+    patchEntityAndCheckAuthorization(getEntityInterface(entity), USER_WITH_DATA_CONSUMER_ROLE.getName(), false);
   }
 
   @Test

catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/permissions/PermissionsResourceTest.java

@@ -39,11 +39,11 @@ import org.openmetadata.catalog.type.MetadataOperation;
 import org.openmetadata.catalog.util.TestUtils;
 
 @TestInstance(TestInstance.Lifecycle.PER_CLASS)
-class PermisssionsResourceTest extends CatalogApplicationTest {
+class PermissionsResourceTest extends CatalogApplicationTest {
   private static final String DATA_STEWARD_ROLE_NAME = "DataSteward";
   private static final String DATA_CONSUMER_ROLE_NAME = "DataConsumer";
-  private static final String DATA_STEWARD_USER_NAME = "user-data-steward";
+  private static final String DATA_STEWARD_USER_NAME = "puser-data-steward";
-  private static final String DATA_CONSUMER_USER_NAME = "user-data-consumer";
+  private static final String DATA_CONSUMER_USER_NAME = "puser-data-consumer";
 
   @BeforeAll
   static void setup() throws IOException {
@@ -69,7 +69,7 @@ class PermisssionsResourceTest extends CatalogApplicationTest {
 
   @ParameterizedTest
   @MethodSource("getPermissionsTestParams")
-  void get_permissions(String username, Map<MetadataOperation, Boolean> expectedOperations)
+  void get_permissiofns(String username, Map<MetadataOperation, Boolean> expectedOperations)
       throws HttpResponseException {
     WebTarget target = getResource("permissions");
     Map<String, String> authHeaders = SecurityUtil.authHeaders(username + "@open-metadata.org");
@@ -99,12 +99,12 @@ class PermisssionsResourceTest extends CatalogApplicationTest {
             DATA_STEWARD_USER_NAME,
             new HashMap<MetadataOperation, Boolean>() {
               {
-                put(MetadataOperation.SuggestDescription, Boolean.FALSE);
-                put(MetadataOperation.SuggestTags, Boolean.FALSE);
                 put(MetadataOperation.UpdateDescription, Boolean.TRUE);
                 put(MetadataOperation.UpdateLineage, Boolean.TRUE);
                 put(MetadataOperation.UpdateOwner, Boolean.TRUE);
                 put(MetadataOperation.UpdateTags, Boolean.TRUE);
+                put(MetadataOperation.SuggestDescription, Boolean.FALSE);
+                put(MetadataOperation.SuggestTags, Boolean.FALSE);
                 put(MetadataOperation.DecryptTokens, Boolean.FALSE);
                 put(MetadataOperation.UpdateTeam, Boolean.FALSE);
               }
@@ -115,10 +115,10 @@ class PermisssionsResourceTest extends CatalogApplicationTest {
               {
                 put(MetadataOperation.SuggestDescription, Boolean.FALSE);
                 put(MetadataOperation.SuggestTags, Boolean.FALSE);
-                put(MetadataOperation.UpdateDescription, Boolean.FALSE);
+                put(MetadataOperation.UpdateDescription, Boolean.TRUE);
                 put(MetadataOperation.UpdateLineage, Boolean.FALSE);
-                put(MetadataOperation.UpdateOwner, Boolean.FALSE);
+                put(MetadataOperation.UpdateOwner, Boolean.TRUE);
-                put(MetadataOperation.UpdateTags, Boolean.FALSE);
+                put(MetadataOperation.UpdateTags, Boolean.TRUE);
                 put(MetadataOperation.DecryptTokens, Boolean.FALSE);
                 put(MetadataOperation.UpdateTeam, Boolean.FALSE);
               }

catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/TeamResourceTest.java

@@ -76,7 +76,6 @@ public class TeamResourceTest extends EntityResourceTest<Team, CreateTeam> {
 
   public TeamResourceTest() {
     super(Entity.TEAM, Team.class, TeamList.class, "teams", TeamResource.FIELDS);
-    this.supportsOwner = false; // TODO fix the test failures after removing this
     this.supportsDots = false;
     this.supportsAuthorizedMetadataOperations = false;
   }
@@ -345,7 +344,8 @@ public class TeamResourceTest extends EntityResourceTest<Team, CreateTeam> {
         .withName(name)
         .withDescription(description)
         .withDisplayName(displayName)
-        .withProfile(PROFILE);
+        .withProfile(PROFILE)
+        .withOwner(owner);
   }
 
   @Override
@@ -360,7 +360,10 @@ public class TeamResourceTest extends EntityResourceTest<Team, CreateTeam> {
   @Override
   public void validateCreatedEntity(Team team, CreateTeam createRequest, Map<String, String> authHeaders) {
     validateCommonEntityFields(
-        getEntityInterface(team), createRequest.getDescription(), TestUtils.getPrincipal(authHeaders), null);
+        getEntityInterface(team),
+        createRequest.getDescription(),
+        TestUtils.getPrincipal(authHeaders),
+        createRequest.getOwner());
 
     assertEquals(createRequest.getProfile(), team.getProfile());
     TestUtils.validateEntityReferences(team.getOwns());
@@ -401,7 +404,10 @@ public class TeamResourceTest extends EntityResourceTest<Team, CreateTeam> {
   @Override
   public void compareEntities(Team expected, Team updated, Map<String, String> authHeaders) {
     validateCommonEntityFields(
-        getEntityInterface(updated), expected.getDescription(), TestUtils.getPrincipal(authHeaders), null);
+        getEntityInterface(updated),
+        expected.getDescription(),
+        TestUtils.getPrincipal(authHeaders),
+        expected.getOwner());
 
     assertEquals(expected.getDisplayName(), updated.getDisplayName());
     assertEquals(expected.getProfile(), updated.getProfile());