Skip to content

Passing more context to permissions and components #616

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 44 additions & 22 deletions invenio_records_resources/services/files/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def check_permission(self, identity, action_name, **kwargs):
action_name = self.config.permission_action_prefix + action_name
return super().check_permission(identity, action_name, **kwargs)

def _get_record(self, id_, identity, action, file_key=None):
def _get_record(self, id_, identity, action, file_key=None, **kwargs):
"""Get the associated record.

If a ``file_key`` is specified and the record in question doesn't have a file
Expand All @@ -61,7 +61,9 @@ def _get_record(self, id_, identity, action, file_key=None):
# FIXME: Remove "registered_only=False" since it breaks access to an
# unpublished record.
record = self.record_cls.pid.resolve(id_, registered_only=False)
self.require_permission(identity, action, record=record, file_key=file_key)
self.require_permission(
identity, action, record=record, file_key=file_key, **kwargs
)

if file_key and file_key not in record.files:
raise FileKeyNotFoundError(id_, file_key)
Expand All @@ -71,9 +73,9 @@ def _get_record(self, id_, identity, action, file_key=None):
#
# High-level API
#
def list_files(self, identity, id_):
def list_files(self, identity, id_, **kwargs):
"""List the files of a record."""
record = self._get_record(id_, identity, "read_files")
record = self._get_record(id_, identity, "read_files", **kwargs)

self.run_components("list_files", id_, identity, record)

Expand All @@ -87,9 +89,9 @@ def list_files(self, identity, id_):
)

@unit_of_work()
def init_files(self, identity, id_, data, uow=None):
def init_files(self, identity, id_, data, uow=None, **kwargs):
"""Initialize the file upload for the record."""
record = self._get_record(id_, identity, "create_files")
record = self._get_record(id_, identity, "create_files", data=data, **kwargs)

self.run_components("init_files", identity, id_, record, data, uow=uow)

Expand All @@ -103,12 +105,14 @@ def init_files(self, identity, id_, data, uow=None):
)

@unit_of_work()
def update_file_metadata(self, identity, id_, file_key, data, uow=None):
def update_file_metadata(self, identity, id_, file_key, data, uow=None, **kwargs):
"""Update the metadata of a file.

:raises FileKeyNotFoundError: If the record has no file for the ``file_key``
"""
record = self._get_record(id_, identity, "create_files", file_key=file_key)
record = self._get_record(
id_, identity, "create_files", file_key=file_key, data=data, **kwargs
)

self.run_components(
"update_file_metadata", identity, id_, file_key, record, data, uow=uow
Expand All @@ -122,12 +126,14 @@ def update_file_metadata(self, identity, id_, file_key, data, uow=None):
links_tpl=self.file_links_item_tpl(id_),
)

def read_file_metadata(self, identity, id_, file_key):
def read_file_metadata(self, identity, id_, file_key, **kwargs):
"""Read the metadata of a file.

:raises FileKeyNotFoundError: If the record has no file for the ``file_key``
"""
record = self._get_record(id_, identity, "read_files", file_key=file_key)
record = self._get_record(
id_, identity, "read_files", file_key=file_key, **kwargs
)

self.run_components("read_file_metadata", identity, id_, file_key, record)

Expand All @@ -140,12 +146,14 @@ def read_file_metadata(self, identity, id_, file_key):
)

@unit_of_work()
def extract_file_metadata(self, identity, id_, file_key, uow=None):
def extract_file_metadata(self, identity, id_, file_key, uow=None, **kwargs):
"""Extract metadata from a file and update the file metadata file.

:raises FileKeyNotFoundError: If the record has no file for the ``file_key``
"""
record = self._get_record(id_, identity, "create_files", file_key=file_key)
record = self._get_record(
id_, identity, "create_files", file_key=file_key, **kwargs
)
file_record = record.files[file_key]

self.run_components(
Expand All @@ -169,12 +177,14 @@ def extract_file_metadata(self, identity, id_, file_key, uow=None):
)

@unit_of_work()
def commit_file(self, identity, id_, file_key, uow=None):
def commit_file(self, identity, id_, file_key, uow=None, **kwargs):
"""Commit a file upload.

:raises FileKeyNotFoundError: If the record has no file for the ``file_key``
"""
record = self._get_record(id_, identity, "commit_files", file_key=file_key)
record = self._get_record(
id_, identity, "commit_files", file_key=file_key, **kwargs
)

self.run_components("commit_file", identity, id_, file_key, record, uow=uow)

Expand All @@ -187,12 +197,14 @@ def commit_file(self, identity, id_, file_key, uow=None):
)

@unit_of_work()
def delete_file(self, identity, id_, file_key, uow=None):
def delete_file(self, identity, id_, file_key, uow=None, **kwargs):
"""Delete a single file.

:raises FileKeyNotFoundError: If the record has no file for the ``file_key``
"""
record = self._get_record(id_, identity, "delete_files", file_key=file_key)
record = self._get_record(
id_, identity, "delete_files", file_key=file_key, **kwargs
)
deleted_file = record.files.delete(file_key, remove_rf=True)

self.run_components(
Expand All @@ -211,9 +223,9 @@ def delete_file(self, identity, id_, file_key, uow=None):
)

@unit_of_work()
def delete_all_files(self, identity, id_, uow=None):
def delete_all_files(self, identity, id_, uow=None, **kwargs):
"""Delete all the files of the record."""
record = self._get_record(id_, identity, "delete_files")
record = self._get_record(id_, identity, "delete_files", **kwargs)

# We have to separate the gathering of the keys from their deletion
# because of how record.files is implemented.
Expand All @@ -235,13 +247,21 @@ def delete_all_files(self, identity, id_, uow=None):

@unit_of_work()
def set_file_content(
self, identity, id_, file_key, stream, content_length=None, uow=None
self, identity, id_, file_key, stream, content_length=None, uow=None, **kwargs
):
"""Save file content.

