singingwolfboy / flask-dance

Doing the OAuth dance with style using Flask, requests, and oauthlib.
https://pypi.python.org/pypi/Flask-Dance/
MIT License
1.01k stars 158 forks source link

Proper way to detect if user is logged in - After doing OAuth Dance #243

Closed KurtKline closed 5 years ago

KurtKline commented 5 years ago

I am implementing the Google multi-user setup where the local user accounts are created automatically when logging in with OAuth. So I am closely following the code from the "Overriding Default Behavior" section of the multi-user setups doc.

Once a user is logged in successfully with OAuth + a user account has been verified / created, is it correct to user the @login_required decorator from Flask-Login to determine if a user is logged in for the rest of the views? Or is using if not current_user.is_authenticated OR if not google.authorized a better solution?

If I try to use if not google.authorized on any view besides the one specified in the blueprint app.register_blueprint(google_blueprint, url_prefix="/login") I get a "ValueError: Cannot get OAuth token without an associated user". However, both @login_required and if not current_user.is_authenticated seem to work for me.

I also noticed in this video tutorial by Pretty Printed uses the @login_required decorator.

Finally, would you recommend a specific package that handles "user roles / role based views" well while also playing nicely with Flask-Login / Flask-Dance?

daenney commented 5 years ago

The @login_required decorator is by far the easiest option if you want to guard a whole view function. Afaik Flask-Dance doesn't provide one though so you'll need to write your own along the lines of:

def login_required(google):
    """Enforces authentication on a route."""
    def decorated(func):
        @wraps(func)
        def decorated_route(*args, **kwargs):
            if not google.authorized:
                return redirect(url_for('google.login'))
            ... we are authorized, do things
            return func(*args, **kwargs)
         return decorated_route
    return decorated

The login_required decorator from Flask-Login gets/retrieves the user ID etc. from the session Flask-Login, not Flask-Dance, created when the person logged in the first time (for example stored in a cookie). So if that requirement passes, it'll never get around to checking anything with the Oauth provider so you won't have an Oauth session meaning google.authorized has no associated user.

From my point of view this is fine. If Flask-Login's @login_required or is_authorized fails, i.e there's no Flask-Login session, it would throw you to the /login page, there it'll do the Oauth dance and you'll have access to google.authorized and once completed a Flask-Login session will be created for that user which you can then use to guard views.

singingwolfboy commented 5 years ago

I generally recommend Flask Principal for role-based management with Flask. As far as I know, it should work just fine with Flask-Dance, Flask-Login, etc.

singingwolfboy commented 5 years ago

@daenney: Flask-Dance does have a built-in version of the login_required decorator: authorization_required. You should be able to use it like this:

google_bp = make_google_blueprint()

@google_bp.session.authorization_required
def restricted_view():
    return "only users with a Google OAuth token can get here"

I was hoping that you could refer to this decorator as @google.authorization_required instead of @google_bp.session.authorization_required, but unfortunately I ran into a chicken-and-egg problem where you can't refer to the google LocalProxy outside of the context of an incoming HTTP request, and decorators are applied outside of that context.

In general, I would recommend using login_required, but if you find a use-case where you need authorization_required, go ahead and use that!

KurtKline commented 5 years ago

@daenney / @singingwolfboy: If I understand correctly, you are both saying that using Flask-Login's built in @login_required decorator should be fine to use, because in order to login (create Flask-Login session) in the first place the user needs to successfully complete the OAuth dance?

@daenney the wrapper function you provided is not necessary correct? Is that only if I am exclusively using Flask-Dance and am not using Flask-Login in my app?

What seems like a good analogy to me is if you go to an event where you have to show your ticket at the entrance and you receive a hand stamp. When you first enter, the ticket (OAuth dance) is required to get you a hand stamp (Flask-Login session). Once you have your hand stamp, you don't need your ticket anymore to access the venue from all entrances (@login_required = hand stamp check). Unless your hand stamp wears off, in which case you would need your ticket again to reauthorize.

I really appreciate the guidance!

singingwolfboy commented 5 years ago

You may also find the flask-dance-multi-provider repo a helpful reference. You'll notice that it uses @login_required to protect the routes that need protecting, specifically because you don't necessarily need to use OAuth to log in -- you can also set a username and password, and log in with that.

singingwolfboy commented 5 years ago

I'm closing this issue due to a lack of updates.