From 6fc47ce73dd18b33ec0bef3397e538f20a4355d9 Mon Sep 17 00:00:00 2001 From: "Karl O. Pinc" Date: Tue, 16 Jul 2024 13:50:34 -0500 Subject: [PATCH] Improve double-upload detection Fix false positive after loss of session (CSRF failure, etc.) Detect after reload of form by pressing "enter" in the URL bar. Detect after navigating to another PGWUI menu item and back. --- src/pgwui_core/core.py | 112 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 99 insertions(+), 13 deletions(-) diff --git a/src/pgwui_core/core.py b/src/pgwui_core/core.py index b13dc59..2e6df07 100644 --- a/src/pgwui_core/core.py +++ b/src/pgwui_core/core.py @@ -292,6 +292,20 @@ class CredsLoadedForm(LoadedForm): ''' self.uh.session[key] = value + def session_del(self, key): + ''' + Deletes data from the session. + + Input: + key The key to delete + + Returns: + + Side effects: + Modifies session + ''' + self.uh.session.pop(key, None) + def read(self): ''' Read form data from the client @@ -302,8 +316,8 @@ class CredsLoadedForm(LoadedForm): # Read our form data - # Keep password and user in the session. All the other - # form varaibles must be re-posted. + # Keep password and user (and db, in AuthLoadedForm, below) in + # the session. All the other form variables must be re-posted. post = self.uh.request.POST session = self.uh.request.session @@ -349,12 +363,18 @@ class AuthLoadedForm(CredsLoadedForm): Attributes: uh The UploadHandler instance using the form - user The Usernamed used to login + user The Username used to login db The db to login to - _form Instantaiated html form object (WXForms) + db_changed + Boolean. Whether the prior request changed some db's content. + "Prior request" means the last time a logged-in session + was submitted; requests resulting in expired sessions are + ignored. + _form Instantiated html form object (WXForms) ''' db = attrs.field(default=None) + db_changed = attrs.field(default=False) def read(self): ''' @@ -363,9 +383,17 @@ class AuthLoadedForm(CredsLoadedForm): # Read parent's data super().read() + post = self.uh.request.POST + session = self.uh.request.session # Keep form variables handy - self['db'] = self._form.db.data + # The db is kept in the session partly for the user's convenience + # when switching between menu items, but mostly so that double-upload + # of the same file can be detected when the user reloads the form + # by pressing "enter" in the URL bar. Because otherwise the + # generated last_key does not have the right db value. + if not self.read_post_and_session(post, session, 'db'): + self['db'] = '' def write(self, result, errors): ''' @@ -409,11 +437,14 @@ class UploadFileForm(AuthLoadedForm): self['literal_col_headings'] = self._form.literal_col_headings.data # Other POST variables involving a file + post = self.uh.request.POST + session = self.uh.request.session + if not self.read_post_and_session(post, session, 'db_changed'): + self['db_changed'] = False self['filename'] = '' self['localfh'] = '' if self['action']: if self._form.datafile.data != '': - post = self.uh.request.POST if hasattr(post['datafile'], 'filename'): self['filename'] = post['datafile'].filename if hasattr(post['datafile'], 'file'): @@ -441,6 +472,8 @@ class UploadFileForm(AuthLoadedForm): literal_col_headings_checked = UNCHECKED response = super().write(result, errors) + # Although we read-in db_changed, we do not write it because + # it, like last_key, is computed. response['filename'] = self['filename'] response['trim_upload'] = trim_upload_checked response['csv_value'] = CSV_VALUE @@ -472,6 +505,9 @@ class UploadDoubleFileFormMixin(UploadFormBaseMixin): Methods: read() Load form from pyramid request object. ''' + # Keep the last_key in both the form and the session; in the + # session because that way double-upload detection works when the + # user presses "enter" in the URL bar. last_key = attrs.field(default=None) def read(self): @@ -479,15 +515,22 @@ class UploadDoubleFileFormMixin(UploadFormBaseMixin): Read form data from the client ''' super().read() - post = self.uh.request.POST - self['last_key'] = post.get('last_key', '') + session = self.uh.request.session + + if not self.read_post_and_session(post, session, 'last_key'): + self['last_key'] = '' def write_response(self, response): ''' Produces the dict pyramid will use to render the form. ''' - response['last_key'] = self['last_key'] + if self.uh.double_upload: + # Erase the last key from all state + response.pop('last_key', None) + self.session_del('last_key') + else: + response['last_key'] = self['last_key'] return super().write_response(response) @@ -1315,7 +1358,10 @@ class UploadHandler(SessionDBHandler): request A pyramid request instance uf An UploadForm instance data (optional) A DBData instance + double_upload Boolean. True when a double-upload is detected. ''' + double_upload = attrs.field(default=False) + def factory(self, ue): ''' Takes an UploadEngine instance @@ -1339,6 +1385,11 @@ class UploadHandler(SessionDBHandler): return errors + def logged_in(self, response): + '''Are we authenticated? + ''' + return response.get('havecreds', False) is True + def double_validator(self, errors): '''Utility function that can optionally be called by a val_input() function. It checks that the same file @@ -1349,7 +1400,8 @@ class UploadHandler(SessionDBHandler): List of errors. Appended to. ''' uf = self.uf - if self.make_double_key() == uf['last_key']: + if self.make_double_key() == uf['last_key'] and uf['db_changed']: + self.double_upload = True errors.append(core_ex.DuplicateUploadError( 'File just uploaded to this db', ('File named ({0}) just uploaded' @@ -1368,6 +1420,11 @@ class UploadHandler(SessionDBHandler): uf = self.uf return self.hash_sequence((uf['db'], uf['filename'])) + def write_db_changed(self, response, db_changed): + '''Save whether the db changed in both the form and the session.''' + response['db_changed'] = db_changed + self.uf.session_put('db_changed', db_changed) + def write_double_key(self, response): '''Utility function. Optionally called from within write() to save a key which is later tested for to determine if @@ -1390,7 +1447,29 @@ class UploadHandler(SessionDBHandler): Side effects: Modifies response. Adds 'last_key' entry used by form to store key. ''' - response['last_key'] = self.make_double_key() + if not self.double_upload: + uf = self.uf + if self.logged_in(response): + if uf['filename'] != '': + # When we have no filename this may be because the + # user pressed "enter" in the URL bar. In that case + # we don't want to change the last_key because we + # may want to get the old value out of the session. + # In any case, with no file name there's not a danger + # of having previously uploaded anything, so no reason + # to worry about double-uploading. + last_key = self.make_double_key() + response['last_key'] = last_key + uf.session_put('last_key', last_key) + else: + # New session. + # The old session exited -- timed out, or the CSRF failed, etc. + # Use the key from the previous session. + if 'last_key' in uf: + uf.session_put('last_key', uf['last_key']) + # Save whether the db changed from the previous session. + if 'db_changed' in uf: + self.write_db_changed(response, uf['db_changed']) def write(self, result, errors): ''' @@ -1406,14 +1485,21 @@ class UploadHandler(SessionDBHandler): errors A list of core_ex.UploadError exceptions. csrf_token Token for detecting CSRF. e_cnt Number of errors. + report_success Boolean. Whether to tell the user the db changed. db_changed Boolean. Whether the db was changed. ''' response = super().write(result, errors) if self.data is not None: response['lines'] = self.data.lineno response['e_cnt'] = len(errors) - response['db_changed'] = (not response['errors'] - and self.uf['action'] != '') + + report_success = not response['errors'] and self.uf['action'] != '' + response['report_success'] = report_success + if 'db_changed' not in response: + # We have _not_ used db_changed from before we unexpectedly + # logged out. Determine here if the db content has changed. + self.write_db_changed(response, report_success) + return response -- 2.34.1