diff --git a/api/config/api_urls.py b/api/config/api_urls.py index c839025e67e5807ff66b231c7dbfc96f2c6f689a..93138e9a5df1df3c8239c5d3740f1b2a5504a6ed 100644 --- a/api/config/api_urls.py +++ b/api/config/api_urls.py @@ -75,9 +75,6 @@ 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"), ] diff --git a/api/config/routing.py b/api/config/routing.py index db8a1d910f6f1ffd7fe72c8bbd2bcf72c4a43247..13a67cd1e4f2e8691713bc64b0db27248249da46 100644 --- a/api/config/routing.py +++ b/api/config/routing.py @@ -1,5 +1,4 @@ from channels.routing import ProtocolTypeRouter, URLRouter -from channels.sessions import SessionMiddlewareStack from django.conf.urls import url from funkwhale_api.common.auth import TokenAuthMiddleware @@ -8,12 +7,8 @@ from funkwhale_api.instance import consumers application = ProtocolTypeRouter( { # Empty for now (http->django views is added by default) - "websocket": SessionMiddlewareStack( - TokenAuthMiddleware( - URLRouter( - [url("^api/v1/activity$", consumers.InstanceActivityConsumer)] - ) - ) + "websocket": TokenAuthMiddleware( + URLRouter([url("^api/v1/activity$", consumers.InstanceActivityConsumer)]) ) } ) diff --git a/api/config/settings/common.py b/api/config/settings/common.py index 220ccbc36e096cdd69737beb5a2ec1f54d40e42d..5f69c36d55016c49efa0adffba1bd51afdeff2f1 100644 --- a/api/config/settings/common.py +++ b/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 = True +SESSION_COOKIE_HTTPONLY = False # 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 = False +CSRF_USE_SESSIONS = True SESSION_ENGINE = "django.contrib.sessions.backends.cache" # Playlist settings diff --git a/api/funkwhale_api/common/consumers.py b/api/funkwhale_api/common/consumers.py index 94930f63d028de038be797e3ab5a19f05df39eec..48c3186384f278be8e1e3480cd4011f73de2a10c 100644 --- a/api/funkwhale_api/common/consumers.py +++ b/api/funkwhale_api/common/consumers.py @@ -1,22 +1,17 @@ -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): - if "user" not in self.scope: - try: - self.scope["user"] = async_to_sync(auth.get_user)(self.scope) - except (ValueError, AssertionError, AttributeError, KeyError): - return self.close() - - if self.scope["user"] and self.scope["user"].is_authenticated: - return self.accept() - else: + try: + assert self.scope["user"].pk is not None + except (AssertionError, AttributeError, KeyError): return self.close() + return self.accept() + def accept(self): super().accept() for group in self.groups: diff --git a/api/funkwhale_api/users/auth_serializers.py b/api/funkwhale_api/users/auth_serializers.py deleted file mode 100644 index 451970904e13efa95119202a2c4ed16feee196db..0000000000000000000000000000000000000000 --- a/api/funkwhale_api/users/auth_serializers.py +++ /dev/null @@ -1,16 +0,0 @@ -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 diff --git a/api/funkwhale_api/users/auth_urls.py b/api/funkwhale_api/users/auth_urls.py deleted file mode 100644 index 385e58da0c76753a2ba253af8a513fd65efc510e..0000000000000000000000000000000000000000 --- a/api/funkwhale_api/users/auth_urls.py +++ /dev/null @@ -1,8 +0,0 @@ -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"), -] diff --git a/api/funkwhale_api/users/auth_views.py b/api/funkwhale_api/users/auth_views.py deleted file mode 100644 index 697b196cf18ff9d98ac5186b9c1df601262e744c..0000000000000000000000000000000000000000 --- a/api/funkwhale_api/users/auth_views.py +++ /dev/null @@ -1,32 +0,0 @@ -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) diff --git a/api/tests/users/test_auth_views.py b/api/tests/users/test_auth_views.py deleted file mode 100644 index 0b375ed31a5afbf13a85f08f21e58ee9b67b0430..0000000000000000000000000000000000000000 --- a/api/tests/users/test_auth_views.py +++ /dev/null @@ -1,86 +0,0 @@ -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 diff --git a/changes/changelog.d/629.bugfix b/changes/changelog.d/629.bugfix deleted file mode 100644 index d8e58c2b0453bb5dc6ad3c74a6cb5b8067227117..0000000000000000000000000000000000000000 --- a/changes/changelog.d/629.bugfix +++ /dev/null @@ -1 +0,0 @@ -Now use cookie-based auth in browser instead of JWT/LocalStorage in (#629) diff --git a/front/package.json b/front/package.json index 5f3826fdc63c1924b6f1f86d162c28633c962ead..22b5f3bb5ed35aebd5cb562c5f752ec2a65f8533 100644 --- a/front/package.json +++ b/front/package.json @@ -17,6 +17,7 @@ "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", diff --git a/front/src/App.vue b/front/src/App.vue index 46826d6e27d8c6c840806f77834525bcbc71937e..fd94a9f4605dc69d386d864f3663c5d928e8e4e8 100644 --- a/front/src/App.vue +++ b/front/src/App.vue @@ -175,9 +175,11 @@ 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`) + let url = this.$store.getters['instance/absoluteUrl'](`api/v1/activity?token=${token}`) url = url.replace('http://', 'ws://') url = url.replace('https://', 'wss://') bridge.connect( diff --git a/front/src/components/audio/Track.vue b/front/src/components/audio/Track.vue index c82b281673cc621bf55841e5856bc04d113a275f..2e0f8c421a0e780247224b93764b55fd476ae541 100644 --- a/front/src/components/audio/Track.vue +++ b/front/src/components/audio/Track.vue @@ -71,6 +71,15 @@ 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 () { diff --git a/front/src/components/library/TrackBase.vue b/front/src/components/library/TrackBase.vue index ece34f4dd0cf91ba8ae66b8351b200311728f8d5..34ce4a2954598cbcb0b83a8509bf26c55dc31ede 100644 --- a/front/src/components/library/TrackBase.vue +++ b/front/src/components/library/TrackBase.vue @@ -154,6 +154,13 @@ 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() { diff --git a/front/src/main.js b/front/src/main.js index a722e68958464baeb60d40c6820f09347264b3ed..9f058d8ecbef6491660647a67e815d0b5beb8f62 100644 --- a/front/src/main.js +++ b/front/src/main.js @@ -16,7 +16,6 @@ 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 @@ -71,9 +70,8 @@ Vue.directive('title', function (el, binding) { ) axios.interceptors.request.use(function (config) { // Do something before request is sent - let csrfToken = cookie.get('csrftoken') - if (csrfToken) { - config.headers['X-CSRFToken'] = csrfToken + if (store.state.auth.token) { + config.headers['Authorization'] = store.getters['auth/header'] } return config }, function (error) { @@ -87,14 +85,9 @@ 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) - if (router.currentRoute.name !== 'login') { - logger.default.warn('Received 401 response from API, redirecting to login form', router.currentRoute.fullPath) - 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') diff --git a/front/src/store/auth.js b/front/src/store/auth.js index aec40fe05725ad51a2d3faf830016a3069aab686..29c4c5d719da539922c36f71a3996b2101ed180b 100644 --- a/front/src/store/auth.js +++ b/front/src/store/auth.js @@ -1,4 +1,5 @@ import axios from 'axios' +import jwtDecode from 'jwt-decode' import logger from '@/logging' import router from '@/router' @@ -14,6 +15,13 @@ export default { moderation: false }, profile: null, + token: '', + tokenData: {} + }, + getters: { + header: state => { + return 'JWT ' + state.token + } }, mutations: { reset (state) { @@ -21,6 +29,8 @@ export default { state.profile = null state.username = '' state.fullUsername = '' + state.token = '' + state.tokenData = {} state.availablePermissions = { federation: false, settings: false, @@ -36,6 +46,8 @@ export default { if (value === false) { state.username = null state.fullUsername = null + state.token = null + state.tokenData = null state.profile = null state.availablePermissions = {} } @@ -51,15 +63,24 @@ 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 + // Send a request to the login URL and save the returned JWT login ({commit, dispatch}, {next, credentials, onError}) { - return axios.post('auth/login/', credentials).then(response => { + return axios.post('token/', 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) @@ -81,28 +102,29 @@ export default { modules.forEach(m => { commit(`${m}/reset`, null, {root: true}) }) - return axios.post('auth/logout/').then(response => { - 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'}) - }) + logger.default.info('Log out, goodbye!') + router.push({name: 'index'}) }, check ({commit, dispatch, state}) { logger.default.info('Checking authentication...') - dispatch('fetchProfile').then(() => { - logger.default.info('Welcome back!') - }).catch(() => { + var jwt = state.token + if (jwt) { + commit('token', jwt) + 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/', {skipLoginRedirect: true}).then((response) => { + axios.get('users/users/me/').then((response) => { logger.default.info('Successfully fetched user profile') dispatch('updateProfile', response.data).then(() => { resolve(response.data) @@ -134,5 +156,13 @@ 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) + }) + } } } diff --git a/front/src/store/instance.js b/front/src/store/instance.js index 1215af5e43f29dd9d1b5d6f339dee7c2382e93d4..6af36e8090e3ba276c7fabaf870ebd251f086a15 100644 --- a/front/src/store/instance.js +++ b/front/src/store/instance.js @@ -114,6 +114,7 @@ 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') diff --git a/front/src/utils/cookie.js b/front/src/utils/cookie.js deleted file mode 100644 index 2b8fd8c79994e474d6eec63de7ec6b760d1ee5a1..0000000000000000000000000000000000000000 --- a/front/src/utils/cookie.js +++ /dev/null @@ -1,7 +0,0 @@ -export default { - get (name) { - var value = "; " + document.cookie; - var parts = value.split("; " + name + "="); - if (parts.length == 2) return parts.pop().split(";").shift(); - } -} diff --git a/front/tests/unit/specs/store/auth.spec.js b/front/tests/unit/specs/store/auth.spec.js index 0bb98bb93e4af121c2b2f6c46605b0272db2d4f0..05581979fb122e9f4d789d653ef1baa56d827226 100644 --- a/front/tests/unit/specs/store/auth.spec.js +++ b/front/tests/unit/specs/store/auth.spec.js @@ -37,21 +37,54 @@ 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({ @@ -67,8 +100,30 @@ 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('auth/login/', { + moxios.stubRequest('token/', { status: 200, response: { token: 'test' @@ -80,7 +135,9 @@ describe('store/auth', () => { testAction({ action: store.actions.login, payload: {credentials: credentials}, - expectedMutations: [], + expectedMutations: [ + { type: 'token', payload: 'test' } + ], expectedActions: [ { type: 'fetchProfile' } ] @@ -130,5 +187,18 @@ 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' } + ] + }) + }) }) })