Implementation of Code Review requests. New format for override key. Strip validate_application_header_before_entity_create from all before_entity_update_validator entries. Comments added for clarity. Consolidate entity validator args to one dict. Remove block on modifying public Collections from previous Issue, so now blocked by new validator and block can be overridden.

Author: kburke

src/app.py

@@ -93,11 +93,6 @@
 # updates to entities with characteristics prohibiting their modification.
 LOCKED_ENTITY_UPDATE_OVERRIDE_KEY = app.config['LOCKED_ENTITY_UPDATE_OVERRIDE_KEY']
 
-# Compile the only accept regular expression pattern for the lockout override key in an HTTP Request Header.
-# N.B. This is case-insensitive matching "Override-Key", but case-sensitive matching the value.
-LOCKED_ENTITY_UPDATE_OVERRIDE_PATTERN = rf"^\s*(?i:override-key)\s+{re.escape(LOCKED_ENTITY_UPDATE_OVERRIDE_KEY)}\s*$"
-LOCKED_ENTITY_UPDATE_OVERRIDE_REGEX = re.compile(pattern=LOCKED_ENTITY_UPDATE_OVERRIDE_PATTERN)
-
 # Suppress InsecureRequestWarning warning when requesting status on https with ssl cert verify disabled
 requests.packages.urllib3.disable_warnings(category = InsecureRequestWarning)
 
@@ -1372,9 +1367,6 @@ def update_entity(id):
     # Normalize user provided entity_type
     normalized_entity_type = schema_manager.normalize_entity_type(entity_dict['entity_type'])
 
-    # Note, we don't support entity level validators on entity update via PUT
-    # Only entity create via POST is supported at the entity level
-    # KBKBKB...or do we?
     # Execute entity level validator defined in schema yaml before entity modification.
     lockout_overridden = False
     try:
@@ -1389,13 +1381,15 @@ def update_entity(id):
     except schema_errors.LockedEntityUpdateException as leue:
         # HTTP header names are case-insensitive, and request.headers.get() returns None if the header doesn't exist
         locked_entity_update_header = request.headers.get(SchemaConstants.LOCKED_ENTITY_UPDATE_HEADER)
-        if locked_entity_update_header and bool(LOCKED_ENTITY_UPDATE_OVERRIDE_REGEX.match(locked_entity_update_header)):
+        if locked_entity_update_header and (LOCKED_ENTITY_UPDATE_OVERRIDE_KEY == locked_entity_update_header):
             lockout_overridden = True
             logger.info(f"For {entity_dict['entity_type']} {entity_dict['uuid']}"
                         f" update prohibited due to {str(leue)},"
                         f" but being overridden by valid {SchemaConstants.LOCKED_ENTITY_UPDATE_HEADER} in request.")
         else:
             forbidden_error(leue)
+    except Exception as e:
+        internal_server_error(e)
 
     # Validate request json against the yaml schema
     # Pass in the entity_dict for missing required key check, this is different from creating new entity
@@ -1414,6 +1408,9 @@ def update_entity(id):
             ValueError) as e:
         bad_request_error(e)
 
+    # Proceed with per-entity updates after passing any entity-level or property-level validations which
+    # would have locked out updates.
+    #
     # Sample, Dataset, and Upload: additional validation, update entity, after_update_trigger
     # Collection and Donor: update entity
     if normalized_entity_type == 'Sample':
@@ -1498,13 +1495,6 @@ def update_entity(id):
         if has_dataset_uuids_to_link or has_dataset_uuids_to_unlink or has_updated_status:
             after_update(normalized_entity_type, user_token, merged_updated_dict)
     elif schema_manager.entity_type_instanceof(normalized_entity_type, 'Collection'):
