Speed up has_access decorator by ~200ms (apache#22858)

Using the ORM to create all the Role, Permission, Action and Resource
objects, only to throw them all away _on every request_ is slow. And
since we are now using the API more and more in the UI it's starting to
get noticeable

This changes the `user.perm` property to issue a custom query that
returns the tuple of action_name, permission_name we want, bypassing the
ORM object inflation entirely, and since `user.roles` isn't needed in
most requests we no longer eagerly load that.

* Fix tests

Caching issues that only crop up in tests (but not ever a problem in the
request life cycle of webserver

.codespellignorelines

@@ -1,2 +1,3 @@
             f"DELETE {source_table} FROM { ', '.join(_from_name(tbl) for tbl in stmt.froms) }"
         for frm in source_query.selectable.froms:
+    roles = relationship("Role", secondary=assoc_user_role, backref="user", lazy="selectin")

airflow/www/auth.py

@@ -35,7 +35,10 @@ def decorated(*args, **kwargs):
             __tracebackhide__ = True  # Hide from pytest traceback.
 
             appbuilder = current_app.appbuilder
-            if not g.user.is_anonymous and not g.user.perms:
+
+            if appbuilder.sm.check_authorization(permissions, request.args.get('dag_id', None)):
+                return func(*args, **kwargs)
+            elif not g.user.is_anonymous and not g.user.perms:
                 return (
                     render_template(
                         'airflow/no_roles_permissions.html',
@@ -46,9 +49,6 @@ def decorated(*args, **kwargs):
                     ),
                     403,
                 )
-
-            if appbuilder.sm.check_authorization(permissions, request.args.get('dag_id', None)):
-                return func(*args, **kwargs)
             else:
                 access_denied = "Access is Denied"
                 flash(access_denied, "danger")

airflow/www/fab_security/sqla/models.py

@@ -20,9 +20,9 @@
 # This product contains a modified portion of 'Flask App Builder' developed by Daniel Vaz Gaspar.
 # (https://github.com/dpgaspar/Flask-AppBuilder).
 # Copyright 2013, Daniel Vaz Gaspar
-from typing import TYPE_CHECKING, Union
+from typing import TYPE_CHECKING, Set, Tuple, Union
 
-from flask import g
+from flask import current_app, g
 from flask_appbuilder.models.sqla import Model
 from sqlalchemy import (
     Boolean,
@@ -181,7 +181,7 @@ class User(Model):
     last_login = Column(DateTime)
     login_count = Column(Integer)
     fail_login_count = Column(Integer)
-    roles = relationship("Role", secondary=assoc_user_role, backref="user", lazy="joined")
+    roles = relationship("Role", secondary=assoc_user_role, backref="user", lazy="selectin")
     created_on = Column(DateTime, default=datetime.datetime.now, nullable=True)
     changed_on = Column(DateTime, default=datetime.datetime.now, nullable=True)
 
@@ -229,10 +229,24 @@ def is_anonymous(self):
 
     @property
     def perms(self):
-        perms = set()
-        for role in self.roles:
-            perms.update((perm.action.name, perm.resource.name) for perm in role.permissions)
-        return perms
+        if not self._perms:
+            # Using the ORM here is _slow_ (Creating lots of objects to then throw them away) since this is in
+            # the path for every request. Avoid it if we can!
+            if current_app:
+                sm = current_app.appbuilder.sm
+                self._perms: Set[Tuple[str, str]] = set(
+                    sm.get_session.query(sm.action_model.name, sm.resource_model.name)
+                    .join(sm.permission_model.action)
+                    .join(sm.permission_model.resource)
+                    .join(sm.permission_model.role)
+                    .filter(sm.role_model.user.contains(self))
+                    .all()
+                )
+            else:
+                self._perms = {
+                    (perm.action.name, perm.resource.name) for role in self.roles for perm in role.permissions
+                }
+        return self._perms
 
     def get_id(self):
         return self.id
@@ -243,6 +257,8 @@ def get_full_name(self):
     def __repr__(self):
         return self.get_full_name()
 
+    _perms = None
+
 
 class RegisterUser(Model):
     """Represents a user registration."""

tests/test_utils/api_connexion_utils.py

@@ -56,6 +56,8 @@ def create_user(app, username, role_name=None, email=None, permissions=None):
     if role_name:
         delete_role(app, role_name)
         role = create_role(app, role_name, permissions)
+    else:
+        role = []
 
     return appbuilder.sm.add_user(
         username=username,
@@ -80,12 +82,12 @@ def create_role(app, name, permissions=None):
     return role
 
 
-def set_user_single_role(app, username, role_name):
+def set_user_single_role(app, user, role_name):
     role = create_role(app, role_name)
-    user = app.appbuilder.sm.find_user(username)
     if role not in user.roles:
         user.roles = [role]
         app.appbuilder.sm.update_user(user)
+        user._perms = None
 
 
 def delete_role(app, name):

tests/www/test_security.py

@@ -425,9 +425,10 @@ def test_get_current_user_permissions(app):
         ) as user:
             assert user.perms == {(action, resource)}
 
-            user._perms = None
-            user.roles = []
-
+        with create_user_scope(
+            app,
+            username='no_perms',
+        ) as user:
             assert len(user.perms) == 0
 
 
@@ -617,7 +618,7 @@ def test_access_control_is_set_on_init(
             )
 
             security_manager.bulk_sync_roles([{'role': negated_role, 'perms': []}])
-            set_user_single_role(app, username, role_name=negated_role)
+            set_user_single_role(app, user, role_name=negated_role)
             assert_user_does_not_have_dag_perms(
                 perms=[permissions.ACTION_CAN_EDIT, permissions.ACTION_CAN_READ],
                 dag_id='access_control_test',
@@ -640,7 +641,7 @@ def test_access_control_stale_perms_are_revoked(
             role_name=role_name,
             permissions=[],
         ) as user:
-            set_user_single_role(app, username, role_name='team-a')
+            set_user_single_role(app, user, role_name='team-a')
             security_manager._sync_dag_view_permissions(
                 'access_control_test', access_control={'team-a': READ_WRITE}
             )
@@ -649,6 +650,8 @@ def test_access_control_stale_perms_are_revoked(
             security_manager._sync_dag_view_permissions(
                 'access_control_test', access_control={'team-a': READ_ONLY}
             )
+            # Clear the cache, to make it pick up new rol perms
+            user._perms = None
             assert_user_has_dag_perms(
                 perms=[permissions.ACTION_CAN_READ], dag_id='access_control_test', user=user
             )

tests/www/views/test_views_base.py

@@ -30,7 +30,7 @@
 
 
 def test_index(admin_client):
-    with assert_queries_count(12):
+    with assert_queries_count(14):
         resp = admin_client.get('/', follow_redirects=True)
     check_content_in_response('DAGs', resp)