dataall
dataall copied to clipboard
In Admin settings Teams Permission Dialog Opens Blank
In Admin settings page -> Teams -> Click "Permissions" => Blank Page Opens.
It should have opened, Tenant Permissions list, where "Manage ----" set of permissions for team can be set.
When we checked the API response, Noticed a "null," attribute (Refer below snippet: Between Manage Datasets and Worksheets)
Appreciate your suggestion to Resolve this issue
{
"data": {
"listTenantPermissions": [
{
"name": "MANAGE_GROUPS",
"description": "Manage teams",
"__typename": "Permission"
},
{
"name": "MANAGE_ENVIRONMENTS",
"description": "Manage environments",
"__typename": "Permission"
},
{
"name": "MANAGE_ORGANIZATIONS",
"description": "Manage organizations",
"__typename": "Permission"
},
{
"name": "MANAGE_GLOSSARIES",
"description": "Manage glossaries",
"__typename": "Permission"
},
{
"name": "MANAGE_DATASETS",
"description": "Manage datasets",
"__typename": "Permission"
},
null,
{
"name": "MANAGE_WORKSHEETS",
"description": "Manage worksheets",
"__typename": "Permission"
}
]
}
}
Hi @sandeephs1, thanks for opening an issue. I have tried to reproduce your issue but I do not get the null
. From what I see, you do not have all modules active, so maybe the issue is there. Is there any module that should be there and is not?
The code that lists the permissions is in dataall/core/permissions/db/tenant_policy_repositories.py
My guess is that the functionfind_permission_by_name
is returning null in one case and it is appending it.
@staticmethod
def list_tenant_permissions(session, username, groups):
if not TenantPolicy.is_tenant_admin(groups):
raise exceptions.UnauthorizedOperation(
action='LIST_TENANT_TEAM_PERMISSIONS',
message=f'User: {username} is not allowed to manage tenant permissions',
)
group_invitation_permissions = []
for p in permissions.TENANT_ALL:
group_invitation_permissions.append(
Permission.find_permission_by_name(
session=session,
permission_name=p,
permission_type=PermissionType.TENANT.name,
)
)
return group_invitation_permissions
If we take a look at the find_permission_by_name
function we see that it queries the Permission
table. The next step would be to compare the permission that is giving the null (from a module that is enabled?) and then try to find it in the RDS table.
We will debug again tomorrow, if you are still blocked we can schedule a meeting
Thanks @dlpzx yes "MANAGE_DASHBOARDS" was missing, which was enabled recently. Updated type in "permission" table. it resolved the issue
I think this issue will be solved by #1145 @petrkalos can you confirm?
@dlpzx I just checked and indeed the issue will be fixed by #1145. To test it I first reproduced the issue (I removed the rows manually and got the blank screen) and then re-executed the lambda and it started working again.
Nevertheless, I wonder if the logic in list_tenant_permissions
is correct. Why do we iterate over TENANT_ALL
instead of querying what already exists in the database, i.e use find_permission_by_type(permission_type=PermissionType.TENANT.name)
?
Good point @petrkalos, I think that it checks it this way because TENANT_ALL
contains all ACTIVE modules tenant permissions and then it goes and finds the permission in RDS. If we just filter from RDS we would get all permissions in RDS independently from whether the module is active or not. We could do it the other way around, list the RDS permissions and check which ones are in TENANT_ALL, but I prefer the error to show that there are missing permissions in RDS
@staticmethod
def list_tenant_permissions():
context = get_context()
username = context.username
groups = context.groups
TenantPolicyValidationService.validate_admin_access(username, groups, 'LIST_TENANT_TEAM_PERMISSIONS')
group_invitation_permissions = []
with context.db_engine.scoped_session() as session:
for p in TENANT_ALL:
group_invitation_permissions.append(
PermissionRepository.find_permission_by_name(
session=session,
permission_name=p,
permission_type=PermissionType.TENANT.name,
)
)
return group_invitation_permissions
It doesn't feel like this is the correct place to check if the app permissions are in sync with the DB and vice versa. I think we must trust that the DB is the ultimate source of truth.
Then we need to delete the permissions for the disabled modules whenever a module is disabled, which is a bit scary. What do you think?
Indeed, that's complicating matters more. I guess so far when we disable a module it doesn't mean that we are also wiping the data that it generated when it was active, right?
Exactly, that is the current behavior. Although cleaning up the RDS database looks good in theory I am afraid that in real life it could happen that someone disables a module by mistake and that could be a bit catastrophical - many ghost resources created by data.all and not visible in data.all.
I'll close this item as complete, if in the future we want to revisit the way we initialize permissions it can be reopened