fix linting

Author: labrenbe

superset/Dockerfile

@@ -18,6 +18,8 @@ RUN microdnf update \
     rm -rf /var/cache/yum
 
 RUN pip install \
+    --no-cache-dir \
+    --upgrade \
     poetry \
     pytest
 

superset/stackable/opa-authorizer/opa_authorizer/opa_manager.py

@@ -17,26 +17,24 @@ class OpaSupersetSecurityManager(SupersetSecurityManager):
     """
     Custom security manager that syncs user-role mappings from Open Policy Agent to Superset.
     """
-    AUTH_OPA_CACHE_MAXSIZE_DEFAULT=1000
-    AUTH_OPA_CACHE_TTL_IN_SEC_DEFAULT=30
-    AUTH_OPA_REQUEST_URL_DEFAULT='http://opa:8081/'
+
+    AUTH_OPA_CACHE_MAXSIZE_DEFAULT = 1000
+    AUTH_OPA_CACHE_TTL_IN_SEC_DEFAULT = 30
+    AUTH_OPA_REQUEST_URL_DEFAULT = "http://opa:8081/"
 
     def __init__(self, appbuilder):
         self.appbuilder = appbuilder
         super().__init__(self.appbuilder)
         config = self.appbuilder.get_app.config
         self.opa_cache = self.opa_cache = TTLCache(
             maxsize=config.get(
-                'AUTH_OPA_CACHE_MAXSIZE',
-                self.AUTH_OPA_CACHE_MAXSIZE_DEFAULT
+                "AUTH_OPA_CACHE_MAXSIZE", self.AUTH_OPA_CACHE_MAXSIZE_DEFAULT
             ),
             ttl=config.get(
-                'AUTH_OPA_CACHE_TTL_IN_SEC',
-                self.AUTH_OPA_CACHE_TTL_IN_SEC_DEFAULT
+                "AUTH_OPA_CACHE_TTL_IN_SEC", self.AUTH_OPA_CACHE_TTL_IN_SEC_DEFAULT
             ),
         )
 
-
     def get_user_roles(self, user: Optional[User] = None) -> List[Role]:
         """
         Retrieves a user's roles from an Open Policy Agent instance updating the
@@ -49,27 +47,29 @@ def get_user_roles(self, user: Optional[User] = None) -> List[Role]:
 
         default_role = self.resolve_role(
             current_app.config.get("AUTH_USER_REGISTRATION_ROLE")
-            )
+        )
 
         opa_role_names = self.get_opa_user_roles(user.username)
-        logging.info('OPA returned roles: %s', opa_role_names)
+        logging.info("OPA returned roles: %s", opa_role_names)
 
         opa_roles = set(map(self.resolve_role, opa_role_names))
-        logging.info('Resolved OPA Roles in Database: %s', opa_roles)
+        logging.info("Resolved OPA Roles in Database: %s", opa_roles)
         # Ensure that in case of a bad or no response from OPA each user will have
         # at least one role.
         if opa_roles == {None} or opa_roles == set():
             opa_roles = {default_role}
 
         if set(user.roles) != opa_roles:
-            logging.info('Found a diff between %s (Superset) and %s (OPA).',
-                    user.roles, opa_roles)
+            logging.info(
+                "Found a diff between %s (Superset) and %s (OPA).",
+                user.roles,
+                opa_roles,
+            )
             user.roles = list(opa_roles)
             self.update_user(user)
 
         return user.roles
 
-
     @cachedmethod(lambda self: self.opa_cache)
     def get_opa_user_roles(self, username: str) -> set[str]:
         """
