Skip to content

Allow Editor and Reviewer profiles to transfer record ownership #9284

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
juanluisrp wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Allow Editor and Reviewer profiles to transfer record ownership #9284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions services/src/main/java/org/fao/geonet/api/records/MetadataSharingApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1256,6 +1256,38 @@ private void updateOwnership(Integer groupIdentifier,
serviceContext, String.valueOf(metadata.getId()))) {
report.addNotEditableMetadataId(metadata.getId());
} else {
UserSession callerSession = ApiUtils.getUserSession(session);
Profile callerProfile = callerSession.getProfile();

if (callerProfile == Profile.Editor || callerProfile == Profile.Reviewer) {
int callerUserId = callerSession.getUserIdAsInt();
Integer sourceUsr = metadata.getSourceInfo().getOwner();
Integer sourceGrp = metadata.getSourceInfo().getGroupOwner();

if (callerProfile == Profile.Editor) {
if (!Objects.equals(sourceUsr, callerUserId)) {
report.addNotEditableMetadataId(metadata.getId());
return;
}
} else {
Set<Integer> reviewerGroups = accessMan.getReviewerGroups(callerSession);
if (sourceGrp == null || !reviewerGroups.contains(sourceGrp)) {
report.addNotEditableMetadataId(metadata.getId());
return;
}
}

if (!ReservedGroup.isReserved(groupIdentifier)) {
List<Integer> myGroups = userGroupRepository.findGroupIds(
UserGroupSpecs.hasUserId(callerUserId));
if (!myGroups.contains(groupIdentifier)) {
report.addMetadataInfos(metadata,
"Transfer to group " + groupIdentifier + " is not allowed for your profile.");
return;
}
}
}

@josegar74 josegar74 May 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@juanluisrp I would refactor a bit the code of the changes:

   private boolean isEditableMetadata(AbstractMetadata metadata, AccessManager accessMan, UserSession callerSession) throws Exception {
        Profile callerProfile = callerSession.getProfile();
        
        if (callerProfile == Profile.Editor || callerProfile == Profile.Reviewer) {
            int callerUserId = callerSession.getUserIdAsInt();
            Integer sourceUsr = metadata.getSourceInfo().getOwner();
            Integer sourceGrp = metadata.getSourceInfo().getGroupOwner();

            if (callerProfile == Profile.Editor) {
                if (!Objects.equals(sourceUsr, callerUserId)) {
                    return false;
                }
            } else {
                Set<Integer> reviewerGroups = accessMan.getReviewerGroups(callerSession);
                if (sourceGrp == null || !reviewerGroups.contains(sourceGrp)) {
                    return false;
                }
            }

           return true;
        } else if (callerProfile == Profile.Administrator) {
            return true;
        } else {
            return false;
        }
    }

    private boolean isDestinationTransferGroupAllowed(Integer groupIdentifier, UserSession callerSession) {
        Profile callerProfile = callerSession.getProfile();

        if (callerProfile == Profile.Editor || callerProfile == Profile.Reviewer) {
            int callerUserId = callerSession.getUserIdAsInt();

            if (!ReservedGroup.isReserved(groupIdentifier)) {
                List<Integer> myGroups = userGroupRepository.findGroupIds(
                    UserGroupSpecs.hasUserId(callerUserId));
                if (!myGroups.contains(groupIdentifier)) {
                    return false;
                }
            }

            return true;
        } else if (callerProfile == Profile.Administrator) {
            return true;
        } else {
            return false;
        }
    }
...
            UserSession callerSession = ApiUtils.getUserSession(session);

            if (!isEditableMetadata(metadata, accessMan, callerSession)) {
                report.addNotEditableMetadataId(metadata.getId());
                return;
            }

            if (!isDestinationTransferGroupAllowed(groupIdentifier, callerSession)) {
                report.addMetadataInfos(metadata,
                    "Transfer to group " + groupIdentifier + " is not allowed for your profile.");
                return;
            }

            // Retrieve the identifiers associated with the metadata uuid.
            // When the workflow is enabled, the metadata can have an approved and a working copy version.
...

@josegar74 josegar74 May 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@juanluisrp I would refactor a bit the code of the changes:

   private boolean isEditableMetadata(AbstractMetadata metadata, AccessManager accessMan, UserSession callerSession) throws Exception {
        Profile callerProfile = callerSession.getProfile();
        
        if (callerProfile == Profile.Editor || callerProfile == Profile.Reviewer) {
            int callerUserId = callerSession.getUserIdAsInt();
            Integer sourceUsr = metadata.getSourceInfo().getOwner();
            Integer sourceGrp = metadata.getSourceInfo().getGroupOwner();

            if (callerProfile == Profile.Editor) {
                if (!Objects.equals(sourceUsr, callerUserId)) {
                    return false;
                }
            } else {
                Set<Integer> reviewerGroups = accessMan.getReviewerGroups(callerSession);
                if (sourceGrp == null || !reviewerGroups.contains(sourceGrp)) {
                    return false;
                }
            }
        } else if (callerProfile == Profile.Administrator) {
            return true;
        }
        
        return false;
    }

    private boolean isDestinationTransferGroupAllowed(Integer groupIdentifier, UserSession callerSession) {
        Profile callerProfile = callerSession.getProfile();

        if (callerProfile == Profile.Editor || callerProfile == Profile.Reviewer) {
            int callerUserId = callerSession.getUserIdAsInt();

            if (!ReservedGroup.isReserved(groupIdentifier)) {
                List<Integer> myGroups = userGroupRepository.findGroupIds(
                    UserGroupSpecs.hasUserId(callerUserId));
                if (!myGroups.contains(groupIdentifier)) {
                    return false;
                }
            }

            return true;
        } else if (callerProfile == Profile.Administrator) {
            return true;
        }
        
        return false;
    }
...
            UserSession callerSession = ApiUtils.getUserSession(session);

            if (!isEditableMetadata(metadata, accessMan, callerSession)) {
                report.addNotEditableMetadataId(metadata.getId());
                return;
            }

            if (!isDestinationTransferGroupAllowed(groupIdentifier, callerSession)) {
                report.addMetadataInfos(metadata,
                    "Transfer to group " + groupIdentifier + " is not allowed for your profile.");
                return;
            }

            // Retrieve the identifiers associated with the metadata uuid.
            // When the workflow is enabled, the metadata can have an approved and a working copy version.
...

@josegar74 josegar74 May 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@juanluisrp I would refactor a bit the code of the changes:

   private boolean isEditableMetadata(AbstractMetadata metadata, AccessManager accessMan, UserSession callerSession) throws Exception {
        Profile callerProfile = callerSession.getProfile();
        
        if (callerProfile == Profile.Editor || callerProfile == Profile.Reviewer) {
            int callerUserId = callerSession.getUserIdAsInt();
            Integer sourceUsr = metadata.getSourceInfo().getOwner();
            Integer sourceGrp = metadata.getSourceInfo().getGroupOwner();

            if (callerProfile == Profile.Editor) {
                if (!Objects.equals(sourceUsr, callerUserId)) {
                    return false;
                }
            } else {
                Set<Integer> reviewerGroups = accessMan.getReviewerGroups(callerSession);
                if (sourceGrp == null || !reviewerGroups.contains(sourceGrp)) {
                    return false;
                }
            }
        } else if (callerProfile == Profile.Administrator) {
            return true;
        }
        
        return false;
    }

    private boolean isDestinationTransferGroupAllowed(Integer groupIdentifier, UserSession callerSession) {
        Profile callerProfile = callerSession.getProfile();

        if (callerProfile == Profile.Editor || callerProfile == Profile.Reviewer) {
            int callerUserId = callerSession.getUserIdAsInt();

            if (!ReservedGroup.isReserved(groupIdentifier)) {
                List<Integer> myGroups = userGroupRepository.findGroupIds(
                    UserGroupSpecs.hasUserId(callerUserId));
                if (!myGroups.contains(groupIdentifier)) {
                    return false;
                }
            }

            return true;
        } else if (callerProfile == Profile.Administrator) {
            return true;
        } else {
            return false;
        }
    }
...
            UserSession callerSession = ApiUtils.getUserSession(session);

            if (!isEditableMetadata(metadata, accessMan, callerSession)) {
                report.addNotEditableMetadataId(metadata.getId());
                return;
            }

            if (!isDestinationTransferGroupAllowed(groupIdentifier, callerSession)) {
                report.addMetadataInfos(metadata,
                    "Transfer to group " + groupIdentifier + " is not allowed for your profile.");
                return;
            }

            // Retrieve the identifiers associated with the metadata uuid.
            // When the workflow is enabled, the metadata can have an approved and a working copy version.
...


// Retrieve the identifiers associated with the metadata uuid.
// When the workflow is enabled, the metadata can have an approved and a working copy version.
List<Integer> idList = metadataUtils.findAllIdsBy(MetadataSpecs.hasMetadataUuid(uuid));
Expand Down
14 changes: 14 additions & 0 deletions services/src/main/java/org/fao/geonet/api/users/transfer/TransferApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,20 @@ public List<UserGroupsResponse> retrieveAllUserGroups(
);
}
return list;
} else if (myProfile == Profile.Reviewer || myProfile == Profile.Editor) {
List<Integer> myGroups = userGroupRepository.findAll(UserGroupSpecs.hasUserId(session.getUserIdAsInt()))
.stream().map(ug -> ug.getGroup().getId()).collect(Collectors.toList());
List<UserGroup> userGroups = userGroupRepository.findAll(UserGroupSpecs.hasGroupIds(myGroups));

if (groupTypes != null) {
userGroups = userGroups.stream()
.filter(ug -> groupTypes.contains(ug.getGroup().getType()))
.collect(Collectors.toList());
}
for (UserGroup ug : userGroups) {
list.add(new UserGroupsResponse(ug.getUser(), ug.getGroup(), ug.getProfile().name()));
}
return list;
Comment on lines +189 to +202

@josegar74 josegar74 May 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would refactor, with previous lines also:

    private List<UserGroupsResponse> getUserGroupsResponse(List<UserGroup> userGroups, List<GroupType> groupTypes) {
        List<UserGroupsResponse> list =  new ArrayList<>();

        if (groupTypes != null) {
            userGroups = userGroups.stream()
                .filter(ug -> groupTypes.contains(ug.getGroup().getType()))
                .collect(Collectors.toList());
        }

        for (UserGroup ug : userGroups) {
            list.add(new UserGroupsResponse(ug.getUser(), ug.getGroup(), ug.getProfile().name()));
        }

        return list;
    }
...
           list.addAll(getUserGroupsResponse(userGroups, groupTypes));
            
            return list;

        } else if (myProfile == Profile.Reviewer || myProfile == Profile.Editor) {
            List<Integer> myGroups = userGroupRepository.findAll(UserGroupSpecs.hasUserId(session.getUserIdAsInt()))
                .stream().map(ug -> ug.getGroup().getId()).collect(Collectors.toList());
            List<UserGroup> userGroups = userGroupRepository.findAll(UserGroupSpecs.hasGroupIds(myGroups));

            return getUserGroupsResponse(userGroups, groupTypes);
        } else {
            throw new SecurityException("You don't have rights to do get the groups for this user");
        }
