sanic-auth
sanic-auth copied to clipboard
User model not so agnostic...
The json mechanism you use right now is failing when the objects are BSON encoded python objects from MongoDB.
Ultimately it seems like Pickle would be a better solution and remove any issues with use-cases ...
On top of that it would be great if the entire user object was available on the session, sanic-auth didn't look for specific field names, and went about this in a more generic way since we can't always assume people can or will code their DB/User models to match.
thanks!
-k. noah
Sorry for the late reply, I was quite busy lately.
On the BSON case, could you show me some example code?
IIRC, Pickle has a infamous history of security problems, it's format is meant to be use within the application, better ever across the boundary of the running process, it used to have bugs of arbitrary code execution. Even though we cryptographically signing the payload, it is not a risk worth taking, IMO.
You can always store anything digitally presentable in JSON with some encoding, base64, for example.
I try to make sanic-auth backend agnostic, so the default implementation is that you described, you can override relevant methods to customized the loading mechanism.
If I misunderstand you point, please let me know.
Thank you.
Hey sorry that was a pretty poorly done bug report. My bad ... it was late and I was spinning my wheels.
Aaany-who.
-
The BSON bug is actually in the sanic-session code, and since they have native support for MongoDB I'm hoping they take the effort to make the BSON support a bit smoother for their end users. They try and serialize straight to JSON which doesn't account for BSON object-IDs, which I'm about 5% sure breaks with their own mongodb backend.
-
There are some issues with the User object being backend agnostic ... at minimum some examples showing use-cases for the decorators would be nice ;)
It looks like you have two mechanisms to override the load_user/serialize methods. You can either subclass and override load_user/serialize .. or you can use the decorator which will monkey patch the load_user method.
There would be a lot of redundant code if you use the monkey-patch/decorator, since you'd have to include that on every method that is authenticated... right?
Maybe I'm missing something, but it seems like it would make more sense to offer that from within the setup() method? If you're going to monkey-patch it seems safer to make sure it's only done once in setup or what-have-you.
Also it's not super clear that with a custom user, you also need to customize the serialize method ...
The requirement of "name" and "id" fields on your "User" model when you serialize() is what I ran into ... since I use 'email' instead of 'name' and there's no unique ID on my user model (other than email).
You could configure which variable names are available and should be hashed with a list[] grabbed in setup(). It would be nice to just infer which vars are available, but that seems like it'd probably be far too much trouble and just create a lot of issues down the road...
def setup(self):
self.serialization_fields = get('SERIALIZATION_FIELDS', ['id'])
self.serializer = get('SERIALIZER', serializer)
self.load_user = get('USER_LOADER', load_user)
#OR
if get('USER_LOADER', None): self.user_loader(get('USER_LOADER'))
def serializer(self, user):
"""prolly could do this with one line :)"""
serialized_user = {}
for field in self.serialization_fields:
serialized_user[field] = getattr(user, field, None)
return serialized_user
- and this is an entirely different bug which I should probably open a different ticket for but this is already such a mess of a bug report that .. why not? Your example for "user_keyword" in the README will throw the runtime error...
@auth.login_required(user_keyword='user')
async def profile(request, user):
return response.json({'user': user})
it automatically pushes user into the response kwargs ... and will throw an error if it's already there, right?
# line 104
` if user_keyword in kwargs:
raise RuntimeError(
'override user keyword %r in route' % user_keyword)
It's just a quick note to tell you that I am still alive :) I was super busy lately, I will read your comment thoroughly in a few days.
Thanks for the feedback.
Any news on this?
Any news on this?
Could you please elaborate? If I understand correctly, it's probably because of the vague documentation, how the user object is retrieved is fully customizable, tell me where is unclear, you could help me improve that.