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

Retrieving the raw API key from within a view #98

Open
florimondmanca opened this issue Jan 28, 2020 · 3 comments
Open

Retrieving the raw API key from within a view #98

florimondmanca opened this issue Jan 28, 2020 · 3 comments
Labels
enhancement Enhancement of an existing feature good first issue Good for newcomers

Comments

@florimondmanca
Copy link
Owner

florimondmanca commented Jan 28, 2020

Is your feature request related to a problem? Please describe.
Currently, it's not obvious how we can access the raw API key from within a view that's protected behind a permission classes. (This may be useful in order to e.g. fetch resources associated to that key).

Users can access the request headers directly:

scheme, _, key = request.META["HTTP_AUTHORIZATION"].partition(" ")

but this essentially duplicates logic that's already defined on the HasAPIKey permission class applied to the view.

The only way to reuse that logic is to instantiate the permission class and call into .get_key()… But it doesn't feel very natural, does it?

class ProjectListView(APIView):
    permission_classes = [HasProjectAPIKey]

    def get(self, request: HttpRequest):
        key: str = HasProjectAPIKey().get_key(request)
        # ...

Describe the solution you'd like

Make .get_key() a class method, so that we can reuse it within a view a bit more naturally:

class ProjectListView(APIView):
    permission_classes = [HasProjectAPIKey]

    def get(self, request: HttpRequest):
        key: str = HasProjectAPIKey.get_key(request)
        # ...

Is this a breaking change? Luckily, it doesn't seem to be:

  • A class method is okay to use on an instance, so if people were already using the first way (HasProjectAPIKey().get_key()), they'll still be able to do it if we switch to a class method.
  • If people customized .get_key() (see Customization: Key parsing) and they defined it as an instance method, then surely they know how they're going to use it, and changing it on the base won't break anything for them anyway (even if they call super().get_key(), since that would also work within an instance method).

Describe alternatives you've considered

There are a couple of other ways we could have gone here, none of which are really okay:

  • Have BaseAPIKey set a magic .current_api_key (or similar) attribute on the view instance/class. We could make it work with type-checking, but it would lead to all sorts of weird edge cases and possible bugs related to shared view state. Also it probably wouldn't work with function-based views.
  • Introduce an APIKeyMiddleware that basically calls .get_key() and sets it on the request instance. This isn't great, because a) it would break type-checking (we're adding a new dynamic attribute on the request that isn't known to django-stubs), and b) it would expose the plaintext API key anywhere the request is accessible, which is a massive potential security hole if the developer isn't careful.
  • Introduce an APIKeyViewMixin that injects a .get_api_key(request) method on the view. That method would inspect the view's permission_classes. The problem is: there might be multiple API key permission classes set on the view, so which one would we use? "The one for which the API key is valid" won't work, because all permissions need to pass for a request to be authorized… So, nope.

The only sensible thing is to let the developer explicitly retrieve the raw API key from the request, using exact the permission class they want to use.

Additional context
Prompted by #93 (comment)

@florimondmanca florimondmanca added good first issue Good for newcomers enhancement Enhancement of an existing feature labels Jan 28, 2020
@florimondmanca florimondmanca changed the title BaseAPIKey.get_key() should be a class method Retrieving the raw API key from within a view Jan 28, 2020
@bodgerbarnett
Copy link

Just a quick followup on this. In order to be able to use the API key to filter objects on a particular model, I think I'm right in saying that, currently, the only way to do this is:-

def get_queryset(self):
    key = HasProjectAPIKey().get_key(self.request)
    project = ProjectAPIKey.objects.get_from_key(key).project
    return super().get_queryset().filter(project=project)

Is there a simpler way? Or are there plans to implement one?

@florimondmanca
Copy link
Owner Author

Hi @bodgerbarnett,

Right, that looks like the way to do this right now.

This issue is quite old so it can definitely benefit from more informed challenging.

What API would you expect to do this kind of filtering based on the request API key? Is there a way we could achieve this by doing something different (perhaps more effective, with less duplicate computation / SQL queries?) than what you’ve suggested under the hood?

@bodgerbarnett
Copy link

I'm not sure what the ideal way to go is, but for the time being, I've created my own ProjectAPIKeyManager with a single method which I can pass a request into and get a ProjectAPIKey out of:-

from rest_framework_api_key.models import APIKeyManager


class ProjectAPIKeyManager(APIKeyManager):
    def get_from_request(self, request):
        from .permissions import HasProjectAPIKey

        key = HasProjectAPIKey().get_key(request)

        if key:
            return self.get_from_key(key)

It works for me at this stage.

The awkward thing seems to be that I've obviously enforced the HasProjectAPIKey permission on the view but then I need to use that class to get the actual key later on but hey, it works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement of an existing feature good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants