diff --git a/lib/pbench/server/api/resources/__init__.py b/lib/pbench/server/api/resources/__init__.py index a8b2e757ee..837199c0d0 100644 --- a/lib/pbench/server/api/resources/__init__.py +++ b/lib/pbench/server/api/resources/__init__.py @@ -4,7 +4,7 @@ import json from json.decoder import JSONDecodeError from logging import Logger -from typing import Any, Callable, List, Union +from typing import Any, Callable, List, Optional, Union from dateutil import parser as date_parser from flask import request @@ -1021,8 +1021,12 @@ def _get_dataset_metadata( elif i == Dataset.UPLOADED: metadata[i] = UtcTimeHelper(dataset.uploaded).to_iso_string() elif Metadata.is_key_path(i, Metadata.USER_METADATA): + native_key = Metadata.get_native_key(i) + user: Optional[User] = None + if native_key == Metadata.USER_NATIVE_KEY: + user = Auth.token_auth.current_user() try: - metadata[i] = Metadata.getvalue(dataset=dataset, key=i) + metadata[i] = Metadata.getvalue(dataset=dataset, key=i, user=user) except MetadataNotFound: metadata[i] = None else: diff --git a/lib/pbench/server/api/resources/datasets_metadata.py b/lib/pbench/server/api/resources/datasets_metadata.py index b510d132c1..dfd2ba4f3f 100644 --- a/lib/pbench/server/api/resources/datasets_metadata.py +++ b/lib/pbench/server/api/resources/datasets_metadata.py @@ -1,10 +1,12 @@ from http import HTTPStatus from logging import Logger +from typing import Optional from flask.json import jsonify from flask.wrappers import Request, Response from pbench.server import JSON, JSONOBJECT, PbenchServerConfig +from pbench.server.api.auth import Auth from pbench.server.api.resources import ( APIAbort, API_OPERATION, @@ -19,6 +21,7 @@ Metadata, MetadataError, ) +from pbench.server.database.models.users import User class DatasetsMetadata(ApiBase): @@ -94,10 +97,7 @@ def _put(self, json_data: JSONOBJECT, _) -> Response: "name": "datasetname", "metadata": [ "dashboard.seen": True, - "user": { - "cloud": "AWS", - "contact": "john.carter@mars.org" - } + "user": {"favorite": True} ] } @@ -111,7 +111,7 @@ def _put(self, json_data: JSONOBJECT, _) -> Response: [ "dashboard.seen": True, - "user": {"cloud": "AWS", "contact": "json.carter@mars.org} + "user": {"favorite": False} ] """ name = json_data["dataset"] @@ -125,16 +125,44 @@ def _put(self, json_data: JSONOBJECT, _) -> Response: # Validate the authenticated user's authorization for the combination # of "owner" and "access". - self._check_authorization(str(dataset.owner_id), dataset.access) + # + # The "unusual" part here is that we make a special case for + # authenticated that are not the owner of the data: we want to allow + # them UPDATE access to PUBLIC datasets (to which they naturally have + # READ access) as long as they're only trying to modify a "user." + # metadata key: + # + # * We want to validate authorized non-owners for READ access if + # they're only trying to modify "user." keys; + # * We want to validate unauthorized users for UPDATE because they have + # READ access to "public" datasets but still can't write even "user." + # metadata; + # * We want to validate authorized users for UPDATE if they're trying + # to set anything other than a "user." key because only the owner can + # do that... + role = API_OPERATION.READ + if not Auth.token_auth.current_user(): + role = API_OPERATION.UPDATE + else: + for k in metadata.keys(): + if Metadata.get_native_key(k) != Metadata.USER_NATIVE_KEY: + role = API_OPERATION.UPDATE + self._check_authorization( + str(dataset.owner_id), dataset.access, check_role=role + ) failures = [] for k, v in metadata.items(): + native_key = Metadata.get_native_key(k) + user: Optional[User] = None + if native_key == Metadata.USER_NATIVE_KEY: + user = Auth.token_auth.current_user() try: - Metadata.setvalue(dataset, k, v) + Metadata.setvalue(key=k, value=v, dataset=dataset, user=user) except MetadataError as e: self.logger.warning("Unable to update key {} = {!r}: {}", k, v, str(e)) failures.append(k) if failures: raise APIAbort(HTTPStatus.INTERNAL_SERVER_ERROR) - results = self._get_metadata(name, list(metadata.keys())) + results = self._get_dataset_metadata(dataset, list(metadata.keys())) return jsonify(results) diff --git a/lib/pbench/server/database/models/datasets.py b/lib/pbench/server/database/models/datasets.py index ff62b00e95..a48e41ffbb 100644 --- a/lib/pbench/server/database/models/datasets.py +++ b/lib/pbench/server/database/models/datasets.py @@ -4,11 +4,11 @@ import os from pathlib import Path import re -from typing import Any, List, Tuple, Union +from typing import Any, List, Optional, Tuple, Union from sqlalchemy import Column, DateTime, Enum, event, ForeignKey, Integer, JSON, String from sqlalchemy.exc import IntegrityError, SQLAlchemyError -from sqlalchemy.orm import relationship, validates +from sqlalchemy.orm import relationship, validates, Query from sqlalchemy.types import TypeDecorator from pbench.server.database.database import Database @@ -821,57 +821,44 @@ class Metadata(Database.Base): # the "server" namespace, and are strictly controlled by keyword path: # e.g., "server.deleted", "server.archived"; # - # Metadata keys intended for use by the dashboard client are in the - # "dashboard" namespace. While these can be modified by any API client, the - # separate namespace provides some protection against accidental - # modifications that might break dashboard semantics. This is an "open" - # namespace, allowing the dashboard to define and manage any keys it needs - # within the "dashboard.*" hierarchy. - # - # Metadata keys within the "user" namespace are reserved for general client - # use, although by convention a second-level key-space should be used to - # provide some isolation. This is an "open" namespace, allowing any API - # client to define new keywords such as "user.contact" or "user.me.you" - # and JSON subdocuments are available at any level. For example, if we've - # set "user.contact.email" and "user.postman.test" then retrieving "user" - # will return a JSON document like - # {"contact": {"email": "value"}, "postman": {"test": "value"}} - # and retrieving "user.contact" would return - # {"email": "value"} + # The "dashboard" and "user" namespaces can be written by an authenticated + # client to track external metadata. The difference is that "dashboard" key + # values are visible to all clients with READ access to the dataset, while + # the "user" namespace is visible only to clients authenticated to the user + # that wrote the data. That is, "dashboard.seen" is a global dashboard + # metadata property visible to all users, while "user.favorite" is visible + # only to the specific user that wrote the value; each authenticated user + # may have its own unique "user.favorite" value. # # The following class constants define the set of currently available # metadata keys, where the "open" namespaces are represented by the - # syntax "user.*". - - # DELETION timestamp for dataset based on user settings and system - # settings at time the dataset is created. - # - # {"server.deletion": "2021-12-25"} - DELETION = "server.deletion" - - # DASHBOARD is arbitrary data saved on behalf of the dashboard client; the - # reserved namespace differs from "user" only in that reserving a primary - # key for the "well known" dashboard client offers some protection against - # key name collisions. + # syntax "dashboard.*" and "user.*" which allow clients to control the + # key names using a "dotted path" notation like "dashboard.seen" or + # "dashboard.contact.email". + + # DASHBOARD is arbitrary data saved on behalf of the dashboard client as a + # JSON document. Writing these keys requires ownership of the referenced + # dataset, and the data is visible to all clients with READ access to the + # dataset. # # {"dashboard.seen": True} DASHBOARD = "dashboard.*" - # USER is arbitrary data saved on behalf of the owning user, as a JSON - # document. - # - # Note that the hierarchical key management in the getvalue and setvalue - # static methods (used consistently by the API layer) allow client code - # to interact with these either as a complete JSON document ("user") or - # as a full dotted key path ("user.contact.name.first") or at any JSON - # layer between. + # USER is arbitrary data saved with the dataset on behalf of an + # authenticated user, as a JSON document. Writing these keys requires READ + # access to the referenced dataset, and are visible only to clients that + # are authenticated as the user which set them. Each user can have its own + # unique value for these keys, for example "user.favorite". # - # API keyword validation uses the trailing ".*" here to indicate that only - # the first element of the path should be validated, allowing the client - # a completely uninterpreted namespace below that. + # {"user.favorite": True} + USER_NATIVE_KEY = "user" + USER = USER_NATIVE_KEY + ".*" + + # DELETION timestamp for dataset based on user settings and system + # settings when the dataset is created. # - # {"user.cloud": "AWS", "user.mood": "CLOUDY"}} - USER = "user.*" + # {"server.deletion": "2021-12-25"} + DELETION = "server.deletion" # REINDEX boolean flag to indicate when a dataset should be re-indexed # @@ -928,8 +915,10 @@ class Metadata(Database.Base): key = Column(String(255), unique=False, nullable=False, index=True) value = Column(JSON, unique=False, nullable=True) dataset_ref = Column(Integer, ForeignKey("datasets.id"), nullable=False) + user_ref = Column(Integer, ForeignKey("users.id"), nullable=True) dataset = relationship("Dataset", back_populates="metadatas", single_parent=True) + user = relationship("User", back_populates="dataset_metadata", single_parent=True) @validates("key") def validate_key(self, _, value: Any) -> str: @@ -980,6 +969,19 @@ def create(**kwargs) -> "Metadata": else: return meta + @staticmethod + def get_native_key(key: str) -> str: + """ + Extract the root key name + + Args: + key: Key path (e.g., "user.tag") + + Returns: + native SQL key name ("user") + """ + return key.lower().split(".")[0] + @staticmethod def is_key_path(key: str, valid: List[str]) -> bool: """ @@ -988,7 +990,7 @@ def is_key_path(key: str, valid: List[str]) -> bool: valid. If the key is a dotted path and the first element plus a trailing ".*" is in the list, then this is an open key namespace where any subsequent path is acceptable: e.g., "user.*" allows "user", or - "user.contact", "user.contact.name", etc. + "user.favorite", "user.notes.status", etc. Args: key: metadata key path @@ -1012,7 +1014,7 @@ def is_key_path(key: str, valid: List[str]) -> bool: return bool(re.fullmatch(Metadata._valid_key_charset, k)) @staticmethod - def getvalue(dataset: Dataset, key: str) -> JSON: + def getvalue(dataset: Dataset, key: str, user: Optional[User] = None) -> JSON: """ Returns the value of the specified key, which may be a dotted hierarchical path (e.g., "server.deleted"). @@ -1021,15 +1023,28 @@ def getvalue(dataset: Dataset, key: str) -> JSON: level Metadata object. The full JSON value of a top level key can be acquired directly using `Metadata.get(dataset, key)` - E.g., for "user.contact.name" with the dataset's Metadata value for the - "user" key as {"contact": {"name": "Dave", "email": "d@example.com"}}, - this would return "Dave", whereas Metadata.get(dataset, "user") would - return the entire user key JSON, such as - {"user" {"contact": {"name": "Dave", "email": "d@example.com}}} + For example, if the metadata database has + + "dashboard": { + "contact": { + "name": "dave", + "email": "d@example.com" + } + } + + then Metadata.get(dataset, "dashboard.contact.name") would return + + "Dave" + + whereas Metadata.get(dataset, "dashboard") would return the entire user + key JSON, such as + + {"dashboard" {"contact": {"name": "Dave", "email": "d@example.com}}} Args: dataset: associated dataset key: hierarchical key path to fetch + user: User-specific key value (used only for "user." namespace) Returns: Value of the key path @@ -1039,7 +1054,7 @@ def getvalue(dataset: Dataset, key: str) -> JSON: keys = key.lower().split(".") native_key = keys.pop(0) try: - meta = Metadata.get(dataset, native_key) + meta = Metadata.get(dataset, native_key, user) except MetadataNotFound: return None value = meta.value @@ -1057,7 +1072,9 @@ def getvalue(dataset: Dataset, key: str) -> JSON: return value @staticmethod - def setvalue(dataset: Dataset, key: str, value: Any) -> "Metadata": + def setvalue( + dataset: Dataset, key: str, value: Any, user: Optional[User] = None + ) -> "Metadata": """ Create or modify an existing metadata value. This method supports hierarchical dotted paths like "dashboard.seen" and should be used in @@ -1083,7 +1100,7 @@ def setvalue(dataset: Dataset, key: str, value: Any) -> "Metadata": native_key = keys.pop(0) found = True try: - meta = Metadata.get(dataset, native_key) + meta = Metadata.get(dataset, native_key, user) # SQLAlchemy determines whether to perform an `update` based on the # Python object reference. We make a copy here to ensure that it @@ -1120,11 +1137,19 @@ def setvalue(dataset: Dataset, key: str, value: Any) -> "Metadata": meta.value = meta_value meta.update() else: - meta = Metadata.create(dataset=dataset, key=native_key, value=meta_value) + meta = Metadata.create( + dataset=dataset, key=native_key, value=meta_value, user=user + ) return meta @staticmethod - def get(dataset: Dataset, key: str) -> "Metadata": + def _query(dataset: Dataset, key: str, user: Optional[User]) -> Query: + return Database.db_session.query(Metadata).filter_by( + dataset=dataset, key=key, user=user + ) + + @staticmethod + def get(dataset: Dataset, key: str, user: Optional[User] = None) -> "Metadata": """ Fetch a Metadata (row) from the database by key name. @@ -1140,11 +1165,7 @@ def get(dataset: Dataset, key: str) -> "Metadata": The Metadata model object """ try: - meta = ( - Database.db_session.query(Metadata) - .filter_by(dataset=dataset, key=key) - .first() - ) + meta = __class__._query(dataset, key, user).first() except SQLAlchemyError as e: Metadata.logger.exception("Can't get {}>>{} from DB", dataset, key) raise MetadataSqlError("getting", dataset, key) from e @@ -1154,7 +1175,7 @@ def get(dataset: Dataset, key: str) -> "Metadata": return meta @staticmethod - def remove(dataset: Dataset, key: str): + def remove(dataset: Dataset, key: str, user: Optional[User] = None): """ remove Remove a metadata key from the dataset @@ -1166,9 +1187,7 @@ def remove(dataset: Dataset, key: str): DatasetSqlError: Something went wrong """ try: - Database.db_session.query(Metadata).filter_by( - dataset=dataset, key=key - ).delete() + __class__._query(dataset, key, user).delete() Database.db_session.commit() except SQLAlchemyError as e: Metadata.logger.exception("Can't remove {}>>{} from DB", dataset, key) @@ -1185,7 +1204,7 @@ def add(self, dataset: Dataset): raise DatasetBadParameterType(dataset, Dataset) try: - Metadata.get(dataset, self.key) + Metadata.get(dataset, self.key, self.user) except MetadataNotFound: pass else: diff --git a/lib/pbench/server/database/models/users.py b/lib/pbench/server/database/models/users.py index b42de389d2..118e60f40a 100644 --- a/lib/pbench/server/database/models/users.py +++ b/lib/pbench/server/database/models/users.py @@ -29,6 +29,12 @@ class User(Database.Base): role = Column(Enum(Roles), unique=False, nullable=True) auth_tokens = relationship("ActiveTokens", backref="users") + # NOTE: this relationship defines a `user` property in `Metadata` + # that refers to the parent `User` object. + dataset_metadata = relationship( + "Metadata", back_populates="user", cascade="all, delete-orphan" + ) + def __str__(self): return f"User, id: {self.id}, username: {self.username}" diff --git a/lib/pbench/test/unit/server/conftest.py b/lib/pbench/test/unit/server/conftest.py index c17f03ffe8..1e212914d1 100644 --- a/lib/pbench/test/unit/server/conftest.py +++ b/lib/pbench/test/unit/server/conftest.py @@ -386,7 +386,7 @@ def provide_metadata(attach_dataset): """ drb = Dataset.query(name="drb") test = Dataset.query(name="test") - Metadata.setvalue(dataset=drb, key="user.contact", value="me@example.com") + Metadata.setvalue(dataset=drb, key="dashboard.contact", value="me@example.com") Metadata.setvalue(dataset=drb, key=Metadata.DELETION, value="2022-12-25") Metadata.setvalue( dataset=drb, @@ -397,7 +397,7 @@ def provide_metadata(attach_dataset): "unit-test.v6.run-toc.2020-05": ["random_md5_string1"], }, ) - Metadata.setvalue(dataset=test, key="user.contact", value="you@example.com") + Metadata.setvalue(dataset=test, key="dashboard.contact", value="you@example.com") Metadata.setvalue(dataset=test, key=Metadata.DELETION, value="2023-01-25") diff --git a/lib/pbench/test/unit/server/database/test_datasets_metadata_db.py b/lib/pbench/test/unit/server/database/test_datasets_metadata_db.py index 9c03b5cc08..aa259c7a9d 100644 --- a/lib/pbench/test/unit/server/database/test_datasets_metadata_db.py +++ b/lib/pbench/test/unit/server/database/test_datasets_metadata_db.py @@ -1,4 +1,6 @@ import pytest +from sqlalchemy import or_ + from pbench.server.database.database import Database from pbench.server.database.models.datasets import ( Dataset, @@ -19,19 +21,19 @@ def test_metadata(self, db_session, create_user): # See if we can create a metadata row ds = Dataset.create(owner=create_user.username, controller="frodo", name="fio") assert ds.metadatas == [] - m = Metadata.create(key="user", value=True, dataset=ds) + m = Metadata.create(key="dashboard", value=True, dataset=ds) assert m is not None assert ds.metadatas == [m] # Try to get it back - m1 = Metadata.get(ds, "user") + m1 = Metadata.get(ds, "dashboard") assert m1.key == m.key assert m1.value == m.value assert m.id == m1.id assert m.dataset_ref == m1.dataset_ref # Check the str() - assert "test(1)|frodo|fio>>user" == str(m) + assert "test(1)|frodo|fio>>dashboard" == str(m) # Try to get a metadata key that doesn't exist with pytest.raises(MetadataNotFound) as exc: @@ -50,33 +52,33 @@ def test_metadata(self, db_session, create_user): # Try to create a key without a value with pytest.raises(MetadataMissingKeyValue): - Metadata(key="user") + Metadata(key="dashboard") # Try to add a duplicate metadata key with pytest.raises(MetadataDuplicateKey) as exc: - m1 = Metadata(key="user", value="IRRELEVANT") + m1 = Metadata(key="dashboard", value="IRRELEVANT") m1.add(ds) - assert exc.value.key == "user" + assert exc.value.key == "dashboard" assert exc.value.dataset == ds assert ds.metadatas == [m] # Try to add a Metadata key to something that's not a dataset with pytest.raises(DatasetBadParameterType) as exc: - m1 = Metadata(key="user", value="DONTCARE") + m1 = Metadata(key="dashboard", value="DONTCARE") m1.add("foobar") assert exc.value.bad_value == "foobar" assert exc.value.expected_type == Dataset.__name__ # Try to create a Metadata with a bad value for the dataset with pytest.raises(DatasetBadParameterType) as exc: - m1 = Metadata.create(key="user", value="TRUE", dataset=[ds]) + m1 = Metadata.create(key="dashboard", value="TRUE", dataset=[ds]) assert exc.value.bad_value == [ds] assert exc.value.expected_type == Dataset.__name__ # Try to update the metadata key m.value = "False" m.update() - m1 = Metadata.get(ds, "user") + m1 = Metadata.get(ds, "dashboard") assert m.id == m1.id assert m.dataset_ref == m1.dataset_ref assert m.key == m1.key @@ -85,27 +87,27 @@ def test_metadata(self, db_session, create_user): # Delete the key and make sure its gone m.delete() with pytest.raises(MetadataNotFound) as exc: - Metadata.get(ds, "user") + Metadata.get(ds, "dashboard") assert exc.value.dataset == ds - assert exc.value.key == "user" + assert exc.value.key == "dashboard" assert ds.metadatas == [] def test_metadata_remove(self, db_session, create_user): """Test that we can remove a Metadata key""" ds = Dataset.create(owner=create_user.username, controller="frodo", name="fio") assert ds.metadatas == [] - m = Metadata(key="user", value="TRUE") + m = Metadata(key="dashboard", value="TRUE") m.add(ds) assert ds.metadatas == [m] - Metadata.remove(ds, "user") + Metadata.remove(ds, "dashboard") assert ds.metadatas == [] with pytest.raises(MetadataNotFound) as exc: - Metadata.get(ds, "user") + Metadata.get(ds, "dashboard") assert exc.value.dataset == ds - assert exc.value.key == "user" + assert exc.value.key == "dashboard" - Metadata.remove(ds, "user") + Metadata.remove(ds, "dashboard") assert ds.metadatas == [] @@ -113,63 +115,121 @@ class TestMetadataNamespace: def test_get_bad_syntax(self, db_session, create_user): ds = Dataset.create(owner=create_user.username, controller="frodo", name="fio") with pytest.raises(MetadataBadKey) as exc: - Metadata.getvalue(ds, "user..foo") + Metadata.getvalue(ds, "dashboard..foo") assert exc.type == MetadataBadKey - assert exc.value.key == "user..foo" - assert str(exc.value) == "Metadata key 'user..foo' is not supported" + assert exc.value.key == "dashboard..foo" + assert str(exc.value) == "Metadata key 'dashboard..foo' is not supported" + + def test_user_metadata(self, db_session, create_user, create_drb_user): + """Various tests on user-mapped Metadata keys""" + # See if we can create a metadata row + ds = Dataset.create(owner=create_user.username, controller="frodo", name="fio") + assert ds.metadatas == [] + t = Metadata.create(key="user", value=True, dataset=ds, user=create_user) + assert t is not None + assert ds.metadatas == [t] + assert create_user.dataset_metadata == [t] + + d = Metadata.create(key="user", value=False, dataset=ds, user=create_drb_user) + assert d is not None + assert ds.metadatas == [t, d] + assert create_user.dataset_metadata == [t] + assert create_drb_user.dataset_metadata == [d] + + g = Metadata.create(key="user", value="text", dataset=ds) + assert g is not None + assert ds.metadatas == [t, d, g] + assert create_user.dataset_metadata == [t] + assert create_drb_user.dataset_metadata == [d] + + assert Metadata.get(key="user", dataset=ds).value == "text" + assert Metadata.get(key="user", dataset=ds, user=create_user).value is True + assert Metadata.get(key="user", dataset=ds, user=create_drb_user).value is False + + Metadata.remove(key="user", dataset=ds, user=create_drb_user) + assert create_drb_user.dataset_metadata == [] + assert create_user.dataset_metadata == [t] + assert ds.metadatas == [t, g] + + Metadata.remove(key="user", dataset=ds) + assert create_user.dataset_metadata == [t] + assert ds.metadatas == [t] + + Metadata.remove(key="user", dataset=ds, user=create_user) + assert create_user.dataset_metadata == [] + assert ds.metadatas == [] + + # Peek under the carpet to look for orphaned metadata objects linked + # to the Dataset or User + metadata = ( + Database.db_session.query(Metadata).filter_by(dataset_ref=ds.id).first() + ) + assert metadata is None + metadata = ( + Database.db_session.query(Metadata) + .filter( + or_( + Metadata.user_ref == create_drb_user.id, + Metadata.user_ref == create_user.id, + Metadata.user_ref is None, + ) + ) + .first() + ) + assert metadata is None def test_set_bad_syntax(self, db_session, create_user): ds = Dataset.create(owner=create_user.username, controller="frodo", name="fio") with pytest.raises(MetadataBadKey) as exc: - Metadata.setvalue(ds, "user.foo.", "irrelevant") + Metadata.setvalue(ds, "dashboard.foo.", "irrelevant") assert exc.type == MetadataBadKey - assert exc.value.key == "user.foo." - assert str(exc.value) == "Metadata key 'user.foo.' is not supported" + assert exc.value.key == "dashboard.foo." + assert str(exc.value) == "Metadata key 'dashboard.foo.' is not supported" def test_set_bad_characters(self, db_session, create_user): ds = Dataset.create(owner=create_user.username, controller="frodo", name="fio") with pytest.raises(MetadataBadKey) as exc: - Metadata.setvalue(ds, "user.*!foo", "irrelevant") + Metadata.setvalue(ds, "dashboard.*!foo", "irrelevant") assert exc.type == MetadataBadKey - assert exc.value.key == "user.*!foo" - assert str(exc.value) == "Metadata key 'user.*!foo' is not supported" + assert exc.value.key == "dashboard.*!foo" + assert str(exc.value) == "Metadata key 'dashboard.*!foo' is not supported" def test_get_novalue(self, db_session, create_user): ds = Dataset.create(owner=create_user.username, controller="frodo", name="fio") - assert Metadata.getvalue(ds, "user.email") is None - assert Metadata.getvalue(ds, "user") is None + assert Metadata.getvalue(ds, "dashboard.email") is None + assert Metadata.getvalue(ds, "dashboard") is None def test_get_bad_path(self, db_session, create_user): ds = Dataset.create(owner=create_user.username, controller="frodo", name="fio") - Metadata.setvalue(ds, "user.contact", "hello") + Metadata.setvalue(ds, "dashboard.contact", "hello") with pytest.raises(MetadataBadStructure) as exc: - Metadata.getvalue(ds, "user.contact.email") + Metadata.getvalue(ds, "dashboard.contact.email") assert exc.type == MetadataBadStructure - assert exc.value.key == "user.contact.email" + assert exc.value.key == "dashboard.contact.email" assert exc.value.element == "contact" assert ( str(exc.value) - == "Key 'contact' value for 'user.contact.email' in test(1)|frodo|fio is not a JSON object" + == "Key 'contact' value for 'dashboard.contact.email' in test(1)|frodo|fio is not a JSON object" ) def test_set_bad_path(self, db_session, create_user): ds = Dataset.create(owner=create_user.username, controller="frodo", name="fio") - Metadata.setvalue(ds, "user.contact", "hello") + Metadata.setvalue(ds, "dashboard.contact", "hello") with pytest.raises(MetadataBadStructure) as exc: - Metadata.setvalue(ds, "user.contact.email", "me@example.com") + Metadata.setvalue(ds, "dashboard.contact.email", "me@example.com") assert exc.type == MetadataBadStructure - assert exc.value.key == "user.contact.email" + assert exc.value.key == "dashboard.contact.email" assert exc.value.element == "contact" assert ( str(exc.value) - == "Key 'contact' value for 'user.contact.email' in test(1)|frodo|fio is not a JSON object" + == "Key 'contact' value for 'dashboard.contact.email' in test(1)|frodo|fio is not a JSON object" ) def test_get_outer_path(self, db_session, create_user): ds = Dataset.create(owner=create_user.username, controller="frodo", name="fio") - Metadata.setvalue(ds, "user.value.hello.english", "hello") - Metadata.setvalue(ds, "user.value.hello.espanol", "hola") - assert Metadata.getvalue(ds, "user.value") == { + Metadata.setvalue(ds, "dashboard.value.hello.english", "hello") + Metadata.setvalue(ds, "dashboard.value.hello.espanol", "hola") + assert Metadata.getvalue(ds, "dashboard.value") == { "hello": {"english": "hello", "espanol": "hola"} } @@ -177,12 +237,12 @@ def test_get_inner_path(self, db_session, create_user): ds = Dataset.create(owner=create_user.username, controller="frodo", name="fio") Metadata.setvalue( ds, - "user.contact", + "dashboard.contact", {"email": "me@example.com", "name": {"first": "My", "last": "Name"}}, ) - assert Metadata.getvalue(ds, "user.contact.email") == "me@example.com" - assert Metadata.getvalue(ds, "user.contact.name.first") == "My" - assert Metadata.getvalue(ds, "user.contact.name") == { + assert Metadata.getvalue(ds, "dashboard.contact.email") == "me@example.com" + assert Metadata.getvalue(ds, "dashboard.contact.name.first") == "My" + assert Metadata.getvalue(ds, "dashboard.contact.name") == { "first": "My", "last": "Name", } @@ -191,12 +251,12 @@ def test_delete_with_metadata(self, db_session, create_user): ds = Dataset.create(owner=create_user.username, controller="frodo", name="fio") Metadata.setvalue( ds, - "user.contact", + "dashboard.contact", {"email": "me@example.com", "name": {"first": "My", "last": "Name"}}, ) - assert Metadata.getvalue(ds, "user.contact.email") == "me@example.com" - assert Metadata.getvalue(ds, "user.contact.name.first") == "My" - assert Metadata.getvalue(ds, "user.contact.name") == { + assert Metadata.getvalue(ds, "dashboard.contact.email") == "me@example.com" + assert Metadata.getvalue(ds, "dashboard.contact.name.first") == "My" + assert Metadata.getvalue(ds, "dashboard.contact.name") == { "first": "My", "last": "Name", } @@ -211,3 +271,30 @@ def test_delete_with_metadata(self, db_session, create_user): # to the deleted Dataset metadata = Database.db_session.query(Metadata).filter_by(dataset_ref=id).first() assert metadata is None + + def test_setgetvalue_user(self, db_session, create_user, create_drb_user): + """ + Verify that we can set and read independent values of "user." namespace + keys across two separate users plus a "non-owned" version (user None) + which is not supported by the current higher level access through the + server APIs. (In other words, the "user" SQL column can be None, as we + use this column only for the "user" key value.) + """ + ds = Dataset.create(owner=create_user.username, controller="frodo", name="fio") + Metadata.setvalue(dataset=ds, key="user.contact", value="Barney") + Metadata.setvalue( + dataset=ds, key="user.contact", value="Fred", user=create_user + ) + Metadata.setvalue( + dataset=ds, key="user.contact", value="Wilma", user=create_drb_user + ) + + assert Metadata.getvalue(dataset=ds, user=None, key="user") == { + "contact": "Barney" + } + assert Metadata.getvalue(dataset=ds, user=create_user, key="user") == { + "contact": "Fred" + } + assert Metadata.getvalue(dataset=ds, user=create_drb_user, key="user") == { + "contact": "Wilma" + } diff --git a/lib/pbench/test/unit/server/test_datasets_metadata.py b/lib/pbench/test/unit/server/test_datasets_metadata.py index 9ab4ac8565..c66a9aaefb 100644 --- a/lib/pbench/test/unit/server/test_datasets_metadata.py +++ b/lib/pbench/test/unit/server/test_datasets_metadata.py @@ -23,20 +23,24 @@ def query_get_as(self, client, server_config, more_datasets, provide_metadata): def query_api( dataset: str, payload: JSON, username: str, expected_status: HTTPStatus ) -> requests.Response: - token = self.token(client, server_config, username) + headers = None + if username: + token = self.token(client, server_config, username) + headers = {"authorization": f"bearer {token}"} response = client.get( f"{server_config.rest_uri}/datasets/metadata/{dataset}", - headers={"authorization": f"bearer {token}"}, + headers=headers, query_string=payload, ) assert response.status_code == expected_status # We need to log out to avoid "duplicate auth token" errors on the # "put" test which does a PUT followed by two GETs. - client.post( - f"{server_config.rest_uri}/logout", - headers={"authorization": f"bearer {token}"}, - ) + if username: + client.post( + f"{server_config.rest_uri}/logout", + headers={"authorization": f"bearer {token}"}, + ) return response return query_api @@ -57,20 +61,24 @@ def query_put_as(self, client, server_config, more_datasets, provide_metadata): def query_api( dataset: str, payload: JSON, username: str, expected_status: HTTPStatus ) -> requests.Response: - token = self.token(client, server_config, username) + headers = None + if username: + token = self.token(client, server_config, username) + headers = {"authorization": f"bearer {token}"} response = client.put( f"{server_config.rest_uri}/datasets/metadata/{dataset}", - headers={"authorization": f"bearer {token}"}, + headers=headers, json=payload, ) assert response.status_code == expected_status # We need to log out to avoid "duplicate auth token" errors on the # test case which does a PUT followed by two GETs. - client.post( - f"{server_config.rest_uri}/logout", - headers={"authorization": f"bearer {token}"}, - ) + if username: + client.post( + f"{server_config.rest_uri}/logout", + headers={"authorization": f"bearer {token}"}, + ) return response return query_api @@ -109,12 +117,7 @@ def test_get1(self, query_get_as): response = query_get_as( "drb", { - "metadata": [ - "dashboard.seen", - "server.deletion", - "dataset.access", - "user.contact", - ] + "metadata": ["dashboard.seen", "server.deletion", "dataset.access"], }, "drb", HTTPStatus.OK, @@ -123,13 +126,14 @@ def test_get1(self, query_get_as): "dashboard.seen": None, "server.deletion": "2022-12-25", "dataset.access": "private", - "user.contact": "me@example.com", } def test_get2(self, query_get_as): response = query_get_as( "drb", - {"metadata": "dashboard.seen,server.deletion,dataset.access,user"}, + { + "metadata": "dashboard.seen,server.deletion,dataset.access", + }, "drb", HTTPStatus.OK, ) @@ -137,7 +141,6 @@ def test_get2(self, query_get_as): "dashboard.seen": None, "server.deletion": "2022-12-25", "dataset.access": "private", - "user": {"contact": "me@example.com"}, } def test_get3(self, query_get_as): @@ -147,8 +150,8 @@ def test_get3(self, query_get_as): "metadata": [ "dashboard.seen", "server.deletion,dataset.access", - "user", - ] + "user.favorite", + ], }, "drb", HTTPStatus.OK, @@ -157,10 +160,10 @@ def test_get3(self, query_get_as): "dashboard.seen": None, "server.deletion": "2022-12-25", "dataset.access": "private", - "user": {"contact": "me@example.com"}, + "user.favorite": None, } - def test_get_unauth(self, query_get_as): + def test_get_private_noauth(self, query_get_as): response = query_get_as( "drb", { @@ -178,12 +181,30 @@ def test_get_unauth(self, query_get_as): == "User test is not authorized to READ a resource owned by drb with private access" ) + def test_get_unauth(self, query_get_as): + response = query_get_as( + "drb", + { + "metadata": [ + "dashboard.seen", + "server.deletion,dataset.access", + "user", + ], + }, + None, + HTTPStatus.UNAUTHORIZED, + ) + assert ( + response.json["message"] + == "Unauthenticated client is not authorized to READ a resource owned by drb with private access" + ) + def test_get_bad_query(self, query_get_as): response = query_get_as( "drb", { "controller": "foobar", - "metadata": "dashboard.seen,server.deletion,dataset.access,user", + "metadata": "dashboard.seen,server.deletion,dataset.access", }, "drb", HTTPStatus.BAD_REQUEST, @@ -267,6 +288,18 @@ def test_put_nowrite(self, query_get_as, query_put_as): == "User test is not authorized to UPDATE a resource owned by drb with public access" ) + def test_put_noauth(self, query_get_as, query_put_as): + response = query_put_as( + "fio_1", + {"metadata": {"dashboard.seen": False, "dashboard.saved": True}}, + None, + HTTPStatus.UNAUTHORIZED, + ) + assert ( + response.json["message"] + == "Unauthenticated client is not authorized to UPDATE a resource owned by drb with public access" + ) + def test_put(self, query_get_as, query_put_as): response = query_put_as( "drb", @@ -276,13 +309,10 @@ def test_put(self, query_get_as, query_put_as): ) assert response.json == {"dashboard.saved": True, "dashboard.seen": False} response = query_get_as( - "drb", - {"metadata": "dashboard,dataset.access"}, - "drb", - HTTPStatus.OK, + "drb", {"metadata": "dashboard,dataset.access"}, "drb", HTTPStatus.OK ) assert response.json == { - "dashboard": {"saved": True, "seen": False}, + "dashboard": {"contact": "me@example.com", "saved": True, "seen": False}, "dataset.access": "private", } @@ -298,3 +328,32 @@ def test_put(self, query_get_as, query_put_as): "dashboard.seen": False, "dataset.access": "private", } + + def test_put_user(self, query_get_as, query_put_as): + response = query_put_as( + "fio_1", + {"metadata": {"user.favorite": True, "user.tag": "AWS"}}, + "drb", + HTTPStatus.OK, + ) + assert response.json == {"user.favorite": True, "user.tag": "AWS"} + response = query_put_as( + "fio_1", + {"metadata": {"user.favorite": False, "user.tag": "RHEL"}}, + "test", + HTTPStatus.OK, + ) + assert response.json == {"user.favorite": False, "user.tag": "RHEL"} + response = query_put_as( + "fio_1", + {"metadata": {"user.favorite": False, "user.tag": "BAD"}}, + None, + HTTPStatus.UNAUTHORIZED, + ) + + response = query_get_as("fio_1", {"metadata": "user"}, "drb", HTTPStatus.OK) + assert response.json == {"user": {"favorite": True, "tag": "AWS"}} + response = query_get_as("fio_1", {"metadata": "user"}, "test", HTTPStatus.OK) + assert response.json == {"user": {"favorite": False, "tag": "RHEL"}} + response = query_get_as("fio_1", {"metadata": "user"}, None, HTTPStatus.OK) + assert response.json == {"user": None}