Skip to content
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 login #74

Open
mjhea0 opened this issue Oct 13, 2014 · 9 comments
Open

flask login #74

mjhea0 opened this issue Oct 13, 2014 · 9 comments

Comments

@mjhea0
Copy link
Contributor

mjhea0 commented Oct 13, 2014

Flask-Login does not handle authentication at all. In fact, in your example, you are handling authentication manually by verifying the supplied username and password with the username and password in the database.

Thus, this is wrong:

  1. "This extension simplifies the process of handling user sessions and authentication."

This, implies that Flask-Login is handling auth -

  1. "Now we can define the signin view that will handle authentication."

If you agree, I will update the wording in the chapter. And add wording in about ways of handling authentication - username/password, persona (https://github.com/garbados/flask-browserid)

@rpicard
Copy link
Owner

rpicard commented Oct 13, 2014

The extension does not handle auth, but it does simplify the process of handling auth. I think the sentence is valid. How would you change the wording?

@mjhea0
Copy link
Contributor Author

mjhea0 commented Oct 14, 2014

Flask-Login does not touch authentication at all. It only handles session management. This is the exactly the intent of Flask-Login - it handles session management, but you must bring your own method of authentication.

Authentication and managing sessions are two very different things.

I'm not sure on the re-word. I need to review again. I can submit a PR. I would probably say something along the lines of, "Flask-Login handles user session management. However, the extension does not handle authentication, so we must bring our own auth scheme. In the example below, we keep it simple and just authenticate a user based on the username and password."

@rpicard
Copy link
Owner

rpicard commented Oct 14, 2014

Yes, you're right. My bad. Please do submit that PR. The way you worded it there sounds pretty good. I'm not sure about going into other schemes, without fully understanding the security implications myself. I think it's best to leave it as the simple username / password from the database. If you strongly oppose that stance though, feel free to make your argument.

@mjhea0
Copy link
Contributor Author

mjhea0 commented Oct 14, 2014

Will do.

On Mon, Oct 13, 2014 at 7:14 PM, Robert Picard notifications@github.com
wrote:

Yes, you're right. My bad. Please do submit that PR. The way you worded it
there sounds pretty good. I'm not sure about going into other schemes
though, without fully understanding the security implications myself. I
think it's best to leave it as the simple username / password from the
database. If you strongly oppose that stance though, feel free to make your
argument.


Reply to this email directly or view it on GitHub
#74 (comment)
.

@rpicard
Copy link
Owner

rpicard commented Oct 14, 2014

Much obliged.

@mjhea0
Copy link
Contributor Author

mjhea0 commented Oct 14, 2014

You also have an issue with not having an is_active method in the models.

https://flask-login.readthedocs.org/en/latest/_modules/flask/ext/login.html#login_user

On Mon, Oct 13, 2014 at 9:22 PM, Robert Picard notifications@github.com
wrote:

Much obliged.


Reply to this email directly or view it on GitHub
#74 (comment)
.

@mjhea0
Copy link
Contributor Author

mjhea0 commented Oct 14, 2014

You really should have a user model with the following properties in order
for Flask-Login to work correctly out of the box:

def is_authenticated(self):
return True

def is_active(self):
return True

def is_anonymous(self):
return False

def get_id(self):
return unicode(self.id)

https://flask-login.readthedocs.org/en/latest/#your-user-class

Almost forgot - you can get the basic implementation of those methods from this mixin:

https://flask-login.readthedocs.org/en/latest/#flask.ext.login.UserMixin

@mjhea0
Copy link
Contributor Author

mjhea0 commented Oct 14, 2014

Better description?

Flask-Login only handles the session machinery needed to help with logging in and logging out users. It's up to you to figure out the best means of authenticating users (username/password, openid, persona, etc) as well as indicating whether a user is "active" or not.

My thoughts

"Active" and "Authenticated" are relative to the application at hand. That's the power that you get with Flask-Login - it's really agnostic to your app's authentication method.

With that, you have the Flask-Login section nested within the Authentication section. I would suggest moving that out to avoid confusion.

To do

  1. Refactor Flask Login (no authentication, how to update your models for Flask-Login)
  2. Move Flask Login out of Authentication part of the book
  3. Talk about the difference between authentication and session management. Provide links to the Flask extensions that deal with various authentication methods (i.e., Flask-OpenID, Flask-Security, Flask-Auth.)

@rpicard
Copy link
Owner

rpicard commented Oct 17, 2014

I think that all sounds good. Like I said though, I'm hesitant to link to extensions I don't necessarily trust. Let's keep it simple and leave extra auth methods out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants