From 6a9f0e9ae8e92324f951294f9cf407c175562539 Mon Sep 17 00:00:00 2001
From: Douglas Cerna <douglascerna@yahoo.com>
Date: Fri, 25 Oct 2024 06:59:29 -0600
Subject: [PATCH] Fix API key generation when user edits are disabled

This also extends test coverage of the views of the accounts and
administration packages.
---
 pyproject.toml                                |   2 +
 .../src/components/accounts/forms.py          |   2 +-
 .../src/components/accounts/views.py          |   2 +-
 .../src/components/administration/forms.py    |   2 +
 .../components/accounts/test_views.py         | 275 +++++++++++++++++-
 .../administration/test_administration.py     | 211 +++++++++++++-
 6 files changed, 484 insertions(+), 10 deletions(-)

diff --git a/pyproject.toml b/pyproject.toml
index 8447059dcc..99e1fa0e6b 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -66,6 +66,8 @@ module = [
     "src.MCPClient.lib.clientScripts.transcribe_file",
     "src.MCPClient.lib.clientScripts.validate_file",
     "tests.archivematicaCommon.test_execute_functions",
+    "tests.dashboard.components.accounts.test_views",
+    "tests.dashboard.components.administration.test_administration",
     "tests.dashboard.fpr.test_views",
     "tests.dashboard.test_oidc",
     "tests.integration.test_oidc_auth",
diff --git a/src/dashboard/src/components/accounts/forms.py b/src/dashboard/src/components/accounts/forms.py
index 64904b184a..91ad4072c1 100644
--- a/src/dashboard/src/components/accounts/forms.py
+++ b/src/dashboard/src/components/accounts/forms.py
@@ -149,7 +149,7 @@ def save(self, commit=True):
 
 
 class ApiKeyForm(forms.Form):
-    regenerate_api_key = forms.CharField(
+    regenerate_api_key = forms.BooleanField(
         widget=forms.CheckboxInput,
         label="Regenerate API key (shown below)?",
         required=False,
diff --git a/src/dashboard/src/components/accounts/views.py b/src/dashboard/src/components/accounts/views.py
index cb189478de..86f33cb66e 100644
--- a/src/dashboard/src/components/accounts/views.py
+++ b/src/dashboard/src/components/accounts/views.py
@@ -88,7 +88,7 @@ def profile(request):
         form = ApiKeyForm(request.POST)
         userprofileform = UserProfileForm(request.POST, instance=user_profile)
         if form.is_valid() and userprofileform.is_valid():
-            if form["regenerate_api_key"] != "":
+            if form.cleaned_data["regenerate_api_key"]:
                 generate_api_key(user)
             userprofileform.save()
 
diff --git a/src/dashboard/src/components/administration/forms.py b/src/dashboard/src/components/administration/forms.py
index e28be8c02f..451bdc1e4a 100644
--- a/src/dashboard/src/components/administration/forms.py
+++ b/src/dashboard/src/components/administration/forms.py
@@ -211,6 +211,8 @@ class StripCharField(forms.CharField):
         """
 
         def to_python(self, value):
+            if value is None:
+                value = ""
             return super(forms.CharField, self).to_python(value).strip()
 
     storage_service_url = forms.CharField(
diff --git a/tests/dashboard/components/accounts/test_views.py b/tests/dashboard/components/accounts/test_views.py
index 582209cef8..8b42a678dc 100644
--- a/tests/dashboard/components/accounts/test_views.py
+++ b/tests/dashboard/components/accounts/test_views.py
@@ -1,11 +1,23 @@
+import hmac
+import uuid
+from hashlib import sha1
+from typing import Type
+from unittest import mock
 from urllib.parse import parse_qs
 from urllib.parse import urlparse
 
 import pytest
+import pytest_django
+from components import helpers
 from components.accounts.views import get_oidc_logout_url
+from django.contrib.auth.models import User
+from django.test import Client
+from django.test import RequestFactory
+from django.urls import reverse
+from tastypie.models import ApiKey
 
 
-def test_get_oidc_logout_url_fails_if_token_is_not_set(rf):
+def test_get_oidc_logout_url_fails_if_token_is_not_set(rf: RequestFactory) -> None:
     request = rf.get("/")
     request.session = {}
 
@@ -13,7 +25,9 @@ def test_get_oidc_logout_url_fails_if_token_is_not_set(rf):
         get_oidc_logout_url(request)
 
 
-def test_get_oidc_logout_url_fails_if_logout_endpoint_is_not_set(rf):
+def test_get_oidc_logout_url_fails_if_logout_endpoint_is_not_set(
+    rf: RequestFactory,
+) -> None:
     request = rf.get("/")
     request.session = {"oidc_id_token": "mytoken"}
 
@@ -23,7 +37,9 @@ def test_get_oidc_logout_url_fails_if_logout_endpoint_is_not_set(rf):
         get_oidc_logout_url(request)
 
 
-def test_get_oidc_logout_url_returns_logout_url(rf, settings):
+def test_get_oidc_logout_url_returns_logout_url(
+    rf: RequestFactory, settings: pytest_django.fixtures.SettingsWrapper
+) -> None:
     settings.OIDC_OP_LOGOUT_ENDPOINT = "http://example.com/logout"
     token = "mytoken"
     request = rf.get("/")
@@ -36,3 +52,256 @@ def test_get_oidc_logout_url_returns_logout_url(rf, settings):
     assert set(query_dict) == {"id_token_hint", "post_logout_redirect_uri"}
     assert query_dict["id_token_hint"] == [token]
     assert query_dict["post_logout_redirect_uri"] == ["http://testserver/"]
+
+
+@pytest.fixture
+def dashboard_uuid() -> None:
+    helpers.set_setting("dashboard_uuid", str(uuid.uuid4()))
+
+
+@pytest.fixture
+def non_administrative_user(django_user_model: Type[User]) -> User:
+    return django_user_model.objects.create_user(
+        username="test",
+        password="test",
+        first_name="Foo",
+        last_name="Bar",
+        email="foobar@example.com",
+    )
+
+
+@pytest.mark.django_db
+def test_edit_user_view_denies_access_to_non_admin_users_editing_others(
+    dashboard_uuid: None,
+    non_administrative_user: User,
+    admin_user: User,
+    client: Client,
+) -> None:
+    client.force_login(non_administrative_user)
+
+    response = client.get(
+        reverse("accounts:edit", kwargs={"id": admin_user.id}), follow=True
+    )
+    assert response.status_code == 200
+
+    content = response.content.decode()
+    assert "Forbidden" in content
+    assert "You do not have enough access privileges for this operation" in content
+
+
+@pytest.fixture
+def non_administrative_user_apikey(non_administrative_user: User) -> ApiKey:
+    return ApiKey.objects.create(user=non_administrative_user)
+
+
+@pytest.mark.django_db
+def test_edit_user_view_renders_user_profile_fields(
+    dashboard_uuid: None,
+    non_administrative_user: User,
+    non_administrative_user_apikey: ApiKey,
+    admin_client: Client,
+) -> None:
+    response = admin_client.get(
+        reverse("accounts:edit", kwargs={"id": non_administrative_user.id}), follow=True
+    )
+    assert response.status_code == 200
+
+    content = response.content.decode()
+    assert f"Edit user {non_administrative_user.username}" in content
+    assert f'name="username" value="{non_administrative_user.username}"' in content
+    assert f'name="first_name" value="{non_administrative_user.first_name}"' in content
+    assert f'name="last_name" value="{non_administrative_user.last_name}"' in content
+    assert f'name="email" value="{non_administrative_user.email}"' in content
+    assert f"<code>{non_administrative_user_apikey.key}</code>" in content
+
+
+@pytest.mark.django_db
+def test_edit_user_view_updates_user_profile_fields(
+    dashboard_uuid: None,
+    non_administrative_user: User,
+    admin_client: Client,
+) -> None:
+    new_username = "newusername"
+    new_password = "newpassword"
+    new_first_name = "bar"
+    new_last_name = "foo"
+    new_email = "newemail@example.com"
+    data = {
+        "username": new_username,
+        "password": new_password,
+        "password_confirmation": new_password,
+        "first_name": new_first_name,
+        "last_name": new_last_name,
+        "email": new_email,
+    }
+
+    response = admin_client.post(
+        reverse("accounts:edit", kwargs={"id": non_administrative_user.id}),
+        data,
+        follow=True,
+    )
+    assert response.status_code == 200
+
+    content = response.content.decode()
+    assert "Saved" in content
+    assert (
+        f'<a href="{reverse("accounts:edit", kwargs={"id": non_administrative_user.id})}">{new_username}</a>'
+        in content
+    )
+    assert f"<td>{new_first_name} {new_last_name}</td>" in content
+    assert f"<td>{new_email}</td>" in content
+
+    non_administrative_user.refresh_from_db()
+    assert non_administrative_user.check_password(new_password)
+
+
+@pytest.mark.django_db
+def test_edit_user_view_regenerates_api_key(
+    dashboard_uuid: None,
+    non_administrative_user: User,
+    non_administrative_user_apikey: ApiKey,
+    admin_client: Client,
+) -> None:
+    data = {
+        "username": non_administrative_user.username,
+        "first_name": non_administrative_user.first_name,
+        "last_name": non_administrative_user.last_name,
+        "email": non_administrative_user.email,
+        "regenerate_api_key": True,
+    }
+    expected_uuid = uuid.uuid4()
+    expected_key = hmac.new(expected_uuid.bytes, digestmod=sha1).hexdigest()
+
+    with mock.patch("uuid.uuid4", return_value=expected_uuid):
+        response = admin_client.post(
+            reverse("accounts:edit", kwargs={"id": non_administrative_user.id}),
+            data,
+            follow=True,
+        )
+    assert response.status_code == 200
+
+    assert "Saved" in response.content.decode()
+
+    non_administrative_user_apikey.refresh_from_db()
+    assert non_administrative_user_apikey.key == expected_key
+
+
+@pytest.mark.django_db
+def test_user_profile_view_allows_users_to_edit_their_profile_fields(
+    dashboard_uuid: None,
+    non_administrative_user: User,
+    non_administrative_user_apikey: ApiKey,
+    client: Client,
+    settings: pytest_django.fixtures.SettingsWrapper,
+) -> None:
+    settings.ALLOW_USER_EDITS = True
+    client.force_login(non_administrative_user)
+
+    response = client.get(
+        reverse("accounts:profile"),
+        follow=True,
+    )
+    assert response.status_code == 200
+
+    content = response.content.decode()
+    assert f"Edit your profile ({non_administrative_user.username})" in content
+    assert f'name="username" value="{non_administrative_user.username}"' in content
+    assert f'name="first_name" value="{non_administrative_user.first_name}"' in content
+    assert f'name="last_name" value="{non_administrative_user.last_name}"' in content
+    assert f'name="email" value="{non_administrative_user.email}"' in content
+    assert f"<code>{non_administrative_user_apikey.key}</code>" in content
+
+
+@pytest.mark.django_db
+def test_user_profile_view_denies_editing_profile_fields_if_setting_disables_it(
+    dashboard_uuid: None,
+    non_administrative_user: User,
+    non_administrative_user_apikey: ApiKey,
+    client: Client,
+    settings: pytest_django.fixtures.SettingsWrapper,
+) -> None:
+    settings.ALLOW_USER_EDITS = False
+    client.force_login(non_administrative_user)
+
+    response = client.get(
+        reverse("accounts:profile"),
+        follow=True,
+    )
+    assert response.status_code == 200
+
+    content = response.content.decode()
+    assert f"Your profile ({non_administrative_user.username})" in content
+    assert f"<dd>{non_administrative_user.username}</dd>" in content
+    assert (
+        f"<dd>{non_administrative_user.first_name} {non_administrative_user.last_name}</dd>"
+        in content
+    )
+    assert f"<dd>{non_administrative_user.email}</dd>" in content
+    assert (
+        f'<dd>{"yes" if non_administrative_user.is_superuser else "no"}</dd>' in content
+    )
+    assert f"<code>{non_administrative_user_apikey.key}</code>" in content
+
+
+@pytest.mark.django_db
+def test_user_profile_view_regenerates_api_key_if_setting_disables_editing(
+    dashboard_uuid: None,
+    non_administrative_user: User,
+    non_administrative_user_apikey: ApiKey,
+    client: Client,
+    settings: pytest_django.fixtures.SettingsWrapper,
+) -> None:
+    settings.ALLOW_USER_EDITS = False
+    client.force_login(non_administrative_user)
+    data = {"regenerate_api_key": True}
+    expected_uuid = uuid.uuid4()
+    expected_key = hmac.new(expected_uuid.bytes, digestmod=sha1).hexdigest()
+
+    with mock.patch("uuid.uuid4", return_value=expected_uuid):
+        response = client.post(
+            reverse("accounts:profile"),
+            data,
+            follow=True,
+        )
+    assert response.status_code == 200
+
+    content = response.content.decode()
+    assert f"Your profile ({non_administrative_user.username})" in content
+    assert f"<dd>{non_administrative_user.username}</dd>" in content
+    assert (
+        f"<dd>{non_administrative_user.first_name} {non_administrative_user.last_name}</dd>"
+        in content
+    )
+    assert f"<dd>{non_administrative_user.email}</dd>" in content
+    assert (
+        f'<dd>{"yes" if non_administrative_user.is_superuser else "no"}</dd>' in content
+    )
+    assert f"<code>{expected_key}</code>" in content
+
+
+@pytest.mark.django_db
+def test_user_profile_view_does_not_regenerate_api_key_if_not_requested(
+    dashboard_uuid: None,
+    non_administrative_user: User,
+    non_administrative_user_apikey: ApiKey,
+    client: Client,
+    settings: pytest_django.fixtures.SettingsWrapper,
+) -> None:
+    settings.ALLOW_USER_EDITS = False
+    client.force_login(non_administrative_user)
+
+    response = client.post(reverse("accounts:profile"), {}, follow=True)
+    assert response.status_code == 200
+
+    content = response.content.decode()
+    assert f"Your profile ({non_administrative_user.username})" in content
+    assert f"<dd>{non_administrative_user.username}</dd>" in content
+    assert (
+        f"<dd>{non_administrative_user.first_name} {non_administrative_user.last_name}</dd>"
+        in content
+    )
+    assert f"<dd>{non_administrative_user.email}</dd>" in content
+    assert (
+        f'<dd>{"yes" if non_administrative_user.is_superuser else "no"}</dd>' in content
+    )
+    assert f"<code>{non_administrative_user_apikey.key}</code>" in content
diff --git a/tests/dashboard/components/administration/test_administration.py b/tests/dashboard/components/administration/test_administration.py
index 15abeb5fff..7778b1913c 100644
--- a/tests/dashboard/components/administration/test_administration.py
+++ b/tests/dashboard/components/administration/test_administration.py
@@ -1,19 +1,29 @@
 import uuid
+from unittest import mock
 
 import pytest
+import pytest_django
+import requests
 from components import helpers
+from django.contrib.auth.models import User
+from django.http import HttpResponseNotFound
+from django.test import Client
 from django.urls import reverse
 from main.models import Report
+from tastypie.models import ApiKey
 
 
 @pytest.fixture
 @pytest.mark.django_db
-def dashboard_uuid():
-    helpers.set_setting("dashboard_uuid", str(uuid.uuid4()))
+def dashboard_uuid() -> str:
+    value = str(uuid.uuid4())
+    helpers.set_setting("dashboard_uuid", value)
+
+    return value
 
 
 @pytest.mark.django_db
-def test_admin_set_language(dashboard_uuid, admin_client):
+def test_admin_set_language(dashboard_uuid: str, admin_client: Client) -> None:
     response = admin_client.get(reverse("administration:admin_set_language"))
     assert response.status_code == 200
 
@@ -23,7 +33,7 @@ def test_admin_set_language(dashboard_uuid, admin_client):
 
 
 @pytest.mark.django_db
-def test_failure_report_delete(dashboard_uuid, admin_client):
+def test_failure_report_delete(dashboard_uuid: str, admin_client: Client) -> None:
     report = Report.objects.create(content="my report")
 
     response = admin_client.post(
@@ -38,7 +48,7 @@ def test_failure_report_delete(dashboard_uuid, admin_client):
 
 
 @pytest.mark.django_db
-def test_failure_report(dashboard_uuid, admin_client):
+def test_failure_report(dashboard_uuid: str, admin_client: Client) -> None:
     report = Report.objects.create(content="my report")
 
     response = admin_client.get(reverse("administration:reports_failures_index"))
@@ -47,3 +57,194 @@ def test_failure_report(dashboard_uuid, admin_client):
     content = response.content.decode()
     assert "<h3>Failure report</h3>" in content
     assert reverse("administration:failure_report", args=[report.pk]) in content
+
+
+@pytest.fixture
+def site_url() -> str:
+    value = "https://example.com/"
+    helpers.set_setting("site_url", value)
+
+    return value
+
+
+@pytest.fixture
+def storage_service_url() -> str:
+    value = "https://ss.example.com/"
+    helpers.set_setting("storage_service_url", value)
+
+    return value
+
+
+@pytest.fixture
+def storage_service_user() -> str:
+    value = "test"
+    helpers.set_setting("storage_service_user", value)
+
+    return value
+
+
+@pytest.fixture
+def storage_service_apikey() -> str:
+    value = "api-key"
+    helpers.set_setting("storage_service_apikey", value)
+
+    return value
+
+
+@pytest.fixture
+def checksum_type() -> str:
+    value = "md5"
+    helpers.set_setting("checksum_type", value)
+
+    return value
+
+
+@pytest.mark.django_db
+@mock.patch(
+    "requests.Session.get",
+    side_effect=[mock.Mock(status_code=200, spec=requests.Response)],
+)
+def test_general_view_renders_initial_dashboard_settings(
+    get: mock.Mock,
+    dashboard_uuid: str,
+    site_url: str,
+    storage_service_url: str,
+    storage_service_user: str,
+    storage_service_apikey: str,
+    checksum_type: str,
+    admin_client: Client,
+) -> None:
+    response = admin_client.get(reverse("administration:general"))
+    assert response.status_code == 200
+
+    content = response.content.decode()
+    assert f"<b>Dashboard UUID</b> {dashboard_uuid}" in content
+    assert f'name="general-site_url" value="{site_url}"' in content
+    assert (
+        f'name="storage-storage_service_url" value="{storage_service_url}"' in content
+    )
+    assert (
+        f'name="storage-storage_service_user" value="{storage_service_user}"' in content
+    )
+    assert (
+        f'name="storage-storage_service_apikey" value="{storage_service_apikey}"'
+        in content
+    )
+    assert f'<option value="{checksum_type}" selected>' in content
+
+
+@pytest.mark.django_db
+@mock.patch("requests.Session.get")
+def test_general_view_renders_warning_if_it_cannot_connect_to_pipeline(
+    get: mock.Mock,
+    dashboard_uuid: str,
+    admin_client: Client,
+) -> None:
+    error = "connection refused"
+    get.side_effect = [requests.exceptions.ConnectionError(error)]
+
+    response = admin_client.get(reverse("administration:general"))
+    assert response.status_code == 200
+
+    content = response.content.decode()
+    assert "Storage Service inaccessible" in content
+    assert error in content
+
+
+@pytest.mark.django_db
+@mock.patch(
+    "requests.Session.get",
+    side_effect=[mock.Mock(status_code=200, spec=requests.Response)],
+)
+def test_general_view_updates_dashboard_settings(
+    get: mock.Mock,
+    dashboard_uuid: str,
+    site_url: str,
+    storage_service_url: str,
+    storage_service_user: str,
+    storage_service_apikey: str,
+    checksum_type: str,
+    admin_client: Client,
+) -> None:
+    new_site_url = "https://other.example.com"
+    new_storage_service_url = "https://other.ss.example.com"
+    new_storage_service_user = "foobar"
+    new_storage_service_apikey = "foobar"
+    new_checksum_type = "sha512"
+
+    data = {
+        "general-site_url": new_site_url,
+        "storage-storage_service_url": new_storage_service_url,
+        "storage-storage_service_user": new_storage_service_user,
+        "storage-storage_service_apikey": new_storage_service_apikey,
+        "checksum algorithm-checksum_type": new_checksum_type,
+    }
+
+    response = admin_client.post(reverse("administration:general"), data)
+    assert response.status_code == 200
+
+    assert "Saved" in response.content.decode()
+    assert helpers.get_setting("site_url", new_site_url)
+    assert helpers.get_setting("storage_service_url", new_storage_service_url)
+    assert helpers.get_setting("storage_service_user", new_storage_service_user)
+    assert helpers.get_setting("storage_service_apikey", new_storage_service_apikey)
+    assert helpers.get_setting("checksum_type", new_checksum_type)
+
+
+class NotFound(Exception):
+    response = HttpResponseNotFound()
+
+
+@pytest.fixture
+def admin_user_apikey(admin_user: User) -> ApiKey:
+    return ApiKey.objects.create(user=admin_user)
+
+
+@pytest.mark.django_db
+@mock.patch("storageService.get_pipeline", side_effect=NotFound)
+@mock.patch("requests.Session.post")
+@mock.patch("platform.node")
+def test_general_view_registers_pipeline_in_storage_service(
+    node: mock.Mock,
+    post: mock.Mock,
+    get_pipeline: mock.Mock,
+    dashboard_uuid: str,
+    site_url: str,
+    storage_service_url: str,
+    storage_service_user: str,
+    storage_service_apikey: str,
+    admin_client: Client,
+    admin_user: User,
+    admin_user_apikey: ApiKey,
+    caplog: pytest.LogCaptureFixture,
+    settings: pytest_django.fixtures.SettingsWrapper,
+) -> None:
+    hostname = "myhost"
+    node.return_value = hostname
+    data = {
+        "storage-storage_service_url": storage_service_url,
+        "storage-storage_service_user": storage_service_user,
+        "storage-storage_service_apikey": storage_service_apikey,
+        "storage-storage_service_use_default_config": True,
+    }
+    shared_directory = "/tmp"
+    settings.SHARED_DIRECTORY = shared_directory
+
+    response = admin_client.post(reverse("administration:general"), data)
+    assert response.status_code == 200
+
+    assert "SS inaccessible or pipeline not registered." in [
+        r.message for r in caplog.records
+    ]
+    post.assert_called_once_with(
+        f"{storage_service_url}api/v2/pipeline/",
+        json={
+            "uuid": dashboard_uuid,
+            "description": f"Archivematica on {hostname}",
+            "create_default_locations": True,
+            "shared_path": shared_directory,
+            "remote_name": site_url,
+            "api_username": admin_user.username,
+            "api_key": admin_user_apikey.key,
+        },
+    )