Skip to content

Commit

Permalink
fix logging download endpoints route gets confused by storage folders
Browse files Browse the repository at this point in the history
  • Loading branch information
marwoodandrew committed Jul 17, 2023
1 parent 8a11583 commit 1db5feb
Show file tree
Hide file tree
Showing 11 changed files with 75 additions and 63 deletions.
2 changes: 1 addition & 1 deletion assets/ui/components/ArticleBodyHtml.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ class ArticleBodyHtml extends React.PureComponent {
vTag.setAttribute('data-plyr-config', '{"controls": ["play-large", "play",' +
'"progress", "volume", "mute", "rewind", "fast-forward", "current-time",' +
'"captions", "restart", "duration", "download"], "urls": {"download": ' +
'"' + vTag.getAttribute('src') + '/' + item._id + '"' +
'"' + vTag.getAttribute('src') + '?item_id=' + item._id + '"' +
'}}');
}

Expand Down
3 changes: 2 additions & 1 deletion features/environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ def before_scenario(context, scenario):
'NEWS_API_ENABLED': True,
'NEWS_API_IMAGE_PERMISSIONS_ENABLED': True,
'NEWS_API_TIME_LIMIT_DAYS': 100,
'NEWS_API_ALLOWED_RENDITIONS': 'original,16-9'
'NEWS_API_ALLOWED_RENDITIONS': 'original,16-9',
'EMBED_PRODUCT_FILTERING': True,
}

if 'rate_limit' in scenario.tags:
Expand Down
2 changes: 1 addition & 1 deletion features/news_api_atom.feature
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ Feature: News API News Search
],
"query": "",
"sd_product_id": "1",
"product_type": "wire"
"product_type": "news_api"
}]
"""
Given "items"
Expand Down
74 changes: 50 additions & 24 deletions newsroom/history.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@
from superdesk import get_resource_service
from superdesk.resource import not_analyzed, not_enabled
from superdesk.utc import utcnow
from flask import json, abort, Blueprint, jsonify, g
from flask import json, abort, Blueprint, jsonify, g, current_app as app
from flask_babel import gettext
from eve.utils import ParsedRequest
from newsroom.utils import get_json_or_400
from newsroom.auth import get_user
from newsroom.products.products import get_products_by_company

blueprint = Blueprint('history', __name__)

Expand Down Expand Up @@ -111,7 +112,48 @@ def create_media_history_record(self, item, association_name, action, user, sect
except (werkzeug.exceptions.Conflict, pymongo.errors.BulkWriteError):
pass

def log_media_download(self, item_id, media_id):
def _find_association(self, item, media_id):
"""
Find the matching media association in the item
:param item: item object
:param media_id: ID of the media
:return: tuple (name, association) or a 404
"""
for name, association in (item.get('associations') or {}).items():
for rendition in association.get("renditions"):
if association.get('renditions').get(rendition).get('media') == media_id:
return name, association
# not found
abort(404)

def _get_permitted_products(self, company, section):
"""
Get the list of permitted Superdesk products for the user's company
:param company: company
:param section: section name
:return: list of permitted products
"""
return [p.get('sd_product_id') for p in get_products_by_company(company, None, section) if
p.get('sd_product_id')]

def _check_permissions(self, item, company, name, section):
"""
Check the passed item rendition is allowed for the given company if required
:param item:
:param company:
:param name:
:param section:
:return:
"""
if app.config.get("EMBED_PRODUCT_FILTERING"):
permitted_products = self._get_permitted_products(company, section)
embed_products = [p.get('code') for p in
((item.get('associations') or {}).get(name) or {}).get('products', [])]

if not len(set(embed_products) & set(permitted_products)):
abort(403)

def log_media_download(self, item_id, media_id, section='wire'):
"""
Given am item, media reference and a user record the download
:param item:
Expand All @@ -123,17 +165,9 @@ def log_media_download(self, item_id, media_id):
if not item:
abort(404)

found = False
# Find the matching media in the item
for name, association in (item.get('associations') or {}).items():
for rendition in item.get("associations").get(name).get("renditions"):
if association.get('renditions').get(rendition).get('media') == media_id:
found = True
break
if found:
break
if not found:
abort(404)
name, association = self._find_association(item, media_id)
self._check_permissions(item, user.get('company'), name, section)

action = 'download ' + association.get('type')
self.create_media_history_record(item, name, action, user, 'wire')

Expand All @@ -148,17 +182,9 @@ def log_api_media_download(self, item_id, media_id):
if not item:
abort(404)

found = False
# Find the matching media in the item
for name, association in (item.get('associations') or {}).items():
for rendition in item.get("associations").get(name).get("renditions"):
if association.get('renditions').get(rendition).get('media') == media_id:
found = True
break
if found:
break
if not found:
abort(404)
name, association = self._find_association(item, media_id)
self._check_permissions(item, g.user, name, 'news_api')

action = 'download ' + association.get('type')
self.create_media_history_record(item, name, action, {'_id': None, 'company': ObjectId(g.user)}, 'news_api')

