Skip to content

Commit

Permalink
[REF] documents, base: binary_content
Browse files Browse the repository at this point in the history
- Changed the binary_content method
to make it more generic and allow alternative ways of checking
the access rights by using a new method check_access_mode.

   Reason: Asked by ODO following documents' security review.

- if all pages are processed, splitting a pdf will
archive the original attachment.

   Reason: FP feedback.

Task: 1853490
  • Loading branch information
ThanhDodeurOdoo committed Sep 26, 2018
1 parent 379b09e commit 2d82692
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 32 deletions.
14 changes: 7 additions & 7 deletions addons/web/controllers/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -424,12 +424,12 @@ def xml2json_from_elementtree(el, preserve_whitespaces=False):

def binary_content(xmlid=None, model='ir.attachment', id=None, field='datas', unique=False,
filename=None, filename_field='datas_fname', download=False, mimetype=None,
default_mimetype='application/octet-stream', share_id=None, share_token=None, access_token=None,
default_mimetype='application/octet-stream', related_id=None, access_mode=None, access_token=None,
env=None):
return request.registry['ir.http'].binary_content(
xmlid=xmlid, model=model, id=id, field=field, unique=unique, filename=filename,
filename_field=filename_field, download=download, mimetype=mimetype,
default_mimetype=default_mimetype, share_id=share_id, share_token=share_token, access_token=access_token,
default_mimetype=default_mimetype, related_id=related_id, access_mode=access_mode, access_token=access_token,
env=env)

#----------------------------------------------------------
Expand Down Expand Up @@ -1007,12 +1007,12 @@ def force_contenttype(self, headers, contenttype='image/png'):
'/web/content/<string:model>/<int:id>/<string:field>/<string:filename>'], type='http', auth="public")
def content_common(self, xmlid=None, model='ir.attachment', id=None, field='datas',
filename=None, filename_field='datas_fname', unique=None, mimetype=None,
download=None, data=None, token=None, share_id=None, share_token=None, access_token=None,
download=None, data=None, token=None, access_token=None, related_id=None, access_mode=None,
**kw):
status, headers, content = binary_content(
xmlid=xmlid, model=model, id=id, field=field, unique=unique, filename=filename,
filename_field=filename_field, download=download, mimetype=mimetype,
access_token=access_token, share_id=share_id, share_token=share_token)
access_token=access_token, related_id=related_id, access_mode=access_mode)
if status == 304:
response = werkzeug.wrappers.Response(status=status, headers=headers)
elif status == 301:
Expand Down Expand Up @@ -1046,12 +1046,12 @@ def content_common(self, xmlid=None, model='ir.attachment', id=None, field='data
'/web/image/<int:id>-<string:unique>/<int:width>x<int:height>/<string:filename>'], type='http', auth="public")
def content_image(self, xmlid=None, model='ir.attachment', id=None, field='datas',
filename_field='datas_fname', unique=None, filename=None, mimetype=None,
download=None, width=0, height=0, crop=False, share_id=None, share_token=None, access_token=None, avoid_if_small=False,
upper_limit=False, signature=False):
download=None, width=0, height=0, crop=False, related_id=None, access_mode=None,
access_token=None, avoid_if_small=False, upper_limit=False, signature=False):
status, headers, content = binary_content(
xmlid=xmlid, model=model, id=id, field=field, unique=unique, filename=filename,
filename_field=filename_field, download=download, mimetype=mimetype,
default_mimetype='image/png', share_id=share_id, share_token=share_token, access_token=access_token)
default_mimetype='image/png', related_id=related_id, access_mode=access_mode, access_token=access_token)
if status == 304:
return werkzeug.wrappers.Response(status=304, headers=headers)
elif status == 301:
Expand Down
6 changes: 3 additions & 3 deletions addons/website/models/ir_http.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ def _handle_exception(cls, exception):
def binary_content(cls, xmlid=None, model='ir.attachment', id=None, field='datas',
unique=False, filename=None, filename_field='datas_fname', download=False,
mimetype=None, default_mimetype='application/octet-stream',
access_token=None, share_id=None, share_token=None, env=None):
access_token=None, related_id=None, access_mode=None, env=None):
env = env or request.env
obj = None
if xmlid:
Expand All @@ -254,8 +254,8 @@ def binary_content(cls, xmlid=None, model='ir.attachment', id=None, field='datas
return super(Http, cls).binary_content(
xmlid=xmlid, model=model, id=id, field=field, unique=unique, filename=filename,
filename_field=filename_field, download=download, mimetype=mimetype,
default_mimetype=default_mimetype, access_token=access_token, share_id=share_id, share_token=share_token,
env=env)
default_mimetype=default_mimetype, access_token=access_token, related_id=related_id,
access_mode=access_mode, env=env)

