ACL with GlideRecord

[JohnnySnow]

Hi Team,

I'm writing an ACL and had to use GlideRecord, I have read that it is not a best practice to use GR in

the ACL scripts, but I'm not sure how to handle this scenario. Can some one please guide?

answer = false;
var assignmentGroups;
var groupArray = [];


if (current) {

    assignmentGroups = current.getValue('related_assignment_groups').toString();// a list field which holds all the related assignment groups worked on that ticket
    groupArray = assignmentGroups.split(',');

//To check if the current user is a manager of any of the related groups if yes, then show the record
    var groupGR = new GlideRecord('sys_user_group');
    groupGR.addQuery('manager', gs.getUserID());
    groupGR.addQuery('sys_id', 'IN', groupArray);
    groupGR.query();

    if (groupGR.next()) {
        answer = true;
    } else {

		//to check if the user is a member of any of the related groups list.
        answer = groupArray.some(function(group) {
            return gs.getUser().isMemberOf(group.trim());
        });
    }
  

}

[Maik Skoddow]

Hi @JohnnySnow ,

pretty good question and there are not many users/admins/developers who are concerned about the performance of ACLs!

I have done extensive research into the performance of ACLs and have come to the conclusion that, as always, there is no "good" or "bad". ServiceNow itself uses GlideRecord queries very intensively in its own ACLs.

If have two optimizations:

1. After
   

   groupGR.addQuery('sys_id', 'IN', groupArray);​

   insert
   

   groupGR.setLimit(1);

   as you only need one result row. This is a common best practice and not only related to ACLs
2. In the else branch, you are performing for each group individually a:

   return gs.getUser().isMemberOf(group.trim())​

   I'm not sure whether ServiceNow is caching group memberships of users and you could think about to use the same approach as before with a GlideRecord query to check the membership on table sys_user_grmember.

Maik

[JohnnySnow]

@Maik Skoddow Thanks for the reply, I didnt  quite understand the 2nd point. Are you asking me to write a before BR on sys_user_grmember? Can you please elaborate?

[JohnnySnow]

I'm not sure if you meant this, but now instead of 2 different queries, I'm using 1 query as below (hopefully better than the previous code)

answer = false;
var assignmentGroups;
var groupArray = [];

if (current) {
    assignmentGroups = current.getValue('related_assignment_groups').toString();
    groupArray = assignmentGroups.split(',');

    var grpmemGR = new GlideRecord('sys_user_grmember');
    var qc = grpmemGR.addQuery('group.manager', gs.getUserID());
    qc.addOrCondition('user', gs.getUserID());
    grpmemGR.addQuery('group', 'IN', groupArray);
    grpmemGR.setLimit(1);
    grpmemGR.query();
    if (grpmemGR.next()) {
        answer = true;
    }


}

[Maik Skoddow]

Hi @JohnnySnow 

I am delighted as this is a great solution!

If all your business tests are passing, you can take this.

Last and minor optimization: There is no need to split the assignment groups into an array. You can pass the comma-separated string directly to the GlideRecord query:

assignmentGroups = current.getValue('related_assignment_groups') || '';

if (assignmentGroups.length > 0) {
    var grpmemGR = new GlideRecord('sys_user_grmember');
    var qc = grpmemGR.addQuery('group.manager', gs.getUserID());

    qc.addOrCondition('user', gs.getUserID());
    grpmemGR.addQuery('group', 'IN', assignmentGroups );
    grpmemGR.setLimit(1);
    grpmemGR.query();

    if (grpmemGR.next()) {
        answer = true;
    }
}

Hint: 

The

|| ''

at the end of the first line makes sure that in case there is no value in related_assignment_groups, at least an empty string is returned. That way, it is possible (and required!) to test if related_assignment_groups has a value or not. 

Maik