Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Finish audit fields for users table #1129

Merged
merged 49 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from 48 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
c2be7e2
Update database.py
michplunkett Oct 9, 2024
9ab33f5
Create 2024-10-09-1859_0a7c591e13b9_add_remaining_audit_fields_to_use…
michplunkett Oct 9, 2024
ef4fc79
Update 2024-10-09-1859_0a7c591e13b9_add_remaining_audit_fields_to_use…
michplunkett Oct 9, 2024
d0b34fc
Delete 2024-10-09-1859_0a7c591e13b9_add_remaining_audit_fields_to_use…
michplunkett Oct 9, 2024
9dcb384
Update database.py
michplunkett Oct 9, 2024
a098944
Create 2024-10-09-1927_bf254c0961ca_add_remaining_audit_fields_for_us…
michplunkett Oct 9, 2024
70f883f
Update database.py
michplunkett Oct 9, 2024
8e065c4
Update 2024-10-09-1927_bf254c0961ca_add_remaining_audit_fields_for_us…
michplunkett Oct 9, 2024
37472ce
Update database.py
michplunkett Oct 9, 2024
0e0e2f7
Create 2024-10-09-2000_99c50fc8d294_complete_audit_field_addition_to_…
michplunkett Oct 9, 2024
5de0e19
Update 2024-10-09-2000_99c50fc8d294_complete_audit_field_addition_to_…
michplunkett Oct 9, 2024
171b657
Update database.py
michplunkett Oct 9, 2024
930c5f3
Update views.py
michplunkett Oct 9, 2024
450e715
Update users.html
michplunkett Oct 9, 2024
a4af572
Update test_user_api.py
michplunkett Oct 9, 2024
46fd1e4
Update test_user_api.py
michplunkett Oct 9, 2024
4842a18
Update users.html
michplunkett Oct 9, 2024
be8caa5
Update test_models.py
michplunkett Oct 9, 2024
102f222
Blep
michplunkett Oct 9, 2024
04c4906
Update conftest.py
michplunkett Oct 9, 2024
464d65d
Disabling
michplunkett Oct 9, 2024
ceda267
Update test_auth.py
michplunkett Oct 9, 2024
faf125f
Update test_user_api.py
michplunkett Oct 9, 2024
3cd4bc9
Update test_auth.py
michplunkett Oct 9, 2024
c03bb4a
Update test_user_api.py
michplunkett Oct 9, 2024
0d7669d
Update sort.html
michplunkett Oct 9, 2024
acf6575
Update views.py
michplunkett Oct 9, 2024
486c418
Update views.py
michplunkett Oct 9, 2024
837849d
Update test_user_api.py
michplunkett Oct 9, 2024
48fbab5
Update views.py
michplunkett Oct 9, 2024
851d9d7
Update test_user_api.py
michplunkett Oct 9, 2024
8ea8a46
Update views.py
michplunkett Oct 9, 2024
08d65a9
Update test_user_api.py
michplunkett Oct 10, 2024
b446385
Update database.py
michplunkett Oct 10, 2024
7bff35f
Update views.py
michplunkett Oct 10, 2024
e77393d
Update test_user_api.py
michplunkett Oct 10, 2024
0d7dbc1
Update users.html
michplunkett Oct 10, 2024
3393a51
Add KEY_APPROVE_REGISTRATIONS
michplunkett Oct 11, 2024
f022e77
Update views.py
michplunkett Oct 11, 2024
4f0c6c1
Update views.py
michplunkett Oct 11, 2024
9800ebd
Update views.py
michplunkett Oct 11, 2024
d05da42
Update views.py
michplunkett Oct 11, 2024
4588b10
Update views.py
michplunkett Oct 11, 2024
42f7d27
Update views.py
michplunkett Oct 11, 2024
c9ad779
Update views.py
michplunkett Oct 11, 2024
83263fe
Update views.py
michplunkett Oct 11, 2024
fc20b27
Update views.py
michplunkett Oct 11, 2024
709b8b0
Update test_models.py
michplunkett Oct 16, 2024
dadd0b1
Merge branch 'develop' into finish-audit-fields-users
michplunkett Oct 24, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 41 additions & 12 deletions OpenOversight/app/auth/views.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from datetime import datetime, timezone
from http import HTTPMethod, HTTPStatus

