search

Police-Data-Accessibility-Project / data-sources-app

An API and UI for using and maintaining the Data Sources database
MIT License
2 stars 4 forks source link

v2 auth refactor #318

Open maxachis opened 1 month ago

maxachis commented 1 month ago

Context

This may be able to exist as a replacement for at least some of the current logic in security.py, and may be more extensible and easier to work with than custom code. However, I have not looked into this, and can't testify as to its utility.

https://flask-httpauth.readthedocs.io/en/latest/ https://github.com/lepture/authlib

Related:

322

296

162

196

Requirements

maxachis commented 2 weeks ago

@joshuagraber What do you think of this as an option?

joshuagraber commented 2 weeks ago

Looks pretty solid and well-supported to me, and IMO it's almost always better to use a well-maintained library for auth, given the complexities around it.

Seems like this will handle 1. basic alias/pw auth and 2. providing access tokens to logged in users. @maxachis Do you feel confident that we can pair this strategy for the private endpoints with our own basic auth strategy for the public endpoints?

--

Also, just bc I was thinking about it: we'll need to generate a basic API key for the client at some point. Nbd, but we'll need that before the client can start talking to the API at all.

josh-chamberlain commented 2 weeks ago

related to #296

joshuagraber commented 2 weeks ago

@josh-chamberlain Ah, yes, that changes the calculus. I didn't know we were looking into oauth. We definitely don't want to build an oauth system if we don't have to. There are just at least a thousand and four ways to get it wrong, and it's something we really don't want to get wrong. @maxachis This lib seems to have Flask support for both oauth and standard HTTP strategies, would that be something to look into?

maxachis commented 2 weeks ago

@josh-chamberlain Ah, yes, that changes the calculus. I didn't know we were looking into oauth. Wedefinitely don't want to build an oauth system if we don't have to. There are just at least a thousand and four ways to get it wrong, and it's something we really don't want to get wrong. @maxachis This lib seems to have Flask support for both oauth and standard HTTP strategies, would that be something to look into?

I'm happy to look into it! This will be a ground floor sort of thing, as I've never implemented something like this before (or indeed any sort of authentication thing).

joshuagraber commented 2 weeks ago

I'm happy to look into it! This will be a ground floor sort of thing, as I've never implemented something like this before (or indeed any sort of authentication thing).

Nice, thank you @maxachis. I've done some auth stuff in node before, so if you're wanting input on architecture or high-level conceptual stuff, lmk. I don't feel fluent enough in Python to know how to implement in a Pythonic way, though 😅.

josh-chamberlain commented 2 weeks ago

@maxachis in addition to whoever you know, PDAP has access to a loose group of experts, who can give advice with various levels of detail. If you come up with questions as you work on this, or want help with the planning, we can make it happen with a quickness.

maxachis commented 2 weeks ago

Beginning the research process. Here's some resources I've found that will likely be useful on our quest ⚔️

maxachis commented 2 weeks ago

@josh-chamberlain One thing worth considering now is whether we want multiple options for Authentication, for example:

These could all be associated with a single user in the database. Adds complexity but useful for those who don't have one of the authentication methods (or don't want to use a particular one with us).

In this comment, I believe you indicated the answer is "yes", but I want to double-check to make sure I'm not missing anything.

maxachis commented 2 weeks ago

@maxachis in addition to whoever you know, PDAP has access to a loose group of experts, who can give advice with various levels of detail. If you come up with questions as you work on this, or want help with the planning, we can make it happen with a quickness.

@josh-chamberlain I would want to reach out to some of these experts, as that would help me figure out how to best plan this development.

josh-chamberlain commented 1 week ago

@maxachis my offer was more for specific questions/advice—I would prefer to start the request for a favor with something a little more concrete, if that makes sense.

re: auth options, we should support GitHub and standard email + pw. I would consider adding google if it were really, really easy, but github is actually preferred so I'd rather do that.

I asked a question in C&S slack which I think you can view:

hey y’all! we’re working on an open-source flask app (vue front end), and none of us have set up auth, permissions, that sort of thing before.

  • we need to do basic login for users, and support OAuth with GitHub
  • using JWT to manage sessions
  • leaning toward authlib
  • if it matters, we’re doing attribute/permission-based access rather than role-based

I don’t have a specific ask, other than sharing any tips/advice you might have for doing this right / with minimal headache! We may generate more questions as we go

it resulted in some advice which we can take or leave:

Clearly, you’re relying on identity being separate from your app which is GREAT.

It’s often necessary to have a User class hierarchy with the app-specific stuff separate from the indentity and authentication stuff. Two parallel data structures lead to a bunch of fussy CLI apps to synch them manually.

These are in response to questions like: How long do you retain user history for folks who no longer have working credentials and haven’t logged in recently?

Having a bunch of click -based CLI commands in the code base where User profiles are kept can seem cluttered, but, I recommend keeping the CLI stuff close to the data model definition.

Also. There are rarely any one-time-only user cleanup or user migration or user audit tasks. They tend to live forever, but only get used once or twice a year. Treat them as CLI apps. Unit test. acceptance test. All the things.

joshuagraber commented 1 week ago

Also. There are rarely any one-time-only user cleanup or user migration or user audit tasks. They tend to live forever, but only get used once or twice a year. Treat them as CLI apps. Unit test. acceptance test. All the things.

This makes sense to me. When I've done migration tasks previously, it's been with utility scripts written in bash or node, and setting them up to be called manually from the command line. But...

It’s often necessary to have a User class hierarchy with the app-specific stuff separate from the indentity and authentication stuff. Two parallel data structures lead to a bunch of fussy CLI apps to synch them manually.

...As long as each is tied to a user id, it doesn't really matter how many different classes we use for the data. i.e. The architecture could have a table that stores UserProfile, which includes name, email, etc., and a table that stores UserAuth, which would contain auth-related stuff... I don't think we would need to write scripts to sync all of this stuff. Just update records as we need to. I guess we could run a whole separate DB and service for auth, but I don't think that's necessary unless we're building it ourselves.

joshuagraber commented 1 week ago

re: auth options, we should support GitHub and standard email + pw. I would consider adding google if it were really, really easy, but github is actually preferred so I'd rather do that.

Also, OAuth is spec'd here, so it should be standardized. Not sure how that library handles it, but if we're adding 1 my thinking is that it should be simple enough to add the other. (But if we want to steer people toward using GH rather than Google, maybe we just go with the one?)