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

Fix API key generation when user edits are disabled #2004

Merged
merged 7 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion src/dashboard/src/components/accounts/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/dashboard/src/components/accounts/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
2 changes: 2 additions & 0 deletions src/dashboard/src/components/administration/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,8 @@
"""

def to_python(self, value):
if value is None:
value = ""

Check warning on line 215 in src/dashboard/src/components/administration/forms.py

View check run for this annotation

Codecov / codecov/patch

src/dashboard/src/components/administration/forms.py#L215

Added line #L215 was not covered by tests
return super(forms.CharField, self).to_python(value).strip()

storage_service_url = forms.CharField(
Expand Down
275 changes: 272 additions & 3 deletions tests/dashboard/components/accounts/test_views.py
Original file line number Diff line number Diff line change
@@ -1,19 +1,33 @@
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 = {}

with pytest.raises(ValueError, match="ID token not found in session."):
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"}

Expand All @@ -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("/")
Expand All @@ -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="[email protected]",
)


@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 = "[email protected]"
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
Loading