From 93dbce681ec89bc45479748aaae06ddd92b64da4 Mon Sep 17 00:00:00 2001 From: Jon Herron Date: Thu, 13 Oct 2022 20:45:04 -0400 Subject: [PATCH] Squashed 'spiffworkflow-backend/' changes from eb89e9529..59e6ee2c8 59e6ee2c8 Merge pull request #136 from sartography/dependabot/github_actions/actions/cache-3.0.11 88c61c83b Bump actions/cache from 3.0.10 to 3.0.11 344f8045b Merge branch 'feature/secrets' 08f112aa5 remove unused import 326c88cd6 lint 516ee5fcd Remove allowed process stuff from secrets 85808cb6b Merge branch 'main' into feature/secrets 6da486fd9 Merge branch 'main' into feature/secrets 8fb5c8612 add environ 1500e234a update staging configs a1f50f09c lint f0111adb8 Merge remote-tracking branch 'origin/main' into feature/secrets 453c4b1e2 add get and update for allowed process paths 35dd02d1c Merge branch 'main' into feature/secrets f3166cc8a Merge branch 'main' into feature/secrets c2d67c008 pre commit 11a24368b mypy ff56ed068 Use secret key when adding allowed process model 1b25d08a3 poetry update git-subtree-dir: spiffworkflow-backend git-subtree-split: 59e6ee2c8baeee516c04a814365b59698dbc61d1 --- .github/workflows/tests.yml | 2 +- .../{5f7d61fa371c_.py => 07ff3fbef405_.py} | 15 +- src/spiffworkflow_backend/api.yml | 56 ----- src/spiffworkflow_backend/config/staging.py | 5 + .../load_database_models.py | 3 - .../models/secret_model.py | 37 +-- .../routes/process_api_blueprint.py | 19 -- .../services/secret_service.py | 84 ------- .../integration/test_secret_service.py | 211 ------------------ wsgi.py | 1 - 10 files changed, 10 insertions(+), 423 deletions(-) rename migrations/versions/{5f7d61fa371c_.py => 07ff3fbef405_.py} (97%) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 47c5f4a2..96b7db85 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -122,7 +122,7 @@ jobs: print("::set-output name=result::{}".format(result)) - name: Restore pre-commit cache - uses: actions/cache@v3.0.10 + uses: actions/cache@v3.0.11 if: matrix.session == 'pre-commit' with: path: ~/.cache/pre-commit diff --git a/migrations/versions/5f7d61fa371c_.py b/migrations/versions/07ff3fbef405_.py similarity index 97% rename from migrations/versions/5f7d61fa371c_.py rename to migrations/versions/07ff3fbef405_.py index 8098392d..6adc88cb 100644 --- a/migrations/versions/5f7d61fa371c_.py +++ b/migrations/versions/07ff3fbef405_.py @@ -1,8 +1,8 @@ """empty message -Revision ID: 5f7d61fa371c +Revision ID: 07ff3fbef405 Revises: -Create Date: 2022-10-11 14:45:41.213890 +Create Date: 2022-10-13 07:56:01.234090 """ from alembic import op @@ -10,7 +10,7 @@ import sqlalchemy as sa # revision identifiers, used by Alembic. -revision = '5f7d61fa371c' +revision = '07ff3fbef405' down_revision = None branch_labels = None depends_on = None @@ -233,14 +233,6 @@ def upgrade(): sa.PrimaryKeyConstraint('id'), sa.UniqueConstraint('principal_id', 'permission_target_id', 'permission', name='permission_assignment_uniq') ) - op.create_table('secret_allowed_process', - sa.Column('id', sa.Integer(), nullable=False), - sa.Column('secret_id', sa.Integer(), nullable=False), - sa.Column('allowed_relative_path', sa.String(length=500), nullable=False), - sa.ForeignKeyConstraint(['secret_id'], ['secret.id'], ), - sa.PrimaryKeyConstraint('id'), - sa.UniqueConstraint('secret_id', 'allowed_relative_path', name='unique_secret_path') - ) op.create_table('spiff_logging', sa.Column('id', sa.Integer(), nullable=False), sa.Column('process_instance_id', sa.Integer(), nullable=False), @@ -313,7 +305,6 @@ def downgrade(): op.drop_table('data_store') op.drop_table('task_event') op.drop_table('spiff_logging') - op.drop_table('secret_allowed_process') op.drop_table('permission_assignment') op.drop_table('message_instance') op.drop_index(op.f('ix_message_correlation_value'), table_name='message_correlation') diff --git a/src/spiffworkflow_backend/api.yml b/src/spiffworkflow_backend/api.yml index 1ea29d20..84cd2cc4 100755 --- a/src/spiffworkflow_backend/api.yml +++ b/src/spiffworkflow_backend/api.yml @@ -1195,41 +1195,6 @@ paths: "404": description: Secret does not exist - /secrets/allowed_process_paths: - post: - operationId: spiffworkflow_backend.routes.process_api_blueprint.add_allowed_process_path - summary: Create an allowed process to a secret - tags: - - Secrets - requestBody: - content: - application/json: - schema: - $ref: "#/components/schemas/SecretAllowedProcessPath" - responses: - "201": - description: Allowed process created successfully - content: - application/json: - schema: - $ref: "#/components/schemas/SecretAllowedProcessPath" - /secrets/allowed_process_paths/{allowed_process_path_id}: - parameters: - - name: allowed_process_path_id - in: path - required: true - description: The id of the allowed process path to delete - schema: - type: integer - delete: - operationId: spiffworkflow_backend.routes.process_api_blueprint.delete_allowed_process_path - summary: Delete an existing allowed process for a secret - tags: - - Secrets - responses: - "204": - description: The allowed process is deleted. - components: securitySchemes: jwt: @@ -1995,12 +1960,6 @@ components: type: number example: 1 nullable: false - allowed_processes: - description: The processes allowed to access this secret - type: array - items: - $ref: "#/components/schemas/SecretAllowedProcessPath" - nullable: true ProcessInstanceLog: properties: id: @@ -2032,18 +1991,3 @@ components: description: The timestamp returned in the log type: number example: 123456789.12345 - SecretAllowedProcessPath: - properties: - id: - description: The id of the allowed process path - type: number - example: 1 - nullable: true - secret_id: - description: The id of the secret associated with this allowed process path - type: number - example: 2 - allowed_relative_path: - description: The allowed process path - type: string - example: /group_one/group_two/model_a diff --git a/src/spiffworkflow_backend/config/staging.py b/src/spiffworkflow_backend/config/staging.py index 55bb285b..53c8af61 100644 --- a/src/spiffworkflow_backend/config/staging.py +++ b/src/spiffworkflow_backend/config/staging.py @@ -1,4 +1,9 @@ """Staging.""" +from os import environ + GIT_COMMIT_ON_SAVE = True GIT_COMMIT_USERNAME = "staging" GIT_COMMIT_EMAIL = "staging@example.com" +SPIFFWORKFLOW_BACKEND_PERMISSIONS_FILE_NAME = environ.get( + "SPIFFWORKFLOW_BACKEND_PERMISSIONS_FILE_NAME", default="staging.yml" +) diff --git a/src/spiffworkflow_backend/load_database_models.py b/src/spiffworkflow_backend/load_database_models.py index 697e6772..33d32c1a 100644 --- a/src/spiffworkflow_backend/load_database_models.py +++ b/src/spiffworkflow_backend/load_database_models.py @@ -45,9 +45,6 @@ from spiffworkflow_backend.models.process_instance import ( from spiffworkflow_backend.models.process_instance_report import ( ProcessInstanceReportModel, ) # noqa: F401 -from spiffworkflow_backend.models.secret_model import ( - SecretAllowedProcessPathModel, -) # noqa: F401 from spiffworkflow_backend.models.secret_model import SecretModel # noqa: F401 from spiffworkflow_backend.models.spiff_logging import SpiffLoggingModel # noqa: F401 from spiffworkflow_backend.models.task_event import TaskEventModel # noqa: F401 diff --git a/src/spiffworkflow_backend/models/secret_model.py b/src/spiffworkflow_backend/models/secret_model.py index 9eab36b4..444a531d 100644 --- a/src/spiffworkflow_backend/models/secret_model.py +++ b/src/spiffworkflow_backend/models/secret_model.py @@ -5,8 +5,6 @@ from flask_bpmn.models.db import db from flask_bpmn.models.db import SpiffworkflowBaseDBModel from marshmallow import Schema from sqlalchemy import ForeignKey -from sqlalchemy.orm import relationship -from sqlalchemy.orm import RelationshipProperty from spiffworkflow_backend.models.user import UserModel @@ -21,29 +19,6 @@ class SecretModel(SpiffworkflowBaseDBModel): value: str = db.Column(db.String(255), nullable=False) creator_user_id: int = db.Column(ForeignKey(UserModel.id), nullable=False) - allowed_processes: RelationshipProperty = relationship( - "SecretAllowedProcessPathModel", cascade="delete" - ) - - -@dataclass() -class SecretAllowedProcessPathModel(SpiffworkflowBaseDBModel): - """Allowed processes can be Process Groups or Process Models. - - We store the path in either case. - """ - - __tablename__ = "secret_allowed_process" - __table_args__ = ( - db.UniqueConstraint( - "secret_id", "allowed_relative_path", name="unique_secret_path" - ), - ) - - id: int = db.Column(db.Integer, primary_key=True) - secret_id: int = db.Column(ForeignKey(SecretModel.id), nullable=False) # type: ignore - allowed_relative_path: str = db.Column(db.String(500), nullable=False) - class SecretModelSchema(Schema): """SecretModelSchema.""" @@ -52,14 +27,4 @@ class SecretModelSchema(Schema): """Meta.""" model = SecretModel - fields = ["key", "value", "creator_user_id", "allowed_processes"] - - -class SecretAllowedProcessSchema(Schema): - """SecretAllowedProcessSchema.""" - - class Meta: - """Meta.""" - - model = SecretAllowedProcessPathModel - fields = ["secret_id", "allowed_relative_path"] + fields = ["key", "value", "creator_user_id"] diff --git a/src/spiffworkflow_backend/routes/process_api_blueprint.py b/src/spiffworkflow_backend/routes/process_api_blueprint.py index 7a164cff..7764ef69 100644 --- a/src/spiffworkflow_backend/routes/process_api_blueprint.py +++ b/src/spiffworkflow_backend/routes/process_api_blueprint.py @@ -48,7 +48,6 @@ from spiffworkflow_backend.models.process_instance_report import ( ) from spiffworkflow_backend.models.process_model import ProcessModelInfo from spiffworkflow_backend.models.process_model import ProcessModelInfoSchema -from spiffworkflow_backend.models.secret_model import SecretAllowedProcessSchema from spiffworkflow_backend.models.secret_model import SecretModel from spiffworkflow_backend.models.secret_model import SecretModelSchema from spiffworkflow_backend.models.spiff_logging import SpiffLoggingModel @@ -1340,24 +1339,6 @@ def delete_secret(key: str) -> Response: return Response(json.dumps({"ok": True}), status=200, mimetype="application/json") -def add_allowed_process_path(body: dict) -> Response: - """Get allowed process paths.""" - allowed_process_path = SecretService.add_allowed_process( - body["secret_id"], g.user.id, body["allowed_relative_path"] - ) - return Response( - json.dumps(SecretAllowedProcessSchema().dump(allowed_process_path)), - status=201, - mimetype="application/json", - ) - - -def delete_allowed_process_path(allowed_process_path_id: int) -> Response: - """Get allowed process paths.""" - SecretService().delete_allowed_process(allowed_process_path_id, g.user.id) - return Response(json.dumps({"ok": True}), status=200, mimetype="application/json") - - def _get_required_parameter_or_raise(parameter: str, post_body: dict[str, Any]) -> Any: """Get_required_parameter_or_raise.""" return_value = None diff --git a/src/spiffworkflow_backend/services/secret_service.py b/src/spiffworkflow_backend/services/secret_service.py index 214bf2a3..f6b35e0a 100644 --- a/src/spiffworkflow_backend/services/secret_service.py +++ b/src/spiffworkflow_backend/services/secret_service.py @@ -3,9 +3,7 @@ from typing import Optional from flask_bpmn.api.api_error import ApiError from flask_bpmn.models.db import db -from sqlalchemy.exc import IntegrityError -from spiffworkflow_backend.models.secret_model import SecretAllowedProcessPathModel from spiffworkflow_backend.models.secret_model import SecretModel # from cryptography.fernet import Fernet @@ -125,85 +123,3 @@ class SecretService: message=f"Cannot delete secret with key: {key}. Resource does not exist.", status_code=404, ) - - @staticmethod - def add_allowed_process( - secret_id: int, user_id: str, allowed_relative_path: str - ) -> SecretAllowedProcessPathModel: - """Add_allowed_process.""" - secret_model = SecretModel.query.filter(SecretModel.id == secret_id).first() - if secret_model: - if secret_model.creator_user_id == user_id: - secret_process_model = SecretAllowedProcessPathModel( - secret_id=secret_model.id, - allowed_relative_path=allowed_relative_path, - ) - assert secret_process_model # noqa: S101 - db.session.add(secret_process_model) - try: - db.session.commit() - except IntegrityError as ie: - db.session.rollback() - raise ApiError( - error_code="add_allowed_process_error", - message=f"Error adding allowed_process with secret {secret_model.id}, " - f"and path: {allowed_relative_path}. Resource already exists. " - f"Original error is {ie}", - status_code=409, - ) from ie - except Exception as e: - # TODO: should we call db.session.rollback() here? - # db.session.rollback() - raise ApiError( - error_code="add_allowed_process_error", - message=f"Could not create an allowed process for secret with key: {secret_model.key} " - f"with path: {allowed_relative_path}. " - f"Original error is {e}", - ) from e - return secret_process_model - else: - raise ApiError( - error_code="add_allowed_process_error", - message=f"User: {user_id} cannot modify the secret with key : {secret_model.key}", - status_code=401, - ) - else: - raise ApiError( - error_code="add_allowed_process_error", - message=f"Cannot add allowed process to secret with key: {secret_id}. Resource does not exist.", - status_code=404, - ) - - @staticmethod - def delete_allowed_process(allowed_process_id: int, user_id: int) -> None: - """Delete_allowed_process.""" - allowed_process = SecretAllowedProcessPathModel.query.filter( - SecretAllowedProcessPathModel.id == allowed_process_id - ).first() - if allowed_process: - secret = SecretModel.query.filter( - SecretModel.id == allowed_process.secret_id - ).first() - assert secret # noqa: S101 - if secret.creator_user_id == user_id: - db.session.delete(allowed_process) - try: - db.session.commit() - except Exception as e: - raise ApiError( - error_code="delete_allowed_process_error", - message=f"There was an exception deleting allowed_process: {allowed_process_id}. " - f"Original error is: {e}", - ) from e - else: - raise ApiError( - error_code="delete_allowed_process_error", - message=f"User: {user_id} cannot delete the allowed_process with id : {allowed_process_id}", - status_code=401, - ) - else: - raise ApiError( - error_code="delete_allowed_process_error", - message=f"Cannot delete allowed_process: {allowed_process_id}. Resource does not exist.", - status_code=404, - ) diff --git a/tests/spiffworkflow_backend/integration/test_secret_service.py b/tests/spiffworkflow_backend/integration/test_secret_service.py index 94637fac..dfe0f3cb 100644 --- a/tests/spiffworkflow_backend/integration/test_secret_service.py +++ b/tests/spiffworkflow_backend/integration/test_secret_service.py @@ -10,11 +10,9 @@ from tests.spiffworkflow_backend.helpers.base_test import BaseTest from werkzeug.test import TestResponse from spiffworkflow_backend.models.process_model import ProcessModelInfo -from spiffworkflow_backend.models.secret_model import SecretAllowedProcessPathModel 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.file_system_service import FileSystemService from spiffworkflow_backend.services.process_model_service import ProcessModelService from spiffworkflow_backend.services.secret_service import SecretService @@ -56,23 +54,6 @@ class SecretServiceTestHelpers(BaseTest): ) return process_model_info - def add_test_secret_allowed_process( - self, client: FlaskClient, user: UserModel - ) -> SecretAllowedProcessPathModel: - """Add_test_secret_allowed_process.""" - process_model_info = self.add_test_process(client, user) - process_model_relative_path = FileSystemService.process_model_relative_path( - process_model_info - ) - - test_secret = self.add_test_secret(user) - allowed_process_model = SecretService().add_allowed_process( - secret_id=test_secret.id, - user_id=user.id, - allowed_relative_path=process_model_relative_path, - ) - return allowed_process_model - class TestSecretService(SecretServiceTestHelpers): """TestSecretService.""" @@ -191,147 +172,6 @@ class TestSecretService(SecretServiceTestHelpers): SecretService.delete_secret(self.test_key + "x", user.id) assert "Resource does not exist" in ae.value.message - def test_secret_add_allowed_process( - self, app: Flask, client: FlaskClient, with_db_and_bpmn_file_cleanup: None - ) -> None: - """Test_secret_add_allowed_process.""" - user = self.find_or_create_user() - test_secret = self.add_test_secret(user) - process_model_info = self.add_test_process(client, user) - - process_model_relative_path = FileSystemService.process_model_relative_path( - process_model_info - ) - allowed_process_model = SecretService().add_allowed_process( - secret_id=test_secret.id, - user_id=user.id, - allowed_relative_path=process_model_relative_path, - ) - - assert allowed_process_model is not None - assert isinstance(allowed_process_model, SecretAllowedProcessPathModel) - assert allowed_process_model.secret_id == test_secret.id - assert ( - allowed_process_model.allowed_relative_path == process_model_relative_path - ) - - assert len(test_secret.allowed_processes) == 1 - assert test_secret.allowed_processes[0] == allowed_process_model - - def test_secret_add_allowed_process_same_process_fails( - self, app: Flask, client: FlaskClient, with_db_and_bpmn_file_cleanup: None - ) -> None: - """Do not allow duplicate entries for secret_id/allowed_relative_path pairs. - - We actually take care of this in the db model with a unique constraint - on the 2 columns. - """ - user = self.find_or_create_user() - test_secret = self.add_test_secret(user) - process_model_info = self.add_test_process(client, user) - - process_model_relative_path = FileSystemService.process_model_relative_path( - process_model_info - ) - SecretService().add_allowed_process( - secret_id=test_secret.id, - user_id=user.id, - allowed_relative_path=process_model_relative_path, - ) - allowed_processes = SecretAllowedProcessPathModel.query.all() - assert len(allowed_processes) == 1 - - with pytest.raises(ApiError) as ae: - SecretService().add_allowed_process( - secret_id=test_secret.id, - user_id=user.id, - allowed_relative_path=process_model_relative_path, - ) - assert "Resource already exists" in ae.value.message - - def test_secret_add_allowed_process_bad_user_fails( - self, app: Flask, client: FlaskClient, with_db_and_bpmn_file_cleanup: None - ) -> None: - """Test_secret_add_allowed_process_bad_user.""" - user = self.find_or_create_user() - process_model_info = self.add_test_process(client, user) - process_model_relative_path = FileSystemService.process_model_relative_path( - process_model_info - ) - test_secret = self.add_test_secret(user) - with pytest.raises(ApiError) as ae: - SecretService().add_allowed_process( - secret_id=test_secret.id, - user_id=user.id + 1, - allowed_relative_path=process_model_relative_path, - ) - assert ( - ae.value.message - == f"User: {user.id+1} cannot modify the secret with key : {self.test_key}" - ) - - def test_secret_add_allowed_process_bad_secret_fails( - self, app: Flask, client: FlaskClient, with_db_and_bpmn_file_cleanup: None - ) -> None: - """Test_secret_add_allowed_process_bad_secret_fails.""" - user = self.find_or_create_user() - process_model_info = self.add_test_process(client, user) - process_model_relative_path = FileSystemService.process_model_relative_path( - process_model_info - ) - test_secret = self.add_test_secret(user) - - with pytest.raises(ApiError) as ae: - SecretService().add_allowed_process( - secret_id=test_secret.id + 1, - user_id=user.id, - allowed_relative_path=process_model_relative_path, - ) - assert "Resource does not exist" in ae.value.message - - def test_secret_delete_allowed_process( - self, app: Flask, client: FlaskClient, with_db_and_bpmn_file_cleanup: None - ) -> None: - """Test_secret_delete_allowed_process.""" - user = self.find_or_create_user() - allowed_process_model = self.add_test_secret_allowed_process(client, user) - - allowed_processes = SecretAllowedProcessPathModel.query.all() - assert len(allowed_processes) == 1 - - SecretService().delete_allowed_process(allowed_process_model.id, user.id) - - allowed_processes = SecretAllowedProcessPathModel.query.all() - assert len(allowed_processes) == 0 - - def test_secret_delete_allowed_process_bad_user_fails( - self, app: Flask, client: FlaskClient, with_db_and_bpmn_file_cleanup: None - ) -> None: - """Test_secret_delete_allowed_process_bad_user_fails.""" - user = self.find_or_create_user() - allowed_process_model = self.add_test_secret_allowed_process(client, user) - with pytest.raises(ApiError) as ae: - SecretService().delete_allowed_process( - allowed_process_model.id, user.id + 1 - ) - message = ae.value.message - assert ( - f"User: {user.id+1} cannot delete the allowed_process with id : {allowed_process_model.id}" - in message - ) - - def test_secret_delete_allowed_process_bad_allowed_process_fails( - self, app: Flask, client: FlaskClient, with_db_and_bpmn_file_cleanup: None - ) -> None: - """Test_secret_delete_allowed_process_bad_allowed_process_fails.""" - user = self.find_or_create_user() - allowed_process_model = self.add_test_secret_allowed_process(client, user) - with pytest.raises(ApiError) as ae: - SecretService().delete_allowed_process( - allowed_process_model.id + 1, user.id - ) - assert "Resource does not exist" in ae.value.message - class TestSecretServiceApi(SecretServiceTestHelpers): """TestSecretServiceApi.""" @@ -441,54 +281,3 @@ class TestSecretServiceApi(SecretServiceTestHelpers): headers=self.logged_in_headers(user), ) assert secret_response.status_code == 404 - - def test_add_secret_allowed_process( - self, app: Flask, client: FlaskClient, with_db_and_bpmn_file_cleanup: None - ) -> None: - """Test add secret allowed process.""" - user = self.find_or_create_user() - test_secret = self.add_test_secret(user) - process_model_info = self.add_test_process(client, user) - process_model_relative_path = FileSystemService.process_model_relative_path( - process_model_info - ) - data = { - "secret_id": test_secret.id, - "allowed_relative_path": process_model_relative_path, - } - response: TestResponse = client.post( - "/v1.0/secrets/allowed_process_paths", - headers=self.logged_in_headers(user), - content_type="application/json", - data=json.dumps(data), - ) - assert response.status_code == 201 - allowed_processes = SecretAllowedProcessPathModel.query.all() - assert len(allowed_processes) == 1 - assert allowed_processes[0].allowed_relative_path == process_model_relative_path - assert allowed_processes[0].secret_id == test_secret.id - - def test_delete_secret_allowed_process( - self, app: Flask, client: FlaskClient, with_db_and_bpmn_file_cleanup: None - ) -> None: - """Test delete secret allowed process.""" - user = self.find_or_create_user() - test_secret = self.add_test_secret(user) - process_model_info = self.add_test_process(client, user) - process_model_relative_path = FileSystemService.process_model_relative_path( - process_model_info - ) - allowed_process = SecretService.add_allowed_process( - test_secret.id, user.id, process_model_relative_path - ) - allowed_processes = SecretAllowedProcessPathModel.query.all() - assert len(allowed_processes) == 1 - assert allowed_processes[0].secret_id == test_secret.id - assert allowed_processes[0].allowed_relative_path == process_model_relative_path - response = client.delete( - f"/v1.0/secrets/allowed_process_paths/{allowed_process.id}", - headers=self.logged_in_headers(user), - ) - assert response.status_code == 200 - allowed_processes = SecretAllowedProcessPathModel.query.all() - assert len(allowed_processes) == 0 diff --git a/wsgi.py b/wsgi.py index f8ca5ca7..d3c0ba5e 100644 --- a/wsgi.py +++ b/wsgi.py @@ -5,7 +5,6 @@ from spiffworkflow_backend import create_app from spiffworkflow_backend.services.acceptance_test_fixtures import ( load_acceptance_test_fixtures, ) -from spiffworkflow_backend.services.data_setup_service import DataSetupService app = create_app()