Open maxachis opened 1 month ago
@maxachis It seems like flask-login
is complementary to authlib
, so that's good. Just dropping the link to plans for the broader auth refactor: https://github.com/Police-Data-Accessibility-Project/data-sources-app/issues/318
I think this issue is a duplicate of #322 , yes? Do you want to close this and add the description to that issue, or am I reading that wrong?
I think this issue is a duplicate of https://github.com/Police-Data-Accessibility-Project/data-sources-app/issues/322 , yes? Do you want to close this and add the description to that issue, or am I reading that wrong?
@josh-chamberlain There's definitely overlap! I think part of it can be consolidated into that, but the question of API Access Tokens is a little more uncertain.
flask-login
can handle any actions taken from the browser. And technically, the cookies provided by flask-login
can be used in non-browser requests, but it's a little less user-friendly than an API key. Hence, I could see a few options:
flask-login
for requests that can only occur through browser, and api keys for non-browser API callsflask-login
for bothflask-login
for neither, and we try something else.I'm also creating a proposal
label to indicate that I am not treating this as WE MUST DO THIS 👿 but more MAYBE WE CAN DO THIS 🧐
@maxachis normally I'm against divergent logic, but the idea of combining them or doing neither seem worse. I think option 1 is my favorite. I think API being governed by encrypted keys / JWT is enough without session management
Yes, that's why I made the new idea
label! I'm just going to merge my label into yours
@josh-chamberlain I'll also want to bring @joshuagraber into this discussion, as he's also quite well-informed on authentication and permissions logic, so his input would be valuable here.
@josh-chamberlain To your earlier point about whether this is a duplicate of #322, I think we can consider #322 to be one component, which would be followed-up by this. I'll direct my attention now to that issue, and then this would follow-up on that.
As far as technical details of the library we use, I'll trust your Python expertise @maxachis. But from my perspective, we should implement the following for auth:
key:value
pair that clients can use to hit any API that does not require user auth. (i.e. data_sources_app:a54os...
or jane_researcher:354no98h32...
)
Blocked by #322 : We should remove Search Tokens before adding in new logic.
Flask-Login is a Flask extension that, per its description, "handles the common tasks of logging in, logging out, and remembering your users' sessions over extended periods of time." It is quite popular, with over 170K users, and has been maintained as recently as a few weeks ago.
If I'm understanding the library correctly, with this logic we can get rid of the access tokens logic as well as the sessions token logic. It will manage sessions automatically, and the access tokens logic can be replaced as described below:
When we log in a user, instead of using an access token, we determine what permissions are assigned to that user, and use that to determine whether they can access a particular endpoint. Through the session management, we maintain a secure state.
This should be able to work in both browser and a requests environment (i.e., via a CURL request). Session/cookies can be preserved between two requests through the methods described here.
We would replace the
@api_required
decorator with a@login_required
decorator (provided by Flask-Login) and a@permissions_required(permissions)
decorator, which would check the user against the permissions assigned to them in the database, and see if they have the permissions passed as the parameter.With this, we should be able to reduce the amount of logic that we have to manage. It will likely make logic such as that with #296 considerably easier.
Things that would need testing include: