Mercurial > p > roundup > code
diff roundup/rest.py @ 5710:0b79bfcb3312
Add support for making an idempotent POST. This allows retrying a POST
that was interrupted. It involves creating a post once only (poe) url
/rest/data/<class>/@poe/<random_token>. This url acts the same as a
post to /rest/data/<class>. However once the @poe url is used, it
can't be used for a second POST.
To make these changes:
1) Take the body of post_collection into a new post_collection_inner
function. Have post_collection call post_collection_inner.
2) Add a handler for POST to rest/data/class/@poe. This will return a
unique POE url. By default the url expires after 30 minutes. The
POE random token is only good for a specific user and is stored in
the session db.
3) Add a handler for POST to rest/data/<class>/@poe/<random token>.
The random token generated in 2 is validated for proper class (if
token is not generic) and proper user and must not have expired.
If everything is valid, call post_collection_inner to process the
input and generate the new entry.
To make recognition of 2 stable (so it's not confused with
rest/data/<:class_name>/<:item_id>), removed @ from
Routing::url_to_regex.
The current Routing.execute method stops on the first regular
expression to match the URL. Since item_id doesn't accept a POST, I
was getting 405 bad method sometimes. My guess is the order of the
regular expressions is not stable, so sometime I would get the right
regexp for /data/<class>/@poe and sometime I would get the one for
/data/<class>/<item_id>. By removing the @ from the url_to_regexp,
there was no way for the item_id case to match @poe.
There are alternate fixes we may need to look at. If a regexp matches
but the method does not, return to the regexp matching loop in
execute() looking for another match. Only once every possible match
has failed should the code return a 405 method failure.
Another fix is to implement a more sophisticated mechanism so that
@Routing.route("/data/<:class_name>/<:item_id>/<:attr_name>", 'PATCH')
has different regexps for matching <:class_name> <:item_id> and
<:attr_name>. Currently the regexp specified by url_to_regex is used
for every component.
Other fixes:
Made failure to find any props in props_from_args return an empty
dict rather than throwing an unhandled error.
Make __init__ for SimulateFieldStorageFromJson handle an empty json
doc. Useful for POSTing to rest/data/class/@poe with an empty
document.
Testing:
added testPostPOE to test/rest_common.py that I think covers
all the code that was added.
Documentation:
Add doc to rest.txt in the "Client API" section titled: Safely
Re-sending POST". Move existing section "Adding new rest endpoints" in
"Client API" to a new second level section called "Programming the
REST API". Also a minor change to the simple rest client moving the
header setting to continuation lines rather than showing one long
line.
| author | John Rouillard <rouilj@ieee.org> |
|---|---|
| date | Sun, 14 Apr 2019 21:07:11 -0400 |
| parents | f9a762678af6 |
| children | aea2cc142c1b |
line wrap: on
line diff
--- a/roundup/rest.py Sat Apr 13 13:53:24 2019 -0400 +++ b/roundup/rest.py Sun Apr 14 21:07:11 2019 -0400 @@ -49,6 +49,16 @@ import logging logger = logging.getLogger('roundup.rest') +import roundup.anypy.random_ as random_ +if not random_.is_weak: + logger.debug("Importing good random generator") +else: + logger.warning("**SystemRandom not available. Using poor random generator") + +import time + +chars = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789' + def _data_decorator(func): """Wrap the returned data into an object.""" def format_object(self, *args, **kwargs): @@ -234,7 +244,7 @@ class Routing(object): __route_map = {} __var_to_regex = re.compile(r"<:(\w+)>") - url_to_regex = r"([\w.\-~!$&'()*+,;=:@\%%]+)" + url_to_regex = r"([\w.\-~!$&'()*+,;=:\%%]+)" @classmethod def route(cls, rule, methods='GET'): @@ -369,6 +379,9 @@ props = {} # props = dict.fromkeys(class_props, None) + if not args: + raise UsageError("No properties found.") + for arg in args: key = arg.name value = arg.value @@ -887,6 +900,94 @@ id: id of the object link: path to the object """ + return self.post_collection_inner(class_name, input) + + @Routing.route("/data/<:class_name>/@poe", 'POST') + @_data_decorator + def post_once_exactly_collection(self, class_name, input): + otks=self.db.Otk + poe_key = ''.join([random_.choice(chars) for x in range(40)]) + while otks.exists(u2s(poe_key)): + poe_key = ''.join([random_.choice(chars) for x in range(40)]) + + try: + lifetime=int(input['lifetime'].value) + except KeyError as e: + lifetime=30 * 60 # 30 minutes + except ValueError as e: + raise UsageError("Value 'lifetime' must be an integer specify lifetime in seconds. Got %s."%input['lifetime'].value) + + if lifetime > 3600 or lifetime < 1: + raise UsageError("Value 'lifetime' must be between 1 second and 1 hour (3600 seconds). Got %s."%input['lifetime'].value) + + try: + # if generic tag exists, we don't care about the value + is_generic=input['generic'] + # we generate a generic POE token + is_generic=True + except KeyError as e: + is_generic=False + + # a POE must be used within lifetime (30 minutes default). + # Default OTK lifetime is 1 week. So to make different + # lifetime, take current time, subtract 1 week and add + # lifetime. + ts = time.time() - (60 * 60 * 24 * 7) + lifetime + if is_generic: + otks.set(u2s(poe_key), uid=self.db.getuid(), + __timestamp=ts ) + else: + otks.set(u2s(poe_key), uid=self.db.getuid(), + class_name=class_name, + __timestamp=ts ) + otks.commit() + + return 200, { 'link': '%s/%s/@poe/%s'%(self.data_path, class_name, poe_key), + 'expires' : ts + (60 * 60 * 24 * 7) } + + @Routing.route("/data/<:class_name>/@poe/<:post_token>", 'POST') + @_data_decorator + def post_once_exactly_collection(self, class_name, post_token, input): + otks=self.db.Otk + + # remove expired keys so we don't use an expired key + otks.clean() + + if not otks.exists(u2s(post_token)): + # Don't log this failure. Would allow attackers to fill + # logs. + raise UsageError("POE token '%s' not valid."%post_token) + + # find out what user owns the key + user = otks.get(u2s(post_token), 'uid', default=None) + # find out what class it was meant for + cn = otks.get(u2s(post_token), 'class_name', default=None) + + # Invalidate the key as it has been used. + otks.destroy(u2s(post_token)) + otks.commit() + + # verify the same user that requested the key is the user + # using the key. + if user != self.db.getuid(): + # Tell the roundup admin that there is an issue + # as the key got compromised. + logger.warn( + 'Post Once key owned by user%s was denied. Used by user%s',user, self.db.getuid() + ) + # Should we indicate to user that the token is invalid + # because they are not the user who owns the key? It could + # be a logic bug in the application. But I assume that + # the key has been stolen and we don't want to tip our hand. + raise UsageError("POE token '%s' not valid."%post_token) + + if cn != class_name and cn != None: + raise UsageError("POE token '%s' not valid for %s, was generated for class %s"%(post_token, class_name, cn)) + + # handle this as though they POSTed to /rest/data/class + return self.post_collection_inner(class_name, input) + + def post_collection_inner(self, class_name, input): if class_name not in self.db.classes: raise NotFound('Class %s not found' % class_name) if not self.db.security.hasPermission( @@ -1744,9 +1845,13 @@ ''' Parse the json string into an internal dict. ''' def raise_error_on_constant(x): raise ValueError("Unacceptable number: %s"%x) - self.json_dict = json.loads(json_string, + try: + self.json_dict = json.loads(json_string, parse_constant = raise_error_on_constant) - self.value = [ self.FsValue(index, self.json_dict[index]) for index in self.json_dict.keys() ] + self.value = [ self.FsValue(index, self.json_dict[index]) for index in self.json_dict.keys() ] + except ValueError as e: + self.json_dict = {} + self.value = None class FsValue: '''Class that does nothing but response to a .value property '''
