Mercurial > p > roundup > code
comparison 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 |
comparison
equal
deleted
inserted
replaced
| 5709:e2378b6afdb5 | 5710:0b79bfcb3312 |
|---|---|
| 46 basestring = str | 46 basestring = str |
| 47 unicode = str | 47 unicode = str |
| 48 | 48 |
| 49 import logging | 49 import logging |
| 50 logger = logging.getLogger('roundup.rest') | 50 logger = logging.getLogger('roundup.rest') |
| 51 | |
| 52 import roundup.anypy.random_ as random_ | |
| 53 if not random_.is_weak: | |
| 54 logger.debug("Importing good random generator") | |
| 55 else: | |
| 56 logger.warning("**SystemRandom not available. Using poor random generator") | |
| 57 | |
| 58 import time | |
| 59 | |
| 60 chars = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789' | |
| 51 | 61 |
| 52 def _data_decorator(func): | 62 def _data_decorator(func): |
| 53 """Wrap the returned data into an object.""" | 63 """Wrap the returned data into an object.""" |
| 54 def format_object(self, *args, **kwargs): | 64 def format_object(self, *args, **kwargs): |
| 55 # get the data / error from function | 65 # get the data / error from function |
| 232 | 242 |
| 233 | 243 |
| 234 class Routing(object): | 244 class Routing(object): |
| 235 __route_map = {} | 245 __route_map = {} |
| 236 __var_to_regex = re.compile(r"<:(\w+)>") | 246 __var_to_regex = re.compile(r"<:(\w+)>") |
| 237 url_to_regex = r"([\w.\-~!$&'()*+,;=:@\%%]+)" | 247 url_to_regex = r"([\w.\-~!$&'()*+,;=:\%%]+)" |
| 238 | 248 |
| 239 @classmethod | 249 @classmethod |
| 240 def route(cls, rule, methods='GET'): | 250 def route(cls, rule, methods='GET'): |
| 241 """A decorator that is used to register a view function for a | 251 """A decorator that is used to register a view function for a |
| 242 given URL rule: | 252 given URL rule: |
| 366 protected_class_props = [ p for p in | 376 protected_class_props = [ p for p in |
| 367 list(cl.getprops(protected=True)) | 377 list(cl.getprops(protected=True)) |
| 368 if p not in unprotected_class_props ] | 378 if p not in unprotected_class_props ] |
| 369 props = {} | 379 props = {} |
| 370 # props = dict.fromkeys(class_props, None) | 380 # props = dict.fromkeys(class_props, None) |
| 381 | |
| 382 if not args: | |
| 383 raise UsageError("No properties found.") | |
| 371 | 384 |
| 372 for arg in args: | 385 for arg in args: |
| 373 key = arg.name | 386 key = arg.name |
| 374 value = arg.value | 387 value = arg.value |
| 375 if key.startswith('@'): | 388 if key.startswith('@'): |
| 885 int: http status code 201 (Created) | 898 int: http status code 201 (Created) |
| 886 dict: a reference item to the created object | 899 dict: a reference item to the created object |
| 887 id: id of the object | 900 id: id of the object |
| 888 link: path to the object | 901 link: path to the object |
| 889 """ | 902 """ |
| 903 return self.post_collection_inner(class_name, input) | |
| 904 | |
| 905 @Routing.route("/data/<:class_name>/@poe", 'POST') | |
| 906 @_data_decorator | |
| 907 def post_once_exactly_collection(self, class_name, input): | |
| 908 otks=self.db.Otk | |
| 909 poe_key = ''.join([random_.choice(chars) for x in range(40)]) | |
| 910 while otks.exists(u2s(poe_key)): | |
| 911 poe_key = ''.join([random_.choice(chars) for x in range(40)]) | |
| 912 | |
| 913 try: | |
| 914 lifetime=int(input['lifetime'].value) | |
| 915 except KeyError as e: | |
| 916 lifetime=30 * 60 # 30 minutes | |
| 917 except ValueError as e: | |
| 918 raise UsageError("Value 'lifetime' must be an integer specify lifetime in seconds. Got %s."%input['lifetime'].value) | |
| 919 | |
| 920 if lifetime > 3600 or lifetime < 1: | |
| 921 raise UsageError("Value 'lifetime' must be between 1 second and 1 hour (3600 seconds). Got %s."%input['lifetime'].value) | |
| 922 | |
| 923 try: | |
| 924 # if generic tag exists, we don't care about the value | |
| 925 is_generic=input['generic'] | |
| 926 # we generate a generic POE token | |
| 927 is_generic=True | |
| 928 except KeyError as e: | |
| 929 is_generic=False | |
| 930 | |
| 931 # a POE must be used within lifetime (30 minutes default). | |
| 932 # Default OTK lifetime is 1 week. So to make different | |
| 933 # lifetime, take current time, subtract 1 week and add | |
| 934 # lifetime. | |
| 935 ts = time.time() - (60 * 60 * 24 * 7) + lifetime | |
| 936 if is_generic: | |
| 937 otks.set(u2s(poe_key), uid=self.db.getuid(), | |
| 938 __timestamp=ts ) | |
| 939 else: | |
| 940 otks.set(u2s(poe_key), uid=self.db.getuid(), | |
| 941 class_name=class_name, | |
| 942 __timestamp=ts ) | |
| 943 otks.commit() | |
| 944 | |
| 945 return 200, { 'link': '%s/%s/@poe/%s'%(self.data_path, class_name, poe_key), | |
| 946 'expires' : ts + (60 * 60 * 24 * 7) } | |
| 947 | |
| 948 @Routing.route("/data/<:class_name>/@poe/<:post_token>", 'POST') | |
| 949 @_data_decorator | |
| 950 def post_once_exactly_collection(self, class_name, post_token, input): | |
| 951 otks=self.db.Otk | |
| 952 | |
| 953 # remove expired keys so we don't use an expired key | |
| 954 otks.clean() | |
| 955 | |
| 956 if not otks.exists(u2s(post_token)): | |
| 957 # Don't log this failure. Would allow attackers to fill | |
| 958 # logs. | |
| 959 raise UsageError("POE token '%s' not valid."%post_token) | |
| 960 | |
| 961 # find out what user owns the key | |
| 962 user = otks.get(u2s(post_token), 'uid', default=None) | |
| 963 # find out what class it was meant for | |
| 964 cn = otks.get(u2s(post_token), 'class_name', default=None) | |
| 965 | |
| 966 # Invalidate the key as it has been used. | |
| 967 otks.destroy(u2s(post_token)) | |
| 968 otks.commit() | |
| 969 | |
| 970 # verify the same user that requested the key is the user | |
| 971 # using the key. | |
| 972 if user != self.db.getuid(): | |
| 973 # Tell the roundup admin that there is an issue | |
| 974 # as the key got compromised. | |
| 975 logger.warn( | |
| 976 'Post Once key owned by user%s was denied. Used by user%s',user, self.db.getuid() | |
| 977 ) | |
| 978 # Should we indicate to user that the token is invalid | |
| 979 # because they are not the user who owns the key? It could | |
| 980 # be a logic bug in the application. But I assume that | |
| 981 # the key has been stolen and we don't want to tip our hand. | |
| 982 raise UsageError("POE token '%s' not valid."%post_token) | |
| 983 | |
| 984 if cn != class_name and cn != None: | |
| 985 raise UsageError("POE token '%s' not valid for %s, was generated for class %s"%(post_token, class_name, cn)) | |
| 986 | |
| 987 # handle this as though they POSTed to /rest/data/class | |
| 988 return self.post_collection_inner(class_name, input) | |
| 989 | |
| 990 def post_collection_inner(self, class_name, input): | |
| 890 if class_name not in self.db.classes: | 991 if class_name not in self.db.classes: |
| 891 raise NotFound('Class %s not found' % class_name) | 992 raise NotFound('Class %s not found' % class_name) |
| 892 if not self.db.security.hasPermission( | 993 if not self.db.security.hasPermission( |
| 893 'Create', self.db.getuid(), class_name | 994 'Create', self.db.getuid(), class_name |
| 894 ): | 995 ): |
| 1742 ''' | 1843 ''' |
| 1743 def __init__(self, json_string): | 1844 def __init__(self, json_string): |
| 1744 ''' Parse the json string into an internal dict. ''' | 1845 ''' Parse the json string into an internal dict. ''' |
| 1745 def raise_error_on_constant(x): | 1846 def raise_error_on_constant(x): |
| 1746 raise ValueError("Unacceptable number: %s"%x) | 1847 raise ValueError("Unacceptable number: %s"%x) |
| 1747 self.json_dict = json.loads(json_string, | 1848 try: |
| 1849 self.json_dict = json.loads(json_string, | |
| 1748 parse_constant = raise_error_on_constant) | 1850 parse_constant = raise_error_on_constant) |
| 1749 self.value = [ self.FsValue(index, self.json_dict[index]) for index in self.json_dict.keys() ] | 1851 self.value = [ self.FsValue(index, self.json_dict[index]) for index in self.json_dict.keys() ] |
| 1852 except ValueError as e: | |
| 1853 self.json_dict = {} | |
| 1854 self.value = None | |
| 1750 | 1855 |
| 1751 class FsValue: | 1856 class FsValue: |
| 1752 '''Class that does nothing but response to a .value property ''' | 1857 '''Class that does nothing but response to a .value property ''' |
| 1753 def __init__(self, name, val): | 1858 def __init__(self, name, val): |
| 1754 self.name=u2s(name) | 1859 self.name=u2s(name) |
