make group admin controls show if you are a group admin

[ryanlerch]

the init method of the MembershipValidator class previously had some lines in it that converted self.group to a list if it was of str or unicode type, rather than a list:

if type(group) is str or unicode:
    self.group = [group]
    # self.group.append(group)
if type(group) is list:
    self.group = group

Based on my limited knowledge of class inheritance, it appears that these conversions were overwritten my the init function in the RoleValidator class. So when the validate method in the MembershipValidator class tried to perform the intersections of the sets to see if the groupname was in the list of permitted groups, self.group was not a list anymore, and always returned False.

This commit moves the logic of converting str and unicode types into the validate method of MembershipValidator, and fixes the symptoms described in Issue #229

[ryanlerch]

thanks @pypingou i have updated that if statement to use the basestring superclass type

[ryanlerch]

@laxathom, I have already updated the PR to use isinstance, as requested by @pypjngou. Not sure I follow your additional request.

[laxathom]

I was referring to replace this

if type(group) is str or unicode: 
    self.group = [group]

if type(group) is list:
        self.group = group

by (as group can only be a basestring or a list)

if isinstance(group, basestring):
    self.group = [group]
else:
    self.group = group

So you avoid to write twice this below by altering the param:

if len(groups.intersection(set(self.group))) > 0:
    log.debug('Granting access')
    return True

[ryanlerch]

Note that

if type(group) is str or unicode: 
    self.group = [group]

if type(group) is list:
        self.group = group

is actually removed in the commit. but will change the commit to remove the duplication

[laxathom]

what duplication? From the diff I see this

if isinstance(self.group, basestring):
     if len(groups.intersection(set([self.group]))) > 0:
         log.debug('Granting access')
         return True
     elif type(self.group) is list:
         if len(groups.intersection(set(self.group))) > 0:
             log.debug('Granting access')
             return True

The idea is to have only one if statement that check the lenght of group, log the "granting access" and returns True.

[ryanlerch]

updated the commit to reduce the 2 lines of code duplication

[ryanlerch]

you asked me to replace:

if type(group) is str or unicode: 
    self.group = [group]

if type(group) is list:
    self.group = group

and it was removed entirely from the commit, and moved further down and reworked. I have now reworked it further to remove the duplication of the two lines, printing the log, and returning true.

[laxathom]

:+1: from me