Introduce the new cs3-python-client library

CHANGELOG.md

@@ -1,5 +1,11 @@
 ## Changelog for the WOPI server
 
+### Future - v11.0.0
+- Adopted the cs3 python client (#161), and optimized the logic
+  that deals with extended attributes
+- Ported xrootd docker image to Alma9 (#164)
+- Forced preview mode for non-regular (non-primary) accounts
+
 ### Fri May 24 2024 - v10.5.0
 - Added timeout settings for GRPC and HTTP connections (#149)
 - Fixed handing of trailing slashes (#151)

README.md

@@ -27,6 +27,7 @@ Contributors (oldest contributions first):
 - Vasco Guita (@vascoguita)
 - Thomas Mueller (@deepdiver1975)
 - Andre Duffeck (@aduffeck)
+- Rasmus Welander (@rawe0)
 
 Initial revision: December 2016 <br/>
 First production version for CERNBox: September 2017 (presented at [oCCon17](https://occon17.owncloud.org) - [slides](https://www.slideshare.net/giuseppelopresti/collaborative-editing-and-more-in-cernbox))<br/>
@@ -60,11 +61,13 @@ To run the tests, either run `pytest` if available in your system, or execute th
 
 ### Test against a Reva CS3 endpoint:
 
+The CS3 storage interface uses the [cs3 python client](https://github.com/cs3org/cs3-python-client), please also refer to its test suite and documentation.
+
 1. Clone reva (https://github.com/cs3org/reva)
 2. Run Reva according to <https://reva.link/docs/tutorials/share-tutorial/> (ie up until step 4 in the instructions)
+3. For a production deployment, configure your `wopiserver.conf` following the example above, and make sure the `iopsecret` file contains the same secret as configured in the [Reva appprovider](https://developer.sciencemesh.io/docs/technical-documentation/iop/iop-optional-configs/collabora-wopi-server/wopiserver)
 4. Configure `test/wopiserver-test.conf` such that the wopiserver can talk to your Reva instance: use [this example](docker/etc/wopiserver.cs3.conf) for a skeleton configuration
 5. Run the tests: `WOPI_STORAGE=cs3 python3 test/test_storageiface.py`
-3. For a production deployment, configure your `wopiserver.conf` following the example above, and make sure the `iopsecret` file contains the same secret as configured in the [Reva appprovider](https://developer.sciencemesh.io/docs/technical-documentation/iop/iop-optional-configs/collabora-wopi-server/wopiserver)
 
 ### Test against an Eos endpoint:
 

requirements.txt

@@ -13,3 +13,4 @@ wheel>=0.38.0 # not directly required, pinned by Snyk to avoid a vulnerability
 setuptools>=70.0.0 # not directly required, pinned by Snyk to avoid a vulnerability
 werkzeug>=3.0.1 # not directly required, pinned by Snyk to avoid a vulnerability
 zipp>=3.19.1 # not directly required, pinned by Snyk to avoid a vulnerability
+cs3client>=1.1

src/core/commoniface.py

@@ -12,15 +12,16 @@
 from base64 import urlsafe_b64encode, urlsafe_b64decode
 from binascii import Error as B64Error
 
+# Standard Linux error messages as also defined in cs3client.exceptions
 
-# standard file missing message
+# file missing message
 ENOENT_MSG = 'No such file or directory'
 
-# standard error thrown when attempting to overwrite a file/xattr in O_EXCL mode
+# error thrown when attempting to overwrite a file/xattr in O_EXCL mode
 # or when a lock operation cannot be performed because of failed preconditions
-EXCL_ERROR = 'File/xattr exists but EXCL mode requested, lock mismatch or lock expired'
+EXCL_ERROR = 'Lock mismatch'
 
-# standard error thrown when attempting an operation without the required access rights
+# attempt an operation without the required access rights
 ACCESS_ERROR = 'Operation not permitted'
 
 # name of the xattr storing the Reva lock

src/core/localiface.py

@@ -101,21 +101,30 @@ def stat(_endpoint, filepath, _userid):
                  (statInfo.st_ino, _getfilepath(filepath), (tend - tstart) * 1000))
         if S_ISDIR(statInfo.st_mode):
             raise IOError('Is a directory')
+        try:
+            xattrs = {
+                k.strip('user.'): os.getxattr(_getfilepath(filepath), k).decode()
+                for k in os.listxattr(_getfilepath(filepath))
+            }
+        except OSError as e:
+            log.info('msg="Failed to invoke listxattr/getxattr" inode="%d" filepath="%s" exception="%s"' %
+                     statInfo.st_ino, _getfilepath(filepath), e)
+            xattrs = {}
         return {
             'inode': common.encodeinode('local', str(statInfo.st_ino)),
             'filepath': filepath,
             'ownerid': str(statInfo.st_uid) + ':' + str(statInfo.st_gid),
             'size': statInfo.st_size,
             'mtime': statInfo.st_mtime,
             'etag': str(statInfo.st_mtime),
+            'xattrs': xattrs,
         }
     except (FileNotFoundError, PermissionError) as e:
         raise IOError(e) from e
 
 
-def statx(endpoint, filepath, userid, versioninv=1):
-    '''Get extended stat info (inode, filepath, ownerid, size, mtime). Equivalent to stat in the case of local storage.
-    The versioninv flag is ignored as local storage always supports version-invariant inodes (cf. CERNBOX-1216).'''
+def statx(endpoint, filepath, userid):
+    '''Get extended stat info (inode, filepath, ownerid, size, mtime). Equivalent to stat in the case of local storage'''
     return stat(endpoint, filepath, userid)
 
 
@@ -147,8 +156,8 @@ def setxattr(endpoint, filepath, userid, key, value, lockmd):
         raise IOError(e) from e
 
 
-def getxattr(_endpoint, filepath, _userid, key):
-    '''Get the extended attribute <key> on behalf of the given userid. Do not raise exceptions'''
+def _getxattr(filepath, key):
+    '''Internal only: get the extended attribute <key>, do not raise exceptions'''
     try:
         return os.getxattr(_getfilepath(filepath), 'user.' + key).decode('UTF-8')
     except OSError as e:
@@ -184,7 +193,7 @@ def setlock(endpoint, filepath, userid, appname, value):
 
 def getlock(endpoint, filepath, _userid):
     '''Get the lock metadata as an xattr on behalf of the given userid'''
-    rawl = getxattr(endpoint, filepath, '0:0', common.LOCKKEY)
+    rawl = _getxattr(filepath, common.LOCKKEY)
     if rawl:
         lock = common.retrieverevalock(rawl)
         if lock['expiration']['seconds'] > time.time():
@@ -227,14 +236,13 @@ def readfile(_endpoint, filepath, _userid, _lockid):
             # the actual read is buffered and managed by the app server
             for chunk in iter(lambda: f.read(chunksize), b''):
                 yield chunk
-    except FileNotFoundError:
+    except FileNotFoundError as fnfe:
         # log this case as info to keep the logs cleaner
         log.info(f'msg="File not found on read" filepath="{filepath}"')
-        # as this is a generator, we yield the error string instead of the file's contents
-        yield IOError('No such file or directory')
+        raise IOError('No such file or directory') from fnfe
     except OSError as e:
         log.error(f'msg="Error opening the file for read" filepath="{filepath}" error="{e}"')
-        yield IOError(e)
+        raise IOError(e) from e
 
 
 def writefile(endpoint, filepath, userid, content, size, lockmd, islock=False):

src/core/wopi.py

@@ -15,7 +15,6 @@
 from urllib.parse import unquote_plus as url_unquote
 from urllib.parse import quote_plus as url_quote
 from urllib.parse import urlparse
-from more_itertools import peekable
 import flask
 import core.wopiutils as utils
 import core.commoniface as common
@@ -105,8 +104,7 @@ def checkFileInfo(fileid, acctok):
         fmd['SupportsRename'] = fmd['UserCanRename'] = enablerename and \
             acctok['viewmode'] in (utils.ViewMode.READ_WRITE, utils.ViewMode.PREVIEW)
         fmd['SupportsUserInfo'] = True
-        uinfo = st.getxattr(acctok['endpoint'], acctok['filename'], acctok['userid'],
-                            utils.USERINFOKEY + '.' + acctok['wopiuser'].split('!')[0])
+        uinfo = statInfo['xattrs'].get(utils.USERINFOKEY + '.' + acctok['wopiuser'].split('!')[0])
         if uinfo:
             fmd['UserInfo'] = uinfo
         if srv.config.get('general', 'earlyfeatures', fallback='False').upper() == 'TRUE':
@@ -142,17 +140,11 @@ def getFile(_fileid, acctok):
     try:
         # TODO for the time being we do not look if the file is locked. Once exclusive locks are implemented in Reva,
         # the lock must be fetched prior to the following call in order to access the file.
-        # get the file reader generator
-        f = peekable(st.readfile(acctok['endpoint'], acctok['filename'], acctok['userid'], None))
-        firstchunk = f.peek()
-        if isinstance(firstchunk, IOError):
-            log.error('msg="GetFile: download failed" endpoint="%s" filename="%s" token="%s" error="%s"' %
-                      (acctok['endpoint'], acctok['filename'], flask.request.args['access_token'][-20:], firstchunk))
-            return utils.createJsonResponse({'message': 'Failed to fetch file from storage'}, http.client.INTERNAL_SERVER_ERROR)
         # stat the file to get the current version
         statInfo = st.statx(acctok['endpoint'], acctok['filename'], acctok['userid'])
         # stream file from storage to client
-        resp = flask.Response(f, mimetype='application/octet-stream')
+        resp = flask.Response(st.readfile(acctok['endpoint'], acctok['filename'], acctok['userid'], None),
+                              mimetype='application/octet-stream')
         resp.status_code = http.client.OK
         resp.headers['Content-Disposition'] = f"attachment; filename*=UTF-8''{url_quote(os.path.basename(acctok['filename']))}"
         resp.headers['X-Frame-Options'] = 'sameorigin'
@@ -163,10 +155,10 @@ def getFile(_fileid, acctok):
         # File is empty, still return OK (strictly speaking, we should return 204 NO_CONTENT)
         return '', http.client.OK
     except IOError as e:
-        # File is readable but statx failed?
-        log.error('msg="GetFile: failed to stat after read, possible race" filename="%s" token="%s" error="%s"' %
-                  (acctok['filename'], flask.request.args['access_token'][-20:], e))
-        return utils.createJsonResponse({'message': 'Failed to access file'}, http.client.INTERNAL_SERVER_ERROR)
+        # File got deleted meanwhile, or some other remote I/O error
+        log.error('msg="GetFile: download failed" endpoint="%s" filename="%s" token="%s" error="%s"' %
+                  (acctok['endpoint'], acctok['filename'], flask.request.args['access_token'][-20:], e))
+        return utils.createJsonResponse({'message': 'Failed to access file from storage'}, http.client.INTERNAL_SERVER_ERROR)
 
 
 #
@@ -194,7 +186,7 @@ def setLock(fileid, reqheaders, acctok):
 
     if retrievedLock or op == 'REFRESH_LOCK':
         # useful for later checks
-        savetime = st.getxattr(acctok['endpoint'], fn, acctok['userid'], utils.LASTSAVETIMEKEY)
+        savetime = statInfo['xattrs'].get(utils.LASTSAVETIMEKEY)
         if savetime and (not savetime.isdigit() or int(savetime) < int(statInfo['mtime'])):
             # we had stale information, discard
             log.warning('msg="Detected external modification" filename="%s" savetime="%s" mtime="%s" token="%s"' %
@@ -230,8 +222,6 @@ def setLock(fileid, reqheaders, acctok):
                 try:
                     retrievedlolock = next(st.readfile(acctok['endpoint'], utils.getLibreOfficeLockName(fn),
                                                        acctok['userid'], None))
-                    if isinstance(retrievedlolock, IOError):
-                        raise retrievedlolock from e
                     retrievedlolock = retrievedlolock.decode()
                     # check that the lock is not stale
                     if datetime.strptime(retrievedlolock.split(',')[3], '%d.%m.%Y %H:%M').timestamp() + \
@@ -600,9 +590,9 @@ def putFile(fileid, acctok):
     try:
         if srv.config.get('general', 'detectexternalmodifications', fallback='True').upper() == 'TRUE':
             # check now the destination file against conflicts if required
-            savetime = st.getxattr(acctok['endpoint'], acctok['filename'], acctok['userid'], utils.LASTSAVETIMEKEY)
-            mtime = None
-            mtime = st.stat(acctok['endpoint'], acctok['filename'], acctok['userid'])['mtime']
+            statInfo = st.statx(acctok['endpoint'], acctok['filename'], acctok['userid'])
+            savetime = statInfo['xattrs'].get(utils.LASTSAVETIMEKEY)
+            mtime = statInfo['mtime']
             if not savetime or not savetime.isdigit() or int(savetime) < int(mtime):
                 # no xattr was there or we got our xattr but mtime is more recent: someone may have updated the file from
                 # a different source (e.g. FUSE or SMB mount), therefore force conflict and return failure to the application
@@ -618,7 +608,7 @@ def putFile(fileid, acctok):
         # Also, note we can't get a time resolution better than one second!
         # Anyhow, the EFSS should support versioning for such cases.
         utils.storeWopiFile(acctok, retrievedLock, utils.LASTSAVETIMEKEY)
-        statInfo = st.statx(acctok['endpoint'], acctok['filename'], acctok['userid'], versioninv=1)
+        statInfo = st.statx(acctok['endpoint'], acctok['filename'], acctok['userid'])
         log.info('msg="File stored successfully" action="edit" user="%s" filename="%s" version="%s" token="%s"' %
                  (acctok['userid'][-20:], acctok['filename'], statInfo['etag'], flask.request.args['access_token'][-20:]))
         resp = flask.Response()

src/core/wopiutils.py

@@ -285,11 +285,8 @@ def retrieveWopiLock(fileid, operation, lockforlog, acctok, overridefn=None):
             pass
         try:
             # then try to read a LibreOffice lock
-            lolockstat = st.statx(acctok['endpoint'], getLibreOfficeLockName(acctok['filename']), acctok['userid'], versioninv=0)
+            lolockstat = st.statx(acctok['endpoint'], getLibreOfficeLockName(acctok['filename']), acctok['userid'])
             lolock = next(st.readfile(acctok['endpoint'], getLibreOfficeLockName(acctok['filename']), acctok['userid'], None))
-            if isinstance(lolock, IOError):
-                # this might be an access error, optimistically move on
-                raise lolock
             lolock = lolock.decode()
             if 'WOPIServer' not in lolock:
                 lolockholder = lolock.split(',')[1] if ',' in lolock else lolockstat['ownerid']