From deef55f1388620968edfd308301d1fb5ba1406de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jose=20Garc=C3=ADa?= Date: Thu, 30 Jan 2025 14:01:21 +0100 Subject: [PATCH] Setting to configure whether editor users who are metadata owners can edit their metadata when they do not have editing privileges for the metadata. --- .../org/fao/geonet/kernel/AccessManager.java | 35 +++-- .../fao/geonet/kernel/setting/Settings.java | 1 + .../org/fao/geonet/api/es/EsHTTPProxy.java | 132 +++++++++--------- .../resources/catalog/locales/en-admin.json | 2 + .../setup/sql/data/data-db-default.sql | 1 + .../sql/migrate/v447/migrate-default.sql | 1 + 6 files changed, 95 insertions(+), 77 deletions(-) diff --git a/core/src/main/java/org/fao/geonet/kernel/AccessManager.java b/core/src/main/java/org/fao/geonet/kernel/AccessManager.java index 3d9e77f356b..65042ab3890 100644 --- a/core/src/main/java/org/fao/geonet/kernel/AccessManager.java +++ b/core/src/main/java/org/fao/geonet/kernel/AccessManager.java @@ -1,5 +1,5 @@ //============================================================================= -//=== Copyright (C) 2001-2007 Food and Agriculture Organization of the +//=== Copyright (C) 2001-2025 Food and Agriculture Organization of the //=== United Nations (FAO-UN), United Nations World Food Programme (WFP) //=== and United Nations Environment Programme (UNEP) //=== @@ -46,6 +46,7 @@ import static org.fao.geonet.kernel.setting.Settings.SYSTEM_INTRANET_IP_SEPARATOR; import static org.fao.geonet.kernel.setting.Settings.SYSTEM_METADATAPRIVS_PUBLICATIONBYGROUPOWNERONLY; +import static org.fao.geonet.kernel.setting.Settings.SYSTEM_METADATAPRIVS_USER_ALWAYS_CAN_EDIT_OWNED_METADATA; import static org.fao.geonet.repository.specification.OperationAllowedSpecs.hasMetadataId; import static org.fao.geonet.repository.specification.OperationAllowedSpecs.hasOperation; import static org.springframework.data.jpa.domain.Specification.where; @@ -114,7 +115,7 @@ public Set getOperations(ServiceContext context, String mdId, String } public Set getOperationNames(ServiceContext context, String mdId, String ip, Collection operations) throws Exception { - Set names = new HashSet(); + Set names = new HashSet<>(); for (Operation op : getOperations(context, mdId, ip, operations)) { names.add(op.getName()); @@ -127,7 +128,7 @@ public Set getOperationNames(ServiceContext context, String mdId, String * Returns all operations permitted by the user on a particular metadata. */ public Set getAllOperations(ServiceContext context, String mdId, String ip) throws Exception { - HashSet operations = new HashSet(); + HashSet operations = new HashSet<>(); Set groups = getUserGroups(context.getUserSession(), ip, false); for (OperationAllowed opAllow : operationAllowedRepository.findByMetadataId(mdId)) { @@ -146,7 +147,7 @@ public Set getAllOperations(ServiceContext context, String mdId, Stri public Set getUserGroups(UserSession usrSess, String ip, boolean editingGroupsOnly) throws Exception { final ConfigurableApplicationContext applicationContext = ApplicationContextHolder.get(); - Set hs = new HashSet(); + Set hs = new HashSet<>(); // add All (1) network group hs.add(ReservedGroup.all.getId()); @@ -193,7 +194,7 @@ public static List getGroups(UserSession session, Profile profile) thro } public Set getReviewerGroups(UserSession usrSess) throws Exception { - Set hs = new HashSet(); + Set hs = new HashSet<>(); // get other groups if ((usrSess != null) && usrSess.isAuthenticated()) { @@ -214,7 +215,7 @@ public Set getReviewerGroups(UserSession usrSess) throws Exception { * @param userId the id of the user */ public Set getVisibleGroups(final int userId) throws Exception { - Set hs = new HashSet(); + Set hs = new HashSet<>(); Optional user = userRepository.findById(userId); @@ -243,10 +244,22 @@ public Set getVisibleGroups(final int userId) throws Exception { *
  • the user has edit rights over the metadata
    • * * + * If the setting to allow edit always to the metadata the owner (independently of the edit privilege in + * the group owner of the metadata) is disabled, only the edit privileges are checked, except for Administrators. + * * @param id The metadata internal identifier */ public boolean canEdit(final ServiceContext context, final String id) throws Exception { - return isOwner(context, id) || hasEditPermission(context, id); + UserSession us = context.getUserSession(); + final Profile profile = us.getProfile(); + + if ((profile == Profile.Administrator) || settingManager.getValueAsBool(SYSTEM_METADATAPRIVS_USER_ALWAYS_CAN_EDIT_OWNED_METADATA, true)) { + return isOwner(context, id) || hasEditPermission(context, id); + } else { + // Ownership is not checked.. If the user is Editor and is the metadata owner, + // can only edit the metadata if has edit privileges for the metadata. + return hasEditPermission(context, id); + } } /** @@ -429,9 +442,9 @@ public boolean hasReviewPermission(final ServiceContext context, final String id return hasReviewPermission(context, info); } - private String GROUPOWNERONLY_STRATEGY = + private static final String GROUPOWNERONLY_STRATEGY = "api.metadata.share.strategy.groupOwnerOnly"; - private String REVIEWERINGROUP_STRATEGY = + private static final String REVIEWERINGROUP_STRATEGY = "api.metadata.share.strategy.reviewerInGroup"; public String getReviewerRule() { @@ -497,9 +510,9 @@ private boolean hasEditingPermissionWithProfile(final ServiceContext context, fi return false; } - Specification spec = where(UserGroupSpecs.hasProfile(profile)).and(UserGroupSpecs.hasUserId(us.getUserIdAsInt())); + Specification spec = where(UserGroupSpecs.hasProfile(profile)).and(UserGroupSpecs.hasUserId(us.getUserIdAsInt())); - List opAlloweds = new ArrayList(); + List opAlloweds = new ArrayList<>(); for (OperationAllowed opAllowed : allOpAlloweds) { opAlloweds.add(opAllowed.getId().getGroupId()); } diff --git a/core/src/main/java/org/fao/geonet/kernel/setting/Settings.java b/core/src/main/java/org/fao/geonet/kernel/setting/Settings.java index c3c7a9e2aa0..4b2a194c40c 100644 --- a/core/src/main/java/org/fao/geonet/kernel/setting/Settings.java +++ b/core/src/main/java/org/fao/geonet/kernel/setting/Settings.java @@ -109,6 +109,7 @@ public class Settings { public static final String SYSTEM_METADATAPRIVS_PUBLICATIONNOTIFICATION_EMAILS = "system/metadataprivs/publication/notificationEmails"; public static final String SYSTEM_METADATAPRIVS_PUBLICATION_NOTIFICATIONLEVEL = "system/metadataprivs/publication/notificationLevel"; public static final String SYSTEM_METADATAPRIVS_PUBLICATION_NOTIFICATIONGROUPS = "system/metadataprivs/publication/notificationGroups"; + public static final String SYSTEM_METADATAPRIVS_USER_ALWAYS_CAN_EDIT_OWNED_METADATA = "system/metadataprivs/userAlwaysCanEditOwnedMetadata"; public static final String SYSTEM_INSPIRE_ATOM_PROTOCOL = "system/inspire/atomProtocol"; public static final String SYSTEM_HARVESTING_MAIL_RECIPIENT = "system/harvesting/mail/recipient"; public static final String SYSTEM_HARVESTING_MAIL_LEVEL3 = "system/harvesting/mail/level3"; diff --git a/services/src/main/java/org/fao/geonet/api/es/EsHTTPProxy.java b/services/src/main/java/org/fao/geonet/api/es/EsHTTPProxy.java index dca972556b7..d96fc3ccbe4 100644 --- a/services/src/main/java/org/fao/geonet/api/es/EsHTTPProxy.java +++ b/services/src/main/java/org/fao/geonet/api/es/EsHTTPProxy.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2018-2024 Food and Agriculture Organization of the + * Copyright (C) 2018-2025 Food and Agriculture Organization of the * United Nations (FAO-UN), United Nations World Food Programme (WFP) * and United Nations Environment Programme (UNEP) * @@ -65,6 +65,7 @@ import org.fao.geonet.kernel.schema.MetadataSchema; import org.fao.geonet.kernel.schema.MetadataSchemaOperationFilter; import org.fao.geonet.kernel.search.EsFilterBuilder; +import org.fao.geonet.kernel.setting.SettingManager; import org.fao.geonet.repository.SourceRepository; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -91,6 +92,8 @@ import java.util.zip.GZIPInputStream; import java.util.zip.GZIPOutputStream; +import static org.fao.geonet.kernel.setting.Settings.SYSTEM_METADATAPRIVS_USER_ALWAYS_CAN_EDIT_OWNED_METADATA; + @RequestMapping(value = { "/{portal}/api" @@ -104,7 +107,7 @@ * The portal and privileges are included the search provided by the user. */ public class EsHTTPProxy { - public static final String[] _validContentTypes = { + protected static final String[] validContentTypes = { "application/json", "text/plain" }; private static final Logger LOGGER = LoggerFactory.getLogger(Geonet.INDEX_ENGINE); @@ -112,7 +115,7 @@ public class EsHTTPProxy { * Privileges filter only allows * * op0 (ie. view operation) contains one of the ids of your groups */ - private static final String filterTemplate = " {\n" + + private static final String FILTER_TEMPLATE = " {\n" + " \t\"query_string\": {\n" + " \t\t\"query\": \"%s\"\n" + " \t}\n" + @@ -145,7 +148,7 @@ public class EsHTTPProxy { /** * Ignore list of headers handled by proxy implementation directly. */ - private String[] proxyHeadersIgnoreList = {"Content-Length"}; + private String[] proxyHeadersIgnoreList = {"Content-Length"}; @Autowired private EsRestClient client; @@ -215,7 +218,7 @@ public static void addUserInfo(ObjectNode doc, ServiceContext context) throws Ex final AccessManager accessManager = context.getBean(AccessManager.class); final boolean isOwner = accessManager.isOwner(context, sourceInfo); final HashSet operations; - boolean canEdit = false; + boolean canEdit = id != null && accessManager.canEdit(context, id); if (isOwner) { operations = Sets.newHashSet(Arrays.asList(ReservedOperation.values())); if (owner != null) { @@ -224,8 +227,6 @@ public static void addUserInfo(ObjectNode doc, ServiceContext context) throws Ex } else { final Collection groups = accessManager.getUserGroups(context.getUserSession(), context.getIpAddress(), false); - final Collection editingGroups = - accessManager.getUserGroups(context.getUserSession(), context.getIpAddress(), true); operations = Sets.newHashSet(); for (ReservedOperation operation : ReservedOperation.values()) { final JsonNode operationNodes = doc.get("_source").get(Geonet.IndexFieldNames.OP_PREFIX + operation.getId()); @@ -234,11 +235,6 @@ public static void addUserInfo(ObjectNode doc, ServiceContext context) throws Ex if (opFields != null) { for (JsonNode field : opFields) { final int groupId = field.asInt(); - if (operation == ReservedOperation.editing - && canEdit == false - && editingGroups.contains(groupId)) { - canEdit = true; - } if (groups.contains(groupId)) { operations.add(operation); @@ -248,9 +244,15 @@ public static void addUserInfo(ObjectNode doc, ServiceContext context) throws Ex } } } - doc.put(Edit.Info.Elem.EDIT, isOwner || canEdit); + + final SettingManager settingManager = context.getBean(SettingManager.class); + if (settingManager.getValueAsBool(SYSTEM_METADATAPRIVS_USER_ALWAYS_CAN_EDIT_OWNED_METADATA, true)) { + doc.put(Edit.Info.Elem.EDIT, isOwner || canEdit); + } else { + doc.put(Edit.Info.Elem.EDIT, canEdit); + } doc.put(Edit.Info.Elem.REVIEW, - id != null ? accessManager.hasReviewPermission(context, id) : false); + id != null && accessManager.hasReviewPermission(context, id)); doc.put(Edit.Info.Elem.OWNER, isOwner); doc.put(Edit.Info.Elem.IS_PUBLISHED_TO_ALL, hasOperation(doc, ReservedGroup.all, ReservedOperation.view)); addReservedOperation(doc, operations, ReservedOperation.view); @@ -359,8 +361,8 @@ public void msearch( @RequestBody @io.swagger.v3.oas.annotations.parameters.RequestBody(description = "JSON request based on Elasticsearch API.", content = @Content(examples = { - @ExampleObject(value = "{\"query\":{\"match\":{\"_id\":\"catalogue_uuid\"}}}") - })) + @ExampleObject(value = "{\"query\":{\"match\":{\"_id\":\"catalogue_uuid\"}}}") + })) String body) throws Exception { ServiceContext context = ApiUtils.createServiceContext(request); call(context, httpSession, request, response, MULTISEARCH_ENDPOINT, body, bucket, relatedTypes); @@ -409,10 +411,10 @@ public void call( } private void call(ServiceContext context, HttpSession httpSession, HttpServletRequest request, - HttpServletResponse response, - String endPoint, String body, - String selectionBucket, - RelatedItemType[] relatedTypes) throws Exception { + HttpServletResponse response, + String endPoint, String body, + String selectionBucket, + RelatedItemType[] relatedTypes) throws Exception { final String url = client.getServerUrl() + "/" + defaultIndex + "/" + endPoint + "?"; // Make query on multiple indices // final String url = client.getServerUrl() + "/" + defaultIndex + ",gn-features/" + endPoint + "?"; @@ -423,23 +425,23 @@ private void call(ServiceContext context, HttpSession httpSession, HttpServletRe // multisearch support final MappingIterator mappingIterator = objectMapper.readerFor(JsonNode.class).readValues(body); - StringBuffer requestBody = new StringBuffer(); + StringBuilder requestBody = new StringBuilder(); while (mappingIterator.hasNextValue()) { - JsonNode node = (JsonNode) mappingIterator.nextValue(); - final JsonNode indexNode = node.get("index"); + JsonNode jsonNode = (JsonNode) mappingIterator.nextValue(); + final JsonNode indexNode = jsonNode.get("index"); if (indexNode != null) { - ((ObjectNode) node).put("index", defaultIndex); + ((ObjectNode) jsonNode).put("index", defaultIndex); } else { - final JsonNode queryNode = node.get("query"); + final JsonNode queryNode = jsonNode.get("query"); if (queryNode != null) { - addFilterToQuery(context, objectMapper, node); + addFilterToQuery(context, objectMapper, jsonNode); if (selectionBucket != null) { // Multisearch are not supposed to work with a bucket. // Only one request is store in session - session.setProperty(Geonet.Session.SEARCH_REQUEST + selectionBucket, node); + session.setProperty(Geonet.Session.SEARCH_REQUEST + selectionBucket, jsonNode); } } - final JsonNode sourceNode = node.get("_source"); + final JsonNode sourceNode = jsonNode.get("_source"); if (sourceNode != null) { if (sourceNode.isArray()) { addRequiredField((ArrayNode) sourceNode); @@ -451,7 +453,7 @@ private void call(ServiceContext context, HttpSession httpSession, HttpServletRe } } } - requestBody.append(node.toString()).append(System.lineSeparator()); + requestBody.append(jsonNode.toString()).append(System.lineSeparator()); } handleRequest(context, httpSession, request, response, url, endPoint, requestBody.toString(), true, selectionBucket, relatedTypes); @@ -527,7 +529,7 @@ private void insertFilter(ObjectNode objectNode, JsonNode nodeFilter) { * Add search privilege criteria to a query. */ private String buildQueryFilter(ServiceContext context, String type, boolean isSearchingForDraft) throws Exception { - return String.format(filterTemplate, + return String.format(FILTER_TEMPLATE, EsFilterBuilder.build(context, type, isSearchingForDraft, node)); } @@ -586,7 +588,7 @@ private void handleRequest(ServiceContext context, "Error is: %s.\nRequest:\n%s.\nError:\n%s.", connectionWithFinalHost.getResponseMessage(), requestBody, - IOUtils.toString(errorDetails) + IOUtils.toString(errorDetails, StandardCharsets.UTF_8) )); return; } @@ -601,14 +603,13 @@ private void handleRequest(ServiceContext context, // content type has to be valid if (!isContentTypeValid(contentType)) { - if (connectionWithFinalHost.getResponseMessage() != null) { - if (connectionWithFinalHost.getResponseMessage().equalsIgnoreCase("Not Found")) { - // content type was not valid because it was a not found page (text/html) - response.sendError(HttpServletResponse.SC_NOT_FOUND, "Remote host not found"); - return; - } + if (connectionWithFinalHost.getResponseMessage() != null && connectionWithFinalHost.getResponseMessage().equalsIgnoreCase("Not Found")) { + // content type was not valid because it was a not found page (text/html) + response.sendError(HttpServletResponse.SC_NOT_FOUND, "Remote host not found"); + return; } + response.sendError(HttpServletResponse.SC_FORBIDDEN, "The content type of the remote host's response \"" + contentType + "\" is not allowed by the proxy rules"); @@ -686,7 +687,7 @@ private void processResponse(ServiceContext context, HttpSession httpSession, addSelectionInfo(doc, selections); } - if ((relatedTypes != null ) && (relatedTypes.length > 0)) { + if ((relatedTypes != null) && (relatedTypes.length > 0)) { addRelatedTypes(doc, relatedTypes, context); } @@ -713,7 +714,7 @@ private void processResponse(ServiceContext context, HttpSession httpSession, addSelectionInfo(doc, selections); } - if ((relatedTypes != null ) && (relatedTypes.length > 0)) { + if ((relatedTypes != null) && (relatedTypes.length > 0)) { addRelatedTypes(doc, relatedTypes, context); } @@ -741,13 +742,11 @@ private void processResponse(ServiceContext context, HttpSession httpSession, */ private String getContentEncoding(Map> headerFields) { for (String headerName : headerFields.keySet()) { - if (headerName != null) { - if ("Content-Encoding".equalsIgnoreCase(headerName)) { - List valuesList = headerFields.get(headerName); - StringBuilder sBuilder = new StringBuilder(); - valuesList.forEach(sBuilder::append); - return sBuilder.toString().toLowerCase(); - } + if (headerName != null && "Content-Encoding".equalsIgnoreCase(headerName)) { + List valuesList = headerFields.get(headerName); + StringBuilder sBuilder = new StringBuilder(); + valuesList.forEach(sBuilder::append); + return sBuilder.toString().toLowerCase(); } } return null; @@ -819,7 +818,7 @@ protected boolean isContentTypeValid(final String contentType) { // focus only on type, not on the text encoding String type = contentType.split(";")[0]; - for (String validTypeContent : EsHTTPProxy._validContentTypes) { + for (String validTypeContent : EsHTTPProxy.validContentTypes) { if (validTypeContent.equals(type)) { return true; } @@ -831,29 +830,29 @@ protected boolean isContentTypeValid(final String contentType) { /** * Process the metadata schema filters to filter out from the Elasticsearch response * the elements defined in the metadata schema filters. - * + *

    * It uses a jsonpath to filter the elements, typically is configured with the following jsonpath, to * filter the ES object elements with an attribute nilReason = 'withheld'. - * - * $.*[?(@.nilReason == 'withheld')] - * + *

    + * $.*[?(@.nilReason == 'withheld')] + *

    * The metadata index process, has to define this attribute. Any element that requires to be filtered, should be * defined as an object in Elasticsearch. - * + *

    * Example for contacts: - * - * - * ... - * - * - * - * - * { - * ... - * "address":"" - * - * ,"nilReason": "withheld" - * + *

    + * + * ... + * + * + *

    + * + * { + * ... + * "address":"" + * + * ,"nilReason": "withheld" + * * * @param mds * @param doc @@ -908,10 +907,11 @@ private void processMetadataSchemaFilters(ServiceContext context, MetadataSchema doc.set("_source", actualObj); } } + private JsonNode filterResponseElements(ObjectMapper mapper, ObjectNode sourceNode, List jsonPathFilters) throws JsonProcessingException { DocumentContext jsonContext = JsonPath.parse(sourceNode.toPrettyString()); - for(String jsonPath : jsonPathFilters) { + for (String jsonPath : jsonPathFilters) { if (StringUtils.isNotBlank(jsonPath)) { try { jsonContext = jsonContext.delete(jsonPath); diff --git a/web-ui/src/main/resources/catalog/locales/en-admin.json b/web-ui/src/main/resources/catalog/locales/en-admin.json index ac324daf305..c83ee5e21a4 100644 --- a/web-ui/src/main/resources/catalog/locales/en-admin.json +++ b/web-ui/src/main/resources/catalog/locales/en-admin.json @@ -738,6 +738,8 @@ "system/metadataprivs/publication/notificationLevel-help": "Define which users to alert when a metadata is published / unpublished", "system/metadataprivs/publication/notificationGroups": "Groups to notify when a metadata is published / unpublished", "system/metadataprivs/publication/notificationGroups-help": "List of groups, separated by the char |, to notify when a metadata is published / unpublished (for 'Notify the group(s) emails' notification level)", + "system/metadataprivs/userAlwaysCanEditOwnedMetadata": "Users can always edit their own metadata", + "system/metadataprivs/userAlwaysCanEditOwnedMetadata-help": "If enabled (default) a user can always edit the metadata records he owns. Otherwise, a user can only edit metadata if they have edit permissions for the metadata in the groups in which the user has the Editor role.", "system/metadatacreate" : "Metadata create", "system/metadatacreate/generateUuid" : "Generate UUID", "system/metadatacreate/generateUuid-help" : "GeoNetwork assigns a random metadata UUID to the metadata created by a user (default). To assign the metadata UUID manually disable this option and fill the metadata UUID prefix.", diff --git a/web/src/main/webapp/WEB-INF/classes/setup/sql/data/data-db-default.sql b/web/src/main/webapp/WEB-INF/classes/setup/sql/data/data-db-default.sql index d9a267e3d93..aac7df0cb7c 100644 --- a/web/src/main/webapp/WEB-INF/classes/setup/sql/data/data-db-default.sql +++ b/web/src/main/webapp/WEB-INF/classes/setup/sql/data/data-db-default.sql @@ -663,6 +663,7 @@ INSERT INTO Settings (name, value, datatype, position, internal) VALUES ('system INSERT INTO Settings (name, value, datatype, position, internal) VALUES ('system/metadataprivs/publicationbyrevieweringroupowneronly', 'true', 2, 9181, 'n'); INSERT INTO Settings (name, value, datatype, position, internal) VALUES ('system/metadataprivs/publication/notificationLevel', '', 0, 9182, 'n'); INSERT INTO Settings (name, value, datatype, position, internal) VALUES ('system/metadataprivs/publication/notificationGroups', '', 0, 9183, 'n'); +INSERT INTO Settings (name, value, datatype, position, internal) VALUES ('system/metadataprivs/userAlwaysCanEditOwnedMetadata', 'true', 2, 9184, 'n'); INSERT INTO Settings (name, value, datatype, position, internal) VALUES ('system/threadedindexing/maxthreads', '1', 1, 9210, 'y'); INSERT INTO Settings (name, value, datatype, position, internal) VALUES ('system/inspire/remotevalidation/url', '', 0, 7211, 'n'); diff --git a/web/src/main/webapp/WEB-INF/classes/setup/sql/migrate/v447/migrate-default.sql b/web/src/main/webapp/WEB-INF/classes/setup/sql/migrate/v447/migrate-default.sql index a2a75e37d54..428e5d68fa3 100644 --- a/web/src/main/webapp/WEB-INF/classes/setup/sql/migrate/v447/migrate-default.sql +++ b/web/src/main/webapp/WEB-INF/classes/setup/sql/migrate/v447/migrate-default.sql @@ -3,3 +3,4 @@ UPDATE Settings SET value='SNAPSHOT' WHERE name='system/platform/subVersion'; INSERT INTO Settings (name, value, datatype, position, internal) VALUES ('system/banner/enable', 'false', 2, 1920, 'n'); INSERT INTO Settings (name, value, datatype, position, internal) VALUES ('system/auditable/enable', 'false', 2, 12010, 'n'); +INSERT INTO Settings (name, value, datatype, position, internal) VALUES ('system/metadataprivs/userAlwaysCanEditOwnedMetadata', 'true', 2, 9184, 'n');