Skip to content

Commit

Permalink
Finish audit fields for users table (#1129)
Browse files Browse the repository at this point in the history
## Fixes issue
#1004

## Description of Changes
Added `disabled_by`, `disabled_at`, `approved_at`, `approved_by`, and
`confirmed_at`, and `confirmed_by` fields to the `users` table so that
we could keep track of users actions on the platform at a more granular
level. I also removed the previously mentioned columns corresponding
boolean columns.

## Tests and Linting
- [x] This branch is up-to-date with the `develop` branch.
- [x] `pytest` passes on my local development environment.
- [x] `pre-commit` passes on my local development environment.
- [x] The `flask db upgrade` and `flask db downgrade` functions work for
both migrations.

```console
I have no name!@c38d30934aa7:/usr/src/app$ %
(env) michaelp@MacBook-Air-18 OpenOversight % docker exec -it openoversight-web-1 bash
I have no name!@5027495d31c0:/usr/src/app$ flask db downgrade
...
INFO  [alembic.runtime.migration] Running downgrade 99c50fc8d294 -> bf254c0961ca, complete audit field addition to users
I have no name!@5027495d31c0:/usr/src/app$ flask db downgrade
...
INFO  [alembic.runtime.migration] Running downgrade bf254c0961ca -> 5865f488470c, add remaining audit fields for users table
I have no name!@5027495d31c0:/usr/src/app$ flask db downgrade
...
INFO  [alembic.runtime.migration] Running downgrade 5865f488470c -> 939ea0f2b26d, change salary column types
I have no name!@5027495d31c0:/usr/src/app$ flask db upgrade
...
INFO  [alembic.runtime.migration] Running upgrade 939ea0f2b26d -> 5865f488470c, change salary column types
INFO  [alembic.runtime.migration] Running upgrade 5865f488470c -> bf254c0961ca, add remaining audit fields for users table
INFO  [alembic.runtime.migration] Running upgrade bf254c0961ca -> 99c50fc8d294, complete audit field addition to users
I have no name!@5027495d31c0:/usr/src/app$
```
  • Loading branch information
michplunkett authored Oct 24, 2024
1 parent b47d7e4 commit 0097663
Show file tree
Hide file tree
Showing 14 changed files with 508 additions and 81 deletions.
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"
KEY_DATABASE_URI = "SQLALCHEMY_DATABASE_URI"
KEY_ENV = "ENV"
KEY_ENV_DEV = "development"
Expand Down
Loading

0 comments on commit 0097663

Please sign in to comment.