From 12d831a321709c6ef1e5dea744619278332c8a97 Mon Sep 17 00:00:00 2001 From: Kevin Chung Date: Wed, 29 Apr 2020 03:15:20 -0400 Subject: [PATCH 1/3] Make CTFd.utils.user.is_admin wrap a cached function so we avoid a DB hit on page loads --- CTFd/cache/__init__.py | 5 +++++ CTFd/utils/user/__init__.py | 11 +++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/CTFd/cache/__init__.py b/CTFd/cache/__init__.py index 78a63b5..200e207 100644 --- a/CTFd/cache/__init__.py +++ b/CTFd/cache/__init__.py @@ -44,3 +44,8 @@ def clear_pages(): cache.delete_memoized(get_pages) cache.delete_memoized(get_page) + + +def clear_user_session(user_id): + from CTFd.utils.user import get_user_type + cache.delete_memoized(get_user_type, user_id=user_id) diff --git a/CTFd/utils/user/__init__.py b/CTFd/utils/user/__init__.py index 81762ba..a5f0c22 100644 --- a/CTFd/utils/user/__init__.py +++ b/CTFd/utils/user/__init__.py @@ -4,6 +4,7 @@ import re from flask import current_app as app from flask import request, session +from CTFd.cache import cache from CTFd.models import Fails, Users, db from CTFd.utils import get_config @@ -32,14 +33,20 @@ def get_current_user_type(fallback=None): return fallback +@cache.memoize() +def get_user_type(user_id): + user = Users.query.filter_by(id=user_id).first() + return user.type + + def authed(): return bool(session.get("id", False)) def is_admin(): if authed(): - user = get_current_user() - return user.type == "admin" + user_type = get_user_type(user_id=session["id"]) + return user_type == "admin" else: return False From ff4ad5185fafaf4b3bafb51c6336b6d8e4ce7ce1 Mon Sep 17 00:00:00 2001 From: Kevin Chung Date: Wed, 29 Apr 2020 03:58:54 -0400 Subject: [PATCH 2/3] Test clear_user_session and clear sessions on user modifications --- CTFd/api/v1/users.py | 5 +++- CTFd/cache/__init__.py | 1 + tests/cache/__init__.py | 0 tests/cache/test_cache.py | 51 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 tests/cache/__init__.py create mode 100644 tests/cache/test_cache.py diff --git a/CTFd/api/v1/users.py b/CTFd/api/v1/users.py index 895c9ea..20e3998 100644 --- a/CTFd/api/v1/users.py +++ b/CTFd/api/v1/users.py @@ -1,7 +1,7 @@ from flask import abort, request from flask_restx import Namespace, Resource -from CTFd.cache import clear_standings +from CTFd.cache import clear_standings, clear_user_session from CTFd.models import ( Awards, Notifications, @@ -107,6 +107,7 @@ class UserPublic(Resource): db.session.close() + clear_user_session(user_id=user_id) clear_standings() return {"success": True, "data": response} @@ -123,6 +124,7 @@ class UserPublic(Resource): db.session.commit() db.session.close() + clear_user_session(user_id=user_id) clear_standings() return {"success": True} @@ -149,6 +151,7 @@ class UserPrivate(Resource): db.session.commit() + clear_user_session(user_id=user.id) response = schema.dump(response.data) db.session.close() diff --git a/CTFd/cache/__init__.py b/CTFd/cache/__init__.py index 200e207..ba31a2b 100644 --- a/CTFd/cache/__init__.py +++ b/CTFd/cache/__init__.py @@ -48,4 +48,5 @@ def clear_pages(): def clear_user_session(user_id): from CTFd.utils.user import get_user_type + cache.delete_memoized(get_user_type, user_id=user_id) diff --git a/tests/cache/__init__.py b/tests/cache/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/cache/test_cache.py b/tests/cache/test_cache.py new file mode 100644 index 0000000..0d01133 --- /dev/null +++ b/tests/cache/test_cache.py @@ -0,0 +1,51 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- + +from CTFd.models import Users +from CTFd.utils.user import is_admin, get_current_user +from CTFd.utils.security.auth import login_user +from tests.helpers import create_ctfd, destroy_ctfd, register_user + +from CTFd.cache import clear_user_session + + +def test_clear_user_session(): + app = create_ctfd() + with app.app_context(): + register_user(app) + + # Users by default should have a non-admin type + user = Users.query.filter_by(id=2).first() + with app.test_request_context("/"): + login_user(user) + user = get_current_user() + assert user.id == 2 + assert user.type == "user" + assert is_admin() is False + + # Set the user's updated type + user = Users.query.filter_by(id=2).first() + user.type = "admin" + app.db.session.commit() + + # The user shouldn't be considered admin because their type is still cached + user = Users.query.filter_by(id=2).first() + with app.test_request_context("/"): + login_user(user) + user = get_current_user() + assert user.id == 2 + assert user.type == "admin" + assert is_admin() is False + + # Clear the user's cached session (for now just the type) + clear_user_session(user_id=2) + + # The user's type should now be admin + user = Users.query.filter_by(id=2).first() + with app.test_request_context("/"): + login_user(user) + user = get_current_user() + assert user.id == 2 + assert user.type == "admin" + assert is_admin() is True + destroy_ctfd(app) From f86b7ae18fa31485a77aebd16f906fd0916b7076 Mon Sep 17 00:00:00 2001 From: Kevin Chung Date: Wed, 29 Apr 2020 20:19:05 -0400 Subject: [PATCH 3/3] Switch to get_user_attrs strategy --- .gitignore | 3 +++ CTFd/cache/__init__.py | 4 ++-- CTFd/constants/users.py | 22 ++++++++++++++++++++++ CTFd/utils/user/__init__.py | 35 ++++++++++++++++++++++++----------- development.txt | 2 +- serve.py | 4 ++++ 6 files changed, 56 insertions(+), 14 deletions(-) create mode 100644 CTFd/constants/users.py diff --git a/.gitignore b/.gitignore index 27f729a..ec0485b 100644 --- a/.gitignore +++ b/.gitignore @@ -73,3 +73,6 @@ CTFd/uploads # JS node_modules/ + +# Flask Profiler files +flask_profiler.sql \ No newline at end of file diff --git a/CTFd/cache/__init__.py b/CTFd/cache/__init__.py index ba31a2b..fcb8b61 100644 --- a/CTFd/cache/__init__.py +++ b/CTFd/cache/__init__.py @@ -47,6 +47,6 @@ def clear_pages(): def clear_user_session(user_id): - from CTFd.utils.user import get_user_type + from CTFd.utils.user import get_user_attrs - cache.delete_memoized(get_user_type, user_id=user_id) + cache.delete_memoized(get_user_attrs, user_id=user_id) diff --git a/CTFd/constants/users.py b/CTFd/constants/users.py new file mode 100644 index 0000000..123e39d --- /dev/null +++ b/CTFd/constants/users.py @@ -0,0 +1,22 @@ +from collections import namedtuple + +UserAttrs = namedtuple( + "UserAttrs", + [ + "id", + "oauth_id", + "name", + "email", + "type", + "secret", + "website", + "affiliation", + "country", + "bracket", + "hidden", + "banned", + "verified", + "team_id", + "created", + ], +) \ No newline at end of file diff --git a/CTFd/utils/user/__init__.py b/CTFd/utils/user/__init__.py index a5f0c22..c754b5e 100644 --- a/CTFd/utils/user/__init__.py +++ b/CTFd/utils/user/__init__.py @@ -5,7 +5,8 @@ from flask import current_app as app from flask import request, session from CTFd.cache import cache -from CTFd.models import Fails, Users, db +from CTFd.constants.users import UserAttrs +from CTFd.models import Fails, Users, db, Teams from CTFd.utils import get_config @@ -17,6 +18,24 @@ def get_current_user(): return None +def get_current_user_attrs(): + if authed(): + return get_user_attrs(user_id=session["id"]) + else: + return None + + +@cache.memoize() +def get_user_attrs(user_id): + user = Users.query.filter_by(id=user_id).first() + if user: + d = {} + for field in UserAttrs._fields: + d[field] = getattr(user, field) + return UserAttrs(**d) + return user + + def get_current_team(): if authed(): user = get_current_user() @@ -27,33 +46,27 @@ def get_current_team(): def get_current_user_type(fallback=None): if authed(): - user = Users.query.filter_by(id=session["id"]).first() + user = get_current_user_attrs() return user.type else: return fallback -@cache.memoize() -def get_user_type(user_id): - user = Users.query.filter_by(id=user_id).first() - return user.type - - def authed(): return bool(session.get("id", False)) def is_admin(): if authed(): - user_type = get_user_type(user_id=session["id"]) - return user_type == "admin" + user = get_current_user_attrs() + return user.type == "admin" else: return False def is_verified(): if get_config("verify_emails"): - user = get_current_user() + user = get_current_user_attrs() if user: return user.verified else: diff --git a/development.txt b/development.txt index 5335076..a88d966 100644 --- a/development.txt +++ b/development.txt @@ -10,7 +10,7 @@ psycopg2-binary==2.7.5 codecov==2.0.15 moto==1.3.7 bandit==1.5.1 -flask_profiler==1.7 +flask_profiler==1.8.1 pytest-xdist==1.28.0 pytest-cov==2.8.1 sphinx_rtd_theme==0.4.3 diff --git a/serve.py b/serve.py index 9d6beeb..1e3a79a 100644 --- a/serve.py +++ b/serve.py @@ -18,6 +18,10 @@ if args.profile: "enabled": app.config["DEBUG"], "storage": {"engine": "sqlite"}, "basicAuth": {"enabled": False}, + "ignore": [ + "^/themes/.*", + "^/events", + ] } flask_profiler.init_app(app) app.config["DEBUG_TB_PROFILER_ENABLED"] = True