Skip to content
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

Fix only record owners can access workflow status API #8667

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Fix only record owners can access workflow status API #8667

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
7f8a96e
Update workflow status API to check if user can view record (and is a…
tylerjmchugh Feb 19, 2025
a23590d
Change message to multilingual
tylerjmchugh Feb 19, 2025
bf00c52
remove unused messages
tylerjmchugh Feb 19, 2025
949a3c9
Update check to follow "minimum user profile allowed to view metadata…
tylerjmchugh Feb 19, 2025
f15a70b
Add missing messages
tylerjmchugh Feb 19, 2025
76ec96d
Fix minimum profile not enforced
tylerjmchugh Feb 20, 2025
1d90af4
Fix cannot get status when user cannot edit
tylerjmchugh Feb 20, 2025
9d82676
Move logic to reusable methods
tylerjmchugh Feb 20, 2025
b42bfcc
Fix french messages
tylerjmchugh Feb 20, 2025
2ec157f
Update messages to use translated profile name from json
tylerjmchugh Feb 21, 2025
a86144c
Fix record not found not handled correctly
tylerjmchugh Feb 21, 2025
bae0649
Handle record not found for getWorkflowStatusByType
tylerjmchugh Feb 21, 2025
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
4 changes: 3 additions & 1 deletion core/src/test/resources/org/fao/geonet/api/Messages.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,9 @@ api.exception.unsatisfiedRequestParameter.description=Unsatisfied request parame
exception.maxUploadSizeExceeded=Maximum upload size of {0} exceeded.
exception.maxUploadSizeExceeded.description=The request was rejected because its size ({0}) exceeds the configured maximum ({1}).
exception.maxUploadSizeExceededUnknownSize.description=The request was rejected because its size exceeds the configured maximum ({0}).
exception.notAllowed.cannotEdit=Operation not allowed. User needs to be able to edit the resource.
exception.notAllowed.cannotView=Operation not allowed. User needs to be able to view the resource.
exception.notAllowed.mustBeProfileOrOwner=Operation not allowed. User must be ''{0}'' or the owner of the resource.
exception.resourceNotFound.metadata=Metadata not found
exception.resourceNotFound.metadata.description=Metadata with UUID ''{0}'' not found.
exception.resourceNotFound.resource=Metadata resource ''{0}'' not found
Expand Down Expand Up @@ -242,7 +245,6 @@ api.metadata.share.errorMetadataNotApproved=The metadata '%s' it's not approved,
api.metadata.share.ErrorUserNotAllowedToPublish=User not allowed to publish the metadata %s. %s
api.metadata.share.strategy.groupOwnerOnly=You need to be administrator, or reviewer of the metadata group.
api.metadata.share.strategy.reviewerInGroup=You need to be administrator, or reviewer of the metadata group or reviewer with edit privilege on the metadata.
api.metadata.status.errorGetStatusNotAllowed=Only the owner of the metadata can get the status. User is not the owner of the metadata.
api.metadata.status.errorSetStatusNotAllowed=Only the owner of the metadata can set the status of this record. User is not the owner of the metadata.

feedback_subject_userFeedback=User feedback
Expand Down
4 changes: 3 additions & 1 deletion core/src/test/resources/org/fao/geonet/api/Messages_fre.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,9 @@ api.exception.unsatisfiedRequestParameter.description=Param\u00E8tre de demande
exception.maxUploadSizeExceeded=La taille maximale du t\u00E9l\u00E9chargement de {0} a \u00E9t\u00E9 exc\u00E9d\u00E9e.
exception.maxUploadSizeExceeded.description=La demande a \u00E9t\u00E9 refus\u00E9e car sa taille ({0}) exc\u00E8de le maximum configur\u00E9 ({1}).
exception.maxUploadSizeExceededUnknownSize.description=La demande a \u00E9t\u00E9 refus\u00E9e car sa taille exc\u00E8de le maximum configur\u00E9 ({0}).
exception.notAllowed.cannotEdit=Op\u00E9ration non autoris\u00E9e. L'utilisateur doit pouvoir modifier la ressource.
exception.notAllowed.cannotView=Op\u00E9ration non autoris\u00E9e. L'utilisateur doit pouvoir visualiser la ressource.
exception.notAllowed.mustBeProfileOrOwner=Op\u00E9ration non autoris\u00E9e. L''utilisateur doit �tre ''{0}'' ou le propri\u00E9taire de la ressource.
exception.resourceNotFound.metadata=Fiches introuvables
exception.resourceNotFound.metadata.description=La fiche ''{0}'' est introuvable.
exception.resourceNotFound.resource=Ressource ''{0}'' introuvable
Expand Down Expand Up @@ -235,7 +238,6 @@ api.metadata.share.errorMetadataNotApproved=La fiche '%s' n'est pas approuv\u00E
api.metadata.share.ErrorUserNotAllowedToPublish=L'utilisateur n'est pas autoris\u00E9 \u00E0 publier la fiche %s. %s
api.metadata.share.strategy.groupOwnerOnly=Vous devez \u00EAtre administrateur ou relecteur du groupe de la fiche.
api.metadata.share.strategy.reviewerInGroup=Vous devez \u00EAtre administrateur ou relecteur du groupe de la fiche ou relecteur avec un privil\u00E8ge de modification sur les fiches.
api.metadata.status.errorGetStatusNotAllowed=Seul le propri\u00E9taire des m\u00E9tadonn\u00E9es peut obtenir le statut de cet enregistrement. L'utilisateur n'est pas le propri\u00E9taire des m\u00E9tadonn\u00E9es
api.metadata.status.errorSetStatusNotAllowed=Seul le propri\u00E9taire des m\u00E9tadonn\u00E9es peut d\u00E9finir le statut de cet enregistrement. L'utilisateur n'est pas le propri\u00E9taire des m\u00E9tadonn\u00E9es

feedback_subject_userFeedback=Commentaire de l'utilisateur
Expand Down
127 changes: 96 additions & 31 deletions services/src/main/java/org/fao/geonet/api/records/MetadataWorkflowApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@
import org.fao.geonet.kernel.metadata.StatusChangeType;
import org.fao.geonet.kernel.search.EsSearchManager;
import org.fao.geonet.kernel.search.IndexingMode;
import org.fao.geonet.kernel.search.Translator;
import org.fao.geonet.kernel.search.TranslatorFactory;
import org.fao.geonet.kernel.setting.SettingManager;
import org.fao.geonet.kernel.setting.Settings;
import org.fao.geonet.languages.FeedbackLanguages;
Expand All @@ -82,6 +84,7 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpSession;
import java.text.MessageFormat;
import java.util.*;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -160,6 +163,9 @@ public class MetadataWorkflowApi {
@Autowired
RoleHierarchy roleHierarchy;

@Autowired
TranslatorFactory translatorFactory;

// The restore function currently supports these states
static final StatusValue.Events[] supportedRestoreStatuses = StatusValue.Events.getSupportedRestoreStatuses();

Expand All @@ -180,13 +186,13 @@ public List<MetadataStatusResponse> getRecordStatusHistory(
@RequestParam(required = false, defaultValue = "true") Boolean approved,
HttpServletRequest request) throws Exception {
ServiceContext context = ApiUtils.createServiceContext(request);

ResourceBundle messages = ApiUtils.getMessagesResourceBundle(request.getLocales());
AbstractMetadata metadata;
try {
metadata = ApiUtils.canViewRecord(metadataUuid, approved, request);
} catch (SecurityException e) {
Log.debug(API.LOG_MODULE_NAME, e.getMessage(), e);
throw new NotAllowedException(ApiParams.API_RESPONSE_NOT_ALLOWED_CAN_VIEW);
throw new NotAllowedException(messages.getString("exception.notAllowed.cannotView"));
}

String sortField = SortUtils.createPath(MetadataStatus_.changeDate);
Expand All @@ -212,12 +218,13 @@ public List<MetadataStatusResponse> getRecordStatusHistoryByType(
@RequestParam(required = false, defaultValue = "true") Boolean approved,
HttpServletRequest request) throws Exception {
ServiceContext context = ApiUtils.createServiceContext(request);
ResourceBundle messages = ApiUtils.getMessagesResourceBundle(request.getLocales());
AbstractMetadata metadata;
try {
metadata = ApiUtils.canViewRecord(metadataUuid, approved, request);
} catch (SecurityException e) {
Log.debug(API.LOG_MODULE_NAME, e.getMessage(), e);
throw new NotAllowedException(ApiParams.API_RESPONSE_NOT_ALLOWED_CAN_VIEW);
throw new NotAllowedException(messages.getString("exception.notAllowed.cannotView"));
}

String sortField = SortUtils.createPath(MetadataStatus_.changeDate);
Expand All @@ -233,7 +240,7 @@ public List<MetadataStatusResponse> getRecordStatusHistoryByType(
@io.swagger.v3.oas.annotations.Operation(summary = "Get last workflow status for a record", description = "")
@RequestMapping(value = "/{metadataUuid}/status/workflow/last", method = RequestMethod.GET, produces = {
MediaType.APPLICATION_JSON_VALUE})
@PreAuthorize("hasAuthority('Editor')")
@PreAuthorize("hasAuthority('RegisteredUser')")
@ApiResponses(value = {@ApiResponse(responseCode = "200", description = "Record status."),
@ApiResponse(responseCode = "403", description = ApiParams.API_RESPONSE_NOT_ALLOWED_CAN_EDIT)})
@ResponseStatus(HttpStatus.OK)
Expand All @@ -243,15 +250,28 @@ public MetadataWorkflowStatusResponse getStatus(
@Parameter(description = "Use approved version or not", example = "true")
@RequestParam(required = false, defaultValue = "true") Boolean approved,
HttpServletRequest request) throws Exception {
AbstractMetadata metadata = ApiUtils.canEditRecord(metadataUuid, approved, request);
AbstractMetadata metadata = ApiUtils.getRecord(metadataUuid);
Locale locale = languageUtils.parseAcceptLanguage(request.getLocales());
ResourceBundle messages = ApiUtils.getMessagesResourceBundle(request.getLocales());
ServiceContext context = ApiUtils.createServiceContext(request, locale.getISO3Language());

// --- only allow the owner of the record to set its status
// If the user does not own the record check if they meet the minimum profile
if (!accessManager.isOwner(context, String.valueOf(metadata.getId()))) {
throw new SecurityException(
messages.getString("api.metadata.status.errorGetStatusNotAllowed"));
Profile userProfile = context.getUserSession().getProfile();
String minimumAllowedProfileName = StringUtils.defaultIfBlank(
settingManager.getValue(Settings.METADATA_HISTORY_ACCESS_LEVEL),
Profile.Editor.toString()
);
Profile minimumAllowedProfile = Profile.valueOf(minimumAllowedProfileName);

if (!minimumAllowedProfile.getProfileAndAllParents().contains(userProfile)) {
// If the user profile is not at least the minimum profile, then the user is not allowed to view record workflow status
String message = getMustBeProfileOrOwnerMessage(minimumAllowedProfileName, messages);
Log.debug(API.LOG_MODULE_NAME, message);
throw new NotAllowedException(message);
}

checkUserCanSeeHistory(minimumAllowedProfile, metadataUuid, messages, request);
}

MetadataStatus recordStatus = metadataStatus.getStatus(metadata.getId());
Expand Down Expand Up @@ -720,12 +740,18 @@ public List<MetadataStatusResponse> getWorkflowStatusByType(
Integer size,
HttpServletRequest request) throws Exception {
ServiceContext context = ApiUtils.createServiceContext(request);
ResourceBundle messages = ApiUtils.getMessagesResourceBundle(request.getLocales());

Profile profile = context.getUserSession().getProfile();
String allowedProfileLevel = org.apache.commons.lang.StringUtils.defaultIfBlank(settingManager.getValue(Settings.METADATA_HISTORY_ACCESS_LEVEL), Profile.Editor.toString());
Profile allowedAccessLevelProfile = Profile.valueOf(allowedProfileLevel);
Profile userProfile = context.getUserSession().getProfile();
String minimumAllowedProfileName = StringUtils.defaultIfBlank(
settingManager.getValue(Settings.METADATA_HISTORY_ACCESS_LEVEL),
Profile.Editor.toString()
);
Profile minimumAllowedProfile = Profile.valueOf(minimumAllowedProfileName);
boolean isMinimumAllowedProfile = minimumAllowedProfile.getProfileAndAllParents().contains(userProfile);
String mustBeProfileOrOwnerMessage = getMustBeProfileOrOwnerMessage(minimumAllowedProfileName, messages);

if (profile != Profile.Administrator) {
if (userProfile != Profile.Administrator) {
if (CollectionUtils.isEmpty(recordIdentifier) &&
CollectionUtils.isEmpty(uuid)) {
throw new NotAllowedException(
Expand All @@ -734,30 +760,24 @@ public List<MetadataStatusResponse> getWorkflowStatusByType(

if (!CollectionUtils.isEmpty(recordIdentifier)) {
for (Integer recordId : recordIdentifier) {
try {
if (allowedAccessLevelProfile == Profile.RegisteredUser) {
ApiUtils.canViewRecord(String.valueOf(recordId), request);
} else {
ApiUtils.canEditRecord(String.valueOf(recordId), request);
}

} catch (SecurityException e) {
throw new NotAllowedException(ApiParams.API_RESPONSE_NOT_ALLOWED_CAN_EDIT);
// Handle record not found
AbstractMetadata metadata = ApiUtils.getRecord(String.valueOf(recordId));
if (!isMinimumAllowedProfile && !accessManager.isOwner(context, metadata.getSourceInfo())) {
Log.debug(API.LOG_MODULE_NAME, mustBeProfileOrOwnerMessage);
throw new NotAllowedException(mustBeProfileOrOwnerMessage);
}
checkUserCanSeeHistory(minimumAllowedProfile, String.valueOf(recordId), messages, request);
}
}
if (!CollectionUtils.isEmpty(uuid)) {
for (String recordId : uuid) {
try {
if (allowedAccessLevelProfile == Profile.RegisteredUser) {
ApiUtils.canViewRecord(recordId, request);
} else {
ApiUtils.canEditRecord(recordId, request);
}

} catch (SecurityException e) {
throw new NotAllowedException(ApiParams.API_RESPONSE_NOT_ALLOWED_CAN_EDIT);
// Handle record not found
AbstractMetadata metadata = ApiUtils.getRecord(recordId);
if (!isMinimumAllowedProfile && !accessManager.isOwner(context, metadata.getSourceInfo())) {
Log.debug(API.LOG_MODULE_NAME, mustBeProfileOrOwnerMessage);
throw new NotAllowedException(mustBeProfileOrOwnerMessage);
}
checkUserCanSeeHistory(minimumAllowedProfile, recordId, messages, request);
}
}
}
Expand Down Expand Up @@ -1245,6 +1265,8 @@ private MetadataStatus getMetadataStatus(String uuidOrInternalId, int statusId,

private String getValidatedStateText(MetadataStatus metadataStatus, State state, HttpServletRequest request, HttpSession httpSession) throws Exception {

ResourceBundle messages = ApiUtils.getMessagesResourceBundle(request.getLocales());

if (!StatusValueType.event.equals(metadataStatus.getStatusValue().getType())
|| !ArrayUtils.contains(supportedRestoreStatuses, StatusValue.Events.fromId(metadataStatus.getStatusValue().getId()))) {
throw new NotAllowedException("Unsupported action on status type '" + metadataStatus.getStatusValue().getType()
Expand Down Expand Up @@ -1281,7 +1303,7 @@ private String getValidatedStateText(MetadataStatus metadataStatus, State state,
ApiUtils.canEditRecord(metadataStatus.getUuid(), request);
} catch (SecurityException e) {
Log.debug(API.LOG_MODULE_NAME, e.getMessage(), e);
throw new NotAllowedException(ApiParams.API_RESPONSE_NOT_ALLOWED_CAN_VIEW);
throw new NotAllowedException(messages.getString("exception.notAllowed.cannotView"));
} catch (ResourceNotFoundException e) {
// If metadata record does not exists then it was deleted so
// we will only allow the administrator, owner to view the contents
Expand Down Expand Up @@ -1363,4 +1385,47 @@ private void changeMetadataStatus(ServiceContext context, AbstractMetadata metad
sa.onStatusChange(listOfStatusChange, true);
}

/**
* Constructs a message indicating that the user must have a specific profile or be the owner to perform an action.
*
* @param messages The resource bundle containing localized messages.
* @param minimumAllowedProfileName The name of the minimum allowed profile.
* @return A formatted message indicating the required profile or ownership.
*/
private String getMustBeProfileOrOwnerMessage(String minimumAllowedProfileName, ResourceBundle messages) {
Translator jsonLocTranslator = translatorFactory.getTranslator("apploc:", messages.getLocale().getISO3Language());
return MessageFormat.format(
messages.getString("exception.notAllowed.mustBeProfileOrOwner"),
jsonLocTranslator.translate(minimumAllowedProfileName)
);
}

/**
* Checks if the user has the necessary permissions to view the history of a record.
*
* @param minimumAllowedProfile The minimum profile required to view the history.
* @param recordId The ID of the record.
* @param messages The resource bundle containing localized messages.
* @param request The HTTP request object.
* @throws Exception If the user does not have the necessary permissions.
*/
private void checkUserCanSeeHistory(Profile minimumAllowedProfile, String recordId, ResourceBundle messages, HttpServletRequest request) throws Exception {
if (minimumAllowedProfile == Profile.RegisteredUser) {
// If the minimum profile is RegisteredUser, then the user must be able to view the record
try {
ApiUtils.canViewRecord(recordId, request);
} catch (SecurityException e) {
Log.debug(API.LOG_MODULE_NAME, e.getMessage(), e);
throw new NotAllowedException(messages.getString("exception.notAllowed.cannotView"));
}
} else if (minimumAllowedProfile == Profile.Editor) {
// If the minimum profile is Editor, then the user must be able to edit the record
try {
ApiUtils.canEditRecord(recordId, request);
} catch (SecurityException e) {
Log.debug(API.LOG_MODULE_NAME, e.getMessage(), e);
throw new NotAllowedException(messages.getString("exception.notAllowed.cannotEdit"));
}
}
}
}
4 changes: 3 additions & 1 deletion web/src/main/webapp/WEB-INF/classes/org/fao/geonet/api/Messages.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,9 @@ api.exception.unsatisfiedRequestParameter.description=Unsatisfied request parame
exception.maxUploadSizeExceeded=Maximum upload size of {0} exceeded.
exception.maxUploadSizeExceeded.description=The request was rejected because its size ({0}) exceeds the configured maximum ({1}).
exception.maxUploadSizeExceededUnknownSize.description=The request was rejected because its size exceeds the configured maximum ({0}).
exception.notAllowed.cannotEdit=Operation not allowed. User needs to be able to edit the resource.
exception.notAllowed.cannotView=Operation not allowed. User needs to be able to view the resource.
exception.notAllowed.mustBeProfileOrOwner=Operation not allowed. User must be ''{0}'' or the owner of the resource.
exception.resourceNotFound.metadata=Metadata not found
exception.resourceNotFound.metadata.description=Metadata with UUID ''{0}'' not found.
exception.resourceNotFound.resource=Metadata resource ''{0}'' not found
Expand Down Expand Up @@ -250,7 +253,6 @@ api.metadata.share.errorMetadataNotApproved=The metadata '%s' is not approved, c
api.metadata.share.ErrorUserNotAllowedToPublish=User not allowed to publish the metadata %s. %s
api.metadata.share.strategy.groupOwnerOnly=You need to be administrator, or reviewer of the metadata group.
api.metadata.share.strategy.reviewerInGroup=You need to be administrator, or reviewer of the metadata group or reviewer with edit privilege on the metadata.
api.metadata.status.errorGetStatusNotAllowed=Only the owner of the metadata can get the status. User is not the owner of the metadata.
api.metadata.status.errorSetStatusNotAllowed=Only the owner of the metadata can set the status of this record. User is not the owner of the metadata.

feedback_subject_userFeedback=User feedback
Expand Down
Loading