@@ -79,35 +79,36 @@ def get_opa_user_roles(self, username: str) -> set[str]:
         the connection to OPA is encountered or if OPA didn't return a list.
         """
         host, port, tls = self.resolve_opa_base_url()
-        client = OpaClient(host = host, port = port, ssl = tls)
+        client = OpaClient(host=host, port=port, ssl=tls)
         try:
             response = client.query_rule(
-                  input_data = {'username': username},
-                  package_path = current_app.config.get('STACKABLE_OPA_PACKAGE'),
-                  rule_name = current_app.config.get('STACKABLE_OPA_RULE'))
+                input_data={"username": username},
+                package_path=current_app.config.get("STACKABLE_OPA_PACKAGE"),
+                rule_name=current_app.config.get("STACKABLE_OPA_RULE"),
+            )
         except HTTPException as exception:
-            logging.error('Encountered an exception while querying OPA:%s', exception)
+            logging.error("Encountered an exception while querying OPA:%s", exception)
             return []
-        roles = response.get('result')
+        roles = response.get("result")
         # If OPA didn't return a result or if the result is not a list, return no roles.
         if roles is None or type(roles).__name__ != "list":
-            logging.error('The OPA query didn\'t return a list: %s', response)
+            logging.error("The OPA query didn't return a list: %s", response)
             return []
         return roles
 
-
     def resolve_opa_base_url(self) -> Tuple[str, int, bool]:
         """
         Extracts connection parameters of an Open Policy Agent instance from config.
 
         :returns: Hostname, port and protocol (http/https).
         """
-        opa_base_path = current_app.config.get('STACKABLE_OPA_BASE_URL', self.AUTH_OPA_REQUEST_URL_DEFAULT)
+        opa_base_path = current_app.config.get(
+            "STACKABLE_OPA_BASE_URL", self.AUTH_OPA_REQUEST_URL_DEFAULT
+        )
         [protocol, host, port] = opa_base_path.split(":")
         # remove any path be appended to the base url
-        port = int(port.split('/')[0])
-        return host.lstrip('/'), port, protocol == 'https'
-
+        port = int(port.split("/")[0])
+        return host.lstrip("/"), port, protocol == "https"
 
     def resolve_role(self, role_name: str) -> Role:
         """
@@ -117,6 +118,6 @@ def resolve_role(self, role_name: str) -> Role:
         """
         role = self.find_role(role_name)
         if role is None:
-            logging.info('Creating role %s as it doesn\'t already exist.', role_name)
+            logging.info("Creating role %s as it doesn't already exist.", role_name)
             self.add_role(role_name)
         return self.find_role(role_name)

superset/stackable/opa-authorizer/tests/opa_manager_test.py

@@ -27,8 +27,8 @@ def opa_security_manager(
 
     with app.app_context():
         mocker.patch(
-            'flask_appbuilder.security.sqla.manager.SecurityManager.create_db',
-            return_value = None
+            "flask_appbuilder.security.sqla.manager.SecurityManager.create_db",
+            return_value=None,
         )
         return OpaSupersetSecurityManager(appbuilder)
 
@@ -40,17 +40,15 @@ def user() -> User:
     """
     user = User()
     user.id = 1234
-    user.first_name = 'mock'
-    user.last_name = 'mock'
-    user.username = 'mock'
-    user.email = '[email protected]'
+    user.first_name = "mock"
+    user.last_name = "mock"
+    user.username = "mock"
+    user.email = "[email protected]"
 
     return user
 
 
-def test_opa_security_manager(
-    opa_security_manager: OpaSupersetSecurityManager
-) -> None:
+def test_opa_security_manager(opa_security_manager: OpaSupersetSecurityManager) -> None:
     """
     Test that the OPA security manager can be built.
     """
