From 955948b0225c1d7498f83e7da8c592f69d077beb Mon Sep 17 00:00:00 2001 From: mark beasley Date: Thu, 6 Jan 2022 12:54:37 -0800 Subject: [PATCH] Clarify and fix api access --- webrecorder/webrecorder/basecontroller.py | 20 +++++++++++++++ webrecorder/webrecorder/collscontroller.py | 27 +++++--------------- webrecorder/webrecorder/listscontroller.py | 6 +++-- webrecorder/webrecorder/models/collection.py | 3 +++ webrecorder/webrecorder/recscontroller.py | 16 +++++++++++- webrecorder/webrecorder/usercontroller.py | 23 +---------------- 6 files changed, 50 insertions(+), 45 deletions(-) diff --git a/webrecorder/webrecorder/basecontroller.py b/webrecorder/webrecorder/basecontroller.py index 3ecf556c4..9f143ecf4 100644 --- a/webrecorder/webrecorder/basecontroller.py +++ b/webrecorder/webrecorder/basecontroller.py @@ -118,6 +118,7 @@ def validate_csrf(self): self._raise_error(403, 'invalid_csrf_token') def get_user(self, api=True, redir_check=True, user=None): + """naively retrieve user""" if redir_check: self.redir_host() @@ -139,6 +140,25 @@ def get_user(self, api=True, redir_check=True, user=None): return user + def get_user_or_raise(self, username=None, status=403, msg='unauthorized'): + """retreive and verify user""" + # ensure correct host + if self.app_host and request.environ.get('HTTP_HOST') != self.app_host: + return self._raise_error(403, 'unauthorized') + + # if no username, check if anon + if not username: + if self.access.is_anon(): + self._raise_error(status, msg) + + user = self.get_user(user=username) + + # check permissions + if not self.access.is_logged_in_user(user) and not self.access.is_superuser(): + self._raise_error(status, msg) + + return user + def load_user_coll(self, api=True, redir_check=True, user=None, coll_name=None): if not isinstance(user, User): user = self.get_user(api=api, redir_check=redir_check, user=user) diff --git a/webrecorder/webrecorder/collscontroller.py b/webrecorder/webrecorder/collscontroller.py index ac4476f6a..d19df22e8 100644 --- a/webrecorder/webrecorder/collscontroller.py +++ b/webrecorder/webrecorder/collscontroller.py @@ -34,7 +34,7 @@ def init_routes(self): req=['title', 'public', 'public_index'], resp='collection') def create_collection(): - user = self.get_user(api=True, redir_check=False) + user = self.get_user_or_raise() data = request.json or {} @@ -57,9 +57,6 @@ def create_collection(): if not self.allow_external: self._raise_error(403, 'external_not_allowed') - #if not is_anon: - # self._raise_error(400, 'not_valid_for_external') - elif is_anon: if coll_name != 'temp': self._raise_error(400, 'invalid_temp_coll_name') @@ -136,7 +133,10 @@ def get_collection(coll_name): @self.api(query=['user'], resp='deleted') def delete_collection(coll_name): - user, collection = self.load_user_coll(coll_name=coll_name) + user = self.get_user_or_raise() + collection = user.get_collection_by_name(coll_name) + + self.access.assert_can_admin_coll(collection) errs = user.remove_collection(collection, delete=True) if errs.get('error'): @@ -223,6 +223,8 @@ def update_collection(coll_name): def get_page_bookmarks(coll_name): user, collection = self.load_user_coll(coll_name=coll_name) + self.access.assert_can_read_coll(collection) + rec = request.query.get('rec') if rec: recording = collection.get_recording(rec) @@ -412,21 +414,6 @@ def generate_derivs(coll_name): return {'queued': res} - # LEGACY ENDPOINTS (to remove) - # Collection view (all recordings) - @self.app.get(['//', '///']) - @self.jinja2_view('collection_info.html') - def coll_info(user, coll_name): - return self.get_collection_info_for_view(user, coll_name) - - @self.app.get(['///', '////']) - @self.jinja2_view('collection_info.html') - def coll_info(user, coll_name, rec_list): - #rec_list = [self.sanitize_title(title) for title in rec_list.split(',')] - return self.get_collection_info_for_view(user, coll_name) - - wr_api_spec.set_curr_tag(None) - def get_collection_info_for_view(self, user, coll_name): self.redir_host() diff --git a/webrecorder/webrecorder/listscontroller.py b/webrecorder/webrecorder/listscontroller.py index 46452ccd3..d678e60dc 100644 --- a/webrecorder/webrecorder/listscontroller.py +++ b/webrecorder/webrecorder/listscontroller.py @@ -103,6 +103,8 @@ def move_list_before(list_id): def reorder_lists(): user, collection = self.load_user_coll() + self.access.assert_can_write_coll(collection) + new_order = request.json.get('order', []) if collection.lists.reorder_objects(new_order): @@ -195,6 +197,8 @@ def delete_bookmark(bid): def reorder_bookmarks(list_id): user, collection, blist = self.load_user_coll_list(list_id) + self.access.assert_can_write_coll(collection) + new_order = request.json.get('order', []) if blist.reorder_bookmarks(new_order): @@ -222,5 +226,3 @@ def load_list(self, collection, list_id): self._raise_error(404, 'no_such_list') return blist - - diff --git a/webrecorder/webrecorder/models/collection.py b/webrecorder/webrecorder/models/collection.py index 45ffc5963..affd8b8eb 100644 --- a/webrecorder/webrecorder/models/collection.py +++ b/webrecorder/webrecorder/models/collection.py @@ -566,6 +566,9 @@ def serialize(self, include_recordings=True, check_slug=False, include_files=False): + # access check + self.access.assert_can_read_coll(self) + data = super(Collection, self).serialize(convert_date=convert_date) data['id'] = self.name diff --git a/webrecorder/webrecorder/recscontroller.py b/webrecorder/webrecorder/recscontroller.py index 7f4034108..35ecf04f2 100644 --- a/webrecorder/webrecorder/recscontroller.py +++ b/webrecorder/webrecorder/recscontroller.py @@ -17,6 +17,8 @@ def init_routes(self): def create_recording(): user, collection = self.load_user_coll() + self.access.assert_can_write_coll(collection) + data = request.json or {} title = data.get('title', '') @@ -34,6 +36,8 @@ def create_recording(): def get_recordings(): user, collection = self.load_user_coll() + self.access.assert_can_read_coll(collection) + recs = collection.get_recordings() return {'recordings': [rec.serialize() for rec in recs]} @@ -44,6 +48,8 @@ def get_recordings(): def get_recording(rec): user, collection, recording = self.load_recording(rec) + self.access.assert_can_read_coll(collection) + if recording: return {'recording': recording.serialize()} else: @@ -56,7 +62,7 @@ def get_recording(rec): def update_rec_desc(rec): user, collection, recording = self.load_recording(rec) - user.access.assert_can_write_coll(collection) + self.access.assert_can_write_coll(collection) data = request.json or {} @@ -73,6 +79,8 @@ def update_rec_desc(rec): def delete_recording(rec): user, collection, recording = self.load_recording(rec) + self.access.assert_can_write_coll(collection) + errs = collection.remove_recording(recording, delete=True) if errs.get('error'): return self._raise_error(400, errs['error']) @@ -140,6 +148,8 @@ def add_page(rec): def list_pages(rec): user, collection, recording = self.load_recording(rec) + self.access.assert_can_read_coll(collection) + pages = collection.list_rec_pages(recording) return {'pages': pages} @@ -149,6 +159,8 @@ def list_pages(rec): def get_num_pages(rec): user, collection, recording = self.load_recording(rec) + self.access.assert_can_read_coll(collection) + return {'count': recording.count_pages() } @self.app.delete('/api/v1/recording//pages') @@ -157,6 +169,8 @@ def get_num_pages(rec): def delete_page(rec): user, collection, recording = self.load_recording(rec) + self.access.assert_can_write_coll(collection) + url = request.json.get('url') ts = request.json.get('timestamp') diff --git a/webrecorder/webrecorder/usercontroller.py b/webrecorder/webrecorder/usercontroller.py index 898919458..bfe33b904 100644 --- a/webrecorder/webrecorder/usercontroller.py +++ b/webrecorder/webrecorder/usercontroller.py @@ -88,25 +88,6 @@ def ensure_login(self): return result - def get_user_or_raise(self, username=None, status=403, msg='unauthorized'): - # ensure correct host - if self.app_host and request.environ.get('HTTP_HOST') != self.app_host: - return self._raise_error(403, 'unauthorized') - - # if no username, check if logged in - if not username: - if self.access.is_anon(): - self._raise_error(status, msg) - return - - user = self.get_user(user=username) - - # check permissions - if not self.access.is_logged_in_user(user) and not self.access.is_superuser(): - self._raise_error(status, msg) - - return user - def init_routes(self): wr_api_spec.set_curr_tag('Auth') @@ -202,8 +183,6 @@ def login(): @self.app.post('/api/v1/auth/logout') def logout(): - self.get_user_or_raise() - self.user_manager.logout() data = {'success': 'logged_out'} @@ -307,7 +286,7 @@ def api_delete_user(username): @self.app.post('/api/v1/user/') def update_user(username): - user = self.get_user(user=username) + user = self.get_user_or_raise(user=username) data = request.json or {}