diff --git a/api/config/settings/common.py b/api/config/settings/common.py index 492d67ca1b68dc920454a3a0d582972fefe7c9c1..87848881d08ad5433ee4851ac78d326a0f979d44 100644 --- a/api/config/settings/common.py +++ b/api/config/settings/common.py @@ -609,6 +609,8 @@ OAUTH2_PROVIDER_ACCESS_TOKEN_MODEL = "users.AccessToken" OAUTH2_PROVIDER_GRANT_MODEL = "users.Grant" OAUTH2_PROVIDER_REFRESH_TOKEN_MODEL = "users.RefreshToken" +SCOPED_TOKENS_MAX_AGE = 60 * 60 * 24 * 3 + # LDAP AUTHENTICATION CONFIGURATION # ------------------------------------------------------------------------------ AUTH_LDAP_ENABLED = env.bool("LDAP_ENABLED", default=False) diff --git a/api/funkwhale_api/common/auth.py b/api/funkwhale_api/common/auth.py index 7717c836babb4940902593185416c6a6430949ed..736364337f73b4959cb1bf6fe9e25302044bce2c 100644 --- a/api/funkwhale_api/common/auth.py +++ b/api/funkwhale_api/common/auth.py @@ -29,6 +29,7 @@ class TokenAuthMiddleware: self.inner = inner def __call__(self, scope): + # XXX: 1.0 remove this, replace with websocket/scopedtoken auth = TokenHeaderAuth() try: user, token = auth.authenticate(scope) diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index 80cf286afd719028b406e032fe36289cad6e3229..93635c11b55be1b10e996ad62f281e43f41e6c2f 100644 --- a/api/funkwhale_api/music/views.py +++ b/api/funkwhale_api/music/views.py @@ -30,6 +30,7 @@ from funkwhale_api.federation import tasks as federation_tasks from funkwhale_api.tags.models import Tag, TaggedItem from funkwhale_api.tags.serializers import TagSerializer from funkwhale_api.users.oauth import permissions as oauth_permissions +from funkwhale_api.users.authentication import ScopedTokenAuthentication from . import filters, licenses, models, serializers, tasks, utils @@ -571,7 +572,7 @@ class ListenViewSet(mixins.RetrieveModelMixin, viewsets.GenericViewSet): serializer_class = serializers.TrackSerializer authentication_classes = ( rest_settings.api_settings.DEFAULT_AUTHENTICATION_CLASSES - + [SignatureAuthentication] + + [SignatureAuthentication, ScopedTokenAuthentication] ) permission_classes = [oauth_permissions.ScopePermission] required_scope = "libraries" diff --git a/api/funkwhale_api/users/authentication.py b/api/funkwhale_api/users/authentication.py new file mode 100644 index 0000000000000000000000000000000000000000..cb9ec8a8e09221d6a3e603adec7c3f74e53e8359 --- /dev/null +++ b/api/funkwhale_api/users/authentication.py @@ -0,0 +1,74 @@ +from django.conf import settings +from django.core import signing + +from rest_framework import authentication +from rest_framework import exceptions +from django.core.exceptions import ValidationError + +from .oauth import scopes as available_scopes + +from . import models + + +def generate_scoped_token(user_id, user_secret, scopes): + if set(scopes) & set(available_scopes.SCOPES_BY_ID) != set(scopes): + raise ValueError("{} contains invalid scopes".format(scopes)) + + return signing.dumps( + { + "user_id": user_id, + "user_secret": str(user_secret), + "scopes": list(sorted(scopes)), + }, + salt="scoped_tokens", + ) + + +def authenticate_scoped_token(token): + try: + payload = signing.loads( + token, salt="scoped_tokens", max_age=settings.SCOPED_TOKENS_MAX_AGE, + ) + except signing.BadSignature: + raise exceptions.AuthenticationFailed("Invalid token signature") + + try: + user_id = int(payload["user_id"]) + user_secret = str(payload["user_secret"]) + scopes = list(payload["scopes"]) + except (KeyError, ValueError, TypeError): + raise exceptions.AuthenticationFailed("Invalid scoped token payload") + + try: + user = ( + models.User.objects.all() + .for_auth() + .get(pk=user_id, secret_key=user_secret, is_active=True) + ) + except (models.User.DoesNotExist, ValidationError): + raise exceptions.AuthenticationFailed("Invalid user") + + return user, scopes + + +class ScopedTokenAuthentication(authentication.BaseAuthentication): + """ + Used when signed token returned by generate_scoped_token are provided via + token= in GET requests. Mostly for <audio src=""> urls, since it's not possible + to override headers sent by the browser when loading media. + """ + + def authenticate(self, request): + data = request.GET + token = data.get("token") + if not token: + return None + + try: + user, scopes = authenticate_scoped_token(token) + except exceptions.AuthenticationFailed: + raise exceptions.AuthenticationFailed("Invalid token") + + setattr(request, "scopes", scopes) + setattr(request, "actor", user.actor) + return user, None diff --git a/api/funkwhale_api/users/oauth/permissions.py b/api/funkwhale_api/users/oauth/permissions.py index 54b3c2627bb28a1263b537e5ebda51ca5c7933b3..63f33926819342a6d4cb4470f95ec1bcf621eb91 100644 --- a/api/funkwhale_api/users/oauth/permissions.py +++ b/api/funkwhale_api/users/oauth/permissions.py @@ -77,6 +77,10 @@ class ScopePermission(permissions.BasePermission): if isinstance(token, models.AccessToken): return self.has_permission_token(token, required_scope) + elif getattr(request, "scopes", None): + return should_allow( + required_scope=required_scope, request_scopes=set(request.scopes) + ) elif request.user.is_authenticated: user_scopes = scopes.get_from_permissions(**request.user.get_permissions()) return should_allow( diff --git a/api/funkwhale_api/users/serializers.py b/api/funkwhale_api/users/serializers.py index e0d9517557f9ecbd099efc72a711cc6549440dd6..542f6e58a42f12cc49b5379e297af302911abf2d 100644 --- a/api/funkwhale_api/users/serializers.py +++ b/api/funkwhale_api/users/serializers.py @@ -22,6 +22,7 @@ from funkwhale_api.moderation import utils as moderation_utils from . import adapters from . import models +from . import authentication as users_authentication @deconstructible @@ -220,6 +221,7 @@ class UserReadSerializer(serializers.ModelSerializer): class MeSerializer(UserReadSerializer): quota_status = serializers.SerializerMethodField() summary = serializers.SerializerMethodField() + tokens = serializers.SerializerMethodField() class Meta(UserReadSerializer.Meta): fields = UserReadSerializer.Meta.fields + [ @@ -227,6 +229,7 @@ class MeSerializer(UserReadSerializer): "instance_support_message_display_date", "funkwhale_support_message_display_date", "summary", + "tokens", ] def get_quota_status(self, o): @@ -237,6 +240,13 @@ class MeSerializer(UserReadSerializer): return return common_serializers.ContentSerializer(o.actor.summary_obj).data + def get_tokens(self, o): + return { + "listen": users_authentication.generate_scoped_token( + user_id=o.pk, user_secret=o.secret_key, scopes=["read:libraries"] + ) + } + class PasswordResetSerializer(PRS): def get_email_options(self): diff --git a/api/tests/music/test_views.py b/api/tests/music/test_views.py index 3d79c0eba1d2a8e25a15d3052af297551f080926..2a362a536c447081bd05ab0b91105c611556b9ff 100644 --- a/api/tests/music/test_views.py +++ b/api/tests/music/test_views.py @@ -15,6 +15,7 @@ from funkwhale_api.federation import api_serializers as federation_api_serialize from funkwhale_api.federation import utils as federation_utils from funkwhale_api.federation import tasks as federation_tasks from funkwhale_api.music import licenses, models, serializers, tasks, views +from funkwhale_api.users import authentication as users_authentication DATA_DIR = os.path.dirname(os.path.abspath(__file__)) @@ -1488,3 +1489,15 @@ def test_other_user_cannot_delete_track(factories, logged_in_api_client): assert response.status_code == 404 track.refresh_from_db() + + +def test_listen_to_track_with_scoped_token(factories, api_client): + user = factories["users.User"]() + token = users_authentication.generate_scoped_token( + user_id=user.pk, user_secret=user.secret_key, scopes=["read:libraries"] + ) + upload = factories["music.Upload"](playable=True) + url = reverse("api:v1:listen-detail", kwargs={"uuid": upload.track.uuid}) + response = api_client.get(url, {"token": token}) + + assert response.status_code == 200 diff --git a/api/tests/users/oauth/test_permissions.py b/api/tests/users/oauth/test_permissions.py index a5cd12034fd2aa4e07b0e5bfa24405d68b2baf7e..c336e88ceaf6d6e2c5f57ecec7a1de7b846f9446 100644 --- a/api/tests/users/oauth/test_permissions.py +++ b/api/tests/users/oauth/test_permissions.py @@ -62,7 +62,7 @@ def test_scope_permission_anonymous_policy( view = mocker.Mock( required_scope="libraries", anonymous_policy=policy, anonymous_scopes=set() ) - request = mocker.Mock(method="GET", user=anonymous_user, actor=None) + request = mocker.Mock(method="GET", user=anonymous_user, actor=None, scopes=None) p = permissions.ScopePermission() @@ -76,7 +76,7 @@ def test_scope_permission_dict_no_required(mocker, anonymous_user): action="read", anonymous_scopes=set(), ) - request = mocker.Mock(method="GET", user=anonymous_user, actor=None) + request = mocker.Mock(method="GET", user=anonymous_user, actor=None, scopes=None) p = permissions.ScopePermission() @@ -97,7 +97,7 @@ def test_scope_permission_user( ): user = factories["users.User"]() should_allow = mocker.patch.object(permissions, "should_allow") - request = mocker.Mock(method=method, user=user, actor=None) + request = mocker.Mock(method=method, user=user, actor=None, scopes=None) view = mocker.Mock( required_scope=required_scope, anonymous_policy=False, action=action ) @@ -131,10 +131,27 @@ def test_scope_permission_token(mocker, factories): ) +def test_scope_permission_request_scopes(mocker, factories): + should_allow = mocker.patch.object(permissions, "should_allow") + request = mocker.Mock(method="POST", scopes=["write:profile", "read:playlists"]) + view = mocker.Mock(required_scope="profile", anonymous_policy=False) + p = permissions.ScopePermission() + + assert p.has_permission(request, view) == should_allow.return_value + + should_allow.assert_called_once_with( + required_scope="write:profile", + request_scopes={"write:profile", "read:playlists"}, + ) + + def test_scope_permission_actor(mocker, factories, anonymous_user): should_allow = mocker.patch.object(permissions, "should_allow") request = mocker.Mock( - method="POST", actor=factories["federation.Actor"](), user=anonymous_user + method="POST", + actor=factories["federation.Actor"](), + user=anonymous_user, + scopes=None, ) view = mocker.Mock(required_scope="profile", anonymous_policy=False) p = permissions.ScopePermission() @@ -151,7 +168,7 @@ def test_scope_permission_token_anonymous_user_auth_required( ): preferences["common__api_authentication_required"] = True should_allow = mocker.patch.object(permissions, "should_allow") - request = mocker.Mock(method="POST", user=anonymous_user, actor=None) + request = mocker.Mock(method="POST", user=anonymous_user, actor=None, scopes=None) view = mocker.Mock(required_scope="profile", anonymous_policy=False) p = permissions.ScopePermission() @@ -166,7 +183,7 @@ def test_scope_permission_token_anonymous_user_auth_not_required( ): preferences["common__api_authentication_required"] = False should_allow = mocker.patch.object(permissions, "should_allow") - request = mocker.Mock(method="POST", user=anonymous_user, actor=None) + request = mocker.Mock(method="POST", user=anonymous_user, actor=None, scopes=None) view = mocker.Mock( required_scope="profile", anonymous_policy="setting", anonymous_scopes=set() ) diff --git a/api/tests/users/test_authentication.py b/api/tests/users/test_authentication.py new file mode 100644 index 0000000000000000000000000000000000000000..3de92bd188047d604936cde95157ecf4a92c60b0 --- /dev/null +++ b/api/tests/users/test_authentication.py @@ -0,0 +1,83 @@ +import pytest + +from django.core import signing + +from funkwhale_api.users import authentication + + +def test_generate_scoped_token(mocker): + dumps = mocker.patch("django.core.signing.dumps") + + result = authentication.generate_scoped_token( + user_id=42, user_secret="hello", scopes=["read"], + ) + + assert result == dumps.return_value + dumps.assert_called_once_with( + {"scopes": ["read"], "user_secret": "hello", "user_id": 42}, + salt="scoped_tokens", + ) + + +def test_authenticate_scoped_token(mocker, factories, settings): + loads = mocker.spy(signing, "loads") + user = factories["users.User"]() + token = signing.dumps( + {"user_id": user.pk, "user_secret": str(user.secret_key), "scopes": ["read"]}, + salt="scoped_tokens", + ) + + logged_user, scopes = authentication.authenticate_scoped_token(token) + + assert scopes == ["read"] + assert logged_user == user + loads.assert_called_once_with( + token, salt="scoped_tokens", max_age=settings.SCOPED_TOKENS_MAX_AGE + ) + + +def test_authenticate_scoped_token_bad_signature(): + with pytest.raises(authentication.exceptions.AuthenticationFailed): + authentication.authenticate_scoped_token("hello") + + +def test_authenticate_scoped_token_bad_secret_key(factories): + user = factories["users.User"]() + token = authentication.generate_scoped_token( + user_id=user.pk, user_secret="invalid", scopes=["read"] + ) + + with pytest.raises(authentication.exceptions.AuthenticationFailed): + authentication.authenticate_scoped_token(token) + + +def test_scope_token_authentication(fake_request, factories, mocker): + user = factories["users.User"]() + actor = user.create_actor() + authenticate_scoped_token = mocker.spy(authentication, "authenticate_scoped_token") + token = authentication.generate_scoped_token( + user_id=user.pk, user_secret=user.secret_key, scopes=["read"] + ) + request = fake_request.get("/", {"token": token}) + auth = authentication.ScopedTokenAuthentication() + + assert auth.authenticate(request) == (user, None) + assert request.scopes == ["read"] + assert request.actor == actor + authenticate_scoped_token.assert_called_once_with(token) + + +def test_scope_token_invalid(fake_request, factories): + token = "test" + request = fake_request.get("/", {"token": token}) + auth = authentication.ScopedTokenAuthentication() + + with pytest.raises(authentication.exceptions.AuthenticationFailed): + auth.authenticate(request) + + +def test_scope_token_missing(fake_request, factories): + request = fake_request.get("/") + auth = authentication.ScopedTokenAuthentication() + + assert auth.authenticate(request) is None diff --git a/api/tests/users/test_serializers.py b/api/tests/users/test_serializers.py index 02d1ac052fe5b027e755eaaacd5bdc4b232e051a..3966d394421dffa6ec59ad9ac207cc2658047656 100644 --- a/api/tests/users/test_serializers.py +++ b/api/tests/users/test_serializers.py @@ -42,3 +42,18 @@ def test_registration_serializer_validates_password_properly(data, expected_erro with pytest.raises(serializers.serializers.ValidationError, match=expected_error): serializer.is_valid(raise_exception=True) + + +def test_me_serializer_includes_tokens(factories, mocker): + user = factories["users.User"]() + generate_scoped_token = mocker.patch( + "funkwhale_api.users.authentication.generate_scoped_token" + ) + expected = {"listen": generate_scoped_token.return_value} + serializer = serializers.MeSerializer(user) + + assert serializer.data["tokens"] == expected + + generate_scoped_token.assert_called_once_with( + user_id=user.pk, user_secret=user.secret_key, scopes=["read:libraries"] + ) diff --git a/front/package.json b/front/package.json index eeed119fd13b04e30b39920f0195ab461d7b5394..f64e619a9caa1e8386dce3abbf6ae831802c954f 100644 --- a/front/package.json +++ b/front/package.json @@ -20,7 +20,6 @@ "fomantic-ui-css": "^2.8.3", "howler": "^2.0.14", "js-logger": "^1.4.1", - "jwt-decode": "^2.2.0", "lodash": "^4.17.10", "moment": "^2.22.2", "qs": "^6.7.0", diff --git a/front/src/App.vue b/front/src/App.vue index 0cfbb5bdccda410057088687fd7c4af3c6263471..0dec8e78374649106dcfa1983c3f02f5a63624c2 100644 --- a/front/src/App.vue +++ b/front/src/App.vue @@ -114,6 +114,10 @@ export default { } await this.fetchNodeInfo() this.$store.dispatch('auth/check') + setInterval(() => { + // used to refresh profile every now and then (important for refreshing scoped tokens) + self.$store.dispatch('auth/check') + }, 1000 * 60 * 60 * 8) this.$store.dispatch('instance/fetchSettings') this.$store.commit('ui/addWebsocketEventHandler', { eventName: 'inbox.item_added', diff --git a/front/src/components/audio/Player.vue b/front/src/components/audio/Player.vue index 91c1ecc5d2e50c91b49ab854ea5596215ea00589..b072b7819008c3576ae8b91296640f593302aa47 100644 --- a/front/src/components/audio/Player.vue +++ b/front/src/components/audio/Player.vue @@ -428,8 +428,17 @@ export default { // so authentication can be checked by the backend // because for audio files we cannot use the regular Authentication // header + let param = "jwt" + let value = this.$store.state.auth.token + if (this.$store.state.auth.scopedTokens && this.$store.state.auth.scopedTokens.listen) { + // used scoped tokens instead of JWT to reduce the attack surface if the token + // is leaked + param = "token" + value = this.$store.state.auth.scopedTokens.listen + } + console.log('HELLO', param, value, this.$store.state.auth.scopedTokens) sources.forEach(e => { - e.url = url.updateQueryString(e.url, 'jwt', this.$store.state.auth.token) + e.url = url.updateQueryString(e.url, param, value) }) } return sources diff --git a/front/src/components/library/TrackBase.vue b/front/src/components/library/TrackBase.vue index 92c85ee210e6efdef5346258f3dd42595b3f317a..8af866daf092aa06f7051565fcddacf631c4189d 100644 --- a/front/src/components/library/TrackBase.vue +++ b/front/src/components/library/TrackBase.vue @@ -229,10 +229,18 @@ export default { this.upload.listen_url ) if (this.$store.state.auth.authenticated) { + let param = "jwt" + let value = this.$store.state.auth.token + if (this.$store.state.auth.scopedTokens && this.$store.state.auth.scopedTokens.listen) { + // used scoped tokens instead of JWT to reduce the attack surface if the token + // is leaked + param = "token" + value = this.$store.state.auth.scopedTokens.listen + } u = url.updateQueryString( u, - "jwt", - encodeURI(this.$store.state.auth.token) + param, + encodeURI(value) ) } return u diff --git a/front/src/store/auth.js b/front/src/store/auth.js index 9163d2482c1ea80e1f2979f80dc25b2b8df908f7..700288d1e2421678f1f34785c2caa7c61f9f6cd5 100644 --- a/front/src/store/auth.js +++ b/front/src/store/auth.js @@ -1,10 +1,14 @@ import Vue from 'vue' import axios from 'axios' -import jwtDecode from 'jwt-decode' import logger from '@/logging' import router from '@/router' import lodash from '@/lodash' +function getDefaultScopedTokens () { + return { + listen: null, + } +} export default { namespaced: true, state: { @@ -18,7 +22,7 @@ export default { }, profile: null, token: '', - tokenData: {} + scopedTokens: getDefaultScopedTokens() }, getters: { header: state => { @@ -34,7 +38,7 @@ export default { state.username = '' state.fullUsername = '' state.token = '' - state.tokenData = {} + state.scopedTokens = getDefaultScopedTokens() state.availablePermissions = { federation: false, settings: false, @@ -51,8 +55,8 @@ export default { state.username = null state.fullUsername = null state.token = null - state.tokenData = null state.profile = null + state.scopedTokens = getDefaultScopedTokens() state.availablePermissions = {} } }, @@ -69,11 +73,9 @@ export default { }, token: (state, value) => { state.token = value - if (value) { - state.tokenData = jwtDecode(value) - } else { - state.tokenData = {} - } + }, + scopedTokens: (state, value) => { + state.scopedTokens = {...value} }, permission: (state, {key, status}) => { state.availablePermissions[key] = status @@ -159,6 +161,9 @@ export default { commit("profile", data) commit("username", data.username) commit("fullUsername", data.full_username) + if (data.tokens) { + commit("scopedTokens", data.tokens) + } Object.keys(data.permissions).forEach(function(key) { // this makes it easier to check for permissions in templates commit("permission", { diff --git a/front/tests/unit/specs/store/auth.spec.js b/front/tests/unit/specs/store/auth.spec.js index 88e2b63fd0294f06cc14c45571a8a49ef7ac1e2b..625c55edc6cc032430599d4af1b1b7c0c4d7e3d7 100644 --- a/front/tests/unit/specs/store/auth.spec.js +++ b/front/tests/unit/specs/store/auth.spec.js @@ -38,7 +38,6 @@ describe('store/auth', () => { const state = { username: 'dummy', token: 'dummy', - tokenData: 'dummy', profile: 'dummy', availablePermissions: 'dummy' } @@ -46,7 +45,6 @@ describe('store/auth', () => { 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({}) }) @@ -54,24 +52,12 @@ describe('store/auth', () => { 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: {} } diff --git a/front/yarn.lock b/front/yarn.lock index cb427c356cf324cb22b59aabd4093753acd4ee60..fa9eb362be3237012a8193848ece703d4df04d6c 100644 --- a/front/yarn.lock +++ b/front/yarn.lock @@ -5839,11 +5839,6 @@ just-extend@^4.0.2: resolved "https://registry.yarnpkg.com/just-extend/-/just-extend-4.0.2.tgz#f3f47f7dfca0f989c55410a7ebc8854b07108afc" integrity sha512-FrLwOgm+iXrPV+5zDU6Jqu4gCRXbWEQg2O3SKONsWE4w7AXFRkryS53bpWdaL9cNol+AmR3AEYz6kn+o0fCPnw== -jwt-decode@^2.2.0: - version "2.2.0" - resolved "https://registry.yarnpkg.com/jwt-decode/-/jwt-decode-2.2.0.tgz#7d86bd56679f58ce6a84704a657dd392bba81a79" - integrity sha1-fYa9VmefWM5qhHBKZX3TkruoGnk= - killable@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/killable/-/killable-1.0.1.tgz#4c8ce441187a061c7474fb87ca08e2a638194892"