Skip to content

Commit

Permalink
Redefined wopiuser variable
Browse files Browse the repository at this point in the history
Now wopiuser is enforced to have the format
`username!userid_as_returned_by_stat`, where the latter
can be `username@idp` for CS3 interfaces or `uid:gid` for xroot.
  • Loading branch information
glpatcern committed Feb 24, 2023
1 parent 6935763 commit 2acdfa1
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 25 deletions.
6 changes: 3 additions & 3 deletions src/core/cs3iface.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ def init(inconfig, inlog):
ctx['cs3gw'] = cs3gw_grpc.GatewayAPIStub(ch)


def getuseridfromcreds(token, _wopiuser):
def getuseridfromcreds(token, wopiuser):
'''Maps a Reva token and wopiuser to the credentials to be used to access the storage.
For the CS3 API case, this is just the token'''
return token
For the CS3 API case this is the token, and wopiuser is expected to be `username!userid_as_returned_by_stat`'''
return token, wopiuser.split('@')[0] + '!' + wopiuser


def _getcs3reference(endpoint, fileref):
Expand Down
2 changes: 1 addition & 1 deletion src/core/localiface.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def init(inconfig, inlog):
def getuseridfromcreds(_token, _wopiuser):
'''Maps a Reva token and wopiuser to the credentials to be used to access the storage.
For the localfs case, this is trivially hardcoded'''
return '0:0'
return '0:0', 'root!0:0'


def stat(_endpoint, filepath, _userid):
Expand Down
10 changes: 5 additions & 5 deletions src/core/wopi.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ def checkFileInfo(fileid, acctok):
fmd['BreadcrumbBrandName'] = srv.config.get('general', 'brandingname', fallback=None)
fmd['BreadcrumbBrandUrl'] = srv.config.get('general', 'brandingurl', fallback=None)
fmd['OwnerId'] = statInfo['ownerid']
fmd['UserId'] = acctok['wopiuser'] # typically same as OwnerId; different when accessing shared documents
fmd['UserId'] = acctok['wopiuser'].split('!')[-1] # typically same as OwnerId; different when accessing shared documents
fmd['Size'] = statInfo['size']
# note that in ownCloud 10 the version is generated as: `'V' + etag + checksum`
fmd['Version'] = 'v%s' % statInfo['etag']
Expand All @@ -94,7 +94,7 @@ def checkFileInfo(fileid, acctok):
fmd['SupportsContainers'] = False # TODO this is all to be implemented
fmd['SupportsUserInfo'] = True
uinfo = st.getxattr(acctok['endpoint'], acctok['filename'], statInfo['ownerid'],
utils.USERINFOKEY + '.' + acctok['wopiuser'].split('@')[0])
utils.USERINFOKEY + '.' + acctok['wopiuser'].split('!')[0])
if uinfo:
fmd['UserInfo'] = uinfo

Expand Down Expand Up @@ -368,7 +368,7 @@ def putRelative(fileid, reqheaders, acctok):
if acctok['viewmode'] != utils.ViewMode.READ_WRITE:
# here we must have an authenticated user with no write rights on the current folder: go to the user's homepath
targetName = srv.homepath.replace('user_initial', acctok['wopiuser'][0]). \
replace('username', acctok['wopiuser'].split('@')[0]) + os.path.sep
replace('username', acctok['wopiuser'].split('!')[0]) + os.path.sep
else:
targetName = os.path.dirname(acctok['filename'])
if suggTarget:
Expand Down Expand Up @@ -427,7 +427,7 @@ def putRelative(fileid, reqheaders, acctok):
# make an attempt in the user's home if possible
if utils.isPrimaryUser(acctok):
targetName = srv.homepath.replace('user_initial', acctok['wopiuser'][0]). \
replace('username', acctok['wopiuser'].split('@')[0]) \
replace('username', acctok['wopiuser'].split('!')[0]) \
+ os.path.sep + os.path.basename(targetName)
try:
utils.storeWopiFile(acctok, None, utils.LASTSAVETIMEKEY, targetName)
Expand Down Expand Up @@ -623,7 +623,7 @@ def putUserInfo(fileid, reqbody, acctok):
lockmd = st.getlock(acctok['endpoint'], acctok['filename'], acctok['userid'])
lockmd = (acctok['appname'], utils.encodeLock(lockmd)) if lockmd else None
st.setxattr(acctok['endpoint'], acctok['filename'], statInfo['ownerid'],
utils.USERINFOKEY + '.' + acctok['wopiuser'].split('@')[0], reqbody.decode(), lockmd)
utils.USERINFOKEY + '.' + acctok['wopiuser'].split('!')[0], reqbody.decode(), lockmd)
log.info('msg="PutUserInfo" user="%s" filename="%s" fileid="%s" token="%s"' %
(acctok['userid'][-20:], acctok['filename'], fileid, flask.request.args['access_token'][-20:]))
return 'OK', http.client.OK
Expand Down
16 changes: 8 additions & 8 deletions src/core/wopiutils.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ def generateAccessToken(userid, fileid, viewmode, user, folderurl, endpoint, app
'''Generates an access token for a given file and a given user, and returns a tuple with
the file's inode and the URL-encoded access token.'''
appname, appediturl, appviewurl = app
username, wopiuser = user
friendlyname, wopiuser = user # wopiuser has the form `username!userid_in_stat_format`
log.debug('msg="Generating token" userid="%s" fileid="%s" endpoint="%s" app="%s"' %
(userid[-20:], fileid, endpoint, appname))
try:
Expand Down Expand Up @@ -243,7 +243,7 @@ def generateAccessToken(userid, fileid, viewmode, user, folderurl, endpoint, app
viewmode = ViewMode.READ_ONLY
tokmd = {
'userid': userid, 'wopiuser': wopiuser, 'filename': statinfo['filepath'], 'fileid': fileid,
'username': username, 'viewmode': viewmode.value, 'folderurl': folderurl, 'endpoint': endpoint,
'username': friendlyname, 'viewmode': viewmode.value, 'folderurl': folderurl, 'endpoint': endpoint,
'appname': appname, 'appediturl': appediturl, 'appviewurl': appviewurl,
'exp': exptime, 'iss': 'cs3org:wopiserver:%s' % WOPIVER # standard claims
}
Expand All @@ -252,9 +252,9 @@ def generateAccessToken(userid, fileid, viewmode, user, folderurl, endpoint, app
acctok = jwt.encode(tokmd, srv.wopisecret, algorithm='HS256')
if 'MS 365' in appname:
srv.allusers.add(userid)
log.info('msg="Access token generated" userid="%s" wopiuser="%s" username="%s" mode="%s" endpoint="%s" filename="%s" '
log.info('msg="Access token generated" userid="%s" wopiuser="%s" friendlyname="%s" mode="%s" endpoint="%s" filename="%s" '
'inode="%s" mtime="%s" folderurl="%s" appname="%s"%s expiration="%d" token="%s"' %
(userid[-20:], wopiuser, username, viewmode, endpoint, statinfo['filepath'], statinfo['inode'], statinfo['mtime'],
(userid[-20:], wopiuser, friendlyname, viewmode, endpoint, statinfo['filepath'], statinfo['inode'], statinfo['mtime'],
folderurl, appname, ' forcelock="True"' if forcelock else '', exptime, acctok[-20:]))
return statinfo['inode'], acctok, viewmode

Expand Down Expand Up @@ -474,7 +474,7 @@ def makeLockSuccessResponse(operation, acctok, lock, oldlock, version):
'''Generates and logs an HTTP 200 response with appropriate headers for Lock/RefreshLock operations'''
session = flask.request.headers.get('X-WOPI-SessionId')
if not session:
session = acctok['wopiuser']
session = acctok['wopiuser'].split('!')[0]
_resolveSession(session, acctok['filename'])

log.info('msg="Successfully locked" lockop="%s" filename="%s" token="%s" sessionId="%s" '
Expand All @@ -494,7 +494,7 @@ def storeWopiFile(acctok, retrievedlock, xakey, targetname=''):
targetname = acctok['filename']
session = flask.request.headers.get('X-WOPI-SessionId')
if not session:
session = acctok['wopiuser']
session = acctok['wopiuser'].split('!')[0]
_resolveSession(session, targetname)

writeerror = None
Expand Down Expand Up @@ -529,7 +529,7 @@ def storeAfterConflict(acctok, retrievedlock, lock, reason):
else:
# let's try the configured user's (or owner's) homepath instead of the current folder
newname = srv.homepath.replace('user_initial', acctok['wopiuser'][0]). \
replace('username', acctok['wopiuser'].split('@')[0]) \
replace('username', acctok['wopiuser'].split('!')[0]) \
+ os.path.sep + os.path.basename(newname)
try:
storeWopiFile(acctok, retrievedlock, LASTSAVETIMEKEY, newname)
Expand All @@ -554,7 +554,7 @@ def storeAfterConflict(acctok, retrievedlock, lock, reason):
def storeForRecovery(content, wopiuser, filename, acctokforlog, exception):
try:
filepath = srv.recoverypath + os.sep + time.strftime('%Y%m%dT%H%M%S') + '_editedby_' \
+ secure_filename(wopiuser.split('@')[0]) + '_origat_' + secure_filename(filename)
+ secure_filename(wopiuser.split('!')[0]) + '_origat_' + secure_filename(filename)
with open(filepath, mode='wb') as f:
written = f.write(content)
if written != len(content):
Expand Down
6 changes: 4 additions & 2 deletions src/core/xrootiface.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,11 @@ def _getfilepath(filepath, encodeamp=False):

def getuseridfromcreds(_token, wopiuser):
'''Maps a Reva token and wopiuser to the credentials to be used to access the storage.
For the xrootd case, we have to resolve the username to uid:gid'''
For the xrootd case, we have to resolve the username to uid:gid and return
username!uid:gid as wopiuser in order to respect the format `username!userid_as_returned_by_stat`'''
userid = getpwnam(wopiuser.split('@')[0]) # a wopiuser has the form username@idp
return str(userid.pw_uid) + ':' + str(userid.pw_gid)
userid = str(userid.pw_uid) + ':' + str(userid.pw_gid)
return userid, wopiuser.split('@')[0] + '!' + userid


def stat(endpoint, filepath, userid):
Expand Down
8 changes: 2 additions & 6 deletions src/wopiserver.py
Original file line number Diff line number Diff line change
Expand Up @@ -329,11 +329,7 @@ def iopOpenInApp():
return 'Missing appname or appurl arguments', http.client.BAD_REQUEST

try:
userid = storage.getuseridfromcreds(usertoken, wopiuser)
if userid != usertoken:
# this happens in hybrid deployments with xrootd as storage interface:
# in this case we decorate the wopiuser with the resolved uid:gid
wopiuser += ':' + userid
userid, wopiuser = storage.getuseridfromcreds(usertoken, wopiuser)
inode, acctok, vm = utils.generateAccessToken(userid, fileid, viewmode, (username, wopiuser), folderurl, endpoint,
(appname, appurl, appviewurl), forcelock=forcelock)
except IOError as e:
Expand Down Expand Up @@ -438,7 +434,7 @@ def iopWopiTest():
return 'Missing arguments', http.client.BAD_REQUEST
if Wopi.useHttps:
return 'WOPI validator not supported in https mode', http.client.BAD_REQUEST
inode, acctok, _ = utils.generateAccessToken(usertoken, filepath, utils.ViewMode.READ_WRITE, ('test', usertoken),
inode, acctok, _ = utils.generateAccessToken(usertoken, filepath, utils.ViewMode.READ_WRITE, ('test', 'test!' + usertoken),
'http://folderurlfortestonly/', endpoint,
('WOPI validator', 'http://fortestonly/', 'http://fortestonly/'))
Wopi.log.info('msg="iopWopiTest: preparing test via WOPI validator" client="%s"' % req.remote_addr)
Expand Down

0 comments on commit 2acdfa1

Please sign in to comment.