From 043614a00514e268d6f68c71369c6a0d02d96bb5 Mon Sep 17 00:00:00 2001 From: Mike Cullerton Date: Tue, 25 Oct 2022 14:12:32 -0400 Subject: [PATCH] Refresh token (#6) * Handle refreshed tokens if present * Small cleanup * No longer require secrets to be modified by the user that created them Rename creator_user_id column to user_id Co-authored-by: Jon Herron Co-authored-by: mike cullerton --- .../{4ba2ed52a63a_.py => 3bd6b0b1b8ae_.py} | 10 +-- .../src/spiffworkflow_backend/api.yml | 4 +- .../models/secret_model.py | 4 +- .../routes/process_api_blueprint.py | 2 +- .../services/secret_service.py | 48 +++++-------- .../services/service_task_service.py | 16 ++++- .../integration/test_secret_service.py | 72 ++----------------- 7 files changed, 48 insertions(+), 108 deletions(-) rename spiffworkflow-backend/migrations/versions/{4ba2ed52a63a_.py => 3bd6b0b1b8ae_.py} (98%) diff --git a/spiffworkflow-backend/migrations/versions/4ba2ed52a63a_.py b/spiffworkflow-backend/migrations/versions/3bd6b0b1b8ae_.py similarity index 98% rename from spiffworkflow-backend/migrations/versions/4ba2ed52a63a_.py rename to spiffworkflow-backend/migrations/versions/3bd6b0b1b8ae_.py index e0578a3a..80c47958 100644 --- a/spiffworkflow-backend/migrations/versions/4ba2ed52a63a_.py +++ b/spiffworkflow-backend/migrations/versions/3bd6b0b1b8ae_.py @@ -1,8 +1,8 @@ """empty message -Revision ID: 4ba2ed52a63a +Revision ID: 3bd6b0b1b8ae Revises: -Create Date: 2022-10-21 09:31:30.520942 +Create Date: 2022-10-25 12:31:50.177599 """ from alembic import op @@ -10,7 +10,7 @@ import sqlalchemy as sa # revision identifiers, used by Alembic. -revision = '4ba2ed52a63a' +revision = '3bd6b0b1b8ae' down_revision = None branch_labels = None depends_on = None @@ -146,10 +146,10 @@ def upgrade(): sa.Column('id', sa.Integer(), nullable=False), sa.Column('key', sa.String(length=50), nullable=False), sa.Column('value', sa.Text(), nullable=False), - sa.Column('creator_user_id', sa.Integer(), nullable=False), + sa.Column('user_id', sa.Integer(), nullable=False), sa.Column('updated_at_in_seconds', sa.Integer(), nullable=True), sa.Column('created_at_in_seconds', sa.Integer(), nullable=True), - sa.ForeignKeyConstraint(['creator_user_id'], ['user.id'], ), + sa.ForeignKeyConstraint(['user_id'], ['user.id'], ), sa.PrimaryKeyConstraint('id'), sa.UniqueConstraint('key') ) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/api.yml b/spiffworkflow-backend/src/spiffworkflow_backend/api.yml index 3d62f468..33718d8d 100755 --- a/spiffworkflow-backend/src/spiffworkflow_backend/api.yml +++ b/spiffworkflow-backend/src/spiffworkflow_backend/api.yml @@ -2090,8 +2090,8 @@ components: type: string example: my_super_secret_value nullable: false - creator_user_id: - description: The id of the logged in user that created this secret + user_id: + description: The id of the logged in user that updated this secret type: number example: 1 nullable: false diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/models/secret_model.py b/spiffworkflow-backend/src/spiffworkflow_backend/models/secret_model.py index fed25b1a..92fd470a 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/models/secret_model.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/models/secret_model.py @@ -17,7 +17,7 @@ class SecretModel(SpiffworkflowBaseDBModel): id: int = db.Column(db.Integer, primary_key=True) key: str = db.Column(db.String(50), unique=True, nullable=False) value: str = db.Column(db.Text(), nullable=False) - creator_user_id: int = db.Column(ForeignKey(UserModel.id), nullable=False) + user_id: int = db.Column(ForeignKey(UserModel.id), nullable=False) updated_at_in_seconds: int = db.Column(db.Integer) created_at_in_seconds: int = db.Column(db.Integer) @@ -29,4 +29,4 @@ class SecretModelSchema(Schema): """Meta.""" model = SecretModel - fields = ["key", "value", "creator_user_id"] + fields = ["key", "value", "user_id"] diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_api_blueprint.py b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_api_blueprint.py index af73c3cd..a43421b3 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_api_blueprint.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_api_blueprint.py @@ -1408,7 +1408,7 @@ def add_secret(body: Dict) -> Response: def update_secret(key: str, body: dict) -> Response: """Update secret.""" - SecretService().update_secret(key, body["value"], body["creator_user_id"]) + SecretService().update_secret(key, body["value"], body["user_id"]) return Response(json.dumps({"ok": True}), status=200, mimetype="application/json") diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/secret_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/secret_service.py index b5557a49..c33ebb91 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/secret_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/secret_service.py @@ -33,12 +33,12 @@ class SecretService: def add_secret( key: str, value: str, - creator_user_id: int, + user_id: int, ) -> SecretModel: """Add_secret.""" # encrypted_key = self.encrypt_key(key) secret_model = SecretModel( - key=key, value=value, creator_user_id=creator_user_id + key=key, value=value, user_id=user_id ) db.session.add(secret_model) try: @@ -67,29 +67,22 @@ class SecretService: def update_secret( key: str, value: str, - creator_user_id: int, + user_id: int, create_if_not_exists: Optional[bool] = False, ) -> None: """Does this pass pre commit?""" secret_model = SecretModel.query.filter(SecretModel.key == key).first() if secret_model: - if secret_model.creator_user_id == creator_user_id: - secret_model.value = value - db.session.add(secret_model) - try: - db.session.commit() - except Exception as e: - db.session.rollback() - raise e - else: - raise ApiError( - error_code="update_secret_error", - message=f"User: {creator_user_id} cannot update the secret with key : {key}", - status_code=401, - ) + secret_model.value = value + db.session.add(secret_model) + try: + db.session.commit() + except Exception as e: + db.session.rollback() + raise e elif create_if_not_exists: SecretService.add_secret( - key=key, value=value, creator_user_id=creator_user_id + key=key, value=value, user_id=user_id ) else: raise ApiError( @@ -103,21 +96,14 @@ class SecretService: """Delete secret.""" secret_model = SecretModel.query.filter(SecretModel.key == key).first() if secret_model: - if secret_model.creator_user_id == user_id: - db.session.delete(secret_model) - try: - db.session.commit() - except Exception as e: - raise ApiError( - error_code="delete_secret_error", - message=f"Could not delete secret with key: {key}. Original error is: {e}", - ) from e - else: + db.session.delete(secret_model) + try: + db.session.commit() + except Exception as e: raise ApiError( error_code="delete_secret_error", - message=f"User: {user_id} cannot delete the secret with key : {key}", - status_code=401, - ) + message=f"Could not delete secret with key: {key}. Original error is: {e}", + ) from e else: raise ApiError( error_code="delete_secret_error", diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/service_task_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/service_task_service.py index 97c87996..0ec8a9cb 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/service_task_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/service_task_service.py @@ -4,6 +4,7 @@ from typing import Any import requests from flask import current_app +from flask import g from spiffworkflow_backend.services.file_system_service import FileSystemService from spiffworkflow_backend.services.secret_service import SecretService @@ -57,7 +58,20 @@ class ServiceTaskDelegate: if proxied_response.status_code != 200: print("got error from connector proxy") - return proxied_response.text + parsed_response = json.loads(proxied_response.text) + + if "refreshed_token_set" not in parsed_response: + return proxied_response.text + + secret_key = parsed_response["auth"] + refreshed_token_set = json.dumps( + parsed_response["refreshed_token_set"] + ) + SecretService().update_secret( + secret_key, refreshed_token_set, g.user.id + ) + + return json.dumps(parsed_response["api_response"]) class ServiceTaskService: 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 3cfc83a7..3735ebc5 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_secret_service.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_secret_service.py @@ -71,7 +71,7 @@ class TestSecretService(SecretServiceTestHelpers): assert test_secret is not None assert test_secret.key == self.test_key assert test_secret.value == self.test_value - assert test_secret.creator_user_id == with_super_admin_user.id + assert test_secret.user_id == with_super_admin_user.id def test_add_secret_duplicate_key_fails( self, @@ -129,24 +129,6 @@ class TestSecretService(SecretServiceTestHelpers): assert new_secret assert new_secret.value == "new_secret_value" # noqa: S105 - def test_update_secret_bad_user_fails( - self, - app: Flask, - client: FlaskClient, - with_db_and_bpmn_file_cleanup: None, - with_super_admin_user: UserModel, - ) -> None: - """Test_update_secret_bad_user.""" - self.add_test_secret(with_super_admin_user) - with pytest.raises(ApiError) as ae: - SecretService.update_secret( - self.test_key, "new_secret_value", with_super_admin_user.id + 1 - ) # noqa: S105 - assert ( - ae.value.message - == f"User: {with_super_admin_user.id+1} cannot update the secret with key : test_key" - ) - def test_update_secret_bad_secret_fails( self, app: Flask, @@ -174,27 +156,11 @@ class TestSecretService(SecretServiceTestHelpers): self.add_test_secret(with_super_admin_user) secrets = SecretModel.query.all() assert len(secrets) == 1 - assert secrets[0].creator_user_id == with_super_admin_user.id + assert secrets[0].user_id == with_super_admin_user.id SecretService.delete_secret(self.test_key, with_super_admin_user.id) secrets = SecretModel.query.all() assert len(secrets) == 0 - def test_delete_secret_bad_user_fails( - self, - app: Flask, - client: FlaskClient, - with_db_and_bpmn_file_cleanup: None, - with_super_admin_user: UserModel, - ) -> None: - """Test_delete_secret_bad_user.""" - self.add_test_secret(with_super_admin_user) - with pytest.raises(ApiError) as ae: - SecretService.delete_secret(self.test_key, with_super_admin_user.id + 1) - assert ( - f"User: {with_super_admin_user.id+1} cannot delete the secret with key" - in ae.value.message - ) - def test_delete_secret_bad_secret_fails( self, app: Flask, @@ -223,7 +189,7 @@ class TestSecretServiceApi(SecretServiceTestHelpers): secret_model = SecretModel( key=self.test_key, value=self.test_value, - creator_user_id=with_super_admin_user.id, + user_id=with_super_admin_user.id, ) data = json.dumps(SecretModelSchema().dump(secret_model)) response: TestResponse = client.post( @@ -234,11 +200,11 @@ class TestSecretServiceApi(SecretServiceTestHelpers): ) assert response.json secret: dict = response.json - for key in ["key", "value", "creator_user_id"]: + for key in ["key", "value", "user_id"]: assert key in secret.keys() assert secret["key"] == self.test_key assert secret["value"] == self.test_value - assert secret["creator_user_id"] == with_super_admin_user.id + assert secret["user_id"] == with_super_admin_user.id def test_get_secret( self, @@ -273,7 +239,7 @@ class TestSecretServiceApi(SecretServiceTestHelpers): secret_model = SecretModel( key=self.test_key, value="new_secret_value", - creator_user_id=with_super_admin_user.id, + user_id=with_super_admin_user.id, ) response = client.put( f"/v1.0/secrets/{self.test_key}", @@ -308,32 +274,6 @@ class TestSecretServiceApi(SecretServiceTestHelpers): with pytest.raises(ApiError): secret = SecretService.get_secret(self.test_key) - def test_delete_secret_bad_user( - self, - app: Flask, - client: FlaskClient, - with_db_and_bpmn_file_cleanup: None, - with_super_admin_user: UserModel, - ) -> None: - """Test_delete_secret_bad_user.""" - user_1 = self.find_or_create_user() - user_2 = self.find_or_create_user("test_user_2") - self.add_test_secret(user_1) - - # ensure user has permissions to delete the given secret - self.add_permissions_to_user( - user_2, - target_uri=f"/v1.0/secrets/{self.test_key}", - permission_names=["delete"], - ) - secret_response = client.delete( - f"/v1.0/secrets/{self.test_key}", - headers=self.logged_in_headers(user_2), - ) - assert secret_response.status_code == 401 - assert secret_response.json - assert secret_response.json["error_code"] == "delete_secret_error" - def test_delete_secret_bad_key( self, app: Flask,