current_user instance is of LocalProxy type

[eka]

Hi, when using current_user instance I was having a weird bug with MongoEngine, then I decided to debug and when checking:

ipdb> type(current_user)
<class 'werkzeug.local.LocalProxy'>

That's not desired if I will use it directly. MongoEngine complains when using that instance during query/update, etc... Is there any way to unwrap the proxy? Right now I have to query to the DB to get the real object.

Thanks

[maxcountryman]

Hi,

The short answer is: yes, there is a way to unwrap proxied objects iirc. However off the top of my head I don't remember the API. Digging into the Flask/Werkzeug docs might help. I'll have to take a look myself later after work.

HTH.

[eka]

Hi, thanks for your reply. Did found the _get_current_object(), but the question is if that current_user shouldn't be already the current_object instead of the proxy one?

[maxcountryman]

Hmm I'm not quite sure I follow. This might be a good question for @leafstorm since that was originally his design I believe.

[maxcountryman]

My sense here is that you should probably just unwrap the proxy in the scope of your view. Doing so beforehand might break other things.

[maxcountryman]

Closing this for now as this was designed to be a proxied object.

[dendik]

I've had a terrible night debugging the broken DB and I lost a few entries because I stored current_user object instead of the user it references.

Please, add a warning in docs to never store current_user in database or assign it as an attribute on any object that live longer than one request. (In my case that covers 100% of uses of current_user, and it is likely to be 100% uses for anyone with a caching ORM or pickle-based DBs, and I would argue that current_user is the case when making a proxy is inaproppriate).

Also, please, add a way to unwrap current_user or provide any public API. By default I consider _get_user not a solution, since: a) it is private function, b) it does not return anonymous user when it should.

The simplest way to fix this is define a function get_current_user, which is exactly the same as lambda that is passed to LocalProxy, and make current_user a LocalProxy for that function. I can provide a commit if you like, but I still ask to change the docs to warn about dangers of current_user and existence of alternatives.

[pahaz]

