diff --git a/.talismanrc b/.talismanrc index 1ddd374bcf..376f66f9ca 100644 --- a/.talismanrc +++ b/.talismanrc @@ -55,8 +55,6 @@ fileignoreconfig: checksum: 3d899b94bc836e0f97b282d569ca27b513be6df54cf718e96a944a99b634444d - filename: .env.sample checksum: aa3f02c8f5d30f989986f9eafaca8bc57d99d09361b9fa9c2158638bcc263f15 -- filename: core/settings.py - checksum: c9a7604687c73a1344d6ec4d5979a521981fd11f0ae35cb6351787d824c1c55f - filename: erp/management/commands/import_apidae.py checksum: 0bf117b99f9b76db4824cfe5aec647f455eaeb4e57c96425b90e6c580302a972 - filename: .github/workflows/lint.yml @@ -93,3 +91,19 @@ fileignoreconfig: checksum: 253aa05b83f0695dec90299bb58538e736bbce3e0200385c63b6dbc3c5294a75 - filename: erp/views.py checksum: 40161683c6d36b3a1d5bb3c0b05594fa48ada9dd88ecc379294fa10f64743eb2 +- filename: api/authentication.py + checksum: 3d3fdfaf5cb8f79e38238d2f75b6fef32c2658e6e71fd2752630bb0797c08279 +- filename: compte/admin.py + checksum: ce749d67704e2fbf4c6257c13a1c11fcdcb0e3d1c49568ae5b1fe4ac149a587a +- filename: compte/migrations/0009_userapikey.py + checksum: 41df5a33bf852a149d6d54fabde1c55955f8290d50cdf7203b93d879a6278542 +- filename: compte/migrations/0010_auto_20260715_1512.py + checksum: d1a902acd238fb94e179d809d5ef3ac8c5c027a6a0581e61f8844a91f6aa8f93 +- filename: tests/api/tests.py + checksum: 37ddb4f8af1dadeaaaec408016119309fc8d59795f3a3a8de726ec19994f2124 +- filename: core/settings.py + checksum: aef0ac9cf58af8afbac59a2bdbaef5425b85e18bf235ea90c1701c9fe3eda3f0 +- filename: api/throttling.py + checksum: d97a8420cfd750484e125234abddadc773e41830fcf76c5ecfbf943e8017cd52 +- filename: tests/api/test_permissions.py + checksum: 861dc1b82574ecf1de0629bddb553ac2bed11536506622e8a42ae479ac622238 diff --git a/api/authentication.py b/api/authentication.py new file mode 100644 index 0000000000..8611058056 --- /dev/null +++ b/api/authentication.py @@ -0,0 +1,24 @@ +from rest_framework.authentication import BaseAuthentication + +from compte.models import UserAPIKey + + +class UserAPIKeyAuthentication(BaseAuthentication): + """New API Key system, linked to a user. Return None if not found to fallback on legacy system.""" + + def authenticate(self, request): + auth = request.META.get("HTTP_AUTHORIZATION") + if not auth: + return None + + parts = auth.split() + if len(parts) != 2: + return None + + key = parts[1] + try: + api_key = UserAPIKey.objects.get_from_key(key) + except UserAPIKey.DoesNotExist: + return None + + return (api_key.user, api_key) diff --git a/api/permissions.py b/api/permissions.py index d491c9da78..343ae60fb9 100644 --- a/api/permissions.py +++ b/api/permissions.py @@ -1,10 +1,10 @@ import sentry_sdk -from django.conf import settings -from django.core.cache import cache from django.utils.translation import gettext as translate from rest_framework import permissions from rest_framework_api_key.models import APIKey +from compte.models import UserAPIKey + SAFE_METHODS = ("GET", "HEAD", "OPTIONS") @@ -20,11 +20,9 @@ def has_permission(self, request, view): return False key = auth_split[1] - if key == cache.get(settings.INTERNAL_API_KEY_NAME): - if view.action in ("default", "list", "translate"): - # Internal api key is allowed to perform only view/list actions, not write operations (create, update, ...) - return True - return False + + if isinstance(request.auth, UserAPIKey): + return True try: with sentry_sdk.start_span(description="Check signature of API KEY"): diff --git a/api/throttling.py b/api/throttling.py new file mode 100644 index 0000000000..30d3b1f51b --- /dev/null +++ b/api/throttling.py @@ -0,0 +1,17 @@ +from django.conf import settings +from rest_framework.throttling import SimpleRateThrottle + + +class FrontendOriginThrottle(SimpleRateThrottle): + """Generous quota for requests that appear to come from our own + frontend. Not a security mechanism (Origin/Referer are spoofable): + it only avoids penalizing normal site usage while pushing + unregistered scraping toward requesting an API key.""" + + scope = "frontend" + + def get_cache_key(self, request, view): + origin = request.META.get("HTTP_ORIGIN") or request.META.get("HTTP_REFERER", "") + if not origin.startswith(settings.SITE_ROOT_URL): + return None + return self.cache_format % {"scope": self.scope, "ident": self.get_ident(request)} diff --git a/api/views.py b/api/views.py index e4263d2b5c..48982e44b9 100644 --- a/api/views.py +++ b/api/views.py @@ -4,7 +4,7 @@ from django.conf import settings from django.db.models import Q from django.shortcuts import get_object_or_404 -from rest_framework import mixins, viewsets +from rest_framework import mixins, permissions, viewsets from rest_framework.decorators import action from rest_framework.filters import BaseFilterBackend from rest_framework.pagination import PageNumberPagination @@ -208,6 +208,7 @@ class AccessibiliteViewSet(mixins.ListModelMixin, mixins.RetrieveModelMixin, vie pagination_class = AccessibilitePagination filter_backends = [AccessibiliteFilterBackend] schema = AccessibiliteSchema() + permission_classes = [permissions.AllowAny] @action(detail=False, methods=["get"]) def help(self, request, pk=None): @@ -303,6 +304,7 @@ class ActiviteViewSet(mixins.ListModelMixin, mixins.RetrieveModelMixin, viewsets pagination_class = ActivitePagination filter_backends = [ActiviteFilterBackend] schema = ActiviteSchema() + permission_classes = [permissions.AllowAny] class ErpPagination(PageNumberPagination): @@ -571,6 +573,18 @@ def get_pagination_class(self): return GeoJsonPagination return ErpPagination + def perform_create(self, serializer): + user = self.request.user if self.request.user.is_authenticated else None + serializer.save(user=user) if user else serializer.save() + + def perform_update(self, serializer): + instance = serializer.instance + user = self.request.user if self.request.user.is_authenticated else None + if user and instance.user_id is None: + serializer.save(user=user.id) + else: + serializer.save() + pagination_class = property(fget=get_pagination_class) @action(methods=["get"], detail=True, url_path="widget", url_name="widget") diff --git a/compte/admin.py b/compte/admin.py index 35ad5e3b6c..a7c4378a48 100644 --- a/compte/admin.py +++ b/compte/admin.py @@ -2,8 +2,10 @@ from django.contrib.auth.admin import UserAdmin from django.contrib.auth.models import User from import_export.admin import ExportMixin +from rest_framework_api_key.admin import APIKeyModelAdmin +from rest_framework_api_key.models import APIKey -from compte.models import UserPreferences, UserStats +from compte.models import UserAPIKey, UserPreferences, UserStats from compte.resources import UserAdminResource @@ -84,6 +86,21 @@ class UserPreferencesAdmin(admin.ModelAdmin): search_fields = ("user__email",) +admin.site.unregister(APIKey) + + +@admin.register(APIKey) +class LegacyAPIKeyAdmin(APIKeyModelAdmin): + def has_add_permission(self, request): + return False + + +@admin.register(UserAPIKey) +class UserAPIKeyAdmin(APIKeyModelAdmin): + list_display = [*APIKeyModelAdmin.list_display, "user"] + search_fields = [*APIKeyModelAdmin.search_fields, "user__username", "user__email"] + + # Replace the default UserAdmin with our custom one admin.site.unregister(User) admin.site.register(User, CustomUserAdmin) diff --git a/compte/migrations/0009_userapikey.py b/compte/migrations/0009_userapikey.py new file mode 100644 index 0000000000..6f9d4bbeb9 --- /dev/null +++ b/compte/migrations/0009_userapikey.py @@ -0,0 +1,66 @@ +# Generated by Django 6.0.7 on 2026-07-15 12:31 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("compte", "0008_nb_erp_administrator"), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name="UserAPIKey", + fields=[ + ( + "id", + models.CharField(editable=False, max_length=150, primary_key=True, serialize=False, unique=True), + ), + ("prefix", models.CharField(editable=False, max_length=8, unique=True)), + ("hashed_key", models.CharField(editable=False, max_length=150)), + ("created", models.DateTimeField(auto_now_add=True, db_index=True)), + ( + "name", + models.CharField( + default=None, + help_text="A free-form name for the API key. Need not be unique. 50 characters max.", + max_length=50, + ), + ), + ( + "revoked", + models.BooleanField( + blank=True, + default=False, + help_text="If the API key is revoked, clients cannot use it anymore. (This cannot be undone.)", + ), + ), + ( + "expiry_date", + models.DateTimeField( + blank=True, + help_text="Once API key expires, clients cannot use it anymore.", + null=True, + verbose_name="Expires", + ), + ), + ( + "user", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="api_keys", + to=settings.AUTH_USER_MODEL, + ), + ), + ], + options={ + "verbose_name": "Clef d'API par utilisateur", + "verbose_name_plural": "Clefs d'API par utilisateur", + "ordering": ("-created",), + "abstract": False, + }, + ), + ] diff --git a/compte/migrations/0010_auto_20260715_1512.py b/compte/migrations/0010_auto_20260715_1512.py new file mode 100644 index 0000000000..6c7a04d957 --- /dev/null +++ b/compte/migrations/0010_auto_20260715_1512.py @@ -0,0 +1,93 @@ +import logging +import re + +from django.db import migrations +from django.utils import timezone + +logger = logging.getLogger("api_keys_migration") + +EMAIL_RE = re.compile(r"[\w\.\-+]+@[\w\-]+\.[\w\.\-]+") + + +def migrate_legacy_keys(apps, schema_editor): + APIKey = apps.get_model("rest_framework_api_key", "APIKey") + UserAPIKey = apps.get_model("compte", "UserAPIKey") + User = apps.get_model(*settings_auth_user_model(apps)) + + now = timezone.now() + + stats = {"migrated": 0, "no_email_found": 0, "no_user_match": 0, "ambiguous": 0, "skipped_state": 0} + + for key in APIKey.objects.all(): + if key.revoked: + stats["skipped_state"] += 1 + continue + if key.expiry_date is not None and key.expiry_date <= now: + stats["skipped_state"] += 1 + continue + + match = EMAIL_RE.search(key.name or "") + if not match: + stats["no_email_found"] += 1 + logger.warning("No email found in key name: prefix=%s name=%r", key.prefix, key.name) + continue + + email = match.group(0) + users = list(User.objects.filter(email__iexact=email)) + + if len(users) == 0: + stats["no_user_match"] += 1 + logger.warning("No user found for email=%s (key prefix=%s)", email, key.prefix) + continue + + if len(users) > 1: + stats["ambiguous"] += 1 + logger.warning("Multiple users found for email=%s (key prefix=%s), skipping", email, key.prefix) + continue + + user = users[0] + + UserAPIKey.objects.create( + id=key.id, + prefix=key.prefix, + hashed_key=key.hashed_key, + created=key.created, + name=key.name, + revoked=key.revoked, + expiry_date=key.expiry_date, + user=user, + ) + + key.revoked = True + key.save(update_fields=["revoked"]) + + stats["migrated"] += 1 + + logger.info("Legacy API key migration done: %s", stats) + + +def settings_auth_user_model(apps): + from django.conf import settings + + app_label, model_name = settings.AUTH_USER_MODEL.split(".") + return app_label, model_name + + +def reverse_migration(apps, schema_editor): + APIKey = apps.get_model("rest_framework_api_key", "APIKey") + UserAPIKey = apps.get_model("compte", "UserAPIKey") + + migrated_ids = list(UserAPIKey.objects.values_list("id", flat=True)) + APIKey.objects.filter(id__in=migrated_ids).update(revoked=False) + UserAPIKey.objects.filter(id__in=migrated_ids).delete() + + +class Migration(migrations.Migration): + dependencies = [ + ("compte", "0009_userapikey"), + ("rest_framework_api_key", "0001_initial"), + ] + + operations = [ + migrations.RunPython(migrate_legacy_keys, reverse_migration), + ] diff --git a/compte/models.py b/compte/models.py index 46a1d950a3..5db2be7e6c 100644 --- a/compte/models.py +++ b/compte/models.py @@ -1,6 +1,7 @@ from django.conf import settings from django.db import models from django.utils.translation import gettext_lazy as translate_lazy +from rest_framework_api_key.models import AbstractAPIKey class EmailToken(models.Model): @@ -65,3 +66,15 @@ def __str__(self) -> str: f"for user #{self.user_id}: {self.nb_erp_created}/{self.nb_erp_edited}/{self.nb_erp_attributed}" f"/{self.nb_profanities}" ) + + +class UserAPIKey(AbstractAPIKey): + user = models.ForeignKey( + settings.AUTH_USER_MODEL, + on_delete=models.CASCADE, + related_name="api_keys", + ) + + class Meta(AbstractAPIKey.Meta): + verbose_name = translate_lazy("Clef d'API par utilisateur") + verbose_name_plural = translate_lazy("Clefs d'API par utilisateur") diff --git a/core/settings.py b/core/settings.py index c2c8c4081b..b5a70014ac 100644 --- a/core/settings.py +++ b/core/settings.py @@ -173,19 +173,22 @@ REST_FRAMEWORK = { + "DEFAULT_AUTHENTICATION_CLASSES": [ + "api.authentication.UserAPIKeyAuthentication", + "rest_framework.authentication.SessionAuthentication", + ], "DEFAULT_PAGINATION_CLASS": "rest_framework.pagination.PageNumberPagination", "PAGE_SIZE": 50, "DEFAULT_THROTTLE_CLASSES": [ - "rest_framework.throttling.AnonRateThrottle", + "api.throttling.FrontendOriginThrottle", "rest_framework.throttling.UserRateThrottle", + "rest_framework.throttling.AnonRateThrottle", ], "DEFAULT_THROTTLE_RATES": { - "anon": "3/second", - "user": "3/second", + "frontend": "5000/hour", + "user": "10000/hour", + "anon": "20/hour", }, - "DEFAULT_PERMISSION_CLASSES": [ - "api.permissions.IsAllowedForAction", - ], "DEFAULT_RENDERER_CLASSES": [ "rest_framework.renderers.JSONRenderer", "api.renderers.GeoJSONRenderer", @@ -194,7 +197,6 @@ ], } -INTERNAL_API_KEY_NAME = "acceslibre - internal uses only" ROOT_URLCONF = "core.urls" diff --git a/erp/imports/serializers.py b/erp/imports/serializers.py index 5cc43fadea..c354c3aba3 100644 --- a/erp/imports/serializers.py +++ b/erp/imports/serializers.py @@ -325,6 +325,10 @@ def update(self, instance, validated_data): enrich_only = self.context.get("enrich_only") or False raw_data = self.initial_data + if "user" in validated_data: + instance.user_id = validated_data["user"] + instance.save(update_fields=["user_id"]) + # if we are updating an ERP, only accessibility, asp_id and import_email are editable if validated_data.get("import_email"): instance.import_email = validated_data["import_email"] diff --git a/erp/views.py b/erp/views.py index ce2a7a2e7f..8fefce3daf 100644 --- a/erp/views.py +++ b/erp/views.py @@ -1,7 +1,5 @@ import datetime import json -import secrets -import string import urllib from decimal import Decimal from io import BytesIO @@ -14,7 +12,6 @@ from django.contrib.admin.models import CHANGE, LogEntry from django.contrib.auth.decorators import login_required from django.contrib.contenttypes.models import ContentType -from django.core.cache import cache from django.core.paginator import Paginator from django.db.models import Q from django.http import HttpResponse, HttpResponseForbidden, JsonResponse @@ -232,18 +229,6 @@ def _search_commune_code_postal(qs, code_insee): ) -def _get_or_create_api_key(): - api_key = cache.get(settings.INTERNAL_API_KEY_NAME) - if api_key: - return api_key - - alphabet = string.ascii_letters + string.digits + string.punctuation - - api_key = "".join(secrets.choice(alphabet) for i in range(32)) - cache.set(settings.INTERNAL_API_KEY_NAME, api_key, timeout=1 * HOURS) - return api_key - - def search(request): filters = cleaned_search_params_as_dict(request.GET) queryset = build_queryset(filters, request.GET) @@ -282,7 +267,6 @@ def search(request): "pager": pager, "pager_base_url": pager_base_url, "paginator": paginator, - "map_api_key": _get_or_create_api_key(), "dynamic_map": True, "equipments_shortcuts": get_equipments_shortcuts(), "equipments": get_equipments(), @@ -389,7 +373,6 @@ def search_in_municipality(request, commune_slug): context = { **filters, - "map_api_key": _get_or_create_api_key(), "pager": pager, "pager_base_url": url.encode_qs(**filters), "paginator": paginator, @@ -541,7 +524,6 @@ def erp_details(request, commune, erp_slug, activite_slug=None): "erp_can_have_image": can_have_image, "erp_can_be_modified": erp.can_be_modified_by(request.user), "need_translation": need_translation, - "api_key": _get_or_create_api_key(), }, ) @@ -747,7 +729,6 @@ def contrib_global_search(request): "next_step_title": schema.SECTION_TRANSPORT, "results": results[:pagination_size], "error": error, - "api_key": _get_or_create_api_key(), "query": { "nom": request.GET.get("what"), "commune": city, diff --git a/locale/en/LC_MESSAGES/django.mo b/locale/en/LC_MESSAGES/django.mo index 4080c8f6f3..94a42fa905 100644 Binary files a/locale/en/LC_MESSAGES/django.mo and b/locale/en/LC_MESSAGES/django.mo differ diff --git a/locale/en/LC_MESSAGES/django.po b/locale/en/LC_MESSAGES/django.po index a452640334..14768fcfea 100644 --- a/locale/en/LC_MESSAGES/django.po +++ b/locale/en/LC_MESSAGES/django.po @@ -6,9 +6,9 @@ msgid "" msgstr "" "Project-Id-Version: PACKAGE VERSION\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2026-07-09 09:53+0200\n" -"PO-Revision-Date: 2026-07-09 09:53+0200\n" -"Last-Translator: \n" +"POT-Creation-Date: 2026-07-16 15:33+0200\n" +"PO-Revision-Date: 2026-07-16 15:11+0200\n" +"Last-Translator: \n" "Language-Team: LANGUAGE \n" "Language: \n" "MIME-Version: 1.0\n" @@ -123,6 +123,12 @@ msgstr "UserStats" msgid "UsersStats" msgstr "UsersStats" +msgid "Clef d'API par utilisateur" +msgstr "API key per user" + +msgid "Clefs d'API par utilisateur" +msgstr "API keys per user" + msgid "Je ne suis pas un robot" msgstr "I am not a robot" @@ -3687,12 +3693,8 @@ msgstr "" "%(completion_rate)s%%" #, python-format -msgid "" -"Pour être conforme à l’obligation réglementaire de registre public " -"d’accessibilité, votre fiche doit être complétée à 100%%." -msgstr "" -"To comply with the regulatory obligation for a public accessibility " -"register, your form must be 100%% complete." +msgid "Pour être conforme à l’obligation réglementaire de registre public d’accessibilité, votre fiche doit être complétée à 100%%." +msgstr "To comply with the regulatory obligation for a public accessibility register, your form must be 100%% complete." msgid "Merci de bien vouloir compléter votre fiche." msgstr "Please complete your form." @@ -5196,13 +5198,8 @@ msgid "Se déclarer en tant que gestionnaire de l’établissement ou agissant p msgstr "Declare yourself as manager of the establishment or acting for the manager." #, python-format -msgid "" -"Remplir toutes les informations demandées, la fiche doit être complète à 100" -" %%. La question sur la présence d’une dérogation doit également être " -"remplie (étape 2)." -msgstr "" -"Fill in all the requested information, the form must be 100%% complete. The " -"question on the presence of an exemption must also be completed (step 2)." +msgid "Remplir toutes les informations demandées, la fiche doit être complète à 100 %%. La question sur la présence d’une dérogation doit également être remplie (étape 2)." +msgstr "Fill in all the requested information, the form must be 100%% complete. The question on the presence of an exemption must also be completed (step 2)." msgid "La fiche doit avoir été mise à jour dans l’année." msgstr "The form must have been updated within the year." diff --git a/locale/fr/LC_MESSAGES/django.po b/locale/fr/LC_MESSAGES/django.po index 803a86710d..777d7d470d 100644 --- a/locale/fr/LC_MESSAGES/django.po +++ b/locale/fr/LC_MESSAGES/django.po @@ -7,7 +7,7 @@ msgid "" msgstr "" "Project-Id-Version: PACKAGE VERSION\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2026-07-09 09:50+0200\n" +"POT-Creation-Date: 2026-07-16 15:11+0200\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: FULL NAME \n" "Language-Team: LANGUAGE \n" @@ -123,6 +123,12 @@ msgstr "" msgid "UsersStats" msgstr "" +msgid "Clef d'API par utilisateur" +msgstr "" + +msgid "Clefs d'API par utilisateur" +msgstr "" + msgid "Je ne suis pas un robot" msgstr "" diff --git a/static/js/geo.js b/static/js/geo.js index cf29ba00e2..fd5e356b60 100644 --- a/static/js/geo.js +++ b/static/js/geo.js @@ -392,7 +392,6 @@ function _getDataPromiseFromAPI(map, page) { timeout: 10000, headers: { Accept: 'application/geo+json', - Authorization: 'Api-Key ' + _getApiKey(), }, }) } @@ -405,10 +404,6 @@ function _getRefreshApiUrl() { return _getRoot().dataset.refreshApiUrl } -function _getApiKey() { - return _getRoot().dataset.apiKey -} - function _getSortType() { return _getRoot().dataset.sortType || '' } diff --git a/static/js/ui/TranslateField.js b/static/js/ui/TranslateField.js index 43529627aa..c3c55d2bbe 100644 --- a/static/js/ui/TranslateField.js +++ b/static/js/ui/TranslateField.js @@ -5,7 +5,6 @@ class TranslateField { this.el = el this.pk = el.dataset.accessPk this.field = el.dataset.field - this.apiKey = el.dataset.apiKey this.btn = this._createBtn() this.result = this._createResult() @@ -41,7 +40,6 @@ class TranslateField { method: 'POST', headers: { 'Content-Type': 'application/json', - 'Authorization': `Api-Key ${this.apiKey}`, 'X-CSRFToken': csrfToken, }, body: JSON.stringify({ diff --git a/templates/common/map.html b/templates/common/map.html index bf8b64f710..71a7e7d97b 100644 --- a/templates/common/map.html +++ b/templates/common/map.html @@ -7,7 +7,6 @@ data-lon="{{ lon|default:'' }}" data-refresh-api-url="{% url "erp-list" %}" data-should-refresh="{{ dynamic_map|default:'False' }}" - data-api-key="{{ map_api_key|default:'' }}" data-erp-identifier="{{ erp.uuid }}" data-should-refresh-on-map-load="{{ should_refresh_map_on_load|default:'True' }}" {% if where and search_type %}data-sort-type="{{ search_type }}" data-where="{{ where_keyword }}"{% endif %} diff --git a/templates/contrib/0a-search_results.html b/templates/contrib/0a-search_results.html index 4ccacb3a5e..6ba4b105c3 100644 --- a/templates/contrib/0a-search_results.html +++ b/templates/contrib/0a-search_results.html @@ -21,7 +21,6 @@ {% endblock navbar %} {% block contrib_content %}
{% include "contrib/includes/contrib-search-section.html" %}
{% endblock contrib_content %} diff --git a/templates/contrib/includes/contrib-external-search-results.html b/templates/contrib/includes/contrib-external-search-results.html index 74e44e98a4..e4de378880 100644 --- a/templates/contrib/includes/contrib-external-search-results.html +++ b/templates/contrib/includes/contrib-external-search-results.html @@ -1,7 +1,6 @@ {% load a4a %} {% load i18n %}

