Modify `has_read_privs` to return false if user is not a member of the default read group

[maxsibilla]

commons/hubmap_commons/hm_auth.py

Lines 346 to 355 in 3c747f1

	#check to see if a user has read privileges
	#the user has read privileges if they are a member of the
	#default read group or if they have write privileges at all per the above has_write_privs method
	def has_read_privs(self, groups_token):
	user_info = self.getUserInfo(groups_token, getGroups = True)
	if isinstance(user_info, Response):
	return user_info
	if not self.data_read_group_uuid is None and self.data_read_group_uuid in user_info['hmgroupids']:
	return True
	return self.has_write_privs(groups_token)

Users should always be a part of the default read group so we need to be able to flag those who have access via Globus but are not yet a part of the default read group. We have run into issues with functionality in the SenNet portal with this exact use-case.

[maxsibilla]

@yuanzhou do you foresee any issues with making a change like this?
Tagging @shirey for reference.

[yuanzhou]

@maxsibilla are you suggesting to return a boolean False rather than the user_info when it's a response object?

if isinstance(user_info, Response): 
    # return user_info
    return False

I guess the reason @shirey returned user_info instead of False is because it returns response with message to the end users that the token is invalid/inactive. One example: https://github.com/hubmapconsortium/commons/blob/main/hubmap_commons/hm_auth.py#L61-L63

If we modify it to just return False, I'm afraid

• we'll lose the invalid token context/message,
• and will need to handle it additionally when the member belongs to the READ group but the token has expired.

If we agree to modify,

• then we'll need to change the ingest-api code for both HuBMAP and SenNet that handles the data-ingest-board login, which may potentially require the ingest board UI to tweak a bit to handle the False as well
• the API Gateway lambda authorizers for both HuBMAP and SenNet use this method, no change needed because they handle everything other than True as unauthorized
• confirm with other developers (PSC) in case they may use this method for something else

[maxsibilla]

My proposal was simply to have this return false if the user is not part of the default read group. I don't know if we should have this line:

     return self.has_write_privs(groups_token) 

This all stems from a conversation I had with @shirey that users should always be a part of the default read group.

The new code would look like this:

    def has_read_privs(self, groups_token):
        user_info = self.getUserInfo(groups_token, getGroups = True)
        if isinstance(user_info, Response):
            return user_info
        if not self.data_read_group_uuid is None and self.data_read_group_uuid in user_info['hmgroupids']:
            return True
        return False

[yuanzhou]

Using return self.has_write_privs(groups_token) at the last line assumes that all the members of data provider groups have READ priv automatically, even if the members are only added to the globus data provider groups but not the READ group. Making this code change would require @shirey to go through all the individual members of each data provider group and ensure they are also in the READ group explicitly, for both HuBMAP and SenNet.

[maxsibilla]

@yuanzhou Thank you for that clarification. I think then maybe we just want a new function that only checks if the user is a part of the default read group. @shirey what do you think?

[maxsibilla]

I did some more digging and it looks like this is what is happening:

• We have some users that are a part of a data provider group BUT are not a member of the default read group
• Because they are a member of the data provider group, hm_auth acknowledges that they have both read and write access
• The users are able to edit entities in the portal but the reindexing of these entities fails because the Search API calls the Entity API /entities/<uuid> GET endpoint, which does a raw check if the user is a member of the default read group:

user_in_globus_read_group(request)

• The Search API /reindex call then has a cascading failure with a 403 error