-        entity_visibility = _get_entity_visibility(  normalized_entity_type=normalized_entity_type
-                                                    ,entity_dict=entity_dict)
-        # Prohibit update of an existing Collection if it meets criteria of being visible to public e.g. has DOI.
-        if entity_visibility == DataVisibilityEnum.PUBLIC:
-            logger.info(f"Attempt to update {normalized_entity_type} with id={id} which has visibility {entity_visibility}.")
-            bad_request_error(f"Cannot update {normalized_entity_type} due '{entity_visibility.value}' visibility.")
-
         # Generate 'before_update_trigger' data and update the entity details in Neo4j
         merged_updated_dict = update_entity_details(request, normalized_entity_type, user_token, json_data_dict, entity_dict)
 

src/instance/app.cfg.example

@@ -28,7 +28,7 @@ NEO4J_PASSWORD = '123'
 
 # Secret value presented with the request header value named by
 # SchemaConstants.LOCKED_ENTITY_UPDATE_HEADER, expected to be off the form
-# X-HuBMAP-Update-Override: Override-Key <LOCKED_ENTITY_UPDATE_OVERRIDE_KEY value which follows>
+# X-HuBMAP-Update-Override: <LOCKED_ENTITY_UPDATE_OVERRIDE_KEY value which follows>
 LOCKED_ENTITY_UPDATE_OVERRIDE_KEY = 'set during deployment'
 
 # Set MEMCACHED_MODE to False to disable the caching for local development

src/schema/provenance_schema.yaml

@@ -194,8 +194,6 @@ ENTITIES:
   ############################################# Collection #############################################
   Collection:
     before_entity_update_validator:
-      # Only allowed applications can modify an existing Collection via PUT # KBKBKB @TODO confirm introducing this
-      - validate_application_header_before_entity_create
       # Halt modification of entities which are "locked", such as a Dataset with status == 'Published'
       - validate_entity_not_locked_before_update
     excluded_properties_from_public_response:
@@ -311,8 +309,6 @@ ENTITIES:
     # Only allowed applications can create new Dataset via POST
     before_entity_create_validator: validate_application_header_before_entity_create
     before_entity_update_validator:
-      # Only allowed applications can modify an existing Dataset via PUT
-      - validate_application_header_before_entity_create
       # Halt modification of entities which are "locked", such as a Dataset with status == 'Published'
       - validate_entity_not_locked_before_update
     # Dataset can be either derivation source or target
@@ -671,8 +667,6 @@ ENTITIES:
     # Only allowed applications can create new Publication via POST
     before_entity_create_validator: validate_application_header_before_entity_create
     before_entity_update_validator:
-      # Only allowed applications can modify an existing Collection via PUT # KBKBKB @TODO confirm introducing this
-      - validate_application_header_before_entity_create
       # Halt modification of entities which are "locked", such as a Dataset with status == 'Published'
       - validate_entity_not_locked_before_update
     # Publications can be either derivation source or target
@@ -780,8 +774,6 @@ ENTITIES:
       - submission_id
       - label
     before_entity_update_validator:
-      # Only allowed applications can modify an existing Collection via PUT # KBKBKB @TODO confirm introducing this
-      - validate_application_header_before_entity_create
       # Halt modification of entities which are "locked", such as a Dataset with status == 'Published'
       - validate_entity_not_locked_before_update
     properties:
@@ -918,8 +910,6 @@ ENTITIES:
         # Both Sample and Donor ancestors of a Sample must have these fields removed
         - submission_id
     before_entity_update_validator:
-      # Only allowed applications can modify an existing Collection via PUT # KBKBKB @TODO confirm introducing this
-      - validate_application_header_before_entity_create
       # Halt modification of entities which are "locked", such as a Dataset with status == 'Published'
       - validate_entity_not_locked_before_update
     properties:
@@ -1146,7 +1136,10 @@ ENTITIES:
   Upload:
     # Only allowed applications can create new Upload via POST
     before_entity_create_validator: validate_application_header_before_entity_create
-    # Upload requires an ancestor of Lab, and and has no allowed decesndents 
+    # No before_entity_update_validator needed for Upload because the entity is
+    # always considered "non-public", and therefore not blocked from update/PUT.
+    #
+    # Upload requires a Lab entity as an ancestor, and has no allowed descendants
     derivation:
       source: false
       target: false # Set to false since the schema doesn't handle Lab currently
