diff --git a/src/c3nav/mapdata/api.py b/src/c3nav/mapdata/api.py index 603dc4c4..0a24858d 100644 --- a/src/c3nav/mapdata/api.py +++ b/src/c3nav/mapdata/api.py @@ -19,7 +19,6 @@ from rest_framework.mixins import RetrieveModelMixin, UpdateModelMixin from rest_framework.response import Response from rest_framework.viewsets import GenericViewSet, ReadOnlyModelViewSet, ViewSet -from c3nav.mapdata.forms import PositionAPIUpdateForm from c3nav.mapdata.models import AccessRestriction, Building, Door, Hole, LocationGroup, MapUpdate, Source, Space from c3nav.mapdata.models.access import AccessPermission, AccessRestrictionGroup from c3nav.mapdata.models.geometry.base import GeometryMixin @@ -485,20 +484,6 @@ class DynamicLocationPositionViewSet(UpdateModelMixin, RetrieveModelMixin, Gener obj = self.get_object() return Response(obj.serialize_position()) - def update(self, request, *args, **kwargs): - instance = self.get_object() - params = request.data - form = PositionAPIUpdateForm(instance=instance, data=params, request=request) - - if not form.is_valid(): - return Response({ - 'errors': form.errors, - }, status=400) - - form.save() - - return Response(form.instance.serialize_position()) - class SourceViewSet(MapdataViewSet): queryset = Source.objects.all() diff --git a/src/c3nav/mapdata/forms.py b/src/c3nav/mapdata/forms.py index 00116cad..4b3dfc42 100644 --- a/src/c3nav/mapdata/forms.py +++ b/src/c3nav/mapdata/forms.py @@ -65,40 +65,3 @@ class I18nModelFormMixin(ModelForm): super().full_clean() for field, values in self.i18n_fields: setattr(self.instance, field.attname, {lang: value for lang, value in values.items() if value}) - - -class PositionAPIUpdateForm(ModelForm): - secret = CharField() - - def __init__(self, *args, request=None, **kwargs): - self.request = request - super().__init__(*args, **kwargs) - - class Meta: - model = Position - fields = ['coordinates_id', 'timeout'] - - def save(self, commit=True): - self.instance.last_coordinates_update = timezone.now() - super().save(commit) - - def clean_secret(self): - # not called api_secret so we don't overwrite it - api_secret = self.cleaned_data['secret'] - if api_secret != self.instance.api_secret: - raise ValidationError(_('Wrong API secret.')) - return api_secret - - def clean_coordinates_id(self): - coordinates_id = self.cleaned_data['coordinates_id'] - if coordinates_id is None: - return coordinates_id - - if not coordinates_id.startswith('c:'): - raise ValidationError(_('Invalid coordinates.')) - - coordinates = get_location_by_id_for_request(self.cleaned_data['coordinates_id'], self.request) - if coordinates is None: - raise ValidationError(_('Invalid coordinates.')) - - return coordinates_id diff --git a/src/c3nav/mapdata/migrations/0088_remove_position_api_secret.py b/src/c3nav/mapdata/migrations/0088_remove_position_api_secret.py new file mode 100644 index 00000000..9f90e6f7 --- /dev/null +++ b/src/c3nav/mapdata/migrations/0088_remove_position_api_secret.py @@ -0,0 +1,16 @@ +# Generated by Django 4.2.1 on 2023-11-24 14:41 + +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("mapdata", "0087_rangingbeacon"), + ] + + operations = [ + migrations.RemoveField( + model_name="position", + name="api_secret", + ), + ] diff --git a/src/c3nav/mapdata/models/locations.py b/src/c3nav/mapdata/models/locations.py index 3723d81d..b10dadd0 100644 --- a/src/c3nav/mapdata/models/locations.py +++ b/src/c3nav/mapdata/models/locations.py @@ -588,7 +588,6 @@ class Position(CustomLocationProxyMixin, models.Model): timeout = models.PositiveSmallIntegerField(_('timeout (in seconds)'), default=0, blank=True, help_text=_('0 for no timeout')) coordinates_id = models.CharField(_('coordinates'), null=True, blank=True, max_length=48) - api_secret = models.CharField(_('api secret'), max_length=64, default=get_position_api_secret) can_search = True can_describe = False diff --git a/src/c3nav/mapdata/newapi/map.py b/src/c3nav/mapdata/newapi/map.py index 0f6eafeb..37072bd6 100644 --- a/src/c3nav/mapdata/newapi/map.py +++ b/src/c3nav/mapdata/newapi/map.py @@ -1,24 +1,25 @@ import json +from typing import Annotated, Optional from django.core.cache import cache from django.core.serializers.json import DjangoJSONEncoder -from django.http import Http404 from django.shortcuts import redirect +from django.utils import timezone from ninja import Query from ninja import Router as APIRouter from ninja import Schema -from ninja.decorators import decorate_view from pydantic import Field as APIField +from pydantic import PositiveInt -from c3nav.api.exceptions import API404 -from c3nav.api.newauth import auth_responses, validate_responses +from c3nav.api.exceptions import API404, APIPermissionDenied, APIRequestValidationFailed +from c3nav.api.newauth import auth_permission_responses, auth_responses, validate_responses from c3nav.api.utils import NonEmptyStr from c3nav.mapdata.models import Source from c3nav.mapdata.models.access import AccessPermission from c3nav.mapdata.models.locations import DynamicLocation, LocationRedirect, Position from c3nav.mapdata.newapi.base import newapi_etag, newapi_stats from c3nav.mapdata.schemas.filters import BySearchableFilter, RemoveGeometryFilter -from c3nav.mapdata.schemas.model_base import AnyLocationID, AnyPositionID +from c3nav.mapdata.schemas.model_base import AnyLocationID, AnyPositionID, CustomLocationID from c3nav.mapdata.schemas.models import (AnyPositionStatusSchema, FullListableLocationSchema, FullLocationSchema, LocationDisplay, SlimListableLocationSchema, SlimLocationSchema) from c3nav.mapdata.schemas.responses import BoundsSchema, LocationGeometry @@ -96,7 +97,7 @@ def _location_retrieve(request, location, detailed: bool, geometry: bool, show_r # todo: cache, visibility, etc… if location is None: - raise API404 + raise API404() if isinstance(location, LocationRedirect): if not show_redirects: @@ -113,7 +114,7 @@ def _location_display(request, location): # todo: cache, visibility, etc… if location is None: - raise API404 + raise API404() if isinstance(location, LocationRedirect): return redirect('../' + str(location.target.slug) + '/details/') # todo: use reverse, make pk+slug work @@ -131,7 +132,7 @@ def _location_geometry(request, location): # todo: cache, visibility, etc… if location is None: - raise API404 + raise API404() if isinstance(location, LocationRedirect): return redirect('../' + str(location.target.slug) + '/geometry/') # todo: use reverse, make pk+slug work @@ -263,7 +264,7 @@ def location_by_slug_geometry(request, location_slug: NonEmptyStr): @map_api_router.get('/get_position/{position_id}/', response={200: AnyPositionStatusSchema, **API404.dict(), **auth_responses}, - summary="get current position of a moving object", + summary="get coordinates of a moving position", description="a numeric ID for a dynamic location or a string ID for the position secret can be used") @newapi_stats('get_position') def get_current_position_by_id(request, position_id: AnyPositionID): @@ -272,10 +273,49 @@ def get_current_position_by_id(request, position_id: AnyPositionID): if isinstance(position_id, int) or position_id.isdigit(): location = get_location_by_id_for_request(position_id, request) if not isinstance(location, DynamicLocation): - raise Http404 + raise API404() if location is None and position_id.startswith('p:'): try: location = Position.objects.get(secret=position_id[2:]) except Position.DoesNotExist: - raise Http404 + raise API404() + return location.serialize_position() + + +class UpdatePositionSchema(Schema): + coordinates_id: Optional[CustomLocationID] = APIField( + description="coordinates to set the location to or None to unset it" + ) + timeout: Optional[int] = APIField( + None, + description="timeout for this new location in seconds, or None if not to change it", + example=None, + ) + + +@map_api_router.put('/get_position/{position_id}/', url_name="position-update", + response={200: AnyPositionStatusSchema, **API404.dict(), **auth_permission_responses}, + summary="set coordinates of a moving position", + description="only the string ID for the position secret must be used") +@newapi_stats('get_position') +def position_update(request, position_id: AnyPositionID, update: UpdatePositionSchema): + # todo: may an API key do this? + if not update.position_id.startswith('p:'): + raise API404() + try: + location = Position.objects.get(secret=update.position_id[2:]) + except Position.DoesNotExist: + raise API404() + if location.owner != request.user: + raise APIPermissionDenied() + + coordinates = get_location_by_id_for_request(update.coordinates_id, request) + if coordinates is None: + raise APIRequestValidationFailed('Cant resolve coordinates.') + + location.coordinates_id = update.coordinates_id + location.timeout = update.timeout + location.last_coordinates_update = timezone.now() + location.save() + return location.serialize_position() diff --git a/src/c3nav/mapdata/schemas/model_base.py b/src/c3nav/mapdata/schemas/model_base.py index dae3c108..6d0fa7eb 100644 --- a/src/c3nav/mapdata/schemas/model_base.py +++ b/src/c3nav/mapdata/schemas/model_base.py @@ -195,6 +195,7 @@ class SimpleGeometryLocationsSchema(Schema): CustomLocationID = Annotated[NonEmptyStr, APIField( title="custom location ID", pattern=r"c:[a-z0-9-_]+:(-?\d+(\.\d+)?):(-?\d+(\.\d+)?)$", + example="c:0:-7.23:12.34", description="level short_name and x/y coordinates form the ID of a custom location" )] PositionID = Annotated[NonEmptyStr, APIField( diff --git a/src/c3nav/routing/api.py b/src/c3nav/routing/api.py index fc4fc431..bd41d4c8 100644 --- a/src/c3nav/routing/api.py +++ b/src/c3nav/routing/api.py @@ -6,7 +6,6 @@ from rest_framework.response import Response from rest_framework.viewsets import ViewSet from c3nav.mapdata.api import api_stats_clean_location_value -from c3nav.mapdata.forms import PositionAPIUpdateForm from c3nav.mapdata.models.access import AccessPermission from c3nav.mapdata.models.locations import Position from c3nav.mapdata.utils.cache.stats import increment_cache_key @@ -159,7 +158,8 @@ class RoutingViewSet(ViewSet): 'coordinates_id': None if location is None else location.pk, } - form = PositionAPIUpdateForm(instance=position, data=form_data, request=request) + # todo: migrate + #form = PositionAPIUpdateForm(instance=position, data=form_data, request=request) if not form.is_valid(): return Response({ diff --git a/src/c3nav/site/templates/site/position_detail.html b/src/c3nav/site/templates/site/position_detail.html index fd04f710..ec354e2e 100644 --- a/src/c3nav/site/templates/site/position_detail.html +++ b/src/c3nav/site/templates/site/position_detail.html @@ -18,10 +18,6 @@ {% trans 'Secret' %}: {{ position.secret }}

-

- {% trans 'API secret' %}: - {{ position.api_secret }} -


{% trans 'How to manage' %}

@@ -33,21 +29,9 @@ {% trans 'We only keep your last position, we do not save any position history.' %}

- {% trans 'To set it via the API, send a JSON PUT request to:' %}
- /api/locations/dynamic/p:{{ position.secret }}/
-

-
-    {
-        "coordinates_id": "c:z:xx.x:yy.y",
-        "secret": "your API secret",
-        "timeout": (in seconds, only if you want to change it)
-    }
-

- {% trans 'To get it via the API, just send a GET request to that URL.' %} -

-

- {% trans 'To access this position on the map, visit:' %}
- /l/p:{{ position.secret }}/ + {% trans 'To get and set it via the API, use this API endpoint:' %}
+ {% url 'api-v2:position-update' position_id=position.slug %} + ({% trans 'View OpenAPI documentation' %})


@@ -57,7 +41,6 @@ {{ form.as_p }} - diff --git a/src/c3nav/site/views.py b/src/c3nav/site/views.py index e41cb940..fe8dc146 100644 --- a/src/c3nav/site/views.py +++ b/src/c3nav/site/views.py @@ -535,9 +535,6 @@ def position_detail(request, pk): if request.POST.get('reset_secret', None): position.secret = get_position_secret() - if request.POST.get('reset_api_secret', None): - position.api_secret = get_position_api_secret() - form = PositionForm(instance=position, data=request.POST) if form.is_valid(): form.save()