From 21992143239a10617e0baf5630db8315c5844ae1 Mon Sep 17 00:00:00 2001 From: burnettk Date: Mon, 29 May 2023 15:04:38 -0400 Subject: [PATCH] only expose value on show, not list, move tests, fix UI --- .../models/secret_model.py | 10 ++ .../routes/secrets_controller.py | 7 +- .../integration/test_secret_service.py | 115 --------------- .../integration/test_secrets_controller.py | 137 ++++++++++++++++++ .../src/components/Notification.tsx | 4 +- .../src/routes/SecretShow.tsx | 71 ++++++--- 6 files changed, 206 insertions(+), 138 deletions(-) create mode 100644 spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_secrets_controller.py diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/models/secret_model.py b/spiffworkflow-backend/src/spiffworkflow_backend/models/secret_model.py index 9ecfe5340..e5b55c4b4 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/models/secret_model.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/models/secret_model.py @@ -1,5 +1,6 @@ """Secret_model.""" from dataclasses import dataclass +from typing import Any from marshmallow import Schema from sqlalchemy import ForeignKey @@ -21,6 +22,15 @@ class SecretModel(SpiffworkflowBaseDBModel): updated_at_in_seconds: int = db.Column(db.Integer) created_at_in_seconds: int = db.Column(db.Integer) + # value is not included in the serialized output because it is sensitive + @property + def serialized(self) -> dict[str, Any]: + return { + "id": self.id, + "key": self.key, + "user_id": self.user_id, + } + class SecretModelSchema(Schema): """SecretModelSchema.""" diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/routes/secrets_controller.py b/spiffworkflow-backend/src/spiffworkflow_backend/routes/secrets_controller.py index 15c8a028c..ada37542a 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/secrets_controller.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/secrets_controller.py @@ -16,7 +16,12 @@ from spiffworkflow_backend.services.user_service import UserService def secret_show(key: str) -> Response: """Secret_show.""" secret = SecretService.get_secret(key) - return make_response(jsonify(secret), 200) + + # normal serialization does not include the secret value, but this is the one endpoint where we want to return the goods + secret_as_dict = secret.serialized + secret_as_dict["value"] = SecretService._decrypt(secret.value) + + return make_response(secret_as_dict, 200) def secret_list( diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_secret_service.py b/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_secret_service.py index 14b2f0465..b4216c10c 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_secret_service.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_secret_service.py @@ -1,5 +1,4 @@ """Test_secret_service.""" -import json import pytest from flask.app import Flask @@ -7,11 +6,9 @@ from flask.testing import FlaskClient from spiffworkflow_backend.exceptions.api_error import ApiError from spiffworkflow_backend.models.process_model import ProcessModelInfo from spiffworkflow_backend.models.secret_model import SecretModel -from spiffworkflow_backend.models.secret_model import SecretModelSchema from spiffworkflow_backend.models.user import UserModel from spiffworkflow_backend.services.process_model_service import ProcessModelService from spiffworkflow_backend.services.secret_service import SecretService -from werkzeug.test import TestResponse # type: ignore from tests.spiffworkflow_backend.helpers.base_test import BaseTest @@ -164,115 +161,3 @@ class TestSecretService(SecretServiceTestHelpers): with pytest.raises(ApiError) as ae: SecretService.delete_secret(self.test_key + "x", with_super_admin_user.id) assert "Resource does not exist" in ae.value.message - - -class TestSecretServiceApi(SecretServiceTestHelpers): - """TestSecretServiceApi.""" - - def test_add_secret( - self, - app: Flask, - client: FlaskClient, - with_db_and_bpmn_file_cleanup: None, - with_super_admin_user: UserModel, - ) -> None: - """Test_add_secret.""" - secret_model = SecretModel( - key=self.test_key, - value=self.test_value, - user_id=with_super_admin_user.id, - ) - data = json.dumps(SecretModelSchema().dump(secret_model)) - response: TestResponse = client.post( - "/v1.0/secrets", - headers=self.logged_in_headers(with_super_admin_user), - content_type="application/json", - data=data, - ) - assert response.json - secret: dict = response.json - for key in ["key", "value", "user_id"]: - assert key in secret.keys() - assert secret["key"] == self.test_key - assert SecretService._decrypt(secret["value"]) == self.test_value - assert secret["user_id"] == with_super_admin_user.id - - def test_get_secret( - self, - app: Flask, - client: FlaskClient, - with_db_and_bpmn_file_cleanup: None, - with_super_admin_user: UserModel, - ) -> None: - """Test get secret.""" - self.add_test_secret(with_super_admin_user) - secret_response = client.get( - f"/v1.0/secrets/{self.test_key}", - headers=self.logged_in_headers(with_super_admin_user), - ) - assert secret_response - assert secret_response.status_code == 200 - assert secret_response.json - assert SecretService._decrypt(secret_response.json["value"]) == self.test_value - - def test_update_secret( - self, - app: Flask, - client: FlaskClient, - with_db_and_bpmn_file_cleanup: None, - with_super_admin_user: UserModel, - ) -> None: - """Test_update_secret.""" - self.add_test_secret(with_super_admin_user) - secret: SecretModel | None = SecretService.get_secret(self.test_key) - assert secret - assert SecretService._decrypt(secret.value) == self.test_value - secret_model = SecretModel( - key=self.test_key, - value="new_secret_value", - user_id=with_super_admin_user.id, - ) - response = client.put( - f"/v1.0/secrets/{self.test_key}", - headers=self.logged_in_headers(with_super_admin_user), - content_type="application/json", - data=json.dumps(SecretModelSchema().dump(secret_model)), - ) - assert response.status_code == 200 - - secret_model = SecretModel.query.filter(SecretModel.key == self.test_key).first() - assert SecretService._decrypt(secret_model.value) == "new_secret_value" - - def test_delete_secret( - self, - app: Flask, - client: FlaskClient, - with_db_and_bpmn_file_cleanup: None, - with_super_admin_user: UserModel, - ) -> None: - """Test delete secret.""" - self.add_test_secret(with_super_admin_user) - secret = SecretService.get_secret(self.test_key) - assert secret - assert SecretService._decrypt(secret.value) == self.test_value - secret_response = client.delete( - f"/v1.0/secrets/{self.test_key}", - headers=self.logged_in_headers(with_super_admin_user), - ) - assert secret_response.status_code == 200 - with pytest.raises(ApiError): - secret = SecretService.get_secret(self.test_key) - - def test_delete_secret_bad_key( - self, - app: Flask, - client: FlaskClient, - with_db_and_bpmn_file_cleanup: None, - with_super_admin_user: UserModel, - ) -> None: - """Test delete secret.""" - secret_response = client.delete( - "/v1.0/secrets/bad_secret_key", - headers=self.logged_in_headers(with_super_admin_user), - ) - assert secret_response.status_code == 404 diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_secrets_controller.py b/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_secrets_controller.py new file mode 100644 index 000000000..2cafaabbe --- /dev/null +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_secrets_controller.py @@ -0,0 +1,137 @@ +from flask.app import Flask +import json +from spiffworkflow_backend.models.secret_model import SecretModel, SecretModelSchema +from spiffworkflow_backend.exceptions.api_error import ApiError +import pytest +from spiffworkflow_backend.models.user import UserModel +from flask.testing import FlaskClient + +from tests.spiffworkflow_backend.integration.test_secret_service import SecretServiceTestHelpers +from spiffworkflow_backend.services.secret_service import SecretService + + +class TestSecretsController(SecretServiceTestHelpers): + def test_add_secret( + self, + app: Flask, + client: FlaskClient, + with_db_and_bpmn_file_cleanup: None, + with_super_admin_user: UserModel, + ) -> None: + """Test_add_secret.""" + secret_model = SecretModel( + key=self.test_key, + value=self.test_value, + user_id=with_super_admin_user.id, + ) + data = json.dumps(SecretModelSchema().dump(secret_model)) + response = client.post( + "/v1.0/secrets", + headers=self.logged_in_headers(with_super_admin_user), + content_type="application/json", + data=data, + ) + assert response.json + secret: dict = response.json + for key in ["key", "value", "user_id"]: + assert key in secret.keys() + assert secret["key"] == self.test_key + assert SecretService._decrypt(secret["value"]) == self.test_value + assert secret["user_id"] == with_super_admin_user.id + + def test_get_secret( + self, + app: Flask, + client: FlaskClient, + with_db_and_bpmn_file_cleanup: None, + with_super_admin_user: UserModel, + ) -> None: + """Test get secret.""" + self.add_test_secret(with_super_admin_user) + secret_response = client.get( + f"/v1.0/secrets/{self.test_key}", + headers=self.logged_in_headers(with_super_admin_user), + ) + assert secret_response + assert secret_response.status_code == 200 + assert secret_response.json + assert SecretService._decrypt(secret_response.json["value"]) == self.test_value + + def test_update_secret( + self, + app: Flask, + client: FlaskClient, + with_db_and_bpmn_file_cleanup: None, + with_super_admin_user: UserModel, + ) -> None: + """Test_update_secret.""" + self.add_test_secret(with_super_admin_user) + secret: SecretModel | None = SecretService.get_secret(self.test_key) + assert secret + assert SecretService._decrypt(secret.value) == self.test_value + secret_model = SecretModel( + key=self.test_key, + value="new_secret_value", + user_id=with_super_admin_user.id, + ) + response = client.put( + f"/v1.0/secrets/{self.test_key}", + headers=self.logged_in_headers(with_super_admin_user), + content_type="application/json", + data=json.dumps(SecretModelSchema().dump(secret_model)), + ) + assert response.status_code == 200 + + secret_model = SecretModel.query.filter(SecretModel.key == self.test_key).first() + assert SecretService._decrypt(secret_model.value) == "new_secret_value" + + def test_delete_secret( + self, + app: Flask, + client: FlaskClient, + with_db_and_bpmn_file_cleanup: None, + with_super_admin_user: UserModel, + ) -> None: + """Test delete secret.""" + self.add_test_secret(with_super_admin_user) + secret = SecretService.get_secret(self.test_key) + assert secret + assert SecretService._decrypt(secret.value) == self.test_value + secret_response = client.delete( + f"/v1.0/secrets/{self.test_key}", + headers=self.logged_in_headers(with_super_admin_user), + ) + assert secret_response.status_code == 200 + with pytest.raises(ApiError): + secret = SecretService.get_secret(self.test_key) + + def test_delete_secret_bad_key( + self, + app: Flask, + client: FlaskClient, + with_db_and_bpmn_file_cleanup: None, + with_super_admin_user: UserModel, + ) -> None: + """Test delete secret.""" + secret_response = client.delete( + "/v1.0/secrets/bad_secret_key", + headers=self.logged_in_headers(with_super_admin_user), + ) + assert secret_response.status_code == 404 + + def test_secret_list( + self, + app: Flask, + client: FlaskClient, + with_db_and_bpmn_file_cleanup: None, + with_super_admin_user: UserModel, + ) -> None: + self.add_test_secret(with_super_admin_user) + secret_response = client.get( + "/v1.0/secrets", + headers=self.logged_in_headers(with_super_admin_user), + ) + assert secret_response.status_code == 200 + first_secret_in_results = secret_response.json["results"][0] + assert first_secret_in_results['key'] == self.test_key + assert 'value' not in first_secret_in_results diff --git a/spiffworkflow-frontend/src/components/Notification.tsx b/spiffworkflow-frontend/src/components/Notification.tsx index 38191de7b..2a853ce2b 100644 --- a/spiffworkflow-frontend/src/components/Notification.tsx +++ b/spiffworkflow-frontend/src/components/Notification.tsx @@ -10,8 +10,8 @@ import { Button } from '@carbon/react'; type OwnProps = { title: string; - children: React.ReactNode; - onClose: (..._args: any[]) => any; + children?: React.ReactNode; + onClose: Function; type?: string; }; diff --git a/spiffworkflow-frontend/src/routes/SecretShow.tsx b/spiffworkflow-frontend/src/routes/SecretShow.tsx index 4a434d8e4..b01ac2f7e 100644 --- a/spiffworkflow-frontend/src/routes/SecretShow.tsx +++ b/spiffworkflow-frontend/src/routes/SecretShow.tsx @@ -1,9 +1,10 @@ import { useEffect, useState } from 'react'; import { useParams, useNavigate } from 'react-router-dom'; // @ts-ignore -import { Stack, Table, Button } from '@carbon/react'; +import { Stack, Table, Button, TextInput } from '@carbon/react'; import HttpService from '../services/HttpService'; import { Secret } from '../interfaces'; +import { Notification } from '../components/Notification'; import ButtonWithConfirmation from '../components/ButtonWithConfirmation'; export default function SecretShow() { @@ -11,7 +12,9 @@ export default function SecretShow() { const params = useParams(); const [secret, setSecret] = useState(null); - const [secretValue, setSecretValue] = useState(secret?.value); + const [displaySecretValue, setDisplaySecretValue] = useState(false); + const [showSuccessNotification, setShowSuccessNotification] = + useState(false); useEffect(() => { HttpService.makeCallToBackend({ @@ -22,22 +25,21 @@ export default function SecretShow() { const handleSecretValueChange = (event: any) => { if (secret) { - setSecretValue(event.target.value); + const newSecret = { ...secret, value: event.target.value }; + setSecret(newSecret); } }; const updateSecretValue = () => { - if (secret && secretValue) { - secret.value = secretValue; + if (secret) { HttpService.makeCallToBackend({ path: `/secrets/${secret.key}`, successCallback: () => { - setSecret(secret); + setShowSuccessNotification(true); }, httpMethod: 'PUT', postBody: { - value: secretValue, - creator_user_id: secret.creator_user_id, + value: secret.value, }, }); } @@ -58,9 +60,17 @@ export default function SecretShow() { }); }; + const successNotificationComponent = ( + setShowSuccessNotification(false)} + /> + ); + if (secret) { return ( <> + {showSuccessNotification && successNotificationComponent}

Secret Key: {secret.key}

-
@@ -77,21 +93,36 @@ export default function SecretShow() { Key - Value + {displaySecretValue && ( + <> + Value + Actions + + )} {params.key} - - - + {displaySecretValue && ( + <> + + + + + {displaySecretValue && ( + + )} + + + )}