diff --git a/Makefile b/Makefile index e62b36b7b..46987ec89 100644 --- a/Makefile +++ b/Makefile @@ -27,7 +27,28 @@ create_db: start done @echo "Postgres is up" ## Creating database - docker-compose run --rm web python ../create_db.py + docker compose run --rm web python ./create_db.py + +.PHONY: create_db_diagram +create_db_diagram: + # Create new dot file showing current version of schema + eralchemy -i postgresql://openoversight:terriblepassword@localhost/openoversight-dev -o database/schema.new.dot + # Sort new version of schema file + sort database/schema.new.dot -o schema.new.dot.sorted + # Create old schema file if it does not exist and then sort it + touch database/schema.dot + sort database/schema.dot -o schema.dot.sorted + # Create a new diagram if there are changes, otherwise clean up files + @if diff schema.dot.sorted schema.new.dot.sorted > /dev/null 2>&1; then \ + echo 'No schema changes detected!'; \ + rm database/schema.new.dot; \ + else \ + echo 'Detected schema changes, making new DB relationship diagram!'; \ + mv database/schema.new.dot database/schema.dot; \ + dot -Tpng -o database/database_relationships.png -Grankdir=TB -Kdot database/schema.dot; \ + fi + # Remove all sorted files + rm schema.dot.sorted schema.new.dot.sorted .PHONY: dev dev: create_empty_secret build start create_db populate diff --git a/OpenOversight/app/auth/views.py b/OpenOversight/app/auth/views.py index 88e509e8e..dd57a0c47 100644 --- a/OpenOversight/app/auth/views.py +++ b/OpenOversight/app/auth/views.py @@ -34,7 +34,10 @@ ResetPasswordEmail, ) from OpenOversight.app.utils.auth import admin_required -from OpenOversight.app.utils.constants import KEY_APPROVE_REGISTRATIONS +from OpenOversight.app.utils.constants import ( + KEY_APPROVE_REGISTRATIONS, + KEY_AUTH_EMAIL_COOLDOWN_HOURS, +) from OpenOversight.app.utils.flask import sitemap from OpenOversight.app.utils.forms import set_dynamic_default from OpenOversight.app.utils.general import validate_redirect_url @@ -175,6 +178,20 @@ def confirm(token): @auth.route("/confirm") @login_required def resend_confirmation(): + now = datetime.now(timezone.utc) + if ( + current_user.last_confirmation_sent_at + and current_user.last_confirmation_sent_at + > now - current_app.config[KEY_AUTH_EMAIL_COOLDOWN_HOURS] + ): + flash( + "We already sent a confirmation email to you recently. Please try again later." + ) + return redirect(url_for("main.index")) + + current_user.last_confirmation_sent_at = now + db.session.commit() + token = current_user.generate_confirmation_token() EmailClient.send_email( ConfirmAccountEmail(current_user.email, user=current_user, token=token) @@ -211,11 +228,21 @@ def password_reset_request(): form = PasswordResetRequestForm() if form.validate_on_submit(): user = User.by_email(form.email.data).first() - if user: + now = datetime.now(timezone.utc) + if user and ( + not user.last_reset_sent_at + or user.last_reset_sent_at + < now - current_app.config[KEY_AUTH_EMAIL_COOLDOWN_HOURS] + ): + user.last_reset_sent_at = now + db.session.commit() + token = user.generate_reset_token() EmailClient.send_email( ResetPasswordEmail(user.email, user=user, token=token) ) + # Show message regardless of whether an email was sent to avoid revealing + # whether an account is associated with the email flash("An email with instructions to reset your password has been sent to you.") return redirect(url_for("auth.login")) else: diff --git a/OpenOversight/app/models/config.py b/OpenOversight/app/models/config.py index 94fdbf78d..36f8f246b 100644 --- a/OpenOversight/app/models/config.py +++ b/OpenOversight/app/models/config.py @@ -1,8 +1,15 @@ import json import os +from datetime import timedelta from OpenOversight.app.utils.constants import ( KEY_APPROVE_REGISTRATIONS, + KEY_AUTH_EMAIL_COOLDOWN_HOURS, + KEY_DATABASE_URI, + KEY_ENV, + KEY_ENV_DEV, + KEY_ENV_PROD, + KEY_ENV_TESTING, KEY_MAIL_PASSWORD, KEY_MAIL_PORT, KEY_MAIL_SERVER, @@ -37,7 +44,7 @@ def __init__(self): # DB Settings self.SQLALCHEMY_TRACK_MODIFICATIONS = False - self.SQLALCHEMY_DATABASE_URI = os.environ.get("SQLALCHEMY_DATABASE_URI") + self.SQLALCHEMY_DATABASE_URI = os.environ.get(KEY_DATABASE_URI) # Protocol Settings self.SITEMAP_URL_SCHEME = "http" @@ -79,6 +86,10 @@ def __init__(self): # User settings self.APPROVE_REGISTRATIONS = os.environ.get(KEY_APPROVE_REGISTRATIONS, False) + # Time a user must wait between consecutive confirm account or reset password requests + self.AUTH_EMAIL_COOLDOWN_HOURS = timedelta( + hours=int(os.environ.get(KEY_AUTH_EMAIL_COOLDOWN_HOURS, 1)) + ) # Map data with open("OpenOversight/map.json") as f: @@ -107,6 +118,8 @@ def __init__(self): self.SQLALCHEMY_ENGINE_OPTIONS = { "connect_args": {"cached_statements": 0}, } + # Prevent user env settings from interfering with tests + self.AUTH_EMAIL_COOLDOWN_HOURS = timedelta(hours=1) class ProductionConfig(BaseConfig): @@ -117,8 +130,8 @@ def __init__(self): config: dict[str, BaseConfig] = { - "development": DevelopmentConfig(), - "testing": TestingConfig(), - "production": ProductionConfig(), + KEY_ENV_DEV: DevelopmentConfig(), + KEY_ENV_TESTING: TestingConfig(), + KEY_ENV_PROD: ProductionConfig(), } -config["default"] = config.get(os.environ.get("ENV", ""), DevelopmentConfig()) +config["default"] = config.get(os.environ.get(KEY_ENV, ""), DevelopmentConfig()) diff --git a/OpenOversight/app/models/database.py b/OpenOversight/app/models/database.py index e419f71e0..c738378f7 100644 --- a/OpenOversight/app/models/database.py +++ b/OpenOversight/app/models/database.py @@ -58,6 +58,28 @@ Base: DeclarativeMeta = db.Model +class TZDateTime(TypeDecorator): + """ + Store tz-aware datetimes as tz-naive UTC datetimes in sqlite. + https://docs.sqlalchemy.org/en/20/core/custom_types.html#store-timezone-aware-timestamps-as-timezone-naive-utc + """ + + cache_ok = True + impl = db.DateTime + + def process_bind_param(self, value, dialect): + if dialect.name == "sqlite" and value is not None: + if not value.tzinfo or value.tzinfo.utcoffset(value) is None: + raise TypeError("tzinfo is required") + value = value.astimezone(timezone.utc).replace(tzinfo=None) + return value + + def process_result_value(self, value, dialect): + if dialect.name == "sqlite" and value is not None: + value = value.replace(tzinfo=timezone.utc) + return value + + class BaseModel(Base): __abstract__ = True @@ -944,6 +966,8 @@ class User(UserMixin, BaseModel): db.ForeignKey("users.id", ondelete="SET NULL", name="users_disabled_by_fkey"), unique=False, ) + last_confirmation_sent_at = db.Column(TZDateTime(timezone=True)) + last_reset_sent_at = db.Column(TZDateTime(timezone=True)) dept_pref = db.Column( db.Integer, db.ForeignKey("departments.id", name="users_dept_pref_fkey") diff --git a/OpenOversight/app/models/emails.py b/OpenOversight/app/models/emails.py index 4d5dd4149..da18d8e0e 100644 --- a/OpenOversight/app/models/emails.py +++ b/OpenOversight/app/models/emails.py @@ -86,8 +86,18 @@ def __init__(self, receiver: str, user, token: str): subject = ( f"{current_app.config[KEY_OO_MAIL_SUBJECT_PREFIX]} Confirm Your Account" ) - body = render_template(f"{self.EMAIL_PATH}confirm.txt", user=user, token=token) - html = render_template(f"{self.EMAIL_PATH}confirm.html", user=user, token=token) + body = render_template( + f"{self.EMAIL_PATH}confirm.txt", + user=user, + token=token, + help_email=current_app.config[KEY_OO_HELP_EMAIL], + ) + html = render_template( + f"{self.EMAIL_PATH}confirm.html", + user=user, + token=token, + help_email=current_app.config[KEY_OO_HELP_EMAIL], + ) super().__init__(body, html, subject, receiver) diff --git a/OpenOversight/app/templates/auth/email/confirm.html b/OpenOversight/app/templates/auth/email/confirm.html index 14a89e677..b7ebcd8e3 100644 --- a/OpenOversight/app/templates/auth/email/confirm.html +++ b/OpenOversight/app/templates/auth/email/confirm.html @@ -7,6 +7,7 @@
Alternatively, you can paste the following link in your browser's address bar:
{{ url_for('auth.confirm', token=token, _external=True) }}
+If you did not create this account, reach out to {{ help_email }} to disable your account.
Sincerely,
The OpenOversight Team
diff --git a/OpenOversight/app/templates/auth/email/confirm.txt b/OpenOversight/app/templates/auth/email/confirm.txt index 41abe7c57..126cb97cb 100644 --- a/OpenOversight/app/templates/auth/email/confirm.txt +++ b/OpenOversight/app/templates/auth/email/confirm.txt @@ -6,6 +6,8 @@ To confirm your account please click on the following link: {{ url_for('auth.confirm', token=token, _external=True) }} +If you did not create this account, reach out to {{ help_email }} to disable your account. + Sincerely, The OpenOversight Team diff --git a/OpenOversight/app/utils/constants.py b/OpenOversight/app/utils/constants.py index baa60b338..620090674 100644 --- a/OpenOversight/app/utils/constants.py +++ b/OpenOversight/app/utils/constants.py @@ -12,6 +12,7 @@ # Config Key Constants KEY_ALLOWED_EXTENSIONS = "ALLOWED_EXTENSIONS" KEY_APPROVE_REGISTRATIONS = "APPROVE_REGISTRATIONS" +KEY_AUTH_EMAIL_COOLDOWN_HOURS = "AUTH_EMAIL_COOLDOWN_HOURS" KEY_DATABASE_URI = "SQLALCHEMY_DATABASE_URI" KEY_ENV = "ENV" KEY_ENV_DEV = "development" diff --git a/OpenOversight/migrations/versions/2025-04-04-0842_edab445d1714_add_email_last_sent_columns.py b/OpenOversight/migrations/versions/2025-04-04-0842_edab445d1714_add_email_last_sent_columns.py new file mode 100644 index 000000000..99bdd6d4e --- /dev/null +++ b/OpenOversight/migrations/versions/2025-04-04-0842_edab445d1714_add_email_last_sent_columns.py @@ -0,0 +1,32 @@ +"""Add email last sent columns + +Revision ID: edab445d1714 +Revises: 99c50fc8d294 +Create Date: 2025-04-04 08:42:14.823515 + +""" + +import sqlalchemy as sa +from alembic import op + + +revision = "edab445d1714" +down_revision = "99c50fc8d294" + + +def upgrade(): + with op.batch_alter_table("users", schema=None) as batch_op: + batch_op.add_column( + sa.Column( + "last_confirmation_sent_at", sa.DateTime(timezone=True), nullable=True + ) + ) + batch_op.add_column( + sa.Column("last_reset_sent_at", sa.DateTime(timezone=True), nullable=True) + ) + + +def downgrade(): + with op.batch_alter_table("users", schema=None) as batch_op: + batch_op.drop_column("last_reset_sent_at") + batch_op.drop_column("last_confirmation_sent_at") diff --git a/OpenOversight/tests/routes/test_auth.py b/OpenOversight/tests/routes/test_auth.py index ed054353e..5259bf564 100644 --- a/OpenOversight/tests/routes/test_auth.py +++ b/OpenOversight/tests/routes/test_auth.py @@ -1,3 +1,4 @@ +from datetime import datetime, timedelta, timezone from http import HTTPStatus from unittest import TestCase from urllib.parse import urlparse @@ -203,6 +204,49 @@ def test_user_can_get_a_confirmation_token_resent(client, session): ) +@pytest.mark.parametrize( + "last_confirmation_sent_at,is_rate_limited", + [ + (None, False), + (datetime.now(timezone.utc) - timedelta(hours=2), False), + (datetime.now(timezone.utc) - timedelta(minutes=10), True), + ], +) +def test_user_rate_limited_if_resending_confirmation_too_soon( + client, session, last_confirmation_sent_at, is_rate_limited +): + # Should only send email if not rate limited + log_capture = TestCase.assertNoLogs if is_rate_limited else TestCase.assertLogs + + with ( + current_app.test_request_context(), + log_capture(current_app.logger) as log, + ): + _, user = login_user(client) + user.last_confirmation_sent_at = last_confirmation_sent_at + session.commit() + + rv = client.get(url_for("auth.resend_confirmation"), follow_redirects=True) + + if not is_rate_limited: + assert b"A new confirmation email has been sent to you." in rv.data + assert ( + f"{current_app.config[KEY_OO_MAIL_SUBJECT_PREFIX]} Confirm Your Account" + in str(log.output) + ) + + # check that confirmation time was updated + assert user.last_confirmation_sent_at > datetime.now( + timezone.utc + ) - timedelta(seconds=1) + else: + assert ( + b"We already sent a confirmation email to you recently. Please try again later." + in rv.data + ) + assert user.last_confirmation_sent_at == last_confirmation_sent_at + + def test_user_can_get_password_reset_token_sent(client, session): with ( current_app.test_request_context(), @@ -247,6 +291,49 @@ def test_user_can_get_password_reset_token_sent_with_differently_cased_email( ) +@pytest.mark.parametrize( + "last_reset_sent_at,is_rate_limited", + [ + (None, False), + (datetime.now(timezone.utc) - timedelta(hours=2), False), + (datetime.now(timezone.utc) - timedelta(minutes=10), True), + ], +) +def test_user_rate_limited_if_resending_reset_too_soon( + client, session, last_reset_sent_at, is_rate_limited +): + with ( + current_app.test_request_context(), + TestCase.assertLogs(current_app.logger) as log, + ): + user = User.query.filter_by(is_administrator=True).first() + user.last_reset_sent_at = last_reset_sent_at + session.commit() + + form = PasswordResetRequestForm(email=user.email) + rv = client.post( + url_for("auth.password_reset_request"), + data=form.data, + follow_redirects=True, + ) + + assert b"An email with instructions to reset your password" in rv.data + if not is_rate_limited: + assert ( + f"{current_app.config[KEY_OO_MAIL_SUBJECT_PREFIX]} Reset Your Password" + in str(log.output) + ) + assert user.last_reset_sent_at > datetime.now(timezone.utc) - timedelta( + seconds=1 + ) + else: + assert ( + f"{current_app.config[KEY_OO_MAIL_SUBJECT_PREFIX]} Reset Your Password" + not in str(log.output) + ) + assert user.last_reset_sent_at == last_reset_sent_at + + def test_user_can_get_reset_password_with_valid_token(client, session): with current_app.test_request_context(): user = User.query.filter_by(is_administrator=True).first() diff --git a/OpenOversight/tests/test_models.py b/OpenOversight/tests/test_models.py index cce40d878..760ec70b4 100644 --- a/OpenOversight/tests/test_models.py +++ b/OpenOversight/tests/test_models.py @@ -3,6 +3,7 @@ import time from decimal import Decimal from unittest.mock import MagicMock +from zoneinfo import ZoneInfo import pytest from sqlalchemy import and_ @@ -18,6 +19,7 @@ Link, Location, Officer, + TZDateTime, User, ) from OpenOversight.app.utils.choices import STATE_CHOICES @@ -577,4 +579,61 @@ def test_currency_type_decorator(dialect_name, original_value, intermediate_valu assert intermediate_value == value value = currency.process_result_value(value, dialect) + + +@pytest.mark.parametrize( + "by_attr, at_attr", + [ + ("approved_by", "approved_at"), + ("confirmed_by", "confirmed_at"), + ("disabled_by", "disabled_at"), + ], +) +def test_user_constraints(mockdata, session, faker, by_attr, at_attr): + now = datetime.datetime.now(datetime.timezone.utc) + user = User( + email=faker.company_email(), + username=faker.word(), + password=faker.word(), + ) + session.add(user) + session.commit() + + setattr(user, at_attr, now) + setattr(user, by_attr, 1) + session.commit() + + # Both or neither "_at" and "_by" must be set + setattr(user, at_attr, None) + setattr(user, by_attr, 1) + with pytest.raises(IntegrityError): + session.commit() + + +@pytest.mark.parametrize( + "dialect_name,original_value,intermediate_value", + [ + ("sqlite", None, None), + ( + "sqlite", + datetime.datetime(1980, 1, 1, hour=0, tzinfo=ZoneInfo("America/Chicago")), + datetime.datetime(1980, 1, 1, hour=6), + ), + ("postgresql", None, None), + ( + "postgresql", + datetime.datetime(1980, 1, 1, hour=0, tzinfo=ZoneInfo("America/Chicago")), + datetime.datetime(1980, 1, 1, hour=0, tzinfo=ZoneInfo("America/Chicago")), + ), + ], +) +def test_tzdatetime_type_decorator(dialect_name, original_value, intermediate_value): + tzdt = TZDateTime(timezone=True) + dialect = MagicMock() + dialect.name = dialect_name + + value = tzdt.process_bind_param(original_value, dialect) + assert intermediate_value == value + + value = tzdt.process_result_value(value, dialect) assert original_value == value