From f357615bfb608456887b1863aa34a39ebd1d73ef Mon Sep 17 00:00:00 2001
From: Kevin Morris <kevr@0cost.org>
Date: Tue, 14 Dec 2021 14:31:46 -0800
Subject: [PATCH] change(users.validate): users can't edit their own account
 types

This commit also decouples testing regarding this feature
into several test functions.

Signed-off-by: Kevin Morris <kevr@0cost.org>

bump

Signed-off-by: Kevin Morris <kevr@0cost.org>
---
 aurweb/users/validate.py             |  18 ++-
 templates/partials/account_form.html |  35 ++---
 test/test_accounts_routes.py         | 221 ++++++++++++++-------------
 3 files changed, 146 insertions(+), 128 deletions(-)

diff --git a/aurweb/users/validate.py b/aurweb/users/validate.py
index 4959e316..5b2f8819 100644
--- a/aurweb/users/validate.py
+++ b/aurweb/users/validate.py
@@ -11,13 +11,16 @@ from typing import List, Optional, Tuple
 from fastapi import Request
 from sqlalchemy import and_
 
-from aurweb import config, db, l10n, models, time, util
+from aurweb import config, db, l10n, logging, models, time, util
+from aurweb.auth import creds
 from aurweb.captcha import get_captcha_answer, get_captcha_salts, get_captcha_token
 from aurweb.exceptions import ValidationError
 from aurweb.models import account_type as at
 from aurweb.models.account_type import ACCOUNT_TYPE_NAME
 from aurweb.models.ssh_pub_key import get_fingerprint
 
+logger = logging.get_logger(__name__)
+
 
 def invalid_fields(E: str = str(), U: str = str(), **kwargs) \
         -> Optional[Tuple[bool, List[str]]]:
@@ -171,26 +174,31 @@ def invalid_account_type(T: int = None, request: Request = None,
                          _: l10n.Translator = None,
                          **kwargs) -> None:
     if T is not None and (T := int(T)) != user.AccountTypeID:
+        has_cred = request.user.has_credential(creds.ACCOUNT_CHANGE_TYPE)
         if T not in ACCOUNT_TYPE_NAME:
             raise ValidationError(["Invalid account type provided."])
-        elif not request.user.is_elevated():
+        elif not has_cred:
             raise ValidationError([
                 "You do not have permission to change account types."])
 
         credential_checks = {
             at.USER_ID: request.user.is_trusted_user,
             at.TRUSTED_USER_ID: request.user.is_trusted_user,
-            at.DEVELOPER_ID: lambda: request.user.is_developer(),
+            at.DEVELOPER_ID: request.user.is_developer,
             at.TRUSTED_USER_AND_DEV_ID: (lambda: request.user.is_trusted_user()
                                          and request.user.is_developer())
         }
         credential_check = credential_checks.get(T)
 
-        if not credential_check():
-            name = ACCOUNT_TYPE_NAME.get(T)
+        name = ACCOUNT_TYPE_NAME.get(T)
+        if not credential_check() or request.user == user:
             error = _("You do not have permission to change "
                       "this user's account type to %s.") % name
             raise ValidationError([error])
+        else:
+            logger.debug(f"Trusted User '{request.user.Username}' has "
+                         f"modified '{user.Username}' account's type to"
+                         f" {name}.")
 
 
 def invalid_captcha(captcha_salt: str = None, captcha: str = None, **kwargs) \
diff --git a/templates/partials/account_form.html b/templates/partials/account_form.html
index 37bb85c4..fc46f8c4 100644
--- a/templates/partials/account_form.html
+++ b/templates/partials/account_form.html
@@ -43,24 +43,25 @@
         </p>
 
         {% if request.user.has_credential(creds.ACCOUNT_CHANGE_TYPE) %}
-            <p>
-                <label for="id_type">
-                    {% trans %}Account Type{% endtrans %}:
-                </label>
-                <select name="T" id="id_type">
-                    {% for value, type in account_types %}
-                        <option value="{{ value }}"
-                                {% if request.user.AccountType.ID == value %}
-                                selected="selected"
-                                {% endif %}
-                        >
-                            {{ type | tr }}
-                        </option>
-                    {% endfor %}
-
-                </select>
-            </p>
+            {% if request.user != user %}
+                <p>
+                    <label for="id_type">
+                        {% trans %}Account Type{% endtrans %}:
+                    </label>
+                    <select name="T" id="id_type">
+                        {% for value, type in account_types %}
+                            <option value="{{ value }}"
+                                    {% if request.user.AccountType.ID == value %}
+                                    selected="selected"
+                                    {% endif %}
+                            >
+                                {{ type | tr }}
+                            </option>
+                        {% endfor %}
 