Expand Down
2 changes: 1 addition & 1 deletion newsroom/news_api/formatters/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def get_version(self, id, version, formatter_name):
if utcnow() - timedelta(days=int(get_setting('news_api_time_limit_days'))) > item.get('versioncreated',
utcnow()):
abort(404)
remove_unpermissioned_embeds(item, g.user)
remove_unpermissioned_embeds(item, g.user, 'news_api')
set_embed_links(item)
ret = formatter.format_item(item)
return {'formatted_item': ret, 'mimetype': formatter.MIMETYPE, 'version': item.get('version')}
17 changes: 4 additions & 13 deletions newsroom/news_api/news/assets/assets.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,6 @@ def init_app(app):
superdesk.blueprint(blueprint, app)


@blueprint.route('/assets/<path:asset_id>/<item_id>', methods=['GET'])
def download(asset_id, item_id):
"""
Called on download of a media item, keeps a record of the download
:param media_id:
:param item_id:
:return:
"""
response = get_item(asset_id)
superdesk.get_resource_service('history').log_api_media_download(item_id, asset_id)
return response


@blueprint.route('/assets/<path:asset_id>', methods=['GET'])
def get_item(asset_id):
auth = app.auth
Expand All @@ -41,6 +28,10 @@ def get_item(asset_id):
abort(401, gettext('Invalid token'))
else:
return abort(401, gettext('Invalid token'))

item_id = request.args.get('item_id')
if item_id:
superdesk.get_resource_service('history').log_api_media_download(item_id, asset_id)
try:
media_file = flask.current_app.media.get(asset_id, ASSETS_RESOURCE)
except bson.errors.InvalidId:
Expand Down
2 changes: 1 addition & 1 deletion newsroom/news_api/news/atom/atom.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def _format_update_date(date):
if ((complete_item.get('associations') or {}).get('featuremedia') or {}).get('renditions'):
if not check_association_permission(complete_item):
continue
remove_unpermissioned_embeds(complete_item, g.user)
remove_unpermissioned_embeds(complete_item, g.user, 'news_api')

entry = SubElement(feed, 'entry')

Expand Down
2 changes: 1 addition & 1 deletion newsroom/news_api/news/rss/rss.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def _format_date_3(date):
if ((complete_item.get('associations') or {}).get('featuremedia') or {}).get('renditions'):
if not check_association_permission(complete_item):
continue
remove_unpermissioned_embeds(complete_item, g.user)
remove_unpermissioned_embeds(complete_item, g.user, 'news_api')

entry = SubElement(channel, 'item')

Expand Down
2 changes: 1 addition & 1 deletion newsroom/news_api/news/search_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def get(self, req, lookup):
doc.pop(field, None)

if 'associations' in orig_request_params.get('include_fields', ''):
remove_unpermissioned_embeds(doc, g.user)
remove_unpermissioned_embeds(doc, g.user, 'news_api')
set_embed_links(doc)
if not check_association_permission(doc):
doc.pop('associations', None)
Expand Down
15 changes: 9 additions & 6 deletions newsroom/news_api/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def set_embed_links(item):
"""

def update_url(item, elem, group):
elem.attrib["src"] = elem.attrib["src"] + "/" + item.get("_id")
elem.attrib["src"] = elem.attrib["src"] + "?item_id=" + item.get("_id")
return True

if not app.config.get("EMBED_PRODUCT_FILTERING"):
Expand All @@ -108,7 +108,7 @@ def update_url(item, elem, group):
if ass.get('renditions', {}).get(rendition, {}).get("href"):
ass.get('renditions', {}).get(rendition, {})["href"] = ass.get('renditions', {}).get(rendition,
{}).get(
"href") + '/' + item.get("_id")
"href") + '?item_id=' + item.get("_id")


def update_embed_urls(item, token):
Expand All @@ -127,9 +127,12 @@ def update_embed(item, elem, group):
src = item.get("associations", {}).get(embed_id, {}).get("renditions", {}).get(
rendition)
if src is not None and elem is not None:
token_param = {'token': token} if token else {}
elem.attrib["src"] = url_for('assets.download', asset_id=src.get('media'), item_id=item.get("_id"),
_external=True, **token_param)
params = {"item_id": item.get("_id")}
if token:
params["token"] = token
elem.attrib["src"] = url_for('assets.get_item', asset_id=src.get('media'),
_external=True, **params)
return True

update_embeds_in_body(item, update_embed, update_embed, update_embed)
if app.config.get("EMBED_PRODUCT_FILTERING"):
update_embeds_in_body(item, update_embed, update_embed, update_embed)
17 changes: 4 additions & 13 deletions newsroom/upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,6 @@ def get_file(key):
return url_for('upload.get_upload', media_id=filename)


@blueprint.route('/assets/<path:media_id>/<item_id>', methods=['GET'])
@login_required
def download(media_id, item_id):
"""
Called on download of a media item, keeps a record of the download
:param media_id:
:param item_id:
:return:
"""
get_resource_service('history').log_media_download(item_id, media_id)
return get_upload(media_id)


@blueprint.route('/assets/<path:media_id>', methods=['GET'])
@login_required
def get_upload(media_id):
Expand Down Expand Up @@ -66,6 +53,10 @@ def get_upload(media_id):
else:
response.headers['Content-Disposition'] = 'inline'

item_id = request.args.get('item_id')
if item_id:
get_resource_service('history').log_media_download(item_id, media_id)

return response


Expand Down

0 comments on commit 1db5feb

Please sign in to comment.