New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flask OAuth object doesn't use cache if cache object is not truthy #98

Closed
rschroll opened this Issue Dec 20, 2018 · 3 comments

Comments

Projects
None yet
2 participants
@rschroll
Copy link

rschroll commented Dec 20, 2018

This is a bit of a corner case, and I'm not 100% sure I've diagnosed it correctly, so here's the background:

We are building a Flask-based login system based on loginpass and Twitter (OAuth1). I found that it would work if the flask.client.OAuth object were passed the a Cache object as implemented in the loginpass examples. But it would fail, with TypeError: 'NoneType' object is not callable, when I passed in a custom cache class subclassing dict:

class DictCache(dict):
    def set(self, k, v, timeout=None):
        self[k] = v
    def delete(self, k):
        self.pop(k, None)

After a while, I determined that I could make this class work by ensuring that it was always truthy:

    def __bool__(self):
        return True

This seems plausible. I see a few places in the code base that test if cache rather than if cache is not None. This DictCache object would be falsey to begin with, and thus would fail these test, while the sample Cache object from loginpass is always truthy.

If the tests of cache's truthiness are just to distinguish it from None, perhaps a test against None can be substituted. Or if there is something about the cache that this test is supposed to reveal, that can be better documented. All I saw was a description of the methods that cache should support.

This behavior is seen on Python 3.7 with Flask 1.0.2, loginpass 0.2.1, and Authlib 0.10.

@lepture lepture closed this in d75d087 Dec 20, 2018

@lepture

This comment has been minimized.

Copy link
Owner

lepture commented Dec 20, 2018

Thanks for your report. I've just fixed it. But you MUST use a real cache service like memcache/redis on production.

@rschroll

This comment has been minimized.

Copy link

rschroll commented Dec 20, 2018

Thanks. I agree -- no one's going to actually run into this in a production setting. But it was definitely a confusing hour of debugging in my dev environment.

@lepture

This comment has been minimized.

Copy link
Owner

lepture commented Dec 20, 2018

@rschroll the code is in master branch now. But there won't be a release soon since I'm working on v0.11.

d75d087

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment