From 0efae3a3d7424d03a4d1fafb33669ca3c41c00b3 Mon Sep 17 00:00:00 2001
From: Benjamin Bertrand <benjamin.bertrand@esss.se>
Date: Thu, 11 Jan 2018 22:55:30 +0100
Subject: [PATCH] Cache user retrieval
The user_id is stored in the flask session. On every page load, we query
the database to get:
- the user objet (user_account table)
- the user's groups (group table) to know what the user can access
If we try to cache the user_load function, the following error is raised
when retrieving the groups:
sqlalchemy.orm.exc.DetachedInstanceError: Parent instance <User at
0x7f51e050a940> is not bound to a Session; lazy load operation of
attribute 'grp' cannot proceed
The groups from AD are only stored to map them with CSENTRY_LDAP_GROUPS.
We don't do any query on them (get all users from one group).
As we always query them with one user, it's more efficient to store them
in the user_account table as an array than in a separate table.
This allows to easily cache the load_user function.
Note that we have to remember to add the current_user to the sqlalchemy session if
we want to modify it or access a relationship (like tokens)!
The load_user function is now cached for 30 minutes. This decreases a
lot the number of database queries.
Fix INFRA-156
---
app/admin/views.py | 6 ------
app/factory.py | 3 +--
app/models.py | 31 ++-----------------------------
app/user/views.py | 8 ++++++++
4 files changed, 11 insertions(+), 37 deletions(-)
diff --git a/app/admin/views.py b/app/admin/views.py
index 96f75ed..69aad41 100644
--- a/app/admin/views.py
+++ b/app/admin/views.py
@@ -51,12 +51,6 @@ class AdminModelView(sqla.ModelView):
return current_user.is_authenticated and current_user.is_admin
-class GroupAdmin(AdminModelView):
- can_create = False
- can_edit = False
- can_delete = False
-
-
class UserAdmin(AdminModelView):
can_create = False
can_edit = False
diff --git a/app/factory.py b/app/factory.py
index 1bdfe49..79bc856 100644
--- a/app/factory.py
+++ b/app/factory.py
@@ -15,7 +15,7 @@ from whitenoise import WhiteNoise
from . import settings, models
from .extensions import (db, migrate, login_manager, ldap_manager, bootstrap,
admin, mail, jwt, toolbar, redis_store, fsession, cache)
-from .admin.views import (AdminModelView, ItemAdmin, UserAdmin, GroupAdmin, TokenAdmin,
+from .admin.views import (AdminModelView, ItemAdmin, UserAdmin, TokenAdmin,
NetworkAdmin)
from .main.views import bp as main
from .inventory.views import bp as inventory
@@ -107,7 +107,6 @@ def create_app(config=None):
cache.init_app(app)
admin.init_app(app)
- admin.add_view(GroupAdmin(models.Group, db.session))
admin.add_view(UserAdmin(models.User, db.session, endpoint='users'))
admin.add_view(TokenAdmin(models.Token, db.session))
admin.add_view(AdminModelView(models.Action, db.session))
diff --git a/app/models.py b/app/models.py
index 89ed072..fa52ac2 100644
--- a/app/models.py
+++ b/app/models.py
@@ -16,7 +16,6 @@ import sqlalchemy as sa
from sqlalchemy.ext.declarative import declared_attr
from sqlalchemy.dialects import postgresql
from sqlalchemy.orm import validates
-from sqlalchemy.ext.associationproxy import association_proxy
from sqlalchemy_continuum import make_versioned, version_class
from citext import CIText
from flask import current_app
@@ -67,6 +66,7 @@ def get_temporary_ics_id():
@login_manager.user_loader
+@cache.memoize(timeout=1800)
def load_user(user_id):
"""User loader callback for flask-login
@@ -95,28 +95,6 @@ def save_user(dn, username, data, memberships):
return user
-# Table required for Many-to-Many relationships between users and groups
-usergroups_table = db.Table(
- 'usergroups',
- db.Column('user_id', db.Integer, db.ForeignKey('user_account.id')),
- db.Column('group_id', db.Integer, db.ForeignKey('group.id'))
-)
-
-
-class Group(db.Model):
- id = db.Column(db.Integer, primary_key=True)
- name = db.Column(db.Text, nullable=False, unique=True)
-
- def __str__(self):
- return self.name
-
-
-def find_or_create_group(name):
- """Return the existing group or a newly created one"""
- group = Group.query.filter_by(name=name).first()
- return group or Group(name=name)
-
-
class User(db.Model, UserMixin):
# "user" is a reserved word in postgresql
# so let's use another name
@@ -126,12 +104,7 @@ class User(db.Model, UserMixin):
username = db.Column(db.Text, nullable=False, unique=True)
display_name = db.Column(db.Text, nullable=False)
email = db.Column(db.Text)
- grp = db.relationship('Group', secondary=usergroups_table,
- backref=db.backref('members', lazy='dynamic'))
- # Proxy the 'name' attribute from the 'grp' relationship
- # See http://docs.sqlalchemy.org/en/latest/orm/extensions/associationproxy.html
- groups = association_proxy('grp', 'name',
- creator=find_or_create_group)
+ groups = db.Column(postgresql.ARRAY(db.Text))
tokens = db.relationship("Token", backref="user")
def get_id(self):
diff --git a/app/user/views.py b/app/user/views.py
index 473f9bc..9440dfc 100644
--- a/app/user/views.py
+++ b/app/user/views.py
@@ -14,6 +14,8 @@ from flask import (Blueprint, render_template, request, redirect, url_for,
from flask_login import login_user, logout_user, login_required, current_user
from flask_ldap3_login.forms import LDAPLoginForm
from .forms import TokenForm
+from ..extensions import cache, db
+from ..models import load_user
from .. import tokens, utils
bp = Blueprint('user', __name__)
@@ -31,6 +33,8 @@ def login():
@bp.route('/logout')
@login_required
def logout():
+ # Don't forget to remove the user from the cache
+ cache.delete_memoized(load_user, str(current_user.id))
logout_user()
return redirect(url_for('user.login'))
@@ -38,6 +42,10 @@ def logout():
@bp.route('/profile', methods=['GET', 'POST'])
@login_required
def profile():
+ if current_user not in db.session:
+ # If the current user is cached, it won't be in the sqlalchemy session
+ # Add it to access the user.tokens relationship in the template
+ db.session.add(current_user)
# Try to get the generated token from the session
token = session.pop('generated_token', None)
form = TokenForm(request.form)
--
GitLab