...

} else {
throw new SecurityException("You don't have rights to do get the groups for this user");
}
Expand Down
97 changes: 97 additions & 0 deletions services/src/test/java/org/fao/geonet/api/records/MetadataSharingApiTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,13 @@
import java.util.List;
import java.util.Map;

import static org.hamcrest.Matchers.greaterThan;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;

/**
Expand All @@ -92,6 +94,9 @@ public class MetadataSharingApiTest extends AbstractServiceIntegrationTest {

private User editorUser;
private User reviewerUser;
private User anotherEditorUser;
private User targetUser;
private Group foreignGroup;
private int metadataId;
private String metadataUuid;
private ServiceContext context;
Expand Down Expand Up @@ -460,11 +465,103 @@ private void createTestData() throws Exception {
_userRepo.save(reviewerUser);
_userGroupRepo.save(new UserGroup().setGroup(sampleGroup).setProfile(Profile.Reviewer).setUser(reviewerUser));

anotherEditorUser = UserRepositoryTest.newUser(_inc);
anotherEditorUser.setUsername("anothereditor");
anotherEditorUser.setProfile(Profile.Editor);
_userRepo.save(anotherEditorUser);
_userGroupRepo.save(new UserGroup().setGroup(sampleGroup).setProfile(Profile.Editor).setUser(anotherEditorUser));

targetUser = UserRepositoryTest.newUser(_inc);
targetUser.setUsername("targetuser");
targetUser.setProfile(Profile.Editor);
_userRepo.save(targetUser);
_userGroupRepo.save(new UserGroup().setGroup(sampleGroup).setProfile(Profile.Editor).setUser(targetUser));

foreignGroup = _groupRepo.save(new Group().setName("foreignGroup"));

Metadata metadata = (Metadata) injectMetadataInDb(getSampleMetadataXml(), context, true);
metadata.getSourceInfo().setOwner(editorUser.getId());
metadata.getSourceInfo().setGroupOwner(SAMPLE_GROUP_ID);
metadataRepository.save(metadata);
metadataId = metadata.getId();
metadataUuid = metadata.getUuid();
}

@Test
public void transferOwnershipAsEditorSucceeds() throws Exception {
MockMvc mockMvc = MockMvcBuilders.webAppContextSetup(this.wac).build();
MockHttpSession mockHttpSession = loginAs(editorUser);

mockMvc.perform(put("/srv/api/records/" + metadataUuid + "/ownership")
.session(mockHttpSession)
.param("groupIdentifier", String.valueOf(SAMPLE_GROUP_ID))
.param("userIdentifier", String.valueOf(targetUser.getId()))
.accept(MediaType.APPLICATION_JSON))
.andExpect(status().isCreated())
.andExpect(jsonPath("$.numberOfRecordsProcessed").value(1))
.andExpect(jsonPath("$.numberOfRecordsNotEditable").value(0));
}

@Test
public void transferOwnershipAsEditorForNotOwnedRecordFails() throws Exception {
// Grant editing permission to SAMPLE_GROUP_ID so that anotherEditorUser passes
// canEdit(), but the ownership check in updateOwnership() still blocks the transfer.
OperationAllowed editOp = new OperationAllowed();
editOp.getId().setMetadataId(metadataId).setGroupId(SAMPLE_GROUP_ID)
.setOperationId(ReservedOperation.editing.getId());
operationAllowedRepository.save(editOp);

MockMvc mockMvc = MockMvcBuilders.webAppContextSetup(this.wac).build();
MockHttpSession mockHttpSession = loginAs(anotherEditorUser);

mockMvc.perform(put("/srv/api/records/" + metadataUuid + "/ownership")
.session(mockHttpSession)
.param("groupIdentifier", String.valueOf(SAMPLE_GROUP_ID))
.param("userIdentifier", String.valueOf(targetUser.getId()))
.accept(MediaType.APPLICATION_JSON))
.andExpect(status().isCreated())
.andExpect(jsonPath("$.numberOfRecordsNotEditable").value(1));
}

@Test
public void transferOwnershipAsEditorToForeignGroupFails() throws Exception {
MockMvc mockMvc = MockMvcBuilders.webAppContextSetup(this.wac).build();
MockHttpSession mockHttpSession = loginAs(editorUser);

mockMvc.perform(put("/srv/api/records/" + metadataUuid + "/ownership")
.session(mockHttpSession)
.param("groupIdentifier", String.valueOf(foreignGroup.getId()))
.param("userIdentifier", String.valueOf(targetUser.getId()))
.accept(MediaType.APPLICATION_JSON))
.andExpect(status().isCreated())
.andExpect(jsonPath("$.numberOfRecordsProcessed").value(0));
}

@Test
public void transferOwnershipAsReviewerForGroupRecordSucceeds() throws Exception {
MockMvc mockMvc = MockMvcBuilders.webAppContextSetup(this.wac).build();
MockHttpSession mockHttpSession = loginAs(reviewerUser);

mockMvc.perform(put("/srv/api/records/" + metadataUuid + "/ownership")
.session(mockHttpSession)
.param("groupIdentifier", String.valueOf(SAMPLE_GROUP_ID))
.param("userIdentifier", String.valueOf(targetUser.getId()))
.accept(MediaType.APPLICATION_JSON))
.andExpect(status().isCreated())
.andExpect(jsonPath("$.numberOfRecordsProcessed").value(1));
}

@Test
public void retrieveUserGroupsAsEditorReturnsWorkgroupUsers() throws Exception {
MockMvc mockMvc = MockMvcBuilders.webAppContextSetup(this.wac).build();
MockHttpSession mockHttpSession = loginAs(editorUser);

mockMvc.perform(get("/srv/api/users/groups")
.session(mockHttpSession)
.param("groupTypes", "Workspace")
.accept(MediaType.APPLICATION_JSON))
.andExpect(status().isOk())
.andExpect(jsonPath("$").isArray())
.andExpect(jsonPath("$.length()").value(greaterThan(0)));
}
}
2 changes: 1 addition & 1 deletion ...i/src/main/resources/catalog/components/search/resultsview/partials/selection-widget.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@
</li>
<li
data-ng-show="!excludePattern.test('transferOwnership')"
data-ng-if="user.isUserAdminOrMore()"
data-ng-if="user.isEditorOrMore()"
>
<a
href=""
Expand Down
2 changes: 1 addition & 1 deletion web-ui/src/main/resources/catalog/components/toolbar/partials/menu-manage-record.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
<span data-translate="">privileges</span>
</a>
</li>
<li role="menuitem" data-ng-if="md.isOwned() && user.isUserAdminOrMore()">
<li role="menuitem" data-ng-if="md.isOwned() && user.isEditorOrMore()">
<a href="" data-ng-click="mdService.openTransferOwnership(md, null, getCatScope())">
<span class="fa fa-fw fa-user"></span>&nbsp;
<span data-translate="">transferOwnership</span>
Expand Down
Loading