From 3119010778da879529377514a41c5244013fe2b5 Mon Sep 17 00:00:00 2001 From: Holger Brunn Date: Mon, 21 Mar 2016 11:15:51 +0100 Subject: [PATCH 1/4] [ADD] base_mixin_restrict_field_access --- base_mixin_restrict_field_access/README.rst | 118 +++++++++ base_mixin_restrict_field_access/__init__.py | 4 + .../__openerp__.py | 15 ++ .../models/__init__.py | 4 + .../models/restrict_field_access_mixin.py | 231 ++++++++++++++++++ .../static/description/icon.png | Bin 0 -> 9455 bytes .../tests/__init__.py | 4 + .../test_base_mixin_restrict_field_access.py | 33 +++ 8 files changed, 409 insertions(+) create mode 100644 base_mixin_restrict_field_access/README.rst create mode 100644 base_mixin_restrict_field_access/__init__.py create mode 100644 base_mixin_restrict_field_access/__openerp__.py create mode 100644 base_mixin_restrict_field_access/models/__init__.py create mode 100644 base_mixin_restrict_field_access/models/restrict_field_access_mixin.py create mode 100644 base_mixin_restrict_field_access/static/description/icon.png create mode 100644 base_mixin_restrict_field_access/tests/__init__.py create mode 100644 base_mixin_restrict_field_access/tests/test_base_mixin_restrict_field_access.py diff --git a/base_mixin_restrict_field_access/README.rst b/base_mixin_restrict_field_access/README.rst new file mode 100644 index 000000000..63ae9770c --- /dev/null +++ b/base_mixin_restrict_field_access/README.rst @@ -0,0 +1,118 @@ +.. image:: https://img.shields.io/badge/licence-AGPL--3-blue.svg + :alt: License: AGPL-3 + +===================== +Restrict field access +===================== + +This module was written to help developers restricting access to fields in a +secure and flexible manner. + +If you're not a developer, this module is not for you as you need to write code +in order to actually use it. + +Usage +===== + +To use this module, you need to: + +.. code:: python + + class ResPartner(models.Model): + # inherit from the mixin + _inherit = ['restrict.field.access.mixin', 'res.partner'] + _name = 'res.partner' + + @api.multi + def _restrict_field_access_get_field_whitelist(self, action='read'): + # return a whitelist (or a blacklist) of fields, depending on the + # action passed + whitelist = [ + 'name', 'parent_id', 'is_company', 'firstname', 'lastname', + 'infix', 'initials', + ] + super(ResPartner, self)\ + ._restrict_field_access_get_field_whitelist(action=action) + if action == 'read': + whitelist.extend(['section_id', 'user_id']) + return whitelist + + @api.multi + def _restrict_field_access_is_field_accessible(self, field_name, + action='read'): + # in case the whitelist is not enough, you can also decide for + # specific records if an action can be carried out on it or not + result = super(ResPartner, self)\ + ._restrict_field_access_is_field_accessible( + field_name, action=action) + if result or not self: + return result + return all(this.section_id in self.env.user.section_ids or + this.user_id == self.env.user + for this in self) + + @api.multi + @api.onchange('section_id', 'user_id') + @api.depends('section_id', 'user_id') + def _compute_restrict_field_access(self): + # if your decision depends on other fields, you probably need to + # override this function in order to attach the correct onchange/ + # depends decorators + return super(ResPartner, self)._compute_restrict_field_access() + + @api.model + def _restrict_field_access_inject_restrict_field_access_domain( + self, domain): + # you also might want to decide with a domain expression which + # records are visible in the first place + domain[:] = expression.AND([ + domain, + [ + '|', + ('section_id', 'in', self.env.user.section_ids.ids), + ('user_id', '=', self.env.user.id), + ], + ]) + +The example code here will allow only reading a few fields for partners of +which the current user is neither the sales person nor in this partner's sales +team. + +For further information, please visit: + +* https://www.odoo.com/forum/help-1 + +Known issues / Roadmap +====================== + +* the code contains some TODOs which should be done + +Bug Tracker +=========== + +Bugs are tracked on `GitHub Issues `_. +In case of trouble, please check there if your issue has already been reported. +If you spotted it first, help us smashing it by providing a detailed and welcomed feedback +`here `_. + +Credits +======= + +Contributors +------------ + +* Holger Brunn + +Maintainer +---------- + +.. image:: https://odoo-community.org/logo.png + :alt: Odoo Community Association + :target: https://odoo-community.org + +This module is maintained by the OCA. + +OCA, or the Odoo Community Association, is a nonprofit organization whose +mission is to support the collaborative development of Odoo features and +promote its widespread use. + +To contribute to this module, please visit https://odoo-community.org. diff --git a/base_mixin_restrict_field_access/__init__.py b/base_mixin_restrict_field_access/__init__.py new file mode 100644 index 000000000..7eda98a23 --- /dev/null +++ b/base_mixin_restrict_field_access/__init__.py @@ -0,0 +1,4 @@ +# -*- coding: utf-8 -*- +# © 2016 Therp BV +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). +from . import models diff --git a/base_mixin_restrict_field_access/__openerp__.py b/base_mixin_restrict_field_access/__openerp__.py new file mode 100644 index 000000000..e577a166d --- /dev/null +++ b/base_mixin_restrict_field_access/__openerp__.py @@ -0,0 +1,15 @@ +# -*- coding: utf-8 -*- +# © 2016 Therp BV +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). +{ + "name": "Restrict field access", + "version": "8.0.1.0.0", + "author": "Therp BV,Odoo Community Association (OCA)", + "license": "AGPL-3", + "category": "Hidden/Dependency", + "summary": "Make it simple to restrict read and/or write access to " + "certain fields base on some condition", + "depends": [ + 'base', + ], +} diff --git a/base_mixin_restrict_field_access/models/__init__.py b/base_mixin_restrict_field_access/models/__init__.py new file mode 100644 index 000000000..a2f49deff --- /dev/null +++ b/base_mixin_restrict_field_access/models/__init__.py @@ -0,0 +1,4 @@ +# -*- coding: utf-8 -*- +# © 2016 Therp BV +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). +from . import restrict_field_access_mixin diff --git a/base_mixin_restrict_field_access/models/restrict_field_access_mixin.py b/base_mixin_restrict_field_access/models/restrict_field_access_mixin.py new file mode 100644 index 000000000..ab9bd5119 --- /dev/null +++ b/base_mixin_restrict_field_access/models/restrict_field_access_mixin.py @@ -0,0 +1,231 @@ +# -*- coding: utf-8 -*- +# © 2016 Therp BV +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). +import json +from lxml import etree +from openerp import _, api, fields, models, SUPERUSER_ID +from openerp.osv import expression # pylint: disable=W0402 + + +class RestrictFieldAccessMixin(models.AbstractModel): + """Mixin to restrict access to fields on record level""" + _name = 'restrict.field.access.mixin' + + # TODO: read_group, __export_rows, everything that was forgotten + + @api.multi + def _compute_restrict_field_access(self): + """determine if restricted field access is active on records. + If you override _restrict_field_access_is_field_accessible to make + fields accessible depending on some other field values, override this + to in order to append an @api.depends that reflects this""" + result = {} + for this in self: + this['restrict_field_access'] = any( + not this._restrict_field_access_is_field_accessible( + field, 'write') + for field in self._fields) + if this['restrict_field_access']: + result['warning'] = { + 'title': _('Warning'), + 'message': _( + 'You will lose access to fields if you save now!'), + } + return result + + # use this field on your forms to be able to hide gui elements + restrict_field_access = fields.Boolean( + 'Field access restricted', compute='_compute_restrict_field_access') + + @api.model + @api.returns('self', lambda x: x.id) + def create(self, vals): + restricted_vals = self._restrict_field_access_filter_vals( + vals, action='create') + return self.browse( + super(RestrictFieldAccessMixin, + # TODO: this allows users to slip in nonallowed + # fields with x2many operations, so we need to reset + # this somewhere, probably just at the beginning of create + self._restrict_field_access_suspend()) + .create(restricted_vals).ids + ) + + @api.multi + def copy(self, default=None): + restricted_default = self._restrict_field_access_filter_vals( + default or {}, action='create') + return self.browse( + super(RestrictFieldAccessMixin, + self._restrict_field_access_suspend()) + .copy(default=restricted_default).ids + ) + + @api.multi + def read(self, fields=None, load='_classic_read'): + result = super(RestrictFieldAccessMixin, self).read( + fields=fields, load=load) + for record in result: + for field in record: + if not self._restrict_field_access_is_field_accessible(field): + record[field] = self._fields[field].convert_to_read( + self._fields[field].null(self.env)) + return result + + @api.multi + def write(self, vals): + for this in self: + # this way, we get the minimal values we can write on all records + vals = this._restrict_field_access_filter_vals( + vals, action='write') + return super(RestrictFieldAccessMixin, self).write(vals) + + @api.model + def _search(self, args, offset=0, limit=None, order=None, count=False, + access_rights_uid=None): + if not args: + return super(RestrictFieldAccessMixin, self)._search( + args, offset=offset, limit=limit, order=order, count=count, + access_rights_uid=access_rights_uid) + args = expression.normalize_domain(args) + has_inaccessible_field = False + for term in args: + if not expression.is_leaf(term): + continue + if not self._restrict_field_access_is_field_accessible( + term[0], 'read'): + has_inaccessible_field = True + break + if has_inaccessible_field: + check_self = self if not access_rights_uid else self.sudo( + access_rights_uid) + check_self\ + ._restrict_field_access_inject_restrict_field_access_domain( + args) + return super(RestrictFieldAccessMixin, self)._search( + args, offset=offset, limit=limit, order=order, count=count, + access_rights_uid=access_rights_uid) + + @api.model + def _restrict_field_access_inject_restrict_field_access_domain( + self, domain): + """inject a proposition to restrict search results to only the ones + where the user may access all fields in the search domain. If you + you override _restrict_field_access_is_field_accessible to make + fields accessible depending on some other field values, override this + in order not to leak information""" + pass + + @api.cr_uid_context + def fields_view_get(self, cr, uid, view_id=None, view_type='form', + context=None, toolbar=False, submenu=False): + # This needs to be oldstyle because res.partner in base passes context + # as positional argument + result = super(RestrictFieldAccessMixin, self).fields_view_get( + cr, uid, view_id=view_id, view_type=view_type, context=context, + toolbar=toolbar, submenu=submenu) + + if view_type == 'search': + return result + + # inject modifiers to make forbidden fields readonly + arch = etree.fromstring(result['arch']) + for field in arch.xpath('//field'): + field.attrib['modifiers'] = json.dumps( + self._restrict_field_access_adjust_field_modifiers( + cr, uid, + field, + json.loads(field.attrib.get('modifiers', '{}')), + context=context)) + + self._restrict_field_access_inject_restrict_field_access_arch( + cr, uid, arch, result['fields'], context=context) + + result['arch'] = etree.tostring(arch, encoding="utf-8") + return result + + @api.model + def _restrict_field_access_inject_restrict_field_access_arch( + self, arch, fields): + """inject the field restrict_field_access into arch if not there""" + if 'restrict_field_access' not in fields: + etree.SubElement(arch, 'field', { + 'name': 'restrict_field_access', + 'modifiers': json.dumps({ + ('tree_' if arch.tag == 'tree' else '') + 'invisible': True + }), + }) + fields['restrict_field_access'] =\ + self._fields['restrict_field_access'].get_description(self.env) + + @api.model + def _restrict_field_access_adjust_field_modifiers(self, field_node, + modifiers): + """inject a readonly modifier to make non-writable fields in a form + readonly""" + # TODO: this can be fooled by embedded views + if not self._restrict_field_access_is_field_accessible( + field_node.attrib['name'], action='write'): + for modifier, value in [('readonly', True), ('required', False)]: + domain = modifiers.get(modifier, []) + if isinstance(domain, list) and domain: + domain = expression.normalize_domain(domain) + elif bool(domain) == value: + # readonly/nonrequired anyways + return modifiers + else: + domain = [] + restrict_domain = [('restrict_field_access', '=', value)] + if domain: + restrict_domain = expression.OR([ + restrict_domain, + domain + ]) + modifiers[modifier] = restrict_domain + return modifiers + + @api.multi + def _restrict_field_access_get_field_whitelist(self, action='read'): + """return whitelisted fields. Those are readable and writable for + everyone, for the rest, it depends on your implementation of + _restrict_field_access_is_field_accessible""" + return models.MAGIC_COLUMNS + [ + self._rec_name, 'display_name', 'restrict_field_access', + ] + + @api.model + def _restrict_field_access_suspend(self): + """set a marker that we don't want to restrict field access""" + # TODO: this is insecure. in the end, we need something in the lines of + # base_suspend_security's uid-hack + return self.with_context(_restrict_field_access_suspend=True) + + @api.model + def _restrict_field_access_get_is_suspended(self): + """return True if we shouldn't check for field access restrictions""" + return self.env.context.get('_restrict_field_access_suspend') + + @api.multi + def _restrict_field_access_filter_vals(self, vals, action='read'): + """remove inaccessible fields from vals""" + assert len(self) <= 1, 'This function needs an empty recordset or '\ + 'exactly one record' + this = self.new(dict((self.copy_data()[0] if self else {}), **vals)) + return dict( + filter( + lambda itemtuple: + this._restrict_field_access_is_field_accessible( + itemtuple[0], action=action), + vals.iteritems())) + + @api.multi + def _restrict_field_access_is_field_accessible(self, field_name, + action='read'): + """return True if the current user can perform specified action on + all records in self. Override for your own logic""" + if self._restrict_field_access_get_is_suspended() or\ + self.env.user.id == SUPERUSER_ID: + return True + whitelist = self._restrict_field_access_get_field_whitelist( + action=action) + return field_name in whitelist diff --git a/base_mixin_restrict_field_access/static/description/icon.png b/base_mixin_restrict_field_access/static/description/icon.png new file mode 100644 index 0000000000000000000000000000000000000000..3a0328b516c4980e8e44cdb63fd945757ddd132d GIT binary patch literal 9455 zcmW++2RxMjAAjx~&dlBk9S+%}OXg)AGE&Cb*&}d0jUxM@u(PQx^-s)697TX`ehR4?GS^qbkof1cslKgkU)h65qZ9Oc=ml_0temigYLJfnz{IDzUf>bGs4N!v3=Z3jMq&A#7%rM5eQ#dc?k~! zVpnB`o+K7|Al`Q_U;eD$B zfJtP*jH`siUq~{KE)`jP2|#TUEFGRryE2`i0**z#*^6~AI|YzIWy$Cu#CSLW3q=GA z6`?GZymC;dCPk~rBS%eCb`5OLr;RUZ;D`}um=H)BfVIq%7VhiMr)_#G0N#zrNH|__ zc+blN2UAB0=617@>_u;MPHN;P;N#YoE=)R#i$k_`UAA>WWCcEVMh~L_ zj--gtp&|K1#58Yz*AHCTMziU1Jzt_jG0I@qAOHsk$2}yTmVkBp_eHuY$A9)>P6o~I z%aQ?!(GqeQ-Y+b0I(m9pwgi(IIZZzsbMv+9w{PFtd_<_(LA~0H(xz{=FhLB@(1&qHA5EJw1>>=%q2f&^X>IQ{!GJ4e9U z&KlB)z(84HmNgm2hg2C0>WM{E(DdPr+EeU_N@57;PC2&DmGFW_9kP&%?X4}+xWi)( z;)z%wI5>D4a*5XwD)P--sPkoY(a~WBw;E~AW`Yue4kFa^LM3X`8x|}ZUeMnqr}>kH zG%WWW>3ml$Yez?i%)2pbKPI7?5o?hydokgQyZsNEr{a|mLdt;X2TX(#B1j35xPnPW z*bMSSOauW>o;*=kO8ojw91VX!qoOQb)zHJ!odWB}d+*K?#sY_jqPdg{Sm2HdYzdEx zOGVPhVRTGPtv0o}RfVP;Nd(|CB)I;*t&QO8h zFfekr30S!-LHmV_Su-W+rEwYXJ^;6&3|L$mMC8*bQptyOo9;>Qb9Q9`ySe3%V$A*9 zeKEe+b0{#KWGp$F+tga)0RtI)nhMa-K@JS}2krK~n8vJ=Ngm?R!9G<~RyuU0d?nz# z-5EK$o(!F?hmX*2Yt6+coY`6jGbb7tF#6nHA zuKk=GGJ;ZwON1iAfG$E#Y7MnZVmrY|j0eVI(DN_MNFJmyZ|;w4tf@=CCDZ#5N_0K= z$;R~bbk?}TpfDjfB&aiQ$VA}s?P}xPERJG{kxk5~R`iRS(SK5d+Xs9swCozZISbnS zk!)I0>t=A<-^z(cmSFz3=jZ23u13X><0b)P)^1T_))Kr`e!-pb#q&J*Q`p+B6la%C zuVl&0duN<;uOsB3%T9Fp8t{ED108<+W(nOZd?gDnfNBC3>M8WE61$So|P zVvqH0SNtDTcsUdzaMDpT=Ty0pDHHNL@Z0w$Y`XO z2M-_r1S+GaH%pz#Uy0*w$Vdl=X=rQXEzO}d6J^R6zjM1u&c9vYLvLp?W7w(?np9x1 zE_0JSAJCPB%i7p*Wvg)pn5T`8k3-uR?*NT|J`eS#_#54p>!p(mLDvmc-3o0mX*mp_ zN*AeS<>#^-{S%W<*mz^!X$w_2dHWpcJ6^j64qFBft-o}o_Vx80o0>}Du;>kLts;$8 zC`7q$QI(dKYG`Wa8#wl@V4jVWBRGQ@1dr-hstpQL)Tl+aqVpGpbSfN>5i&QMXfiZ> zaA?T1VGe?rpQ@;+pkrVdd{klI&jVS@I5_iz!=UMpTsa~mBga?1r}aRBm1WS;TT*s0f0lY=JBl66Upy)-k4J}lh=P^8(SXk~0xW=T9v*B|gzIhN z>qsO7dFd~mgxAy4V?&)=5ieYq?zi?ZEoj)&2o)RLy=@hbCRcfT5jigwtQGE{L*8<@Yd{zg;CsL5mvzfDY}P-wos_6PfprFVaeqNE%h zKZhLtcQld;ZD+>=nqN~>GvROfueSzJD&BE*}XfU|H&(FssBqY=hPCt`d zH?@s2>I(|;fcW&YM6#V#!kUIP8$Nkdh0A(bEVj``-AAyYgwY~jB zT|I7Bf@%;7aL7Wf4dZ%VqF$eiaC38OV6oy3Z#TER2G+fOCd9Iaoy6aLYbPTN{XRPz z;U!V|vBf%H!}52L2gH_+j;`bTcQRXB+y9onc^wLm5wi3-Be}U>k_u>2Eg$=k!(l@I zcCg+flakT2Nej3i0yn+g+}%NYb?ta;R?(g5SnwsQ49U8Wng8d|{B+lyRcEDvR3+`O{zfmrmvFrL6acVP%yG98X zo&+VBg@px@i)%o?dG(`T;n*$S5*rnyiR#=wW}}GsAcfyQpE|>a{=$Hjg=-*_K;UtD z#z-)AXwSRY?OPefw^iI+ z)AXz#PfEjlwTes|_{sB?4(O@fg0AJ^g8gP}ex9Ucf*@_^J(s_5jJV}c)s$`Myn|Kd z$6>}#q^n{4vN@+Os$m7KV+`}c%4)4pv@06af4-x5#wj!KKb%caK{A&Y#Rfs z-po?Dcb1({W=6FKIUirH&(yg=*6aLCekcKwyfK^JN5{wcA3nhO(o}SK#!CINhI`-I z1)6&n7O&ZmyFMuNwvEic#IiOAwNkR=u5it{B9n2sAJV5pNhar=j5`*N!Na;c7g!l$ z3aYBqUkqqTJ=Re-;)s!EOeij=7SQZ3Hq}ZRds%IM*PtM$wV z@;rlc*NRK7i3y5BETSKuumEN`Xu_8GP1Ri=OKQ$@I^ko8>H6)4rjiG5{VBM>B|%`&&s^)jS|-_95&yc=GqjNo{zFkw%%HHhS~e=s zD#sfS+-?*t|J!+ozP6KvtOl!R)@@-z24}`9{QaVLD^9VCSR2b`b!KC#o;Ki<+wXB6 zx3&O0LOWcg4&rv4QG0)4yb}7BFSEg~=IR5#ZRj8kg}dS7_V&^%#Do==#`u zpy6{ox?jWuR(;pg+f@mT>#HGWHAJRRDDDv~@(IDw&R>9643kK#HN`!1vBJHnC+RM&yIh8{gG2q zA%e*U3|N0XSRa~oX-3EAneep)@{h2vvd3Xvy$7og(sayr@95+e6~Xvi1tUqnIxoIH zVWo*OwYElb#uyW{Imam6f2rGbjR!Y3`#gPqkv57dB6K^wRGxc9B(t|aYDGS=m$&S!NmCtrMMaUg(c zc2qC=2Z`EEFMW-me5B)24AqF*bV5Dr-M5ig(l-WPS%CgaPzs6p_gnCIvTJ=Y<6!gT zVt@AfYCzjjsMEGi=rDQHo0yc;HqoRNnNFeWZgcm?f;cp(6CNylj36DoL(?TS7eU#+ z7&mfr#y))+CJOXQKUMZ7QIdS9@#-}7y2K1{8)cCt0~-X0O!O?Qx#E4Og+;A2SjalQ zs7r?qn0H044=sDN$SRG$arw~n=+T_DNdSrarmu)V6@|?1-ZB#hRn`uilTGPJ@fqEy zGt(f0B+^JDP&f=r{#Y_wi#AVDf-y!RIXU^0jXsFpf>=Ji*TeqSY!H~AMbJdCGLhC) zn7Rx+sXw6uYj;WRYrLd^5IZq@6JI1C^YkgnedZEYy<&4(z%Q$5yv#Boo{AH8n$a zhb4Y3PWdr269&?V%uI$xMcUrMzl=;w<_nm*qr=c3Rl@i5wWB;e-`t7D&c-mcQl7x! zZWB`UGcw=Y2=}~wzrfLx=uet<;m3~=8I~ZRuzvMQUQdr+yTV|ATf1Uuomr__nDf=X zZ3WYJtHp_ri(}SQAPjv+Y+0=fH4krOP@S&=zZ-t1jW1o@}z;xk8 z(Nz1co&El^HK^NrhVHa-_;&88vTU>_J33=%{if;BEY*J#1n59=07jrGQ#IP>@u#3A z;!q+E1Rj3ZJ+!4bq9F8PXJ@yMgZL;>&gYA0%_Kbi8?S=XGM~dnQZQ!yBSgcZhY96H zrWnU;k)qy`rX&&xlDyA%(a1Hhi5CWkmg(`Gb%m(HKi-7Z!LKGRP_B8@`7&hdDy5n= z`OIxqxiVfX@OX1p(mQu>0Ai*v_cTMiw4qRt3~NBvr9oBy0)r>w3p~V0SCm=An6@3n)>@z!|o-$HvDK z|3D2ZMJkLE5loMKl6R^ez@Zz%S$&mbeoqH5`Bb){Ei21q&VP)hWS2tjShfFtGE+$z zzCR$P#uktu+#!w)cX!lWN1XU%K-r=s{|j?)Akf@q#3b#{6cZCuJ~gCxuMXRmI$nGtnH+-h z+GEi!*X=AP<|fG`1>MBdTb?28JYc=fGvAi2I<$B(rs$;eoJCyR6_bc~p!XR@O-+sD z=eH`-ye})I5ic1eL~TDmtfJ|8`0VJ*Yr=hNCd)G1p2MMz4C3^Mj?7;!w|Ly%JqmuW zlIEW^Ft%z?*|fpXda>Jr^1noFZEwFgVV%|*XhH@acv8rdGxeEX{M$(vG{Zw+x(ei@ zmfXb22}8-?Fi`vo-YVrTH*C?a8%M=Hv9MqVH7H^J$KsD?>!SFZ;ZsvnHr_gn=7acz z#W?0eCdVhVMWN12VV^$>WlQ?f;P^{(&pYTops|btm6aj>_Uz+hqpGwB)vWp0Cf5y< zft8-je~nn?W11plq}N)4A{l8I7$!ks_x$PXW-2XaRFswX_BnF{R#6YIwMhAgd5F9X zGmwdadS6(a^fjHtXg8=l?Rc0Sm%hk6E9!5cLVloEy4eh(=FwgP`)~I^5~pBEWo+F6 zSf2ncyMurJN91#cJTy_u8Y}@%!bq1RkGC~-bV@SXRd4F{R-*V`bS+6;W5vZ(&+I<9$;-V|eNfLa5n-6% z2(}&uGRF;p92eS*sE*oR$@pexaqr*meB)VhmIg@h{uzkk$9~qh#cHhw#>O%)b@+(| z^IQgqzuj~Sk(J;swEM-3TrJAPCq9k^^^`q{IItKBRXYe}e0Tdr=Huf7da3$l4PdpwWDop%^}n;dD#K4s#DYA8SHZ z&1!riV4W4R7R#C))JH1~axJ)RYnM$$lIR%6fIVA@zV{XVyx}C+a-Dt8Y9M)^KU0+H zR4IUb2CJ{Hg>CuaXtD50jB(_Tcx=Z$^WYu2u5kubqmwp%drJ6 z?Fo40g!Qd<-l=TQxqHEOuPX0;^z7iX?Ke^a%XT<13TA^5`4Xcw6D@Ur&VT&CUe0d} z1GjOVF1^L@>O)l@?bD~$wzgf(nxX1OGD8fEV?TdJcZc2KoUe|oP1#=$$7ee|xbY)A zDZq+cuTpc(fFdj^=!;{k03C69lMQ(|>uhRfRu%+!k&YOi-3|1QKB z z?n?eq1XP>p-IM$Z^C;2L3itnbJZAip*Zo0aw2bs8@(s^~*8T9go!%dHcAz2lM;`yp zD=7&xjFV$S&5uDaiScyD?B-i1ze`+CoRtz`Wn+Zl&#s4&}MO{@N!ufrzjG$B79)Y2d3tBk&)TxUTw@QS0TEL_?njX|@vq?Uz(nBFK5Pq7*xj#u*R&i|?7+6# z+|r_n#SW&LXhtheZdah{ZVoqwyT{D>MC3nkFF#N)xLi{p7J1jXlmVeb;cP5?e(=f# zuT7fvjSbjS781v?7{)-X3*?>tq?)Yd)~|1{BDS(pqC zC}~H#WXlkUW*H5CDOo<)#x7%RY)A;ShGhI5s*#cRDA8YgqG(HeKDx+#(ZQ?386dv! zlXCO)w91~Vw4AmOcATuV653fa9R$fyK8ul%rG z-wfS zihugoZyr38Im?Zuh6@RcF~t1anQu7>#lPpb#}4cOA!EM11`%f*07RqOVkmX{p~KJ9 z^zP;K#|)$`^Rb{rnHGH{~>1(fawV0*Z#)}M`m8-?ZJV<+e}s9wE# z)l&az?w^5{)`S(%MRzxdNqrs1n*-=jS^_jqE*5XDrA0+VE`5^*p3CuM<&dZEeCjoz zR;uu_H9ZPZV|fQq`Cyw4nscrVwi!fE6ciMmX$!_hN7uF;jjKG)d2@aC4ropY)8etW=xJvni)8eHi`H$%#zn^WJ5NLc-rqk|u&&4Z6fD_m&JfSI1Bvb?b<*n&sfl0^t z=HnmRl`XrFvMKB%9}>PaA`m-fK6a0(8=qPkWS5bb4=v?XcWi&hRY?O5HdulRi4?fN zlsJ*N-0Qw+Yic@s0(2uy%F@ib;GjXt01Fmx5XbRo6+n|pP(&nodMoap^z{~q ziEeaUT@Mxe3vJSfI6?uLND(CNr=#^W<1b}jzW58bIfyWTDle$mmS(|x-0|2UlX+9k zQ^EX7Nw}?EzVoBfT(-LT|=9N@^hcn-_p&sqG z&*oVs2JSU+N4ZD`FhCAWaS;>|wH2G*Id|?pa#@>tyxX`+4HyIArWDvVrX)2WAOQff z0qyHu&-S@i^MS-+j--!pr4fPBj~_8({~e1bfcl0wI1kaoN>mJL6KUPQm5N7lB(ui1 zE-o%kq)&djzWJ}ob<-GfDlkB;F31j-VHKvQUGQ3sp`CwyGJk_i!y^sD0fqC@$9|jO zOqN!r!8-p==F@ZVP=U$qSpY(gQ0)59P1&t@y?5rvg<}E+GB}26NYPp4f2YFQrQtot5mn3wu_qprZ=>Ig-$ zbW26Ws~IgY>}^5w`vTB(G`PTZaDiGBo5o(tp)qli|NeV( z@H_=R8V39rt5J5YB2Ky?4eJJ#b`_iBe2ot~6%7mLt5t8Vwi^Jy7|jWXqa3amOIoRb zOr}WVFP--DsS`1WpN%~)t3R!arKF^Q$e12KEqU36AWwnCBICpH4XCsfnyrHr>$I$4 z!DpKX$OKLWarN7nv@!uIA+~RNO)l$$w}p(;b>mx8pwYvu;dD_unryX_NhT8*Tj>BTrTTL&!?O+%Rv;b?B??gSzdp?6Uug9{ zd@V08Z$BdI?fpoCS$)t4mg4rT8Q_I}h`0d-vYZ^|dOB*Q^S|xqTV*vIg?@fVFSmMpaw0qtTRbx} z({Pg?#{2`sc9)M5N$*N|4;^t$+QP?#mov zGVC@I*lBVrOU-%2y!7%)fAKjpEFsgQc4{amtiHb95KQEwvf<(3T<9-Zm$xIew#P22 zc2Ix|App^>v6(3L_MCU0d3W##AB0M~3D00EWoKZqsJYT(#@w$Y_H7G22M~ApVFTRHMI_3be)Lkn#0F*V8Pq zc}`Cjy$bE;FJ6H7p=0y#R>`}-m4(0F>%@P|?7fx{=R^uFdISRnZ2W_xQhD{YuR3t< z{6yxu=4~JkeA;|(J6_nv#>Nvs&FuLA&PW^he@t(UwFFE8)|a!R{`E`K`i^ZnyE4$k z;(749Ix|oi$c3QbEJ3b~D_kQsPz~fIUKym($a_7dJ?o+40*OLl^{=&oq$<#Q(yyrp z{J-FAniyAw9tPbe&IhQ|a`DqFTVQGQ&Gq3!C2==4x{6EJwiPZ8zub-iXoUtkJiG{} zPaR&}_fn8_z~(=;5lD-aPWD3z8PZS@AaUiomF!G8I}Mf>e~0g#BelA-5#`cj;O5>N Xviia!U7SGha1wx#SCgwmn*{w2TRX*I literal 0 HcmV?d00001 diff --git a/base_mixin_restrict_field_access/tests/__init__.py b/base_mixin_restrict_field_access/tests/__init__.py new file mode 100644 index 000000000..f8cbe58b7 --- /dev/null +++ b/base_mixin_restrict_field_access/tests/__init__.py @@ -0,0 +1,4 @@ +# -*- coding: utf-8 -*- +# © 2016 Therp BV +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). +from . import test_base_mixin_restrict_field_access diff --git a/base_mixin_restrict_field_access/tests/test_base_mixin_restrict_field_access.py b/base_mixin_restrict_field_access/tests/test_base_mixin_restrict_field_access.py new file mode 100644 index 000000000..73d536e20 --- /dev/null +++ b/base_mixin_restrict_field_access/tests/test_base_mixin_restrict_field_access.py @@ -0,0 +1,33 @@ +# -*- coding: utf-8 -*- +# © 2016 Therp BV +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). +from openerp.tests.common import TransactionCase +from openerp import models + + +class TestBaseMixinRestrictFieldAccess(TransactionCase): + def test_base_mixin_restrict_field_access(self): + # inherit from our mixin + class ResPartner(models.Model): + _inherit = ['restrict.field.access.mixin', 'res.partner'] + _name = 'res.partner' + + # setup the model + res_partner = ResPartner._build_model(self.registry, self.cr).browse( + self.cr, self.uid, [], context={}) + res_partner._prepare_setup() + res_partner._setup_base(False) + res_partner._setup_fields() + res_partner._setup_complete() + # run tests as nonprivileged user + partner = res_partner.sudo(self.env.ref('base.user_demo').id).create({ + 'name': 'testpartner', + }) + partner.copy() + partner.write({ + 'name': 'testpartner2', + }) + partner.search([]) + self.assertTrue(partner.restrict_field_access) + partner.fields_view_get() + # TODO: a lot more tests From 536adf40553e3b30159de27868a757e16c790882 Mon Sep 17 00:00:00 2001 From: Danny Adair Date: Mon, 1 May 2017 21:33:42 +1200 Subject: [PATCH 2/4] [FIX] restrict exports and read_group * Add implementation for restricted exporting and grouping * Use original __export_rows() for models not using the mixin * Assume all fields if not specified, like in original read_group() * Return inaccessible fields in read_group() with null values * No need to remove 'restrict_field_access' --- .../models/restrict_field_access_mixin.py | 97 ++++++++++++++++++- 1 file changed, 95 insertions(+), 2 deletions(-) diff --git a/base_mixin_restrict_field_access/models/restrict_field_access_mixin.py b/base_mixin_restrict_field_access/models/restrict_field_access_mixin.py index ab9bd5119..47fb36cbd 100644 --- a/base_mixin_restrict_field_access/models/restrict_field_access_mixin.py +++ b/base_mixin_restrict_field_access/models/restrict_field_access_mixin.py @@ -4,6 +4,8 @@ import json from lxml import etree from openerp import _, api, fields, models, SUPERUSER_ID +from openerp.addons.web.controllers.main import Export +from openerp.http import request from openerp.osv import expression # pylint: disable=W0402 @@ -11,8 +13,6 @@ class RestrictFieldAccessMixin(models.AbstractModel): """Mixin to restrict access to fields on record level""" _name = 'restrict.field.access.mixin' - # TODO: read_group, __export_rows, everything that was forgotten - @api.multi def _compute_restrict_field_access(self): """determine if restricted field access is active on records. @@ -72,6 +72,85 @@ class RestrictFieldAccessMixin(models.AbstractModel): self._fields[field].null(self.env)) return result + def read_group(self, cr, uid, domain, fields, groupby, offset=0, limit=None, context=None, orderby=False, lazy=True): + """ + Remove inaccessible fields from 'fields', 'groupby' and 'orderby'. + + If this removes all 'fields', return no records. + If this removes all 'groupby', group by first remaining field. + If this removes 'orderby', don't specify order. + """ + requested_fields = fields or self._columns.keys() + sanitised_fields = [ + f for f in requested_fields if self._restrict_field_access_is_field_accessible( + cr, uid, [], f + ) + ] + if not sanitised_fields: + return [] + + sanitised_groupby = [] + groupby = [groupby] if isinstance(groupby, basestring) else groupby + for groupby_part in groupby: + groupby_field = groupby_part.split(':')[0] + if self._restrict_field_access_is_field_accessible(cr, uid, [], groupby_field): + sanitised_groupby.append(groupby_part) + if not sanitised_groupby: + sanitised_groupby.append(sanitised_fields[0]) + + if orderby: + sanitised_orderby = [] + for orderby_part in orderby.split(','): + orderby_field = orderby_part.split()[0] + if self._restrict_field_access_is_field_accessible(cr, uid, [], orderby_field): + sanitised_orderby.append(orderby_part) + sanitised_orderby = sanitised_orderby and ','.join(sanitised_orderby) or False + else: + sanitised_orderby = False + + result = super(RestrictFieldAccessMixin, self).read_group( + cr, + uid, + domain, + sanitised_fields, + sanitised_groupby, + offset=offset, + limit=limit, + context=context, + orderby=sanitised_orderby, + lazy=lazy + ) + # Add inaccessible fields back in with null values + inaccessible_fields = [f for f in requested_fields if f not in sanitised_fields] + for field_name in inaccessible_fields: + field = self._columns[field_name] + if lazy: + result.append( + { + '__domain': [(True, '=', True)], + field_name: field.null(self.env), + field_name + '_count': 0L + } + ) + else: + result.append( + { + '__domain': [(True, '=', True)], + field_name: field.null(self.env), + '__count': 0L + } + ) + return result + + @api.multi + def _BaseModel__export_rows(self, fields): + """Don't export inaccessible fields""" + if isinstance(self, RestrictFieldAccessMixin): + sanitised_fields = [f for f in fields if f and self._restrict_field_access_is_field_accessible(f[0])] + return super(RestrictFieldAccessMixin, self)._BaseModel__export_rows(sanitised_fields) + else: + return super(RestrictFieldAccessMixin, self)._BaseModel__export_rows(fields) + @api.multi def write(self, vals): for this in self: @@ -229,3 +308,17 @@ class RestrictFieldAccessMixin(models.AbstractModel): whitelist = self._restrict_field_access_get_field_whitelist( action=action) return field_name in whitelist + + +class RestrictedExport(Export): + """Don't (even offer to) export inaccessible fields""" + def fields_get(self, model): + Model = request.session.model(model) + fields = Model.fields_get(False, request.context) + model = request.env[model] + if isinstance(model, RestrictFieldAccessMixin): + sanitised_fields = {k:fields[k] for k in fields if model._restrict_field_access_is_field_accessible(k)} + return sanitised_fields + else: + return fields + From f264bdb27a8f442cabda838eeb807909056c33c6 Mon Sep 17 00:00:00 2001 From: Holger Brunn Date: Mon, 1 May 2017 14:36:22 +0200 Subject: [PATCH 3/4] [RFR] clean up community contribution --- base_mixin_restrict_field_access/__init__.py | 1 + .../__openerp__.py | 2 +- .../controllers/__init__.py | 4 + .../controllers/main.py | 21 +++ .../models/restrict_field_access_mixin.py | 152 ++++++++---------- 5 files changed, 90 insertions(+), 90 deletions(-) create mode 100644 base_mixin_restrict_field_access/controllers/__init__.py create mode 100644 base_mixin_restrict_field_access/controllers/main.py diff --git a/base_mixin_restrict_field_access/__init__.py b/base_mixin_restrict_field_access/__init__.py index 7eda98a23..0188fced6 100644 --- a/base_mixin_restrict_field_access/__init__.py +++ b/base_mixin_restrict_field_access/__init__.py @@ -2,3 +2,4 @@ # © 2016 Therp BV # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). from . import models +from . import controllers diff --git a/base_mixin_restrict_field_access/__openerp__.py b/base_mixin_restrict_field_access/__openerp__.py index e577a166d..52bbac6cf 100644 --- a/base_mixin_restrict_field_access/__openerp__.py +++ b/base_mixin_restrict_field_access/__openerp__.py @@ -10,6 +10,6 @@ "summary": "Make it simple to restrict read and/or write access to " "certain fields base on some condition", "depends": [ - 'base', + 'web', ], } diff --git a/base_mixin_restrict_field_access/controllers/__init__.py b/base_mixin_restrict_field_access/controllers/__init__.py new file mode 100644 index 000000000..1b21ddd33 --- /dev/null +++ b/base_mixin_restrict_field_access/controllers/__init__.py @@ -0,0 +1,4 @@ +# -*- coding: utf-8 -*- +# © 2017 Therp BV +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). +from . import main diff --git a/base_mixin_restrict_field_access/controllers/main.py b/base_mixin_restrict_field_access/controllers/main.py new file mode 100644 index 000000000..f2632e1bf --- /dev/null +++ b/base_mixin_restrict_field_access/controllers/main.py @@ -0,0 +1,21 @@ +# -*- coding: utf-8 -*- +# © 2017 Therp BV +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). +from openerp.http import request +from openerp.addons.web.controllers.main import Export +from ..models.restrict_field_access_mixin import RestrictFieldAccessMixin + + +class RestrictedExport(Export): + """Don't (even offer to) export inaccessible fields""" + def fields_get(self, model): + fields = super(RestrictedExport, self).fields_get(model) + model = request.env[model] + if isinstance(model, RestrictFieldAccessMixin): + sanitised_fields = { + k: fields[k] for k in fields + if model._restrict_field_access_is_field_accessible(k) + } + return sanitised_fields + else: + return fields diff --git a/base_mixin_restrict_field_access/models/restrict_field_access_mixin.py b/base_mixin_restrict_field_access/models/restrict_field_access_mixin.py index 47fb36cbd..181a2d97b 100644 --- a/base_mixin_restrict_field_access/models/restrict_field_access_mixin.py +++ b/base_mixin_restrict_field_access/models/restrict_field_access_mixin.py @@ -4,8 +4,6 @@ import json from lxml import etree from openerp import _, api, fields, models, SUPERUSER_ID -from openerp.addons.web.controllers.main import Export -from openerp.http import request from openerp.osv import expression # pylint: disable=W0402 @@ -72,84 +70,70 @@ class RestrictFieldAccessMixin(models.AbstractModel): self._fields[field].null(self.env)) return result - def read_group(self, cr, uid, domain, fields, groupby, offset=0, limit=None, context=None, orderby=False, lazy=True): - """ - Remove inaccessible fields from 'fields', 'groupby' and 'orderby'. - - If this removes all 'fields', return no records. - If this removes all 'groupby', group by first remaining field. - If this removes 'orderby', don't specify order. - """ - requested_fields = fields or self._columns.keys() - sanitised_fields = [ - f for f in requested_fields if self._restrict_field_access_is_field_accessible( - cr, uid, [], f + @api.model + def read_group(self, domain, fields, groupby, offset=0, limit=None, + orderby=False, lazy=True): + """Restrict reading if we read an inaccessible field""" + has_inaccessible_field = False + has_inaccessible_field |= any( + not self._restrict_field_access_is_field_accessible(f) + for f in fields or self._fields.keys() + ) + has_inaccessible_field |= any( + expression.is_leaf(term) and + not self._restrict_field_access_is_field_accessible( + term[0].split('.')[0] + ) + for term in domain + ) + if groupby: + if isinstance(groupby, basestring): + groupby = [groupby] + has_inaccessible_field |= any( + not self._restrict_field_access_is_field_accessible( + f.split(':')[0] + ) + for f in groupby ) - ] - if not sanitised_fields: - return [] - - sanitised_groupby = [] - groupby = [groupby] if isinstance(groupby, basestring) else groupby - for groupby_part in groupby: - groupby_field = groupby_part.split(':')[0] - if self._restrict_field_access_is_field_accessible(cr, uid, [], groupby_field): - sanitised_groupby.append(groupby_part) - if not sanitised_groupby: - sanitised_groupby.append(sanitised_fields[0]) - if orderby: - sanitised_orderby = [] - for orderby_part in orderby.split(','): - orderby_field = orderby_part.split()[0] - if self._restrict_field_access_is_field_accessible(cr, uid, [], orderby_field): - sanitised_orderby.append(orderby_part) - sanitised_orderby = sanitised_orderby and ','.join(sanitised_orderby) or False - else: - sanitised_orderby = False + has_inaccessible_field |= any( + not self._restrict_field_access_is_field_accessible(f.split()) + for f in orderby.split(',') + ) + # just like with search, we restrict read_group to the accessible + # records, because we'd either leak data otherwise or have very wrong + # results + if has_inaccessible_field: + self._restrict_field_access_inject_restrict_field_access_domain( + domain + ) - result = super(RestrictFieldAccessMixin, self).read_group( - cr, - uid, - domain, - sanitised_fields, - sanitised_groupby, - offset=offset, - limit=limit, - context=context, - orderby=sanitised_orderby, - lazy=lazy + return super(RestrictFieldAccessMixin, self).read_group( + domain, fields, groupby, offset=offset, limit=limit, + orderby=orderby, lazy=lazy ) - # Add inaccessible fields back in with null values - inaccessible_fields = [f for f in requested_fields if f not in sanitised_fields] - for field_name in inaccessible_fields: - field = self._columns[field_name] - if lazy: - result.append( - { - '__domain': [(True, '=', True)], - field_name: field.null(self.env), - field_name + '_count': 0L - } - ) - else: - result.append( - { - '__domain': [(True, '=', True)], - field_name: field.null(self.env), - '__count': 0L - } - ) - return result @api.multi def _BaseModel__export_rows(self, fields): - """Don't export inaccessible fields""" - if isinstance(self, RestrictFieldAccessMixin): - sanitised_fields = [f for f in fields if f and self._restrict_field_access_is_field_accessible(f[0])] - return super(RestrictFieldAccessMixin, self)._BaseModel__export_rows(sanitised_fields) - else: - return super(RestrictFieldAccessMixin, self)._BaseModel__export_rows(fields) + """Null inaccessible fields""" + result = [] + for this in self: + rows = super(RestrictFieldAccessMixin, this)\ + ._BaseModel__export_rows(fields) + for row in rows: + for i, path in enumerate(fields): + # we only need to take care of our own fields, super calls + # __export_rows again for x2x exports + if not path or len(path) > 1: + continue + if not this._restrict_field_access_is_field_accessible( + path[0], + ) and row[i]: + row[i] = self._fields[path[0]].convert_to_export( + self._fields[path[0]].null(self.env), self.env + ) + result.extend(rows) + return result @api.multi def write(self, vals): @@ -198,6 +182,7 @@ class RestrictFieldAccessMixin(models.AbstractModel): @api.cr_uid_context def fields_view_get(self, cr, uid, view_id=None, view_type='form', context=None, toolbar=False, submenu=False): + # pylint: disable=R8110 # This needs to be oldstyle because res.partner in base passes context # as positional argument result = super(RestrictFieldAccessMixin, self).fields_view_get( @@ -301,24 +286,13 @@ class RestrictFieldAccessMixin(models.AbstractModel): def _restrict_field_access_is_field_accessible(self, field_name, action='read'): """return True if the current user can perform specified action on - all records in self. Override for your own logic""" + all records in self. Override for your own logic. + This function is also called with an empty recordset to get a list + of fields which are accessible unconditionally""" if self._restrict_field_access_get_is_suspended() or\ - self.env.user.id == SUPERUSER_ID: + self.env.user.id == SUPERUSER_ID or\ + not self and action == 'read': return True whitelist = self._restrict_field_access_get_field_whitelist( action=action) return field_name in whitelist - - -class RestrictedExport(Export): - """Don't (even offer to) export inaccessible fields""" - def fields_get(self, model): - Model = request.session.model(model) - fields = Model.fields_get(False, request.context) - model = request.env[model] - if isinstance(model, RestrictFieldAccessMixin): - sanitised_fields = {k:fields[k] for k in fields if model._restrict_field_access_is_field_accessible(k)} - return sanitised_fields - else: - return fields - From ad02c342861fb1089146f55471e8d100466ff08d Mon Sep 17 00:00:00 2001 From: Holger Brunn Date: Mon, 1 May 2017 18:15:56 +0200 Subject: [PATCH 4/4] [IMP] finalize PoC --- base_mixin_restrict_field_access/README.rst | 9 +- .../__openerp__.py | 1 + .../models/restrict_field_access_mixin.py | 36 ++++--- .../test_base_mixin_restrict_field_access.py | 93 +++++++++++++++++-- 4 files changed, 116 insertions(+), 23 deletions(-) diff --git a/base_mixin_restrict_field_access/README.rst b/base_mixin_restrict_field_access/README.rst index 63ae9770c..03f02ac8c 100644 --- a/base_mixin_restrict_field_access/README.rst +++ b/base_mixin_restrict_field_access/README.rst @@ -6,7 +6,7 @@ Restrict field access ===================== This module was written to help developers restricting access to fields in a -secure and flexible manner. +secure and flexible manner on record level. If you're not a developer, this module is not for you as you need to write code in order to actually use it. @@ -14,7 +14,9 @@ in order to actually use it. Usage ===== -To use this module, you need to: +To use this module, you need to inherit this mixin for the model whose fields +you want to restrict, and implement at least the following methods to do +something useful: .. code:: python @@ -77,6 +79,9 @@ The example code here will allow only reading a few fields for partners of which the current user is neither the sales person nor in this partner's sales team. +Read the comments of the mixin, that's part of the documentation. Also have a +look at the tests, that's another example on how to use this code. + For further information, please visit: * https://www.odoo.com/forum/help-1 diff --git a/base_mixin_restrict_field_access/__openerp__.py b/base_mixin_restrict_field_access/__openerp__.py index 52bbac6cf..ce5859e0c 100644 --- a/base_mixin_restrict_field_access/__openerp__.py +++ b/base_mixin_restrict_field_access/__openerp__.py @@ -11,5 +11,6 @@ "certain fields base on some condition", "depends": [ 'web', + 'base_suspend_security', ], } diff --git a/base_mixin_restrict_field_access/models/restrict_field_access_mixin.py b/base_mixin_restrict_field_access/models/restrict_field_access_mixin.py index 181a2d97b..49eb08355 100644 --- a/base_mixin_restrict_field_access/models/restrict_field_access_mixin.py +++ b/base_mixin_restrict_field_access/models/restrict_field_access_mixin.py @@ -5,6 +5,8 @@ import json from lxml import etree from openerp import _, api, fields, models, SUPERUSER_ID from openerp.osv import expression # pylint: disable=W0402 +from openerp.addons.base_suspend_security.base_suspend_security import\ + BaseSuspendSecurityUid class RestrictFieldAccessMixin(models.AbstractModel): @@ -64,10 +66,14 @@ class RestrictFieldAccessMixin(models.AbstractModel): result = super(RestrictFieldAccessMixin, self).read( fields=fields, load=load) for record in result: + this = self.browse(record['id']) for field in record: - if not self._restrict_field_access_is_field_accessible(field): + if not this._restrict_field_access_is_field_accessible(field): record[field] = self._fields[field].convert_to_read( self._fields[field].null(self.env)) + if self._fields[field] in self.env.cache: + self.env.cache[self._fields[field]].pop( + record['id'], False) return result @api.model @@ -129,8 +135,12 @@ class RestrictFieldAccessMixin(models.AbstractModel): if not this._restrict_field_access_is_field_accessible( path[0], ) and row[i]: - row[i] = self._fields[path[0]].convert_to_export( - self._fields[path[0]].null(self.env), self.env + field = self._fields[path[0]] + row[i] = field.convert_to_export( + field.convert_to_cache( + field.null(self.env), this, validate=False, + ), + self.env ) result.extend(rows) return result @@ -189,7 +199,9 @@ class RestrictFieldAccessMixin(models.AbstractModel): cr, uid, view_id=view_id, view_type=view_type, context=context, toolbar=toolbar, submenu=submenu) - if view_type == 'search': + # TODO: for editable trees, we'll have to inject this into the + # form the editable list view creates on the fly + if view_type != 'form': return result # inject modifiers to make forbidden fields readonly @@ -260,14 +272,12 @@ class RestrictFieldAccessMixin(models.AbstractModel): @api.model def _restrict_field_access_suspend(self): """set a marker that we don't want to restrict field access""" - # TODO: this is insecure. in the end, we need something in the lines of - # base_suspend_security's uid-hack - return self.with_context(_restrict_field_access_suspend=True) + return self.suspend_security() @api.model def _restrict_field_access_get_is_suspended(self): """return True if we shouldn't check for field access restrictions""" - return self.env.context.get('_restrict_field_access_suspend') + return isinstance(self.env.uid, BaseSuspendSecurityUid) @api.multi def _restrict_field_access_filter_vals(self, vals, action='read'): @@ -288,10 +298,14 @@ class RestrictFieldAccessMixin(models.AbstractModel): """return True if the current user can perform specified action on all records in self. Override for your own logic. This function is also called with an empty recordset to get a list - of fields which are accessible unconditionally""" + of fields which are accessible unconditionally. + Note that this function is called *very* often. Even small things + like saying self.env.user.id instead of self.env.uid will give you a + massive performance penalty""" if self._restrict_field_access_get_is_suspended() or\ - self.env.user.id == SUPERUSER_ID or\ - not self and action == 'read': + self.env.uid == SUPERUSER_ID or\ + not self and action == 'read' and\ + self._fields[field_name].required: return True whitelist = self._restrict_field_access_get_field_whitelist( action=action) diff --git a/base_mixin_restrict_field_access/tests/test_base_mixin_restrict_field_access.py b/base_mixin_restrict_field_access/tests/test_base_mixin_restrict_field_access.py index 73d536e20..abef5b343 100644 --- a/base_mixin_restrict_field_access/tests/test_base_mixin_restrict_field_access.py +++ b/base_mixin_restrict_field_access/tests/test_base_mixin_restrict_field_access.py @@ -1,17 +1,50 @@ # -*- coding: utf-8 -*- # © 2016 Therp BV # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). +from openerp import api, models +from openerp.osv import expression from openerp.tests.common import TransactionCase -from openerp import models class TestBaseMixinRestrictFieldAccess(TransactionCase): def test_base_mixin_restrict_field_access(self): - # inherit from our mixin + # inherit from our mixin. Here we want to restrict access to + # all fields when the partner has a credit limit of less than 42 + # and the current user is not an admin class ResPartner(models.Model): _inherit = ['restrict.field.access.mixin', 'res.partner'] _name = 'res.partner' + # implement a record specific whitelist: credit limit is only + # visible for normal users if it's below 42 + @api.multi + def _restrict_field_access_is_field_accessible( + self, field_name, action='read' + ): + result = super(ResPartner, self)\ + ._restrict_field_access_is_field_accessible( + field_name, action=action + ) + if not self._restrict_field_access_get_is_suspended() and\ + not self.env.user.has_group('base.group_system') and\ + field_name not in models.MAGIC_COLUMNS and self: + result = all( + this.sudo().credit_limit < 42 for this in self + ) + return result + + # and as the documentation says, we need to add a domain to enforce + # this + def _restrict_field_access_inject_restrict_field_access_domain( + self, domain + ): + domain[:] = expression.AND([ + expression.normalize_domain(domain), + [('credit_limit', '<', 42)] + ]) + # call base-suspend_security's register hook + self.env['ir.rule']._register_hook() + # setup the model res_partner = ResPartner._build_model(self.registry, self.cr).browse( self.cr, self.uid, [], context={}) @@ -19,15 +52,55 @@ class TestBaseMixinRestrictFieldAccess(TransactionCase): res_partner._setup_base(False) res_partner._setup_fields() res_partner._setup_complete() + # run tests as nonprivileged user - partner = res_partner.sudo(self.env.ref('base.user_demo').id).create({ + partner_model = res_partner.sudo(self.env.ref('base.user_demo').id) + partner = partner_model.create({ 'name': 'testpartner', }) - partner.copy() - partner.write({ - 'name': 'testpartner2', - }) - partner.search([]) + self.assertFalse(partner.restrict_field_access) + partner.sudo().write({'credit_limit': 42}) + partner.invalidate_cache() self.assertTrue(partner.restrict_field_access) - partner.fields_view_get() - # TODO: a lot more tests + self.assertFalse(partner.credit_limit) + self.assertTrue(partner.sudo().credit_limit) + # not searching for some restricted field should yield the partner + self.assertIn(partner, partner_model.search([])) + # but searching for it should not + self.assertNotIn( + partner, + partner_model.search([ + ('credit_limit', '=', 42) + ]) + ) + # when we copy stuff, restricted fields should be copied, but still + # be inaccessible + new_partner = partner.copy() + self.assertFalse(new_partner.credit_limit) + self.assertTrue(new_partner.sudo().credit_limit) + # check that our field injection works + fields_view_get = partner.fields_view_get() + self.assertIn('restrict_field_access', fields_view_get['arch']) + # check that the export does null offending values + export = partner._BaseModel__export_rows([['id'], ['credit_limit']]) + self.assertEqual(export[0][1], '0.0') + # but that it does export the value when it's fine + partner.sudo().write({'credit_limit': 41}) + partner.invalidate_cache() + export = partner._BaseModel__export_rows([['id'], ['credit_limit']]) + self.assertEqual(export[0][1], '41.0') + # read_group should behave like search: restrict to records with our + # field accessible if a restricted field is requested, unrestricted + # otherwise + data = partner_model.read_group( + [], [], ['user_id'] + ) + self.assertEqual(data[0]['credit_limit'], 41) + # but users with permissions should see the sum for all records + data = partner_model.sudo().read_group( + [], [], ['user_id'] + ) + self.assertEqual( + data[0]['credit_limit'], + sum(partner_model.sudo().search([]).mapped('credit_limit')) + )