瀏覽代碼

[useradmin] Assign filebrowser permissions to the default group (#4223)

- This commit automatically grants File Browser permissions to the default user group.
- This simplifies the setup process by ensuring that users in the default group have immediate access to the File Browser without requiring manual permission adjustments.

- If the users who are getting this access in the fresh installation or after upgrade want previous experience back, then the Hue admins can change these group permissions from the "Administer User" section again.
Harsh Gupta 3 月之前
父節點
當前提交
58afec66fe

+ 18 - 22
apps/useradmin/src/useradmin/models.py

@@ -37,15 +37,11 @@ Note that Django itself has a notion of users, groups, and permissions. We re-us
 notion of permissions. The permissions notion in Django is strongly tied to what models you may or may not edit, and there are
 elaborations to manipulate this row by row. This does not map nicely onto actions which may not relate to database models.
 """
-import sys
+import collections
 import json
 import logging
-import collections
-from datetime import datetime
 from enum import Enum
 
-from django.contrib.auth import models as auth_models
-from django.contrib.auth.models import AbstractUser, BaseUserManager
 from django.core.cache import cache
 from django.db import connection, models, transaction
 from django.utils import timezone as dtz
@@ -53,15 +49,14 @@ from django.utils.translation import gettext_lazy as _t
 
 from desktop import appmanager
 from desktop.conf import ENABLE_CONNECTORS, ENABLE_ORGANIZATIONS
-from desktop.lib.connectors.models import Connector, _get_installed_connectors
+from desktop.lib.connectors.models import _get_installed_connectors, Connector
 from desktop.lib.exceptions_renderable import PopupException
-from desktop.lib.idbroker.conf import is_idbroker_enabled
 from desktop.monkey_patches import monkey_patch_username_validator
 from useradmin.conf import DEFAULT_USER_GROUP
-from useradmin.permissions import GroupPermission, HuePermission, LdapGroup
+from useradmin.permissions import GroupPermission, HuePermission, LdapGroup  # noqa: F401
 
 if ENABLE_ORGANIZATIONS.get():
-  from useradmin.organization import Organization, OrganizationGroup as Group, OrganizationUser as User, get_organization
+  from useradmin.organization import get_organization, Organization, OrganizationGroup as Group, OrganizationUser as User
 else:
   from django.contrib.auth.models import Group, User
   def get_organization(): pass
@@ -157,7 +152,7 @@ def get_profile(user):
     # Lazily create profile.
     try:
       profile = UserProfile.objects.get(user=user)
-    except UserProfile.DoesNotExist as e:
+    except UserProfile.DoesNotExist:
       profile = create_profile_for_user(user)
     user._cached_userman_profile = profile
     return profile
@@ -285,17 +280,18 @@ def update_app_permissions(**kwargs):
     default_group = get_default_user_group()
     if default_group:
       for new_dp in added:
-        if not (new_dp.app == 'useradmin' and new_dp.action == 'access') and \
-            not (new_dp.app == 'useradmin' and new_dp.action == 'superuser') and \
-            not (new_dp.app == 'metastore' and new_dp.action == 'write') and \
-            not (new_dp.app == 'hbase' and new_dp.action == 'write') and \
-            not (new_dp.app == 'security' and new_dp.action == 'impersonate') and \
-            not (new_dp.app == 'filebrowser' and new_dp.action == 's3_access' and not is_idbroker_enabled('s3a')) and \
-            not (new_dp.app == 'filebrowser' and new_dp.action == 'gs_access' and not is_idbroker_enabled('gs')) and \
-            not (new_dp.app == 'filebrowser' and new_dp.action == 'adls_access') and \
-            not (new_dp.app == 'filebrowser' and new_dp.action == 'abfs_access') and \
-            not (new_dp.app == 'filebrowser' and new_dp.action == 'ofs_access') and \
-            not (new_dp.app == 'oozie' and new_dp.action == 'disable_editor_access'):
+        # Start with the list of permissions that should ALWAYS be blacklisted.
+        blacklist_conditions = [
+          (new_dp.app == "useradmin" and new_dp.action == "access"),
+          (new_dp.app == "useradmin" and new_dp.action == "superuser"),
+          (new_dp.app == "metastore" and new_dp.action == "write"),
+          (new_dp.app == "hbase" and new_dp.action == "write"),
+          (new_dp.app == "security" and new_dp.action == "impersonate"),
+          (new_dp.app == "oozie" and new_dp.action == "disable_editor_access"),
+        ]
+
+        # If the current permission does not match ANY of the blacklist conditions, grant it.
+        if not any(blacklist_conditions):
           GroupPermission.objects.create(group=default_group, hue_permission=new_dp)
 
     available = HuePermission.objects.count()
@@ -317,7 +313,7 @@ def install_sample_user(django_user=None):
   """
   Setup the de-activated sample user with a certain id. Do not create a user profile.
   """
-  from desktop.models import SAMPLE_USER_ID, get_sample_user_install
+  from desktop.models import get_sample_user_install, SAMPLE_USER_ID
   from hadoop import cluster
 
   user = None

+ 24 - 31
desktop/core/src/desktop/lib/fs/proxyfs_test.py

@@ -21,7 +21,7 @@ import pytest
 
 from desktop.lib.django_test_util import make_logged_in_client
 from desktop.lib.fs import ProxyFS
-from desktop.lib.test_utils import add_permission, remove_from_group
+from desktop.lib.test_utils import add_permission, revoke_permission
 from useradmin.models import User
 
 
@@ -140,7 +140,7 @@ class MockFs(object):
 class TestFsPermissions(object):
 
   def test_fs_permissions_regular_user(self):
-    user_client = make_logged_in_client(username='test', groupname='default', recreate=True, is_superuser=False)
+    make_logged_in_client(username='test', groupname='default', recreate=True, is_superuser=False)
     user = User.objects.get(username='test')
 
     s3fs, adls, hdfs = MockFs("s3_access"), MockFs("adls_access"), MockFs()
@@ -153,13 +153,13 @@ class TestFsPermissions(object):
 
     f = proxy_fs._get_fs
 
-    remove_from_group(user.username, 'has_s3')
-    remove_from_group(user.username, 'has_adls')
-    remove_from_group(user.username, 'has_abfs')
-    remove_from_group(user.username, 'has_gs')
-    remove_from_group(user.username, 'has_ofs')
+    # FS permissions are present by default, so we'll revoke them firstand check if the exception is raised
+    revoke_permission('default', 'filebrowser', 's3_access')
+    revoke_permission('default', 'filebrowser', 'adls_access')
+    revoke_permission('default', 'filebrowser', 'abfs_access')
+    revoke_permission('default', 'filebrowser', 'gs_access')
+    revoke_permission('default', 'filebrowser', 'ofs_access')
 
-    # No perms by default
     with pytest.raises(Exception):
       f('s3a://bucket')
     with pytest.raises(Exception):
@@ -177,32 +177,25 @@ class TestFsPermissions(object):
     f('hdfs://path')
     f('/tmp')
 
-    try:
-      # Add perm
-      add_permission('test', 'has_s3', permname='s3_access', appname='filebrowser')
-      add_permission('test', 'has_adls', permname='adls_access', appname='filebrowser')
-      add_permission('test', 'has_abfs', permname='abfs_access', appname='filebrowser')
-      add_permission('test', 'has_gs', permname='gs_access', appname='filebrowser')
-      add_permission('test', 'has_ofs', permname='ofs_access', appname='filebrowser')
+    # Then add the perms and check if the exception is not raised
+    add_permission('test', 'has_s3', permname='s3_access', appname='filebrowser')
+    add_permission('test', 'has_adls', permname='adls_access', appname='filebrowser')
+    add_permission('test', 'has_abfs', permname='abfs_access', appname='filebrowser')
+    add_permission('test', 'has_gs', permname='gs_access', appname='filebrowser')
+    add_permission('test', 'has_ofs', permname='ofs_access', appname='filebrowser')
 
-      f('s3a://bucket')
-      f('S3A://bucket/key')
-      f('adl://net/key')
-      f('adl:/key')
-      f('abfs:/key')
-      f('hdfs://path')
-      f('/tmp')
-      f('gs://bucket')
-      f('ofs://volume/bucket/key')
-    finally:
-      remove_from_group(user.username, 'has_s3')
-      remove_from_group(user.username, 'has_adls')
-      remove_from_group(user.username, 'has_abfs')
-      remove_from_group(user.username, 'has_gs')
-      remove_from_group(user.username, 'has_ofs')
+    f('s3a://bucket')
+    f('S3A://bucket/key')
+    f('adl://net/key')
+    f('adl:/key')
+    f('abfs:/key')
+    f('hdfs://path')
+    f('/tmp')
+    f('gs://bucket')
+    f('ofs://volume/bucket/key')
 
   def test_fs_permissions_admin_user(self):
-    user_client = make_logged_in_client(username='admin', groupname='default', recreate=True, is_superuser=True)
+    make_logged_in_client(username='admin', groupname='default', recreate=True, is_superuser=True)
     user = User.objects.get(username='admin')
 
     s3fs, adls, hdfs = MockFs("s3_access"), MockFs("adls_access"), MockFs()

+ 21 - 11
desktop/libs/aws/src/aws/conf_tests.py

@@ -16,12 +16,12 @@
 
 import logging
 
-import pytest
 from django.test import TestCase
 
 from aws import conf
 from desktop.conf import RAZ
 from desktop.lib.django_test_util import make_logged_in_client
+from desktop.lib.test_utils import add_permission, revoke_permission
 from useradmin.models import User
 
 LOG = logging.getLogger()
@@ -83,7 +83,7 @@ class TestAWSConf(TestCase):
       conf.clear_cache()
 
   def test_has_s3_access(self):
-    # When RAZ is not enabled
+    # When user is not admin, RAZ is not enabled and only s3 perms are already present in the "default" group
     resets = [
       RAZ.IS_ENABLED.set_for_testing(False),
       conf.AWS_ACCOUNTS.set_for_testing({'default': {
@@ -93,40 +93,50 @@ class TestAWSConf(TestCase):
       }})
     ]
     try:
-      assert not conf.has_s3_access(self.user)
+      assert conf.has_s3_access(self.user)
     finally:
       for reset in resets:
         reset()
       conf.clear_cache()
 
-    # When only RAZ is enabled (S3 in Azure cluster)
+    # When user is not admin, RAZ is enabled along with S3 config and s3 perms are not present in the "default" group
     resets = [
       RAZ.IS_ENABLED.set_for_testing(True),
       conf.AWS_ACCOUNTS.set_for_testing({'default': {
-        'region': None,
-        'host': None,
+        'region': 'us-west-2',
+        'host': 's3-us-west-2.amazonaws.com',
         'allow_environment_credentials': 'false'
       }})
     ]
     try:
-      assert not conf.has_s3_access(self.user)
+      revoke_permission('default', 'filebrowser', 's3_access')
+      assert conf.has_s3_access(self.user)
     finally:
+      add_permission('test_user', 'default', 's3_access', 'filebrowser')
       for reset in resets:
         reset()
       conf.clear_cache()
 
-    # When RAZ is enabled along with S3 config
+    # When only user is admin, RAZ is not enabled along with S3 config but s3 perms are not present in the "default" group
     resets = [
-      RAZ.IS_ENABLED.set_for_testing(True),
+      RAZ.IS_ENABLED.set_for_testing(False),
       conf.AWS_ACCOUNTS.set_for_testing({'default': {
-        'region': 'us-west-2',
-        'host': 's3-us-west-2.amazonaws.com',
+        'region': None,
+        'host': None,
         'allow_environment_credentials': 'false'
       }})
     ]
     try:
+      revoke_permission('default', 'filebrowser', 's3_access')
+      self.user.is_superuser = True
+      self.user.save()
       assert conf.has_s3_access(self.user)
     finally:
+      add_permission('test_user', 'default', 's3_access', 'filebrowser')
+
+      self.user.is_superuser = False
+      self.user.save()
+
       for reset in resets:
         reset()
       conf.clear_cache()