@@ -66,24 +64,25 @@ def test_add_roles(
     """
     Test that roles are correctly added to a user.
     """
-    opa_roles = ['Test1', 'Test2', 'Test3']
+    opa_roles = ["Test1", "Test2", "Test3"]
 
     with app.app_context():
         mocker.patch(
-            'flask_appbuilder.security.sqla.manager.SecurityManager.update_user',
-            return_value = True
+            "flask_appbuilder.security.sqla.manager.SecurityManager.update_user",
+            return_value=True,
         )
         mocker.patch(
-            'opa_authorizer.opa_manager.OpaSupersetSecurityManager.get_opa_user_roles',
-            return_value = opa_roles
+            "opa_authorizer.opa_manager.OpaSupersetSecurityManager.get_opa_user_roles",
+            return_value=opa_roles,
         )
         mocker.patch(
-            'opa_authorizer.opa_manager.OpaSupersetSecurityManager.resolve_role', 
-            wraps=mock_resolve_role
+            "opa_authorizer.opa_manager.OpaSupersetSecurityManager.resolve_role",
+            wraps=mock_resolve_role,
         )
 
-        assert set(map(lambda r: r.name, opa_security_manager.get_user_roles(user))) \
-            == set(opa_roles)
+        assert set(
+            map(lambda r: r.name, opa_security_manager.get_user_roles(user))
+        ) == set(opa_roles)
 
 
 def test_change_roles(
@@ -95,26 +94,27 @@ def test_change_roles(
     """
     Test that roles are correcty changed on a user.
     """
-    opa_roles = ['Test4']
+    opa_roles = ["Test4"]
 
     with app.app_context():
         mocker.patch(
-            'flask_appbuilder.security.sqla.manager.SecurityManager.update_user',
-            return_value = True
+            "flask_appbuilder.security.sqla.manager.SecurityManager.update_user",
+            return_value=True,
         )
         mocker.patch(
-            'opa_authorizer.opa_manager.OpaSupersetSecurityManager.get_opa_user_roles',
-            return_value = opa_roles
+            "opa_authorizer.opa_manager.OpaSupersetSecurityManager.get_opa_user_roles",
+            return_value=opa_roles,
         )
         mocker.patch(
-            'opa_authorizer.opa_manager.OpaSupersetSecurityManager.resolve_role', 
-            wraps=mock_resolve_role
+            "opa_authorizer.opa_manager.OpaSupersetSecurityManager.resolve_role",
+            wraps=mock_resolve_role,
         )
-        user_roles = ['Test1', 'Test2', 'Test3']
+        user_roles = ["Test1", "Test2", "Test3"]
         user.roles = list(map(opa_security_manager.resolve_role, user_roles))
 
-        assert set(map(lambda r: r.name, opa_security_manager.get_user_roles(user))) \
-            == set(opa_roles)
+        assert set(
+            map(lambda r: r.name, opa_security_manager.get_user_roles(user))
+        ) == set(opa_roles)
 
 
 def test_no_roles(
@@ -130,19 +130,21 @@ def test_no_roles(
 
     with app.app_context():
         mocker.patch(
-            'flask_appbuilder.security.sqla.manager.SecurityManager.update_user',
-            return_value = True
+            "flask_appbuilder.security.sqla.manager.SecurityManager.update_user",
+            return_value=True,
         )
         mocker.patch(
-            'opa_authorizer.opa_manager.OpaSupersetSecurityManager.get_opa_user_roles',
-            return_value = opa_roles
+            "opa_authorizer.opa_manager.OpaSupersetSecurityManager.get_opa_user_roles",
+            return_value=opa_roles,
         )
         mocker.patch(
-            'opa_authorizer.opa_manager.OpaSupersetSecurityManager.resolve_role', 
-            wraps=mock_resolve_role
+            "opa_authorizer.opa_manager.OpaSupersetSecurityManager.resolve_role",
+            wraps=mock_resolve_role,
         )
 
-        assert set(map(lambda r: r.name, opa_security_manager.get_user_roles(user))) == {'Public'}
+        assert set(
+            map(lambda r: r.name, opa_security_manager.get_user_roles(user))
+        ) == {"Public"}
 
 
 def test_get_opa_roles(
@@ -153,20 +155,17 @@ def test_get_opa_roles(
     """
     Test that roles are correctly extracted from the OPA response.
     """
-    roles = ['Test1', 'Test2', 'Test3']
-    response = {'result': roles}
+    roles = ["Test1", "Test2", "Test3"]
+    response = {"result": roles}
 
     with app.app_context():
+        mocker.patch("opa_client.opa.OpaClient.query_rule", return_value=response)
         mocker.patch(
-            'opa_client.opa.OpaClient.query_rule',
-            return_value = response
-        )
-        mocker.patch(
-            'opa_authorizer.opa_manager.OpaSupersetSecurityManager.resolve_opa_base_url',
-            return_value = ('opa-instance', 8081, False)
+            "opa_authorizer.opa_manager.OpaSupersetSecurityManager.resolve_opa_base_url",
+            return_value=("opa-instance", 8081, False),
         )
 
-        assert opa_security_manager.get_opa_user_roles('User1') == roles
+        assert opa_security_manager.get_opa_user_roles("User1") == roles
 
 
 def test_get_opa_roles_no_result(
@@ -177,19 +176,16 @@ def test_get_opa_roles_no_result(
     """
     Test that no roles are returned if the OPA response doesn't contain a query result.
     """
-    response = {'error': 'error occurred'}
+    response = {"error": "error occurred"}
 
     with app.app_context():
+        mocker.patch("opa_client.opa.OpaClient.query_rule", return_value=response)
         mocker.patch(
-            'opa_client.opa.OpaClient.query_rule',
-            return_value = response
-        )
-        mocker.patch(
-            'opa_authorizer.opa_manager.OpaSupersetSecurityManager.resolve_opa_base_url',
-            return_value = ('opa-instance', 8081, False)
+            "opa_authorizer.opa_manager.OpaSupersetSecurityManager.resolve_opa_base_url",
+            return_value=("opa-instance", 8081, False),
         )
 
-        assert opa_security_manager.get_opa_user_roles('User1') == []
+        assert opa_security_manager.get_opa_user_roles("User1") == []
 
 
 def test_get_opa_roles_not_a_list(
@@ -200,19 +196,16 @@ def test_get_opa_roles_not_a_list(
     """
     Test that no roles are returned if the query result doesn't contain a list.
     """
-    response = {'result': "['Test1', 'Test2', 'Test3']"} # type string not list
+    response = {"result": "['Test1', 'Test2', 'Test3']"}  # type string not list
 
     with app.app_context():
+        mocker.patch("opa_client.opa.OpaClient.query_rule", return_value=response)
         mocker.patch(
-            'opa_client.opa.OpaClient.query_rule',
-            return_value = response
-        )
-        mocker.patch(
-            'opa_authorizer.opa_manager.OpaSupersetSecurityManager.resolve_opa_base_url',
-            return_value = ('opa-instance', 8081, False)
+            "opa_authorizer.opa_manager.OpaSupersetSecurityManager.resolve_opa_base_url",
+            return_value=("opa-instance", 8081, False),
         )
 
-        assert opa_security_manager.get_opa_user_roles('User1') == []
+        assert opa_security_manager.get_opa_user_roles("User1") == []
 
 
 def test_resolve_opa_base_url(
@@ -225,10 +218,13 @@ def test_resolve_opa_base_url(
     """
     with app.app_context():
         mocker.patch(
-            'flask.current_app.config.get',
-            return_value = 'http://opa-instance:8081'
+            "flask.current_app.config.get", return_value="http://opa-instance:8081"
+        )
+        assert opa_security_manager.resolve_opa_base_url() == (
+            "opa-instance",
+            8081,
+            False,
         )
-        assert opa_security_manager.resolve_opa_base_url() == ('opa-instance', 8081, False)
 
 
 def test_resolve_opa_base_url_with_path(
@@ -242,10 +238,14 @@ def test_resolve_opa_base_url_with_path(
     """
     with app.app_context():
         mocker.patch(
-            'flask.current_app.config.get',
-            return_value = 'http://opa-instance:8081/v1/data/superset'
+            "flask.current_app.config.get",
+            return_value="http://opa-instance:8081/v1/data/superset",
+        )
+        assert opa_security_manager.resolve_opa_base_url() == (
+            "opa-instance",
+            8081,
+            False,
         )
-        assert opa_security_manager.resolve_opa_base_url() == ('opa-instance', 8081, False)
 
 
 def mock_resolve_role(role_name: str) -> Role: