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

Use HTTP basic authentication for API #291

Open
lfaraone opened this issue Oct 16, 2018 · 8 comments
Open

Use HTTP basic authentication for API #291

lfaraone opened this issue Oct 16, 2018 · 8 comments
Labels
enhancement V4 To be added to version 4
Milestone

Comments

@lfaraone
Copy link
Contributor

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:[email protected]-style URLs, or the -u pub:priv syntax in cURL.

@grahamgilbert grahamgilbert added the V4 To be added to version 4 label Oct 16, 2018
@grahamgilbert
Copy link
Member

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
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor

jokdbx commented Oct 16, 2018

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
Copy link
Member

grahamgilbert commented Oct 16, 2018

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

@sheagcraig sheagcraig added this to the 4.0.0 milestone Nov 30, 2018
@sheagcraig
Copy link
Contributor

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
Copy link
Contributor

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

Cheers.

@grahamgilbert
Copy link
Member

It might be, no immediate plans to add it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement V4 To be added to version 4
Projects
None yet
Development

No branches or pull requests

5 participants