+                    </select>
+                </p>
+            {% endif %}
             <p>
                 <label for="id_suspended">
                     {% trans %}Account Suspended{% endtrans %}:
diff --git a/test/test_accounts_routes.py b/test/test_accounts_routes.py
index d3435089..57a2d38e 100644
--- a/test/test_accounts_routes.py
+++ b/test/test_accounts_routes.py
@@ -3,6 +3,7 @@ import tempfile
 
 from datetime import datetime
 from http import HTTPStatus
+from logging import DEBUG
 from subprocess import Popen
 
 import lxml.html
@@ -14,7 +15,8 @@ from aurweb import captcha, db, logging
 from aurweb.asgi import app
 from aurweb.db import create, query
 from aurweb.models.accepted_term import AcceptedTerm
-from aurweb.models.account_type import DEVELOPER_ID, TRUSTED_USER_AND_DEV_ID, TRUSTED_USER_ID, USER_ID, AccountType
+from aurweb.models.account_type import (DEVELOPER_ID, TRUSTED_USER, TRUSTED_USER_AND_DEV_ID, TRUSTED_USER_ID, USER_ID,
+                                        AccountType)
 from aurweb.models.ban import Ban
 from aurweb.models.session import Session
 from aurweb.models.ssh_pub_key import SSHPubKey, get_fingerprint
@@ -54,13 +56,18 @@ def client() -> TestClient:
     yield TestClient(app=app)
 
 
+def create_user(username: str) -> User:
+    email = f"{username}@example.org"
+    user = create(User, Username=username, Email=email,
+                  Passwd="testPassword",
+                  AccountTypeID=USER_ID)
+    return user
+
+
 @pytest.fixture
 def user() -> User:
     with db.begin():
-        user = create(User, Username=TEST_USERNAME, Email=TEST_EMAIL,
-                      RealName="Test UserZ", Passwd="testPassword",
-                      IRCNick="testZ", AccountTypeID=USER_ID)
-
+        user = create_user(TEST_USERNAME)
     yield user
 
 
@@ -986,118 +993,130 @@ def test_post_account_edit_password(client: TestClient, user: User):
     assert user.valid_password("newPassword")
 
 
-def test_post_account_edit_account_types(client: TestClient, user: User):
-    request = Request()
-    sid = user.login(request, "testPassword")
-    cookies = {"AURSID": sid}
+def test_post_account_edit_self_type_as_user(client: TestClient, user: User):
+    cookies = {"AURSID": user.login(Request(), "testPassword")}
     endpoint = f"/account/{user.Username}/edit"
 
-    # As a normal user, we cannot see the "Account Type:" input.
     with client as request:
         resp = request.get(endpoint, cookies=cookies)
     assert resp.status_code == int(HTTPStatus.OK)
     assert "id_type" not in resp.text
 