{% translate "Autres établissements connus à ajouter et compléter" %}

diff --git a/templates/contrib/includes/contrib-internal-search-results.html b/templates/contrib/includes/contrib-internal-search-results.html index 443a2579c6..737c68a60f 100644 --- a/templates/contrib/includes/contrib-internal-search-results.html +++ b/templates/contrib/includes/contrib-internal-search-results.html @@ -1,7 +1,6 @@ {% load a4a %} {% load i18n %}

diff --git a/templates/erp/includes/access_comment.html b/templates/erp/includes/access_comment.html index 731028616b..286caa1abd 100644 --- a/templates/erp/includes/access_comment.html +++ b/templates/erp/includes/access_comment.html @@ -6,7 +6,6 @@

{% translate "Commentaire" %
  • {{ access.commentaire|linebreaksbr }}
    • diff --git a/templates/erp/includes/access_entrance.html b/templates/erp/includes/access_entrance.html index 0fa0fd3611..9dbc1b2ba2 100644 --- a/templates/erp/includes/access_entrance.html +++ b/templates/erp/includes/access_entrance.html @@ -108,7 +108,6 @@

    {% translate "Entrée" %}
    diff --git a/templates/erp/includes/access_parking.html b/templates/erp/includes/access_parking.html index 27fba688a9..d4e45eb995 100644 --- a/templates/erp/includes/access_parking.html +++ b/templates/erp/includes/access_parking.html @@ -24,7 +24,6 @@

    {% translate "Transport et s {% if access.transport_information %} :
    diff --git a/tests/api/test_permissions.py b/tests/api/test_permissions.py index 4d83ab96a3..9adcf50343 100644 --- a/tests/api/test_permissions.py +++ b/tests/api/test_permissions.py @@ -1,12 +1,14 @@ import datetime + from unittest.mock import MagicMock import pytest from rest_framework.test import APIRequestFactory from rest_framework_api_key.models import APIKey +from compte.models import UserAPIKey from api.permissions import IsAllowedForAction -from erp.views import _get_or_create_api_key +from tests.factories import UserFactory @pytest.mark.django_db @@ -14,43 +16,41 @@ class TestPermissions: perm = IsAllowedForAction() factory = APIRequestFactory() - def test_bad_key_value(self): - request = self.factory.get("/", headers={"Authorization": "Api-Key FOO"}) + def _request_with_auth(self, header=None, auth=None): + """Builds a request as DRF would present it to a permission: + raw WSGIRequest + `.auth` populated by authentication (or None + if no authenticator matched, mirroring DRF's default).""" + kwargs = {"headers": {"Authorization": header}} if header else {} + request = self.factory.get("/", **kwargs) + request.auth = auth + return request + + def test_no_header(self): + request = self._request_with_auth() assert self.perm.has_permission(request, MagicMock(action="list")) is False def test_bad_key_format(self): - request = self.factory.get("/", headers={"Authorization": "FOO"}) + request = self._request_with_auth(header="FOO") assert self.perm.has_permission(request, MagicMock(action="list")) is False - def test_internal_key_for_get(self, settings): - cache_settings = settings.CACHES.copy() - cache_settings["default"]["BACKEND"] = "django.core.cache.backends.locmem.LocMemCache" - settings.CACHES = cache_settings - - internal_key = _get_or_create_api_key() - assert internal_key, "None or empty internal key generated." - request = self.factory.get("/", headers={"Authorization": f"Api-Key {internal_key}"}) - - assert self.perm.has_permission(request, MagicMock(action="list")) is True - - def test_internal_key_for_post(self, settings): - cache_settings = settings.CACHES.copy() - cache_settings["default"]["BACKEND"] = "django.core.cache.backends.locmem.LocMemCache" - settings.CACHES = cache_settings - - internal_key = _get_or_create_api_key() - assert internal_key, "None or empty internal key generated." - request = self.factory.get("/", headers={"Authorization": f"Api-Key {internal_key}"}) + def test_bad_key_value(self): + request = self._request_with_auth(header="Api-Key FOO") - assert self.perm.has_permission(request, MagicMock(action="delete")) is False + assert self.perm.has_permission(request, MagicMock(action="list")) is False - def test_api_key(self, settings): + def test_legacy_api_key_grants_access(self): _, api_key = APIKey.objects.create_key( - name=settings.INTERNAL_API_KEY_NAME, expiry_date=datetime.datetime.now() + datetime.timedelta(hours=1.2) + name="new-key", expiry_date=datetime.datetime.now() + datetime.timedelta(hours=1.2) ) + request = self._request_with_auth(header=f"Api-Key {api_key}") + + assert self.perm.has_permission(request, MagicMock(action="list")) is True - request = self.factory.get("/", headers={"Authorization": f"Api-Key {api_key}"}) + def test_user_api_key_grants_access(self): + user = UserFactory() + api_key_instance, key = UserAPIKey.objects.create_key(name="user-key", user=user) + request = self._request_with_auth(header=f"Api-Key {key}", auth=api_key_instance) assert self.perm.has_permission(request, MagicMock(action="list")) is True diff --git a/tests/api/tests.py b/tests/api/tests.py index 634b863f4b..886008ab72 100644 --- a/tests/api/tests.py +++ b/tests/api/tests.py @@ -4,20 +4,198 @@ from unittest.mock import ANY, MagicMock, PropertyMock, patch import pytest +from django.contrib.auth import get_user_model from django.contrib.gis.geos import Point from django.urls import reverse from rest_framework.test import APIClient +from rest_framework_api_key.models import APIKey +from api.authentication import UserAPIKeyAuthentication +from compte.models import UserAPIKey from erp import schema from erp.models import Accessibilite, Erp, ExternalSource from tests.factories import AccessibiliteFactory, ActiviteFactory, CommuneFactory, ErpFactory +User = get_user_model() + @pytest.fixture def api_client(): return APIClient() +@pytest.mark.django_db +class TestUserAPIKeyAuthenticationOnRpaErp: + def _rpa_erp(self, mocker, user=None): + erp = ErpFactory(with_accessibility=True, user=user) + mocker.patch("erp.models.Erp.rpa", new_callable=PropertyMock(return_value=True)) + return erp + + def test_create_user_api_key_is_assigned_on_erp(self, api_client): + owner = User.objects.create_user(username="creator") + _, key = UserAPIKey.objects.create_key(name="creator-key", user=owner) + + ActiviteFactory(nom="Mairie") + CommuneFactory(nom="Montreuil", code_postaux=["93100"], code_insee="93048", departement="93") + + payload = { + "activite": "Mairie", + "nom": "Mairie de Montreuil", + "numero": "101", + "voie": "rue Francis de Pressencé", + "commune": "Montreuil", + "code_insee": "93048", + "code_postal": "93100", + "accessibilite": {"entree_porte_presence": True}, + } + + response = api_client.post( + reverse("erp-list"), + data=payload, + format="json", + headers={"Authorization": f"Api-Key {key}"}, + ) + assert response.status_code == 201, response.json() + + erp = Erp.objects.get(nom="Mairie de Montreuil") + assert erp.user == owner + + def test_user_api_key_is_assigned_on_erp(self, api_client, mocker): + owner = User.objects.create_user(username="owner") + erp = ErpFactory(with_accessibility=True, user=None) + _, key = UserAPIKey.objects.create_key(name="owner-key", user=owner) + + response = api_client.patch( + reverse("erp-detail", kwargs={"slug": erp.slug}), + data={"accessibilite": {"commentaire": "updated via api key"}}, + format="json", + headers={"Authorization": f"Api-Key {key}"}, + ) + assert response.status_code == 200 + + erp.refresh_from_db() + assert erp.user == owner + + def test_user_api_key_owner_can_modify_rpa_erp(self, api_client, mocker): + owner = User.objects.create_user(username="rpa_owner") + erp = self._rpa_erp(mocker, user=owner) + _, key = UserAPIKey.objects.create_key(name="owner-key", user=owner) + + response = api_client.patch( + reverse("erp-detail", kwargs={"slug": erp.slug}), + data={"accessibilite": {"commentaire": "updated via api key"}}, + format="json", + headers={"Authorization": f"Api-Key {key}"}, + ) + assert response.status_code == 200 + + def test_user_api_key_stranger_cannot_modify_rpa_erp(self, api_client, mocker): + owner = User.objects.create_user(username="rpa_owner2") + stranger = User.objects.create_user(username="stranger") + erp = self._rpa_erp(mocker, user=owner) + _, key = UserAPIKey.objects.create_key(name="stranger-key", user=stranger) + + response = api_client.patch( + reverse("erp-detail", kwargs={"slug": erp.slug}), + data={"accessibilite": {"commentaire": "nope"}}, + format="json", + headers={"Authorization": f"Api-Key {key}"}, + ) + assert response.status_code == 403 + + def test_no_key_cannot_modify_rpa_erp(self, api_client, mocker): + owner = User.objects.create_user(username="rpa_owner3") + erp = self._rpa_erp(mocker, user=owner) + + response = api_client.patch( + reverse("erp-detail", kwargs={"slug": erp.slug}), + data={"accessibilite": {"commentaire": "nope"}}, + format="json", + ) + assert response.status_code == 403 + + def test_legacy_api_key_does_not_grant_ownership_on_rpa_erp(self, api_client, mocker): + owner = User.objects.create_user(username="rpa_owner4") + erp = self._rpa_erp(mocker, user=owner) + _, key = APIKey.objects.create_key(name="legacy-key") + + response = api_client.patch( + reverse("erp-detail", kwargs={"slug": erp.slug}), + data={"accessibilite": {"commentaire": "nope"}}, + format="json", + headers={"Authorization": f"Api-Key {key}"}, + ) + assert response.status_code == 403 + + def test_legacy_api_key_still_works_for_non_rpa_erp(self, api_client): + erp = ErpFactory(with_accessibility=True) + _, key = APIKey.objects.create_key(name="legacy-key-nominal") + + response = api_client.patch( + reverse("erp-detail", kwargs={"slug": erp.slug}), + data={"accessibilite": {"commentaire": "still works"}}, + format="json", + headers={"Authorization": f"Api-Key {key}"}, + ) + assert response.status_code == 200 + + def test_anonymous_still_open_on_non_rpa_erp(self, api_client): + erp = ErpFactory(with_accessibility=True) + + response = api_client.patch( + reverse("erp-detail", kwargs={"slug": erp.slug}), + data={"accessibilite": {"commentaire": "still works too"}}, + format="json", + ) + assert response.status_code == 200 + + def test_invalid_key_falls_through_gracefully(self, api_client): + response = api_client.get( + reverse("erp-list"), + headers={"Authorization": "Api-Key totally-invalid-key"}, + ) + assert response.status_code == 200 + + def test_malformed_authorization_header_does_not_crash(self, api_client): + response = api_client.get( + reverse("erp-list"), + headers={"Authorization": "not-even-two-parts"}, + ) + assert response.status_code == 200 + + +@pytest.mark.django_db +class TestUserAPIKeyAuthenticationUnit: + def test_no_header_returns_none(self, rf): + request = rf.get("/") + assert UserAPIKeyAuthentication().authenticate(request) is None + + def test_valid_key_returns_user_and_key_instance(self, rf): + user = User.objects.create_user(username="alice") + _, key = UserAPIKey.objects.create_key(name="alice-key", user=user) + request = rf.get("/", HTTP_AUTHORIZATION=f"Api-Key {key}") + + result = UserAPIKeyAuthentication().authenticate(request) + + assert result is not None + auth_user, auth_key = result + assert auth_user == user + assert auth_key.user == user + + def test_unknown_key_returns_none(self, rf): + request = rf.get("/", HTTP_AUTHORIZATION="Api-Key nonexistent-key") + assert UserAPIKeyAuthentication().authenticate(request) is None + + def test_revoked_key_returns_none(self, rf): + user = User.objects.create_user(username="bob") + api_key_obj, key = UserAPIKey.objects.create_key(name="bob-key", user=user) + api_key_obj.revoked = True + api_key_obj.save() + + request = rf.get("/", HTTP_AUTHORIZATION=f"Api-Key {key}") + assert UserAPIKeyAuthentication().authenticate(request) is None + + @pytest.fixture def initial_erp(): boulangerie = ActiviteFactory(nom="Boulangerie")