This is realy bead. I`am find bug then use this.

@app.before_request
def global_user():
    g.user = current_user

and python social auth: https://github.com/omab/python-social-auth/blob/master/social/actions.py#L46-L47

Traceback (most recent call last):
  File "C:\Python27\lib\site-packages\flask\app.py", line 1836, in __call__
    return self.wsgi_app(environ, start_response)
  File "C:\Python27\lib\site-packages\flask\app.py", line 1820, in wsgi_app
    response = self.make_response(self.handle_exception(e))
  File "C:\Python27\lib\site-packages\flask\app.py", line 1403, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "C:\Python27\lib\site-packages\flask\app.py", line 1817, in wsgi_app
    response = self.full_dispatch_request()
  File "C:\Python27\lib\site-packages\flask\app.py", line 1478, in full_dispatch_request
    response = self.make_response(rv)
  File "C:\Python27\lib\site-packages\flask\app.py", line 1577, in make_response
    rv = self.response_class.force_type(rv, request.environ)
  File "C:\Python27\lib\site-packages\werkzeug\wrappers.py", line 824, in force_type
    response = BaseResponse(*_run_wsgi_app(response, environ))
  File "C:\Python27\lib\site-packages\werkzeug\wrappers.py", line 57, in _run_wsgi_app
    return _run_wsgi_app(*args)
  File "C:\Python27\lib\site-packages\werkzeug\test.py", line 854, in run_wsgi_app
    app_iter = app(environ, start_response)
  File "C:\Python27\lib\site-packages\werkzeug\local.py", line 366, in <lambda>
    __call__ = lambda x, *a, **kw: x._get_current_object()(*a, **kw)
TypeError: 'User' object is not callable

[maxcountryman]

@pahaz what bug exactly? Can you provide a test case? Thank you.

[pahaz]

Problem, in other app: python-social-auth, but this problem connected with this issues.

And this problem, realy hard to debug. You can find reason this: https://github.com/omab/python-social-auth/blob/master/social/actions.py#L46-L47

[maxcountryman]

@pahaz you should report this issue to the team that manages that project.

[pahaz]

Yes, of course. https://github.com/omab/python-social-auth/issues/257

[pahaz]

But, problem in flask-login logic too. It is not obvious that the current_user is a Proxy! I think, need use g.current_user. It will be greater transparency of behavior.

[pahaz]

May be you reopen this issues?!

[dendik]

pahaz, the good workaround in my case was to add this mixin to my User class:

class TrueSelf(object):

    @property
    def self(self):
        """If self was pointed to by a proxy, present the true self.

        E.g. if self was returned by current_user, which is a proxy,
        we actually want to have the actual User object on an entry,
        not the one varying with each request.

        Also vital for storing into DB.
        """
        return self

class User(..., TrueSelf):
    ...

And then I could store current_user.self (current_user itself is still the proxy!) in the database. Or, in your case, it would be g.user = current_user.self (you have to test it; I don't remember if current_user already takes the proper value in before_request handler).

I support your request to reopen the ticket!

[maxcountryman]

@eka a much belated answer to your original question: You can get the object behind the proxy by calling _get_current_object.

[maxcountryman]

@pahaz and @dendik I don't see any reason to reopen this. Flask-Login was designed to expose a current_user as a proxy just like flask.session and so forth. You need to design your applications around this interface, not the other way around.

[dendik]

maxcountryman, I disagree that _get_current_object is a good answer as I pointed out in my first comment...

[maxcountryman]

@dendik adding a getter like you suggested before might be fine.

[dendik]

@maxcountryman, though this is not exactly a bug, it is a common stumbling point and should be documented as such.

Also, there exist no kosher (I mean PEP-8 kosher) documented way to extract the true value of the current_user.

IMHO each of the two points is enough to reopen the ticket.

[maxcountryman]

@dendik I'm amenable to pull requests.

[dendik]

@maxcountryman, thanks for the hint and for taking your time!

Is this a suitable solution?

[xsleonard]

TLDR: current_user._get_current_object().

http://flask-login.readthedocs.org/en/latest/#flask.ext.login.current_user

A proxy for the current user.

Of course its a proxy object, how else would you be able to do from flask.ext.login import current_user and have it reference the correct value in a request? This is how context bound globals work.

current_user._get_current_object() does exactly what is needed here. This is part of the werkzeug.local.LocalProxy API. Its not flask-login's responsibility to remind users how the core framework works.

You don't need to call _get_user(), its only bypassing the proxy and it wasn't intended to be called directly so YMMV. As for the mixin, you need a problem before you can have a workaround. You're still calling _get_current_object() underneath.

Open an issue with the database lib you're using if its letting you save LocalProxy or reference values to your db. LocalProxy isn't pickable and any method call on it (as would be done to convert it to its serialized representation) should forward to the proxied object.

[dendik]

@xsleonard, TLDR: the problem exists; mixin / my pull request are both solutions.

I would agree with your solution, IF: a) _get_current_object() were documented as such in both flask-login and werkzeug (it is documented as such in neither of them currently), b) name of _get_current_object() did not start with underscore and it would really be nice, if c) a warning message in large unfriendly letters were in the docs of werkzeug and docs of flask-login referenced it.

In the present state I hate to call a solution something that i) might go away, since it is someone's deep internal function, and nobody cares about it, and ii) it is undocumented in both places and is royal pain in the behind do find out, since the code breaks in seemingly totally unrelated places.

I spent one painful night of my own time to find this trivial truth out, and I wish no one follows my steps. That is why I spend another couple of hours persuading people to add some warnings and a more stable/reliable workaround. I sent a pull request to @maxcountryman with something that could pass for a solution, and I hope to see the patch accepted or to hear some better ideas.

One more point: my solution with the mixin works. Obviously, it is underdocumented: it's only meaningful line of code seems to do nothing at all, which is not the case. Please ponder about it for a longer time. The idea is: current_user is a proxy, but any method called through it gets hold on the real object.

You are absolutely correct that _get_user() simply bypasses the proxy. Bypassing the proxy is eventually the idea of any solution possible here. And also you are absolutely correct that the problem with saving current_user to the database is with LocalProxy not being picklable. (Actually, there is one more problem: saving "current_user", if succsessful, tells database, that "whoever is currently logged in is the one you need". Which is wrong in virtually all cases I can imagine. So LocalProxy being not picklable actually solves another possible problem here).

[maxcountryman]

@dendik I don't think that PR can be merged as-is. I do not want to expose _get_user. What I had meant was a wrapper around _get_current_object. But that seems unnecessary or redundant in retrospect. Furthermore, I'm starting to think this is, in general, a bad idea.

Why would you want to store a user object directly in the database? Ask yourself, would you store flask.session or flask.request this way?

it is undocumented in both places

It is documented in werkzeug, where you would expect to find documentation about the werkzeug implementation. I find it a stretch that we should have to document werkzeug in Flask-Login too.

But I'm fine with adding a note in the documentation that states current_user is a proxy. However this should be readily apparent to people familiar with Flask. (How else would you expect this to work?) Also with the caveat that nowhere in the documentation should we be advocating for persisting current_user directly in the db; this so most definitely not what anyone should be doing.

[maxcountryman]

Actually as @xsleonard pointed out, we already have a note about that in the documentation. I think this issue is moot at this point. It was closed two years ago, we are doing what I think is the correct thing here and there is a standard way of accessing the underlying werkzeug API. Can we say case closed?

[xsleonard]

Take it up with werkzeug if you don't like the prefix. It doesn't have an underscore to convey that it shouldn't be called, it has it because the proxy shouldn't put anything in proxied object's 'public' namespace.

As pointed out, flask-login states that current_user is a proxy. In two places. If you want to submit a PR to make it clear that its werkzeug's LocalProxy thats fine.

LocalProxy._get_current_object() is documented in werkzeug here: http://werkzeug.pocoo.org/docs/local/#werkzeug.local.LocalProxy._get_current_object

Of course your mixin works. The proxy calls _get_current_object() to get the real user object, gets 'self' off of that with getattr, and that calls self() as a property, which returns the bound self. Which is the same value that was returned by _get_current_object().

_get_current_object() == _get_current_object().self.self.self...self.

You had a problem because for some reason you were able to save a LocalProxy object to your database. There should have been an exception, unless your db is extremely forgiving in which case you need stricter tests. Even if you knew not to try to persist current_user you could still do it mistakenly.

It might be safe to bypass the proxy with _get_user() but it needs to be reviewed before its made public. However, there's no reason to. _get_current_object() exists.

[dendik]

@maxcountryman now that you mention it, saving the whole flask.request would make sense for cases when one needs to retrospect one's webserver activity without prior knowlege on where to look. That would be the perfect thing to do in cases when the request seemed strange for some reason (and as a sysadmin I would keep the request to later see if it was a break-in attempt). That was an interesting idea.

In early experimental stages of developing a web-service it seems the most obvious and reasonable idea to dump anything at all into any pickle database to later profile the result and replace the bottlenecks with faster DB backends. So, storing a user object in a database is a perfectly sane solution.

As yet, we saw at least one more frustrated user since me (@pahaz) and did exactly nothing to prevent the same frustration from happening. Can we truly say the case is closed then?

There is virtually no difference, whether to duplicate interface of _get_current_object or of _get_user, but I see no way to avoid such duplication at all. Actually, if you accept the point of view that storing the user itself in a database is OK, there happens to be less use for current_user proxy than for a function that returns the real user object.

The problem is that there is currently no documented way to get object of the currently logged in user.

I see three alternatives:

• to nag Armin Ronacher to add more docs in this place about LocalProxy and to add some more user-friendly way to unwrap it (or a piece of docs in big letters saying that it's OK to call a method with underscore, and this method will never go away); still this requires you to have an example of how to apply this to current_user
• the solution I PR'ed you
• to add something like true_self property to UserMixin, and still some warning message about dangers of storing proxies in the databases

Let me add one more point of view: you designed flask-login and this forced you to fully understand LocalProxy of werkzeug. A typical flask user does not by have to know anything about werkzeug. Especially some corner cases of LocalProxy, which is not the front-page feature of werkzeug. There is even no explanation about what nature of proxy current_user is in the current docs!

[dendik]

@xsleonard speaking of tests, what prevents you from running them on flask-login to see if my patch doesn't break anything?

You are right that this should be fixed in werkzeug. I'll get down to it someday probably...

My point is: saving user in a database is a fine usecase. But you seem to be punishing programmers for it for some reason.

[dendik]

By punishing I mean this: even with warnings and docs current_user._get_current_object() is 34 chars, almost half of the PEP8 line length. And the meaning of this line is not exactly obvious.

[davidism]

Your patch won't break anything in Flask-Login, but what about people who depend on _get_user as it is named now? Your patch instantly breaks all their code with no warning.

I think we've driven this into the ground, can it die now?

[dendik]

@davidism you are right, a warning should be added. The PR was rejected though. Besides, why would anyone rely on internal function of some other project? Is the idea of leading underscore in python not exactly about warning people to never rely on this name to be present or do the same thing in the future?

The problem is: the current solution is not very friendly to small brain programmers like me. If this is OK with you, go ahead, let it die.

[ghost]

Why can't we have a simple stupid function like get_current_user (returning the result of LoginManager.user_loader) that, unlike a proxy, will just work predictably in all cases?

Proxies are evil.

[davidism]

Proxies are evil.

Reconsider using Flask then.

[asteinlein]

+1 to having a get_current_user() or some such that returns the current object of the proxy -- I really don't understand the controversy.

I was just bit by this exact thing and found this issue, when using Flask-OAuthlib to store grant objects in a Redis cache. A grant must be created for the current user (and Flask-OAuthlib expects the user object on the grant, not just an identifier/username). But, just as evident by this discussion, I can't do grant = Grant(user=current_user, ...) and save that grant to Redis, because of this issue.

To be honest, I don't quite understand the reasoning behind having a current_user global proxy instead of putting the current user into Flask's g, which seems designed for stuff like this. Anyhow, that's a design decision that has long since been made, so OK. But why not make users lives easier by exposing a get_current_user() that doesn't affect this library's proxy usage, or existing code using this library, at all?

[maxcountryman]

@asteinlein there's nothing stopping you from implementing that yourself. I'm locking this topic now.