-    # Invalid account types return an error.
-    post_data = {
-        "U": user.Username,
-        "E": user.Email,
-        "T": 0,  # Invalid type ID
-        "passwd": "testPassword"
-    }
-    with client as request:
-        resp = request.post(endpoint, data=post_data, cookies=cookies)
-    assert resp.status_code == int(HTTPStatus.BAD_REQUEST)
-    errors = get_errors(resp.text)
-    expected = "Invalid account type provided."
-    assert errors[0].text.strip() == expected
-
-    # Nor can we change any account types.
-    post_data = {
+    data = {
         "U": user.Username,
         "E": user.Email,
         "T": TRUSTED_USER_ID,
         "passwd": "testPassword"
     }
     with client as request:
-        resp = request.post(endpoint, data=post_data, cookies=cookies)
+        resp = request.post(endpoint, data=data, cookies=cookies)
     assert resp.status_code == int(HTTPStatus.BAD_REQUEST)
+
     errors = get_errors(resp.text)
     expected = "You do not have permission to change account types."
     assert errors[0].text.strip() == expected
 
-    # Change user from USER_ID to TRUSTED_USER_ID.
+
+def test_post_account_edit_other_user_as_user(client: TestClient, user: User):
     with db.begin():
-        user.AccountTypeID = TRUSTED_USER_ID
+        user2 = create_user("test2")
+
+    cookies = {"AURSID": user.login(Request(), "testPassword")}
+    endpoint = f"/account/{user2.Username}/edit"
 
-    # As a trusted user, we can see the "Account Type:" input.
     with client as request:
-        resp = request.get(endpoint, cookies=cookies)
+        resp = request.get(endpoint, cookies=cookies,
+                           allow_redirects=False)
+    assert resp.status_code == int(HTTPStatus.SEE_OTHER)
+    assert resp.headers.get("location") == f"/account/{user2.Username}"
+
+
+def test_post_account_edit_self_type_as_tu(client: TestClient, tu_user: User):
+    cookies = {"AURSID": tu_user.login(Request(), "testPassword")}
+    endpoint = f"/account/{tu_user.Username}/edit"
+
+    # We cannot see the Account Type field on our own edit page.
+    with client as request:
+        resp = request.get(endpoint, cookies=cookies, allow_redirects=False)
     assert resp.status_code == int(HTTPStatus.OK)
-    assert "id_type" in resp.text
+    assert "id_type" not in resp.text
 
-    # As a trusted user, we cannot change account type to DEVELOPER_ID.
-    post_data = {
-        "U": user.Username,
-        "E": user.Email,
-        "T": DEVELOPER_ID,
-        "passwd": "testPassword"
-    }
-    with client as request:
-        resp = request.post(endpoint, data=post_data, cookies=cookies)
-    assert resp.status_code == int(HTTPStatus.BAD_REQUEST)
-    errors = get_errors(resp.text)
-    expected = ("You do not have permission to change "
-                "this user's account type to Developer.")
-    assert errors[0].text.strip() == expected
-
-    # However, we can modify our account type to USER_ID.
-    post_data = {
-        "U": user.Username,
-        "E": user.Email,
+    # We cannot modify our own account type.
+    data = {
+        "U": tu_user.Username,
+        "E": tu_user.Email,
         "T": USER_ID,
         "passwd": "testPassword"
     }
     with client as request:
-        resp = request.post(endpoint, data=post_data, cookies=cookies)
-    assert resp.status_code == int(HTTPStatus.OK)
-
-    # No errors should be displayed.
-    errors = get_errors(resp.text)
-    assert not errors
-
-    # Make sure it got changed to USER_ID as we intended.
-    assert user.AccountTypeID == USER_ID
-
-    # Change user to a TU & Dev, which can change themselves to a Developer.
-    with db.begin():
-        user.AccountTypeID = TRUSTED_USER_AND_DEV_ID
-
-    # As a TU & Dev, we can absolutely change all account types.
-    # For example, from TRUSTED_USER_AND_DEV_ID to DEVELOPER_ID:
-    post_data = {
-        "U": user.Username,
-        "E": user.Email,
-        "T": DEVELOPER_ID,
-        "passwd": "testPassword"
-    }
-    with client as request:
-        resp = request.post(endpoint, data=post_data, cookies=cookies)
-    assert resp.status_code == int(HTTPStatus.OK)
-    assert user.AccountTypeID == DEVELOPER_ID
-
-    # But we can't change a user to a Trusted User & Developer when
-    # we're just a Developer.
-    post_data = {
-        "U": user.Username,
-        "E": user.Email,
-        "T": TRUSTED_USER_AND_DEV_ID,
-        "passwd": "testPassword"
-    }
-    with client as request:
-        resp = request.post(endpoint, data=post_data, cookies=cookies)
+        resp = request.post(endpoint, data=data, cookies=cookies)
     assert resp.status_code == int(HTTPStatus.BAD_REQUEST)
-    assert user.AccountTypeID == DEVELOPER_ID
+
+    errors = get_errors(resp.text)
+    expected = ("You do not have permission to change this "
+                "user's account type to User.")
+    assert errors[0].text.strip() == expected
+
+
+def test_post_account_edit_other_user_type_as_tu(
+        client: TestClient, tu_user: User, caplog: pytest.LogCaptureFixture):
+    caplog.set_level(DEBUG)
+
+    with db.begin():
+        user2 = create_user("test2")
+
+    cookies = {"AURSID": tu_user.login(Request(), "testPassword")}
+    endpoint = f"/account/{user2.Username}/edit"
+
+    # As a TU, we can see the Account Type field for other users.
+    with client as request:
+        resp = request.get(endpoint, cookies=cookies, allow_redirects=False)
+    assert resp.status_code == int(HTTPStatus.OK)
+    assert "id_type" in resp.text
+
+    # As a TU, we can modify other user's account types.
+    data = {
+        "U": user2.Username,
+        "E": user2.Email,
+        "T": TRUSTED_USER_ID,
+        "passwd": "testPassword"
+    }
+    with client as request:
+        resp = request.post(endpoint, data=data, cookies=cookies)
+    assert resp.status_code == int(HTTPStatus.OK)
+
+    # Let's make sure the DB got updated properly.
+    assert user2.AccountTypeID == TRUSTED_USER_ID
+
+    # and also that this got logged out at DEBUG level.
+    expected = (f"Trusted User '{tu_user.Username}' has "
+                f"modified '{user2.Username}' account's type to"
+                f" {TRUSTED_USER}.")
+    assert expected in caplog.text
+
+
+def test_post_account_edit_other_user_type_as_tu_invalid_type(
+        client: TestClient, tu_user: User, caplog: pytest.LogCaptureFixture):
+    with db.begin():
+        user2 = create_user("test2")
+
+    cookies = {"AURSID": tu_user.login(Request(), "testPassword")}
+    endpoint = f"/account/{user2.Username}/edit"
+
+    # As a TU, we can modify other user's account types.
+    data = {
+        "U": user2.Username,
+        "E": user2.Email,
+        "T": 0,
+        "passwd": "testPassword"
+    }
+    with client as request:
+        resp = request.post(endpoint, data=data, cookies=cookies)
+    assert resp.status_code == int(HTTPStatus.BAD_REQUEST)
+
+    errors = get_errors(resp.text)
+    expected = "Invalid account type provided."
+    assert errors[0].text.strip() == expected
 
 
 def test_get_account(client: TestClient, user: User):
@@ -1199,11 +1218,7 @@ def test_post_accounts(client: TestClient, user: User, tu_user: User):
     users = [user]
     with db.begin():
         for i in range(10):
-            _user = create(User, Username=f"test_{i}",
-                           Email=f"test_{i}@example.org",
-                           RealName=f"Test #{i}",
-                           Passwd="testPassword",
-                           IRCNick=f"test_#{i}")
+            _user = create_user(f"test_{i}")
             users.append(_user)
 
     sid = user.login(Request(), "testPassword")
@@ -1230,8 +1245,8 @@ def test_post_accounts(client: TestClient, user: User, tu_user: User):
 
         assert atype.text.strip() == str(_user.AccountType)
         assert suspended.text.strip() == "Active"
-        assert real_name.text.strip() == _user.RealName
-        assert irc_nick.text == _user.IRCNick
+        assert real_name.text == (_user.RealName or None)
+        assert irc_nick.text == (_user.IRCNick or None)
         assert pgp_key.text == (_user.PGPKey or None)
 
         edit = edit.xpath("./a")
@@ -1435,15 +1450,9 @@ def test_post_accounts_irc(client: TestClient, user: User, tu_user: User):
 
 def test_post_accounts_sortby(client: TestClient, user: User, tu_user: User):
     # Create a second user so we can compare sorts.
-    account_type = query(AccountType,
-                         AccountType.ID == DEVELOPER_ID).first()
     with db.begin():
-        create(User, Username="test2",
-               Email="test2@example.org",
-               RealName="Test User 2",
-               Passwd="testPassword",
-               IRCNick="test2",
-               AccountType=account_type)
+        user_ = create_user("test2")
+        user_.AccountTypeID = DEVELOPER_ID
 
     sid = user.login(Request(), "testPassword")
     cookies = {"AURSID": sid}
@@ -1464,8 +1473,8 @@ def test_post_accounts_sortby(client: TestClient, user: User, tu_user: User):
     assert len(rows) == 2
 
     def compare_text_values(column, lhs, rhs):
-        return [row[column].text.strip() for row in lhs] \
-            == [row[column].text.strip() for row in rhs]
+        return [row[column].text for row in lhs] \
+            == [row[column].text for row in rhs]
 
     # Test the username rows are ordered the same.
     assert compare_text_values(0, first_rows, rows) is True