aplex commented on a change in pull request #3238:
URL: https://github.com/apache/gobblin/pull/3238#discussion_r586838437
##########
File path:
gobblin-restli/gobblin-flow-config-service/gobblin-flow-config-service-server/src/main/java/org/apache/gobblin/service/FlowConfigsV2Resource.java
##########
@@ -208,18 +217,59 @@ private Properties getHeaders() {
return headerProperties;
}
+ /**
+ * Check that this update or delete operation is allowed, throw a {@link
FlowConfigLoggedException} if not.
+ */
+ public void checkUpdateDeleteAllowed(FlowConfig originalFlowConfig,
FlowConfig updatedFlowConfig) {
+ List<ServiceRequester> requesterList =
this.requesterService.findRequesters(this);
+ if (updatedFlowConfig != null) {
+ checkPropertyUpdatesAllowed(requesterList, updatedFlowConfig);
+ }
+ checkRequester(originalFlowConfig, requesterList);
+ }
+
+ /**
+ * Check that the properties being updated are allowed to be updated. This
includes:
+ * 1. Checking that the requester is part of the owningGroup if it is being
modified
+ * 2. Checking if the {@link RequesterService#REQUESTER_LIST} is being
modified, and only allow it if a user is changing
+ * it to themselves.
+ */
+ public void checkPropertyUpdatesAllowed(List<ServiceRequester>
requesterList, FlowConfig updatedFlowConfig) {
+ if (requesterList == null ||
this.requesterService.isRequesterWhitelisted(requesterList)) {
+ return;
+ }
+
+ // Check that requester is part of owning group if owning group is being
updated
+ if (updatedFlowConfig.hasOwningGroup() &&
!this.groupOwnershipService.isMemberOfGroup(requesterList,
updatedFlowConfig.getOwningGroup())) {
+ throw new FlowConfigLoggedException(HttpStatus.S_401_UNAUTHORIZED,
"Requester not part of owning group specified");
+ }
+
+ if (updatedFlowConfig.hasProperties() &&
updatedFlowConfig.getProperties().containsKey(RequesterService.REQUESTER_LIST))
{
+ List<ServiceRequester> updatedRequesterList;
+ try {
+ updatedRequesterList =
RequesterService.deserialize(updatedFlowConfig.getProperties().get(RequesterService.REQUESTER_LIST));
+ } catch (Exception e) {
+ throw new FlowConfigLoggedException(HttpStatus.S_400_BAD_REQUEST,
RequesterService.REQUESTER_LIST + " property was "
+ + "provided but could not be deserialized", e);
+ }
+
+ if (!updatedRequesterList.equals(requesterList)) {
+ throw new FlowConfigLoggedException(HttpStatus.S_401_UNAUTHORIZED,
RequesterService.REQUESTER_LIST + " property may "
+ + "only be updated to yourself. Requesting user: " + requesterList
+ ", updated requester: " + updatedRequesterList);
+ }
+ }
+ }
+
/**
* Check that all {@link ServiceRequester}s in this request are contained
within the original service requester list
* or is part of the original requester's owning group when the flow was
submitted. If they are not, throw a {@link FlowConfigLoggedException} with
{@link HttpStatus#S_401_UNAUTHORIZED}.
* If there is a failure when deserializing the original requester list,
throw a {@link FlowConfigLoggedException} with
* {@link HttpStatus#S_400_BAD_REQUEST}.
- * @param requesterService the {@link RequesterService} used to verify the
requester
* @param originalFlowConfig original flow config to find original requester
* @param requesterList list of requesters for this request
*/
- public void checkRequester(
- RequesterService requesterService, FlowConfig originalFlowConfig,
List<ServiceRequester> requesterList) {
- if (requesterList == null) {
+ public void checkRequester(FlowConfig originalFlowConfig,
List<ServiceRequester> requesterList) {
+ if (requesterList == null ||
this.requesterService.isRequesterWhitelisted(requesterList)) {
Review comment:
yep, let's remove it then (if we can't figure out the reason for it to
exist from git blame). In security code it's safer to deny access in unknown or
ambiguous situations.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
