From a10ef9066d6a7f8dc638fa71bca9102c8ad15822 Mon Sep 17 00:00:00 2001 From: Carlos Lopez Date: Wed, 15 Jul 2020 07:00:25 -0600 Subject: [PATCH 01/12] Github integration with admin --- Pipfile | 1 + Pipfile.lock | 23 ++++++++++++++++++- crc/api/admin.py | 58 ++++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 77 insertions(+), 5 deletions(-) diff --git a/Pipfile b/Pipfile index 0e5e21dd..67ee8473 100644 --- a/Pipfile +++ b/Pipfile @@ -44,6 +44,7 @@ webtest = "*" werkzeug = "*" xlrd = "*" xlsxwriter = "*" +pygithub = "*" [requires] python_version = "3.7" diff --git a/Pipfile.lock b/Pipfile.lock index 909cf764..42cee26b 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "97a15c4ade88db2b384d52436633889a4d9b0bdcaeea86b8a679ebda6f73fb59" + "sha256": "a4a720761a082a0ca31d2be17c2ea137e1d487ba2de538db334c8dc396770665" }, "pipfile-spec": 6, "requires": { @@ -235,6 +235,13 @@ "index": "pypi", "version": "==5.2" }, + "deprecated": { + "hashes": [ + "sha256:525ba66fb5f90b07169fdd48b6373c18f1ee12728ca277ca44567a367d9d7f74", + "sha256:a766c1dccb30c5f6eb2b203f87edd1d8588847709c78589e1521d769addc8218" + ], + "version": "==1.2.10" + }, "docutils": { "hashes": [ "sha256:0c5b78adfbf7762415433f5515cd5c9e762339e23369dbe8000d84a4bf4ab3af", @@ -669,6 +676,14 @@ ], "version": "==2.20" }, + "pygithub": { + "hashes": [ + "sha256:8375a058ec651cc0774244a3bc7395cf93617298735934cdd59e5bcd9a1df96e", + "sha256:d2d17d1e3f4474e070353f201164685a95b5a92f5ee0897442504e399c7bc249" + ], + "index": "pypi", + "version": "==1.51" + }, "pygments": { "hashes": [ "sha256:647344a061c249a3b74e230c739f434d7ea4d8b1d5f3721bc0f3558049b38f44", @@ -988,6 +1003,12 @@ "index": "pypi", "version": "==1.0.1" }, + "wrapt": { + "hashes": [ + "sha256:b62ffa81fb85f4332a4f609cab4ac40709470da05643a082ec1eb88e6d9b97d7" + ], + "version": "==1.12.1" + }, "wtforms": { "hashes": [ "sha256:6ff8635f4caeed9f38641d48cfe019d0d3896f41910ab04494143fc027866e1b", diff --git a/crc/api/admin.py b/crc/api/admin.py index 4e96fcd8..3943626f 100644 --- a/crc/api/admin.py +++ b/crc/api/admin.py @@ -3,16 +3,18 @@ import json from flask import url_for from flask_admin import Admin +from flask_admin.actions import action from flask_admin.contrib import sqla from flask_admin.contrib.sqla import ModelView +from github import Github, UnknownObjectException from werkzeug.utils import redirect from jinja2 import Markup from crc import db, app from crc.api.user import verify_token, verify_token_admin from crc.models.approval import ApprovalModel -from crc.models.file import FileModel -from crc.models.task_event import TaskEventModel +from crc.models.file import FileModel, FileDataModel +from crc.models.stats import TaskEventModel from crc.models.study import StudyModel from crc.models.user import UserModel from crc.models.workflow import WorkflowModel @@ -34,26 +36,73 @@ class AdminModelView(sqla.ModelView): # redirect to login page if user doesn't have access return redirect(url_for('home')) + class UserView(AdminModelView): column_filters = ['uid'] + class StudyView(AdminModelView): column_filters = ['id', 'primary_investigator_id'] column_searchable_list = ['title'] + class ApprovalView(AdminModelView): column_filters = ['study_id', 'approver_uid'] + class WorkflowView(AdminModelView): column_filters = ['study_id', 'id'] + class FileView(AdminModelView): - column_filters = ['workflow_id'] + column_filters = ['workflow_id', 'type'] + + @action('publish', 'Publish', 'Are you sure you want to publish this file(s)?') + def action_publish(self, ids): + # TODO: Move token to settings and replace docs repo + _github = Github('d082288d6192b45b2f8cefcefc1a0a2806554c9e') + repo = _github.get_user().get_repo('crispy-fiesta') + + for file_id in ids: + file_data_model = FileDataModel.query.filter_by(file_model_id=file_id).first() + try: + repo_file = repo.get_contents(file_data_model.file_model.name) + except UnknownObjectException: + repo.create_file( + path=file_data_model.file_model.name, + message=f'Creating {file_data_model.file_model.name}', + content=file_data_model.data + ) + else: + updated = repo.update_file( + path=repo_file.path, + message=f'Updating {file_data_model.file_model.name}', + content=file_data_model.data, + sha=repo_file.sha + ) + + @action('update', 'Update', 'Are you sure you want to update this file(s)?') + def action_update(self, ids): + _github = Github('d082288d6192b45b2f8cefcefc1a0a2806554c9e') + repo = _github.get_user().get_repo('crispy-fiesta') + + for file_id in ids: + file_data_model = FileDataModel.query.filter_by(file_model_id=file_id).first() + try: + repo_file = repo.get_contents(file_data_model.file_model.name) + except UnknownObjectException: + # Add message indicating file is not in the repo + pass + else: + file_data_model.data = repo_file.content + db.session.add(file_data_model) + db.session.commit() + def json_formatter(view, context, model, name): value = getattr(model, name) json_value = json.dumps(value, ensure_ascii=False, indent=2) - return Markup('
{}
'.format(json_value)) + return Markup(f'
{json_value}
') class TaskEventView(AdminModelView): column_filters = ['workflow_id', 'action'] @@ -62,6 +111,7 @@ class TaskEventView(AdminModelView): 'form_data': json_formatter, } + admin = Admin(app) admin.add_view(StudyView(StudyModel, db.session)) From 419d06c95b3c0c7fc88bfb611d848c3a5648f65b Mon Sep 17 00:00:00 2001 From: Carlos Lopez Date: Fri, 17 Jul 2020 09:49:47 -0600 Subject: [PATCH 02/12] Updating file by latest version --- config/default.py | 3 +++ crc/api/admin.py | 20 ++++++++++++++------ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/config/default.py b/config/default.py index 5c8f8c51..357ad8bb 100644 --- a/config/default.py +++ b/config/default.py @@ -45,6 +45,9 @@ PB_STUDY_DETAILS_URL = environ.get('PB_STUDY_DETAILS_URL', default=PB_BASE_URL + LDAP_URL = environ.get('LDAP_URL', default="ldap.virginia.edu").strip('/') # No trailing slash or http:// LDAP_TIMEOUT_SEC = int(environ.get('LDAP_TIMEOUT_SEC', default=1)) +# Github token +GH_TOKEN = '6cbd5f3a1764a8d15b27d66f64ac80ae13b393a9' + # Email configuration DEFAULT_SENDER = 'askresearch@virginia.edu' FALLBACK_EMAILS = ['askresearch@virginia.edu', 'sartographysupport@googlegroups.com'] diff --git a/crc/api/admin.py b/crc/api/admin.py index 3943626f..4edce46e 100644 --- a/crc/api/admin.py +++ b/crc/api/admin.py @@ -7,6 +7,7 @@ from flask_admin.actions import action from flask_admin.contrib import sqla from flask_admin.contrib.sqla import ModelView from github import Github, UnknownObjectException +from sqlalchemy import desc from werkzeug.utils import redirect from jinja2 import Markup @@ -60,7 +61,8 @@ class FileView(AdminModelView): @action('publish', 'Publish', 'Are you sure you want to publish this file(s)?') def action_publish(self, ids): # TODO: Move token to settings and replace docs repo - _github = Github('d082288d6192b45b2f8cefcefc1a0a2806554c9e') + gh_token = app.config['GH_TOKEN'] + _github = Github(gh_token) repo = _github.get_user().get_repo('crispy-fiesta') for file_id in ids: @@ -83,20 +85,26 @@ class FileView(AdminModelView): @action('update', 'Update', 'Are you sure you want to update this file(s)?') def action_update(self, ids): - _github = Github('d082288d6192b45b2f8cefcefc1a0a2806554c9e') + gh_token = app.config['GH_TOKEN'] + _github = Github(gh_token) repo = _github.get_user().get_repo('crispy-fiesta') for file_id in ids: - file_data_model = FileDataModel.query.filter_by(file_model_id=file_id).first() + file_data_model = FileDataModel.query.filter_by( + file_model_id=file_id + ).order_by( + desc(FileDataModel.version) + ).first() try: repo_file = repo.get_contents(file_data_model.file_model.name) except UnknownObjectException: # Add message indicating file is not in the repo pass else: - file_data_model.data = repo_file.content - db.session.add(file_data_model) - db.session.commit() + import pdb; pdb.set_trace() + file_data_model.data = repo_file.decoded_content + self.session.add(file_data_model) + self.session.commit() def json_formatter(view, context, model, name): From f4eb592b87e0cedcb6520102ab217d0afb6b4f6c Mon Sep 17 00:00:00 2001 From: Carlos Lopez Date: Fri, 17 Jul 2020 12:10:55 -0600 Subject: [PATCH 03/12] Extracting token to env var --- config/default.py | 2 +- crc/api/admin.py | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/config/default.py b/config/default.py index 357ad8bb..f1afc810 100644 --- a/config/default.py +++ b/config/default.py @@ -46,7 +46,7 @@ LDAP_URL = environ.get('LDAP_URL', default="ldap.virginia.edu").strip('/') # No LDAP_TIMEOUT_SEC = int(environ.get('LDAP_TIMEOUT_SEC', default=1)) # Github token -GH_TOKEN = '6cbd5f3a1764a8d15b27d66f64ac80ae13b393a9' +GITHUB_TOKEN = environ.get('GITHUB_TOKEN', None) # Email configuration DEFAULT_SENDER = 'askresearch@virginia.edu' diff --git a/crc/api/admin.py b/crc/api/admin.py index 4edce46e..aa10ab24 100644 --- a/crc/api/admin.py +++ b/crc/api/admin.py @@ -60,8 +60,8 @@ class FileView(AdminModelView): @action('publish', 'Publish', 'Are you sure you want to publish this file(s)?') def action_publish(self, ids): - # TODO: Move token to settings and replace docs repo - gh_token = app.config['GH_TOKEN'] + # TODO: Replace docs repo + gh_token = app.config['GITHUB_TOKEN'] _github = Github(gh_token) repo = _github.get_user().get_repo('crispy-fiesta') @@ -85,7 +85,7 @@ class FileView(AdminModelView): @action('update', 'Update', 'Are you sure you want to update this file(s)?') def action_update(self, ids): - gh_token = app.config['GH_TOKEN'] + gh_token = app.config['GITHUB_TOKEN'] _github = Github(gh_token) repo = _github.get_user().get_repo('crispy-fiesta') @@ -101,7 +101,6 @@ class FileView(AdminModelView): # Add message indicating file is not in the repo pass else: - import pdb; pdb.set_trace() file_data_model.data = repo_file.decoded_content self.session.add(file_data_model) self.session.commit() From 331a6c0aebc045aaaa5a773d8809aabbd0a21296 Mon Sep 17 00:00:00 2001 From: Carlos Lopez Date: Fri, 17 Jul 2020 12:52:09 -0600 Subject: [PATCH 04/12] Fixing tests --- crc/api/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crc/api/admin.py b/crc/api/admin.py index aa10ab24..3e1f560c 100644 --- a/crc/api/admin.py +++ b/crc/api/admin.py @@ -15,7 +15,7 @@ from crc import db, app from crc.api.user import verify_token, verify_token_admin from crc.models.approval import ApprovalModel from crc.models.file import FileModel, FileDataModel -from crc.models.stats import TaskEventModel +from crc.models.task_event import TaskEventModel from crc.models.study import StudyModel from crc.models.user import UserModel from crc.models.workflow import WorkflowModel From d34d08b12106b789fc692259ad05225552ba2474 Mon Sep 17 00:00:00 2001 From: Carlos Lopez Date: Fri, 17 Jul 2020 13:33:42 -0600 Subject: [PATCH 05/12] Trying to force re-run to clear SonarCloud --- crc/api/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crc/api/admin.py b/crc/api/admin.py index 3e1f560c..2af990a0 100644 --- a/crc/api/admin.py +++ b/crc/api/admin.py @@ -98,7 +98,7 @@ class FileView(AdminModelView): try: repo_file = repo.get_contents(file_data_model.file_model.name) except UnknownObjectException: - # Add message indicating file is not in the repo + # TODO: Add message indicating file is not in the repo pass else: file_data_model.data = repo_file.decoded_content From 73400ed6c73d5516631d875a8a49f680be69f566 Mon Sep 17 00:00:00 2001 From: Carlos Lopez Date: Fri, 17 Jul 2020 16:59:25 -0600 Subject: [PATCH 06/12] Adding proper tests for files publishing --- crc/api/admin.py | 44 +-------------- crc/services/file_service.py | 49 ++++++++++++++++ tests/files/test_file_service.py | 95 ++++++++++++++++++++++++++++++++ tests/test_ldap_service.py | 2 +- 4 files changed, 147 insertions(+), 43 deletions(-) diff --git a/crc/api/admin.py b/crc/api/admin.py index 2af990a0..7e85eace 100644 --- a/crc/api/admin.py +++ b/crc/api/admin.py @@ -6,7 +6,6 @@ from flask_admin import Admin from flask_admin.actions import action from flask_admin.contrib import sqla from flask_admin.contrib.sqla import ModelView -from github import Github, UnknownObjectException from sqlalchemy import desc from werkzeug.utils import redirect from jinja2 import Markup @@ -60,50 +59,11 @@ class FileView(AdminModelView): @action('publish', 'Publish', 'Are you sure you want to publish this file(s)?') def action_publish(self, ids): - # TODO: Replace docs repo - gh_token = app.config['GITHUB_TOKEN'] - _github = Github(gh_token) - repo = _github.get_user().get_repo('crispy-fiesta') - - for file_id in ids: - file_data_model = FileDataModel.query.filter_by(file_model_id=file_id).first() - try: - repo_file = repo.get_contents(file_data_model.file_model.name) - except UnknownObjectException: - repo.create_file( - path=file_data_model.file_model.name, - message=f'Creating {file_data_model.file_model.name}', - content=file_data_model.data - ) - else: - updated = repo.update_file( - path=repo_file.path, - message=f'Updating {file_data_model.file_model.name}', - content=file_data_model.data, - sha=repo_file.sha - ) + FileService.publish_to_github(ids) @action('update', 'Update', 'Are you sure you want to update this file(s)?') def action_update(self, ids): - gh_token = app.config['GITHUB_TOKEN'] - _github = Github(gh_token) - repo = _github.get_user().get_repo('crispy-fiesta') - - for file_id in ids: - file_data_model = FileDataModel.query.filter_by( - file_model_id=file_id - ).order_by( - desc(FileDataModel.version) - ).first() - try: - repo_file = repo.get_contents(file_data_model.file_model.name) - except UnknownObjectException: - # TODO: Add message indicating file is not in the repo - pass - else: - file_data_model.data = repo_file.decoded_content - self.session.add(file_data_model) - self.session.commit() + FileService.update_from_github(ids) def json_formatter(view, context, model, name): diff --git a/crc/services/file_service.py b/crc/services/file_service.py index 6ba2e1ad..90b30e42 100644 --- a/crc/services/file_service.py +++ b/crc/services/file_service.py @@ -2,6 +2,7 @@ import hashlib import json import os from datetime import datetime +from github import Github, UnknownObjectException from uuid import UUID from lxml import etree @@ -332,3 +333,51 @@ class FileService(object): file_model.archived = True session.commit() app.logger.info("Failed to delete file, so archiving it instead. %i, due to %s" % (file_id, str(ie))) + + @staticmethod + def update_from_github(file_ids): + gh_token = app.config['GITHUB_TOKEN'] + _github = Github(gh_token) + repo = _github.get_user().get_repo('crispy-fiesta') + + for file_id in file_ids: + file_data_model = FileDataModel.query.filter_by( + file_model_id=file_id + ).order_by( + desc(FileDataModel.version) + ).first() + try: + repo_file = repo.get_contents(file_data_model.file_model.name) + except UnknownObjectException: + # TODO: Add message indicating file is not in the repo + pass + else: + file_data_model.data = repo_file.decoded_content + session.add(file_data_model) + session.commit() + + @staticmethod + def publish_to_github(file_ids): + gh_token = app.config['GITHUB_TOKEN'] + _github = Github(gh_token) + repo = _github.get_user().get_repo('crispy-fiesta') + + for file_id in file_ids: + file_data_model = FileDataModel.query.filter_by(file_model_id=file_id).first() + try: + repo_file = repo.get_contents(file_data_model.file_model.name) + except UnknownObjectException: + repo.create_file( + path=file_data_model.file_model.name, + message=f'Creating {file_data_model.file_model.name}', + content=file_data_model.data + ) + return {'created': True} + else: + updated = repo.update_file( + path=repo_file.path, + message=f'Updating {file_data_model.file_model.name}', + content=file_data_model.data, + sha=repo_file.sha + ) + return {'updated': True} diff --git a/tests/files/test_file_service.py b/tests/files/test_file_service.py index dd95e458..e32f6bfb 100644 --- a/tests/files/test_file_service.py +++ b/tests/files/test_file_service.py @@ -1,9 +1,45 @@ +from github import UnknownObjectException +from sqlalchemy import desc from tests.base_test import BaseTest +from unittest.mock import patch, Mock from crc import db +from crc.models.file import FileDataModel from crc.services.file_service import FileService from crc.services.workflow_processor import WorkflowProcessor + +class FakeGithubCreates(Mock): + def get_user(var): + class FakeUser(Mock): + def get_repo(var, name): + class FakeRepo(Mock): + def get_contents(var, filename): + raise UnknownObjectException(status='Failure', data='Failed data') + def update_file(var, path, message, content, sha): + pass + return FakeRepo() + return FakeUser() + + +class FakeGithub(Mock): + def get_user(var): + class FakeUser(Mock): + def get_repo(var, name): + class FakeRepo(Mock): + def get_contents(var, filename): + fake_file = Mock() + fake_file.decoded_content = b'Some bytes' + fake_file.path = '/el/path/' + fake_file.data = 'Serious data' + fake_file.sha = 'Sha' + return fake_file + def update_file(var, path, message, content, sha): + pass + return FakeRepo() + return FakeUser() + + class TestFileService(BaseTest): """Largely tested via the test_file_api, and time is tight, but adding new tests here.""" @@ -103,3 +139,62 @@ class TestFileService(BaseTest): binary_data=b'5678') file_models = FileService.get_workflow_files(workflow_id=workflow.id) self.assertEqual(2, len(file_models)) + + @patch('crc.services.file_service.Github') + def test_update_from_github(self, mock_github): + mock_github.return_value = FakeGithub() + + self.load_example_data() + self.create_reference_document() + workflow = self.create_workflow('file_upload_form') + processor = WorkflowProcessor(workflow) + task = processor.next_task() + irb_code = "UVACompl_PRCAppr" # The first file referenced in pb required docs. + file_model = FileService.add_workflow_file(workflow_id=workflow.id, + irb_doc_code=irb_code, + name="anything.png", content_type="text", + binary_data=b'1234') + FileService.update_from_github([file_model.id]) + + file_model_data = FileDataModel.query.filter_by( + file_model_id=file_model.id + ).order_by( + desc(FileDataModel.version) + ).first() + self.assertEqual(file_model_data.data, b'Some bytes') + + @patch('crc.services.file_service.Github') + def test_publish_to_github_creates(self, mock_github): + mock_github.return_value = FakeGithubCreates() + + self.load_example_data() + self.create_reference_document() + workflow = self.create_workflow('file_upload_form') + processor = WorkflowProcessor(workflow) + task = processor.next_task() + irb_code = "UVACompl_PRCAppr" # The first file referenced in pb required docs. + file_model = FileService.add_workflow_file(workflow_id=workflow.id, + irb_doc_code=irb_code, + name="anything.png", content_type="text", + binary_data=b'1234') + result = FileService.publish_to_github([file_model.id]) + + self.assertEqual(result['created'], True) + + @patch('crc.services.file_service.Github') + def test_publish_to_github_updates(self, mock_github): + mock_github.return_value = FakeGithub() + + self.load_example_data() + self.create_reference_document() + workflow = self.create_workflow('file_upload_form') + processor = WorkflowProcessor(workflow) + task = processor.next_task() + irb_code = "UVACompl_PRCAppr" # The first file referenced in pb required docs. + file_model = FileService.add_workflow_file(workflow_id=workflow.id, + irb_doc_code=irb_code, + name="anything.png", content_type="text", + binary_data=b'1234') + result = FileService.publish_to_github([file_model.id]) + + self.assertEqual(result['updated'], True) diff --git a/tests/test_ldap_service.py b/tests/test_ldap_service.py index a6c4b364..d1e0ee21 100644 --- a/tests/test_ldap_service.py +++ b/tests/test_ldap_service.py @@ -30,4 +30,4 @@ class TestLdapService(BaseTest): user_info = LdapService.user_info("nosuch") self.assertFalse(True, "An API error should be raised.") except ApiError as ae: - self.assertEqual("missing_ldap_record", ae.code) \ No newline at end of file + self.assertEqual("missing_ldap_record", ae.code) From 59d04feb2340691931807e0b9ec989947186c271 Mon Sep 17 00:00:00 2001 From: Carlos Lopez Date: Fri, 17 Jul 2020 17:08:44 -0600 Subject: [PATCH 07/12] Adding missing import --- crc/api/admin.py | 1 + 1 file changed, 1 insertion(+) diff --git a/crc/api/admin.py b/crc/api/admin.py index 7e85eace..74ea3c37 100644 --- a/crc/api/admin.py +++ b/crc/api/admin.py @@ -18,6 +18,7 @@ from crc.models.task_event import TaskEventModel from crc.models.study import StudyModel from crc.models.user import UserModel from crc.models.workflow import WorkflowModel +from crc.services.file_service import FileService class AdminModelView(sqla.ModelView): From a39cacdf00818ad87bc7e2d48ade9c33db0242a4 Mon Sep 17 00:00:00 2001 From: Carlos Lopez Date: Sun, 19 Jul 2020 21:53:18 -0600 Subject: [PATCH 08/12] Ldap lookup script --- crc/scripts/ldap_lookup.py | 78 +++++++++++++++++++++++++++ crc/scripts/request_approval.py | 2 +- tests/ldap/test_ldap_lookup_script.py | 51 ++++++++++++++++++ 3 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 crc/scripts/ldap_lookup.py create mode 100644 tests/ldap/test_ldap_lookup_script.py diff --git a/crc/scripts/ldap_lookup.py b/crc/scripts/ldap_lookup.py new file mode 100644 index 00000000..62bd287a --- /dev/null +++ b/crc/scripts/ldap_lookup.py @@ -0,0 +1,78 @@ +import copy + +from crc import app +from crc.api.common import ApiError +from crc.scripts.script import Script +from crc.services.ldap_service import LdapService + + +USER_DETAILS = { + "PIComputingID": { + "value": "", + "data": { + }, + "label": "invalid uid" + } +} + + +class LdapLookup(Script): + """This Script allows to be introduced as part of a workflow and called from there, taking + a UID as input and looking it up through LDAP to return the person's details """ + + def get_description(self): + return """ +Attempts to create a dictionary with person details, using the +provided argument (a UID) and look it up through LDAP. + +Example: +LdapLookup PIComputingID +""" + + def do_task_validate_only(self, task, *args, **kwargs): + self.get_user_info(task, args) + + def do_task(self, task, *args, **kwargs): + args = [arg for arg in args if type(arg) == str] + user_info = self.get_user_info(task, args) + + user_details = copy.deepcopy(USER_DETAILS) + user_details['PIComputingID']['value'] = user_info['uid'] + if len(user_info.keys()) > 1: + user_details['PIComputingID']['label'] = user_info.pop('label') + else: + user_info.pop('uid') + user_details['PIComputingID']['data'] = user_info + return user_details + + def get_user_info(self, task, args): + if len(args) < 1: + raise ApiError(code="missing_argument", + message="Ldap lookup script requires one argument. The " + "UID for the person we want to look up") + + arg = args.pop() # Extracting only one value for now + uid = task.workflow.script_engine.evaluate_expression(task, arg) + if not isinstance(uid, str): + raise ApiError(code="invalid_argument", + message="Ldap lookup script requires one 1 UID argument, of type string.") + user_info_dict = {} + try: + user_info = LdapService.user_info(uid) + user_info_dict = { + "display_name": user_info.display_name, + "given_name": user_info.given_name, + "email_address": user_info.email_address, + "telephone_number": user_info.telephone_number, + "title": user_info.title, + "department": user_info.department, + "affiliation": user_info.affiliation, + "sponsor_type": user_info.sponsor_type, + "uid": user_info.uid, + "label": user_info.proper_name() + } + except: + user_info_dict['uid'] = uid + app.logger.error(f'Ldap lookup failed for UID {uid}') + + return user_info_dict diff --git a/crc/scripts/request_approval.py b/crc/scripts/request_approval.py index 0a4c76ff..a82e17a0 100644 --- a/crc/scripts/request_approval.py +++ b/crc/scripts/request_approval.py @@ -11,7 +11,7 @@ class RequestApproval(Script): return """ Creates an approval request on this workflow, by the given approver_uid(s)," Takes multiple arguments, which should point to data located in current task -or be quoted strings. The order is important. Approvals will be processed +or be quoted strings. The order is important. Approvals will be processed in this order. Example: diff --git a/tests/ldap/test_ldap_lookup_script.py b/tests/ldap/test_ldap_lookup_script.py new file mode 100644 index 00000000..7cb7ff55 --- /dev/null +++ b/tests/ldap/test_ldap_lookup_script.py @@ -0,0 +1,51 @@ +from tests.base_test import BaseTest + +from crc.services.workflow_processor import WorkflowProcessor +from crc.scripts.ldap_lookup import LdapLookup +from crc import db, mail + + +class TestLdapLookupScript(BaseTest): + + def test_get_existing_user_details(self): + self.load_example_data() + self.create_reference_document() + workflow = self.create_workflow('empty_workflow') + processor = WorkflowProcessor(workflow) + task = processor.next_task() + + task.data = { + 'PIComputingID': 'dhf8r' + } + + script = LdapLookup() + user_details = script.do_task(task, workflow.study_id, workflow.id, "PIComputingID") + + self.assertEqual(user_details['PIComputingID']['label'], 'Dan Funk - (dhf8r)') + self.assertEqual(user_details['PIComputingID']['value'], 'dhf8r') + self.assertEqual(user_details['PIComputingID']['data']['display_name'], 'Dan Funk') + self.assertEqual(user_details['PIComputingID']['data']['given_name'], 'Dan') + self.assertEqual(user_details['PIComputingID']['data']['email_address'], 'dhf8r@virginia.edu') + self.assertEqual(user_details['PIComputingID']['data']['telephone_number'], '+1 (434) 924-1723') + self.assertEqual(user_details['PIComputingID']['data']['title'], 'E42:He\'s a hoopy frood') + self.assertEqual(user_details['PIComputingID']['data']['department'], 'E0:EN-Eng Study of Parallel Universes') + self.assertEqual(user_details['PIComputingID']['data']['affiliation'], 'faculty') + self.assertEqual(user_details['PIComputingID']['data']['sponsor_type'], 'Staff') + self.assertEqual(user_details['PIComputingID']['data']['uid'], 'dhf8r') + + def test_get_invalid_user_details(self): + self.load_example_data() + self.create_reference_document() + workflow = self.create_workflow('empty_workflow') + processor = WorkflowProcessor(workflow) + task = processor.next_task() + + task.data = { + 'PIComputingID': 'rec3z' + } + + script = LdapLookup() + user_details = script.do_task(task, workflow.study_id, workflow.id, "PIComputingID") + self.assertEqual(user_details['PIComputingID']['label'], 'invalid uid') + self.assertEqual(user_details['PIComputingID']['value'], 'rec3z') + self.assertEqual(user_details['PIComputingID']['data'], {}) From 313770d5389d37c5988f76d1a0edb14d7b8baa2a Mon Sep 17 00:00:00 2001 From: Carlos Lopez Date: Sun, 19 Jul 2020 21:53:18 -0600 Subject: [PATCH 09/12] Ldap lookup script --- crc/scripts/ldap_lookup.py | 78 +++++++++++++++++++++++++++ crc/scripts/request_approval.py | 2 +- tests/ldap/test_ldap_lookup_script.py | 51 ++++++++++++++++++ 3 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 crc/scripts/ldap_lookup.py create mode 100644 tests/ldap/test_ldap_lookup_script.py diff --git a/crc/scripts/ldap_lookup.py b/crc/scripts/ldap_lookup.py new file mode 100644 index 00000000..62bd287a --- /dev/null +++ b/crc/scripts/ldap_lookup.py @@ -0,0 +1,78 @@ +import copy + +from crc import app +from crc.api.common import ApiError +from crc.scripts.script import Script +from crc.services.ldap_service import LdapService + + +USER_DETAILS = { + "PIComputingID": { + "value": "", + "data": { + }, + "label": "invalid uid" + } +} + + +class LdapLookup(Script): + """This Script allows to be introduced as part of a workflow and called from there, taking + a UID as input and looking it up through LDAP to return the person's details """ + + def get_description(self): + return """ +Attempts to create a dictionary with person details, using the +provided argument (a UID) and look it up through LDAP. + +Example: +LdapLookup PIComputingID +""" + + def do_task_validate_only(self, task, *args, **kwargs): + self.get_user_info(task, args) + + def do_task(self, task, *args, **kwargs): + args = [arg for arg in args if type(arg) == str] + user_info = self.get_user_info(task, args) + + user_details = copy.deepcopy(USER_DETAILS) + user_details['PIComputingID']['value'] = user_info['uid'] + if len(user_info.keys()) > 1: + user_details['PIComputingID']['label'] = user_info.pop('label') + else: + user_info.pop('uid') + user_details['PIComputingID']['data'] = user_info + return user_details + + def get_user_info(self, task, args): + if len(args) < 1: + raise ApiError(code="missing_argument", + message="Ldap lookup script requires one argument. The " + "UID for the person we want to look up") + + arg = args.pop() # Extracting only one value for now + uid = task.workflow.script_engine.evaluate_expression(task, arg) + if not isinstance(uid, str): + raise ApiError(code="invalid_argument", + message="Ldap lookup script requires one 1 UID argument, of type string.") + user_info_dict = {} + try: + user_info = LdapService.user_info(uid) + user_info_dict = { + "display_name": user_info.display_name, + "given_name": user_info.given_name, + "email_address": user_info.email_address, + "telephone_number": user_info.telephone_number, + "title": user_info.title, + "department": user_info.department, + "affiliation": user_info.affiliation, + "sponsor_type": user_info.sponsor_type, + "uid": user_info.uid, + "label": user_info.proper_name() + } + except: + user_info_dict['uid'] = uid + app.logger.error(f'Ldap lookup failed for UID {uid}') + + return user_info_dict diff --git a/crc/scripts/request_approval.py b/crc/scripts/request_approval.py index 0a4c76ff..a82e17a0 100644 --- a/crc/scripts/request_approval.py +++ b/crc/scripts/request_approval.py @@ -11,7 +11,7 @@ class RequestApproval(Script): return """ Creates an approval request on this workflow, by the given approver_uid(s)," Takes multiple arguments, which should point to data located in current task -or be quoted strings. The order is important. Approvals will be processed +or be quoted strings. The order is important. Approvals will be processed in this order. Example: diff --git a/tests/ldap/test_ldap_lookup_script.py b/tests/ldap/test_ldap_lookup_script.py new file mode 100644 index 00000000..7cb7ff55 --- /dev/null +++ b/tests/ldap/test_ldap_lookup_script.py @@ -0,0 +1,51 @@ +from tests.base_test import BaseTest + +from crc.services.workflow_processor import WorkflowProcessor +from crc.scripts.ldap_lookup import LdapLookup +from crc import db, mail + + +class TestLdapLookupScript(BaseTest): + + def test_get_existing_user_details(self): + self.load_example_data() + self.create_reference_document() + workflow = self.create_workflow('empty_workflow') + processor = WorkflowProcessor(workflow) + task = processor.next_task() + + task.data = { + 'PIComputingID': 'dhf8r' + } + + script = LdapLookup() + user_details = script.do_task(task, workflow.study_id, workflow.id, "PIComputingID") + + self.assertEqual(user_details['PIComputingID']['label'], 'Dan Funk - (dhf8r)') + self.assertEqual(user_details['PIComputingID']['value'], 'dhf8r') + self.assertEqual(user_details['PIComputingID']['data']['display_name'], 'Dan Funk') + self.assertEqual(user_details['PIComputingID']['data']['given_name'], 'Dan') + self.assertEqual(user_details['PIComputingID']['data']['email_address'], 'dhf8r@virginia.edu') + self.assertEqual(user_details['PIComputingID']['data']['telephone_number'], '+1 (434) 924-1723') + self.assertEqual(user_details['PIComputingID']['data']['title'], 'E42:He\'s a hoopy frood') + self.assertEqual(user_details['PIComputingID']['data']['department'], 'E0:EN-Eng Study of Parallel Universes') + self.assertEqual(user_details['PIComputingID']['data']['affiliation'], 'faculty') + self.assertEqual(user_details['PIComputingID']['data']['sponsor_type'], 'Staff') + self.assertEqual(user_details['PIComputingID']['data']['uid'], 'dhf8r') + + def test_get_invalid_user_details(self): + self.load_example_data() + self.create_reference_document() + workflow = self.create_workflow('empty_workflow') + processor = WorkflowProcessor(workflow) + task = processor.next_task() + + task.data = { + 'PIComputingID': 'rec3z' + } + + script = LdapLookup() + user_details = script.do_task(task, workflow.study_id, workflow.id, "PIComputingID") + self.assertEqual(user_details['PIComputingID']['label'], 'invalid uid') + self.assertEqual(user_details['PIComputingID']['value'], 'rec3z') + self.assertEqual(user_details['PIComputingID']['data'], {}) From 522f848682f296351ec8d849ab5ee07d37e70df6 Mon Sep 17 00:00:00 2001 From: Carlos Lopez Date: Tue, 21 Jul 2020 20:53:24 -0600 Subject: [PATCH 10/12] Addressing feedback --- crc/scripts/ldap_lookup.py | 88 +++++++++-------------- tests/data/ldap_replace/ldap_replace.bpmn | 69 ++++++++++++++++++ tests/ldap/test_ldap_lookup_script.py | 84 +++++++++++++++++----- 3 files changed, 171 insertions(+), 70 deletions(-) create mode 100644 tests/data/ldap_replace/ldap_replace.bpmn diff --git a/crc/scripts/ldap_lookup.py b/crc/scripts/ldap_lookup.py index 62bd287a..88e2986a 100644 --- a/crc/scripts/ldap_lookup.py +++ b/crc/scripts/ldap_lookup.py @@ -6,73 +6,55 @@ from crc.scripts.script import Script from crc.services.ldap_service import LdapService -USER_DETAILS = { - "PIComputingID": { - "value": "", - "data": { - }, - "label": "invalid uid" - } -} - - -class LdapLookup(Script): +class LdapReplace(Script): """This Script allows to be introduced as part of a workflow and called from there, taking - a UID as input and looking it up through LDAP to return the person's details """ + a UID (or several) as input and looking it up through LDAP to return the person's details """ def get_description(self): return """ Attempts to create a dictionary with person details, using the provided argument (a UID) and look it up through LDAP. -Example: -LdapLookup PIComputingID +Examples: +#! LdapReplace supervisor +#! LdapReplace supervisor collaborator +#! LdapReplace supervisor cosupervisor collaborator """ def do_task_validate_only(self, task, *args, **kwargs): - self.get_user_info(task, args) + self.set_users_info_in_task(task, args) def do_task(self, task, *args, **kwargs): args = [arg for arg in args if type(arg) == str] - user_info = self.get_user_info(task, args) + self.set_users_info_in_task(task, args) - user_details = copy.deepcopy(USER_DETAILS) - user_details['PIComputingID']['value'] = user_info['uid'] - if len(user_info.keys()) > 1: - user_details['PIComputingID']['label'] = user_info.pop('label') - else: - user_info.pop('uid') - user_details['PIComputingID']['data'] = user_info - return user_details - - def get_user_info(self, task, args): + def set_users_info_in_task(self, task, args): if len(args) < 1: raise ApiError(code="missing_argument", - message="Ldap lookup script requires one argument. The " - "UID for the person we want to look up") + message="Ldap replace script requires at least one argument. The " + "UID for the person(s) we want to look up") - arg = args.pop() # Extracting only one value for now - uid = task.workflow.script_engine.evaluate_expression(task, arg) - if not isinstance(uid, str): - raise ApiError(code="invalid_argument", - message="Ldap lookup script requires one 1 UID argument, of type string.") - user_info_dict = {} - try: - user_info = LdapService.user_info(uid) - user_info_dict = { - "display_name": user_info.display_name, - "given_name": user_info.given_name, - "email_address": user_info.email_address, - "telephone_number": user_info.telephone_number, - "title": user_info.title, - "department": user_info.department, - "affiliation": user_info.affiliation, - "sponsor_type": user_info.sponsor_type, - "uid": user_info.uid, - "label": user_info.proper_name() - } - except: - user_info_dict['uid'] = uid - app.logger.error(f'Ldap lookup failed for UID {uid}') - - return user_info_dict + users_info = {} + for arg in args: + uid = task.workflow.script_engine.evaluate_expression(task, arg) + if not isinstance(uid, str): + raise ApiError(code="invalid_argument", + message="Ldap replace script found an invalid argument, type string is required") + user_info_dict = {} + try: + user_info = LdapService.user_info(uid) + user_info_dict = { + "display_name": user_info.display_name, + "given_name": user_info.given_name, + "email_address": user_info.email_address, + "telephone_number": user_info.telephone_number, + "title": user_info.title, + "department": user_info.department, + "affiliation": user_info.affiliation, + "sponsor_type": user_info.sponsor_type, + "uid": user_info.uid, + "proper_name": user_info.proper_name() + } + except: + app.logger.error(f'Ldap replace failed for UID {uid}') + task.data[arg] = user_info_dict diff --git a/tests/data/ldap_replace/ldap_replace.bpmn b/tests/data/ldap_replace/ldap_replace.bpmn new file mode 100644 index 00000000..77f8c7ad --- /dev/null +++ b/tests/data/ldap_replace/ldap_replace.bpmn @@ -0,0 +1,69 @@ + + + + + Flow_1synsig + + + Flow_11e7jgz + + + Flow_08n2npe + Flow_1xlrgne + #! LdapReplace Supervisor Investigator + + + + + + + + + + + + Flow_1synsig + Flow_08n2npe + + + + Flow_1xlrgne + Flow_11e7jgz + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/ldap/test_ldap_lookup_script.py b/tests/ldap/test_ldap_lookup_script.py index 7cb7ff55..d56126cc 100644 --- a/tests/ldap/test_ldap_lookup_script.py +++ b/tests/ldap/test_ldap_lookup_script.py @@ -1,7 +1,7 @@ from tests.base_test import BaseTest from crc.services.workflow_processor import WorkflowProcessor -from crc.scripts.ldap_lookup import LdapLookup +from crc.scripts.ldap_lookup import LdapReplace from crc import db, mail @@ -18,20 +18,56 @@ class TestLdapLookupScript(BaseTest): 'PIComputingID': 'dhf8r' } - script = LdapLookup() + script = LdapReplace() user_details = script.do_task(task, workflow.study_id, workflow.id, "PIComputingID") - self.assertEqual(user_details['PIComputingID']['label'], 'Dan Funk - (dhf8r)') - self.assertEqual(user_details['PIComputingID']['value'], 'dhf8r') - self.assertEqual(user_details['PIComputingID']['data']['display_name'], 'Dan Funk') - self.assertEqual(user_details['PIComputingID']['data']['given_name'], 'Dan') - self.assertEqual(user_details['PIComputingID']['data']['email_address'], 'dhf8r@virginia.edu') - self.assertEqual(user_details['PIComputingID']['data']['telephone_number'], '+1 (434) 924-1723') - self.assertEqual(user_details['PIComputingID']['data']['title'], 'E42:He\'s a hoopy frood') - self.assertEqual(user_details['PIComputingID']['data']['department'], 'E0:EN-Eng Study of Parallel Universes') - self.assertEqual(user_details['PIComputingID']['data']['affiliation'], 'faculty') - self.assertEqual(user_details['PIComputingID']['data']['sponsor_type'], 'Staff') - self.assertEqual(user_details['PIComputingID']['data']['uid'], 'dhf8r') + self.assertEqual(task.data['PIComputingID']['display_name'], 'Dan Funk') + self.assertEqual(task.data['PIComputingID']['given_name'], 'Dan') + self.assertEqual(task.data['PIComputingID']['email_address'], 'dhf8r@virginia.edu') + self.assertEqual(task.data['PIComputingID']['telephone_number'], '+1 (434) 924-1723') + self.assertEqual(task.data['PIComputingID']['title'], 'E42:He\'s a hoopy frood') + self.assertEqual(task.data['PIComputingID']['department'], 'E0:EN-Eng Study of Parallel Universes') + self.assertEqual(task.data['PIComputingID']['affiliation'], 'faculty') + self.assertEqual(task.data['PIComputingID']['sponsor_type'], 'Staff') + self.assertEqual(task.data['PIComputingID']['uid'], 'dhf8r') + self.assertEqual(task.data['PIComputingID']['proper_name'], 'Dan Funk - (dhf8r)') + + def test_get_existing_users_details(self): + self.load_example_data() + self.create_reference_document() + workflow = self.create_workflow('empty_workflow') + processor = WorkflowProcessor(workflow) + task = processor.next_task() + + task.data = { + 'supervisor': 'dhf8r', + 'investigator': 'lb3dp' + } + + script = LdapReplace() + user_details = script.do_task(task, workflow.study_id, workflow.id, "supervisor", "investigator") + + self.assertEqual(task.data['supervisor']['display_name'], 'Dan Funk') + self.assertEqual(task.data['supervisor']['given_name'], 'Dan') + self.assertEqual(task.data['supervisor']['email_address'], 'dhf8r@virginia.edu') + self.assertEqual(task.data['supervisor']['telephone_number'], '+1 (434) 924-1723') + self.assertEqual(task.data['supervisor']['title'], 'E42:He\'s a hoopy frood') + self.assertEqual(task.data['supervisor']['department'], 'E0:EN-Eng Study of Parallel Universes') + self.assertEqual(task.data['supervisor']['affiliation'], 'faculty') + self.assertEqual(task.data['supervisor']['sponsor_type'], 'Staff') + self.assertEqual(task.data['supervisor']['uid'], 'dhf8r') + self.assertEqual(task.data['supervisor']['proper_name'], 'Dan Funk - (dhf8r)') + + self.assertEqual(task.data['investigator']['display_name'], 'Laura Barnes') + self.assertEqual(task.data['investigator']['given_name'], 'Laura') + self.assertEqual(task.data['investigator']['email_address'], 'lb3dp@virginia.edu') + self.assertEqual(task.data['investigator']['telephone_number'], '+1 (434) 924-1723') + self.assertEqual(task.data['investigator']['title'], 'E0:Associate Professor of Systems and Information Engineering') + self.assertEqual(task.data['investigator']['department'], 'E0:EN-Eng Sys and Environment') + self.assertEqual(task.data['investigator']['affiliation'], 'faculty') + self.assertEqual(task.data['investigator']['sponsor_type'], 'Staff') + self.assertEqual(task.data['investigator']['uid'], 'lb3dp') + self.assertEqual(task.data['investigator']['proper_name'], 'Laura Barnes - (lb3dp)') def test_get_invalid_user_details(self): self.load_example_data() @@ -44,8 +80,22 @@ class TestLdapLookupScript(BaseTest): 'PIComputingID': 'rec3z' } - script = LdapLookup() + script = LdapReplace() user_details = script.do_task(task, workflow.study_id, workflow.id, "PIComputingID") - self.assertEqual(user_details['PIComputingID']['label'], 'invalid uid') - self.assertEqual(user_details['PIComputingID']['value'], 'rec3z') - self.assertEqual(user_details['PIComputingID']['data'], {}) + + self.assertEqual(task.data['PIComputingID'], {}) + + def test_bpmn_task_receives_user_details(self): + workflow = self.create_workflow('ldap_replace') + + task_data = { + 'Supervisor': 'dhf8r', + 'Investigator': 'lb3dp' + } + task = self.get_workflow_api(workflow).next_task + + self.complete_form(workflow, task, task_data) + + task = self.get_workflow_api(workflow).next_task + + self.assertEqual(task.data['Supervisor']['proper_name'], 'Dan Funk - (dhf8r)') From 41cbce8e01c054e589025bbab9964c31e30403bd Mon Sep 17 00:00:00 2001 From: Carlos Lopez Date: Tue, 21 Jul 2020 21:08:08 -0600 Subject: [PATCH 11/12] Updating bpmn test --- crc/scripts/{ldap_lookup.py => ldap_replace.py} | 0 tests/ldap/test_ldap_lookup_script.py | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename crc/scripts/{ldap_lookup.py => ldap_replace.py} (100%) diff --git a/crc/scripts/ldap_lookup.py b/crc/scripts/ldap_replace.py similarity index 100% rename from crc/scripts/ldap_lookup.py rename to crc/scripts/ldap_replace.py diff --git a/tests/ldap/test_ldap_lookup_script.py b/tests/ldap/test_ldap_lookup_script.py index d56126cc..c19f0144 100644 --- a/tests/ldap/test_ldap_lookup_script.py +++ b/tests/ldap/test_ldap_lookup_script.py @@ -1,7 +1,7 @@ from tests.base_test import BaseTest from crc.services.workflow_processor import WorkflowProcessor -from crc.scripts.ldap_lookup import LdapReplace +from crc.scripts.ldap_replace import LdapReplace from crc import db, mail From 74e5e07114b09c6c824fc7b6afa4a1aedea0f909 Mon Sep 17 00:00:00 2001 From: Carlos Lopez Date: Thu, 23 Jul 2020 07:41:29 -0600 Subject: [PATCH 12/12] Testing for all values --- tests/ldap/test_ldap_lookup_script.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/ldap/test_ldap_lookup_script.py b/tests/ldap/test_ldap_lookup_script.py index c19f0144..220ca9c8 100644 --- a/tests/ldap/test_ldap_lookup_script.py +++ b/tests/ldap/test_ldap_lookup_script.py @@ -98,4 +98,13 @@ class TestLdapLookupScript(BaseTest): task = self.get_workflow_api(workflow).next_task + self.assertEqual(task.data['Supervisor']['display_name'], 'Dan Funk') + self.assertEqual(task.data['Supervisor']['given_name'], 'Dan') + self.assertEqual(task.data['Supervisor']['email_address'], 'dhf8r@virginia.edu') + self.assertEqual(task.data['Supervisor']['telephone_number'], '+1 (434) 924-1723') + self.assertEqual(task.data['Supervisor']['title'], 'E42:He\'s a hoopy frood') + self.assertEqual(task.data['Supervisor']['department'], 'E0:EN-Eng Study of Parallel Universes') + self.assertEqual(task.data['Supervisor']['affiliation'], 'faculty') + self.assertEqual(task.data['Supervisor']['sponsor_type'], 'Staff') + self.assertEqual(task.data['Supervisor']['uid'], 'dhf8r') self.assertEqual(task.data['Supervisor']['proper_name'], 'Dan Funk - (dhf8r)')