:raises FileKeyNotFoundError: If the record has no file for the ``file_key``
"""
record = self._get_record(id_, identity, "set_content_files", file_key=file_key)
record = self._get_record(
id_,
identity,
"set_content_files",
file_key=file_key,
stream=stream,
content_length=content_length,
**kwargs
)
errors = None
try:
self.run_components(
Expand Down Expand Up @@ -272,12 +292,14 @@ def set_file_content(
links_tpl=self.file_links_item_tpl(id_),
)

def get_file_content(self, identity, id_, file_key):
def get_file_content(self, identity, id_, file_key, **kwargs):
"""Retrieve file content.

:raises FileKeyNotFoundError: If the record has no file for the ``file_key``
"""
record = self._get_record(id_, identity, "get_content_files", file_key=file_key)
record = self._get_record(
id_, identity, "get_content_files", file_key=file_key, **kwargs
)

self.run_components("get_file_content", identity, id_, file_key, record)

Expand Down
52 changes: 36 additions & 16 deletions invenio_records_resources/services/records/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ def search(
self, identity, params=None, search_preference=None, expand=False, **kwargs
):
"""Search for records matching the querystring."""
self.require_permission(identity, "search")
self.require_permission(identity, "search", params=params, **kwargs)

# Prepare and execute the search
params = params or {}
Expand All @@ -263,7 +263,9 @@ def scan(
self, identity, params=None, search_preference=None, expand=False, **kwargs
):
"""Scan for records matching the querystring."""
self.require_permission(identity, "search")
self.require_permission(
identity, "search", params=params, expand=expand, **kwargs
)

# Prepare and execute the search as scan()
params = params or {}
Expand Down Expand Up @@ -292,7 +294,14 @@ def reindex(
**kwargs,
):
"""Reindex records matching the query parameters."""
self.require_permission(identity, "search")
self.require_permission(
identity,
"search",
params=params,
search_query=search_query,
extra_filter=extra_filter,
**kwargs,
)

# prepare and update query
params = params or {}
Expand Down Expand Up @@ -320,25 +329,34 @@ def reindex(
return True

@unit_of_work()
def create(self, identity, data, uow=None, expand=False):
def create(self, identity, data, uow=None, expand=False, **kwargs):
"""Create a record.

:param identity: Identity of user creating the record.
:param data: Input data according to the data schema.
"""
return self._create(self.record_cls, identity, data, uow=uow, expand=expand)
return self._create(
self.record_cls, identity, data, uow=uow, expand=expand, **kwargs
)

@unit_of_work()
def _create(
self, record_cls, identity, data, raise_errors=True, uow=None, expand=False
self,
record_cls,
identity,
data,
raise_errors=True,
uow=None,
expand=False,
**kwargs,
):
"""Create a record.

:param identity: Identity of user creating the record.
:param dict data: Input data according to the data schema.
:param bool raise_errors: raise schema ValidationError or not.
"""
self.require_permission(identity, "create")
self.require_permission(identity, "create", data=data, expand=expand, **kwargs)

# Validate data and create record with pid
data, errors = self.schema.load(
Expand Down Expand Up @@ -376,18 +394,18 @@ def _create(
expand=expand,
)

def read(self, identity, id_, expand=False, action="read"):
def read(self, identity, id_, expand=False, action="read", **kwargs):
"""Retrieve a record."""
# Resolve and require permission
record = self.record_cls.pid.resolve(id_)
try:
self.require_permission(identity, action, record=record)
self.require_permission(identity, action, record=record, **kwargs)
except PermissionDeniedError:
raise RecordPermissionDeniedError(action_name=action, record=record)
# Run components
for component in self.components:
if hasattr(component, "read"):
component.read(identity, record=record)
component.read(identity, record=record, **kwargs)

return self.result_item(
self,
Expand All @@ -399,11 +417,11 @@ def read(self, identity, id_, expand=False, action="read"):
expand=expand,
)

def exists(self, identity, id_):
def exists(self, identity, id_, **kwargs):
"""Check if the record exists and user has permission."""
try:
record = self.record_cls.pid.resolve(id_)
self.require_permission(identity, "read", record=record)
self.require_permission(identity, "read", record=record, **kwargs)
return True
except (PIDDoesNotExistError, PermissionDeniedError):
return False
Expand Down Expand Up @@ -476,14 +494,16 @@ def read_all(self, identity, fields, max_records=150, **kwargs):
return self.result_list(self, identity, results)

@unit_of_work()
def update(self, identity, id_, data, revision_id=None, uow=None, expand=False):
def update(
self, identity, id_, data, revision_id=None, uow=None, expand=False, **kwargs
):
"""Replace a record."""
record = self.record_cls.pid.resolve(id_)

self.check_revision_id(record, revision_id)

# Permissions
self.require_permission(identity, "update", record=record)
self.require_permission(identity, "update", record=record, data=data, **kwargs)

data, _ = self.schema.load(
data, context=dict(identity=identity, pid=record.pid, record=record)
Expand All @@ -505,14 +525,14 @@ def update(self, identity, id_, data, revision_id=None, uow=None, expand=False):
)

@unit_of_work()
def delete(self, identity, id_, revision_id=None, uow=None):
def delete(self, identity, id_, revision_id=None, uow=None, **kwargs):
"""Delete a record from database and search indexes."""
record = self.record_cls.pid.resolve(id_)

self.check_revision_id(record, revision_id)

# Permissions
self.require_permission(identity, "delete", record=record)
self.require_permission(identity, "delete", record=record, **kwargs)

# Run components
self.run_components("delete", identity, record=record, uow=uow)
Expand Down