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

Identifying owner of APIKey #20

Closed
camflan opened this issue Apr 3, 2019 · 13 comments
Closed

Identifying owner of APIKey #20

camflan opened this issue Apr 3, 2019 · 13 comments
Labels
question Further information is requested

Comments

@camflan
Copy link

camflan commented Apr 3, 2019

This looks like a great lib, though I have a few questions.

It appears as though this lib is positioned as an authorization lib, not authentication. As such, I assume that the authentication should happen before ApiKey creation? And because of that, I can assume it's authenticated when used to make a call to the API?

What's the best way to identify the owner of the ApiKey so I can manage which resources a particular request has access to? Is it as simple as using a FK to link this ApiKey to a user/organization/etc and then use that for filtering the querysets of the resources?

@ssrebelious
Copy link

ssrebelious commented Apr 3, 2019

I assume that the authentication should happen before ApiKey creation?

It is not necessary so. You can create custom endpoint for key generation. For my project I created endpoint that allows unauthorized access and generates and returns the key to the caller. In the background this endpoint launches a process that grants access to the user to certain records in the database.

Here is the example of filtering for the version 0.3.0 of the library.

from rest_framework_api_key.settings import TOKEN_HEADER

class MyView(ListAPIView):
    def get_queryset(self):
        token = self.request.META[TOKEN_HEADER]
        queryset = MyModel.objects.filter(clients__token=token)   # `clients` is M2M relation here
        return queryset

@florimondmanca
Copy link
Owner

florimondmanca commented Apr 3, 2019

It appears as though this lib is positioned as an authorization lib, not authentication

Hmm. Maybe this is a poor choice of words on my end.

If you think about it, API keys don't have a built-in permission system (like OAuth does with scopes): they're simply an identifier that allows to access a resource based on the hypothesis that the person who sent the API key with the request is the same than the person who was given that API key in the first place.

But it's not authentication either because we can't relate a particular user to the API key — precisely because the system that uses the API key does not have a Django User account.

In this sense it's probably more an identification system than anything else. Do you think this term could help avoid ambiguity?

Is it as simple as using a FK to link this ApiKey to a user/organization/etc and then use that for filtering the querysets of the resources?

I believe this should do the trick!

I'm curious about the actual use case you have here — you're welcome to share more details if you can. :-)

@camflan
Copy link
Author

camflan commented Apr 3, 2019

@florimondmanca yep, no problem. I understand where you're coming from. We are getting into some partner integrations that will connect to our platform to read data. However, these partners will only be able to read a subset of our endpoints, and within those, a subset of the resources. This is based on service tiers and also which of our users have selected the partners for integration.

For example, I'd like to have X partner have access to read from our /locationsendpoint, but only for the customers who have opted in for this service.

It sounds like, a. "authentication" will happen when I generate the key/secret and give it to them. And then "authorization" will happen based on how I manage the ApiKey relationship -- like, attach ApiKeys to a Partner model, and then a M2M (or another relationship) from Partner to opted-in customers/etc and then I can filter the queryset based on the ApiKey match on those customer objects. Hopefully that's not too convoluted or slow.

Does that sound like a not-bad idea? :)

@camflan
Copy link
Author

camflan commented Apr 3, 2019

@ssrebelious cool, I think I'm on the same page 👍

@florimondmanca
Copy link
Owner

florimondmanca commented Apr 3, 2019

^ Oops!

@camflan So, for example, user A says "I want to enable integration with Partner X", which means Partner X will access resources related to user A (and that of all other users who, like user A, have enabled the integration), and they'll do that using their (Partner X's) API key?

I tried to convert that to code:

from django.db import models
from django.contrib.auth.models import AbstractUser
from rest_framework.generics import ListAPIView
from rest_framework_api_key.models import APIKey

class Partner(models.Model):
    api_key = models.OneToOneField(APIKey, on_delete=models.PROTECT)
    ...

class User(AbstractUser):
    # Enabled by the user
    integrations = models.ManyToManyField(Partner)
    ...

class Resource(models.Model):
    owner = models.ForeignKey(User)
    ...

class ResourceListView(ListAPIView):
    # Endpoint accessed by a partner
    permission_classes = [HasAPIKey]

    def get_queryset(self):
        token = self.request.META["Api-Token"]
        return Resource.objects.filter(owner__integrations__api_key__token=token)

That .filter() clause at the end looks pretty nasty; I'm not sure how it would behave from a performance point of view, to be honest.

