salopensource / sal

Modular reporting for Endpoints
Apache License 2.0
212 stars 64 forks source link

Use HTTP basic authentication for API #291

Open lfaraone opened 6 years ago

lfaraone commented 6 years ago

The usage of Publickey and Privatekey headers is non-standard -- it also makes it very difficult to test the API in a browser or from JavaScript.

Ideally, SAL would support HTTP Basic authentication, which would allow using http://pub:priv@sal.example.com-style URLs, or the -u pub:priv syntax in cURL.

grahamgilbert commented 6 years ago

What are your thoughts on this @sheagcraig ? I’m thinking it should be one or the other. This could either be a setting, but I’m leaning towards the v3 api when we bump to Sal 4.

sheagcraig commented 6 years ago

I've never been sure why the token approach was used, although I've never seen it as unworkable. HTTP Basic would be a lot easier in some regards, and it's super easy to set up in DRF.

I think ultimately if @grahamgilbert thinks it's a safe thing to do, then we just need to start talking about it in documentation that we're going to do it and to prepare for a v3 API so it doesn't break anything for anyone.

grahamgilbert commented 6 years ago

The main consideration is not breaking things. I’m thinking this may be a decent opportunity to introduce user level api keys that only have access to business units the user does. I should probably group this around a v3 api milestone really.

I would like to retain api keys as saml installs don’t have passwords.

Let’s get this on the board after we’ve finished the python 3 / Django 2 migration.

jokdbx commented 6 years ago

Using users as api keys/token would make it so that the API can be scoped to a BU or given specific permissions within the dataset. as it stands, the api token gives you access to everything in sal (with just ro/rw options)

grahamgilbert commented 6 years ago

Yes, that was what I was getting at. An additional model to the present god mode (system level if you want) api keys.

sheagcraig commented 4 years ago

I'm putting together a group of features and cleanup as a plan to work towards the next major release. If we swapped out the current API auth for Django Rest Framework's builtin TokenAuthentication, would that work for you guys? https://www.django-rest-framework.org/api-guide/authentication/#tokenauthentication

We would add settings views for granting/revoking tokens that we can figure out in more detail based on everyone's needs. Probably something like no tokens by default, GA can give a user a token.

Then we can use the existing decorators in Sal to protect API endpoints so that a user making an API request with a token will have their Machine results filtered by their user's business unit privileges. This will affect all foreignkey related tables as well.

Somewhat related, I'm looking at moving the device checkin into the API, so there will be some automation for creating API tokens for checkin that is basically the same end-result (i.e. auth = "sal:")

KevinHock commented 3 years ago

Just to re-bump this issue, is it something y'all think will be worked on in the future?

Cheers.

grahamgilbert commented 3 years ago

It might be, no immediate plans to add it.