From 3cf1e9427f65d4885b75a7399682ab6b04b16bc2 Mon Sep 17 00:00:00 2001 From: RayPlante Date: Wed, 21 Aug 2024 07:35:42 -0400 Subject: [PATCH 1/4] dbio.project: set default permissions based on configuration --- python/nistoar/midas/dbio/project.py | 34 +++++++++++++++++-- .../tests/nistoar/midas/dbio/test_project.py | 8 +++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/python/nistoar/midas/dbio/project.py b/python/nistoar/midas/dbio/project.py index 4399aab..95af9dd 100644 --- a/python/nistoar/midas/dbio/project.py +++ b/python/nistoar/midas/dbio/project.py @@ -25,6 +25,7 @@ from nistoar.pdr.utils.prov import Agent, Action from nistoar.id.versions import OARVersion from nistoar.pdr import ARK_NAAN +from nistoar.base.config import ConfigurationException _STATUS_ACTION_CREATE = RecordStatus.CREATE_ACTION _STATUS_ACTION_UPDATE = RecordStatus.UPDATE_ACTION @@ -42,10 +43,16 @@ class ProjectService(MIDASSystem): to a particular user at construction time (as given by a :py:class:`~nistoar.pdr.utils.Agent` instance); thus, requests to this service are subject to internal Authorization checks. - This base service supports a two parameters, ``dbio`` and ``clients``. The optional ``dbio`` - parameter will be passed to the :py:class:`~nistoar.midas.dbio.base.DBClientFactory`'s + This base service supports three parameters, ``dbio``, ``default_perms``, and ``clients``. The + optional ``dbio`` parameter will be passed to the :py:class:`~nistoar.midas.dbio.base.DBClientFactory`'s ``create_client()`` function to create the :py:class:`~nistoar.midas.dbio.base.DBClient`. + The optional ``default_perms`` is an object that sets the ACLs for newly created project records. + Its optional properties name the permisson types that defaults are to be set for, including "read", + "write", "admin", and "delete" but can also include other (non-standard) category names. Each + property is a list of user identifiers that the should be given the particular type of permission. + Typically, only virtual group identifiers (like "grp0:public") make sense. + The ``clients`` parameter is an object that places restrictions on the creation of records based on which group the user is part of. The keys of the object are user group names that are authorized to use this service, and whose values are themselves objects @@ -92,7 +99,24 @@ def __init__(self, project_type: str, dbclient_factory: DBClientFactory, config: if not _subsysabbrev: _subsysabbrev = "DBIO" super(ProjectService, self).__init__(_subsys, _subsysabbrev) + + # set configuration, check values self.cfg = config + for param in "clients default_perms".split(): + if not isinstance(self.cfg.get(param,{}), Mapping): + raise ConfigurationException("%s: value is not a object as required: %s" % + (param, type(self.cfg.get(param)))) + for param,val in self.cfg.get('clients',{}).items(): + if not isinstance(val, Mapping): + raise ConfigurationException("clients.%s: value is not a object as required: %s" % + (param, repr(val))) + for param,val in self.cfg.get('default_perms',{}).items(): + if not isinstance(val, list) or not all(isinstance(p, str) for p in val): + raise ConfigurationException( + "default_perms.%s: value is not a list of strings as required: %s" % + (param, repr(val)) + ) + if not who: who = Agent("dbio.project", Agent.USER, Agent.ANONYMOUS, Agent.PUBLIC) self.who = who @@ -124,6 +148,7 @@ def create_record(self, name, data=None, meta=None) -> ProjectRecord: if self.dbcli.user_id == ANONYMOUS: self.log.warning("A new record requested for an anonymous user") prec = self.dbcli.create_record(name, shoulder) + self._set_default_perms(prec.acls) if meta: meta = self._moderate_metadata(meta, shoulder) @@ -144,6 +169,11 @@ def create_record(self, name, data=None, meta=None) -> ProjectRecord: self.log.info("Created %s record %s (%s) for %s", self.dbcli.project, prec.id, prec.name, self.who) return prec + def _set_default_perms(self, acls: ACLs): + defs = self.cfg.get("default_perms", {}) + for perm in defs: + acls.grant_perm_to(perm, *defs[perm]) + def delete_record(self, id) -> ProjectRecord: """ delete the draft record. This may leave a stub record in place if, for example, the record diff --git a/python/tests/nistoar/midas/dbio/test_project.py b/python/tests/nistoar/midas/dbio/test_project.py index 75510d1..b82c2b9 100644 --- a/python/tests/nistoar/midas/dbio/test_project.py +++ b/python/tests/nistoar/midas/dbio/test_project.py @@ -40,6 +40,10 @@ def setUp(self): "default_shoulder": "mdm0" } }, + "default_perms": { + "read": ["grp0:public"], + "edit": ["grp0:overlord"] + }, "dbio": { "allowed_project_shoulders": ["mdm1", "spc1"], "default_shoulder": "mdm0" @@ -114,6 +118,10 @@ def test_create_record(self): self.assertEqual(prec.status.message, "draft created") self.assertEqual(prec.status.state, "edit") + self.assertEqual(prec.acls._perms.get("read"), [ self.project.who.actor, "grp0:public"]) + self.assertEqual(prec.acls._perms.get("write"), [ self.project.who.actor ]) + self.assertEqual(prec.acls._perms.get("edit"), [ "grp0:overlord"]) + self.assertTrue(self.project.dbcli.name_exists("goob")) prec2 = self.project.get_record(prec.id) self.assertEqual(prec2.name, "goob") From 0f91c348013b27516a1425995648cd8ae0ef7b9e Mon Sep 17 00:00:00 2001 From: RayPlante Date: Thu, 31 Oct 2024 07:21:22 -0400 Subject: [PATCH 2/4] dap.fm.apiclient: temporary fix for fm webdav switch to x509-based auth --- python/nistoar/midas/dap/fm/apiclient.py | 59 +++++++++++++++++- .../nistoar/midas/dap/fm/test_apiclient.py | 62 ++++++++++++++++++- 2 files changed, 116 insertions(+), 5 deletions(-) diff --git a/python/nistoar/midas/dap/fm/apiclient.py b/python/nistoar/midas/dap/fm/apiclient.py index 3616ff5..c0b4bc6 100644 --- a/python/nistoar/midas/dap/fm/apiclient.py +++ b/python/nistoar/midas/dap/fm/apiclient.py @@ -98,6 +98,8 @@ def __init__(self, config): "auth.username and/or auth.password") self.auth_user = authcfg['username'] self.auth_pass = authcfg['password'] + self.webdavauthurl = authcfg.get('webdav_auth_url') + self.webdavauthtype = authcfg.get('webdav_auth_type', 'userpass') self.token = None def authenticate(self): @@ -234,14 +236,17 @@ def get_uploads_directory(self, record_name): """ path = f"{record_name}/{record_name}" url = f"{self.dav_base}/{path}" - auth = (self.auth_user, self.auth_pass) header = {"Depth": "0", "Content-type": "application/xml"} + passw = self.auth_pass + if self.webdavauthurl: + passw = self._webdav_auth(self.webdavauthurl, self.webdavauthtype) + auth = (self.auth_user, passw) try: resp = requests.request("PROPFIND", url, data=webdav.info_request, headers=header, auth=auth) except requests.RequestException as ex: - raise FileSpaceConnectionError("Problem communicating with file manaager: "+str(ex)) + raise FileSpaceConnectionError("Problem communicating with file manager: "+str(ex)) if resp.status_code == 401: # Expired token or authentication failure raise AuthenticationFailure("File manager credentials not accepted") @@ -276,6 +281,56 @@ def get_uploads_directory(self, record_name): except etree.XMLSyntaxError as ex: raise FileSpaceServerError("Server returned unparseable XML") + def _webdav_auth(self, authurl: str, authtype: str): + if not authurl: + return self.auth_pass + + if not authtype: + authtype = "userpass" + + if authtype == 'private_net': + return self._webdav_auth_by_private_net(authurl) + if authtype == 'userpass': + return self.auth_pass + + raise FileSpaceException(f"Unknown WebDAV authentication type: {authtype}") + + def _webdav_auth_by_private_net(self, authurl: str): + header = {"X-Client-Verify": "SUCCESS", "X-Client-CN": self.auth_user, + "X-Client-DN": f"CN={self.auth_user}"} + try: + resp = requests.post(authurl, headers=header) + except requests.RequestException as ex: + raise FileSpaceConnectionError("Problem communicating with file manager authenticater: "+str(ex)) + + if resp.status_code == 401: # authentication failure + raise AuthenticationFailure("File manager WebDAV credentials not accepted") + elif resp.status_code == 404: # Not Found + raise FileSpaceException("WebDAV authenticater URL apparently incorrect") + elif resp.status_code >= 500: + raise FileSpaceException("File manager server error: {resp.reason}") + elif resp.status_code >= 400: + msg = resp.reason + if '/json' in resp.headers.get("content-header"): + body = response.json() + if isinstance(body, Mapping) and 'message' in body: + msg = body['message'] + elif '/xml' in resp.headers.get("content-header"): + try: + body = etree.parse(resp.text).getroot() + msgel = body.find(".//{DAV:}message") + if msgel: + msg = msgel.text + except Exception as ex: + msg += " (no parseable message in response body)" + raise FileSpaceException(msg, resp.status_code) + + resp = resp.json() + try: + return resp['temporary_password'] + except KeyError: + raise FileSpaceServerException("Unexpected output from WebDAV authenticator: "+resp.text) + def determine_uploads_url(self, record_name): """ diff --git a/python/tests/nistoar/midas/dap/fm/test_apiclient.py b/python/tests/nistoar/midas/dap/fm/test_apiclient.py index 575322e..5ec9246 100644 --- a/python/tests/nistoar/midas/dap/fm/test_apiclient.py +++ b/python/tests/nistoar/midas/dap/fm/test_apiclient.py @@ -23,11 +23,16 @@ def setUp(self): self.mock_response_200 = Mock() self.mock_response_200.status_code = 200 + # with mock auth + self.file_manager = self.make_fm() + + def make_fm(self): # Mock the authenticate method to prevent the FileManager constructor # from making a real HTTP request upon object instantiation with patch.object(FileManager, 'authenticate', return_value='mock_token'): - self.file_manager = FileManager(self.config) - self.file_manager.token = "token" + file_manager = FileManager(self.config) + file_manager.token = "token" + return file_manager @patch('requests.post') def test_authenticate_success(self, mock_post): @@ -200,6 +205,39 @@ def test_get_uploads_directory(self, mock_request): self.assertEqual(props.get('permissions'), "RGDNVCK") self.assertIn("created", props) self.assertIn("modified", props) + + @patch('requests.post') + def test_webdav_auth_by_private_net(self, mock_request): + mock_response = Mock() + mock_response.status_code = 200 + mock_response.text = '{"temporary_password": "dontguessmeplease"}' + def tojson(): + return json.loads(mock_response.text) + mock_response.json = tojson + mock_request.return_value = mock_response + + pw = self.file_manager._webdav_auth_by_private_net("https://whoknows/auth") + self.assertEqual(pw, "dontguessmeplease") + + @patch('requests.post') + def test_webdav_auth(self, mock_request): + mock_response = Mock() + mock_response.status_code = 200 + mock_response.text = '{"temporary_password": "dontguessmeplease"}' + def tojson(): + return json.loads(mock_response.text) + mock_response.json = tojson + mock_request.return_value = mock_response + + self.assertEqual(self.file_manager.auth_user, self.config['auth']['username']) + self.assertEqual(self.file_manager.auth_pass, self.config['auth']['password']) + self.assertNotEqual(self.config['auth']['password'], "dontguessmeplease") + wdaurl = "https://whoknows/auth" + self.assertEqual(self.file_manager._webdav_auth(None, None), self.config['auth']['password']) + self.assertEqual(self.file_manager._webdav_auth(wdaurl, "userpass"), self.config['auth']['password']) + self.assertEqual(self.file_manager._webdav_auth(wdaurl, None), self.config['auth']['password']) + self.assertEqual(self.file_manager._webdav_auth(wdaurl, ""), self.config['auth']['password']) + self.assertEqual(self.file_manager._webdav_auth(wdaurl, "private_net"), "dontguessmeplease") @patch('requests.request') def test_determine_uploads_url(self, mock_request): @@ -215,7 +253,25 @@ def test_determine_uploads_url(self, mock_request): self.assertEqual(self.file_manager.determine_uploads_url("mds3-0012"), "http://goober.net/nc/192?dir=/mds3-0012/mds3-0012") - + + @patch('requests.request') + def test_get_uploads_directory_with_wdauth(self, mock_request): + mock_response = Mock() + mock_response.status_code = 200 + with open(pfrespf) as fd: + mock_response.text = fd.read() + mock_request.return_value = mock_response + + self.config['auth']['webdav_auth_url'] = "https://whoknows/auth" + self.file_manager = self.make_fm() + props = self.file_manager.get_uploads_directory("mds3-0012") + self.assertEqual(props.get('type'), "folder") + self.assertEqual(props.get('fileid'), "192") + self.assertEqual(props.get('size'), "4997166") + self.assertEqual(props.get('permissions'), "RGDNVCK") + self.assertIn("created", props) + self.assertIn("modified", props) + if __name__ == '__main__': From 77f1565556cfe240bcdd2cdd282deb29c9688c0f Mon Sep 17 00:00:00 2001 From: RayPlante Date: Fri, 1 Nov 2024 11:22:52 -0400 Subject: [PATCH 3/4] dap: fix fm error handling bugs --- python/nistoar/midas/dap/fm/apiclient.py | 8 +++++--- python/nistoar/midas/dap/nerdstore/fmfs.py | 14 +++++++------- python/nistoar/midas/dap/service/mds3.py | 4 ++-- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/python/nistoar/midas/dap/fm/apiclient.py b/python/nistoar/midas/dap/fm/apiclient.py index c0b4bc6..fb033d9 100644 --- a/python/nistoar/midas/dap/fm/apiclient.py +++ b/python/nistoar/midas/dap/fm/apiclient.py @@ -256,11 +256,11 @@ def get_uploads_directory(self, record_name): raise FileSpaceException("File manager server error: {resp.reason}") elif resp.status_code >= 400: msg = resp.reason - if '/json' in resp.headers.get("content-header"): + if '/json' in resp.headers.get("content-type"): body = response.json() if isinstance(body, Mapping) and 'message' in body: - msg = body['message'] - elif '/xml' in resp.headers.get("content-header"): + msg = str(body['message']) + elif '/xml' in resp.headers.get("content-type"): try: body = etree.parse(resp.text).getroot() msgel = body.find(".//{DAV:}message") @@ -268,6 +268,8 @@ def get_uploads_directory(self, record_name): msg = msgel.text except Exception as ex: msg += " (no parseable message in response body)" + else: + msg += " (detail not specified)" raise FileSpaceException(msg, resp.status_code) base = self.dav_base diff --git a/python/nistoar/midas/dap/nerdstore/fmfs.py b/python/nistoar/midas/dap/nerdstore/fmfs.py index e785a88..1a720a4 100644 --- a/python/nistoar/midas/dap/nerdstore/fmfs.py +++ b/python/nistoar/midas/dap/nerdstore/fmfs.py @@ -199,7 +199,7 @@ def _scan_files(self): if not isinstance(resp, Mapping): self._res.log.error("Unexpected response from scan request: "+ "not a JSON object (is URL correct?)") - raise RemoteStorageException("%s: failed to trigger file scan") + raise RemoteStorageException(f"{self._res.id}: failed to trigger file scan") elif 'scan_id' not in resp: self._res.log.error("Unexpected response from scan request; no scan_id included") @@ -218,11 +218,11 @@ def _scan_files(self): self._res.log.error("Unexected response from scan request: failed to parse as JSON (%s)", str(ex)) self._res.log.warning("(Is the fm base URL correct?)") - raise RemoteStorageException("%s: Failed to trigger file scan (unexpected response format)") \ - from ex + raise RemoteStorageException("%s: Failed to trigger file scan (unexpected response format)" % + self._res.id) from ex except Exception as ex: - raise RemoteStorageException("%s: failed to trigger file scan: %s", self._res.id, str(ex)) \ + raise RemoteStorageException(f"{self._res.id}: failed to trigger file scan: {str(ex)}") \ from ex return self._get_file_scan() @@ -241,11 +241,11 @@ def _get_file_scan(self): self._res.log.error("Unexected response while retrieving scan: failed to parse as JSON (%s)", str(ex)) self._res.log.warning("(Is the fm base URL correct?)") - raise RemoteStorageException("%s: Failed to trigger file scan (unexpected response format)") \ - from ex + raise RemoteStorageException("%s: Failed to trigger file scan (unexpected response format)" % + self._res.id) from ex except Exception as ex: - raise RemoteStorageException("%s: failed to trigger file scan: %s", self._res.id, str(ex)) \ + raise RemoteStorageException(f"{self._res.id}: failed to trigger file scan: {str(ex)}") \ from ex if 'contents' not in resp or not isinstance(resp['contents'], list): diff --git a/python/nistoar/midas/dap/service/mds3.py b/python/nistoar/midas/dap/service/mds3.py index bbeb811..eec52f9 100644 --- a/python/nistoar/midas/dap/service/mds3.py +++ b/python/nistoar/midas/dap/service/mds3.py @@ -406,8 +406,8 @@ def create_record(self, name, data=None, meta=None) -> ProjectRecord: if prec.file_space.get('file_count', -2) < 0: self.log.warning("Failed to initialize file listing from file manager") except Exception as ex: - self.log.error("Failed to initialize file listing: problem accessing file manager: %s", - str(ex)) + self.log.exception("Failed to initialize file listing: problem accessing file manager: %s", + str(ex)) prec.data = self._summarize(nerd) if data: From 3d10addd8c5bc14171625734cbd9381c7597ff63 Mon Sep 17 00:00:00 2001 From: RayPlante Date: Fri, 8 Nov 2024 15:32:27 -0500 Subject: [PATCH 4/4] .github/workflows: update artifact uploading version as recommended by github --- .github/workflows/integration.yml | 4 ++-- .github/workflows/main.yml | 4 ++-- .github/workflows/python-source.yml | 2 +- .github/workflows/testall.yml | 8 +++++++- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index e875bd3..2f0553b 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -8,7 +8,7 @@ jobs: runs-on: ubuntu-20.04 steps: - name: Checkout repository - uses: actions/checkout@v2 + uses: actions/checkout@v4 with: submodules: true @@ -26,7 +26,7 @@ jobs: run: scripts/testall.docker python - name: Upload Artifacts - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v4 if: failure() with: name: test-artifacts diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index b8a3798..2710ddd 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -8,7 +8,7 @@ jobs: runs-on: ubuntu-20.04 steps: - name: Checkout repository - uses: actions/checkout@v2 + uses: actions/checkout@v4 with: submodules: true @@ -27,7 +27,7 @@ jobs: run: scripts/testall.docker python - name: Upload Artifacts - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v4 if: failure() with: name: test-artifacts diff --git a/.github/workflows/python-source.yml b/.github/workflows/python-source.yml index 219e05a..7c9becb 100644 --- a/.github/workflows/python-source.yml +++ b/.github/workflows/python-source.yml @@ -12,7 +12,7 @@ jobs: runs-on: ubuntu-20.04 steps: - name: Checkout repository - uses: actions/checkout@v2 + uses: actions/checkout@v4 with: submodules: true diff --git a/.github/workflows/testall.yml b/.github/workflows/testall.yml index 9c1bb29..c1ea8b3 100644 --- a/.github/workflows/testall.yml +++ b/.github/workflows/testall.yml @@ -12,7 +12,7 @@ jobs: runs-on: ubuntu-20.04 steps: - name: Checkout repository - uses: actions/checkout@v2 + uses: actions/checkout@v4 with: submodules: true @@ -29,3 +29,9 @@ jobs: - name: Build & Run Python Tests via Docker run: scripts/testall.docker python + - name: Upload Artifacts + uses: actions/upload-artifact@v4 + if: failure() + with: + name: test-artifacts + path: python/build/test-artifacts/