Cleanup fulfilled requests after approve (#1953)

* Cleanup fulfilled requests after approve

* Modified tests

* Moved to separate test, add user to access functions

* Moved to separate test and added test cases

* Fixed issue with dryrun

* More changes based on comments

superset/config.py

@@ -35,7 +35,7 @@
 SUPERSET_WEBSERVER_ADDRESS = '0.0.0.0'
 SUPERSET_WEBSERVER_PORT = 8088
 SUPERSET_WEBSERVER_TIMEOUT = 60
-
+EMAIL_NOTIFICATIONS = False
 CUSTOM_SECURITY_MANAGER = None
 # ---------------------------------------------------------
 

superset/utils.py

@@ -63,13 +63,13 @@ class SupersetTemplateException(SupersetException):
     pass
 
 
-def can_access(security_manager, permission_name, view_name):
+def can_access(sm, permission_name, view_name, user):
     """Protecting from has_access failing from missing perms/view"""
-    try:
-        return security_manager.has_access(permission_name, view_name)
-    except:
-        pass
-    return False
+    return (
+        sm.is_item_public(permission_name, view_name) or
+        (not user.is_anonymous() and
+         sm._has_view_access(user, permission_name, view_name))
+    )
 
 
 def flasher(msg, severity=None):
@@ -436,7 +436,7 @@ def notify_user_about_perm_udate(
     subject = __('[Superset] Access to the datasource %(name)s was granted',
                  name=datasource.full_name)
     send_email_smtp(user.email, subject, msg, config, bcc=granter.email,
-                    dryrun=config.get('EMAIL_NOTIFICATIONS'))
+                    dryrun=not config.get('EMAIL_NOTIFICATIONS'))
 
 
 def send_email_smtp(to, subject, html_content, config, files=None,
@@ -478,7 +478,7 @@ def send_email_smtp(to, subject, html_content, config, files=None,
                 Name=basename
             ))
 
-    send_MIME_email(smtp_mail_from, recipients, msg, config, dryrun)
+    send_MIME_email(smtp_mail_from, recipients, msg, config, dryrun=dryrun)
 
 
 def send_MIME_email(e_from, e_to, mime_msg, config, dryrun=False):

superset/views.py

@@ -49,30 +49,34 @@
 
 
 class BaseSupersetView(BaseView):
-    def can_access(self, permission_name, view_name):
-        return utils.can_access(appbuilder.sm, permission_name, view_name)
+    def can_access(self, permission_name, view_name, user=None):
+        if not user:
+            user = g.user
+        return utils.can_access(
+            appbuilder.sm, permission_name, view_name, user)
 
-    def all_datasource_access(self):
+    def all_datasource_access(self, user=None):
         return self.can_access(
-            "all_datasource_access", "all_datasource_access")
+            "all_datasource_access", "all_datasource_access", user=user)
 
-    def database_access(self, database):
+    def database_access(self, database, user=None):
         return (
-            self.can_access("all_database_access", "all_database_access") or
-            self.can_access("database_access", database.perm)
+            self.can_access(
+                "all_database_access", "all_database_access", user=user) or
+            self.can_access("database_access", database.perm, user=user)
         )
 
-    def schema_access(self, datasource):
+    def schema_access(self, datasource, user=None):
         return (
-            self.database_access(datasource.database) or
-            self.all_datasource_access() or
-            self.can_access("schema_access", datasource.schema_perm)
+            self.database_access(datasource.database, user=user) or
+            self.all_datasource_access(user=user) or
+            self.can_access("schema_access", datasource.schema_perm, user=user)
         )
 
-    def datasource_access(self, datasource):
+    def datasource_access(self, datasource, user=None):
         return (
-            self.schema_access(datasource) or
-            self.can_access("datasource_access", datasource.perm)
+            self.schema_access(datasource, user=user) or
+            self.can_access("datasource_access", datasource.perm, user=user)
         )
 
     def datasource_access_by_name(
@@ -82,7 +86,7 @@ def datasource_access_by_name(
             return True
 
         schema_perm = utils.get_schema_perm(database, schema)
-        if schema and utils.can_access(sm, 'schema_access', schema_perm):
+        if schema and utils.can_access(sm, 'schema_access', schema_perm, g.user):
             return True
 
         datasources = SourceRegistry.query_datasources_by_name(
@@ -1286,6 +1290,14 @@ def request_access(self):
     @has_access
     @expose("/approve")
     def approve(self):
+        def clean_fulfilled_requests(session):
+            for r in session.query(DAR).all():
+                datasource = SourceRegistry.get_datasource(
+                    r.datasource_type, r.datasource_id, session)
+                user = sm.get_user_by_id(r.created_by_fk)
+                if self.datasource_access(datasource, user):
+                    session.delete(r)
+            session.commit()
         datasource_type = request.args.get('datasource_type')
         datasource_id = request.args.get('datasource_id')
         created_by_username = request.args.get('created_by')
@@ -1347,7 +1359,7 @@ def approve(self):
                     g.user, requested_by, role, datasource,
                     'email/role_extended.txt', app.config)
                 flash(msg, "info")
-
+            clean_fulfilled_requests(session)
         else:
             flash(__("You have no permission to approve this request"),
                   "danger")