From 4325b1be4f8b826804d90358d6994033a6ceacff Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Sun, 6 May 2018 11:18:28 +0200
Subject: [PATCH] Removed radios and listening recording for anonymous users as
 it was buggy

---
 api/config/settings/common.py            |  1 -
 api/funkwhale_api/history/models.py      |  7 ------
 api/funkwhale_api/history/serializers.py |  8 ++-----
 api/funkwhale_api/history/views.py       | 24 +++++++++-----------
 api/funkwhale_api/radios/models.py       |  2 --
 api/funkwhale_api/radios/serializers.py  |  8 +++----
 api/funkwhale_api/radios/views.py        | 22 ++++++-------------
 api/funkwhale_api/users/middleware.py    | 11 ----------
 api/tests/history/test_history.py        | 15 -------------
 api/tests/radios/test_radios.py          | 28 ++++++------------------
 front/src/store/player.js                |  5 ++++-
 11 files changed, 33 insertions(+), 98 deletions(-)
 delete mode 100644 api/funkwhale_api/users/middleware.py

diff --git a/api/config/settings/common.py b/api/config/settings/common.py
index d2948064..50bc52fe 100644
--- a/api/config/settings/common.py
+++ b/api/config/settings/common.py
@@ -144,7 +144,6 @@ INSTALLED_APPS = DJANGO_APPS + THIRD_PARTY_APPS + LOCAL_APPS
 MIDDLEWARE = (
     # Make sure djangosecure.middleware.SecurityMiddleware is listed first
     'django.contrib.sessions.middleware.SessionMiddleware',
-    'funkwhale_api.users.middleware.AnonymousSessionMiddleware',
     'corsheaders.middleware.CorsMiddleware',
     'django.middleware.common.CommonMiddleware',
     'django.middleware.csrf.CsrfViewMiddleware',
diff --git a/api/funkwhale_api/history/models.py b/api/funkwhale_api/history/models.py
index 762d5bf7..480461d3 100644
--- a/api/funkwhale_api/history/models.py
+++ b/api/funkwhale_api/history/models.py
@@ -21,13 +21,6 @@ class Listening(models.Model):
     class Meta:
         ordering = ('-creation_date',)
 
-    def save(self, **kwargs):
-        if not self.user and not self.session_key:
-            raise ValidationError('Cannot have both session_key and user empty for listening')
-
-        super().save(**kwargs)
-
-
     def get_activity_url(self):
         return '{}/listenings/tracks/{}'.format(
             self.user.get_activity_url(), self.pk)
diff --git a/api/funkwhale_api/history/serializers.py b/api/funkwhale_api/history/serializers.py
index 8fe6fa6e..f7333f24 100644
--- a/api/funkwhale_api/history/serializers.py
+++ b/api/funkwhale_api/history/serializers.py
@@ -36,13 +36,9 @@ class ListeningSerializer(serializers.ModelSerializer):
 
     class Meta:
         model = models.Listening
-        fields = ('id', 'user', 'session_key', 'track', 'creation_date')
-
+        fields = ('id', 'user', 'track', 'creation_date')
 
     def create(self, validated_data):
-        if self.context.get('user'):
-            validated_data['user'] = self.context.get('user')
-        else:
-            validated_data['session_key'] = self.context['session_key']
+        validated_data['user'] = self.context['user']
 
         return super().create(validated_data)
diff --git a/api/funkwhale_api/history/views.py b/api/funkwhale_api/history/views.py
index d5cbe316..bea96a41 100644
--- a/api/funkwhale_api/history/views.py
+++ b/api/funkwhale_api/history/views.py
@@ -1,4 +1,5 @@
 from rest_framework import generics, mixins, viewsets
+from rest_framework import permissions
 from rest_framework import status
 from rest_framework.response import Response
 from rest_framework.decorators import detail_route
@@ -10,31 +11,26 @@ from funkwhale_api.music.serializers import TrackSerializerNested
 from . import models
 from . import serializers
 
-class ListeningViewSet(mixins.CreateModelMixin,
-                          mixins.RetrieveModelMixin,
-                          viewsets.GenericViewSet):
+
+class ListeningViewSet(
+        mixins.CreateModelMixin,
+        mixins.RetrieveModelMixin,
+        viewsets.GenericViewSet):
 
     serializer_class = serializers.ListeningSerializer
     queryset = models.Listening.objects.all()
-    permission_classes = [ConditionalAuthentication]
+    permission_classes = [permissions.IsAuthenticated]
 
     def perform_create(self, serializer):
         r = super().perform_create(serializer)
-        if self.request.user.is_authenticated:
-            record.send(serializer.instance)
+        record.send(serializer.instance)
         return r
 
     def get_queryset(self):
         queryset = super().get_queryset()
-        if self.request.user.is_authenticated:
-            return queryset.filter(user=self.request.user)
-        else:
-            return queryset.filter(session_key=self.request.session.session_key)
+        return queryset.filter(user=self.request.user)
 
     def get_serializer_context(self):
         context = super().get_serializer_context()
-        if self.request.user.is_authenticated:
-            context['user'] = self.request.user
-        else:
-            context['session_key'] = self.request.session.session_key
+        context['user'] = self.request.user
         return context
diff --git a/api/funkwhale_api/radios/models.py b/api/funkwhale_api/radios/models.py
index 0273b538..8758abc6 100644
--- a/api/funkwhale_api/radios/models.py
+++ b/api/funkwhale_api/radios/models.py
@@ -55,8 +55,6 @@ class RadioSession(models.Model):
     related_object = GenericForeignKey('related_object_content_type', 'related_object_id')
 
     def save(self, **kwargs):
-        if not self.user and not self.session_key:
-            raise ValidationError('Cannot have both session_key and user empty for radio session')
         self.radio.clean(self)
         super().save(**kwargs)
 
diff --git a/api/funkwhale_api/radios/serializers.py b/api/funkwhale_api/radios/serializers.py
index 2e7e6a40..195b382c 100644
--- a/api/funkwhale_api/radios/serializers.py
+++ b/api/funkwhale_api/radios/serializers.py
@@ -38,6 +38,7 @@ class RadioSerializer(serializers.ModelSerializer):
 
         return super().save(**kwargs)
 
+
 class RadioSessionTrackSerializerCreate(serializers.ModelSerializer):
     class Meta:
         model = models.RadioSessionTrack
@@ -62,17 +63,14 @@ class RadioSessionSerializer(serializers.ModelSerializer):
             'user',
             'creation_date',
             'custom_radio',
-            'session_key')
+        )
 
     def validate(self, data):
         registry[data['radio_type']]().validate_session(data, **self.context)
         return data
 
     def create(self, validated_data):
