pyeve / eve-sqlalchemy

SQLAlchemy data layer for Eve-powered RESTful APIs
http://eve-sqlalchemy.readthedocs.io
Other
234 stars 70 forks source link

New constructor that pass a reference to eve when initializing the auth object #67

Closed joystein closed 9 years ago

joystein commented 9 years ago

Otherwise have to (as far as i can see) do:

app = Eve(
    settings=SETTINGS_PATH,
    validator=ValidatorSQL,
    data=SQL,
    auth=TokenAuth,
)

# NOTE: This is a bit convoluted. TokenAuth needs reference to db.
TokenAuth.app = app
goneri commented 9 years ago

Thanks for the PR @joystein. I'm not the maintainer of this repository but I've some few comments.

Commit 21995c25fc207bb0896dfc6a2c844b34c3d5cfb3 is just a fix for the previous one. You should just merge it in the previous commit. Your branch commit a merge commit (e6611a6), you should rebase your branch against upstream master instead of using merge.

The pep8 failure can easily be fixed. Here again, adjust the existing commit (e.g: git commit --amend). Also, a new unit-test would be a plus.

joystein commented 9 years ago

Thanks for reviewing @goneri. How does it look now?

amleczko commented 9 years ago

I don't have problems with accessing db from my TokenAuth. Can you show your implementation of it?

joystein commented 9 years ago

It looks like this:

from eve.auth import TokenAuth

class TokenAuth(TokenAuth):

    def __init__(self, app):
        self.app = app

    def check_auth(self, token, allowed_roles, resources, method):

        # Verify if the token is valid
        username = User.verify_auth_token(token)
        user = self.app.data.driver.session.query(User).filter_by(username=username).one()
        self.set_request_auth_value(user.id)

        if username:
            return True
        else:
            return False

Would you mind showing how you would do it?

amleczko commented 9 years ago

Well it's part of the documentation - http://eve-sqlalchemy.readthedocs.org/en/stable/tutorial.html#authentication-example

joystein commented 9 years ago

There you have app available because its instantiated in the same file as TokenAuth is defined, right?. I have moved TokenAuth to a separate auth module file as that way of organizing files made more sense in my project.

The approach is the same as for data and media parameters in the eve app constructor: https://github.com/nicolaiarocci/eve/blob/develop/eve/flaskapp.py#L144 https://github.com/nicolaiarocci/eve/blob/develop/eve/flaskapp.py#L148

amleczko commented 9 years ago

Well you can use:

from flask import current_app as app

Not that I'm a big fan of this approach - but I'm also using it in one of my projects.

joystein commented 9 years ago

Right. So this is the flask way of doing things then. I'm new to flask so please excuse my ignorance.

Ideally one would rather have changed auth() to auth(self) in https://github.com/nicolaiarocci/eve/blob/develop/eve/flaskapp.py#L152 to achieve the same.