ACL for HR Case
- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
‎04-30-2015 11:12 AM
We are currently implementing HR Service Management and have a requirement where an assignment group can restrict case access to their assignment group only.
I have achieved this by adding a new field on the form 'Restrict Case Access' and a new read ACL rule:
Everything appears to work as expected, however, my question is, should I be using 'hr_Factory.getSecurityManager(current, gs).canRead()' as many of the other HR ACL rules appear to be using - after reading the wiki I am confused on how to work with these custom wrappers.
Am i creating issue for myself by not using the wrappers?
- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
‎04-30-2015 03:14 PM
Good question! My perspective is that the hr_factory in the ACL's (and also the corresponding before_query business rule) obfuscates rules that are normally easy to interpret. I often disable the hr_factory ACL and write my own rules in a script, similar to what you have done. Here are some examples:
Example 1: Tier 3 Agents Can Only See Cases Assigned to Them
Note: In this example, the order of this script evaluations is important. The Tier 3 agent will have the hr_reader role, but the script evaluates to false before that condiction if the person is in HR — Tier 3 and the case is not assigned to them. But for agents not in HR — Tier 3, they can still see all cases.
answer = canReadSupervisorCases();
function canReadSupervisorCases() {
if(gs.getUserID() == current.opened_by) {
return true;
}
else if(gs.getUser().isMemberOf('HR - Tier 3') && gs.getUserID() == current.assigned_to) {
return true;
} else if(gs.getUser().isMemberOf('HR - Tier 3') && gs.getUserID() != current.assigned_to) {
return false;
}
else if(gs.hasRole("hr_case_reader")) {
return true;
} else {
return false;
}
}
Example 2: Submitter can always see their cases. HR Leaders can view cases submitted by supervisors or HR personnel where they are not flagged as private.
Note: In this example, we also used the condition builder during the demo to show how you can set u_private to false, as shown below.
Script:
answer = canReadSupervisorCases();
function canReadSupervisorCases() {
var isMember = 'false';
var grp = new GlideRecord('sys_user_group');
grp.addQuery('name', 'HR_Supervisors');
grp.query();
grp.next();
var gr = new GlideRecord('sys_user_grmember');
gr.addQuery('user', current.opened_by);
gr.addQuery('group', grp.sys_id);
gr.query();
if (gr.next()) {
isMember = 'true';
}
if(gs.getUserID() == current.opened_by) {
return true;
} else if(gs.getUser().isMemberOf('HR_Leadership') && isMember == 'true') {
return true;
}
else if(gs.hasRole("hr_case_reader") && isMember == 'false') {
return true;
} else {
return false;
}
}
- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
‎05-08-2015 07:20 AM
Hi,
I would say yes you are causing potential issues for yourself by putting the code directly within the ACL rather than using the factory (and associated classes).
It is good to have your security logic in one place, rather than mixed in different ACLs. I'm guessing you might want to restrict the sys_email table and the relevant emails based on the same security, you're going to need to copy and paste this code there.
In addition to this, you're using a hr_case.* ACL. That means any ACL on task that you need to override to do the same security as the .* ACL, you'll need to copy the code there.
WHat happens afterwards if there's a bug in the code, or if you want to update the security logic? You'll need to go through the system looking for all the identical code and update it.
Using the factory, you have one place, fix/change it once, and it'll do it everywhere
It does take a while to get your head around the layered architecture, but it is really really useful and powerful when done correctly
- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
‎05-08-2015 07:54 AM
Hi Ahmed Hmeid,
Would you be able to provide an example, using my ACL above with the layered architecture of the wrapper.
It feels the more I look at the wrappers and wiki, the more confused I get
- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
‎05-08-2015 08:32 AM
Sure - I'll try my best to describe here - it's a difficult topic... this is going to be a long one
So the idea is that the factory has the ability to pick a custom class to use for the ACL in this instance. In addition to this, hr_factory is the only out of the box script include you should edit, the rest of the script includes should be extended if needing to edit them (i'll get to that).
Working through the OOB functions, the one script that's called is:
hr_Factory.getSecurityManager(current, gs).canRead()
So this calls the function getSecurityManager within the hr_factory script include.
Looking at this function, it then calls another script include called hr_BaseFactory with the same function call.
This function has a switch statement in, based on the table name that's being passed in from the initial gliderecord, it can will return a new instance of a particular class.
So in the instance of the hr_case ACL, hr_factory.getSecurityManager(current,gs) actually is the same as writing:
new hr_CaseSecurityManager(current,gs)
So then to see the 'canRead' function, you can go to the hr_caseSecurityManager script and look at the canRead function from there.
Hopefully you can already see the benefit, that the same line of code, hr_factory.getSecurityManager(current,gs), can now be called from multiple different places and actually provide completely different logic if triggered from the hr_case table compared to the hr_task table etc.
So how to edit the out of the box functions, in your example, you don't want to use the canRead functionality within the hr_caseSecurityManager.
First of all, you need to extend the hr_caseSecurityManager script, to do this, create a new script include, lets called it hr_caseSecurityManager_custom. Copy and paste the below script into the script include:
var hr_caseSecurityManager_custom = Class.create();
hr_caseSecurityManager_custom.prototype = Object.extendsObject(hr_caseSecurityManager , {
});
The hr_caseSecurityManager_custom script include now extends the hr_caseSecurityManager script include. What this means is by default it gets all of the same functions/properties and the original class.
So 'new hr_caseSecurityManager_custom(current,gs).canRead()' would return an identical result to 'new hr_caseSecurityManager(current,gs).canRead()'
If you want the new script include to have a different can read function, you just override it by putting it within the extension class such as:
var hr_caseSecurityManager_custom = Class.create();
hr_caseSecurityManager_custom.prototype = Object.extendsObject(hr_caseSecurityManager , {
canRead : function() {
return gs.hasRole('hr_case_reader');
}
});
That's it, it now has all the same functions as the out of the box script include, except for canRead which has it's own logic.
So the next step is to get this script include called instead of the original one. To do this, edit the hr_factory directly. Change:
hr_Factory.getSecurityManager = function(_gr, _gs) {
return hr_BaseFactory.getSecurityManager(_gr, _gs);
};
to this:
hr_Factory.getSecurityManager = function(_gr, _gs) {
if (_gr.getTableName() == 'hr_case') {
return new hr_caseSecurityManager_custom(_gr,_gs);
}
return hr_BaseFactory.getSecurityManager(_gr, _gs);
};
Now for hr_case records, it'll call the new script include, for everything else will continue as normal.
May seem like a complicated approach and longer, but the payoffs in the long run for maintenance and upgrades are very worth it. For upgrades, because the out of the box script includes (apart from hr_factory) have not been touched, they will all be upgraded and any new functionality, will be added to the parent classes and any amendments you've made on the extension will still remain.
Hope that makes sense