-        if self.context.get('user'):
-            validated_data['user'] = self.context.get('user')
-        else:
-            validated_data['session_key'] = self.context['session_key']
+        validated_data['user'] = self.context['user']
         if validated_data.get('related_object_id'):
             radio = registry[validated_data['radio_type']]()
             validated_data['related_object'] = radio.get_related_object(validated_data['related_object_id'])
diff --git a/api/funkwhale_api/radios/views.py b/api/funkwhale_api/radios/views.py
index ffd1d165..37c07c5e 100644
--- a/api/funkwhale_api/radios/views.py
+++ b/api/funkwhale_api/radios/views.py
@@ -2,6 +2,7 @@ from django.db.models import Q
 from django.http import Http404
 
 from rest_framework import generics, mixins, viewsets
+from rest_framework import permissions
 from rest_framework import status
 from rest_framework.response import Response
 from rest_framework.decorators import detail_route, list_route
@@ -24,7 +25,7 @@ class RadioViewSet(
         viewsets.GenericViewSet):
 
     serializer_class = serializers.RadioSerializer
-    permission_classes = [ConditionalAuthentication]
+    permission_classes = [permissions.IsAuthenticated]
     filter_class = filtersets.RadioFilter
 
     def get_queryset(self):
@@ -84,21 +85,15 @@ class RadioSessionViewSet(mixins.CreateModelMixin,
 
     serializer_class = serializers.RadioSessionSerializer
     queryset = models.RadioSession.objects.all()
-    permission_classes = [ConditionalAuthentication]
+    permission_classes = [permissions.IsAuthenticated]
 
     def get_queryset(self):
         queryset = super().get_queryset()
-        if self.request.user.is_authenticated:
-            return queryset.filter(user=self.request.user)
-        else:
-            return queryset.filter(session_key=self.request.session.session_key)
+        return queryset.filter(user=self.request.user)
 
     def get_serializer_context(self):
         context = super().get_serializer_context()
-        if self.request.user.is_authenticated:
-            context['user'] = self.request.user
-        else:
-            context['session_key'] = self.request.session.session_key
+        context['user'] = self.request.user
         return context
 
 
@@ -106,17 +101,14 @@ class RadioSessionTrackViewSet(mixins.CreateModelMixin,
                                viewsets.GenericViewSet):
     serializer_class = serializers.RadioSessionTrackSerializer
     queryset = models.RadioSessionTrack.objects.all()
-    permission_classes = [ConditionalAuthentication]
+    permission_classes = [permissions.IsAuthenticated]
 
     def create(self, request, *args, **kwargs):
         serializer = self.get_serializer(data=request.data)
         serializer.is_valid(raise_exception=True)
         session = serializer.validated_data['session']
         try:
-            if request.user.is_authenticated:
-                assert request.user == session.user
-            else:
-                assert request.session.session_key == session.session_key
+            assert request.user == session.user
         except AssertionError:
             return Response(status=status.HTTP_403_FORBIDDEN)
         track = session.radio.pick()
diff --git a/api/funkwhale_api/users/middleware.py b/api/funkwhale_api/users/middleware.py
deleted file mode 100644
index e3eba95f..00000000
--- a/api/funkwhale_api/users/middleware.py
+++ /dev/null
@@ -1,11 +0,0 @@
-
-
-class AnonymousSessionMiddleware:
-    def __init__(self, get_response):
-        self.get_response = get_response
-
-    def __call__(self, request):
-        if not request.session.session_key:
-            request.session.save()
-        response = self.get_response(request)
-        return response
diff --git a/api/tests/history/test_history.py b/api/tests/history/test_history.py
index 563cf2f0..20272559 100644
--- a/api/tests/history/test_history.py
+++ b/api/tests/history/test_history.py
@@ -14,21 +14,6 @@ def test_can_create_listening(factories):
     l = models.Listening.objects.create(user=user, track=track)
 
 
-def test_anonymous_user_can_create_listening_via_api(
-        client, factories, preferences):
-    preferences['common__api_authentication_required'] = False
-    track = factories['music.Track']()
-    url = reverse('api:v1:history:listenings-list')
-    response = client.post(url, {
-        'track': track.pk,
-    })
-
-    listening = models.Listening.objects.latest('id')
-
-    assert listening.track == track
-    assert listening.session_key == client.session.session_key
-
-
 def test_logged_in_user_can_create_listening_via_api(
         logged_in_client, factories, activity_muted):
     track = factories['music.Track']()
diff --git a/api/tests/radios/test_radios.py b/api/tests/radios/test_radios.py
index e78bd0e2..b166b648 100644
--- a/api/tests/radios/test_radios.py
+++ b/api/tests/radios/test_radios.py
@@ -151,20 +151,6 @@ def test_can_start_radio_for_logged_in_user(logged_in_client):
     assert session.user == logged_in_client.user
 
 
-def test_can_start_radio_for_anonymous_user(api_client, db, preferences):
-    preferences['common__api_authentication_required'] = False
-    url = reverse('api:v1:radios:sessions-list')
-    response = api_client.post(url, {'radio_type': 'random'})
-
-    assert response.status_code == 201
-
-    session = models.RadioSession.objects.latest('id')
-
-    assert session.radio_type == 'random'
-    assert session.user is None
-    assert session.session_key == api_client.session.session_key
-
-
 def test_can_get_track_for_session_from_api(factories, logged_in_client):
     files = factories['music.TrackFile'].create_batch(1)
     tracks = [f.track for f in files]
@@ -227,25 +213,25 @@ def test_can_start_tag_radio(factories):
 
     radio = radios.TagRadio()
     session = radio.start_session(user, related_object=tag)
-    assert session.radio_type =='tag'
+    assert session.radio_type == 'tag'
     for i in range(5):
         assert radio.pick() in good_tracks
 
 
-def test_can_start_artist_radio_from_api(api_client, preferences, factories):
-    preferences['common__api_authentication_required'] = False
+def test_can_start_artist_radio_from_api(
+        logged_in_api_client, preferences, factories):
     artist = factories['music.Artist']()
     url = reverse('api:v1:radios:sessions-list')
 
-    response = api_client.post(
+    response = logged_in_api_client.post(
         url, {'radio_type': 'artist', 'related_object_id': artist.id})
 
     assert response.status_code == 201
 
     session = models.RadioSession.objects.latest('id')
 
-    assert session.radio_type, 'artist'
-    assert session.related_object, artist
+    assert session.radio_type == 'artist'
+    assert session.related_object == artist
 
 
 def test_can_start_less_listened_radio(factories):
@@ -257,6 +243,6 @@ def test_can_start_less_listened_radio(factories):
     good_tracks = [f.track for f in good_files]
     radio = radios.LessListenedRadio()
     session = radio.start_session(user)
-    assert session.related_object == user
+
     for i in range(5):
         assert radio.pick() in good_tracks
diff --git a/front/src/store/player.js b/front/src/store/player.js
index ed437c3f..2149b51f 100644
--- a/front/src/store/player.js
+++ b/front/src/store/player.js
@@ -85,7 +85,10 @@ export default {
     togglePlay ({commit, state}) {
       commit('playing', !state.playing)
     },
-    trackListened ({commit}, track) {
+    trackListened ({commit, rootState}, track) {
+      if (!rootState.auth.authenticated) {
+        return
+      }
       return axios.post('history/listenings/', {'track': track.id}).then((response) => {}, (response) => {
         logger.default.error('Could not record track in history')
       })
-- 
GitLab