HR Feature: built-in option to restrict visibility of employee attachments?

Bug #969198 reported by Alan Lord
32
This bug affects 5 people
Affects Status Importance Assigned to Milestone
Odoo Addons (MOVED TO GITHUB)
Confirmed
Wishlist
OpenERP R&D Addons Team 1

Bug Description

We are migrating a customer from 6.0 to 6.1. I raised this issue under their OpenERP Enterprise contract [573293] but the support team have asked me to report the bug here.

In the hr.employee module *any* other employee on the system can create, read or DELETE attachments on any other employee's main page. This occurs in both Web and GTK Clients.

In my opinion an Employee should be able to read *any* attachment on their own employee record only. They should be able to remove (delete) only those attachments which they themselves added.

The HR Manager (& possibly HR User) should be able to add, read and remove attachments from any employees.

Unfortunately, I do not believe this configuration is possible currently as the domain rules do not appear to have scope beyond a single object and the employee_id doesn't match their user_id. I think to achieve this you need to be able to read the res_id of the ir.attachment object then, if the res_model is hr.employee, get the user_id of the appropriate hr.employee record to match against.

I was trying to create an Access Rule like this:

[('user_id','=',user.id),('res_model','=','hr.employee'),('hr.employee[res_id].user_id','=',user.id)]

But of course it doesn't work.

security vulnerability: yes → no
visibility: private → public
Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

Hello Alan,

I'm not sure if this is a real bug or more of a specific customization that you want to add for certain companies...

You might be able to do it with a pair of Record Rules if you change your requirement to be: "employees can only see attachments that they created themselves, regardless". For example the normal employee group would have one rule to restrict access to employee attachments:
   ['|',('res_model','!=','hr.employee'),('user_id','=',user.id)]

and the HR Manager/HR Officer groups would have a Rule that cancels the normal rule for employees:
   [(1,'=',1)]

Now if you really need to have a special permission for your own employee attachments you probably need to extend the user model by adding an "employee_id" relationship that can be used to check for this special case:
  ['|',('res_model','!=','hr.employee'),'|',('user_id','=',user.id),('res_id','=',user.employee_id.id)]
If the "employee_id" field was automatically computed by looking for the only employee that matches the user, it would make everything quite simple. We might add such a field in the future indeed, as there are many cases where this "reverse" relationship would be useful.

Revision history for this message
Alan Lord (theopensourcerer) wrote :

Thanks for the suggestion but if I understand it correctly, it doesn't really fix the problem.

If the Employee can only view his own attachments then that won't work.

Think of the main use-case here...

The HR or your line Manager is the one who is just as likely to be adding attachments to their employees' profiles as the employee themselves (perhaps even more so), they could be various company documents and policies etc. not just the highly private ones that are stored in the Employee's Contracts area.

I think that right now, the way OpenERP configures attachments on an employee object is a bug. I mean, really would you expect that *anyone* in your company could come along and add or delete attachments to your Employee record?

I appreciate that I may have a marginally more specific use-case than everyone else, but nevertheless I believe that the bug is that attachments are totally insecure on an employee record.

Revision history for this message
Amit Parik (amit-parik) wrote : Re: Can not set a access rights on particular records. Currently we can set access rights based on a object

Hello Alan,

As you replied and reported a bug you have stated like this.

1)Employee Group: -Can read all employee's attachment.
                                 :- Can edit and delete only those attachment which is created by him/her.

2)HR/User or Manager Group : Can able to add, read and remove attachments from any employees.

Your 2nd point which is working fine with access rights of OpenERP and currently we can stratified this kind of visibility/security in OpenERP because in OpenERP we have provided a security in three different ways as follow.

1) Groups : It gives us a object(menu) and action(button) based visibility (Either this group can seen or not).
2) Access rights : Create , read, write, delete (all access) access based on object, It means that either group can read all the record or can not read any of the record.

3) Record Rule : Which is most important, We can give visibility based on record, means by using record rules we can set security like groups can see only particular record. If you have created a record rule then you can seen only those record which will satisfied this domain. So also by using record rule we can not set a access right on particular record.

As per your requirement you need a particular rights on your particular record. Above all three option will not solve your issue.

This issue doesn't affect only for attachment, It will apply as a generic way also we can not say this as a bug rather than it's good improvement.

We have to consider this as a feature request and implement this type of security in feature which will solve many this type of issues. That's why I am considering this as a wishlist and as a generic way assign to the sever side.

