diff --git a/supysonic/api/__init__.py b/supysonic/api/__init__.py index aa75563..8b78ac4 100644 --- a/supysonic/api/__init__.py +++ b/supysonic/api/__init__.py @@ -60,7 +60,6 @@ def authorize(): if request.authorization: status, user = UserManager.try_auth(request.authorization.username, request.authorization.password) if status == UserManager.SUCCESS: - request.username = request.authorization.username request.user = user return raise Unauthorized() @@ -73,7 +72,6 @@ def authorize(): if status != UserManager.SUCCESS: raise Unauthorized() - request.username = username request.user = user @api.before_request diff --git a/supysonic/api/albums_songs.py b/supysonic/api/albums_songs.py index da4c6c4..9775698 100644 --- a/supysonic/api/albums_songs.py +++ b/supysonic/api/albums_songs.py @@ -28,7 +28,9 @@ from pony.orm import select, desc, avg, max, min, count from ..db import Folder, Artist, Album, Track, RatingFolder, StarredFolder, StarredArtist, StarredAlbum, StarredTrack, User from ..db import now from ..py23 import dict + from . import api +from .exceptions import GenericError, NotFound @api.route('/getRandomSongs.view', methods = [ 'GET', 'POST' ]) def rand_songs(): @@ -49,7 +51,7 @@ def rand_songs(): query = query.filter(lambda t: t.genre == genre) if fid: if not Folder.exists(id = fid, root = True): - return request.formatter.error(70, 'Unknown folder') + raise NotFound('Folder') query = query.filter(lambda t: t.root_folder.id == fid) @@ -59,10 +61,9 @@ def rand_songs(): @api.route('/getAlbumList.view', methods = [ 'GET', 'POST' ]) def album_list(): - ltype, size, offset = map(request.values.get, [ 'type', 'size', 'offset' ]) - if not ltype: - return request.formatter.error(10, 'Missing type') + ltype = request.values['type'] + size, offset = map(request.values.get, [ 'size', 'offset' ]) size = int(size) if size else 10 offset = int(offset) if offset else 0 @@ -86,7 +87,7 @@ def album_list(): elif ltype == 'alphabeticalByArtist': query = query.order_by(lambda f: f.parent.name + f.name) else: - return request.formatter.error(0, 'Unknown search type') + raise GenericError('Unknown search type') return request.formatter('albumList', dict( album = [ f.as_subsonic_child(request.user) for f in query.limit(size, offset) ] @@ -94,10 +95,9 @@ def album_list(): @api.route('/getAlbumList2.view', methods = [ 'GET', 'POST' ]) def album_list_id3(): - ltype, size, offset = map(request.values.get, [ 'type', 'size', 'offset' ]) - if not ltype: - return request.formatter.error(10, 'Missing type') + ltype = request.values['type'] + size, offset = map(request.values.get, [ 'size', 'offset' ]) size = int(size) if size else 10 offset = int(offset) if offset else 0 @@ -119,7 +119,7 @@ def album_list_id3(): elif ltype == 'alphabeticalByArtist': query = query.order_by(lambda a: a.artist.name + a.name) else: - return request.formatter.error(0, 'Unknown search type') + raise GenericError('Unknown search type') return request.formatter('albumList2', dict( album = [ f.as_subsonic_album(request.user) for f in query.limit(size, offset) ] diff --git a/supysonic/api/annotation.py b/supysonic/api/annotation.py index c8a6923..d754f22 100644 --- a/supysonic/api/annotation.py +++ b/supysonic/api/annotation.py @@ -32,6 +32,7 @@ from ..lastfm import LastFm from ..py23 import dict from . import api, get_entity +from .exceptions import GenericError, MissingParameter, NotFound def try_star(cls, starred_cls, eid): """ Stars an entity @@ -91,7 +92,7 @@ def star(): id, albumId, artistId = map(request.values.getlist, [ 'id', 'albumId', 'artistId' ]) if not id and not albumId and not artistId: - return request.formatter.error(10, 'Missing parameter') + raise MissingParameter('id, albumId or artistId') errors = [] for eid in id: @@ -116,7 +117,7 @@ def unstar(): id, albumId, artistId = map(request.values.getlist, [ 'id', 'albumId', 'artistId' ]) if not id and not albumId and not artistId: - return request.formatter.error(10, 'Missing parameter') + raise MissingParameter('id, albumId or artistId') errors = [] for eid in id: @@ -138,18 +139,14 @@ def unstar(): @api.route('/setRating.view', methods = [ 'GET', 'POST' ]) def rate(): - id, rating = map(request.values.get, [ 'id', 'rating' ]) - if not id or not rating: - return request.formatter.error(10, 'Missing parameter') + id = request.values['id'] + rating = request.values['rating'] - try: - uid = uuid.UUID(id) - rating = int(rating) - except ValueError: - return request.formatter.error(0, 'Invalid parameter') + uid = uuid.UUID(id) + rating = int(rating) if not 0 <= rating <= 5: - return request.formatter.error(0, 'rating must be between 0 and 5 (inclusive)') + raise GenericError('rating must be between 0 and 5 (inclusive)') if rating == 0: delete(r for r in RatingTrack if r.user.id == request.user.id and r.rated.id == uid) @@ -163,7 +160,7 @@ def rate(): rated = Folder[uid] rating_cls = RatingFolder except ObjectNotFound: - return request.formatter.error(70, 'Unknown id') + raise NotFound('Track or Folder') try: rating_info = rating_cls[request.user, uid] @@ -177,14 +174,7 @@ def rate(): def scrobble(): res = get_entity(Track) t, submission = map(request.values.get, [ 'time', 'submission' ]) - - if t: - try: - t = int(t) / 1000 - except ValueError: - return request.formatter.error(0, 'Invalid time value') - else: - t = int(time.time()) + t = int(t) / 1000 if t else int(time.time()) lfm = LastFm(current_app.config['LASTFM'], request.user, current_app.logger) diff --git a/supysonic/api/browse.py b/supysonic/api/browse.py index f84a452..0102a8a 100644 --- a/supysonic/api/browse.py +++ b/supysonic/api/browse.py @@ -152,5 +152,5 @@ def track_info(): @api.route('/getVideos.view', methods = [ 'GET', 'POST' ]) def list_videos(): - return request.formatter.error(0, 'Video streaming not supported') + return request.formatter.error(0, 'Video streaming not supported'), 501 diff --git a/supysonic/api/chat.py b/supysonic/api/chat.py index 450d228..10df04b 100644 --- a/supysonic/api/chat.py +++ b/supysonic/api/chat.py @@ -37,10 +37,7 @@ def get_chat(): @api.route('/addChatMessage.view', methods = [ 'GET', 'POST' ]) def add_chat_message(): - msg = request.values.get('message') - if not msg: - return request.formatter.error(10, 'Missing message') - + msg = request.values['message'] ChatMessage(user = request.user, message = msg) return request.formatter.empty diff --git a/supysonic/api/errors.py b/supysonic/api/errors.py index 4348e68..e60236f 100644 --- a/supysonic/api/errors.py +++ b/supysonic/api/errors.py @@ -13,7 +13,7 @@ from pony.orm import ObjectNotFound from werkzeug.exceptions import BadRequestKeyError from . import api -from .exceptions import GenericError, MissingParameter, NotFound +from .exceptions import GenericError, MissingParameter, NotFound, ServerError @api.errorhandler(ValueError) def value_error(e): @@ -34,7 +34,7 @@ def not_found(e): def generic_error(e): # pragma: nocover rollback() if not current_app.testing: - return GenericError(e), 500 + return ServerError(e) else: raise e diff --git a/supysonic/api/exceptions.py b/supysonic/api/exceptions.py index 47e9800..27d85bd 100644 --- a/supysonic/api/exceptions.py +++ b/supysonic/api/exceptions.py @@ -31,6 +31,9 @@ class GenericError(SubsonicAPIError): super(GenericError, self).__init__(*args, **kwargs) self.message = message +class ServerError(GenericError): + code = 500 + class MissingParameter(SubsonicAPIError): api_code = 10 diff --git a/supysonic/api/media.py b/supysonic/api/media.py index 75501a2..48656ca 100644 --- a/supysonic/api/media.py +++ b/supysonic/api/media.py @@ -34,6 +34,7 @@ from ..db import Track, Album, Artist, Folder, User, ClientPrefs, now from ..py23 import dict from . import api, get_entity +from .exceptions import GenericError, MissingParameter, NotFound, ServerError def prepare_transcoding_cmdline(base_cmdline, input_file, input_format, output_format, output_bitrate): if not base_cmdline: @@ -84,7 +85,7 @@ def stream_media(): if not transcoder: message = 'No way to transcode from {} to {}'.format(src_suffix, dst_suffix) current_app.logger.info(message) - return request.formatter.error(0, message) + raise GenericError(message) transcoder, decoder, encoder = map(lambda x: prepare_transcoding_cmdline(x, res.path, src_suffix, dst_suffix, dst_bitrate), [ transcoder, decoder, encoder ]) try: @@ -95,7 +96,7 @@ def stream_media(): dec_proc = subprocess.Popen(decoder, stdout = subprocess.PIPE) proc = subprocess.Popen(encoder, stdin = dec_proc.stdout, stdout = subprocess.PIPE) except OSError: - return request.formatter.error(0, 'Error while running the transcoding process') + raise ServerError('Error while running the transcoding process') def transcode(): try: @@ -135,7 +136,7 @@ def download_media(): def cover_art(): res = get_entity(Folder) if not res.has_cover_art or not os.path.isfile(os.path.join(res.path, 'cover.jpg')): - return request.formatter.error(70, 'Cover art not found') + raise NotFound('Cover art') size = request.values.get('size') if size: @@ -160,11 +161,8 @@ def cover_art(): @api.route('/getLyrics.view', methods = [ 'GET', 'POST' ]) def lyrics(): - artist, title = map(request.values.get, [ 'artist', 'title' ]) - if not artist: - return request.formatter.error(10, 'Missing artist parameter') - if not title: - return request.formatter.error(10, 'Missing title parameter') + artist = request.values['artist'] + title = request.values['title'] query = Track.select(lambda t: title in t.title and artist in t.artist.name) for track in query: diff --git a/supysonic/api/playlists.py b/supysonic/api/playlists.py index 609b511..c2a42a1 100644 --- a/supysonic/api/playlists.py +++ b/supysonic/api/playlists.py @@ -26,6 +26,7 @@ from ..db import Playlist, User, Track from ..py23 import dict from . import api, get_entity +from .exceptions import Forbidden, MissingParameter, NotFound @api.route('/getPlaylists.view', methods = [ 'GET', 'POST' ]) def list_playlists(): @@ -34,11 +35,11 @@ def list_playlists(): username = request.values.get('username') if username: if not request.user.admin: - return request.formatter.error(50, 'Restricted to admins') + raise Forbidden() user = User.get(name = username) if user is None: - return request.formatter.error(70, 'No such user') + raise NotFound('User') query = Playlist.select(lambda p: p.user.name == username).order_by(Playlist.name) @@ -48,7 +49,7 @@ def list_playlists(): def show_playlist(): res = get_entity(Playlist) if res.user.id != request.user.id and not request.user.admin: - return request.formatter.error('50', 'Private playlist') + raise Forbidden() info = res.as_subsonic_playlist(request.user) info['entry'] = [ t.as_subsonic_child(request.user, request.client) for t in res.get_tracks() ] @@ -65,7 +66,7 @@ def create_playlist(): playlist = Playlist[playlist_id] if playlist.user.id != request.user.id and not request.user.admin: - return request.formatter.error(50, "You're not allowed to modify a playlist that isn't yours") + raise Forbidden() playlist.clear() if name: @@ -73,10 +74,10 @@ def create_playlist(): elif name: playlist = Playlist(user = request.user, name = name) else: - return request.formatter.error(10, 'Missing playlist id or name') + raise MissingParameter('playlistId or name') - songs = map(uuid.UUID, songs) for sid in songs: + sid = uuid.UUID(sid) track = Track[sid] playlist.add(track) @@ -86,7 +87,7 @@ def create_playlist(): def delete_playlist(): res = get_entity(Playlist) if res.user.id != request.user.id and not request.user.admin: - return request.formatter.error(50, "You're not allowed to delete a playlist that isn't yours") + raise Forbidden() res.delete() return request.formatter.empty @@ -95,7 +96,7 @@ def delete_playlist(): def update_playlist(): res = get_entity(Playlist, 'playlistId') if res.user.id != request.user.id and not request.user.admin: - return request.formatter.error(50, "You're not allowed to delete a playlist that isn't yours") + raise Forbidden() playlist = res name, comment, public = map(request.values.get, [ 'name', 'comment', 'public' ]) diff --git a/supysonic/api/search.py b/supysonic/api/search.py index 673292f..6a56d53 100644 --- a/supysonic/api/search.py +++ b/supysonic/api/search.py @@ -25,7 +25,9 @@ from pony.orm import select from ..db import Folder, Track, Artist, Album from ..py23 import dict + from . import api +from .exceptions import MissingParameter @api.route('/search.view', methods = [ 'GET', 'POST' ]) def old_search(): @@ -58,7 +60,7 @@ def old_search(): match = [ r.as_subsonic_child(request.user) if isinstance(r, Folder) else r.as_subsonic_child(request.user, request.client) for r in res ] )) else: - return request.formatter.error(10, 'Missing search parameter') + raise MissingParameter('search') return request.formatter('searchResult', dict( totalHits = query.count(), @@ -68,8 +70,9 @@ def old_search(): @api.route('/search2.view', methods = [ 'GET', 'POST' ]) def new_search(): - query, artist_count, artist_offset, album_count, album_offset, song_count, song_offset = map( - request.values.get, [ 'query', 'artistCount', 'artistOffset', 'albumCount', 'albumOffset', 'songCount', 'songOffset' ]) + query = request.values['query'] + artist_count, artist_offset, album_count, album_offset, song_count, song_offset = map( + request.values.get, [ 'artistCount', 'artistOffset', 'albumCount', 'albumOffset', 'songCount', 'songOffset' ]) artist_count = int(artist_count) if artist_count else 20 artist_offset = int(artist_offset) if artist_offset else 0 @@ -78,9 +81,6 @@ def new_search(): song_count = int(song_count) if song_count else 20 song_offset = int(song_offset) if song_offset else 0 - if not query: - return request.formatter.error(10, 'Missing query parameter') - artists = select(t.folder.parent for t in Track if query in t.folder.parent.name).limit(artist_count, artist_offset) albums = select(t.folder for t in Track if query in t.folder.name).limit(album_count, album_offset) songs = Track.select(lambda t: query in t.title).limit(song_count, song_offset) @@ -93,8 +93,9 @@ def new_search(): @api.route('/search3.view', methods = [ 'GET', 'POST' ]) def search_id3(): - query, artist_count, artist_offset, album_count, album_offset, song_count, song_offset = map( - request.values.get, [ 'query', 'artistCount', 'artistOffset', 'albumCount', 'albumOffset', 'songCount', 'songOffset' ]) + query = request.values['query'] + artist_count, artist_offset, album_count, album_offset, song_count, song_offset = map( + request.values.get, [ 'artistCount', 'artistOffset', 'albumCount', 'albumOffset', 'songCount', 'songOffset' ]) artist_count = int(artist_count) if artist_count else 20 artist_offset = int(artist_offset) if artist_offset else 0 @@ -103,9 +104,6 @@ def search_id3(): song_count = int(song_count) if song_count else 20 song_offset = int(song_offset) if song_offset else 0 - if not query: - return request.formatter.error(10, 'Missing query parameter') - artists = Artist.select(lambda a: query in a.name).limit(artist_count, artist_offset) albums = Album.select(lambda a: query in a.name).limit(album_count, album_offset) songs = Track.select(lambda t: query in t.title).limit(song_count, song_offset) diff --git a/supysonic/api/user.py b/supysonic/api/user.py index 7ec937c..6fff192 100644 --- a/supysonic/api/user.py +++ b/supysonic/api/user.py @@ -19,87 +19,86 @@ # along with this program. If not, see . from flask import request +from functools import wraps from ..db import User from ..managers.user import UserManager from ..py23 import dict from . import api, decode_password +from .exceptions import Forbidden, GenericError, NotFound + +def admin_only(f): + @wraps(f) + def decorated(*args, **kwargs): + if not request.user.admin: + raise Forbidden() + return f(*args, **kwargs) + return decorated @api.route('/getUser.view', methods = [ 'GET', 'POST' ]) def user_info(): - username = request.values.get('username') - if username is None: - return request.formatter.error(10, 'Missing username') + username = request.values['username'] - if username != request.username and not request.user.admin: - return request.formatter.error(50, 'Admin restricted') + if username != request.user.name and not request.user.admin: + raise Forbidden() user = User.get(name = username) if user is None: - return request.formatter.error(70, 'Unknown user') + raise NotFound('User') return request.formatter('user', user.as_subsonic_user()) @api.route('/getUsers.view', methods = [ 'GET', 'POST' ]) +@admin_only def users_info(): - if not request.user.admin: - return request.formatter.error(50, 'Admin restricted') - return request.formatter('users', dict(user = [ u.as_subsonic_user() for u in User.select() ] )) @api.route('/createUser.view', methods = [ 'GET', 'POST' ]) +@admin_only def user_add(): - if not request.user.admin: - return request.formatter.error(50, 'Admin restricted') - - username, password, email, admin = map(request.values.get, [ 'username', 'password', 'email', 'adminRole' ]) - if not username or not password or not email: - return request.formatter.error(10, 'Missing parameter') + username = request.values['username'] + password = request.values['password'] + email = request.values['email'] + admin = request.values.get('adminRole') admin = True if admin in (True, 'True', 'true', 1, '1') else False password = decode_password(password) status = UserManager.add(username, password, email, admin) if status == UserManager.NAME_EXISTS: - return request.formatter.error(0, 'There is already a user with that username') + raise GenericError('There is already a user with that username') return request.formatter.empty @api.route('/deleteUser.view', methods = [ 'GET', 'POST' ]) +@admin_only def user_del(): - if not request.user.admin: - return request.formatter.error(50, 'Admin restricted') - - username = request.values.get('username') - if not username: - return request.formatter.error(10, 'Missing parameter') + username = request.values['username'] user = User.get(name = username) if user is None: - return request.formatter.error(70, 'Unknown user') + raise NotFound('User') status = UserManager.delete(user.id) if status != UserManager.SUCCESS: - return request.formatter.error(0, UserManager.error_str(status)) + raise GenericError(UserManager.error_str(status)) return request.formatter.empty @api.route('/changePassword.view', methods = [ 'GET', 'POST' ]) def user_changepass(): - username, password = map(request.values.get, [ 'username', 'password' ]) - if not username or not password: - return request.formatter.error(10, 'Missing parameter') + username = request.values['username'] + password = request.values['password'] - if username != request.username and not request.user.admin: - return request.formatter.error(50, 'Admin restricted') + if username != request.user.name and not request.user.admin: + raise Forbidden() password = decode_password(password) status = UserManager.change_password2(username, password) - if status != UserManager.SUCCESS: - code = 0 - if status == UserManager.NO_SUCH_USER: - code = 70 - return request.formatter.error(code, UserManager.error_str(status)) + if status == UserManager.NO_SUCH_USER: + raise NotFound('User') + elif status != UserManager.SUCCESS: + raise GenericError(UserManager.error_str(status)) return request.formatter.empty diff --git a/tests/api/test_search.py b/tests/api/test_search.py index 2935136..6303c78 100644 --- a/tests/api/test_search.py +++ b/tests/api/test_search.py @@ -162,12 +162,12 @@ class SearchTestCase(ApiTestBase): def test_search2(self): # invalid parameters - self._make_request('search2', { 'artistCount': 'string' }, error = 0) - self._make_request('search2', { 'artistOffset': 'sstring' }, error = 0) - self._make_request('search2', { 'albumCount': 'string' }, error = 0) - self._make_request('search2', { 'albumOffset': 'sstring' }, error = 0) - self._make_request('search2', { 'songCount': 'string' }, error = 0) - self._make_request('search2', { 'songOffset': 'sstring' }, error = 0) + self._make_request('search2', { 'query': 'a', 'artistCount': 'string' }, error = 0) + self._make_request('search2', { 'query': 'a', 'artistOffset': 'sstring' }, error = 0) + self._make_request('search2', { 'query': 'a', 'albumCount': 'string' }, error = 0) + self._make_request('search2', { 'query': 'a', 'albumOffset': 'sstring' }, error = 0) + self._make_request('search2', { 'query': 'a', 'songCount': 'string' }, error = 0) + self._make_request('search2', { 'query': 'a', 'songOffset': 'sstring' }, error = 0) # no search self._make_request('search2', error = 10) @@ -250,12 +250,12 @@ class SearchTestCase(ApiTestBase): # to have folders that don't share names with artists or albums def test_search3(self): # invalid parameters - self._make_request('search3', { 'artistCount': 'string' }, error = 0) - self._make_request('search3', { 'artistOffset': 'sstring' }, error = 0) - self._make_request('search3', { 'albumCount': 'string' }, error = 0) - self._make_request('search3', { 'albumOffset': 'sstring' }, error = 0) - self._make_request('search3', { 'songCount': 'string' }, error = 0) - self._make_request('search3', { 'songOffset': 'sstring' }, error = 0) + self._make_request('search3', { 'query': 'a', 'artistCount': 'string' }, error = 0) + self._make_request('search3', { 'query': 'a', 'artistOffset': 'sstring' }, error = 0) + self._make_request('search3', { 'query': 'a', 'albumCount': 'string' }, error = 0) + self._make_request('search3', { 'query': 'a', 'albumOffset': 'sstring' }, error = 0) + self._make_request('search3', { 'query': 'a', 'songCount': 'string' }, error = 0) + self._make_request('search3', { 'query': 'a', 'songOffset': 'sstring' }, error = 0) # no search self._make_request('search3', error = 10)