diff --git a/CHANGELOG.md b/CHANGELOG.md index 1abb0da5f..445950ffd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,24 @@ +2.5.0 / 2020-06-02 +================== + +**General** +* Use a session invalidation strategy inspired by Django. Newly generated user sessions will now include a HMAC of the user's password. When the user's password is changed by someone other than the user the previous HMACs will no longer be valid and the user will be logged out when they next attempt to perform an action. +* A user and team's place, and score are now cached and invalidated on score changes. + +**API** +* Add `/api/v1/challenges?view=admin` to allow admin users to see all challenges regardless of their visibility state +* Add `/api/v1/users?view=admin` to allow admin users to see all users regardless of their hidden/banned state +* Add `/api/v1/teams?view=admin` to allow admin users to see all teams regardless of their hidden/banned state +* The scoreboard endpoints `/api/v1/scoreboard` & `/api/v1/scoreboard/top/[count]` should now be more performant because score and place for Users/Teams are now cached + +**Deployment** +* `docker-compose` now provides a basic nginx configuration and deploys nginx on port 80 + +**Miscellaneous** +* The `get_config` and `get_page` config utilities now use SQLAlchemy Core instead of SQLAlchemy ORM for slight speedups +* Update Flask-Migrate to 2.5.3 and regenerate the migration environment. Fixes using `%` signs in database passwords. + + 2.4.3 / 2020-05-24 ================== diff --git a/CTFd/__init__.py b/CTFd/__init__.py index 44b620b72..6c199bdfe 100644 --- a/CTFd/__init__.py +++ b/CTFd/__init__.py @@ -31,7 +31,7 @@ reload(sys) # noqa: F821 sys.setdefaultencoding("utf-8") -__version__ = "2.4.3" +__version__ = "2.5.0" class CTFdRequest(Request): diff --git a/CTFd/api/v1/challenges.py b/CTFd/api/v1/challenges.py index 9868476e5..411c00791 100644 --- a/CTFd/api/v1/challenges.py +++ b/CTFd/api/v1/challenges.py @@ -47,31 +47,36 @@ def get(self): # This can return None (unauth) if visibility is set to public user = get_current_user() - challenges = ( - Challenges.query.filter( - and_(Challenges.state != "hidden", Challenges.state != "locked") - ) - .order_by(Challenges.value) - .all() - ) - - if user: - solve_ids = ( - Solves.query.with_entities(Solves.challenge_id) - .filter_by(account_id=user.account_id) - .order_by(Solves.challenge_id.asc()) + # Admins can request to see everything + if is_admin() and request.args.get("view") == "admin": + challenges = Challenges.query.order_by(Challenges.value).all() + solve_ids = set([challenge.id for challenge in challenges]) + else: + challenges = ( + Challenges.query.filter( + and_(Challenges.state != "hidden", Challenges.state != "locked") + ) + .order_by(Challenges.value) .all() ) - solve_ids = set([value for value, in solve_ids]) - # TODO: Convert this into a re-useable decorator - if is_admin(): - pass + if user: + solve_ids = ( + Solves.query.with_entities(Solves.challenge_id) + .filter_by(account_id=user.account_id) + .order_by(Solves.challenge_id.asc()) + .all() + ) + solve_ids = set([value for value, in solve_ids]) + + # TODO: Convert this into a re-useable decorator + if is_admin(): + pass + else: + if config.is_teams_mode() and get_current_team() is None: + abort(403) else: - if config.is_teams_mode() and get_current_team() is None: - abort(403) - else: - solve_ids = set() + solve_ids = set() response = [] tag_schema = TagSchema(view="user", many=True) diff --git a/CTFd/api/v1/teams.py b/CTFd/api/v1/teams.py index d387f81aa..cba196b1a 100644 --- a/CTFd/api/v1/teams.py +++ b/CTFd/api/v1/teams.py @@ -22,7 +22,11 @@ class TeamList(Resource): @check_account_visibility def get(self): - teams = Teams.query.filter_by(hidden=False, banned=False) + if is_admin() and request.args.get("view") == "admin": + teams = Teams.query.filter_by() + else: + teams = Teams.query.filter_by(hidden=False, banned=False) + user_type = get_current_user_type(fallback="user") view = copy.deepcopy(TeamSchema.views.get(user_type)) view.remove("members") diff --git a/CTFd/api/v1/users.py b/CTFd/api/v1/users.py index 20e39980f..0ec2edf08 100644 --- a/CTFd/api/v1/users.py +++ b/CTFd/api/v1/users.py @@ -22,6 +22,7 @@ check_score_visibility, ) from CTFd.utils.email import sendmail, user_created_notification +from CTFd.utils.security.auth import update_user from CTFd.utils.user import get_current_user, get_current_user_type, is_admin users_namespace = Namespace("users", description="Endpoint to retrieve Users") @@ -31,7 +32,11 @@ class UserList(Resource): @check_account_visibility def get(self): - users = Users.query.filter_by(banned=False, hidden=False) + if is_admin() and request.args.get("view") == "admin": + users = Users.query.filter_by() + else: + users = Users.query.filter_by(banned=False, hidden=False) + response = UserSchema(view="user", many=True).dump(users) if response.errors: @@ -151,7 +156,9 @@ def patch(self): db.session.commit() - clear_user_session(user_id=user.id) + # Update user's session for the new session hash + update_user(user) + response = schema.dump(response.data) db.session.close() diff --git a/CTFd/cache/__init__.py b/CTFd/cache/__init__.py index 9ea10e033..a06d003fe 100644 --- a/CTFd/cache/__init__.py +++ b/CTFd/cache/__init__.py @@ -26,6 +26,7 @@ def clear_config(): def clear_standings(): + from CTFd.models import Users, Teams from CTFd.utils.scores import get_standings, get_team_standings, get_user_standings from CTFd.api.v1.scoreboard import ScoreboardDetail, ScoreboardList from CTFd.api import api @@ -33,6 +34,10 @@ def clear_standings(): cache.delete_memoized(get_standings) cache.delete_memoized(get_team_standings) cache.delete_memoized(get_user_standings) + cache.delete_memoized(Users.get_score) + cache.delete_memoized(Users.get_place) + cache.delete_memoized(Teams.get_score) + cache.delete_memoized(Teams.get_place) cache.delete(make_cache_key(path="scoreboard.listing")) cache.delete(make_cache_key(path=api.name + "." + ScoreboardList.endpoint)) cache.delete(make_cache_key(path=api.name + "." + ScoreboardDetail.endpoint)) diff --git a/CTFd/models/__init__.py b/CTFd/models/__init__.py index 777059bb7..e2d47f881 100644 --- a/CTFd/models/__init__.py +++ b/CTFd/models/__init__.py @@ -5,6 +5,7 @@ from sqlalchemy.ext.hybrid import hybrid_property from sqlalchemy.orm import column_property, validates +from CTFd.cache import cache from CTFd.utils.crypto import hash_password from CTFd.utils.humanize.numbers import ordinalize @@ -322,6 +323,7 @@ def get_awards(self, admin=False): awards = awards.filter(Awards.date < dt) return awards.all() + @cache.memoize() def get_score(self, admin=False): score = db.func.sum(Challenges.value).label("score") user = ( @@ -354,6 +356,7 @@ def get_score(self, admin=False): else: return 0 + @cache.memoize() def get_place(self, admin=False, numeric=False): """ This method is generally a clone of CTFd.scoreboard.get_standings. @@ -487,12 +490,14 @@ def get_awards(self, admin=False): return awards.all() + @cache.memoize() def get_score(self, admin=False): score = 0 for member in self.members: score += member.get_score(admin=admin) return score + @cache.memoize() def get_place(self, admin=False, numeric=False): """ This method is generally a clone of CTFd.scoreboard.get_standings. diff --git a/CTFd/utils/__init__.py b/CTFd/utils/__init__.py index e3a3e6963..f4dce6cce 100644 --- a/CTFd/utils/__init__.py +++ b/CTFd/utils/__init__.py @@ -23,7 +23,9 @@ def get_app_config(key, default=None): @cache.memoize() def _get_config(key): - config = Configs.query.filter_by(key=key).first() + config = db.session.execute( + Configs.__table__.select().where(Configs.key == key) + ).fetchone() if config and config.value: value = config.value if value and value.isdigit(): diff --git a/CTFd/utils/config/pages.py b/CTFd/utils/config/pages.py index c5811e992..cb9cb5352 100644 --- a/CTFd/utils/config/pages.py +++ b/CTFd/utils/config/pages.py @@ -1,5 +1,5 @@ from CTFd.cache import cache -from CTFd.models import Pages +from CTFd.models import db, Pages @cache.memoize() @@ -12,4 +12,8 @@ def get_pages(): @cache.memoize() def get_page(route): - return Pages.query.filter(Pages.route == route, Pages.draft.isnot(True)).first() + return db.session.execute( + Pages.__table__.select() + .where(Pages.route == route) + .where(Pages.draft.isnot(True)) + ).fetchone() diff --git a/CTFd/utils/security/auth.py b/CTFd/utils/security/auth.py index 9692e8451..70a248230 100644 --- a/CTFd/utils/security/auth.py +++ b/CTFd/utils/security/auth.py @@ -8,6 +8,7 @@ from CTFd.models import UserTokens, db from CTFd.utils.encoding import hexencode from CTFd.utils.security.csrf import generate_nonce +from CTFd.utils.security.signing import hmac def login_user(user): @@ -15,6 +16,17 @@ def login_user(user): session["name"] = user.name session["email"] = user.email session["nonce"] = generate_nonce() + session["hash"] = hmac(user.password) + + # Clear out any currently cached user attributes + clear_user_session(user_id=user.id) + + +def update_user(user): + session["id"] = user.id + session["name"] = user.name + session["email"] = user.email + session["hash"] = hmac(user.password) # Clear out any currently cached user attributes clear_user_session(user_id=user.id) diff --git a/CTFd/utils/security/signing.py b/CTFd/utils/security/signing.py index e0fe6a069..6887cf0d7 100644 --- a/CTFd/utils/security/signing.py +++ b/CTFd/utils/security/signing.py @@ -1,3 +1,7 @@ +import hashlib +import hmac as _hmac +import six + from flask import current_app from itsdangerous import Signer from itsdangerous.exc import ( # noqa: F401 @@ -7,6 +11,8 @@ ) from itsdangerous.url_safe import URLSafeTimedSerializer +from CTFd.utils import string_types + def serialize(data, secret=None): if secret is None: @@ -34,3 +40,16 @@ def unsign(data, secret=None): secret = current_app.config["SECRET_KEY"] s = Signer(secret) return s.unsign(data) + + +def hmac(data, secret=None, digest=hashlib.sha1): + if secret is None: + secret = current_app.config["SECRET_KEY"] + + if six.PY3: + if isinstance(data, string_types): + data = data.encode("utf-8") + if isinstance(secret, string_types): + secret = secret.encode("utf-8") + + return _hmac.new(key=secret, msg=data, digestmod=digest).hexdigest() diff --git a/CTFd/utils/user/__init__.py b/CTFd/utils/user/__init__.py index 7f21dcaba..f9689f16e 100644 --- a/CTFd/utils/user/__init__.py +++ b/CTFd/utils/user/__init__.py @@ -2,18 +2,28 @@ import re from flask import current_app as app -from flask import request, session +from flask import abort, redirect, request, session, url_for from CTFd.cache import cache from CTFd.constants.users import UserAttrs from CTFd.constants.teams import TeamAttrs from CTFd.models import Fails, Users, db, Teams, Tracking from CTFd.utils import get_config +from CTFd.utils.security.signing import hmac +from CTFd.utils.security.auth import logout_user def get_current_user(): if authed(): user = Users.query.filter_by(id=session["id"]).first() + + # Check if the session is still valid + session_hash = session.get("hash") + if session_hash: + if session_hash != hmac(user.password): + logout_user() + abort(redirect(url_for("auth.login", next=request.full_path))) + return user else: return None diff --git a/conf/nginx/http.conf b/conf/nginx/http.conf new file mode 100644 index 000000000..ff4061354 --- /dev/null +++ b/conf/nginx/http.conf @@ -0,0 +1,49 @@ +worker_processes 4; + +events { + + worker_connections 1024; +} + +http { + + # Configuration containing list of application servers + upstream app_servers { + + server ctfd:8000; + } + + server { + + listen 80; + + client_max_body_size 4G; + + # Handle Server Sent Events for Notifications + location /events { + + proxy_pass http://app_servers; + proxy_set_header Connection ''; + proxy_http_version 1.1; + chunked_transfer_encoding off; + proxy_buffering off; + proxy_cache off; + proxy_redirect off; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Host $server_name; + } + + # Proxy connections to the application servers + location / { + + proxy_pass http://app_servers; + proxy_redirect off; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Host $server_name; + } + } +} diff --git a/docker-compose.yml b/docker-compose.yml index 89f81e64c..d40f6c582 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -15,6 +15,7 @@ services: - LOG_FOLDER=/var/log/CTFd - ACCESS_LOG=- - ERROR_LOG=- + - REVERSE_PROXY=true volumes: - .data/CTFd/logs:/var/log/CTFd - .data/CTFd/uploads:/var/uploads @@ -25,6 +26,15 @@ services: default: internal: + nginx: + image: nginx:1.17 + volumes: + - ./conf/nginx/http.conf:/etc/nginx/nginx.conf + ports: + - 80:80 + depends_on: + - ctfd + db: image: mariadb:10.4.12 restart: always diff --git a/docs/conf.py b/docs/conf.py index c41aeca5a..2f54e776e 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -26,7 +26,7 @@ # The short X.Y version version = u"" # The full version, including alpha/beta/rc tags -release = u"2.4.3" +release = u"2.5.0" # -- General configuration --------------------------------------------------- diff --git a/docs/index.rst b/docs/index.rst index cf3d31016..243e14621 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -27,6 +27,7 @@ CTFd is written in Python and makes use of the Flask web framework. deployment configuration + management scoring themes plugins diff --git a/docs/management.rst b/docs/management.rst new file mode 100644 index 000000000..3ea348890 --- /dev/null +++ b/docs/management.rst @@ -0,0 +1,7 @@ +Management +========== + +Challenges +---------- + +The `ctfcli `_ tool can be used to manage challenges and sync them up to a specified CTFd instance via the CTFd API. It can be used as part of a build system or amongst multilpe users collaborating via version control. diff --git a/migrations/README b/migrations/README deleted file mode 100755 index 98e4f9c44..000000000 --- a/migrations/README +++ /dev/null @@ -1 +0,0 @@ -Generic single-database configuration. \ No newline at end of file diff --git a/migrations/env.py b/migrations/env.py old mode 100755 new mode 100644 index 96ab204b1..5f824edba --- a/migrations/env.py +++ b/migrations/env.py @@ -1,30 +1,31 @@ from __future__ import with_statement import logging +from logging.config import fileConfig -# from logging.config import fileConfig +from sqlalchemy import engine_from_config +from sqlalchemy import pool from alembic import context -# add your model's MetaData object here -# for 'autogenerate' support -# from myapp import mymodel -# target_metadata = mymodel.Base.metadata -from flask import current_app -from sqlalchemy import engine_from_config, pool - # this is the Alembic Config object, which provides # access to the values within the .ini file in use. config = context.config # Interpret the config file for Python logging. # This line sets up loggers basically. -# http://stackoverflow.com/questions/42427487/using-alembic-config-main-redirects-log-output -# fileConfig(config.config_file_name) +fileConfig(config.config_file_name, disable_existing_loggers=False) logger = logging.getLogger("alembic.env") +# add your model's MetaData object here +# for 'autogenerate' support +# from myapp import mymodel +# target_metadata = mymodel.Base.metadata +from flask import current_app + config.set_main_option( - "sqlalchemy.url", current_app.config.get("SQLALCHEMY_DATABASE_URI") + "sqlalchemy.url", + str(current_app.extensions["migrate"].db.engine.url).replace("%", "%%"), ) target_metadata = current_app.extensions["migrate"].db.metadata @@ -47,7 +48,7 @@ def run_migrations_offline(): """ url = config.get_main_option("sqlalchemy.url") - context.configure(url=url) + context.configure(url=url, target_metadata=target_metadata, literal_binds=True) with context.begin_transaction(): context.run_migrations() @@ -63,7 +64,7 @@ def run_migrations_online(): # this callback is used to prevent an auto-migration from being generated # when there are no changes to the schema - # reference: http://alembic.readthedocs.org/en/latest/cookbook.html + # reference: http://alembic.zzzcomputing.com/en/latest/cookbook.html def process_revision_directives(context, revision, directives): if getattr(config.cmd_opts, "autogenerate", False): script = directives[0] @@ -71,26 +72,22 @@ def process_revision_directives(context, revision, directives): directives[:] = [] logger.info("No changes in schema detected.") - engine = engine_from_config( + connectable = engine_from_config( config.get_section(config.config_ini_section), prefix="sqlalchemy.", poolclass=pool.NullPool, ) - connection = engine.connect() - context.configure( - connection=connection, - target_metadata=target_metadata, - compare_type=True, - process_revision_directives=process_revision_directives, - **current_app.extensions["migrate"].configure_args - ) + with connectable.connect() as connection: + context.configure( + connection=connection, + target_metadata=target_metadata, + process_revision_directives=process_revision_directives, + **current_app.extensions["migrate"].configure_args + ) - try: with context.begin_transaction(): context.run_migrations() - finally: - connection.close() if context.is_offline_mode(): diff --git a/package.json b/package.json index 7d8627d90..53330669b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "ctfd", - "version": "2.4.3", + "version": "2.5.0", "description": "CTFd is a Capture The Flag framework focusing on ease of use and customizability. It comes with everything you need to run a CTF and it's easy to customize with plugins and themes.", "main": "index.js", "directories": { diff --git a/requirements.txt b/requirements.txt index 0ffc196bb..27f11bba4 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,7 +2,7 @@ Flask==1.1.1 Werkzeug==0.16.0 Flask-SQLAlchemy==2.4.1 Flask-Caching==1.4.0 -Flask-Migrate==2.5.2 +Flask-Migrate==2.5.3 Flask-Script==2.0.6 SQLAlchemy==1.3.11 SQLAlchemy-Utils==0.36.0 diff --git a/tests/api/v1/test_challenges.py b/tests/api/v1/test_challenges.py index 4987f5c30..31dbed8e2 100644 --- a/tests/api/v1/test_challenges.py +++ b/tests/api/v1/test_challenges.py @@ -135,6 +135,25 @@ def test_api_challenges_get_admin(): destroy_ctfd(app) +def test_api_challenges_get_hidden_admin(): + """Can an admin see hidden challenges in API list response""" + app = create_ctfd() + with app.app_context(): + gen_challenge(app.db, state="hidden") + gen_challenge(app.db) + + with login_as_user(app, "admin") as admin: + challenges_list = admin.get("/api/v1/challenges", json="").get_json()[ + "data" + ] + assert len(challenges_list) == 1 + challenges_list = admin.get( + "/api/v1/challenges?view=admin", json="" + ).get_json()["data"] + assert len(challenges_list) == 2 + destroy_ctfd(app) + + def test_api_challenges_post_admin(): """Can a user post /api/v1/challenges if admin""" app = create_ctfd() diff --git a/tests/api/v1/test_teams.py b/tests/api/v1/test_teams.py index a67e6db52..a701a4698 100644 --- a/tests/api/v1/test_teams.py +++ b/tests/api/v1/test_teams.py @@ -696,6 +696,9 @@ def test_api_accessing_hidden_banned_users(): app.db.session.commit() with login_as_user(app, name="visible_user") as client: + list_teams = client.get("/api/v1/teams").get_json()["data"] + assert len(list_teams) == 0 + assert client.get("/api/v1/teams/1").status_code == 404 assert client.get("/api/v1/teams/1/solves").status_code == 404 assert client.get("/api/v1/teams/1/fails").status_code == 404 @@ -707,6 +710,10 @@ def test_api_accessing_hidden_banned_users(): assert client.get("/api/v1/teams/2/awards").status_code == 404 with login_as_user(app, name="admin") as client: + # Admins see hidden teams in lists + list_users = client.get("/api/v1/teams?view=admin").get_json()["data"] + assert len(list_users) == 2 + assert client.get("/api/v1/teams/1").status_code == 200 assert client.get("/api/v1/teams/1/solves").status_code == 200 assert client.get("/api/v1/teams/1/fails").status_code == 200 diff --git a/tests/api/v1/test_users.py b/tests/api/v1/test_users.py index f05c83b00..da2a8bc56 100644 --- a/tests/api/v1/test_users.py +++ b/tests/api/v1/test_users.py @@ -764,12 +764,19 @@ def test_api_accessing_hidden_users(): app.db.session.commit() with login_as_user(app, name="visible_user") as client: + list_users = client.get("/api/v1/users").get_json()["data"] + assert len(list_users) == 1 + assert client.get("/api/v1/users/3").status_code == 404 assert client.get("/api/v1/users/3/solves").status_code == 404 assert client.get("/api/v1/users/3/fails").status_code == 404 assert client.get("/api/v1/users/3/awards").status_code == 404 with login_as_user(app, name="admin") as client: + # Admins see the user in lists + list_users = client.get("/api/v1/users?view=admin").get_json()["data"] + assert len(list_users) == 3 + assert client.get("/api/v1/users/3").status_code == 200 assert client.get("/api/v1/users/3/solves").status_code == 200 assert client.get("/api/v1/users/3/fails").status_code == 200 @@ -788,12 +795,19 @@ def test_api_accessing_banned_users(): app.db.session.commit() with login_as_user(app, name="visible_user") as client: + list_users = client.get("/api/v1/users").get_json()["data"] + assert len(list_users) == 1 + assert client.get("/api/v1/users/3").status_code == 404 assert client.get("/api/v1/users/3/solves").status_code == 404 assert client.get("/api/v1/users/3/fails").status_code == 404 assert client.get("/api/v1/users/3/awards").status_code == 404 with login_as_user(app, name="admin") as client: + # Admins see the user in lists + list_users = client.get("/api/v1/users?view=admin").get_json()["data"] + assert len(list_users) == 3 + assert client.get("/api/v1/users/3").status_code == 200 assert client.get("/api/v1/users/3/solves").status_code == 200 assert client.get("/api/v1/users/3/fails").status_code == 200 diff --git a/tests/utils/test_sessions.py b/tests/utils/test_sessions.py index e5d8a0fed..376a2582e 100644 --- a/tests/utils/test_sessions.py +++ b/tests/utils/test_sessions.py @@ -1,4 +1,4 @@ -from tests.helpers import create_ctfd, destroy_ctfd +from tests.helpers import create_ctfd, destroy_ctfd, login_as_user, register_user def test_sessions_set_httponly(): @@ -19,3 +19,44 @@ def test_sessions_set_samesite(): cookie = dict(r.headers)["Set-Cookie"] assert "SameSite=" in cookie destroy_ctfd(app) + + +def test_session_invalidation_on_admin_password_change(): + app = create_ctfd() + with app.app_context(): + register_user(app) + with login_as_user(app, name="admin") as admin, login_as_user(app) as user: + + r = user.get("/settings") + assert r.status_code == 200 + + r = admin.patch("/api/v1/users/2", json={"password": "password2"}) + assert r.status_code == 200 + + r = user.get("/settings") + # User's password was changed + # They should be logged out + assert r.location.startswith("http://localhost/login") + assert r.status_code == 302 + destroy_ctfd(app) + + +def test_session_invalidation_on_user_password_change(): + app = create_ctfd() + with app.app_context(): + register_user(app) + with login_as_user(app) as user: + + r = user.get("/settings") + assert r.status_code == 200 + + data = {"confirm": "password", "password": "new_password"} + + r = user.patch("/api/v1/users/me", json=data) + assert r.status_code == 200 + + r = user.get("/settings") + # User initiated their own password change + # They should not be logged out + assert r.status_code == 200 + destroy_ctfd(app)