Bug #7634
openauthorize method doesn't check all flags
0%
Description
So, there is the following method called authorize
:
def authorize(self, env, client, path, method):
if method.debug:
if not client.debug:
self.log.info("authorize: failed, client does not have debug enabled")
return None
return client
if method.read:
if not client.read:
self.log.info("authorize: failed, client does not have read enabled")
return None
return client
if method.write:
if not (client.write or client.test):
self.log.info("authorize: failed, client is not allowed to write or test")
return None
return client
And then there are a bunch of exposed methods, the following one called getDebug
is interesting:
@expose(read=True, debug=True)
@json_wrapper
def getDebug(self):
return {
"environment": self.req.env,
...
}
If in authorize
, read was checked before debug, it wouldn't matter what is the value of debug in getDebug
, because the method would only check read and then return.
So the question is, is that a bug or a feature? I want to add another flag - managed - but I would need to add it at the beginning of the authorize
method. I think it shouldn't depend on the order of checks (if statements).
Updated by Pavel Kácha over 1 year ago
You're right, this looks like a bug. The getDebug method should be exposed as 'debug' only, nothing more. Same with 'managed'.
Updated by Rajmund Hruška over 1 year ago
Pavel Kácha wrote in #note-1:
You're right, this looks like a bug. The getDebug method should be exposed as 'debug' only, nothing more. Same with 'managed'.
'read' flag is True by default, so removing it from getDebug
wouldn't actually change anything. Anyway, I put the 'manage' check before 'read' check and that works for now. But it would be nice to fix this bug in the future, so I suggest keeping this issue open.
Updated by Pavel Kácha over 1 year ago
Do we actually need to authorize by combination of permissions? Say (read AND debug)? I guess not, so more straightforward API might be something around expose(perm)
, like @expose("read")
?