From b9f0c6aecdbcf341aa12827e41be82486f36aa2f Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Fri, 15 Mar 2019 15:52:30 +0100
Subject: [PATCH] Revert "Merge branch '629-cookie-auth' into 'develop'"
This reverts commit 8b47af8b810dd01e6cf64b474d79583aed7c544f, reversing
changes made to c0055b3b20d3e9493dfd4cc190b528014481983e.
---
api/config/api_urls.py | 3 -
api/config/routing.py | 9 +--
api/config/settings/common.py | 4 +-
api/funkwhale_api/common/consumers.py | 17 ++--
api/funkwhale_api/users/auth_serializers.py | 16 ----
api/funkwhale_api/users/auth_urls.py | 8 --
api/funkwhale_api/users/auth_views.py | 32 --------
api/tests/users/test_auth_views.py | 86 ---------------------
changes/changelog.d/629.bugfix | 1 -
front/package.json | 1 +
front/src/App.vue | 4 +-
front/src/components/audio/Track.vue | 9 +++
front/src/components/library/TrackBase.vue | 7 ++
front/src/main.js | 15 +---
front/src/store/auth.js | 62 +++++++++++----
front/src/store/instance.js | 1 +
front/src/utils/cookie.js | 7 --
front/tests/unit/specs/store/auth.spec.js | 74 +++++++++++++++++-
18 files changed, 153 insertions(+), 203 deletions(-)
delete mode 100644 api/funkwhale_api/users/auth_serializers.py
delete mode 100644 api/funkwhale_api/users/auth_urls.py
delete mode 100644 api/funkwhale_api/users/auth_views.py
delete mode 100644 api/tests/users/test_auth_views.py
delete mode 100644 changes/changelog.d/629.bugfix
delete mode 100644 front/src/utils/cookie.js
diff --git a/api/config/api_urls.py b/api/config/api_urls.py
index c839025e..93138e9a 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 db8a1d91..13a67cd1 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 220ccbc3..5f69c36d 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 94930f63..48c31863 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 45197090..00000000
--- 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 385e58da..00000000
--- 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 697b196c..00000000
--- 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 0b375ed3..00000000
--- 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 d8e58c2b..00000000
--- 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 5f3826fd..22b5f3bb 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 46826d6e..fd94a9f4 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 c82b2816..2e0f8c42 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 ece34f4d..34ce4a29 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 a722e689..9f058d8e 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 aec40fe0..29c4c5d7 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 1215af5e..6af36e80 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 2b8fd8c7..00000000
--- 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 0bb98bb9..05581979 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' }
+ ]
+ })
+ })
})
})
--
GitLab