Thank you!

summary: - Any Employee has full CRUD of every other Employee's Attachments
+ Can not set a access rights on particular records. Currently we can set
+ access rights based on a object
affects: openobject-addons → openobject-server
Changed in openobject-server:
assignee: nobody → OpenERP's Framework R&D (openerp-dev-framework)
importance: Undecided → Wishlist
status: New → Confirmed
Revision history for this message
Amit Parik (amit-parik) wrote :

Hello,

Please look in to lp:983018 which is provided a good suggestion and this suggestion helps to improve this.

So I request to framework team before going to fix please look lp:983018 's suggestion.

Thank you!

Revision history for this message
Alan Lord (theopensourcerer) wrote :

Hi,

I suspect that the suggestion mentioned in #4 would be one way to do it but I not totally sure I understand the proposal.

I have spent some more time looking at this today and still can't see an easy workaround to stop employees having full CRUD on every employees' attachments.

There are required fields missing from ir.attachment...

Odony's suggestion in #1 of ['|',('res_model','!=','hr.employee'),('user_id','=',user.id)] doesn't work because there is no "user_id" value in ir.attachment. The only similar field is the "create_uid" but this is the user id of the person who *added* the attachment in the first place (i.e. not necessarily the employee to which the attachment belongs).

I still feel that this is a bug.

As it stands right now, *any* employee can read, create and delete attachments from *any* other employee. Surely that is a bug?

Revision history for this message
Amit Parik (amit-parik) wrote :

Hello Alan,

I think you misunderstood my comment. First of don't try again with record rule because record rule provides visibility based on particular record, So by using record rule either you are able to see the record or not.

Now comes to comment#4, I have suggested to take care about lp:983018 's suggestion because it's the easy way to do this things.
Also this is not applied to employee object only but it's generalise way./

Again here I want to say that currently this kind of facility/feature doesn't available in OpenERP. So will be consider this on feature roadmaps.

So don't hesitate on this issue.

Thank you!

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

I think this deserves a status update:

The default access control system for attachments mirrors exactly the access rights of the record on which they are attached. This means a user can view an attachment only if they can read the record, delete an attachment only if they have the right to delete the record, and add an attachment only if they have the right to create new records of the same kind). This is the behavior implemented in the framework itself, and has been like this since OpenERP 6.0.

This behavior changes when the `document` (DMS) module is installed, because it introduces a transversal view for attachments, based on a filesystem-like directory hierarchy. Attachments can then be filed into directories with their own access control, and the access control of the directories applies in addition to the default access control.

Based on the above and the default access control for Employee records, any employee should by default be able to view the attachments on any other employee record they can access, but they should not be able to add or delete attachments, only HR Officers/Managers would be able to do that.
There was previously a bug in the `document` module that broke this logic and allowed adding and deleting attachments as soon as you were able to read a record. This was fixed a while ago in OpenERP 7.0 [1], and I have just tested it on runbot - seems to work fine.

The missing part in this default behavior is that you would like to completely prevent normal employees from even viewing the attachments on other employee records. This is currently not built-in but can be done using an extra ir.rule filter, similarly to what I suggested in comment #1. In OpenERP 7.0 the HR module adds an "employee_ids" relationship on user records, so the rule could be as follows:
  ['|',('res_model','!=','hr.employee'),('res_id','in',[e.id for e in user.employee_ids])]
If you add this rule to the "Employee" group (everyone), you will presumably need to add a reciprocal rule on the "HR Officer" in order to re-authorize them to access all employee attachments, as follows:
  [('res_model','=','hr.employee')]

I have just tested the combination of these two ir.rules on 7.0 and it seems to work as intended.

The question remains: should this combination of 2 rules become a built-in option in the HR Settings, or should it remain an extra customization? If this is a frequent feature request we could consider adding it.
Meanwhile I think the above explanation will mitigate the original bug report.

Note: re-targetting the bug to `addons` as this is mainly a concern of the HR module (and was never really a server bug)

[1] revision 9306 revid:<email address hidden>

affects: openobject-server → openobject-addons
Changed in openobject-addons:
assignee: OpenERP's Framework R&D (openerp-dev-framework) → OpenERP R&D Addons Team 1 (openerp-dev-addons1)
summary: - Can not set a access rights on particular records. Currently we can set
- access rights based on a object
+ HR Feature: built-in option to restrict visibility of employee
+ attachments?
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.