From 65571a5dd5fba825e0cd2516a13bd6d225cb24e2 Mon Sep 17 00:00:00 2001 From: Alejandro Roiz Walss Date: Tue, 13 Aug 2024 10:36:21 -0600 Subject: [PATCH 1/3] revert escape values --- confidant/routes/credentials.py | 55 ++++---------------- confidant/routes/services.py | 10 ++-- tests/unit/confidant/routes/identity_test.py | 2 +- 3 files changed, 13 insertions(+), 54 deletions(-) diff --git a/confidant/routes/credentials.py b/confidant/routes/credentials.py index 7434e1a5..570deb5d 100644 --- a/confidant/routes/credentials.py +++ b/confidant/routes/credentials.py @@ -4,7 +4,7 @@ import re import uuid -from flask import blueprints, escape, jsonify, request +from flask import blueprints, jsonify, request from pynamodb.exceptions import DoesNotExist, PutError from confidant import authnz, clients, settings @@ -628,29 +628,18 @@ def create_credential(): id = str(uuid.uuid4()).replace('-', '') # Try to save to the archive revision = 1 - for key, value in credential_pairs.items(): - value = escape(value) - credential_pairs[key] = value credential_pairs = json.dumps(credential_pairs) data_key = keymanager.create_datakey(encryption_context={'id': id}) cipher = CipherManager(data_key['plaintext'], version=2) 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}', data_type='archive-credential', - name=sanitized_name, + name=data['name'], credential_pairs=credential_pairs, - metadata=metadata, + metadata=data.get('metadata'), revision=revision, enabled=data.get('enabled'), data_key=data_key['ciphertext'], @@ -664,7 +653,7 @@ def create_credential(): cred = Credential( id=id, data_type='credential', - name=sanitized_name, + name=data['name'], credential_pairs=credential_pairs, metadata=data.get('metadata'), revision=revision, @@ -688,7 +677,10 @@ def create_credential(): include_credential_pairs=True, ) credential_response.permissions = permissions - return credential_response_schema.dumps(credential_response) + schema = credential_response_schema.dumps(credential_response) + logger.info(f"jsonify: {jsonify(schema)}") + logger.info(f"schema: {schema} ") + return schema @blueprint.route('/v1/credentials//services', methods=['GET']) @@ -826,30 +818,13 @@ def update_credential(id): # 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']): + filter_condition=Credential.name == data.get('name', _cred.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, @@ -909,18 +884,6 @@ def update_credential(id): 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( diff --git a/confidant/routes/services.py b/confidant/routes/services.py index be42c733..4d1257da 100644 --- a/confidant/routes/services.py +++ b/confidant/routes/services.py @@ -1,6 +1,6 @@ import logging -from flask import blueprints, escape, jsonify, request +from flask import blueprints, jsonify, request from pynamodb.exceptions import DoesNotExist, PutError from confidant import authnz, settings @@ -641,13 +641,9 @@ def map_service_credentials(id): filtered_credential_ids = [cred.id for cred in credentials] # Try to save to the archive - if _service: - service_id = _service.id - else: - service_id = escape(id) try: Service( - id='{0}-{1}'.format(service_id, revision), + id='{0}-{1}'.format(id, revision), data_type='archive-service', credentials=filtered_credential_ids, blind_credentials=data.get('blind_credentials'), @@ -662,7 +658,7 @@ def map_service_credentials(id): try: service = Service( - id=service_id, + id=id, data_type='service', credentials=filtered_credential_ids, blind_credentials=data.get('blind_credentials'), diff --git a/tests/unit/confidant/routes/identity_test.py b/tests/unit/confidant/routes/identity_test.py index 329cb997..c14fc680 100644 --- a/tests/unit/confidant/routes/identity_test.py +++ b/tests/unit/confidant/routes/identity_test.py @@ -63,7 +63,7 @@ def acl_module_check(resource_type: str, action: str) -> Union[bool, None]: 'xsrf_cookie_name': 'CSRF_TOKEN', 'maintenance_mode': True, 'history_page_limit': 50, - 'defined_tags': [], + 'defined_tags': ['ROTATION_EXCLUDED', 'FINANCIALLY_SENSITIVE'], 'permissions': { 'credentials': { 'list': True, From ddecc3f5df8fd1678831b43e0b4ce24dfda923c6 Mon Sep 17 00:00:00 2001 From: Alejandro Roiz Walss Date: Tue, 13 Aug 2024 10:44:17 -0600 Subject: [PATCH 2/3] cleaner code --- confidant/routes/credentials.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/confidant/routes/credentials.py b/confidant/routes/credentials.py index 570deb5d..44073b3b 100644 --- a/confidant/routes/credentials.py +++ b/confidant/routes/credentials.py @@ -637,7 +637,7 @@ def create_credential(): cred = Credential( id=f'{id}-{revision}', data_type='archive-credential', - name=data['name'], + name=data.get('name'), credential_pairs=credential_pairs, metadata=data.get('metadata'), revision=revision, @@ -653,7 +653,7 @@ def create_credential(): cred = Credential( id=id, data_type='credential', - name=data['name'], + name=data.get('name'), credential_pairs=credential_pairs, metadata=data.get('metadata'), revision=revision, @@ -677,10 +677,7 @@ def create_credential(): include_credential_pairs=True, ) credential_response.permissions = permissions - schema = credential_response_schema.dumps(credential_response) - logger.info(f"jsonify: {jsonify(schema)}") - logger.info(f"schema: {schema} ") - return schema + return credential_response_schema.dumps(credential_response) @blueprint.route('/v1/credentials//services', methods=['GET']) @@ -815,12 +812,11 @@ def update_credential(id): 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 + # existing credential name if data.get('name') != _cred.name: for cred in Credential.data_type_date_index.query( 'credential', - filter_condition=Credential.name == data.get('name', _cred.name)): + filter_condition=Credential.name == data.get('name')): # Conflict, the name already exists msg = f'Name already exists. See id: {cred.id}' return jsonify({'error': msg, 'reference': cred.id}), 409 From 609ae13a35ce8c71442af0c8c97402cee01833a1 Mon Sep 17 00:00:00 2001 From: Alejandro Roiz Walss Date: Tue, 13 Aug 2024 10:51:48 -0600 Subject: [PATCH 3/3] fix tests --- tests/unit/confidant/routes/identity_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/confidant/routes/identity_test.py b/tests/unit/confidant/routes/identity_test.py index c14fc680..329cb997 100644 --- a/tests/unit/confidant/routes/identity_test.py +++ b/tests/unit/confidant/routes/identity_test.py @@ -63,7 +63,7 @@ def acl_module_check(resource_type: str, action: str) -> Union[bool, None]: 'xsrf_cookie_name': 'CSRF_TOKEN', 'maintenance_mode': True, 'history_page_limit': 50, - 'defined_tags': ['ROTATION_EXCLUDED', 'FINANCIALLY_SENSITIVE'], + 'defined_tags': [], 'permissions': { 'credentials': { 'list': True,