I think you could simplify the query by having partners send their partner_id (i.e. the primary key; this could also be a generated string instead of the default auto-incremented integer) along with the API key. This could be in another custom header. That way, I think you could use .filter(owner__integrations=partner_id) instead. I think it's also a better separation of concerns in terms of API access vs which users' resources the partner has access to. :-)

@camflan
Copy link
Author

camflan commented Apr 3, 2019

@florimondmanca Yep, that's really close to what I was thinking, schema/pseudocode wise. There are some ways to shortcut it like storing a bitfield in redis of userIds that are enabled per ApiKey or Partner, etc. Then you can just say, getbit partners:12324:integration:users <user_pk>. If it's 1, good to go. This would be super fast, but increases complexity....though it might be totally worth it, because... while I think you're on a good track with the partner-id, I worry that the ability to just send a different partner-id to get access to other data is a bit worrying from a security aspect. And I don't want to play security by obfuscation by having them send a hashid of some sort, etc.

thoughts?

@florimondmanca
Copy link
Owner

I worry that the ability to just send a different partner-id to get access to other data is a bit worrying from a security aspect.

Yes, that’s a real no-no in retrospect!

As a middle ground you could simply retrieve the Partner that corresponds to the given API token (provided there is a 1-1 match between the two), and the filter query would basically be the same as my last suggestion. You’d need an extra database query to get the Partner but that would do the trick IMO.

I’m not familiar with Redis bitfields but that does sound like a very performant alternative indeed.

@camflan
Copy link
Author

camflan commented Apr 3, 2019

Ah, yeah. Duh - instead of a big join that’d be way easier. Good call 😎

I’m going to give this a shot tonight 👍🏻

@florimondmanca
Copy link
Owner

@camflan Did you manage to make this work? I'm wondering if we should document this pattern somewhere — your experience could be useful in that regard. :-)

@camflan
Copy link
Author

camflan commented Apr 7, 2019

I haven’t had a chance yet. I’ll definitely report back once I get it going and then we can add an example 👌🏼

@florimondmanca florimondmanca added the question Further information is requested label Apr 9, 2019
@camflan
Copy link
Author

camflan commented Apr 29, 2019

@florimondmanca so, this does work but I'm not super in love with it. I'm going to keep going with it and see about refactoring/improving it later on or if I have an epiphany while finishing

Here's an example,

models,

class IntegrationPartner(models.Model):
    api_key = models.OneToOneField(APIKey, on_delete=models.PROTECT)
    name = models.CharField(max_length=255,)

class Organization(models.Model):
    integrations = models.ManyToManyField(Partner)
    name = models.CharField(max_length=255,)

views,

class PartnerAPIView(APIView):
    permission_classes = [permissions.IsAuthenticated | HasAPIKey]
    renderer_classes = (JSONRenderer,)

    def get_partner(self):
        api_key = self.request.META.get("HTTP_X_API_KEY", None)
        partner = None

        if api_key:
            try:
                partner = IntegrationPartner.objects.get_by_api_key(api_key)
            except IntegrationPartner.DoesNotExist:
                pass

        return (api_key is not None, partner)


class OrganizationView(PartnerAPIView):
    def get(self, request):
        user = request.user

        qs = Organization.objects.exclude(is_active=False)

        if user.is_authenticated:
            return qs.filter(users=user)

        (_, partner) = self.get_partner()
        return qs.filter(integrations=partner)


class OrgUser(PartnerAPIView):
    def get(self, request):
        user = request.user

        qs = OrgUser.objects.exclude(organization__is_active=False)

        if user.is_authenticated:
            return qs.filter(organization__users=user)

        (_, partner) = self.get_partner()
        return qs.filter(organization__integrations=partner)

I also experimented with extending HasAPIKey permissions to add a has_object_permissions implementation, but since I needed to filter lists from the parent queryset anyways, I removed this for now - might be useful for model detail APIs? /shrug

@florimondmanca
Copy link
Owner

@camflan Thanks a ton for getting back to this!

It’s good to see this solution is working out. Why are you not very happy about it? Does it feel hacky, or does it induce boilerplate?

I guess if we tried to implement this over several models we would start to see a pattern emerging.

For example, maybe we could add some model (or manager) mixin to aid with traversing relationships via an API key. This way we could encapsulate retrieving the API key fro headers and the like.

@florimondmanca
Copy link
Owner

Hi, I'm closing this for housekeeping purposes. I think the upcoming features in v1.3 (the abstract API key model and base permissions in particular) should help with this kind of use case. :-) See #42. Thanks!

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

No branches or pull requests

3 participants