@@ -1295,8 +1288,6 @@ ENTITIES:
       source: false
       target: false
     before_entity_update_validator:
-      # Only allowed applications can modify an existing Collection via PUT # KBKBKB @TODO confirm introducing this
-      - validate_application_header_before_entity_create
       # Halt modification of entities which are "locked", such as a Dataset with status == 'Published'
       - validate_entity_not_locked_before_update
     properties:

src/schema/schema_manager.py

@@ -1171,6 +1171,8 @@ def validate_json_data_against_schema(json_data_dict, normalized_entity_type, ex
     One of the normalized entity types defined in the schema yaml: Donor, Sample, Dataset, Upload, Upload, Publication
 request: Flask request object
     The instance of Flask request passed in from application request
+existing_entity_dict : dict
+    The dictionary for an entity, retrieved from Neo4j, for use during update/PUT validations
 """
 def execute_entity_level_validator(validator_type, normalized_entity_type, request, existing_entity_dict=None):
     global _schema
@@ -1196,12 +1198,17 @@ def execute_entity_level_validator(validator_type, normalized_entity_type, reque
 
                     logger.info(f"To run {validator_type}: {validator_method_name} defined for entity {normalized_entity_type}")
 
+                    # Create a dictionary to hold data need by any entity validator, which must be populated
+                    # with validator specific requirements when the method to be called is determined.
+                    options_dict = {}
                     if existing_entity_dict is None:
                         # Execute the entity-level validation for create/POST
-                        validator_method_to_call(normalized_entity_type, request)
+                        options_dict['http_request'] = request
+                        validator_method_to_call(options_dict)
                     else:
                         # Execute the entity-level validation for update/PUT
-                        validator_method_to_call(normalized_entity_type, request, existing_entity_dict)
+                        options_dict['existing_entity_dict']= existing_entity_dict
+                        validator_method_to_call(options_dict)
                 except schema_errors.MissingApplicationHeaderException as e:
                     raise schema_errors.MissingApplicationHeaderException(e)
                 except schema_errors.InvalidApplicationHeaderException as e:
@@ -1212,6 +1219,7 @@ def execute_entity_level_validator(validator_type, normalized_entity_type, reque
                     msg = f"Failed to call the {validator_type} method: {validator_method_name} defined for entity {normalized_entity_type}"
                     # Log the full stack trace, prepend a line with our message
                     logger.exception(msg)
+                    raise e
 
 
 """

src/schema/schema_validators.py

@@ -29,7 +29,13 @@
 request: Flask request
     The instance of Flask request passed in from application request
 """
-def validate_application_header_before_entity_create(normalized_entity_type, request, existing_entity_id):
+def validate_application_header_before_entity_create(options_dict):
+    if 'http_request' in options_dict:
+        request = options_dict['http_request']
+    else:
+        logger.error(f"validate_application_header_before_entity_create() expected 'http_request' in"
+                     f" options_dict, but it was missing in {str(options_dict)}.")
+        raise KeyError("Entity validator internal misconfiguration.")
     # A list of applications allowed to create this new entity or update Dataset and Upload
     # Use lowercase for comparison
     applications_allowed = [
@@ -50,7 +56,13 @@ def validate_application_header_before_entity_create(normalized_entity_type, req
 request: Flask request
     The instance of Flask request passed in from application request
 """
-def validate_entity_not_locked_before_update(normalized_entity_type, request, existing_entity_dict):
+def validate_entity_not_locked_before_update(options_dict):
+    if 'existing_entity_dict' in options_dict:
+        existing_entity_dict = options_dict['existing_entity_dict']
+    else:
+        logger.error(f"validate_entity_not_locked_before_update() expected 'existing_entity_dict' in"
+                     f" options_dict, but it was missing in {str(options_dict)}.")
+        raise KeyError("Entity validator internal misconfiguration.")
     _is_entity_locked_against_update(existing_entity_dict)
 
 ##############################################################################################