from flask import (
Expand Down Expand Up @@ -34,6 +35,7 @@
ResetPasswordEmail,
)
from OpenOversight.app.utils.auth import admin_required
from OpenOversight.app.utils.constants import KEY_APPROVE_REGISTRATIONS
from OpenOversight.app.utils.forms import set_dynamic_default
from OpenOversight.app.utils.general import validate_redirect_url

Expand All @@ -57,7 +59,8 @@ def static_routes():
def before_request():
if (
current_user.is_authenticated
and not current_user.confirmed
and not current_user.confirmed_at
and not current_user.confirmed_by
and request.endpoint
and request.endpoint[:5] != "auth."
and request.endpoint not in ["static", "bootstrap.static"]
Expand All @@ -67,9 +70,15 @@ def before_request():

@auth.route("/unconfirmed")
def unconfirmed():
if current_user.is_anonymous or current_user.confirmed:
if current_user.is_anonymous or (
current_user.confirmed_at and current_user.confirmed_by
):
return redirect(url_for("main.index"))
if current_app.config["APPROVE_REGISTRATIONS"] and not current_user.approved:
if (
current_app.config[KEY_APPROVE_REGISTRATIONS]
and not current_user.approved_at
and not current_user.approved_by
):
return render_template("auth/unapproved.html")
else:
return render_template("auth/unconfirmed.html")
Expand Down Expand Up @@ -112,11 +121,16 @@ def register():
email=form.email.data,
username=form.username.data,
password=form.password.data,
approved=False if current_app.config["APPROVE_REGISTRATIONS"] else True,
approved_at=None
if current_app.config[KEY_APPROVE_REGISTRATIONS]
else datetime.now(timezone.utc),
approved_by=None
if current_app.config[KEY_APPROVE_REGISTRATIONS]
else User.query.filter_by(is_administrator=True).first().id,
)
db.session.add(user)
db.session.commit()
if current_app.config["APPROVE_REGISTRATIONS"]:
if current_app.config[KEY_APPROVE_REGISTRATIONS]:
admins = User.query.filter_by(is_administrator=True).all()
for admin in admins:
EmailClient.send_email(
Expand All @@ -141,9 +155,11 @@ def register():
@auth.route("/confirm/<token>", methods=[HTTPMethod.GET])
@login_required
def confirm(token):
if current_user.confirmed:
if current_user.confirmed_at and current_user.confirmed_by:
return redirect(url_for("main.index"))
if current_user.confirm(token):
if current_user.confirm(
token, User.query.filter_by(is_administrator=True).first().id
):
admins = User.query.filter_by(is_administrator=True).all()
for admin in admins:
EmailClient.send_email(
Expand Down Expand Up @@ -313,18 +329,31 @@ def edit_user(user_id):
flash("You cannot edit your own account!")
form = EditUserForm(obj=user)
return render_template("auth/user.html", user=user, form=form)
already_approved = user.approved
already_approved = (
user.approved_at is not None and user.approved_by is not None
)
if form.approved.data:
user.approve_user(current_user.id)

if form.confirmed.data:
user.confirm_user(current_user.id)

if form.is_disabled.data:
user.disable_user(current_user.id)

form.populate_obj(user)
db.session.add(user)
db.session.commit()

# automatically send a confirmation email when approving an
# unconfirmed user
if (
current_app.config["APPROVE_REGISTRATIONS"]
current_app.config[KEY_APPROVE_REGISTRATIONS]
and not already_approved
and user.approved
and not user.confirmed
and user.approved_at
and user.approved_by
and not user.confirmed_at
and not user.confirmed_by
):
admin_resend_confirmation(user)

Expand Down Expand Up @@ -353,7 +382,7 @@ def delete_user(user_id):


def admin_resend_confirmation(user):
if user.confirmed:
if user.confirmed_at and current_user.confirmed_by:
flash(f"User {user.username} is already confirmed.")
else:
token = user.generate_confirmation_token()
Expand Down
3 changes: 2 additions & 1 deletion OpenOversight/app/models/config.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import os

from OpenOversight.app.utils.constants import (
KEY_APPROVE_REGISTRATIONS,
KEY_DATABASE_URI,
KEY_ENV,
KEY_ENV_DEV,
Expand Down Expand Up @@ -80,7 +81,7 @@ def __init__(self):
self.MAX_CONTENT_LENGTH = 50 * MEGABYTE

# User settings
self.APPROVE_REGISTRATIONS = os.environ.get("APPROVE_REGISTRATIONS", False)
self.APPROVE_REGISTRATIONS = os.environ.get(KEY_APPROVE_REGISTRATIONS, False)


class DevelopmentConfig(BaseConfig):
Expand Down
79 changes: 72 additions & 7 deletions OpenOversight/app/models/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import re
import time
import uuid
from datetime import date, datetime
from datetime import date, datetime, timezone
from typing import List, Optional

from authlib.jose import JoseError, JsonWebToken
Expand Down Expand Up @@ -722,8 +722,18 @@ class User(UserMixin, BaseModel):
email = db.Column(db.String(64), unique=True, index=True)
username = db.Column(db.String(64), unique=True, index=True)
password_hash = db.Column(db.String(128))
confirmed = db.Column(db.Boolean, default=False)
approved = db.Column(db.Boolean, default=False)
confirmed_at = db.Column(db.DateTime(timezone=True))
confirmed_by = db.Column(
db.Integer,
db.ForeignKey("users.id", ondelete="SET NULL", name="users_confirmed_by_fkey"),
unique=False,
)
approved_at = db.Column(db.DateTime(timezone=True))
approved_by = db.Column(
db.Integer,
db.ForeignKey("users.id", ondelete="SET NULL", name="users_approved_by_fkey"),
unique=False,
)
is_area_coordinator = db.Column(db.Boolean, default=False)
ac_department_id = db.Column(
db.Integer, db.ForeignKey("departments.id", name="users_ac_department_id_fkey")
Expand All @@ -734,7 +744,13 @@ class User(UserMixin, BaseModel):
foreign_keys=[ac_department_id],
)
is_administrator = db.Column(db.Boolean, default=False)
is_disabled = db.Column(db.Boolean, default=False)
disabled_at = db.Column(db.DateTime(timezone=True))
disabled_by = db.Column(
db.Integer,
db.ForeignKey("users.id", ondelete="SET NULL", name="users_disabled_by_fkey"),
unique=False,
)

dept_pref = db.Column(
db.Integer, db.ForeignKey("departments.id", name="users_dept_pref_fkey")
)
Expand Down Expand Up @@ -769,6 +785,21 @@ class User(UserMixin, BaseModel):
unique=False,
)

__table_args__ = (
CheckConstraint(
"(disabled_at IS NULL and disabled_by IS NULL) or (disabled_at IS NOT NULL and disabled_by IS NOT NULL)",
name="users_disabled_constraint",
),
CheckConstraint(
"(confirmed_at IS NULL and confirmed_by IS NULL) or (confirmed_at IS NOT NULL and confirmed_by IS NOT NULL)",
name="users_confirmed_constraint",
),
CheckConstraint(
"(approved_at IS NULL and approved_by IS NULL) or (approved_at IS NOT NULL and approved_by IS NOT NULL)",
name="users_approved_constraint",
),
)

def is_admin_or_coordinator(self, department: Optional[Department]) -> bool:
return self.is_administrator or (
department is not None
Expand Down Expand Up @@ -825,7 +856,7 @@ def generate_confirmation_token(self, expiration=3600):
payload = {"confirm": self.uuid}
return self._jwt_encode(payload, expiration).decode(ENCODING_UTF_8)

def confirm(self, token):
def confirm(self, token, confirming_user_id: int):
try:
data = self._jwt_decode(token)
except JoseError as e:
Expand All @@ -838,7 +869,8 @@ def confirm(self, token):
self.uuid,
)
return False
self.confirmed = True
self.confirmed_at = datetime.now(timezone.utc)
self.confirmed_by = confirming_user_id
db.session.add(self)
db.session.commit()
return True
Expand Down Expand Up @@ -891,7 +923,40 @@ def get_id(self):
@property
def is_active(self):
"""Override UserMixin.is_active to prevent disabled users from logging in."""
return not self.is_disabled
return not self.disabled_at

def approve_user(self, approving_user_id: int):
"""Handle approving logic."""
if self.approved_at or self.approved_by:
return False

self.approved_at = datetime.now(timezone.utc)
self.approved_by = approving_user_id
db.session.add(self)
db.session.commit()
return True

def confirm_user(self, confirming_user_id: int):
"""Handle confirming logic."""
if self.confirmed_at or self.confirmed_by:
return False

self.confirmed_at = datetime.now(timezone.utc)
self.confirmed_by = confirming_user_id
db.session.add(self)
db.session.commit()
return True

def disable_user(self, disabling_user_id: int):
"""Handle disabling logic."""
if self.disabled_at or self.disabled_by:
return False

self.disabled_at = datetime.now(timezone.utc)
self.disabled_by = disabling_user_id
db.session.add(self)
db.session.commit()
return True

def __repr__(self):
return f"<User {self.username!r}>"
6 changes: 3 additions & 3 deletions OpenOversight/app/templates/auth/users.html
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ <h1 class="page-header">Users</h1>
<a href="mailto:{{ user.email }}">{{ user.email }}</a>
</td>
<td>
{% if user.is_disabled %}
{% if user.disabled_at and user.disabled_by %}
Disabled
{% elif user.confirmed %}
{% elif user.confirmed_at and user.confirmed_by %}
Active
{% elif user.approved %}
{% elif user.approved_at and user.approved_by %}
Pending Confirmation
{% else %}
Pending Approval
Expand Down
4 changes: 2 additions & 2 deletions OpenOversight/app/templates/cop_face.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
{% block content %}
<div class="container theme-showcase py-5" role="main">
{% if current_user and current_user.is_authenticated %}
{% if image and not current_user.is_disabled %}
{% if image and not current_user.disabled_at and not current_user_disabled_by %}
<div class="row">
<div class="text-center">
<h1>
Expand Down Expand Up @@ -160,7 +160,7 @@ <h2>
</div>
</div>
</div>
{% elif current_user.is_disabled %}
{% elif current_user.disabled_at and current_user.disabled_by %}
<h3>Your account has been disabled due to too many incorrect classifications/tags!</h3>
<p>
<a href="mailto:[email protected]"
Expand Down
4 changes: 2 additions & 2 deletions OpenOversight/app/templates/profile.html
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ <h3 class="card-title">User Statistics</h3>
role="button">Show leaderboard</a>
</p>
<h3>Account Status</h3>
{% if user.is_disabled %}
{% if user.disabled_at and user.disabled_by %}
<p>Disabled</p>
{% elif user.is_disabled == False %}
{% elif not user.disabled_at and not user.disabled_by %}
<p>Enabled</p>
{% endif %}
{% if current_user.is_administrator and not user.is_administrators %}
Expand Down
4 changes: 2 additions & 2 deletions OpenOversight/app/templates/sort.html
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
{% block content %}
<div class="container theme-showcase py-5" role="main">
{% if current_user and current_user.is_authenticated %}
{% if image and current_user.is_disabled == False %}
{% if image and not current_user.disabled_at and not current_user.disabled_by %}
<div class="row">
<div class="text-center">
<h1>
Expand Down Expand Up @@ -70,7 +70,7 @@ <h1>
<img class="img-responsive" src="{{ path }}" alt="Picture to be sorted">
</div>
</div>
{% elif current_user.is_disabled == True %}
{% elif current_user.disabled_at and current_user.disabled_by %}
<h3>Your account has been disabled due to too many incorrect classifications/tags!</h3>
<p>
<a href="mailto:[email protected]"
Expand Down
1 change: 1 addition & 0 deletions OpenOversight/app/utils/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

# Config Key Constants
KEY_ALLOWED_EXTENSIONS = "ALLOWED_EXTENSIONS"
KEY_APPROVE_REGISTRATIONS = "APPROVE_REGISTRATIONS"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this as a constant because it was being used throughout the tests.

KEY_DATABASE_URI = "SQLALCHEMY_DATABASE_URI"
KEY_ENV = "ENV"
KEY_ENV_DEV = "development"
Expand Down
Loading
Loading