Resolve "Use cookies instead of local storage for auth in Web UI"

api/config/api_urls.py

@@ -75,6 +75,9 @@ v1_patterns += [
         r"^users/",
         include(("funkwhale_api.users.api_urls", "users"), namespace="users"),
     ),
+    url(
+        r"^auth/", include(("funkwhale_api.users.auth_urls", "auth"), namespace="auth")
+    ),
     url(r"^token/$", jwt_views.obtain_jwt_token, name="token"),
     url(r"^token/refresh/$", jwt_views.refresh_jwt_token, name="token_refresh"),
 ]

api/config/routing.py

@@ -1,4 +1,5 @@
 from channels.routing import ProtocolTypeRouter, URLRouter
+from channels.sessions import SessionMiddlewareStack
 from django.conf.urls import url
 
 from funkwhale_api.common.auth import TokenAuthMiddleware
@@ -7,8 +8,12 @@ from funkwhale_api.instance import consumers
 application = ProtocolTypeRouter(
     {
         # Empty for now (http->django views is added by default)
-        "websocket": TokenAuthMiddleware(
+        "websocket": SessionMiddlewareStack(
-            URLRouter([url("^api/v1/activity$", consumers.InstanceActivityConsumer)])
+            TokenAuthMiddleware(
+                URLRouter(
+                    [url("^api/v1/activity$", consumers.InstanceActivityConsumer)]
+                )
+            )
         )
     }
 )

api/config/settings/common.py

@@ -330,7 +330,7 @@ AUTHENTICATION_BACKENDS = (
     "funkwhale_api.users.auth_backends.ModelBackend",
     "allauth.account.auth_backends.AuthenticationBackend",
 )
-SESSION_COOKIE_HTTPONLY = False
+SESSION_COOKIE_HTTPONLY = True
 # Some really nice defaults
 ACCOUNT_AUTHENTICATION_METHOD = "username_email"
 ACCOUNT_EMAIL_REQUIRED = True
@@ -537,7 +537,7 @@ MUSICBRAINZ_HOSTNAME = env("MUSICBRAINZ_HOSTNAME", default="musicbrainz.org")
 
 # Custom Admin URL, use {% url 'admin:index' %}
 ADMIN_URL = env("DJANGO_ADMIN_URL", default="^api/admin/")
-CSRF_USE_SESSIONS = True
+CSRF_USE_SESSIONS = False
 SESSION_ENGINE = "django.contrib.sessions.backends.cache"
 
 # Playlist settings

api/funkwhale_api/common/consumers.py

@@ -1,16 +1,21 @@
+from asgiref.sync import async_to_sync
 from channels.generic.websocket import JsonWebsocketConsumer
-
+from channels import auth
 from funkwhale_api.common import channels
 
 
 class JsonAuthConsumer(JsonWebsocketConsumer):
     def connect(self):
-        try:
+        if "user" not in self.scope:
-            assert self.scope["user"].pk is not None
+            try:
-        except (AssertionError, AttributeError, KeyError):
+                self.scope["user"] = async_to_sync(auth.get_user)(self.scope)
-            return self.close()
+            except (ValueError, AssertionError, AttributeError, KeyError):
+                return self.close()
 
-        return self.accept()
+        if self.scope["user"] and self.scope["user"].is_authenticated:
+            return self.accept()
+        else:
+            return self.close()
 
     def accept(self):
         super().accept()

api/funkwhale_api/users/auth_serializers.py

@@ -0,0 +1,16 @@
+from django.contrib import auth
+
+from rest_framework import serializers
+
+
+class LoginSerializer(serializers.Serializer):
+    username = serializers.CharField()
+    password = serializers.CharField()
+
+    def validate(self, validated_data):
+        user = auth.authenticate(request=None, **validated_data)
+        if user is None:
+            raise serializers.ValidationError("Invalid username or password")
+
+        validated_data["user"] = user
+        return validated_data

api/funkwhale_api/users/auth_urls.py

@@ -0,0 +1,8 @@
+from django.conf.urls import url
+
+from . import auth_views
+
+urlpatterns = [
+    url(r"^login/$", auth_views.LoginView.as_view(), name="login"),
+    url(r"^logout/$", auth_views.LogoutView.as_view(), name="logout"),
+]

api/funkwhale_api/users/auth_views.py

@@ -0,0 +1,32 @@
+from django.contrib import auth
+
+from rest_framework import response
+from rest_framework import views
+
+from . import auth_serializers
+
+
+class LoginView(views.APIView):
+    authentication_classes = []
+    permission_classes = []
+
+    def post(self, request, *args, **kwargs):
+
+        serializer = auth_serializers.LoginSerializer(data=request.data)
+        serializer.is_valid(raise_exception=True)
+
+        auth.login(request=request, user=serializer.validated_data["user"])
+
+        payload = {}
+
+        return response.Response(payload)
+
+
+class LogoutView(views.APIView):
+    authentication_classes = []
+    permission_classes = []
+
+    def post(self, request, *args, **kwargs):
+        auth.logout(request)
+        payload = {}
+        return response.Response(payload)

api/tests/users/test_auth_views.py

@@ -0,0 +1,86 @@
+from django.contrib import auth
+from django.urls import reverse
+
+
+def test_restricted_access(api_client, db):
+    url = reverse("api:v1:artists-list")
+    response = api_client.get(url)
+
+    assert response.status_code == 401
+
+
+def test_login_correct(api_client, factories, mocker):
+    login = mocker.spy(auth, "login")
+    password = "hellotest"
+    user = factories["users.User"]()
+    user.set_password(password)
+    user.save()
+
+    url = reverse("api:v1:auth:login")
+    data = {"username": user.username, "password": password}
+    expected = {}
+    response = api_client.post(url, data)
+
+    assert response.status_code == 200
+    assert response.data == expected
+    login.assert_called_once_with(request=mocker.ANY, user=user)
+
+
+def test_login_incorrect(api_client, factories, mocker):
+    login = mocker.spy(auth, "login")
+    user = factories["users.User"]()
+
+    url = reverse("api:v1:auth:login")
+    data = {"username": user.username, "password": "invalid"}
+    response = api_client.post(url, data)
+
+    assert response.status_code == 400
+
+    login.assert_not_called()
+
+
+def test_login_inactive(api_client, factories, mocker):
+    login = mocker.spy(auth, "login")
+    password = "hellotest"
+    user = factories["users.User"](is_active=False)
+    user.set_password(password)
+    user.save()
+
+    url = reverse("api:v1:auth:login")
+    data = {"username": user.username, "password": password}
+    response = api_client.post(url, data)
+
+    assert response.status_code == 400
+    assert "Invalid username or password" in response.data["non_field_errors"]
+
+    login.assert_not_called()
+
+
+def test_logout(logged_in_api_client, factories, mocker):
+    logout = mocker.spy(auth, "logout")
+
+    url = reverse("api:v1:auth:logout")
+    response = logged_in_api_client.post(url)
+
+    assert response.status_code == 200
+    assert response.data == {}
+    logout.assert_called_once_with(request=mocker.ANY)
+
+
+def test_logout_real(api_client, factories):
+    password = "hellotest"
+    user = factories["users.User"]()
+    user.set_password(password)
+    user.save()
+
+    url = reverse("api:v1:auth:login")
+    data = {"username": user.username, "password": password}
+    response = api_client.post(url, data)
+
+    url = reverse("api:v1:auth:logout")
+    response = api_client.post(url)
+
+    url = reverse("api:v1:artists-list")
+    response = api_client.get(url)
+
+    assert response.status_code == 401

changes/changelog.d/629.bugfix

@@ -0,0 +1 @@
+Now use cookie-based auth in browser instead of JWT/LocalStorage in (#629)

front/package.json

@@ -17,7 +17,6 @@
     "django-channels": "^1.1.6",
     "howler": "^2.0.14",
     "js-logger": "^1.4.1",
-    "jwt-decode": "^2.2.0",
     "lodash": "^4.17.10",
     "masonry-layout": "^4.2.2",
     "moment": "^2.22.2",

front/src/App.vue

@@ -175,11 +175,9 @@ export default {
       }
       this.disconnect()
       let self = this
-      let token = this.$store.state.auth.token
-      // let token = 'test'
       const bridge = new WebSocketBridge()
       this.bridge = bridge
-      let url = this.$store.getters['instance/absoluteUrl'](`api/v1/activity?token=${token}`)
+      let url = this.$store.getters['instance/absoluteUrl'](`api/v1/activity`)
       url = url.replace('http://', 'ws://')
       url = url.replace('https://', 'wss://')
       bridge.connect(

front/src/components/audio/Track.vue

@@ -71,15 +71,6 @@ export default {
           'mp3'
         )
       })
-      if (this.$store.state.auth.authenticated) {
-        // we need to send the token directly in url
-        // so authentication can be checked by the backend
-        // because for audio files we cannot use the regular Authentication
-        // header
-        sources.forEach(e => {
-          e.url = url.updateQueryString(e.url, 'jwt', this.$store.state.auth.token)
-        })
-      }
       return sources
     },
     updateProgressThrottled () {

front/src/components/library/TrackBase.vue

@@ -154,13 +154,6 @@ export default {
       let u = this.$store.getters["instance/absoluteUrl"](
         this.upload.listen_url
       )
-      if (this.$store.state.auth.authenticated) {
-        u = url.updateQueryString(
-          u,
-          "jwt",
-          encodeURI(this.$store.state.auth.token)
-        )
-      }
       return u
     },
     cover() {

front/src/main.js

@@ -16,6 +16,7 @@ import GetTextPlugin from 'vue-gettext'
 import { sync } from 'vuex-router-sync'
 import locales from '@/locales'
 
+import cookie from '@/utils/cookie'
 import filters from '@/filters' // eslint-disable-line
 import globals from '@/components/globals' // eslint-disable-line
 
@@ -70,8 +71,9 @@ Vue.directive('title', function (el, binding) {
 )
 axios.interceptors.request.use(function (config) {
   // Do something before request is sent
-  if (store.state.auth.token) {
+  let csrfToken = cookie.get('csrftoken')
-    config.headers['Authorization'] = store.getters['auth/header']
+  if (csrfToken) {
+    config.headers['X-CSRFToken'] = csrfToken
   }
   return config
 }, function (error) {
@@ -85,9 +87,14 @@ axios.interceptors.response.use(function (response) {
 }, function (error) {
   error.backendErrors = []
   if (error.response.status === 401) {
+    if (error.response.config.skipLoginRedirect) {
+      return Promise.reject(error)
+    }
     store.commit('auth/authenticated', false)
-    logger.default.warn('Received 401 response from API, redirecting to login form', router.currentRoute.fullPath)
+    if (router.currentRoute.name !== 'login') {
-    router.push({name: 'login', query: {next: router.currentRoute.fullPath}})
+      logger.default.warn('Received 401 response from API, redirecting to login form', router.currentRoute.fullPath)
+      router.push({name: 'login', query: {next: router.currentRoute.fullPath}})
+    }
   }
   if (error.response.status === 404) {
     error.backendErrors.push('Resource not found')

front/src/store/auth.js

@@ -1,5 +1,4 @@
 import axios from 'axios'
-import jwtDecode from 'jwt-decode'
 import logger from '@/logging'
 import router from '@/router'
 
@@ -15,13 +14,6 @@ export default {
       moderation: false
     },
     profile: null,
-    token: '',
-    tokenData: {}
-  },
-  getters: {
-    header: state => {
-      return 'JWT ' + state.token
-    }
   },
   mutations: {
     reset (state) {
@@ -29,8 +21,6 @@ export default {
       state.profile = null
       state.username = ''
       state.fullUsername = ''
-      state.token = ''
-      state.tokenData = {}
       state.availablePermissions = {
         federation: false,
         settings: false,
@@ -46,8 +36,6 @@ export default {
       if (value === false) {
         state.username = null
         state.fullUsername = null
-        state.token = null
-        state.tokenData = null
         state.profile = null
         state.availablePermissions = {}
       }
@@ -63,24 +51,15 @@ export default {
         state.profile.avatar = value
       }
     },
-    token: (state, value) => {
-      state.token = value
-      if (value) {
-        state.tokenData = jwtDecode(value)
-      } else {
-        state.tokenData = {}
-      }
-    },
     permission: (state, {key, status}) => {
       state.availablePermissions[key] = status
     }
   },
   actions: {
-    // Send a request to the login URL and save the returned JWT
+    // Send a request to the login URL
     login ({commit, dispatch}, {next, credentials, onError}) {
-      return axios.post('token/', credentials).then(response => {
+      return axios.post('auth/login/', credentials).then(response => {
         logger.default.info('Successfully logged in as', credentials.username)
-        commit('token', response.data.token)
         dispatch('fetchProfile').then(() => {
           // Redirect to a specified route
           router.push(next)
@@ -102,29 +81,28 @@ export default {
       modules.forEach(m => {
         commit(`${m}/reset`, null, {root: true})
       })
-      logger.default.info('Log out, goodbye!')
+      return axios.post('auth/logout/').then(response => {
-      router.push({name: 'index'})
+        logger.default.info('Successfully logged out')
+      }, response => {
+        // we cannot contact the backend but we can at least clear our local cookies
+        logger.default.info('Backend unreachable, cleaning local cookies…')
+      }).finally(() => {
+        logger.default.info('Log out, goodbye!')
+        router.push({name: 'index'})
+      })
     },
     check ({commit, dispatch, state}) {
       logger.default.info('Checking authentication...')
-      var jwt = state.token
+      dispatch('fetchProfile').then(() => {
-      if (jwt) {
+        logger.default.info('Welcome back!')
-        commit('token', jwt)
+      }).catch(() => {
-        dispatch('fetchProfile')
-        dispatch('refreshToken')
-      } else {
         logger.default.info('Anonymous user')
         commit('authenticated', false)
-      }
+      })
     },
     fetchProfile ({commit, dispatch, state}) {
-      if (document) {
-        // this is to ensure we do not have any leaking cookie set by django
-        document.cookie = 'sessionid=; Path=/; Expires=Thu, 01 Jan 1970 00:00:01 GMT;'
-      }
-
       return new Promise((resolve, reject) => {
-        axios.get('users/users/me/').then((response) => {
+        axios.get('users/users/me/', {skipLoginRedirect: true}).then((response) => {
           logger.default.info('Successfully fetched user profile')
           dispatch('updateProfile', response.data).then(() => {
             resolve(response.data)
@@ -156,13 +134,5 @@ export default {
         resolve()
       })
     },
-    refreshToken ({commit, dispatch, state}) {
-      return axios.post('token/refresh/', {token: state.token}).then(response => {
-        logger.default.info('Refreshed auth token')
-        commit('token', response.data.token)
-      }, response => {
-        logger.default.error('Error while refreshing token', response.data)
-      })
-    }
   }
 }

front/src/store/instance.js

@@ -114,7 +114,6 @@ export default {
         commit(`${m}/reset`, null, {root: true})
       })
     },
-    // Send a request to the login URL and save the returned JWT
     fetchSettings ({commit}, payload) {
       return axios.get('instance/settings/').then(response => {
         logger.default.info('Successfully fetched instance settings')

front/src/utils/cookie.js

@@ -0,0 +1,7 @@
+export default {
+  get (name) {
+    var value = "; " + document.cookie;
+    var parts = value.split("; " + name + "=");
+    if (parts.length == 2) return parts.pop().split(";").shift();
+  }
+}

front/tests/unit/specs/store/auth.spec.js

@@ -37,54 +37,21 @@ describe('store/auth', () => {
     it('authenticated false', () => {
       const state = {
         username: 'dummy',
-        token: 'dummy',
-        tokenData: 'dummy',
         profile: 'dummy',
         availablePermissions: 'dummy'
       }
       store.mutations.authenticated(state, false)
       expect(state.authenticated).to.equal(false)
       expect(state.username).to.equal(null)
-      expect(state.token).to.equal(null)
-      expect(state.tokenData).to.equal(null)
       expect(state.profile).to.equal(null)
       expect(state.availablePermissions).to.deep.equal({})
     })
-    it('token null', () => {
-      const state = {}
-      store.mutations.token(state, null)
-      expect(state.token).to.equal(null)
-      expect(state.tokenData).to.deep.equal({})
-    })
-    it('token real', () => {
-      // generated on http://kjur.github.io/jsjws/tool_jwt.html
-      const state = {}
-      let token = 'eyJhbGciOiJub25lIiwidHlwIjoiSldUIn0.eyJpc3MiOiJodHRwczovL2p3dC1pZHAuZXhhbXBsZS5jb20iLCJzdWIiOiJtYWlsdG86bWlrZUBleGFtcGxlLmNvbSIsIm5iZiI6MTUxNTUzMzQyOSwiZXhwIjoxNTE1NTM3MDI5LCJpYXQiOjE1MTU1MzM0MjksImp0aSI6ImlkMTIzNDU2IiwidHlwIjoiaHR0cHM6Ly9leGFtcGxlLmNvbS9yZWdpc3RlciJ9.'
-      let tokenData = {
-        iss: 'https://jwt-idp.example.com',
-        sub: 'mailto:mike@example.com',
-        nbf: 1515533429,
-        exp: 1515537029,
-        iat: 1515533429,
-        jti: 'id123456',
-        typ: 'https://example.com/register'
-      }
-      store.mutations.token(state, token)
-      expect(state.token).to.equal(token)
-      expect(state.tokenData).to.deep.equal(tokenData)
-    })
     it('permissions', () => {
       const state = { availablePermissions: {} }
       store.mutations.permission(state, {key: 'admin', status: true})
       expect(state.availablePermissions).to.deep.equal({admin: true})
     })
   })
-  describe('getters', () => {
-    it('header', () => {
-      const state = { token: 'helloworld' }
-      expect(store.getters['header'](state)).to.equal('JWT helloworld')
-    })
-  })
   describe('actions', () => {
     it('logout', () => {
       testAction({
@@ -100,30 +67,8 @@ describe('store/auth', () => {
         ]
       })
     })
-    it('check jwt null', () => {
-      testAction({
-        action: store.actions.check,
-        params: {state: {}},
-        expectedMutations: [
-          { type: 'authenticated', payload: false }
-        ]
-      })
-    })
-    it('check jwt set', () => {
-      testAction({
-        action: store.actions.check,
-        params: {state: {token: 'test', username: 'user'}},
-        expectedMutations: [
-          { type: 'token', payload: 'test' }
-        ],
-        expectedActions: [
-          { type: 'fetchProfile' },
-          { type: 'refreshToken' }
-        ]
-      })
-    })
     it('login success', () => {
-      moxios.stubRequest('token/', {
+      moxios.stubRequest('auth/login/', {
         status: 200,
         response: {
           token: 'test'
@@ -135,9 +80,7 @@ describe('store/auth', () => {
       testAction({
         action: store.actions.login,
         payload: {credentials: credentials},
-        expectedMutations: [
+        expectedMutations: [],
-          { type: 'token', payload: 'test' }
-        ],
         expectedActions: [
           { type: 'fetchProfile' }
         ]
@@ -187,18 +130,5 @@ describe('store/auth', () => {
         ]
       })
     })
-    it('refreshToken', () => {
-      moxios.stubRequest('token/refresh/', {
-        status: 200,
-        response: {token: 'newtoken'}
-      })
-      testAction({
-        action: store.actions.refreshToken,
-        params: {state: {token: 'oldtoken'}},
-        expectedMutations: [
-          { type: 'token', payload: 'newtoken' }
-        ]
-      })
-    })
   })
 })