@classmethod
def _xmlid_to_obj(cls, env, xmlid):
Expand Down
3 changes: 3 additions & 0 deletions odoo/addons/base/models/ir_attachment.py
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,9 @@ def _split_pdf_groups(self, pdf_groups, remainder=False):
output_page.addPage(input_pdf.getPage(i))
new_pdf_id = self._make_pdf(output_page, name_ext)
new_pdf_ids.append(new_pdf_id)
self.write({'active': False})
elif not len(remainder_set):
self.write({'active': False})
return new_pdf_ids

def split_pdf(self, indices, remainder=False):
Expand Down
42 changes: 20 additions & 22 deletions odoo/addons/base/models/ir_http.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,11 +248,26 @@ def content_disposition(cls, filename):
def _xmlid_to_obj(cls, env, xmlid):
return env.ref(xmlid, False)

@classmethod
def check_access_mode(cls, env, id, access_mode, model, access_token=None, related_id=None):
"""
Implemented by each module to define an additional way to check access.
:param env: the env of binary_content
:param id: id of the record from which to fetch the binary
:param access_mode: typically a string that describes the behaviour of the custom check
:param model: the model of the object for which binary_content was called
:param related_id: optional id to check security.
:return: the object or falsy result if not valid.
"""
return None


@classmethod
def binary_content(cls, xmlid=None, model='ir.attachment', id=None, field='datas',
unique=False, filename=None, filename_field='datas_fname', download=False,
mimetype=None, default_mimetype='application/octet-stream',
access_token=None, share_id=None, share_token=None, env=None):
access_token=None, related_id=None, access_mode=None, env=None):
""" Get file, attachment or downloadable content
If the ``xmlid`` and ``id`` parameter is omitted, fetches the default value for the
Expand All @@ -268,8 +283,8 @@ def binary_content(cls, xmlid=None, model='ir.attachment', id=None, field='datas
:param str filename_field: if not create an filename with model-id-field
:param bool download: apply headers to download the file
:param str mimetype: mintype of the field (for headers)
:param share_id: the id of the documents.share that contains the attachment
:param share_token: the token of the documents.share that contains the attachment
:param related_id: the id of another record used for custom_check
:param access_mode: if truthy, will call custom_check to fetch the object that contains the binary.
:param str default_mimetype: default mintype if no mintype found
:param str access_token: optional token for unauthenticated access
only available for ir.attachment
Expand All @@ -281,31 +296,14 @@ def binary_content(cls, xmlid=None, model='ir.attachment', id=None, field='datas
obj = None
if xmlid:
obj = cls._xmlid_to_obj(env, xmlid)
if access_mode:
obj = cls.check_access_mode(env, id, access_mode, model, access_token=access_token, related_id=related_id)
elif id and model == 'ir.attachment' and access_token:
obj = env[model].sudo().browse(int(id))
if not consteq(obj.access_token or '', access_token):
return (403, [], None)
elif id and share_id and share_token:
share = env['documents.share'].sudo().browse(int(share_id))
if share:
if share.state == 'expired':
return (403, [], None)
if not consteq(share.access_token, share_token):
return (403, [], None)
elif share.type == 'ids' and (id in share.attachment_ids.ids):
obj = env[model].sudo().browse(int(id))
elif share.type == 'domain':
obj = env[model].sudo().browse(int(id))
share_domain = []
if share.domain:
share_domain = literal_eval(share.domain)
domain = [['folder_id', '=', share.folder_id.id]] + share_domain
attachments_check = http.request.env['ir.attachment'].sudo().search(domain)
if obj not in attachments_check:
return (403, [], None)
elif id and model in env.registry:
obj = env[model].browse(int(id))

# obj exists
if not obj or not obj.exists() or field not in obj:
return (404, [], None)
Expand Down

0 comments on commit 2d82692

Please sign in to comment.