Revert "Further Sanitize User Input (#425)"

This reverts commit 0bdd433.

VERSION

@@ -1 +1 @@
-6.5.7
+6.5.5

confidant/routes/credentials.py

@@ -594,8 +594,9 @@ def create_credential():
     '''
     with stats.timer('create_credential'):
         if not acl_module_check(resource_type='credential', action='create'):
-            msg = f"{authnz.get_logged_in_user()}"
-            msg += "does not have access to create credentials"
+            msg = "{} does not have access to create credentials".format(
+                authnz.get_logged_in_user()
+            )
             error_msg = {'error': msg}
             return jsonify(error_msg), 403
 
@@ -620,7 +621,7 @@ def create_credential():
         for cred in Credential.data_type_date_index.query(
                 'credential', filter_condition=Credential.name == data['name']):
             # Conflict, the name already exists
-            msg = f'Name already exists. See id: {cred.id}'
+            msg = 'Name already exists. See id: {0}'.format(cred.id)
             return jsonify({'error': msg, 'reference': cred.id}), 409
         # Generate an initial stable ID to allow name changes
         id = str(uuid.uuid4()).replace('-', '')
@@ -635,20 +636,13 @@ def create_credential():
         credential_pairs = cipher.encrypt(credential_pairs)
         last_rotation_date = misc.utcnow()
 
-        metadata = data.get('metadata', {})
-        for key, value in metadata.items():
-            value = escape(value)
-            metadata[key] = value
-
-        data['documentation'] = escape(data.get('documentation'))
-
         sanitized_name = escape(data['name'])
         cred = Credential(
-            id=f'{id}-{revision}',
+            id='{0}-{1}'.format(id, revision),
             data_type='archive-credential',
             name=sanitized_name,
             credential_pairs=credential_pairs,
-            metadata=metadata,
+            metadata=data.get('metadata'),
             revision=revision,
             enabled=data.get('enabled'),
             data_key=data_key['ciphertext'],
@@ -803,8 +797,10 @@ def update_credential(id):
         if not acl_module_check(resource_type='credential',
                                 action='update',
                                 resource_id=id):
-            msg = f"{authnz.get_logged_in_user()}"
-            msg += f"does not have access to update credential {id}"
+            msg = "{} does not have access to update credential {}".format(
+                authnz.get_logged_in_user(),
+                id
+            )
             error_msg = {'error': msg, 'reference': id}
             return jsonify(error_msg), 403
 
@@ -820,34 +816,6 @@ def update_credential(id):
         if not isinstance(data.get('metadata', {}), dict):
             return jsonify({'error': 'metadata must be a dict'}), 400
 
-        # We check for a name change and ensure it doesn't conflict with an
-        # existing credential and to ensure we don't escape the name if it
-        # hasn't changed
-        if data.get('name') != _cred.name:
-            data['name'] = escape(data.get('name'))
-            for cred in Credential.data_type_date_index.query(
-                    'credential',
-                    filter_condition=Credential.name == data['name']):
-                # Conflict, the name already exists
-                msg = f'Name already exists. See id: {cred.id}'
-                return jsonify({'error': msg, 'reference': cred.id}), 409
-
-        # Escape metadata values by checking for new metadata keys and values
-        # to ensure we don't escape values that haven't changed
-        if data.get('metadata') != _cred.metadata:
-            new_metadata = {
-                key: value
-                for key, value in data.get('metadata', {}).items()
-                if key not in _cred.metadata or
-                value != _cred.metadata.get(key)
-            }
-            for key, value in new_metadata.items():
-                value = escape(value)
-                data['metadata'][key] = value
-
-        if data.get('documentation') != _cred.documentation:
-            data['documentation'] = escape(data.get('documentation'))
-
         update = {
             'name': data.get('name', _cred.name),
             'last_rotation_date': _cred.last_rotation_date,
@@ -906,19 +874,6 @@ def update_credential(id):
             # this is a new credential pair and update last_rotation_date
             if credential_pairs != _cred.decrypted_credential_pairs:
                 update['last_rotation_date'] = misc.utcnow()
-
-                # We escape credential pairs by checking for new credential
-                # pairs and values to ensure we don't escape values that haven't
-                # changed
-                new_credential_pairs = {
-                    key: value
-                    for key, value in credential_pairs.items()
-                    if key not in _cred.decrypted_credential_pairs or
-                    value != _cred.decrypted_credential_pairs.get(key)
-                }
-                for key, value in new_credential_pairs.items():
-                    value = escape(value)
-                    credential_pairs[key] = value
             data_key = keymanager.create_datakey(encryption_context={'id': id})
             cipher = CipherManager(data_key['plaintext'], version=2)
             update['credential_pairs'] = cipher.encrypt(
@@ -928,7 +883,7 @@ def update_credential(id):
         # Try to save to the archive
         try:
             Credential(
-                id=f'{id}-{revision}',
+                id='{0}-{1}'.format(id, revision),
                 name=update['name'],
                 data_type='archive-credential',
                 credential_pairs=update['credential_pairs'],
@@ -970,8 +925,8 @@ def update_credential(id):
 
         if services:
             service_names = [x.id for x in services]
-            msg = f'Updated credential "{cred.name}"'
-            msg += f'({cred.id}); Revision {cred.revision}'
+            msg = 'Updated credential "{0}" ({1}); Revision {2}'
+            msg = msg.format(cred.name, cred.id, cred.revision)
             graphite.send_event(service_names, msg)
             webhook.send_event('credential_update', service_names, [cred.id])
         permissions = {

tests/unit/confidant/routes/credentials_test.py

@@ -501,11 +501,6 @@ def test_update_credential(mocker: MockerFixture, credential: Credential):
         'confidant.routes.credentials.Credential.get',
         return_value=credential,
     )
-    mocker.patch(
-        ('confidant.routes.credentials.Credential'
-         '.data_type_date_index.query'),
-        return_value=[],
-    )
     mocker.patch(
         ('confidant.routes.credentials.credentialmanager'
          '.get_latest_credential_revision'),
@@ -572,30 +567,7 @@ def test_update_credential(mocker: MockerFixture, credential: Credential):
     assert ret.status_code == 400
     assert 'Credential Pairs cannot be empty.' == json_data['error']
 
-    # Credential name already exists (ie: query returns a value)
-    mocker.patch(
-        'confidant.routes.credentials.Credential.data_type_date_index.query',
-        return_value=[credential],
-    )
-    ret = app.test_client().put(
-        '/v1/credentials/123',
-        headers={"Content-Type": 'application/json'},
-        data=json.dumps({
-            'name': 'me',
-            'documentation': 'doc',
-            'credential_pairs': {'key': 'value'},
-        }),
-    )
-    json_data = json.loads(ret.data)
-    assert ret.status_code == 409
-    assert 'Name already exists' in json_data['error']
-
     # All good
-    mocker.patch(
-        ('confidant.routes.credentials.Credential'
-         '.data_type_date_index.query'),
-        return_value=[],
-    )
     mocker.patch(
         ('confidant.routes.credentials.servicemanager'
          '.pair_key_conflicts_for_services'),