Skip to content

Commit

Permalink
Accept annotation.metadata as plain text, not a JWE
Browse files Browse the repository at this point in the history
  • Loading branch information
marcospri committed Oct 5, 2023
1 parent dd2ed56 commit eaaa8d5
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 43 deletions.
4 changes: 2 additions & 2 deletions h/schemas/annotation.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ class AnnotationSchema(JSONSchema):
},
"text": {"type": "string"},
"uri": {"type": "string"},
"metadata_jwe": {"type": "string", "required": False},
"metadata": {"type": "object"},
},
}

Expand Down Expand Up @@ -173,7 +173,7 @@ def validate(self, data):
appstruct.pop("document", {}), new_appstruct["target_uri"]
)

new_appstruct["metadata_jwe"] = appstruct.pop("metadata_jwe", None)
new_appstruct["metadata"] = appstruct.pop("metadata", None)

new_appstruct["extra"] = appstruct

Expand Down
1 change: 0 additions & 1 deletion h/security/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import logging

from h.security.encryption import ( # noqa:F401
decrypt_jwe_dict,
derive_key,
password_context,
token_urlsafe,
Expand Down
19 changes: 3 additions & 16 deletions h/services/annotation_metadata.py
Original file line number Diff line number Diff line change
@@ -1,38 +1,25 @@
import json
import os

from sqlalchemy import func, select
from sqlalchemy.dialects.postgresql import insert
from sqlalchemy.orm import Session

from h.models import Annotation, AnnotationMetadata, AnnotationSlim
from h.security import decrypt_jwe_dict


class AnnotationMetadataService:
def __init__(self, db: Session):
self._db = db
self._secret = os.environ.get("JWE_SECRET_LMS")

def set_annotation_metadata_from_jwe(self, annotation: Annotation, jwe: str):
"""
Set the decrypted `jwe` dict as the metadata for `annotation`.
It will create a new AnnotationMetadata row or update the existing one.
"""
annotation_metadata = decrypt_jwe_dict(self._secret, jwe)

self._set_annotation_metadata(annotation, annotation_metadata)
return annotation_metadata

def _set_annotation_metadata(self, annotation: Annotation, metadata: dict):
def set(self, annotation: Annotation, data: dict):
"""Sets `data` as the `annotation`'s metadata."""
stmt = insert(AnnotationMetadata).from_select(
["annotation_id", "data"],
select(
# Select annotation_slim.id via the JOIN
AnnotationSlim.id,
# Just use a literal for the actual data
func.jsonb(json.dumps(metadata)),
func.jsonb(json.dumps(data)),
)
.join(Annotation)
.where(Annotation.id == annotation.id),
Expand Down
8 changes: 3 additions & 5 deletions h/services/annotation_write.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def create_annotation(self, data: dict) -> Annotation:
+ _("Annotation {id} does not exist").format(id=references[0])
)

annotation_metadata_jwe = data.pop("metadata_jwe", None)
annotation_metadata = data.pop("metadata", None)
document_data = data.pop("document", {})
annotation = Annotation(**data)

Expand All @@ -78,10 +78,8 @@ def create_annotation(self, data: dict) -> Annotation:
self._db.add(annotation)
self.upsert_annotation_slim(annotation)

if annotation_metadata_jwe:
self._annotation_metadata_service.set_annotation_metadata_from_jwe(
annotation, annotation_metadata_jwe
)
if annotation_metadata:
self._annotation_metadata_service.set(annotation, annotation_metadata)

self._search_index_service._queue.add_by_id( # pylint: disable=protected-access
annotation.id, tag="storage.create_annotation", schedule_in=60
Expand Down
29 changes: 13 additions & 16 deletions tests/h/services/annotation_metadata_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,41 +7,38 @@


class TestAnnotationMetadataService:
def test_it(self, svc, factories, decrypt_jwe_dict, db_session):
decrypt_jwe_dict.side_effect = [{"some": "data"}, {"new": "data"}]
def test_it(self, svc, factories, db_session):
metadata = {"some": "data"}
anno = factories.Annotation()
anno_slim = factories.AnnotationSlim(annotation=anno)
db_session.flush()

svc.set_annotation_metadata_from_jwe(anno, sentinel.jwe)
svc.set(anno, metadata)

decrypt_jwe_dict.assert_called_once_with("secret", sentinel.jwe)
anno_metadata = (
db_session.query(AnnotationMetadata)
.filter_by(annotation_id=anno_slim.id)
.one()
)
assert anno_metadata.data == {"some": "data"}
assert anno_metadata.data == metadata

# Second call updates the existing record
svc.set_annotation_metadata_from_jwe(anno, sentinel.jwe)
metadata = {"new": "data"}
svc.set(anno, metadata)
db_session.refresh(anno_metadata)
assert anno_metadata.data == {"new": "data"}
assert anno_metadata.data == metadata

def test_factory(self, pyramid_request):
def test_factory(self, AnnotationMetadataService, db_session, pyramid_request):
svc = factory(sentinel.context, pyramid_request)

assert isinstance(svc, AnnotationMetadataService)

@pytest.fixture
def decrypt_jwe_dict(self, patch):
return patch("h.services.annotation_metadata.decrypt_jwe_dict")
AnnotationMetadataService.assert_called_once_with(db=db_session)
assert svc == AnnotationMetadataService.return_value

@pytest.fixture
def svc(self, db_session, pyramid_request):
pyramid_request.db = db_session
return AnnotationMetadataService(db_session)

@pytest.fixture(autouse=True)
def environ(self, monkeypatch):
monkeypatch.setenv("JWE_SECRET_LMS", "secret")
@pytest.fixture
def AnnotationMetadataService(self, patch):
return patch("h.services.annotation_metadata.AnnotationMetadataService")
6 changes: 3 additions & 3 deletions tests/h/services/annotation_write_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,12 @@ def test_create_annotation_with_metadata(
group = factories.Group()
create_data["references"] = None
create_data["groupid"] = group.pubid
create_data["metadata_jwe"] = sentinel.metadata_jwe
create_data["metadata"] = sentinel.metadata

result = svc.create_annotation(create_data)

annotation_metadata_service.set_annotation_metadata_from_jwe.assert_called_once_with(
result, sentinel.metadata_jwe
annotation_metadata_service.set.assert_called_once_with(
result, sentinel.metadata
)

def test_create_annotation_as_root(
Expand Down

0 comments on commit eaaa8d5

Please sign in to comment.