-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Use project-scoped temporal tokens to interact with the API from the builders #10378
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
Changes from all commits
0508e22
58aed74
31387db
b966c2e
bc5a648
1cff5bf
49e8eb6
ba7356b
eddc6a4
93a493e
1169421
4a67956
623bf20
d3a5d22
2467464
8878394
4cd5242
7ca4819
c352aa6
9da76fa
475fee7
f261ecc
df55b2c
dd00fc2
5f1dde4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
from django.contrib import admin | ||
from rest_framework_api_key.admin import APIKeyModelAdmin | ||
|
||
from readthedocs.api.v2.models import BuildAPIKey | ||
|
||
|
||
@admin.register(BuildAPIKey) | ||
class BuildAPIKeyAdmin(APIKeyModelAdmin): | ||
raw_id_fields = ["project"] | ||
search_fields = [*APIKeyModelAdmin.search_fields, "project__slug"] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
from django.apps import AppConfig | ||
|
||
|
||
class APIV2Config(AppConfig): | ||
name = "readthedocs.api.v2" | ||
verbose_name = "API V2" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
# Generated by Django 3.2.18 on 2023-05-31 20:40 | ||
|
||
import django.db.models.deletion | ||
from django.db import migrations, models | ||
|
||
|
||
class Migration(migrations.Migration): | ||
initial = True | ||
|
||
dependencies = [ | ||
("projects", "0100_project_readthedocs_yaml_path"), | ||
] | ||
|
||
operations = [ | ||
migrations.CreateModel( | ||
name="BuildAPIKey", | ||
fields=[ | ||
( | ||
"id", | ||
models.CharField( | ||
editable=False, | ||
max_length=150, | ||
primary_key=True, | ||
serialize=False, | ||
unique=True, | ||
), | ||
), | ||
("prefix", models.CharField(editable=False, max_length=8, unique=True)), | ||
("hashed_key", models.CharField(editable=False, max_length=150)), | ||
("created", models.DateTimeField(auto_now_add=True, db_index=True)), | ||
( | ||
"name", | ||
models.CharField( | ||
default=None, | ||
help_text="A free-form name for the API key. Need not be unique. 50 characters max.", | ||
max_length=50, | ||
), | ||
), | ||
( | ||
"revoked", | ||
models.BooleanField( | ||
blank=True, | ||
default=False, | ||
help_text="If the API key is revoked, clients cannot use it anymore. (This cannot be undone.)", | ||
), | ||
), | ||
( | ||
"expiry_date", | ||
models.DateTimeField( | ||
blank=True, | ||
help_text="Once API key expires, clients cannot use it anymore.", | ||
null=True, | ||
verbose_name="Expires", | ||
), | ||
), | ||
( | ||
"project", | ||
models.ForeignKey( | ||
help_text="Project that this API key grants access to", | ||
on_delete=django.db.models.deletion.CASCADE, | ||
related_name="build_api_keys", | ||
to="projects.project", | ||
), | ||
), | ||
], | ||
options={ | ||
"verbose_name": "Build API key", | ||
"verbose_name_plural": "Build API keys", | ||
"ordering": ("-created",), | ||
"abstract": False, | ||
}, | ||
), | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
from datetime import timedelta | ||
|
||
from django.db import models | ||
from django.utils import timezone | ||
from django.utils.translation import gettext_lazy as _ | ||
from rest_framework_api_key.models import AbstractAPIKey, BaseAPIKeyManager | ||
|
||
from readthedocs.projects.models import Project | ||
|
||
|
||
class BuildAPIKeyManager(BaseAPIKeyManager): | ||
def create_key(self, project): | ||
""" | ||
Create a new API key for a project. | ||
|
||
Build API keys are valid for 3 hours, | ||
and can be revoked at any time by hitting the /api/v2/revoke/ endpoint. | ||
""" | ||
expiry_date = timezone.now() + timedelta(hours=3) | ||
return super().create_key( | ||
# Name is required, so we use the project slug for it. | ||
name=project.slug, | ||
expiry_date=expiry_date, | ||
project=project, | ||
) | ||
|
||
|
||
class BuildAPIKey(AbstractAPIKey): | ||
|
||
""" | ||
API key for securely interacting with the API from the builders. | ||
|
||
The key is attached to a single project, | ||
it can be used to have write access to the API V2. | ||
""" | ||
|
||
project = models.ForeignKey( | ||
Project, | ||
on_delete=models.CASCADE, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This adds more overhead to the deletion of the project. We may want to make this |
||
related_name="build_api_keys", | ||
help_text=_("Project that this API key grants access to"), | ||
) | ||
|
||
objects = BuildAPIKeyManager() | ||
|
||
class Meta(AbstractAPIKey.Meta): | ||
verbose_name = _("Build API key") | ||
verbose_name_plural = _("Build API keys") |
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,7 +1,9 @@ | ||||||||||||||||
"""Defines access permissions for the API.""" | ||||||||||||||||
|
||||||||||||||||
from rest_framework import permissions | ||||||||||||||||
from rest_framework_api_key.permissions import BaseHasAPIKey, KeyParser | ||||||||||||||||
|
||||||||||||||||
from readthedocs.api.v2.models import BuildAPIKey | ||||||||||||||||
from readthedocs.builds.models import Version | ||||||||||||||||
|
||||||||||||||||
|
||||||||||||||||
|
@@ -65,3 +67,37 @@ def has_permission(self, request, view): | |||||||||||||||
.exists() | ||||||||||||||||
) | ||||||||||||||||
return has_access | ||||||||||||||||
|
||||||||||||||||
|
||||||||||||||||
class TokenKeyParser(KeyParser): | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Odd this isn't builtin... seems pretty standard? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They default to |
||||||||||||||||
|
||||||||||||||||
""" | ||||||||||||||||
Custom key parser to use ``Token {TOKEN}`` as format. | ||||||||||||||||
|
||||||||||||||||
This is the same format we use in API V3 for auth/authz. | ||||||||||||||||
""" | ||||||||||||||||
|
||||||||||||||||
keyword = "Token" | ||||||||||||||||
|
||||||||||||||||
|
||||||||||||||||
class HasBuildAPIKey(BaseHasAPIKey): | ||||||||||||||||
|
||||||||||||||||
""" | ||||||||||||||||
Custom permission to inject the build API key into the request. | ||||||||||||||||
|
||||||||||||||||
This avoids having to parse the key again on each view. | ||||||||||||||||
The key is injected in the ``request.build_api_key`` attribute | ||||||||||||||||
only if it's valid, otherwise it's set to ``None``. | ||||||||||||||||
""" | ||||||||||||||||
|
||||||||||||||||
model = BuildAPIKey | ||||||||||||||||
key_parser = TokenKeyParser() | ||||||||||||||||
|
||||||||||||||||
def has_permission(self, request, view): | ||||||||||||||||
build_api_key = None | ||||||||||||||||
has_permission = super().has_permission(request, view) | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't fully understand what code this is calling... What is actually being validated here? Is it in our code, or a third-party package? And is it checking the project matches the API key somehow? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We rely on the permission class from the package to make sure that the API key given is valid, we just override it to attach the key to the request, otherwise we need to parse the key and get it from the db every time we want to use from a view. readthedocs.org/readthedocs/api/v2/permissions.py Lines 85 to 91 in f261ecc
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Gotcha, so it's basically just checking that the key exists? Not checking it against any specific user or anything. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, api keys aren't attached to a user, authorization for a given project is done via the queryset. |
||||||||||||||||
if has_permission: | ||||||||||||||||
key = self.get_key(request) | ||||||||||||||||
build_api_key = self.model.objects.get_from_key(key) | ||||||||||||||||
request.build_api_key = build_api_key | ||||||||||||||||
return has_permission |
Uh oh!
There was an error while loading. Please reload this page.