theLaborInVain / kdm-manager-api

The API used by https://kdm-manager.com and related Kingdom Death: Monster utilities.
Other
3 stars 0 forks source link

users.User failure on init #53

Closed toconnell closed 3 years ago

toconnell commented 3 years ago

User OID: 666 Method: POST URL: http://api.kdm-manager.com/login JSON: {'username': 'xxx@gmail.com', 'password': 'xxx'}

Traceback (most recent call last):
File "/home/toconnell/kdm-manager-api/venv/lib/python3.6/site-packages/flask/app.py", line 1950, in full_dispatch_request
 rv = self.dispatch_request()
File "/home/toconnell/kdm-manager-api/venv/lib/python3.6/site-packages/flask/app.py", line 1936, in dispatch_request
 return self.view_functions[rule.endpoint](**req.view_args)
File "/home/toconnell/kdm-manager-api/app/utils/crossdomain.py", line 56, in wrapped_function
 resp = flask.make_response(func(*args, **kwargs))
File "/home/toconnell/kdm-manager-api/app/routes.py", line 318, in get_token
 flask.request.json.get("password", None)
File "/home/toconnell/kdm-manager-api/app/models/users.py", line 85, in authenticate
 U = User(_id=user["_id"])
File "/home/toconnell/kdm-manager-api/app/models/users.py", line 435, in __init__
 flask.request.User.user['login'] == self.user.get('login', None)
TypeError: 'NoneType' object is not subscriptable
toconnell commented 3 years ago

Interestingly, this one occurred with a user that was created back in 2016 (5 years and 12 days ago). It also throws an odd error when attempting to import it into the dev server, so it looks like this one is probably going to be...multifaceted.

toconnell commented 3 years ago

This is a problem in the user assets base class definition, specifically in the _requestresponse() method:

1522         if getattr(self, action, None) is not None:
1523             method = getattr(self, action)
1524             if getattr(method, '_web_method', False):
1525                 method()
1526             else:
1527                 err = "The %s endpoint is mapped to an internal method!"
1528                 return flask.Response(response=err % action, status=400)

What we're doing is calling the method and then returning a serialized version of the user asset, but with the users.User, we actually get a Flask response type return from some of these methods and need to respect that.

toconnell commented 3 years ago

I just monkey-patched this last bit in production in order to test it in production, because obviously that's a good place to test code:

@@ -1522,7 +1522,9 @@ class UserAsset(object):
         if getattr(self, action, None) is not None:
             method = getattr(self, action)
             if getattr(method, '_web_method', False):
-                method()
+                method_response = method()
+                if isinstance(method_response, flask.Response):
+                    return method_response
toconnell commented 3 years ago

So, ultimately, the issue here is that we bomb out while trying to record the fact that a user who is not authenticated yet is using the API. This is part of the move (in the last couple of releases) away from logging webapp activity directly to the database using the legacy webapp.

I think the fix for now is just going to be addin an additional conditional to the line that records user activity:

diff --git a/app/models/users.py b/app/models/users.py
index 389ad00..4430c2a 100644
--- a/app/models/users.py
+++ b/app/models/users.py
@@ -432,6 +432,7 @@ class User(models.UserAsset):
         if (
             flask.request and
             hasattr(flask.request, 'User') and
+            self.user.get('login', None) is not None and
             flask.request.User.user['login'] == self.user.get('login', None)
         ):
             self.set_latest_action(