From 667f747417d3d1116b49bd1be1f4dc545ec448a2 Mon Sep 17 00:00:00 2001 From: Payam Date: Fri, 7 Aug 2026 18:28:08 +0400 Subject: [PATCH 1/5] Add organization API keys and an organization-scoped v1 API Organization admins can now issue scoped API keys for their own organization and use them against a new /api/v1/ surface covering enrollment creation, enrollment listing and course listing. One ApiKey model serves both kinds rather than two near-identical models: they share generation, verification, expiry, revocation and listing, and the only real difference is a single field. The kinds are told apart by an explicit key_type column with a database check constraint tying it to the presence of an organization, so a dropped filter cannot silently produce a key with deployment-wide authority. Views take the organization from the key rather than from the URL or body, so a key can only ever act on the organization it was issued for. Storage moves from reversible Fernet encryption to a SHA-256 hash of a 256-bit secret, with the token shaped elk__. The non-secret key_id makes verification one indexed lookup instead of decrypting every candidate row, and replaces the JWT wrapper whose only real function was carrying the row's salt for that lookup. Existing keys keep working: a data migration derives the hash from the stored ciphertext, and legacy JWTs resolve through the same lookup. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 19 ++ django_email_learning/decorators.py | 90 +++++--- .../commands/rotate_encryption_key.py | 7 +- ...0017_apikey_add_type_and_hashed_storage.py | 73 ++++++ .../0018_backfill_api_key_hashes.py | 77 +++++++ .../0019_apikey_drop_encrypted_key.py | 54 +++++ django_email_learning/models/__init__.py | 2 +- django_email_learning/models/api_keys.py | 214 +++++++++++++++-- .../organization_api/__init__.py | 0 .../organization_api/serializers.py | 93 ++++++++ .../organization_api/urls.py | 10 + .../organization_api/views.py | 218 ++++++++++++++++++ .../platform/api/serializers/__init__.py | 11 +- .../platform/api/serializers/misc.py | 71 ++++-- django_email_learning/platform/api/urls.py | 12 + .../platform/api/views/__init__.py | 4 + .../platform/api/views/misc.py | 146 ++++++++++-- django_email_learning/platform/views/misc.py | 27 ++- .../services/api_key_service.py | 103 +++++++++ django_email_learning/urls.py | 4 + docs/source/index.rst | 1 + docs/source/platform/api_keys.rst | 28 ++- .../technical/encryption-key-management.rst | 9 +- docs/source/technical/organization-api.rst | 186 +++++++++++++++ .../platform/settings_api_keys/ApiKeys.jsx | 184 ++++++++------- frontend/src/test/platform/ApiKeys.test.jsx | 94 ++++++-- .../test_views/test_check_imap_job_view.py | 6 +- .../test_cleanup_job_executions_view.py | 6 +- ...eactivate_inactive_enrollments_job_view.py | 6 +- .../test_deliver_contents_job_view.py | 6 +- .../test_job_execution_status_view.py | 6 +- .../test_send_newsletters_job_view.py | 6 +- .../test_send_reminders_job_view.py | 6 +- tests/models/test_api_key.py | 155 +++++++++++++ tests/organization_api/__init__.py | 0 tests/organization_api/conftest.py | 80 +++++++ tests/organization_api/test_authentication.py | 141 +++++++++++ tests/organization_api/test_courses_api.py | 59 +++++ .../organization_api/test_enrollments_api.py | 181 +++++++++++++++ .../api/test_views/test_api_key_view.py | 96 ++++++-- .../test_organization_api_key_view.py | 173 ++++++++++++++ 41 files changed, 2438 insertions(+), 226 deletions(-) create mode 100644 django_email_learning/migrations/0017_apikey_add_type_and_hashed_storage.py create mode 100644 django_email_learning/migrations/0018_backfill_api_key_hashes.py create mode 100644 django_email_learning/migrations/0019_apikey_drop_encrypted_key.py create mode 100644 django_email_learning/organization_api/__init__.py create mode 100644 django_email_learning/organization_api/serializers.py create mode 100644 django_email_learning/organization_api/urls.py create mode 100644 django_email_learning/organization_api/views.py create mode 100644 django_email_learning/services/api_key_service.py create mode 100644 docs/source/technical/organization-api.rst create mode 100644 tests/models/test_api_key.py create mode 100644 tests/organization_api/__init__.py create mode 100644 tests/organization_api/conftest.py create mode 100644 tests/organization_api/test_authentication.py create mode 100644 tests/organization_api/test_courses_api.py create mode 100644 tests/organization_api/test_enrollments_api.py create mode 100644 tests/platform/api/test_views/test_organization_api_key_view.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b666454..5304112c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,25 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm Changes prior to v1.0.0 are available in the [git history](https://github.com/AvaCodeSolutions/django-email-learning/commits/master). +## [Unreleased] + +### Added + +- **Organization API keys and a new organization-scoped API** — Organization admins can now issue API keys for their own organization via `POST /api/platform/organizations//api-keys/`, and use them against a new `/api/v1/` surface: create an enrollment (`POST /api/v1/enrollments/`), list enrollments (`GET /api/v1/enrollments/`), and list courses (`GET /api/v1/courses/`). Keys carry explicit scopes — `enrollments:write`, `enrollments:read`, `courses:read` — and an optional expiry. The organization is taken from the key itself rather than from the URL or request body, so a key can only ever act on the organization it was issued for; a slug or id belonging to another organization reads as `404`. Only organization *admins* can issue keys, since a key acts with whatever scopes it carries. Requests are rate limited per key (defaults 120/60s, configurable via `DJANGO_EMAIL_LEARNING["ORGANIZATION_API_RATE_LIMITS"]`). This is separate from the existing unauthenticated `/api/public/` embed surface. Management is API-only for now — there is no organization-facing settings screen for these keys yet. See the new [Organization API](https://django-email-learning.readthedocs.io/en/latest/technical/organization-api.html) reference. +- **API keys now support naming, expiry, revocation and last-used tracking** — Both platform and organization keys take a `name` and an optional `expires_at`, record `last_used_at` on each authenticated request (at minute resolution, so recording activity doesn't cost a write per request), and can be revoked. + +### Security + +- **API keys are stored hashed instead of reversibly encrypted** (breaking) — Keys were held as Fernet ciphertext under `ENCRYPTION_SECRET_KEY`, which meant anyone with the database and that setting could recover every key in plaintext; the listing endpoint and the settings UI both did exactly that on every page load. Keys are now stored as a SHA-256 hash of a 256-bit random secret and the full token is returned **once**, at creation. The token format is `elk__`, where `key_id` is a non-secret public identifier that makes verification a single indexed lookup instead of decrypting every candidate row. **Existing keys keep working** — a data migration derives the new hash from the stored ciphertext, and the pre-3.1.0 JWT resolves through the same lookup — but they can no longer be *read back*: the API Keys page now shows each key's `key_id` and status rather than the key itself, and an operator who has lost a key must issue a replacement. Support for the legacy JWT format will be removed in a future release. +- **The JWT wrapper around API keys is no longer issued** (breaking) — Keys were handed out as a JWT signed with `JWT_SECRET_KEY` and a fixed `exp` of `datetime.max`. It never expired and carried no claim that wasn't already the credential; its only real function was smuggling the row's `salt` to narrow the old decrypt-and-compare lookup, which the new `key_id` does directly. New keys are issued as the bare token. Existing JWTs are still accepted. +- **Organization keys cannot reach platform endpoints, and vice versa** — The two kinds are distinguished by an explicit `key_type` column with a database check constraint tying it to the presence of an organization, rather than by inferring "platform" from the organization being null. A dropped filter therefore cannot silently produce a key with deployment-wide authority; `check_api_key` asserts the platform type positively, and the job-trigger endpoints return `403` for an organization key. + +### Changed + +- **`POST /api/platform/api_keys/` returns `token` instead of `key`** (breaking) — The creation response now carries the full token as `token`, alongside metadata including `key_id`, `name`, `key_type`, `scopes`, `expires_at`, `revoked_at` and `last_used_at`. `GET /api/platform/api_keys/` returns that same metadata and **never** returns a usable credential; it is also now filtered to platform keys only. Anything reading `key` from either response needs updating. +- **`DELETE /api/platform/api_keys//` revokes rather than deletes** (breaking) — The row is retained with `revoked_at` set, so the audit trail of which keys existed and when each was last used survives. The response message changed from `"API Key deleted successfully"` to `"API Key revoked successfully"`. Revoked keys fail authentication with `401`. +- **`rotate_encryption_key` no longer processes API keys** — Hashes cannot be re-encrypted and do not need rotating. The command still rotates `ImapConnection.password`. Rotating `ENCRYPTION_SECRET_KEY` no longer invalidates API keys. + ## [3.0.0] - 2026-08-05 > **A deliberately small major.** Nothing here requires a migration on the scale of 2.0.0 — for most projects the upgrade is a no-op. The major bump reflects that two changes alter behavior existing callers can observe, not that the release is large. Read the two entries below if you query organizations by name or render the `Organization` model directly. diff --git a/django_email_learning/decorators.py b/django_email_learning/decorators.py index 8e257573..badc3f0c 100644 --- a/django_email_learning/decorators.py +++ b/django_email_learning/decorators.py @@ -5,11 +5,11 @@ from django.http import JsonResponse from django_email_learning.apps import PLATFORM_ADMIN_GROUP_NAME -from django_email_learning.models import ApiKey, OrganizationUser -from django_email_learning.services.jwt_service import ( - ExpiredTokenException, - InvalidTokenException, - decode_jwt, +from django_email_learning.models import ApiKeyType, OrganizationUser +from django_email_learning.services.api_key_service import ( + ApiKeyAuthenticationError, + authenticate_token, + extract_bearer_token, ) @@ -139,37 +139,67 @@ def _wrapped_view(request, *view_args, **view_kwargs) -> JsonResponse: # type: def check_api_key() -> typing.Callable: + """Authenticates a *platform* API key. + + The key type is asserted positively rather than inferred from the key + having no organization: these endpoints act deployment-wide, so an + organization key must never reach them by default. + """ + + def decorator(view_func: typing.Callable) -> typing.Callable: + @wraps(view_func) + def _wrapped_view(request, *view_args, **view_kwargs) -> JsonResponse: # type: ignore[no-untyped-def] + try: + api_key = authenticate_token(extract_bearer_token(request)) + except ApiKeyAuthenticationError as e: + return JsonResponse({"error": e.message}, status=e.status) + + if api_key.key_type != ApiKeyType.PLATFORM: + return JsonResponse({"error": "Forbidden"}, status=403) + + request.api_key = api_key + return view_func(request, *view_args, **view_kwargs) + + return _wrapped_view + + return decorator + + +def require_organization_api_key(scopes: typing.Iterable[str] = ()) -> typing.Callable: + """Authenticates an *organization* API key carrying all of `scopes`. + + Binds `request.api_key` and `request.organization`. The organization is + taken from the key itself; a view must read it from there rather than from + the URL, or a caller could act on an organization its key doesn't cover. + Where a URL does name an organization it has to agree with the key. + """ + required_scopes = set(scopes) + def decorator(view_func: typing.Callable) -> typing.Callable: @wraps(view_func) def _wrapped_view(request, *view_args, **view_kwargs) -> JsonResponse: # type: ignore[no-untyped-def] - authorization_header = request.headers.get("Authorization") - if not authorization_header: - return JsonResponse({"error": "Authorization header missing"}, status=401) - authorization_header_parts = authorization_header.split(" ") - if len(authorization_header_parts) != 2 or authorization_header_parts[0] != "Bearer": + try: + api_key = authenticate_token(extract_bearer_token(request)) + except ApiKeyAuthenticationError as e: + return JsonResponse({"error": e.message}, status=e.status) + + if api_key.key_type != ApiKeyType.ORGANIZATION: + return JsonResponse({"error": "Forbidden"}, status=403) + + missing_scopes = required_scopes - set(api_key.scopes) + if missing_scopes: return JsonResponse( - {"error": "Invalid Authorization header format. Expected: Bearer "}, - status=401, + {"error": f"API key is missing required scope(s): {', '.join(sorted(missing_scopes))}"}, + status=403, ) - api_key = authorization_header_parts[1] - try: - key_data = decode_jwt(api_key) - possible_keys = ApiKey.objects.filter(salt=key_data["salt"]) - key_matched = False - for possible_key in possible_keys: - key_value = possible_key.decrypt_password(possible_key.key) - if key_value == key_data["key"]: - key_matched = True - break - if not key_matched: - return JsonResponse({"error": "Invalid API key"}, status=401) - except ExpiredTokenException: - return JsonResponse({"error": "Expired Json Web Token"}, status=401) - except InvalidTokenException: - return JsonResponse({"error": "Invalid Json Web Token"}, status=401) - except KeyError: - return JsonResponse({"error": "Json Web Token missing required fields"}, status=401) + # 404 rather than 403 for a mismatch: a key holder shouldn't be able + # to probe which other organization ids exist. + if "organization_id" in view_kwargs and view_kwargs["organization_id"] != api_key.organization_id: + return JsonResponse({"error": "Not found"}, status=404) + + request.api_key = api_key + request.organization = api_key.organization return view_func(request, *view_args, **view_kwargs) return _wrapped_view diff --git a/django_email_learning/management/commands/rotate_encryption_key.py b/django_email_learning/management/commands/rotate_encryption_key.py index fc5a99d4..660724f8 100644 --- a/django_email_learning/management/commands/rotate_encryption_key.py +++ b/django_email_learning/management/commands/rotate_encryption_key.py @@ -4,7 +4,7 @@ from django.core.management.base import BaseCommand, CommandParser, OutputWrapper from django.db import transaction -from django_email_learning.models import ApiKey, ImapConnection +from django_email_learning.models import ImapConnection from django_email_learning.models.mixin_models import EncryptionMixin logger = logging.getLogger(__name__) @@ -96,10 +96,11 @@ def handle(self, *args, **options) -> None: # type: ignore[no-untyped-def] if dry_run: self.stdout.write(self.style.WARNING("Running in DRY-RUN mode — no changes will be written.")) - # Models and their encrypted field + # Models and their encrypted field. ApiKey used to appear here; it now + # stores only a hash of its secret, which by definition cannot be + # re-encrypted under a new key and needs no rotation. targets: list[tuple[type[EncryptionMixin], str]] = [ (ImapConnection, "password"), - (ApiKey, "key"), ] total = 0 diff --git a/django_email_learning/migrations/0017_apikey_add_type_and_hashed_storage.py b/django_email_learning/migrations/0017_apikey_add_type_and_hashed_storage.py new file mode 100644 index 00000000..78b20556 --- /dev/null +++ b/django_email_learning/migrations/0017_apikey_add_type_and_hashed_storage.py @@ -0,0 +1,73 @@ +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + """Adds the new ApiKey columns as nullable so 0018 can backfill them. + + Split across three migrations because the backfill in 0018 has to read the + old encrypted `key` column, which 0019 then drops. + """ + + dependencies = [ + ("django_email_learning", "0016_alter_organization_name"), + ] + + operations = [ + migrations.AddField( + model_name="apikey", + name="key_type", + field=models.CharField( + choices=[("platform", "Platform"), ("organization", "Organization")], + db_index=True, + max_length=20, + null=True, + ), + ), + migrations.AddField( + model_name="apikey", + name="organization", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="api_keys", + to="django_email_learning.organization", + ), + ), + migrations.AddField( + model_name="apikey", + name="name", + field=models.CharField(max_length=100, null=True), + ), + migrations.AddField( + model_name="apikey", + name="key_id", + field=models.CharField(editable=False, max_length=32, null=True), + ), + migrations.AddField( + model_name="apikey", + name="secret_hash", + field=models.CharField(editable=False, max_length=64, null=True), + ), + migrations.AddField( + model_name="apikey", + name="scopes", + field=models.JSONField(blank=True, default=list), + ), + migrations.AddField( + model_name="apikey", + name="expires_at", + field=models.DateTimeField(blank=True, null=True), + ), + migrations.AddField( + model_name="apikey", + name="revoked_at", + field=models.DateTimeField(blank=True, null=True), + ), + migrations.AddField( + model_name="apikey", + name="last_used_at", + field=models.DateTimeField(blank=True, null=True), + ), + ] diff --git a/django_email_learning/migrations/0018_backfill_api_key_hashes.py b/django_email_learning/migrations/0018_backfill_api_key_hashes.py new file mode 100644 index 00000000..7b3db951 --- /dev/null +++ b/django_email_learning/migrations/0018_backfill_api_key_hashes.py @@ -0,0 +1,77 @@ +import base64 +import hashlib +import secrets +import sys + +from cryptography.fernet import Fernet, InvalidToken +from cryptography.hazmat.primitives import hashes +from cryptography.hazmat.primitives.kdf.pbkdf2 import PBKDF2HMAC +from django.conf import settings +from django.db import migrations + + +def _fernet(salt: str) -> Fernet: + """Reimplements EncryptionMixin._fernet as it stood before this release. + + Inlined rather than imported: the model method is removed by 0019, and a + data migration must keep working against the code of its own era rather + than whatever the model looks like today. + """ + kdf = PBKDF2HMAC( + algorithm=hashes.SHA256(), + length=32, + salt=salt.encode(), + iterations=100000, + ) + secret = str(settings.DJANGO_EMAIL_LEARNING["ENCRYPTION_SECRET_KEY"]) + return Fernet(base64.urlsafe_b64encode(kdf.derive(secret.encode()))) + + +def backfill(apps, schema_editor) -> None: # type: ignore[no-untyped-def] + """Derives key_id/secret_hash for existing keys from the encrypted column. + + Existing credentials keep working: the pre-3.1 JWT carries the raw key, and + the new authentication path hashes whatever it extracts from that JWT into + the same `secret_hash` written here. + + A key whose ciphertext won't decrypt — the usual cause is a rotated + ENCRYPTION_SECRET_KEY — gets a random hash no token can ever match, so the + row survives the non-null constraint in 0019 while failing closed. The + alternative, deleting it, would silently widen access if that key was the + only thing gating an endpoint. + """ + ApiKey = apps.get_model("django_email_learning", "ApiKey") + undecryptable = [] + + for api_key in ApiKey.objects.all(): + try: + plaintext = _fernet(api_key.salt).decrypt(api_key.key.encode()).decode() + secret_hash = hashlib.sha256(plaintext.encode()).hexdigest() + except (InvalidToken, ValueError, TypeError): + undecryptable.append(api_key.pk) + secret_hash = hashlib.sha256(secrets.token_urlsafe(32).encode()).hexdigest() + + api_key.key_type = "platform" + api_key.key_id = secrets.token_hex(12) + api_key.secret_hash = secret_hash + api_key.name = f"Legacy platform key #{api_key.pk}" + api_key.scopes = [] + api_key.save(update_fields=["key_type", "key_id", "secret_hash", "name", "scopes"]) + + if undecryptable: + print( + f"\n WARNING: {len(undecryptable)} API key(s) could not be decrypted " + f"(ids: {', '.join(str(pk) for pk in undecryptable)}). They have been " + f"left in place but can no longer authenticate; issue replacements.", + file=sys.stderr, + ) + + +class Migration(migrations.Migration): + dependencies = [ + ("django_email_learning", "0017_apikey_add_type_and_hashed_storage"), + ] + + # Irreversible in substance rather than in form: 0019 drops the plaintext + # source, and a hash cannot be turned back into the key it came from. + operations = [migrations.RunPython(backfill, migrations.RunPython.noop)] diff --git a/django_email_learning/migrations/0019_apikey_drop_encrypted_key.py b/django_email_learning/migrations/0019_apikey_drop_encrypted_key.py new file mode 100644 index 00000000..ab36deeb --- /dev/null +++ b/django_email_learning/migrations/0019_apikey_drop_encrypted_key.py @@ -0,0 +1,54 @@ +from django.db import migrations, models + + +class Migration(migrations.Migration): + """Tightens the backfilled columns and drops the reversible key storage. + + After this runs the deployment no longer holds anything that can be turned + back into a usable credential. + """ + + dependencies = [ + ("django_email_learning", "0018_backfill_api_key_hashes"), + ] + + operations = [ + migrations.RemoveField(model_name="apikey", name="key"), + migrations.RemoveField(model_name="apikey", name="salt"), + migrations.AlterField( + model_name="apikey", + name="key_type", + field=models.CharField( + choices=[("platform", "Platform"), ("organization", "Organization")], + db_index=True, + max_length=20, + ), + ), + migrations.AlterField( + model_name="apikey", + name="name", + field=models.CharField(max_length=100), + ), + migrations.AlterField( + model_name="apikey", + name="key_id", + field=models.CharField(editable=False, max_length=32, unique=True), + ), + migrations.AlterField( + model_name="apikey", + name="secret_hash", + field=models.CharField(editable=False, max_length=64, unique=True), + ), + migrations.AlterModelOptions( + name="apikey", + options={"ordering": ["-created_at"]}, + ), + migrations.AddConstraint( + model_name="apikey", + constraint=models.CheckConstraint( + condition=models.Q(("key_type", "platform"), ("organization__isnull", True)) + | models.Q(("key_type", "organization"), ("organization__isnull", False)), + name="api_key_organization_matches_key_type", + ), + ), + ] diff --git a/django_email_learning/models/__init__.py b/django_email_learning/models/__init__.py index 5c6f3c07..309af6ce 100644 --- a/django_email_learning/models/__init__.py +++ b/django_email_learning/models/__init__.py @@ -1,5 +1,5 @@ # ruff: noqa: F401 -from .api_keys import ApiKey +from .api_keys import ApiKey, ApiKeyScope, ApiKeyType from .course_contents import ( Answer, Assignment, diff --git a/django_email_learning/models/api_keys.py b/django_email_learning/models/api_keys.py index 360707da..5a35f5ed 100644 --- a/django_email_learning/models/api_keys.py +++ b/django_email_learning/models/api_keys.py @@ -1,30 +1,216 @@ -import base64 -import uuid +import datetime +import hashlib +import hmac +import secrets -from cryptography.fernet import InvalidToken from django.contrib.auth import get_user_model -from django.core.validators import MinLengthValidator +from django.core.exceptions import ValidationError from django.db import models +from django.utils import timezone -from .mixin_models import EncryptionMixin +from .organizations import Organization User = get_user_model() +TOKEN_PREFIX = "elk" +KEY_ID_BYTES = 12 +SECRET_BYTES = 32 -class ApiKey(EncryptionMixin): - key = models.CharField(max_length=256, unique=True, validators=[MinLengthValidator(50)]) +# How stale `last_used_at` is allowed to get before a successful authentication +# writes it back. Without this every authenticated request would issue a write +# purely to record activity, which on a hot endpoint costs far more than the +# resolution of the value is worth. +LAST_USED_RESOLUTION_SECONDS = 60 + + +class ApiKeyType(models.TextChoices): + PLATFORM = "platform", "Platform" + ORGANIZATION = "organization", "Organization" + + +class ApiKeyScope(models.TextChoices): + """Permissions an organization key can carry. + + Deliberately coarse: a scope names a resource and an access level, not an + endpoint, so adding an endpoint to an existing resource doesn't strand + callers on a key that predates it. + """ + + COURSES_READ = "courses:read", "Read courses" + ENROLLMENTS_READ = "enrollments:read", "Read enrollments" + ENROLLMENTS_WRITE = "enrollments:write", "Create enrollments" + + +def hash_secret(secret: str) -> str: + """Hashes the secret half of a token for storage and lookup. + + Plain SHA-256 rather than a password hash: the secret is 256 bits from + `secrets.token_urlsafe`, so there is no dictionary to attack and the + stretching a slow KDF buys would only add latency to every authenticated + request. + """ + return hashlib.sha256(secret.encode()).hexdigest() + + +class ApiKey(models.Model): + """A bearer credential for the machine-facing APIs. + + Two kinds share this table, told apart by `key_type` rather than by + inferring it from `organization` being null. The distinction matters: + a platform key can trigger deployment-wide jobs, so "which kind is this?" + must be an assertion a caller has to satisfy positively, not a property + that a dropped filter can accidentally produce. + + Only a hash of the secret is stored. The full token is returned once, at + creation, and cannot be recovered afterwards. + """ + + key_type = models.CharField(max_length=20, choices=ApiKeyType.choices, db_index=True) + organization = models.ForeignKey( + Organization, + on_delete=models.CASCADE, + related_name="api_keys", + null=True, + blank=True, + ) + name = models.CharField(max_length=100) + # Public half of the token: identifies the row so verification is a single + # indexed lookup, and is safe to display in the UI and write to logs. + key_id = models.CharField(max_length=32, unique=True, editable=False) + secret_hash = models.CharField(max_length=64, unique=True, editable=False) + scopes = models.JSONField(default=list, blank=True) created_at = models.DateTimeField(auto_now_add=True) created_by = models.ForeignKey(User, on_delete=models.SET_NULL, null=True, blank=True) + expires_at = models.DateTimeField(null=True, blank=True) + revoked_at = models.DateTimeField(null=True, blank=True) + last_used_at = models.DateTimeField(null=True, blank=True) + + class Meta: + ordering = ["-created_at"] + constraints = [ + models.CheckConstraint( + condition=models.Q(key_type=ApiKeyType.PLATFORM, organization__isnull=True) + | models.Q(key_type=ApiKeyType.ORGANIZATION, organization__isnull=False), + name="api_key_organization_matches_key_type", + ) + ] + + def __str__(self) -> str: + scope = self.organization.name if self.organization else "platform" + return f"{self.name} ({scope})" + + @staticmethod + def generate_key_id() -> str: + # Hex, not url-safe base64: `_` is the token's delimiter, and the + # base64 alphabet includes it, which would make the split ambiguous. + return secrets.token_hex(KEY_ID_BYTES) + + @staticmethod + def generate_secret() -> str: + return secrets.token_urlsafe(SECRET_BYTES) + + @classmethod + def build_token(cls, key_id: str, secret: str) -> str: + return f"{TOKEN_PREFIX}_{key_id}_{secret}" + + @classmethod + def split_token(cls, token: str) -> tuple[str, str] | None: + """Splits a token into its (key_id, secret) halves. + + Returns None for anything that isn't in this format, which the caller + should treat as "not one of our tokens" rather than as invalid — legacy + JWT credentials still reach the same authentication path. + """ + # maxsplit=2 keeps the secret intact: it is url-safe base64, so it may + # legitimately contain the delimiter. key_id is hex and cannot. + parts = token.split("_", 2) + if len(parts) != 3 or parts[0] != TOKEN_PREFIX or not parts[1] or not parts[2]: + return None + return parts[1], parts[2] @classmethod - def generate_key(cls) -> str: - return base64.urlsafe_b64encode(uuid.uuid4().bytes + uuid.uuid4().bytes).decode().rstrip("=") + def create( + cls, + *, + key_type: str, + name: str, + organization_id: int | None = None, + scopes: list[str] | None = None, + created_by: User | None = None, # type: ignore[valid-type] + expires_at: datetime.datetime | None = None, + ) -> tuple["ApiKey", str]: + """Creates a key and returns it alongside the one-time plaintext token. + + The token is the only point at which the secret exists in a readable + form; nothing persists it, so a caller that discards it has to issue a + replacement key. + """ + secret = cls.generate_secret() + api_key = cls( + key_type=key_type, + name=name, + organization_id=organization_id, + scopes=scopes or [], + created_by=created_by, + expires_at=expires_at, + key_id=cls.generate_key_id(), + secret_hash=hash_secret(secret), + ) + api_key.save() + return api_key, cls.build_token(api_key.key_id, secret) + + def clean(self) -> None: + super().clean() + if self.key_type == ApiKeyType.PLATFORM and self.organization_id is not None: + raise ValidationError({"organization": "Platform keys must not belong to an organization."}) + if self.key_type == ApiKeyType.ORGANIZATION and self.organization_id is None: + raise ValidationError({"organization": "Organization keys must belong to an organization."}) + if not isinstance(self.scopes, list) or any(not isinstance(scope, str) for scope in self.scopes): + raise ValidationError({"scopes": "Scopes must be a list of strings."}) + # Platform keys are all-or-nothing by design: they gate deployment-wide + # operations that aren't modelled as organization resources, so there is + # nothing for a scope to narrow them to. + if self.key_type == ApiKeyType.PLATFORM and self.scopes: + raise ValidationError({"scopes": "Platform keys do not take scopes."}) + invalid_scopes = set(self.scopes) - set(ApiKeyScope.values) + if invalid_scopes: + raise ValidationError({"scopes": f"Unknown scopes: {', '.join(sorted(invalid_scopes))}."}) def save(self, *args, **kwargs) -> None: # type: ignore[no-untyped-def] - try: - self.decrypt_password(self.key) - # Key is already encrypted - except InvalidToken: - self.key = self._encrypt_password(self.key) self.full_clean() super().save(*args, **kwargs) + + @property + def is_expired(self) -> bool: + return self.expires_at is not None and self.expires_at <= timezone.now() + + @property + def is_revoked(self) -> bool: + return self.revoked_at is not None + + @property + def is_usable(self) -> bool: + return not self.is_revoked and not self.is_expired + + def matches_secret(self, secret: str) -> bool: + return hmac.compare_digest(self.secret_hash, hash_secret(secret)) + + def has_scope(self, scope: str) -> bool: + return scope in self.scopes + + def revoke(self) -> None: + self.revoked_at = timezone.now() + self.save(update_fields=["revoked_at"]) + + def touch_last_used(self) -> None: + """Records that the key authenticated a request, at minute resolution. + + Written with `update()` rather than `save()` so recording activity can + never fail a request that has already authenticated — a full_clean() + here would surface unrelated validation errors from a stale row. + """ + now = timezone.now() + if self.last_used_at is not None and (now - self.last_used_at).total_seconds() < LAST_USED_RESOLUTION_SECONDS: + return + ApiKey.objects.filter(pk=self.pk).update(last_used_at=now) + self.last_used_at = now diff --git a/django_email_learning/organization_api/__init__.py b/django_email_learning/organization_api/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/django_email_learning/organization_api/serializers.py b/django_email_learning/organization_api/serializers.py new file mode 100644 index 00000000..78ea710d --- /dev/null +++ b/django_email_learning/organization_api/serializers.py @@ -0,0 +1,93 @@ +from datetime import datetime +from typing import List, Optional + +from pydantic import BaseModel, ConfigDict, Field, field_validator + +from django_email_learning.models import Course, Enrollment +from django_email_learning.public.api.serializers import EmailValidatedRequest + + +class EnrollmentCreateRequest(EmailValidatedRequest): + course_slug: str = Field(min_length=1) + subscribe_to_newsletter: bool = False + + @field_validator("email") + def normalize_email(cls, value: str) -> str: + # Learner.save() lowercases on write, so normalizing here keeps the + # lookup and the stored row agreeing on the same address. + return value.lower() + + +class CourseResponse(BaseModel): + id: int + slug: str + title: str + description: Optional[str] = None + language: str + enabled: bool + is_public: bool + + @staticmethod + def from_django_model(course: Course) -> "CourseResponse": + return CourseResponse.model_validate( + { + "id": course.id, + "slug": course.slug, + "title": course.title, + "description": course.description, + "language": course.language, + "enabled": course.enabled, + "is_public": course.is_public, + } + ) + + model_config = ConfigDict(from_attributes=True) + + +class EnrollmentResponse(BaseModel): + id: int + email: str + course_slug: str + status: str + enrolled_at: datetime + activated_at: Optional[datetime] = None + + @staticmethod + def from_django_model(enrollment: Enrollment) -> "EnrollmentResponse": + return EnrollmentResponse.model_validate( + { + "id": enrollment.id, + "email": enrollment.learner.email, + "course_slug": enrollment.course.slug, + "status": enrollment.status, + "enrolled_at": enrollment.enrolled_at, + "activated_at": enrollment.activated_at, + } + ) + + model_config = ConfigDict(from_attributes=True) + + +class EnrollmentListQuery(BaseModel): + """Query-string parameters for listing enrollments. + + `limit` is capped rather than unbounded so a caller can't turn one request + into a full table scan of a large organization. + """ + + course_slug: Optional[str] = None + email: Optional[str] = None + status: Optional[str] = None + limit: int = Field(default=50, ge=1, le=200) + offset: int = Field(default=0, ge=0) + + @field_validator("email") + def normalize_email(cls, value: Optional[str]) -> Optional[str]: + return value.lower() if value else value + + +class PaginatedEnrollmentsResponse(BaseModel): + enrollments: List[EnrollmentResponse] + total: int + limit: int + offset: int diff --git a/django_email_learning/organization_api/urls.py b/django_email_learning/organization_api/urls.py new file mode 100644 index 00000000..066bd582 --- /dev/null +++ b/django_email_learning/organization_api/urls.py @@ -0,0 +1,10 @@ +from django.urls import path + +from django_email_learning.organization_api.views import CoursesView, EnrollmentsView + +app_name = "django_email_learning" + +urlpatterns = [ + path("enrollments/", EnrollmentsView.as_view(), name="enrollments"), + path("courses/", CoursesView.as_view(), name="courses"), +] diff --git a/django_email_learning/organization_api/views.py b/django_email_learning/organization_api/views.py new file mode 100644 index 00000000..06e50f06 --- /dev/null +++ b/django_email_learning/organization_api/views.py @@ -0,0 +1,218 @@ +"""The organization-scoped public API (v1). + +Authenticated with an organization API key rather than a session. Every view +reads the organization from `request.organization`, which the decorator takes +from the key itself — never from the request body or the URL, so a key can only +ever act on the organization it was issued for. + +Distinct from `public.api`, which is the unauthenticated, embeddable surface +for third-party pages: that one is gated by a publishable embed token and is +deliberately limited to what an anonymous visitor may do. +""" + +import json +import logging +import uuid + +from django.conf import settings +from django.http import JsonResponse +from django.utils.decorators import method_decorator +from django.views import View +from django.views.decorators.csrf import csrf_exempt +from pydantic import ValidationError + +from django_email_learning.decorators import require_organization_api_key +from django_email_learning.models import ( + ApiKeyScope, + Course, + Enrollment, + EnrollmentStatus, + NewsletterSubscriber, +) +from django_email_learning.organization_api import serializers +from django_email_learning.public.api.rate_limiting import is_rate_limited +from django_email_learning.services.command_models.enroll_command import EnrollCommand +from django_email_learning.services.command_models.exceptions.blocked_email_error import ( + BlockedEmailError, +) +from django_email_learning.services.command_models.exceptions.enrollment_already_exists_error import ( + EnrollmentAlreadyExistsError, +) +from django_email_learning.services.command_models.exceptions.invalid_course_slug_error import ( + InvalidCourseSlugError, +) +from django_email_learning.services.command_models.exceptions.learner_cap_exceeded_error import ( + LearnerCapExceededError, +) +from django_email_learning.services.utils import mask_email + +logger = logging.getLogger(__name__) + +DEFAULT_RATE_LIMITS = { + "PER_KEY_LIMIT": 120, + "PER_KEY_WINDOW_SECONDS": 60, +} + +TOO_MANY_REQUESTS_MESSAGE = "Too many requests. Please try again later." +INVALID_JSON_MESSAGE = "Invalid JSON payload" + + +def get_rate_limit_settings() -> dict: + configured = getattr(settings, "DJANGO_EMAIL_LEARNING", {}).get("ORGANIZATION_API_RATE_LIMITS", {}) + return {**DEFAULT_RATE_LIMITS, **configured} + + +class RateLimitedApiView(View): + """Applies a per-key request budget. + + Keyed on `key_id` rather than on the client IP: a server-to-server caller + may sit behind a shared egress address, and the key is the thing whose + usage we actually want to bound. + """ + + def check_rate_limit(self, request) -> JsonResponse | None: # type: ignore[no-untyped-def] + limits = get_rate_limit_settings() + if is_rate_limited( + f"org_api:{request.api_key.key_id}", + limit=limits["PER_KEY_LIMIT"], + window_seconds=limits["PER_KEY_WINDOW_SECONDS"], + ): + return JsonResponse({"error": TOO_MANY_REQUESTS_MESSAGE}, status=429) + return None + + +@method_decorator(csrf_exempt, name="dispatch") +@method_decorator(require_organization_api_key(scopes=[ApiKeyScope.ENROLLMENTS_WRITE]), name="post") +@method_decorator(require_organization_api_key(scopes=[ApiKeyScope.ENROLLMENTS_READ]), name="get") +class EnrollmentsView(RateLimitedApiView): + def post(self, request, *args, **kwargs) -> JsonResponse: # type: ignore[no-untyped-def] + rate_limited = self.check_rate_limit(request) + if rate_limited: + return rate_limited + + try: + payload = serializers.EnrollmentCreateRequest.model_validate(json.loads(request.body or "{}")) + except json.JSONDecodeError: + return JsonResponse({"error": INVALID_JSON_MESSAGE}, status=400) + except ValidationError as e: + return JsonResponse({"error": e.json()}, status=400) + + organization_id = request.organization.id + + # Unlike the embeddable endpoint this doesn't require the course to be + # public - the caller holds a key for this organization, so a private + # course of its own is legitimately within reach. It must still be + # enabled, which EnrollCommand enforces. + try: + course = Course.objects.get(slug=payload.course_slug, organization_id=organization_id) + except Course.DoesNotExist: + return JsonResponse({"error": "Course not found"}, status=404) + + command = EnrollCommand( + email=payload.email, + course_slug=payload.course_slug, + organization_id=organization_id, + ) + try: + command.execute() + except EnrollmentAlreadyExistsError: + return JsonResponse({"status": "already_enrolled"}, status=200) + except InvalidCourseSlugError: + return JsonResponse({"error": "Course not found"}, status=404) + except BlockedEmailError as e: + error_reference = uuid.uuid4() + logger.warning(f"Blocked email error: {e} (error_id: {error_reference})") + return JsonResponse({"error": "Email is blocked", "error_id": str(error_reference)}, status=403) + except LearnerCapExceededError as e: + error_reference = uuid.uuid4() + logger.warning(f"Learner cap exceeded: {e} (error_id: {error_reference})") + return JsonResponse({"error": "Not enough slots available", "error_id": str(error_reference)}, status=403) + except Exception as e: + error_reference = uuid.uuid4() + logger.error(f"Unexpected error: {e} (error_id: {error_reference})") + return JsonResponse({"error": "An unexpected error occurred", "error_id": str(error_reference)}, status=500) + + if payload.subscribe_to_newsletter and course.newsletter_id: + # Created unconfirmed and without its own confirmation email: the + # enrollment still has to be verified, and doing so proves ownership + # of this address, which VerifyEnrollmentCommand then applies here. + NewsletterSubscriber.objects.get_or_create(newsletter_id=course.newsletter_id, email=payload.email) + + enrollment = ( + Enrollment.objects.filter( + learner__email=payload.email, + learner__organization_id=organization_id, + course=course, + ) + .select_related("learner", "course") + .order_by("-enrolled_at") + .first() + ) + logger.info( + "API key %s enrolled %s in course '%s' (organization %s)", + request.api_key.key_id, + mask_email(payload.email), + payload.course_slug, + organization_id, + ) + if enrollment is None: + # Shouldn't happen - execute() succeeded - but returning a body + # that claims an id we couldn't read would be worse than saying so. + return JsonResponse({"status": "enrolled"}, status=201) + return JsonResponse( + { + "status": "enrolled", + "enrollment": serializers.EnrollmentResponse.from_django_model(enrollment).model_dump(mode="json"), + }, + status=201, + ) + + def get(self, request, *args, **kwargs) -> JsonResponse: # type: ignore[no-untyped-def] + rate_limited = self.check_rate_limit(request) + if rate_limited: + return rate_limited + + try: + query = serializers.EnrollmentListQuery.model_validate(request.GET.dict()) + except ValidationError as e: + return JsonResponse({"error": e.json()}, status=400) + + if query.status is not None and query.status not in {status.value for status in EnrollmentStatus}: + return JsonResponse({"error": f"Unknown status '{query.status}'"}, status=400) + + enrollments = Enrollment.objects.filter(course__organization_id=request.organization.id).select_related( + "learner", "course" + ) + if query.course_slug: + enrollments = enrollments.filter(course__slug=query.course_slug) + if query.email: + enrollments = enrollments.filter(learner__email=query.email) + if query.status: + enrollments = enrollments.filter(status=query.status) + + total = enrollments.count() + page = enrollments.order_by("-enrolled_at")[query.offset : query.offset + query.limit] + + return JsonResponse( + serializers.PaginatedEnrollmentsResponse( + enrollments=[serializers.EnrollmentResponse.from_django_model(e) for e in page], + total=total, + limit=query.limit, + offset=query.offset, + ).model_dump(mode="json"), + status=200, + ) + + +@method_decorator(require_organization_api_key(scopes=[ApiKeyScope.COURSES_READ]), name="get") +class CoursesView(RateLimitedApiView): + def get(self, request, *args, **kwargs) -> JsonResponse: # type: ignore[no-untyped-def] + rate_limited = self.check_rate_limit(request) + if rate_limited: + return rate_limited + + courses = Course.objects.filter(organization_id=request.organization.id).order_by("title") + return JsonResponse( + {"courses": [serializers.CourseResponse.from_django_model(c).model_dump(mode="json") for c in courses]}, + status=200, + ) diff --git a/django_email_learning/platform/api/serializers/__init__.py b/django_email_learning/platform/api/serializers/__init__.py index 690a2c82..ec18e274 100644 --- a/django_email_learning/platform/api/serializers/__init__.py +++ b/django_email_learning/platform/api/serializers/__init__.py @@ -47,7 +47,12 @@ LessonResponse, LessonUpdate, ) -from django_email_learning.platform.api.serializers.misc import ApiKeyResponse +from django_email_learning.platform.api.serializers.misc import ( + ApiKeyCreatedResponse, + ApiKeyResponse, + CreateOrganizationApiKeyRequest, + CreatePlatformApiKeyRequest, +) from django_email_learning.platform.api.serializers.newsletters import ( CreateNewsletterRequest, CreateSendoutRequest, @@ -160,4 +165,8 @@ "UpdateSendoutRequest", "SendoutDetailResponse", "NewsletterSubscriberResponse", + "ApiKeyResponse", + "ApiKeyCreatedResponse", + "CreatePlatformApiKeyRequest", + "CreateOrganizationApiKeyRequest", ] diff --git a/django_email_learning/platform/api/serializers/misc.py b/django_email_learning/platform/api/serializers/misc.py index 63482fa2..4273552e 100644 --- a/django_email_learning/platform/api/serializers/misc.py +++ b/django_email_learning/platform/api/serializers/misc.py @@ -1,35 +1,78 @@ from datetime import datetime -from typing import Optional +from typing import List, Optional -from django.utils import timezone -from pydantic import BaseModel, ConfigDict +from pydantic import BaseModel, ConfigDict, Field, field_validator -from django_email_learning.models import ApiKey -from django_email_learning.services.jwt_service import generate_jwt +from django_email_learning.models import ApiKey, ApiKeyScope class ApiKeyResponse(BaseModel): + """Metadata about a key. Deliberately carries no secret. + + The token is returned exactly once, by `ApiKeyCreatedResponse` at creation + time. `key_id` identifies the key afterwards — for display, for revoking + it, and for correlating it with logs. + """ + id: int - key: str + key_id: str + name: str + key_type: str + organization_id: Optional[int] = None + scopes: List[str] = Field(default_factory=list) created_at: datetime created_by: Optional[str] = None + expires_at: Optional[datetime] = None + revoked_at: Optional[datetime] = None + last_used_at: Optional[datetime] = None @staticmethod def from_django_model(api_key: ApiKey) -> "ApiKeyResponse": - decrypted_key = api_key.decrypt_password(api_key.key) - salt = api_key.salt - jwt_key = generate_jwt( - {"key": decrypted_key, "salt": salt}, - exp=datetime.max.replace(tzinfo=timezone.get_current_timezone()), - ) - return ApiKeyResponse.model_validate( { "id": api_key.id, # type: ignore[attr-defined] - "key": jwt_key, + "key_id": api_key.key_id, + "name": api_key.name, + "key_type": api_key.key_type, + "organization_id": api_key.organization_id, + "scopes": api_key.scopes, "created_at": api_key.created_at, "created_by": api_key.created_by.username if api_key.created_by else None, + "expires_at": api_key.expires_at, + "revoked_at": api_key.revoked_at, + "last_used_at": api_key.last_used_at, } ) model_config = ConfigDict(from_attributes=True) + + +class ApiKeyCreatedResponse(ApiKeyResponse): + """The creation response, and the only place the token is ever readable.""" + + token: str + + @staticmethod + def from_created_key(api_key: ApiKey, token: str) -> "ApiKeyCreatedResponse": + return ApiKeyCreatedResponse.model_validate( + {**ApiKeyResponse.from_django_model(api_key).model_dump(), "token": token} + ) + + +class CreatePlatformApiKeyRequest(BaseModel): + name: str = Field(default="Platform key", min_length=1, max_length=100) + expires_at: Optional[datetime] = None + + +class CreateOrganizationApiKeyRequest(BaseModel): + name: str = Field(min_length=1, max_length=100) + scopes: List[str] = Field(min_length=1) + expires_at: Optional[datetime] = None + + @field_validator("scopes") + def validate_scopes(cls, value: List[str]) -> List[str]: + unknown = set(value) - set(ApiKeyScope.values) + if unknown: + raise ValueError(f"Unknown scopes: {', '.join(sorted(unknown))}") + # Deduplicated so the stored list matches what a caller sees back. + return sorted(set(value)) diff --git a/django_email_learning/platform/api/urls.py b/django_email_learning/platform/api/urls.py index 78bfe114..1cc5a1e4 100644 --- a/django_email_learning/platform/api/urls.py +++ b/django_email_learning/platform/api/urls.py @@ -19,6 +19,7 @@ OauthGetGroupListView, OauthGroupEnrollment, OauthSessionView, + OrganizationApiKeyView, OrganizationsView, OrganizationUsersView, ReorderCourseContentView, @@ -29,6 +30,7 @@ SingleCourseView, SingleLearnerView, SingleNewsletterView, + SingleOrganizationApiKeyView, SingleOrganizationUserView, SingleOrganizationView, SingleSendoutView, @@ -200,6 +202,16 @@ SubmissionReview.as_view(), name="submission_review", ), + path( + "organizations//api-keys/", + OrganizationApiKeyView.as_view(), + name="organization_api_keys_list", + ), + path( + "organizations//api-keys//", + SingleOrganizationApiKeyView.as_view(), + name="organization_api_keys_detail", + ), path("status/jobs/", JobsStatus.as_view(), name="jobs_status"), path("api_keys/", ApiKeyView.as_view(), name="api_keys_list"), path( diff --git a/django_email_learning/platform/api/views/__init__.py b/django_email_learning/platform/api/views/__init__.py index 3ed42eb1..f604d695 100644 --- a/django_email_learning/platform/api/views/__init__.py +++ b/django_email_learning/platform/api/views/__init__.py @@ -25,8 +25,10 @@ FileView, JobHealthStatus, JobsStatus, + OrganizationApiKeyView, RootView, SingleApiKeyView, + SingleOrganizationApiKeyView, UpdateSessionView, ) from django_email_learning.platform.api.views.newsletters import ( @@ -92,6 +94,8 @@ "JobHealthStatus", "ApiKeyView", "SingleApiKeyView", + "OrganizationApiKeyView", + "SingleOrganizationApiKeyView", "JobsStatus", "RootView", "FileView", diff --git a/django_email_learning/platform/api/views/misc.py b/django_email_learning/platform/api/views/misc.py index b2a78e2a..f081d829 100644 --- a/django_email_learning/platform/api/views/misc.py +++ b/django_email_learning/platform/api/views/misc.py @@ -6,6 +6,7 @@ from urllib.parse import urlparse from django.conf import settings +from django.core.exceptions import ValidationError as DjangoValidationError from django.core.files.storage import default_storage from django.db.utils import IntegrityError from django.http import JsonResponse @@ -22,6 +23,7 @@ from django_email_learning.error_responses import log_and_conflict_response from django_email_learning.models import ( ApiKey, + ApiKeyType, JobExecution, JobName, OrganizationUser, @@ -42,44 +44,156 @@ class JobHealthStatus(StrEnum): CRITICAL = "critical" +def _parse_json_body(request) -> dict: # type: ignore[no-untyped-def] + """Reads an optional JSON body, treating anything that isn't one as `{}`. + + A bodyless POST from a form-encoded client still carries a body, so the + content type - not emptiness - is what decides whether there is JSON to + read. Fields the caller omits then fall back to their defaults, and any + that have none surface as ordinary validation errors. + """ + if not request.body or not (request.content_type or "").startswith("application/json"): + return {} + return json.loads(request.body) + + @method_decorator(is_platform_admin(), name="post") @method_decorator(is_platform_admin(), name="get") class ApiKeyView(View): + """Platform-wide keys. Restricted to platform admins, since these gate + deployment-wide operations rather than any one organization's data. + """ + def post(self, request, *args, **kwargs) -> JsonResponse: # type: ignore[no-untyped-def] try: - key = ApiKey.generate_key() - api_key = ApiKey(key=key, created_by=request.user) - api_key.save() - return JsonResponse( - serializers.ApiKeyResponse.from_django_model(api_key).model_dump(), - status=201, - ) + payload = serializers.CreatePlatformApiKeyRequest.model_validate(_parse_json_body(request)) + except json.JSONDecodeError: + return JsonResponse({"error": "Invalid request body"}, status=400) except ValidationError as e: return JsonResponse({"error": e.json()}, status=400) + + try: + api_key, token = ApiKey.create( + key_type=ApiKeyType.PLATFORM, + name=payload.name, + created_by=request.user, + expires_at=payload.expires_at, + ) + except DjangoValidationError as e: + return JsonResponse({"error": e.message_dict}, status=400) except IntegrityError as e: return log_and_conflict_response(logger, e, "Creating API key") + return JsonResponse( + serializers.ApiKeyCreatedResponse.from_created_key(api_key, token).model_dump(mode="json"), + status=201, + ) + def get(self, request, *args, **kwargs) -> JsonResponse: # type: ignore[no-untyped-def] - api_keys = ApiKey.objects.all() # type: ignore[attr-defined] - response_list = [] - for api_key in api_keys: - response_list.append(serializers.ApiKeyResponse.from_django_model(api_key).model_dump()) - return JsonResponse({"api_keys": response_list}, status=200) + api_keys = ApiKey.objects.filter(key_type=ApiKeyType.PLATFORM).select_related("created_by") + return JsonResponse( + { + "api_keys": [ + serializers.ApiKeyResponse.from_django_model(api_key).model_dump(mode="json") + for api_key in api_keys + ] + }, + status=200, + ) @method_decorator(is_platform_admin(), name="delete") class SingleApiKeyView(View): def delete(self, request, *args, **kwargs): # type: ignore[no-untyped-def] try: - api_key = ApiKey.objects.get(id=kwargs["api_key_id"]) - api_key.delete() - return JsonResponse({"message": "API Key deleted successfully"}, status=200) + api_key = ApiKey.objects.get(id=kwargs["api_key_id"], key_type=ApiKeyType.PLATFORM) except ApiKey.DoesNotExist: return JsonResponse({"error": "API Key not found"}, status=404) + + api_key.revoke() + return JsonResponse({"message": "API Key revoked successfully"}, status=200) + + +@method_decorator(is_an_organization_member(only_admin=True), name="post") +@method_decorator(is_an_organization_member(only_admin=True), name="get") +class OrganizationApiKeyView(View): + """Keys an organization's own admins issue for the public API. + + Admin-only within the organization: a key is a bearer credential that acts + with the scopes it was granted, so issuing one is a privilege escalation + for any role that couldn't already do the thing the scope permits. + """ + + def post(self, request, organization_id: int, *args, **kwargs) -> JsonResponse: # type: ignore[no-untyped-def] + try: + payload = serializers.CreateOrganizationApiKeyRequest.model_validate(_parse_json_body(request)) + except json.JSONDecodeError: + return JsonResponse({"error": "Invalid request body"}, status=400) except ValidationError as e: return JsonResponse({"error": e.json()}, status=400) + + try: + api_key, token = ApiKey.create( + key_type=ApiKeyType.ORGANIZATION, + name=payload.name, + organization_id=organization_id, + scopes=payload.scopes, + created_by=request.user, + expires_at=payload.expires_at, + ) + except DjangoValidationError as e: + return JsonResponse({"error": e.message_dict}, status=400) except IntegrityError as e: - return log_and_conflict_response(logger, e, "Creating API key") + return log_and_conflict_response(logger, e, "Creating organization API key") + + logger.info( + "Organization API key %s created for organization %s by user %s", + api_key.key_id, + organization_id, + request.user.id, + ) + return JsonResponse( + serializers.ApiKeyCreatedResponse.from_created_key(api_key, token).model_dump(mode="json"), + status=201, + ) + + def get(self, request, organization_id: int, *args, **kwargs) -> JsonResponse: # type: ignore[no-untyped-def] + api_keys = ApiKey.objects.filter( + key_type=ApiKeyType.ORGANIZATION, organization_id=organization_id + ).select_related("created_by") + return JsonResponse( + { + "api_keys": [ + serializers.ApiKeyResponse.from_django_model(api_key).model_dump(mode="json") + for api_key in api_keys + ] + }, + status=200, + ) + + +@method_decorator(is_an_organization_member(only_admin=True), name="delete") +class SingleOrganizationApiKeyView(View): + def delete(self, request, organization_id: int, api_key_id: int, *args, **kwargs) -> JsonResponse: # type: ignore[no-untyped-def] + # Filtering on organization_id as well as the key id keeps one + # organization's admin from revoking another's key by guessing an id. + try: + api_key = ApiKey.objects.get( + id=api_key_id, + organization_id=organization_id, + key_type=ApiKeyType.ORGANIZATION, + ) + except ApiKey.DoesNotExist: + return JsonResponse({"error": "API Key not found"}, status=404) + + api_key.revoke() + logger.info( + "Organization API key %s revoked for organization %s by user %s", + api_key.key_id, + organization_id, + request.user.id, + ) + return JsonResponse({"message": "API Key revoked successfully"}, status=200) # Job health is deployment-wide operational state, not organization data, so diff --git a/django_email_learning/platform/views/misc.py b/django_email_learning/platform/views/misc.py index cf337046..0f715ace 100644 --- a/django_email_learning/platform/views/misc.py +++ b/django_email_learning/platform/views/misc.py @@ -32,16 +32,31 @@ def get_locale_messages(self) -> Dict[str, str]: "settings": _("Settings"), "api_keys": _("API Keys"), "add_api_key": _("Add API Key"), - "display_key": _("Display Key"), - "hide_key": _("Hide Key"), "actions": _("Actions"), - "key": _("Key"), + "key_id": _("Key ID"), + "name": _("Name"), + "status": _("Status"), + "active": _("Active"), + "revoked": _("Revoked"), + "expired": _("Expired"), + "last_used": _("Last Used"), + "never_used": _("Never used"), "created_at": _("Created At"), - "delete": _("Delete"), - "are_you_sure_delete_key": _("Are you sure you want to delete this API key?"), + "revoke": _("Revoke"), + "are_you_sure_revoke_key": _( + "Are you sure you want to revoke this API key? Anything using it will stop working immediately." + ), "created_by": _("Created By"), "cancel": _("Cancel"), - "confirm_deletion": _("Confirm Deletion"), + "copy": _("Copy"), + "copied": _("Copied"), + "confirm_revocation": _("Confirm Revocation"), + "done": _("Done"), + "new_api_key_created": _("New API key created"), + "copy_key_now_warning": _( + "Copy this key now. For security it is stored hashed, so this is the only time it can be shown." + ), + "no_api_keys_found": _("No API keys yet."), "api_key_intro": _( "API keys allow external applications to interact with the platform and execute jobs." " This is ideal for using cloud scheduling or third-party integrations instead of managing" diff --git a/django_email_learning/services/api_key_service.py b/django_email_learning/services/api_key_service.py new file mode 100644 index 00000000..4241e772 --- /dev/null +++ b/django_email_learning/services/api_key_service.py @@ -0,0 +1,103 @@ +"""Authentication for the machine-facing APIs. + +Verification is a single indexed lookup on the token's public `key_id` half +followed by a constant-time comparison of the hashed secret half. Credentials +issued before 3.1.0 are JWTs carrying the raw key, and are resolved through the +same hash so both formats converge on one code path. +""" + +import hmac +import typing + +from django_email_learning.models import ApiKey +from django_email_learning.models.api_keys import hash_secret +from django_email_learning.services.jwt_service import ( + ExpiredTokenException, + InvalidTokenException, + decode_jwt, +) + +INVALID_KEY_MESSAGE = "Invalid API key" + +# Compared against when no row matches, so that a lookup miss costs the same as +# a wrong secret and can't be distinguished by how long the response took. +_DUMMY_HASH = hash_secret("dummy-secret-for-constant-time-comparison") + + +class ApiKeyAuthenticationError(Exception): + """Carries the response a failed authentication should produce. + + Messages are deliberately uniform for every "we couldn't resolve this to a + key" case: distinguishing an unknown key_id from a bad secret would let a + caller confirm which key ids exist. + """ + + def __init__(self, message: str, status: int = 401) -> None: + super().__init__(message) + self.message = message + self.status = status + + +def extract_bearer_token(request: typing.Any) -> str: + authorization_header = request.headers.get("Authorization") + if not authorization_header: + raise ApiKeyAuthenticationError("Authorization header missing") + parts = authorization_header.split(" ") + if len(parts) != 2 or parts[0] != "Bearer": + raise ApiKeyAuthenticationError("Invalid Authorization header format. Expected: Bearer ") + return parts[1] + + +def _authenticate_legacy_jwt(token: str) -> ApiKey: + """Resolves a pre-3.1.0 JWT credential. + + The JWT never added anything a bearer token doesn't already have — it was + signed with a fixed `exp` of `datetime.max` and its only real payload was + the salt needed to narrow the old decrypt-and-compare lookup. It is + accepted here purely so existing deployments don't break on upgrade. + """ + try: + payload = decode_jwt(token) + except ExpiredTokenException: + raise ApiKeyAuthenticationError("Expired Json Web Token") + except InvalidTokenException: + raise ApiKeyAuthenticationError("Invalid Json Web Token") + + if "key" not in payload or "salt" not in payload: + raise ApiKeyAuthenticationError("Json Web Token missing required fields") + + api_key = ApiKey.objects.select_related("organization").filter(secret_hash=hash_secret(payload["key"])).first() + if api_key is None: + raise ApiKeyAuthenticationError(INVALID_KEY_MESSAGE) + return api_key + + +def authenticate_token(token: str) -> ApiKey: + """Resolves a bearer token to a usable ApiKey, or raises. + + Anything that isn't in the `elk__` shape falls through to + the legacy JWT path rather than being rejected outright. + """ + split = ApiKey.split_token(token) + if split is None: + api_key = _authenticate_legacy_jwt(token) + else: + key_id, secret = split + candidate = ApiKey.objects.select_related("organization").filter(key_id=key_id).first() + if candidate is None: + # Hash anyway, so a lookup miss costs the same as a wrong secret. + hmac.compare_digest(_DUMMY_HASH, hash_secret(secret)) + raise ApiKeyAuthenticationError(INVALID_KEY_MESSAGE) + if not candidate.matches_secret(secret): + raise ApiKeyAuthenticationError(INVALID_KEY_MESSAGE) + api_key = candidate + + # Only reported once the caller has proved possession of the secret, so + # neither message tells an attacker anything they didn't already hold. + if api_key.is_revoked: + raise ApiKeyAuthenticationError("API key has been revoked") + if api_key.is_expired: + raise ApiKeyAuthenticationError("API key has expired") + + api_key.touch_last_used() + return api_key diff --git a/django_email_learning/urls.py b/django_email_learning/urls.py index 991bda1d..e36070ab 100644 --- a/django_email_learning/urls.py +++ b/django_email_learning/urls.py @@ -3,6 +3,7 @@ from django_email_learning.analytics import urls as analytics_urls from django_email_learning.jobs.api import urls as jobs_api_urls +from django_email_learning.organization_api import urls as organization_api_urls from django_email_learning.personalised import urls as personalised_urls from django_email_learning.personalised.api import urls as personalised_api_urls from django_email_learning.platform import urls as platform_urls @@ -19,6 +20,9 @@ include(personalised_api_urls, namespace="api_personalised"), ), path("api/public/", include(public_api_urls, namespace="api_public")), + # Organization-scoped API authenticated with an organization API key. Kept + # separate from api/public/, which is the unauthenticated embed surface. + path("api/v1/", include(organization_api_urls, namespace="api_v1")), path("platform/", include(platform_urls, namespace="platform")), path("public/", include(public_urls, namespace="public")), path("my/", include(personalised_urls, namespace="personalised")), diff --git a/docs/source/index.rst b/docs/source/index.rst index 7a21004d..62c00100 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -32,6 +32,7 @@ Django Email Learning documentation technical/ai-configuration technical/management-commands technical/jobs-api + technical/organization-api technical/encryption-key-management diff --git a/docs/source/platform/api_keys.rst b/docs/source/platform/api_keys.rst index e2debbd0..597c3fd2 100644 --- a/docs/source/platform/api_keys.rst +++ b/docs/source/platform/api_keys.rst @@ -2,7 +2,17 @@ API Keys ======== API Keys provide a secure way to authenticate programmatic access to the Django Email Learning platform. They are designed to enable automated job execution without the need to set up cron jobs directly on the server. -API keys are encrypted in the database using the `ENCRYPTION_SECRET_KEY` defined in your settings. If this key is changed, all existing API keys will become invalid. + +Only a SHA-256 hash of each key is stored, so the platform cannot recover a key +once it has been issued. The full key is shown **once**, immediately after +creation — copy it then, or issue a replacement. + +.. note:: + + The keys described on this page are **platform** keys, which act + deployment-wide. Organization admins can also issue **organization** keys, + scoped to a single organization's data — see + :doc:`../technical/organization-api`. .. image:: ../../images/api-keys.png :alt: API Keys Management Interface @@ -40,15 +50,23 @@ Creating an API Key 1. Navigate to **Settings** → **API Keys** from the platform navigation menu 2. Click the **Add API Key** button -3. The system will generate a secure, random API key +3. The system generates a secure, random API key and displays it once +Copy the key from that dialog before closing it. It is not stored in a +recoverable form and cannot be shown again. Managing API Keys ----------------- The API Keys page displays all existing keys with the following information: -* **Key** - A partial view of the API key (for security, only a portion is shown after creation) -* **Created At** - Timestamp when the key was created +* **Key ID** - The key's public identifier, used to recognise it here and in logs. It is not a credential and cannot be used to authenticate. +* **Status** - Active, Revoked, or Expired * **Created By** - The username of the administrator who created the key -* **Actions** - Delete button to revoke the key +* **Created At** - Timestamp when the key was created +* **Last Used** - When the key last authenticated a request, or "Never used" +* **Actions** - Revoke button + +Revoking a key stops it working immediately. The record is kept rather than +deleted, so the history of which keys existed — and when each was last used — +survives revocation. diff --git a/docs/source/technical/encryption-key-management.rst b/docs/source/technical/encryption-key-management.rst index 9d7550de..da863325 100644 --- a/docs/source/technical/encryption-key-management.rst +++ b/docs/source/technical/encryption-key-management.rst @@ -1,7 +1,13 @@ Encryption Key Management ========================= -Django Email Learning encrypts sensitive data at rest — specifically **IMAP passwords** and **API keys** — using `Fernet symmetric encryption `_ backed by your ``ENCRYPTION_SECRET_KEY`` setting. +Django Email Learning encrypts sensitive data at rest — specifically **IMAP passwords** — using `Fernet symmetric encryption `_ backed by your ``ENCRYPTION_SECRET_KEY`` setting. + +.. note:: + + API keys were encrypted this way before 3.1.0. They are now stored as a + SHA-256 hash instead, which cannot be reversed and therefore needs no + rotation. Rotating ``ENCRYPTION_SECRET_KEY`` no longer affects them. This page explains how the encryption works, when and why you might need to rotate the key, and how to do so safely. @@ -19,7 +25,6 @@ Because each row uses its own salt, a compromise of one row's ciphertext does no Models that store encrypted fields: - ``ImapConnection.password`` -- ``ApiKey.key`` When to Rotate the Key ----------------------- diff --git a/docs/source/technical/organization-api.rst b/docs/source/technical/organization-api.rst new file mode 100644 index 00000000..a2b00c85 --- /dev/null +++ b/docs/source/technical/organization-api.rst @@ -0,0 +1,186 @@ +Organization API (v1) +===================== + +The organization API lets an organization's own systems act on its data from +outside the platform — enrolling a learner from a partner site's signup flow, +for example, or reconciling enrollment state with an internal CRM. + +It is authenticated with an **organization API key**, which an organization +admin issues for themselves. Every request acts on the organization the key +was issued for, and on no other. + +.. note:: + + This is distinct from the two API surfaces that already existed: + + * :doc:`jobs-api` is authenticated with a *platform* key and triggers + deployment-wide jobs. Organization keys cannot reach it. + * ``/api/public/`` is unauthenticated and gated by a publishable embed + token, for widgets on third-party pages. It is limited to what an + anonymous visitor may do. + +Issuing a Key +------------- + +Organization **admins** can create keys for their own organization. Editors, +instructors and viewers cannot: a key acts with whatever scopes it carries, so +issuing one would let a non-admin hand out access it does not itself have. + +.. note:: + + Key management is API-only for now — there is no organization-facing + settings screen for these keys yet. + +.. code-block:: http + + POST /api/platform/organizations//api-keys/ + Content-Type: application/json + + { + "name": "Partner signup integration", + "scopes": ["enrollments:write"], + "expires_at": "2027-01-01T00:00:00Z" + } + +``name`` and ``scopes`` are required; ``expires_at`` is optional and the key +never expires without it. + +The response is the **only** time the token is readable: + +.. code-block:: json + + { + "id": 7, + "key_id": "a1b2c3d4e5f60718293a4b5c", + "name": "Partner signup integration", + "key_type": "organization", + "organization_id": 3, + "scopes": ["enrollments:write"], + "created_at": "2026-08-07T10:00:00Z", + "created_by": "orgadmin", + "expires_at": "2027-01-01T00:00:00Z", + "revoked_at": null, + "last_used_at": null, + "token": "elk_a1b2c3d4e5f60718293a4b5c_" + } + +Only a SHA-256 hash of the secret is stored, so a lost token cannot be +recovered — issue a replacement and revoke the old one. ``key_id`` is the +public half: it identifies the key in the UI, in logs, and when revoking it, +and is not itself a credential. + +``GET`` the same URL to list the organization's keys (metadata only, never the +token). ``DELETE /api/platform/organizations//api-keys//`` +revokes a key; the row is kept so the record of what existed survives. + +Scopes +------ + +.. list-table:: + :header-rows: 1 + + * - Scope + - Grants + * - ``courses:read`` + - List the organization's courses + * - ``enrollments:read`` + - List the organization's enrollments + * - ``enrollments:write`` + - Create enrollments + +A scope names a resource and an access level rather than an endpoint, so +adding an endpoint to an existing resource does not strand callers on a key +that predates it. + +Authentication +-------------- + +Pass the token as a bearer token: + +.. code-block:: http + + Authorization: Bearer elk__ + +Failures return ``401`` with an ``error`` message, except for a key that +authenticates but lacks the required scope or is of the wrong type, which +returns ``403``. A revoked or expired key returns ``401``. + +Endpoints +--------- + +Create an enrollment +^^^^^^^^^^^^^^^^^^^^ + +Requires ``enrollments:write``. + +.. code-block:: http + + POST /api/v1/enrollments/ + Content-Type: application/json + + { + "email": "learner@example.com", + "course_slug": "intro-to-widgets", + "subscribe_to_newsletter": false + } + +The course is resolved against the key's organization, so a slug belonging to +another organization reads as ``404``. Unlike the embeddable public endpoint, +a course does not need to be public — but it must be enabled. + +The learner receives a verification email and the enrollment starts as +``unverified``; it becomes ``active`` once they confirm. + +Responses: + +* ``201`` — enrolled, with the created enrollment in the body +* ``200`` ``{"status": "already_enrolled"}`` — a non-deactivated enrollment already exists +* ``403`` — the email is blocked, or the organization is at its learner cap +* ``404`` — no such enabled course in this organization + +List enrollments +^^^^^^^^^^^^^^^^ + +Requires ``enrollments:read``. + +.. code-block:: http + + GET /api/v1/enrollments/?course_slug=intro-to-widgets&status=active&limit=50&offset=0 + +Optional filters: ``course_slug``, ``email``, ``status`` (one of +``unverified``, ``active``, ``completed``, ``deactivated``). ``limit`` defaults +to 50 and is capped at 200. The response carries ``enrollments``, ``total``, +``limit`` and ``offset``. + +List courses +^^^^^^^^^^^^ + +Requires ``courses:read``. + +.. code-block:: http + + GET /api/v1/courses/ + +Returns every course in the organization, including disabled ones — a caller +needs to see a disabled course to understand why enrolling into it failed. + +Rate Limiting +------------- + +Requests are budgeted per key (by ``key_id``, not by client IP, since a +server-to-server caller may sit behind a shared egress address). Exceeding the +budget returns ``429``. Defaults are 120 requests per 60 seconds; override them +in settings: + +.. code-block:: python + + DJANGO_EMAIL_LEARNING = { + "ORGANIZATION_API_RATE_LIMITS": { + "PER_KEY_LIMIT": 120, + "PER_KEY_WINDOW_SECONDS": 60, + }, + } + +Rate limiting is backed by Django's cache framework. A per-process +``LocMemCache`` under-counts across worker processes, so a shared backend such +as Redis is recommended in production. diff --git a/frontend/platform/settings_api_keys/ApiKeys.jsx b/frontend/platform/settings_api_keys/ApiKeys.jsx index d0d3e941..2e906669 100644 --- a/frontend/platform/settings_api_keys/ApiKeys.jsx +++ b/frontend/platform/settings_api_keys/ApiKeys.jsx @@ -1,11 +1,9 @@ import Base from "../../src/components/Base"; import EmptyTableState from "../../src/components/EmptyTableState.jsx"; -import { Box, Button, IconButton, Grid, Dialog, Typography, TableContainer, Table, TableHead, TableRow,TableBody, TableCell } from "@mui/material"; +import { Box, Button, IconButton, Grid, Dialog, Typography, TableContainer, Table, TableHead, TableRow,TableBody, TableCell, Chip, Alert } from "@mui/material"; import AddIcon from '@mui/icons-material/Add'; import DeleteIcon from '@mui/icons-material/Delete'; -import VisibilityIcon from '@mui/icons-material/Visibility'; -import VisibilityOffIcon from '@mui/icons-material/VisibilityOff'; import ContentCopyIcon from '@mui/icons-material/ContentCopy'; import render, {useAppContext} from "../../src/render"; import { useState, useEffect } from "react"; @@ -13,8 +11,7 @@ import apiClient from "../../src/apiClient.js"; import { sanitizeEndpointUrl } from '../../src/sanitizeUrl.js'; - -const DeleteConfirmationDialog = ({apiKey, onCancel, onSuccess}) => { +const RevokeConfirmationDialog = ({apiKey, onCancel, onSuccess}) => { const { localeMessages, apiBaseUrl: rawApiBaseUrl } = useAppContext(); const apiBaseUrl = sanitizeEndpointUrl(rawApiBaseUrl); @@ -22,10 +19,10 @@ const DeleteConfirmationDialog = ({apiKey, onCancel, onSuccess}) => { return ( - {localeMessages["confirm_deletion"]} + {localeMessages["confirm_revocation"]} - {localeMessages["are_you_sure_delete_key"]} + {localeMessages["are_you_sure_revoke_key"]} ); } +/** + * Shown once, immediately after creation. The server stores only a hash, so + * this dialog is the only opportunity the user has to copy the token - hence + * the warning and the deliberate lack of any "show key" affordance elsewhere. + */ +const NewApiKeyDialog = ({token, onClose}) => { + const { localeMessages } = useAppContext(); + const [copied, setCopied] = useState(false); + + const copyToken = async () => { + try { + await navigator.clipboard.writeText(token); + setCopied(true); + } catch (error) { + console.error('Failed to copy API key:', error); + } + }; + + return ( + + + {localeMessages["new_api_key_created"]} + + + {localeMessages["copy_key_now_warning"]} + + + + {token} + + + + + + + {copied && {localeMessages["copied"]}} + + + + ); +} + +const statusOf = (key) => { + if (key.revoked_at) return 'revoked'; + if (key.expires_at && new Date(key.expires_at) <= new Date()) return 'expired'; + return 'active'; +} + const ApiKeys = () => { const [dialogOpen, setDialogOpen] = useState(false); const [dialogContent, setDialogContent] = useState(null); @@ -58,39 +109,29 @@ const ApiKeys = () => { const apiBaseUrl = sanitizeEndpointUrl(rawApiBaseUrl); useEffect(() => { - // Fetch API keys from the backend if (!loaded) { - apiClient.get(`${apiBaseUrl}/api_keys/`) - .then(data => { - setApiKeyList(data.api_keys.map((key) => ({ - id: key.id, - key: key.key, - created_by: key.created_by, - created_at: key.created_at, - visible: false, - }))); - }) - .finally(() => { - setLoaded(true); - }); - } + apiClient.get(`${apiBaseUrl}/api_keys/`) + .then(data => { + setApiKeyList(data.api_keys); + }) + .finally(() => { + setLoaded(true); + }); + } }, [loaded]); const addApiKey = () => { apiClient.post(`${apiBaseUrl}/api_keys/`) .then(data => { - data.visible = false; - setApiKeyList([...apiKeyList, data]); + setDialogContent( { + setDialogOpen(false); + setLoaded(false); + }} />); + setDialogOpen(true); }); } - const copyApiKey = async (apiKeyValue) => { - try { - await navigator.clipboard.writeText(apiKeyValue); - } catch (error) { - console.error('Failed to copy API key:', error); - } - } + const cellSx = { textAlign: direction === 'rtl' ? 'right' : 'left' }; return ( @@ -111,71 +152,56 @@ const ApiKeys = () => { - {localeMessages["key"]} - {localeMessages["created_by"]} - {localeMessages["created_at"]} + {localeMessages["key_id"]} + {localeMessages["status"]} + {localeMessages["created_by"]} + {localeMessages["created_at"]} + {localeMessages["last_used"]} {localeMessages["actions"]} {apiKeyList.length === 0 && ( )} - { apiKeyList.map((key) => ( + { apiKeyList.map((key) => { + const status = statusOf(key); + return ( - - - - { key.visible ? key.key : '••••••••••••••••' } - - copyApiKey(key.key)} - aria-label={localeMessages['copy'] || 'Copy'} - > - - - + + + { key.key_id } + - {key.created_by} - {key.created_at} + + + + {key.created_by} + {key.created_at} + {key.last_used_at || localeMessages["never_used"]} - {setDialogContent( setDialogOpen(false)} onSuccess={() => { + {status !== 'revoked' && + {setDialogContent( setDialogOpen(false)} onSuccess={() => { setLoaded(false); setDialogOpen(false); }} />); setDialogOpen(true);}}> - {key.visible ? - { - setApiKeyList(apiKeyList.map((k) => { - if (k.id === key.id) { - return {...k, visible: false}; - } - return k; - })); - }}> : - { - setApiKeyList(apiKeyList.map((k) => { - if (k.id === key.id) { - return {...k, visible: true}; - } - return k; - })); - }}> } - ))} + )})}
diff --git a/frontend/src/test/platform/ApiKeys.test.jsx b/frontend/src/test/platform/ApiKeys.test.jsx index b90b4524..05dfe54a 100644 --- a/frontend/src/test/platform/ApiKeys.test.jsx +++ b/frontend/src/test/platform/ApiKeys.test.jsx @@ -35,19 +35,44 @@ function setupFetch(apiKeys = []) { }); } +const activeKey = { + id: '1', + key_id: 'a1b2c3d4e5f6', + name: 'CI runner', + key_type: 'platform', + scopes: [], + created_by: 'admin', + created_at: '2024-01-01', + expires_at: null, + revoked_at: null, + last_used_at: null, +}; + const localeMessages = { api_keys: 'API Keys', api_key_intro: 'Use API keys to access the API.', add_api_key: 'Add API Key', - key: 'Key', + key_id: 'Key ID', + name: 'Name', + status: 'Status', + active: 'Active', + revoked: 'Revoked', + expired: 'Expired', + last_used: 'Last Used', + never_used: 'Never used', created_by: 'Created By', created_at: 'Created At', actions: 'Actions', copy: 'Copy', - confirm_deletion: 'Confirm Deletion', - are_you_sure_delete_key: 'Are you sure you want to delete this API key?', + copied: 'Copied', + done: 'Done', + new_api_key_created: 'New API key created', + copy_key_now_warning: 'Copy this key now. It cannot be shown again.', + confirm_revocation: 'Confirm Revocation', + are_you_sure_revoke_key: 'Are you sure you want to revoke this API key?', cancel: 'Cancel', - delete: 'Delete', + revoke: 'Revoke', + no_api_keys_found: 'No API keys yet.', organizations: 'Organizations', course_management: 'Courses', learners: 'Learners', @@ -83,26 +108,44 @@ describe('ApiKeys', () => { ); }); - it('shows the key table after loading keys', async () => { - setupFetch([ - { id: '1', key: 'abc123', created_by: 'admin', created_at: '2024-01-01', visible: false }, - ]); + it('shows the key id and metadata after loading keys', async () => { + setupFetch([activeKey]); renderWithProviders(, { appContext: { localeMessages, isPlatformAdmin: true }, }); await waitFor(() => expect(screen.getByText('admin')).toBeInTheDocument()); + expect(screen.getByText('a1b2c3d4e5f6')).toBeInTheDocument(); expect(screen.getByText('2024-01-01')).toBeInTheDocument(); + expect(screen.getByText('Never used')).toBeInTheDocument(); + expect(screen.getByText('Active')).toBeInTheDocument(); + }); + + it('marks a revoked key as revoked and offers no revoke action', async () => { + setupFetch([{ ...activeKey, revoked_at: '2024-02-01' }]); + renderWithProviders(, { + appContext: { localeMessages, isPlatformAdmin: true }, + }); + await waitFor(() => expect(screen.getByText('Revoked')).toBeInTheDocument()); + expect( + screen.queryAllByRole('button').find((btn) => btn.querySelector('[data-testid="DeleteIcon"]')) + ).toBeUndefined(); }); - it('adds a new API key when Add API Key is clicked', async () => { + it('marks a key past its expiry as expired', async () => { + setupFetch([{ ...activeKey, expires_at: '2020-01-01T00:00:00Z' }]); + renderWithProviders(, { + appContext: { localeMessages, isPlatformAdmin: true }, + }); + await waitFor(() => expect(screen.getByText('Expired')).toBeInTheDocument()); + }); + + it('shows the token once after creating a key', async () => { const user = userEvent.setup(); global.fetch.mockImplementation((url, options) => { if (url.includes('/api_keys/') && options?.method === 'POST') { return Promise.resolve({ ok: true, - json: () => Promise.resolve({ - id: '2', key: 'new-key-xyz', created_by: 'admin', created_at: '2024-06-01', visible: false, - }), + json: () => Promise.resolve({ ...activeKey, id: '2', token: 'elk_abc123_supersecret' }), }); } if (url.includes('/api_keys/')) { @@ -123,25 +166,34 @@ describe('ApiKeys', () => { expect(screen.getByRole('button', { name: /Add API Key/ })).toBeInTheDocument() ); await user.click(screen.getByRole('button', { name: /Add API Key/ })); - await waitFor(() => expect(screen.getByText('2024-06-01')).toBeInTheDocument()); + + await waitFor(() => expect(screen.getByText('New API key created')).toBeInTheDocument()); + expect(screen.getByTestId('new-api-key-token')).toHaveTextContent('elk_abc123_supersecret'); + expect(screen.getByText('Copy this key now. It cannot be shown again.')).toBeInTheDocument(); + }); + + it('never renders a token for keys returned by the listing', async () => { + setupFetch([activeKey]); + renderWithProviders(, { + appContext: { localeMessages, isPlatformAdmin: true }, + }); + await waitFor(() => expect(screen.getByText('admin')).toBeInTheDocument()); + expect(screen.queryByTestId('new-api-key-token')).not.toBeInTheDocument(); }); - it('shows delete confirmation dialog when delete icon is clicked', async () => { - setupFetch([ - { id: '1', key: 'abc123', created_by: 'admin', created_at: '2024-01-01', visible: false }, - ]); + it('shows revoke confirmation dialog when the revoke icon is clicked', async () => { + setupFetch([activeKey]); const user = userEvent.setup(); renderWithProviders(, { appContext: { localeMessages, isPlatformAdmin: true }, }); await waitFor(() => expect(screen.getByText('admin')).toBeInTheDocument()); - // Find the button containing the DeleteIcon svg - const deleteButton = screen.getAllByRole('button').find( + const revokeButton = screen.getAllByRole('button').find( (btn) => btn.querySelector('[data-testid="DeleteIcon"]') ); - await user.click(deleteButton); + await user.click(revokeButton); await waitFor(() => - expect(screen.getByText('Confirm Deletion')).toBeInTheDocument() + expect(screen.getByText('Confirm Revocation')).toBeInTheDocument() ); }); }); diff --git a/tests/jobs/api/test_views/test_check_imap_job_view.py b/tests/jobs/api/test_views/test_check_imap_job_view.py index e7cda661..f851d567 100644 --- a/tests/jobs/api/test_views/test_check_imap_job_view.py +++ b/tests/jobs/api/test_views/test_check_imap_job_view.py @@ -50,7 +50,7 @@ def test_check_imap_with_valid_api_key(mock_submit, superadmin_client): create_key_response = superadmin_client.post(reverse("django_email_learning:api_platform:api_keys_list")) response = superadmin_client.get( URL, - HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['key']}", + HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['token']}", ) assert response.status_code == 202 body = response.json() @@ -72,7 +72,7 @@ def test_check_imap_already_running_returns_409(superadmin_client): create_key_response = superadmin_client.post(reverse("django_email_learning:api_platform:api_keys_list")) response = superadmin_client.get( URL, - HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['key']}", + HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['token']}", ) assert response.status_code == 409 assert response.json() == { @@ -89,7 +89,7 @@ def test_check_imap_submission_failure_triggers_job_execution_failed_metric( create_key_response = superadmin_client.post(reverse("django_email_learning:api_platform:api_keys_list")) response = superadmin_client.get( URL, - HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['key']}", + HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['token']}", ) assert response.status_code == 500 diff --git a/tests/jobs/api/test_views/test_cleanup_job_executions_view.py b/tests/jobs/api/test_views/test_cleanup_job_executions_view.py index 5cda8f22..33dab658 100644 --- a/tests/jobs/api/test_views/test_cleanup_job_executions_view.py +++ b/tests/jobs/api/test_views/test_cleanup_job_executions_view.py @@ -51,7 +51,7 @@ def test_cleanup_job_executions_with_valid_api_key(mock_call_command, superadmin create_key_response = superadmin_client.post(reverse("django_email_learning:api_platform:api_keys_list")) response = superadmin_client.get( URL, - HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['key']}", + HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['token']}", ) assert response.status_code == 202 @@ -75,7 +75,7 @@ def test_cleanup_job_executions_failed_triggers_job_execution_failed_metric( create_key_response = superadmin_client.post(reverse("django_email_learning:api_platform:api_keys_list")) response = superadmin_client.get( URL, - HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['key']}", + HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['token']}", ) assert response.status_code == 500 @@ -93,7 +93,7 @@ def test_cleanup_job_executions_returns_400_for_invalid_days(superadmin_client): create_key_response = superadmin_client.post(reverse("django_email_learning:api_platform:api_keys_list")) response = superadmin_client.get( f"{URL}?days=invalid", - HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['key']}", + HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['token']}", ) assert response.status_code == 400 diff --git a/tests/jobs/api/test_views/test_deactivate_inactive_enrollments_job_view.py b/tests/jobs/api/test_views/test_deactivate_inactive_enrollments_job_view.py index 2fa513d3..edcb3ecb 100644 --- a/tests/jobs/api/test_views/test_deactivate_inactive_enrollments_job_view.py +++ b/tests/jobs/api/test_views/test_deactivate_inactive_enrollments_job_view.py @@ -54,7 +54,7 @@ def test_deactivate_inactive_enrollments_with_valid_api_key(mock_submit, superad create_key_response = superadmin_client.post(reverse("django_email_learning:api_platform:api_keys_list")) response = superadmin_client.get( URL, - HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['key']}", + HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['token']}", ) assert response.status_code == 202 body = response.json() @@ -76,7 +76,7 @@ def test_deactivate_inactive_enrollments_already_running_returns_409(superadmin_ create_key_response = superadmin_client.post(reverse("django_email_learning:api_platform:api_keys_list")) response = superadmin_client.get( URL, - HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['key']}", + HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['token']}", ) assert response.status_code == 409 assert response.json() == { @@ -93,7 +93,7 @@ def test_deactivate_inactive_enrollments_submission_failure_triggers_job_executi create_key_response = superadmin_client.post(reverse("django_email_learning:api_platform:api_keys_list")) response = superadmin_client.get( URL, - HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['key']}", + HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['token']}", ) assert response.status_code == 500 diff --git a/tests/jobs/api/test_views/test_deliver_contents_job_view.py b/tests/jobs/api/test_views/test_deliver_contents_job_view.py index 108e77a2..7b2eb704 100644 --- a/tests/jobs/api/test_views/test_deliver_contents_job_view.py +++ b/tests/jobs/api/test_views/test_deliver_contents_job_view.py @@ -50,7 +50,7 @@ def test_deliver_content_with_valid_api_key(mock_submit, superadmin_client): create_key_response = superadmin_client.post(reverse("django_email_learning:api_platform:api_keys_list")) response = superadmin_client.get( URL, - HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['key']}", + HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['token']}", ) assert response.status_code == 202 body = response.json() @@ -72,7 +72,7 @@ def test_deliver_content_already_running_returns_409(superadmin_client): create_key_response = superadmin_client.post(reverse("django_email_learning:api_platform:api_keys_list")) response = superadmin_client.get( URL, - HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['key']}", + HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['token']}", ) assert response.status_code == 409 assert response.json() == { @@ -89,7 +89,7 @@ def test_deliver_content_submission_failure_triggers_job_execution_failed_metric create_key_response = superadmin_client.post(reverse("django_email_learning:api_platform:api_keys_list")) response = superadmin_client.get( URL, - HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['key']}", + HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['token']}", ) assert response.status_code == 500 diff --git a/tests/jobs/api/test_views/test_job_execution_status_view.py b/tests/jobs/api/test_views/test_job_execution_status_view.py index 7522a877..5da22c67 100644 --- a/tests/jobs/api/test_views/test_job_execution_status_view.py +++ b/tests/jobs/api/test_views/test_job_execution_status_view.py @@ -33,7 +33,7 @@ def test_job_execution_status_returns_running_execution(superadmin_client): create_key_response = superadmin_client.post(reverse("django_email_learning:api_platform:api_keys_list")) response = superadmin_client.get( _status_url(job_execution.id), - HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['key']}", + HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['token']}", ) assert response.status_code == 200 body = response.json() @@ -53,7 +53,7 @@ def test_job_execution_status_returns_failed_execution_with_error(superadmin_cli create_key_response = superadmin_client.post(reverse("django_email_learning:api_platform:api_keys_list")) response = superadmin_client.get( _status_url(job_execution.id), - HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['key']}", + HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['token']}", ) assert response.status_code == 200 body = response.json() @@ -65,7 +65,7 @@ def test_job_execution_status_not_found(superadmin_client): create_key_response = superadmin_client.post(reverse("django_email_learning:api_platform:api_keys_list")) response = superadmin_client.get( _status_url(999999), - HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['key']}", + HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['token']}", ) assert response.status_code == 404 assert response.json() == {"error": "Job execution not found"} diff --git a/tests/jobs/api/test_views/test_send_newsletters_job_view.py b/tests/jobs/api/test_views/test_send_newsletters_job_view.py index 2e44a55f..8f6354a5 100644 --- a/tests/jobs/api/test_views/test_send_newsletters_job_view.py +++ b/tests/jobs/api/test_views/test_send_newsletters_job_view.py @@ -36,7 +36,7 @@ def test_send_newsletters_with_valid_api_key(mock_submit, superadmin_client): create_key_response = superadmin_client.post(reverse("django_email_learning:api_platform:api_keys_list")) response = superadmin_client.get( URL, - HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['key']}", + HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['token']}", ) assert response.status_code == 202 body = response.json() @@ -58,7 +58,7 @@ def test_send_newsletters_already_running_returns_409(superadmin_client): create_key_response = superadmin_client.post(reverse("django_email_learning:api_platform:api_keys_list")) response = superadmin_client.get( URL, - HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['key']}", + HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['token']}", ) assert response.status_code == 409 assert response.json() == { @@ -75,7 +75,7 @@ def test_send_newsletters_submission_failure_triggers_job_execution_failed_metri create_key_response = superadmin_client.post(reverse("django_email_learning:api_platform:api_keys_list")) response = superadmin_client.get( URL, - HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['key']}", + HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['token']}", ) assert response.status_code == 500 diff --git a/tests/jobs/api/test_views/test_send_reminders_job_view.py b/tests/jobs/api/test_views/test_send_reminders_job_view.py index 1d2272e4..46341121 100644 --- a/tests/jobs/api/test_views/test_send_reminders_job_view.py +++ b/tests/jobs/api/test_views/test_send_reminders_job_view.py @@ -50,7 +50,7 @@ def test_send_reminders_with_valid_api_key(mock_submit, superadmin_client): create_key_response = superadmin_client.post(reverse("django_email_learning:api_platform:api_keys_list")) response = superadmin_client.get( URL, - HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['key']}", + HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['token']}", ) assert response.status_code == 202 body = response.json() @@ -72,7 +72,7 @@ def test_send_reminders_already_running_returns_409(superadmin_client): create_key_response = superadmin_client.post(reverse("django_email_learning:api_platform:api_keys_list")) response = superadmin_client.get( URL, - HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['key']}", + HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['token']}", ) assert response.status_code == 409 assert response.json() == { @@ -89,7 +89,7 @@ def test_send_reminders_submission_failure_triggers_job_execution_failed_metric( create_key_response = superadmin_client.post(reverse("django_email_learning:api_platform:api_keys_list")) response = superadmin_client.get( URL, - HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['key']}", + HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['token']}", ) assert response.status_code == 500 diff --git a/tests/models/test_api_key.py b/tests/models/test_api_key.py new file mode 100644 index 00000000..b92c196d --- /dev/null +++ b/tests/models/test_api_key.py @@ -0,0 +1,155 @@ +import datetime + +import pytest +from django.core.exceptions import ValidationError +from django.db import IntegrityError, transaction +from django.utils import timezone + +from django_email_learning.models import ApiKey, ApiKeyScope, ApiKeyType +from django_email_learning.models.api_keys import hash_secret + + +def test_create_platform_key_returns_token_matching_stored_hash(db): + api_key, token = ApiKey.create(key_type=ApiKeyType.PLATFORM, name="Ops key") + + assert token.startswith(f"elk_{api_key.key_id}_") + _, secret = ApiKey.split_token(token) + assert api_key.matches_secret(secret) + assert api_key.secret_hash == hash_secret(secret) + + +def test_stored_key_does_not_contain_the_secret(db): + _, token = ApiKey.create(key_type=ApiKeyType.PLATFORM, name="Ops key") + _, secret = ApiKey.split_token(token) + + stored = ApiKey.objects.get(name="Ops key") + assert secret not in stored.secret_hash + # There is no field anywhere on the row that can be turned back into the token. + assert not any(secret in str(value) for value in stored.__dict__.values()) + + +def test_split_token_preserves_secrets_containing_the_delimiter(db): + """The secret is url-safe base64, whose alphabet includes the `_` delimiter, + so the split has to be bounded rather than greedy.""" + key_id, secret = ApiKey.split_token("elk_abc123_secret_with_underscores") + assert key_id == "abc123" + assert secret == "secret_with_underscores" + + +@pytest.mark.parametrize( + "token", + ["", "notatoken", "elk_only-two", "wrongprefix_abc_secret", "elk__secret", "elk_abc_"], +) +def test_split_token_rejects_malformed_tokens(token): + assert ApiKey.split_token(token) is None + + +def test_platform_key_cannot_belong_to_an_organization(db): + with pytest.raises(ValidationError): + ApiKey.create(key_type=ApiKeyType.PLATFORM, name="Bad key", organization_id=1) + + +def test_organization_key_requires_an_organization(db): + with pytest.raises(ValidationError): + ApiKey.create( + key_type=ApiKeyType.ORGANIZATION, + name="Bad key", + scopes=[ApiKeyScope.COURSES_READ], + ) + + +def test_database_constraint_rejects_mismatched_key_type(db): + """The check constraint is the backstop for writes that bypass clean(), + so that a platform key can never be produced by omitting a filter.""" + api_key, _ = ApiKey.create( + key_type=ApiKeyType.ORGANIZATION, + name="Org key", + organization_id=1, + scopes=[ApiKeyScope.COURSES_READ], + ) + with pytest.raises(IntegrityError), transaction.atomic(): + ApiKey.objects.filter(pk=api_key.pk).update(key_type=ApiKeyType.PLATFORM) + + +def test_platform_key_rejects_scopes(db): + with pytest.raises(ValidationError): + ApiKey.create( + key_type=ApiKeyType.PLATFORM, + name="Scoped platform key", + scopes=[ApiKeyScope.COURSES_READ], + ) + + +def test_unknown_scope_is_rejected(db): + with pytest.raises(ValidationError): + ApiKey.create( + key_type=ApiKeyType.ORGANIZATION, + name="Org key", + organization_id=1, + scopes=["courses:destroy"], + ) + + +def test_revoked_key_is_not_usable(db): + api_key, _ = ApiKey.create(key_type=ApiKeyType.PLATFORM, name="Ops key") + assert api_key.is_usable + + api_key.revoke() + assert api_key.is_revoked + assert not api_key.is_usable + + +def test_expired_key_is_not_usable(db): + api_key, _ = ApiKey.create( + key_type=ApiKeyType.PLATFORM, + name="Ops key", + expires_at=timezone.now() - datetime.timedelta(seconds=1), + ) + assert api_key.is_expired + assert not api_key.is_usable + + +def test_future_expiry_is_still_usable(db): + api_key, _ = ApiKey.create( + key_type=ApiKeyType.PLATFORM, + name="Ops key", + expires_at=timezone.now() + datetime.timedelta(days=1), + ) + assert not api_key.is_expired + assert api_key.is_usable + + +def test_touch_last_used_writes_once_per_resolution_window(db): + api_key, _ = ApiKey.create(key_type=ApiKeyType.PLATFORM, name="Ops key") + + api_key.touch_last_used() + api_key.refresh_from_db() + first_seen = api_key.last_used_at + assert first_seen is not None + + # A second call inside the window must not issue another write. + api_key.touch_last_used() + api_key.refresh_from_db() + assert api_key.last_used_at == first_seen + + +def test_touch_last_used_writes_again_once_stale(db): + api_key, _ = ApiKey.create(key_type=ApiKeyType.PLATFORM, name="Ops key") + stale = timezone.now() - datetime.timedelta(minutes=5) + ApiKey.objects.filter(pk=api_key.pk).update(last_used_at=stale) + api_key.refresh_from_db() + + api_key.touch_last_used() + api_key.refresh_from_db() + assert api_key.last_used_at > stale + + +def test_has_scope(db): + api_key, _ = ApiKey.create( + key_type=ApiKeyType.ORGANIZATION, + name="Org key", + organization_id=1, + scopes=[ApiKeyScope.ENROLLMENTS_WRITE], + ) + assert api_key.has_scope(ApiKeyScope.ENROLLMENTS_WRITE) + assert not api_key.has_scope(ApiKeyScope.COURSES_READ) diff --git a/tests/organization_api/__init__.py b/tests/organization_api/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/organization_api/conftest.py b/tests/organization_api/conftest.py new file mode 100644 index 00000000..2461edc7 --- /dev/null +++ b/tests/organization_api/conftest.py @@ -0,0 +1,80 @@ +import pytest +from django.core.cache import cache +from django.test import Client + +from django_email_learning.models import ( + ApiKey, + ApiKeyScope, + ApiKeyType, + Course, + Organization, +) + +ALL_SCOPES = [ + ApiKeyScope.ENROLLMENTS_WRITE, + ApiKeyScope.ENROLLMENTS_READ, + ApiKeyScope.COURSES_READ, +] + + +@pytest.fixture(autouse=True) +def clear_rate_limit_cache(): + """Rate limiting is cache-backed and the counters outlive a test, so a + later test would otherwise inherit an earlier one's budget.""" + cache.clear() + yield + cache.clear() + + +@pytest.fixture() +def api_client() -> Client: + """A plain client. The root conftest's `client` is indirectly parametrized + by role, which these key-authenticated endpoints have no use for. + """ + return Client() + + +@pytest.fixture() +def enabled_course(db, course) -> Course: + course.enabled = True + course.save() + return course + + +@pytest.fixture() +def other_organization(db) -> Organization: + organization = Organization(name="Other Organization") + organization.save() + return organization + + +@pytest.fixture() +def other_organization_course(db, other_organization) -> Course: + course = Course( + title="Other Course", + slug="other-course", + organization=other_organization, + enabled=True, + ) + course.save() + return course + + +def make_key(scopes, organization_id: int = 1) -> str: + _, token = ApiKey.create( + key_type=ApiKeyType.ORGANIZATION, + name="Test key", + organization_id=organization_id, + scopes=list(scopes), + ) + return token + + +@pytest.fixture() +def api_token(db) -> str: + return make_key(ALL_SCOPES) + + +@pytest.fixture() +def auth(api_token): + return {"HTTP_AUTHORIZATION": f"Bearer {api_token}"} diff --git a/tests/organization_api/test_authentication.py b/tests/organization_api/test_authentication.py new file mode 100644 index 00000000..0344ed99 --- /dev/null +++ b/tests/organization_api/test_authentication.py @@ -0,0 +1,141 @@ +"""Authentication and authorization for the v1 organization API. + +Exercised through the courses endpoint, which is the cheapest authenticated +view; the decorator under test is shared by every endpoint in this API. +""" + +import datetime + +import pytest +from django.urls import reverse +from django.utils import timezone + +from django_email_learning.models import ApiKey, ApiKeyScope, ApiKeyType +from django_email_learning.services.jwt_service import generate_jwt + +from .conftest import make_key + +URL = reverse("django_email_learning:api_v1:courses") + + +def test_request_without_a_key_is_rejected(api_client, db): + response = api_client.get(URL) + assert response.status_code == 401 + assert response.json() == {"error": "Authorization header missing"} + + +@pytest.mark.parametrize("header", ["Basic sometoken", "no-space", "Bearer a b"]) +def test_malformed_authorization_header_is_rejected(api_client, db, header): + response = api_client.get(URL, HTTP_AUTHORIZATION=header) + assert response.status_code == 401 + assert response.json() == {"error": "Invalid Authorization header format. Expected: Bearer "} + + +def test_unknown_key_id_is_rejected(api_client, db): + response = api_client.get(URL, HTTP_AUTHORIZATION="Bearer elk_deadbeef_notarealsecret") + assert response.status_code == 401 + assert response.json() == {"error": "Invalid API key"} + + +def test_wrong_secret_for_a_real_key_id_is_rejected(api_client, db): + api_key, _ = ApiKey.create( + key_type=ApiKeyType.ORGANIZATION, + name="Test key", + organization_id=1, + scopes=[ApiKeyScope.COURSES_READ], + ) + response = api_client.get(URL, HTTP_AUTHORIZATION=f"Bearer elk_{api_key.key_id}_wrongsecret") + assert response.status_code == 401 + # Identical to the unknown-key-id message, so a caller can't confirm which + # key ids exist by comparing responses. + assert response.json() == {"error": "Invalid API key"} + + +def test_valid_key_is_accepted(api_client, db): + token = make_key([ApiKeyScope.COURSES_READ]) + assert api_client.get(URL, HTTP_AUTHORIZATION=f"Bearer {token}").status_code == 200 + + +def test_platform_key_cannot_use_the_organization_api(api_client, db): + """A platform key carries deployment-wide authority and no organization, + so it must not fall through to an organization-scoped endpoint.""" + _, token = ApiKey.create(key_type=ApiKeyType.PLATFORM, name="Ops key") + response = api_client.get(URL, HTTP_AUTHORIZATION=f"Bearer {token}") + assert response.status_code == 403 + + +def test_organization_key_cannot_use_the_jobs_api(api_client, db): + """The mirror image: an organization key must not reach platform endpoints.""" + token = make_key([ApiKeyScope.COURSES_READ]) + response = api_client.get( + reverse("django_email_learning:api_jobs:check_imap_connections"), + HTTP_AUTHORIZATION=f"Bearer {token}", + ) + assert response.status_code == 403 + + +def test_missing_scope_is_rejected(api_client, db): + token = make_key([ApiKeyScope.ENROLLMENTS_WRITE]) + response = api_client.get(URL, HTTP_AUTHORIZATION=f"Bearer {token}") + assert response.status_code == 403 + assert "courses:read" in response.json()["error"] + + +def test_revoked_key_is_rejected(api_client, db): + api_key, token = ApiKey.create( + key_type=ApiKeyType.ORGANIZATION, + name="Test key", + organization_id=1, + scopes=[ApiKeyScope.COURSES_READ], + ) + api_key.revoke() + + response = api_client.get(URL, HTTP_AUTHORIZATION=f"Bearer {token}") + assert response.status_code == 401 + assert response.json() == {"error": "API key has been revoked"} + + +def test_expired_key_is_rejected(api_client, db): + _, token = ApiKey.create( + key_type=ApiKeyType.ORGANIZATION, + name="Test key", + organization_id=1, + scopes=[ApiKeyScope.COURSES_READ], + expires_at=timezone.now() - datetime.timedelta(seconds=1), + ) + response = api_client.get(URL, HTTP_AUTHORIZATION=f"Bearer {token}") + assert response.status_code == 401 + assert response.json() == {"error": "API key has expired"} + + +def test_successful_request_records_last_used(api_client, db): + api_key, token = ApiKey.create( + key_type=ApiKeyType.ORGANIZATION, + name="Test key", + organization_id=1, + scopes=[ApiKeyScope.COURSES_READ], + ) + assert api_key.last_used_at is None + + api_client.get(URL, HTTP_AUTHORIZATION=f"Bearer {token}") + + api_key.refresh_from_db() + assert api_key.last_used_at is not None + + +def test_legacy_jwt_credentials_still_authenticate(api_client, db): + """Keys issued before 3.1.0 were handed out as a JWT wrapping the raw key. + The backfill hashed that same value, so the old token resolves through the + new lookup and existing deployments don't break on upgrade. + """ + api_key, token = ApiKey.create(key_type=ApiKeyType.PLATFORM, name="Ops key") + _, secret = ApiKey.split_token(token) + legacy_token = generate_jwt({"key": secret, "salt": "irrelevant"}) + + response = api_client.get( + reverse("django_email_learning:api_jobs:job_execution_status", kwargs={"job_execution_id": 1}), + HTTP_AUTHORIZATION=f"Bearer {legacy_token}", + ) + # 404 rather than 401: authentication succeeded, the job execution just + # doesn't exist. + assert response.status_code == 404 diff --git a/tests/organization_api/test_courses_api.py b/tests/organization_api/test_courses_api.py new file mode 100644 index 00000000..f970003b --- /dev/null +++ b/tests/organization_api/test_courses_api.py @@ -0,0 +1,59 @@ +from unittest import mock + +from django.urls import reverse + +URL = reverse("django_email_learning:api_v1:courses") + + +def test_listing_courses(api_client, auth, enabled_course): + response = api_client.get(URL, **auth) + assert response.status_code == 200 + + courses = response.json()["courses"] + assert len(courses) == 1 + assert courses[0]["slug"] == enabled_course.slug + assert courses[0]["title"] == enabled_course.title + assert courses[0]["enabled"] is True + + +def test_listing_excludes_other_organizations_courses(api_client, auth, enabled_course, other_organization_course): + courses = api_client.get(URL, **auth).json()["courses"] + assert [c["slug"] for c in courses] == [enabled_course.slug] + + +def test_listing_includes_disabled_courses(api_client, auth, course): + """A caller needs to see a disabled course to understand why enrolling into + it fails, so the listing isn't filtered by `enabled`.""" + courses = api_client.get(URL, **auth).json()["courses"] + assert [c["enabled"] for c in courses] == [False] + + +def test_rate_limit_returns_429(api_client, auth, enabled_course): + with mock.patch( + "django_email_learning.organization_api.views.get_rate_limit_settings", + return_value={"PER_KEY_LIMIT": 2, "PER_KEY_WINDOW_SECONDS": 60}, + ): + assert api_client.get(URL, **auth).status_code == 200 + assert api_client.get(URL, **auth).status_code == 200 + response = api_client.get(URL, **auth) + + assert response.status_code == 429 + assert response.json()["error"] == "Too many requests. Please try again later." + + +def test_rate_limit_is_per_key(api_client, auth, enabled_course, db): + """Budgets are keyed on key_id, so one caller exhausting its allowance + can't lock out another key on the same organization.""" + from django_email_learning.models import ApiKeyScope + + from .conftest import make_key + + other_token = make_key([ApiKeyScope.COURSES_READ]) + + with mock.patch( + "django_email_learning.organization_api.views.get_rate_limit_settings", + return_value={"PER_KEY_LIMIT": 1, "PER_KEY_WINDOW_SECONDS": 60}, + ): + assert api_client.get(URL, **auth).status_code == 200 + assert api_client.get(URL, **auth).status_code == 429 + assert api_client.get(URL, HTTP_AUTHORIZATION=f"Bearer {other_token}").status_code == 200 diff --git a/tests/organization_api/test_enrollments_api.py b/tests/organization_api/test_enrollments_api.py new file mode 100644 index 00000000..6f3fb764 --- /dev/null +++ b/tests/organization_api/test_enrollments_api.py @@ -0,0 +1,181 @@ +import json + +from django.core import mail +from django.urls import reverse + +from django_email_learning.models import ( + ApiKeyScope, + Enrollment, + EnrollmentStatus, + Learner, +) + +from .conftest import make_key + +URL = reverse("django_email_learning:api_v1:enrollments") + + +def _post(api_client, auth, **payload): + return api_client.post(URL, data=json.dumps(payload), content_type="application/json", **auth) + + +def test_enrolling_creates_an_unverified_enrollment(api_client, auth, enabled_course): + response = _post(api_client, auth, email="learner@example.com", course_slug=enabled_course.slug) + + assert response.status_code == 201 + body = response.json() + assert body["status"] == "enrolled" + assert body["enrollment"]["email"] == "learner@example.com" + assert body["enrollment"]["course_slug"] == enabled_course.slug + assert body["enrollment"]["status"] == EnrollmentStatus.UNVERIFIED + + enrollment = Enrollment.objects.get(id=body["enrollment"]["id"]) + assert enrollment.course == enabled_course + assert enrollment.learner.organization_id == 1 + + +def test_enrolling_sends_a_verification_email(api_client, auth, enabled_course): + mail.outbox.clear() + _post(api_client, auth, email="learner@example.com", course_slug=enabled_course.slug) + assert len(mail.outbox) == 1 + assert "learner@example.com" in mail.outbox[0].to + + +def test_email_is_normalized_to_lowercase(api_client, auth, enabled_course): + response = _post(api_client, auth, email="Learner@Example.COM", course_slug=enabled_course.slug) + assert response.status_code == 201 + assert response.json()["enrollment"]["email"] == "learner@example.com" + assert Learner.objects.filter(email="learner@example.com", organization_id=1).exists() + + +def test_enrolling_twice_reports_already_enrolled(api_client, auth, enabled_course): + _post(api_client, auth, email="learner@example.com", course_slug=enabled_course.slug) + response = _post(api_client, auth, email="learner@example.com", course_slug=enabled_course.slug) + + assert response.status_code == 200 + assert response.json() == {"status": "already_enrolled"} + assert Enrollment.objects.count() == 1 + + +def test_enrolling_in_an_unknown_course_returns_404(api_client, auth, enabled_course): + response = _post(api_client, auth, email="learner@example.com", course_slug="no-such-course") + assert response.status_code == 404 + + +def test_enrolling_in_a_disabled_course_returns_404(api_client, auth, course): + """The course exists and belongs to the organization, but EnrollCommand + only accepts enabled courses.""" + assert course.enabled is False + response = _post(api_client, auth, email="learner@example.com", course_slug=course.slug) + assert response.status_code == 404 + + +def test_enrolling_in_a_private_course_is_allowed(api_client, auth, enabled_course): + """Unlike the embeddable public endpoint, an authenticated organization key + may enrol into its own non-public courses.""" + enabled_course.is_public = False + enabled_course.save() + + response = _post(api_client, auth, email="learner@example.com", course_slug=enabled_course.slug) + assert response.status_code == 201 + + +def test_cannot_enroll_into_another_organizations_course(api_client, auth, other_organization_course): + """The course slug is resolved against the key's organization, so naming + another organization's course reads as 'not found' rather than reaching it.""" + response = _post(api_client, auth, email="learner@example.com", course_slug=other_organization_course.slug) + assert response.status_code == 404 + assert not Enrollment.objects.exists() + + +def test_blocked_email_is_rejected(api_client, auth, enabled_course, blocked_email): + response = _post(api_client, auth, email=blocked_email.email, course_slug=enabled_course.slug) + assert response.status_code == 403 + assert response.json()["error"] == "Email is blocked" + + +def test_invalid_email_is_rejected(api_client, auth, enabled_course): + response = _post(api_client, auth, email="not-an-email", course_slug=enabled_course.slug) + assert response.status_code == 400 + + +def test_missing_course_slug_is_rejected(api_client, auth, enabled_course): + response = _post(api_client, auth, email="learner@example.com") + assert response.status_code == 400 + + +def test_malformed_json_is_rejected(api_client, auth, enabled_course): + response = api_client.post(URL, data="{not json", content_type="application/json", **auth) + assert response.status_code == 400 + + +def test_write_scope_is_required_to_enroll(api_client, enabled_course, db): + token = make_key([ApiKeyScope.ENROLLMENTS_READ]) + response = api_client.post( + URL, + data=json.dumps({"email": "learner@example.com", "course_slug": enabled_course.slug}), + content_type="application/json", + HTTP_AUTHORIZATION=f"Bearer {token}", + ) + assert response.status_code == 403 + assert not Enrollment.objects.exists() + + +def test_listing_enrollments(api_client, auth, enabled_course): + _post(api_client, auth, email="a@example.com", course_slug=enabled_course.slug) + _post(api_client, auth, email="b@example.com", course_slug=enabled_course.slug) + + response = api_client.get(URL, **auth) + assert response.status_code == 200 + body = response.json() + assert body["total"] == 2 + assert {e["email"] for e in body["enrollments"]} == {"a@example.com", "b@example.com"} + + +def test_listing_filters_by_email_and_course(api_client, auth, enabled_course): + _post(api_client, auth, email="a@example.com", course_slug=enabled_course.slug) + _post(api_client, auth, email="b@example.com", course_slug=enabled_course.slug) + + body = api_client.get(URL, {"email": "a@example.com"}, **auth).json() + assert body["total"] == 1 + assert body["enrollments"][0]["email"] == "a@example.com" + + body = api_client.get(URL, {"course_slug": "no-such-course"}, **auth).json() + assert body["total"] == 0 + + +def test_listing_rejects_an_unknown_status(api_client, auth, enabled_course): + assert api_client.get(URL, {"status": "banished"}, **auth).status_code == 400 + + +def test_listing_caps_the_page_size(api_client, auth, enabled_course): + assert api_client.get(URL, {"limit": "5000"}, **auth).status_code == 400 + + +def test_listing_paginates(api_client, auth, enabled_course): + for i in range(3): + _post(api_client, auth, email=f"learner{i}@example.com", course_slug=enabled_course.slug) + + body = api_client.get(URL, {"limit": 2, "offset": 0}, **auth).json() + assert body["total"] == 3 + assert len(body["enrollments"]) == 2 + + body = api_client.get(URL, {"limit": 2, "offset": 2}, **auth).json() + assert len(body["enrollments"]) == 1 + + +def test_listing_excludes_other_organizations_enrollments(api_client, auth, enabled_course, other_organization_course): + other_learner = Learner(email="elsewhere@example.com", organization=other_organization_course.organization) + other_learner.save() + Enrollment.objects.create(learner=other_learner, course=other_organization_course) + _post(api_client, auth, email="ours@example.com", course_slug=enabled_course.slug) + + body = api_client.get(URL, **auth).json() + assert body["total"] == 1 + assert body["enrollments"][0]["email"] == "ours@example.com" + + +def test_read_scope_is_required_to_list(api_client, enabled_course, db): + token = make_key([ApiKeyScope.ENROLLMENTS_WRITE]) + response = api_client.get(URL, HTTP_AUTHORIZATION=f"Bearer {token}") + assert response.status_code == 403 diff --git a/tests/platform/api/test_views/test_api_key_view.py b/tests/platform/api/test_views/test_api_key_view.py index 4b9312a0..4c23e552 100644 --- a/tests/platform/api/test_views/test_api_key_view.py +++ b/tests/platform/api/test_views/test_api_key_view.py @@ -1,33 +1,103 @@ +import json + import pytest from django.urls import reverse +from django_email_learning.models import ApiKey, ApiKeyScope, ApiKeyType + URL = reverse("django_email_learning:api_platform:api_keys_list") -def test_create_api_key(superadmin_client): +def _detail_url(api_key_id: int) -> str: + return reverse("django_email_learning:api_platform:api_keys_detail", kwargs={"api_key_id": api_key_id}) + + +def test_create_api_key_returns_the_token_once(superadmin_client): response = superadmin_client.post(URL) assert response.status_code == 201 data = response.json() - assert "id" in data - assert "key" in data - assert "created_at" in data + + assert data["token"].startswith(f"elk_{data['key_id']}_") + assert data["key_type"] == ApiKeyType.PLATFORM + assert data["name"] == "Platform key" assert data["created_by"] == "superadmin" - created_key = data["key"] + assert data["scopes"] == [] + assert data["organization_id"] is None + + +def test_listing_keys_never_returns_the_token(superadmin_client): + create_response = superadmin_client.post(URL) + token = create_response.json()["token"] response = superadmin_client.get(URL) assert response.status_code == 200 - data = response.json() - assert "api_keys" in data - api_keys = data["api_keys"] - assert any(api_key["key"] == created_key for api_key in api_keys) + api_keys = response.json()["api_keys"] + + assert len(api_keys) == 1 + assert api_keys[0]["key_id"] == create_response.json()["key_id"] + # The whole point of hashing: nothing in the listing can be replayed. + assert "token" not in api_keys[0] + assert token not in json.dumps(api_keys) + + +def test_create_api_key_accepts_a_name(superadmin_client): + response = superadmin_client.post( + URL, + data=json.dumps({"name": "CI runner"}), + content_type="application/json", + ) + assert response.status_code == 201 + assert response.json()["name"] == "CI runner" + + +def test_listing_excludes_organization_keys(superadmin_client, db): + ApiKey.create( + key_type=ApiKeyType.ORGANIZATION, + name="Org key", + organization_id=1, + scopes=[ApiKeyScope.COURSES_READ], + ) + superadmin_client.post(URL) + + api_keys = superadmin_client.get(URL).json()["api_keys"] + assert [key["key_type"] for key in api_keys] == [ApiKeyType.PLATFORM] + + +def test_revoking_a_key_keeps_the_row(superadmin_client): + api_key_id = superadmin_client.post(URL).json()["id"] + + response = superadmin_client.delete(_detail_url(api_key_id)) + assert response.status_code == 200 + assert response.json() == {"message": "API Key revoked successfully"} + + # Kept rather than deleted so the audit trail of what existed survives. + api_key = ApiKey.objects.get(id=api_key_id) + assert api_key.revoked_at is not None + assert not api_key.is_usable + + +def test_revoking_an_unknown_key_returns_404(superadmin_client, db): + assert superadmin_client.delete(_detail_url(9999)).status_code == 404 + + +def test_platform_delete_cannot_reach_an_organization_key(superadmin_client, db): + org_key, _ = ApiKey.create( + key_type=ApiKeyType.ORGANIZATION, + name="Org key", + organization_id=1, + scopes=[ApiKeyScope.COURSES_READ], + ) + assert superadmin_client.delete(_detail_url(org_key.id)).status_code == 404 @pytest.mark.parametrize("client", ["editor", "viewer", "instructor"], indirect=["client"]) def test_organization_user_cannot_create_api_key(client): - response = client.post(URL) - assert response.status_code == 403 + assert client.post(URL).status_code == 403 def test_platform_admin_can_create_api_key(platform_admin_client): - response = platform_admin_client.post(URL) - assert response.status_code == 201 + assert platform_admin_client.post(URL).status_code == 201 + + +def test_anonymous_cannot_create_api_key(anonymous_client, db): + assert anonymous_client.post(URL).status_code == 401 diff --git a/tests/platform/api/test_views/test_organization_api_key_view.py b/tests/platform/api/test_views/test_organization_api_key_view.py new file mode 100644 index 00000000..c9dedf5d --- /dev/null +++ b/tests/platform/api/test_views/test_organization_api_key_view.py @@ -0,0 +1,173 @@ +import json + +import pytest +from django.contrib.auth.models import User +from django.test import Client +from django.urls import reverse + +from django_email_learning.models import ( + ApiKey, + ApiKeyScope, + ApiKeyType, + Organization, + OrganizationUser, +) + + +def _list_url(organization_id: int = 1) -> str: + return reverse( + "django_email_learning:api_platform:organization_api_keys_list", + kwargs={"organization_id": organization_id}, + ) + + +def _detail_url(api_key_id: int, organization_id: int = 1) -> str: + return reverse( + "django_email_learning:api_platform:organization_api_keys_detail", + kwargs={"organization_id": organization_id, "api_key_id": api_key_id}, + ) + + +def _create_payload(**overrides) -> dict: + return {"name": "Partner integration", "scopes": [ApiKeyScope.ENROLLMENTS_WRITE.value], **overrides} + + +@pytest.fixture() +def other_organization(db) -> Organization: + organization = Organization(name="Other Organization") + organization.save() + return organization + + +@pytest.fixture() +def other_org_admin_client(db, users, other_organization) -> Client: + # Depends on `users` so the fixture's explicitly-numbered rows are inserted + # before this one claims an auto id. + user = User.objects.create(username="otherorgadmin", email="other@example.com") + OrganizationUser.objects.create(user=user, organization=other_organization, role="admin") + client = Client() + client.force_login(user) + session = client.session + session["active_organization_id"] = other_organization.id + session.save() + return client + + +def test_org_admin_can_create_a_scoped_key(org_admin_client): + response = org_admin_client.post( + _list_url(), + data=json.dumps(_create_payload(scopes=[ApiKeyScope.ENROLLMENTS_WRITE.value, ApiKeyScope.COURSES_READ.value])), + content_type="application/json", + ) + assert response.status_code == 201 + data = response.json() + + assert data["token"].startswith(f"elk_{data['key_id']}_") + assert data["key_type"] == ApiKeyType.ORGANIZATION + assert data["organization_id"] == 1 + assert data["scopes"] == [ApiKeyScope.COURSES_READ.value, ApiKeyScope.ENROLLMENTS_WRITE.value] + assert data["created_by"] == "orgadmin" + + +def test_created_key_is_scoped_to_the_url_organization(org_admin_client): + org_admin_client.post(_list_url(), data=json.dumps(_create_payload()), content_type="application/json") + assert ApiKey.objects.get(key_type=ApiKeyType.ORGANIZATION).organization_id == 1 + + +def test_listing_never_returns_the_token(org_admin_client): + token = org_admin_client.post( + _list_url(), data=json.dumps(_create_payload()), content_type="application/json" + ).json()["token"] + + response = org_admin_client.get(_list_url()) + assert response.status_code == 200 + assert token not in json.dumps(response.json()) + + +def test_listing_only_returns_this_organizations_keys(org_admin_client, other_organization): + ApiKey.create( + key_type=ApiKeyType.ORGANIZATION, + name="Someone else's key", + organization_id=other_organization.id, + scopes=[ApiKeyScope.COURSES_READ], + ) + org_admin_client.post(_list_url(), data=json.dumps(_create_payload()), content_type="application/json") + + api_keys = org_admin_client.get(_list_url()).json()["api_keys"] + assert [key["name"] for key in api_keys] == ["Partner integration"] + + +def test_listing_excludes_platform_keys(org_admin_client): + ApiKey.create(key_type=ApiKeyType.PLATFORM, name="Ops key") + assert org_admin_client.get(_list_url()).json()["api_keys"] == [] + + +def test_unknown_scope_is_rejected(org_admin_client): + response = org_admin_client.post( + _list_url(), + data=json.dumps(_create_payload(scopes=["courses:destroy"])), + content_type="application/json", + ) + assert response.status_code == 400 + + +def test_scopes_are_required(org_admin_client): + response = org_admin_client.post( + _list_url(), + data=json.dumps({"name": "No scopes"}), + content_type="application/json", + ) + assert response.status_code == 400 + + +def test_revoking_a_key(org_admin_client): + api_key_id = org_admin_client.post( + _list_url(), data=json.dumps(_create_payload()), content_type="application/json" + ).json()["id"] + + response = org_admin_client.delete(_detail_url(api_key_id)) + assert response.status_code == 200 + assert ApiKey.objects.get(id=api_key_id).revoked_at is not None + + +def test_admin_cannot_revoke_another_organizations_key(other_org_admin_client, org_admin_client, other_organization): + """The lookup filters on organization as well as id, so guessing a key id + from another organization must not be enough to revoke it.""" + api_key_id = org_admin_client.post( + _list_url(), data=json.dumps(_create_payload()), content_type="application/json" + ).json()["id"] + + response = other_org_admin_client.delete(_detail_url(api_key_id, organization_id=other_organization.id)) + assert response.status_code == 404 + assert ApiKey.objects.get(id=api_key_id).revoked_at is None + + +def test_admin_cannot_create_a_key_for_another_organization(other_org_admin_client): + response = other_org_admin_client.post( + _list_url(organization_id=1), + data=json.dumps(_create_payload()), + content_type="application/json", + ) + assert response.status_code == 403 + + +def test_admin_cannot_list_another_organizations_keys(other_org_admin_client): + assert other_org_admin_client.get(_list_url(organization_id=1)).status_code == 403 + + +@pytest.mark.parametrize("client", ["editor", "viewer", "instructor"], indirect=["client"]) +def test_non_admin_members_cannot_create_a_key(client): + """A key acts with whatever scopes it carries, so issuing one would let a + non-admin hand out access it doesn't itself have.""" + response = client.post(_list_url(), data=json.dumps(_create_payload()), content_type="application/json") + assert response.status_code == 403 + + +@pytest.mark.parametrize("client", ["editor", "viewer", "instructor"], indirect=["client"]) +def test_non_admin_members_cannot_list_keys(client): + assert client.get(_list_url()).status_code == 403 + + +def test_anonymous_cannot_create_a_key(anonymous_client, db): + response = anonymous_client.post(_list_url(), data=json.dumps(_create_payload()), content_type="application/json") + assert response.status_code == 401 From 58924a7c1f21c639375470a61712f6b0f88387ce Mon Sep 17 00:00:00 2001 From: Payam Date: Fri, 7 Aug 2026 19:30:51 +0400 Subject: [PATCH 2/5] Narrow ApiKeyScope to enrollments:create and require a scope The v1 API only needs enrollment creation for now, so the scope enumeration drops to a single member. The two read endpoints go with it rather than being left guarded by a create scope, which would grant reads on the strength of a write permission. Organization keys must now carry at least one scope. Every organization endpoint requires one, so a scopeless key is a credential that authenticates and can then do nothing; rejecting it at creation beats issuing something that 403s on every call. Enforced in clean() rather than as a database constraint - unlike the key_type/organization constraint, whose failure mode is a key gaining deployment-wide authority, this one fails closed, so a backstop below the model buys hygiene rather than safety. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 2 +- django_email_learning/models/api_keys.py | 11 ++- .../organization_api/serializers.py | 55 +---------- .../organization_api/urls.py | 3 +- .../organization_api/views.py | 54 +---------- docs/source/technical/organization-api.rst | 45 ++------- tests/models/test_api_key.py | 29 ++++-- tests/organization_api/conftest.py | 10 +- tests/organization_api/test_authentication.py | 71 +++++++++----- tests/organization_api/test_courses_api.py | 59 ----------- .../organization_api/test_enrollments_api.py | 97 +++++-------------- .../api/test_views/test_api_key_view.py | 4 +- .../test_organization_api_key_view.py | 31 +++++- 13 files changed, 147 insertions(+), 324 deletions(-) delete mode 100644 tests/organization_api/test_courses_api.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 5304112c..b7addf9c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ Changes prior to v1.0.0 are available in the [git history](https://github.com/Av ### Added -- **Organization API keys and a new organization-scoped API** — Organization admins can now issue API keys for their own organization via `POST /api/platform/organizations//api-keys/`, and use them against a new `/api/v1/` surface: create an enrollment (`POST /api/v1/enrollments/`), list enrollments (`GET /api/v1/enrollments/`), and list courses (`GET /api/v1/courses/`). Keys carry explicit scopes — `enrollments:write`, `enrollments:read`, `courses:read` — and an optional expiry. The organization is taken from the key itself rather than from the URL or request body, so a key can only ever act on the organization it was issued for; a slug or id belonging to another organization reads as `404`. Only organization *admins* can issue keys, since a key acts with whatever scopes it carries. Requests are rate limited per key (defaults 120/60s, configurable via `DJANGO_EMAIL_LEARNING["ORGANIZATION_API_RATE_LIMITS"]`). This is separate from the existing unauthenticated `/api/public/` embed surface. Management is API-only for now — there is no organization-facing settings screen for these keys yet. See the new [Organization API](https://django-email-learning.readthedocs.io/en/latest/technical/organization-api.html) reference. +- **Organization API keys and a new organization-scoped API** — Organization admins can now issue API keys for their own organization via `POST /api/platform/organizations//api-keys/`, and use them against a new `/api/v1/` surface. v1 covers one endpoint, `POST /api/v1/enrollments/`, which enrolls an email address in one of the organization's courses. Keys carry explicit scopes — `enrollments:create` is the only one for now, and an organization key must carry at least one — plus an optional expiry. The organization is taken from the key itself rather than from the URL or request body, so a key can only ever act on the organization it was issued for; a slug or id belonging to another organization reads as `404`. Only organization *admins* can issue keys, since a key acts with whatever scopes it carries. Requests are rate limited per key (defaults 120/60s, configurable via `DJANGO_EMAIL_LEARNING["ORGANIZATION_API_RATE_LIMITS"]`). This is separate from the existing unauthenticated `/api/public/` embed surface. Management is API-only for now — there is no organization-facing settings screen for these keys yet. See the new [Organization API](https://django-email-learning.readthedocs.io/en/latest/technical/organization-api.html) reference. - **API keys now support naming, expiry, revocation and last-used tracking** — Both platform and organization keys take a `name` and an optional `expires_at`, record `last_used_at` on each authenticated request (at minute resolution, so recording activity doesn't cost a write per request), and can be revoked. ### Security diff --git a/django_email_learning/models/api_keys.py b/django_email_learning/models/api_keys.py index 5a35f5ed..9e7451c8 100644 --- a/django_email_learning/models/api_keys.py +++ b/django_email_learning/models/api_keys.py @@ -31,14 +31,12 @@ class ApiKeyType(models.TextChoices): class ApiKeyScope(models.TextChoices): """Permissions an organization key can carry. - Deliberately coarse: a scope names a resource and an access level, not an + Deliberately coarse: a scope names a resource and an action, not an endpoint, so adding an endpoint to an existing resource doesn't strand callers on a key that predates it. """ - COURSES_READ = "courses:read", "Read courses" - ENROLLMENTS_READ = "enrollments:read", "Read enrollments" - ENROLLMENTS_WRITE = "enrollments:write", "Create enrollments" + ENROLLMENTS_CREATE = "enrollments:create", "Create enrollments" def hash_secret(secret: str) -> str: @@ -172,6 +170,11 @@ def clean(self) -> None: # nothing for a scope to narrow them to. if self.key_type == ApiKeyType.PLATFORM and self.scopes: raise ValidationError({"scopes": "Platform keys do not take scopes."}) + # Every organization endpoint requires a scope, so a scopeless key is a + # credential that authenticates and can then do nothing. Rejecting it at + # creation beats handing someone a key that 403s on every call. + if self.key_type == ApiKeyType.ORGANIZATION and not self.scopes: + raise ValidationError({"scopes": "Organization keys must carry at least one scope."}) invalid_scopes = set(self.scopes) - set(ApiKeyScope.values) if invalid_scopes: raise ValidationError({"scopes": f"Unknown scopes: {', '.join(sorted(invalid_scopes))}."}) diff --git a/django_email_learning/organization_api/serializers.py b/django_email_learning/organization_api/serializers.py index 78ea710d..9ba7c4c0 100644 --- a/django_email_learning/organization_api/serializers.py +++ b/django_email_learning/organization_api/serializers.py @@ -1,9 +1,9 @@ from datetime import datetime -from typing import List, Optional +from typing import Optional from pydantic import BaseModel, ConfigDict, Field, field_validator -from django_email_learning.models import Course, Enrollment +from django_email_learning.models import Enrollment from django_email_learning.public.api.serializers import EmailValidatedRequest @@ -18,32 +18,6 @@ def normalize_email(cls, value: str) -> str: return value.lower() -class CourseResponse(BaseModel): - id: int - slug: str - title: str - description: Optional[str] = None - language: str - enabled: bool - is_public: bool - - @staticmethod - def from_django_model(course: Course) -> "CourseResponse": - return CourseResponse.model_validate( - { - "id": course.id, - "slug": course.slug, - "title": course.title, - "description": course.description, - "language": course.language, - "enabled": course.enabled, - "is_public": course.is_public, - } - ) - - model_config = ConfigDict(from_attributes=True) - - class EnrollmentResponse(BaseModel): id: int email: str @@ -66,28 +40,3 @@ def from_django_model(enrollment: Enrollment) -> "EnrollmentResponse": ) model_config = ConfigDict(from_attributes=True) - - -class EnrollmentListQuery(BaseModel): - """Query-string parameters for listing enrollments. - - `limit` is capped rather than unbounded so a caller can't turn one request - into a full table scan of a large organization. - """ - - course_slug: Optional[str] = None - email: Optional[str] = None - status: Optional[str] = None - limit: int = Field(default=50, ge=1, le=200) - offset: int = Field(default=0, ge=0) - - @field_validator("email") - def normalize_email(cls, value: Optional[str]) -> Optional[str]: - return value.lower() if value else value - - -class PaginatedEnrollmentsResponse(BaseModel): - enrollments: List[EnrollmentResponse] - total: int - limit: int - offset: int diff --git a/django_email_learning/organization_api/urls.py b/django_email_learning/organization_api/urls.py index 066bd582..40862d05 100644 --- a/django_email_learning/organization_api/urls.py +++ b/django_email_learning/organization_api/urls.py @@ -1,10 +1,9 @@ from django.urls import path -from django_email_learning.organization_api.views import CoursesView, EnrollmentsView +from django_email_learning.organization_api.views import EnrollmentsView app_name = "django_email_learning" urlpatterns = [ path("enrollments/", EnrollmentsView.as_view(), name="enrollments"), - path("courses/", CoursesView.as_view(), name="courses"), ] diff --git a/django_email_learning/organization_api/views.py b/django_email_learning/organization_api/views.py index 06e50f06..2adbd37a 100644 --- a/django_email_learning/organization_api/views.py +++ b/django_email_learning/organization_api/views.py @@ -26,7 +26,6 @@ ApiKeyScope, Course, Enrollment, - EnrollmentStatus, NewsletterSubscriber, ) from django_email_learning.organization_api import serializers @@ -82,8 +81,7 @@ def check_rate_limit(self, request) -> JsonResponse | None: # type: ignore[no-u @method_decorator(csrf_exempt, name="dispatch") -@method_decorator(require_organization_api_key(scopes=[ApiKeyScope.ENROLLMENTS_WRITE]), name="post") -@method_decorator(require_organization_api_key(scopes=[ApiKeyScope.ENROLLMENTS_READ]), name="get") +@method_decorator(require_organization_api_key(scopes=[ApiKeyScope.ENROLLMENTS_CREATE]), name="post") class EnrollmentsView(RateLimitedApiView): def post(self, request, *args, **kwargs) -> JsonResponse: # type: ignore[no-untyped-def] rate_limited = self.check_rate_limit(request) @@ -166,53 +164,3 @@ def post(self, request, *args, **kwargs) -> JsonResponse: # type: ignore[no-unt }, status=201, ) - - def get(self, request, *args, **kwargs) -> JsonResponse: # type: ignore[no-untyped-def] - rate_limited = self.check_rate_limit(request) - if rate_limited: - return rate_limited - - try: - query = serializers.EnrollmentListQuery.model_validate(request.GET.dict()) - except ValidationError as e: - return JsonResponse({"error": e.json()}, status=400) - - if query.status is not None and query.status not in {status.value for status in EnrollmentStatus}: - return JsonResponse({"error": f"Unknown status '{query.status}'"}, status=400) - - enrollments = Enrollment.objects.filter(course__organization_id=request.organization.id).select_related( - "learner", "course" - ) - if query.course_slug: - enrollments = enrollments.filter(course__slug=query.course_slug) - if query.email: - enrollments = enrollments.filter(learner__email=query.email) - if query.status: - enrollments = enrollments.filter(status=query.status) - - total = enrollments.count() - page = enrollments.order_by("-enrolled_at")[query.offset : query.offset + query.limit] - - return JsonResponse( - serializers.PaginatedEnrollmentsResponse( - enrollments=[serializers.EnrollmentResponse.from_django_model(e) for e in page], - total=total, - limit=query.limit, - offset=query.offset, - ).model_dump(mode="json"), - status=200, - ) - - -@method_decorator(require_organization_api_key(scopes=[ApiKeyScope.COURSES_READ]), name="get") -class CoursesView(RateLimitedApiView): - def get(self, request, *args, **kwargs) -> JsonResponse: # type: ignore[no-untyped-def] - rate_limited = self.check_rate_limit(request) - if rate_limited: - return rate_limited - - courses = Course.objects.filter(organization_id=request.organization.id).order_by("title") - return JsonResponse( - {"courses": [serializers.CourseResponse.from_django_model(c).model_dump(mode="json") for c in courses]}, - status=200, - ) diff --git a/docs/source/technical/organization-api.rst b/docs/source/technical/organization-api.rst index a2b00c85..ad059f48 100644 --- a/docs/source/technical/organization-api.rst +++ b/docs/source/technical/organization-api.rst @@ -38,7 +38,7 @@ issuing one would let a non-admin hand out access it does not itself have. { "name": "Partner signup integration", - "scopes": ["enrollments:write"], + "scopes": ["enrollments:create"], "expires_at": "2027-01-01T00:00:00Z" } @@ -55,7 +55,7 @@ The response is the **only** time the token is readable: "name": "Partner signup integration", "key_type": "organization", "organization_id": 3, - "scopes": ["enrollments:write"], + "scopes": ["enrollments:create"], "created_at": "2026-08-07T10:00:00Z", "created_by": "orgadmin", "expires_at": "2027-01-01T00:00:00Z", @@ -81,16 +81,12 @@ Scopes * - Scope - Grants - * - ``courses:read`` - - List the organization's courses - * - ``enrollments:read`` - - List the organization's enrollments - * - ``enrollments:write`` + * - ``enrollments:create`` - Create enrollments -A scope names a resource and an access level rather than an endpoint, so -adding an endpoint to an existing resource does not strand callers on a key -that predates it. +A scope names a resource and an action rather than an endpoint, so adding an +endpoint to an existing resource does not strand callers on a key that predates +it. More scopes will be added as the API grows; a key must carry at least one. Authentication -------------- @@ -111,7 +107,7 @@ Endpoints Create an enrollment ^^^^^^^^^^^^^^^^^^^^ -Requires ``enrollments:write``. +Requires ``enrollments:create``. .. code-block:: http @@ -138,31 +134,8 @@ Responses: * ``403`` — the email is blocked, or the organization is at its learner cap * ``404`` — no such enabled course in this organization -List enrollments -^^^^^^^^^^^^^^^^ - -Requires ``enrollments:read``. - -.. code-block:: http - - GET /api/v1/enrollments/?course_slug=intro-to-widgets&status=active&limit=50&offset=0 - -Optional filters: ``course_slug``, ``email``, ``status`` (one of -``unverified``, ``active``, ``completed``, ``deactivated``). ``limit`` defaults -to 50 and is capped at 200. The response carries ``enrollments``, ``total``, -``limit`` and ``offset``. - -List courses -^^^^^^^^^^^^ - -Requires ``courses:read``. - -.. code-block:: http - - GET /api/v1/courses/ - -Returns every course in the organization, including disabled ones — a caller -needs to see a disabled course to understand why enrolling into it failed. +Creating an enrollment is the only endpoint in v1. Read endpoints for +enrollments and courses will follow, each behind its own scope. Rate Limiting ------------- diff --git a/tests/models/test_api_key.py b/tests/models/test_api_key.py index b92c196d..016154fa 100644 --- a/tests/models/test_api_key.py +++ b/tests/models/test_api_key.py @@ -54,7 +54,7 @@ def test_organization_key_requires_an_organization(db): ApiKey.create( key_type=ApiKeyType.ORGANIZATION, name="Bad key", - scopes=[ApiKeyScope.COURSES_READ], + scopes=[ApiKeyScope.ENROLLMENTS_CREATE], ) @@ -65,7 +65,7 @@ def test_database_constraint_rejects_mismatched_key_type(db): key_type=ApiKeyType.ORGANIZATION, name="Org key", organization_id=1, - scopes=[ApiKeyScope.COURSES_READ], + scopes=[ApiKeyScope.ENROLLMENTS_CREATE], ) with pytest.raises(IntegrityError), transaction.atomic(): ApiKey.objects.filter(pk=api_key.pk).update(key_type=ApiKeyType.PLATFORM) @@ -76,7 +76,7 @@ def test_platform_key_rejects_scopes(db): ApiKey.create( key_type=ApiKeyType.PLATFORM, name="Scoped platform key", - scopes=[ApiKeyScope.COURSES_READ], + scopes=[ApiKeyScope.ENROLLMENTS_CREATE], ) @@ -149,7 +149,24 @@ def test_has_scope(db): key_type=ApiKeyType.ORGANIZATION, name="Org key", organization_id=1, - scopes=[ApiKeyScope.ENROLLMENTS_WRITE], + scopes=[ApiKeyScope.ENROLLMENTS_CREATE], ) - assert api_key.has_scope(ApiKeyScope.ENROLLMENTS_WRITE) - assert not api_key.has_scope(ApiKeyScope.COURSES_READ) + assert api_key.has_scope(ApiKeyScope.ENROLLMENTS_CREATE) + assert not api_key.has_scope("something:else") + + +def test_organization_key_requires_at_least_one_scope(db): + """A scopeless organization key would authenticate and then be refused by + every endpoint, so it's rejected at creation rather than issued.""" + with pytest.raises(ValidationError): + ApiKey.create(key_type=ApiKeyType.ORGANIZATION, name="Scopeless key", organization_id=1) + + with pytest.raises(ValidationError): + ApiKey.create(key_type=ApiKeyType.ORGANIZATION, name="Scopeless key", organization_id=1, scopes=[]) + + +def test_platform_key_needs_no_scopes(db): + """The rule is specific to organization keys - platform keys are + all-or-nothing and must stay scopeless.""" + api_key, _ = ApiKey.create(key_type=ApiKeyType.PLATFORM, name="Ops key") + assert api_key.scopes == [] diff --git a/tests/organization_api/conftest.py b/tests/organization_api/conftest.py index 2461edc7..b51edefc 100644 --- a/tests/organization_api/conftest.py +++ b/tests/organization_api/conftest.py @@ -10,11 +10,7 @@ Organization, ) -ALL_SCOPES = [ - ApiKeyScope.ENROLLMENTS_WRITE, - ApiKeyScope.ENROLLMENTS_READ, - ApiKeyScope.COURSES_READ, -] +ALL_SCOPES = [ApiKeyScope.ENROLLMENTS_CREATE] @pytest.fixture(autouse=True) @@ -60,7 +56,7 @@ def other_organization_course(db, other_organization) -> Course: return course -def make_key(scopes, organization_id: int = 1) -> str: +def make_key(scopes=ALL_SCOPES, organization_id: int = 1) -> str: _, token = ApiKey.create( key_type=ApiKeyType.ORGANIZATION, name="Test key", @@ -72,7 +68,7 @@ def make_key(scopes, organization_id: int = 1) -> str: @pytest.fixture() def api_token(db) -> str: - return make_key(ALL_SCOPES) + return make_key() @pytest.fixture() diff --git a/tests/organization_api/test_authentication.py b/tests/organization_api/test_authentication.py index 0344ed99..ec51d234 100644 --- a/tests/organization_api/test_authentication.py +++ b/tests/organization_api/test_authentication.py @@ -1,10 +1,12 @@ """Authentication and authorization for the v1 organization API. -Exercised through the courses endpoint, which is the cheapest authenticated -view; the decorator under test is shared by every endpoint in this API. +Exercised through the enrollments endpoint; the decorator under test is shared +by every endpoint in this API. Rejections happen in the decorator, before the +view body runs, so most of these need no valid request payload. """ import datetime +import json import pytest from django.urls import reverse @@ -15,24 +17,33 @@ from .conftest import make_key -URL = reverse("django_email_learning:api_v1:courses") +URL = reverse("django_email_learning:api_v1:enrollments") + + +def _post(api_client, course_slug="sample-course", **headers): + return api_client.post( + URL, + data=json.dumps({"email": "learner@example.com", "course_slug": course_slug}), + content_type="application/json", + **headers, + ) def test_request_without_a_key_is_rejected(api_client, db): - response = api_client.get(URL) + response = _post(api_client) assert response.status_code == 401 assert response.json() == {"error": "Authorization header missing"} @pytest.mark.parametrize("header", ["Basic sometoken", "no-space", "Bearer a b"]) def test_malformed_authorization_header_is_rejected(api_client, db, header): - response = api_client.get(URL, HTTP_AUTHORIZATION=header) + response = _post(api_client, HTTP_AUTHORIZATION=header) assert response.status_code == 401 assert response.json() == {"error": "Invalid Authorization header format. Expected: Bearer "} def test_unknown_key_id_is_rejected(api_client, db): - response = api_client.get(URL, HTTP_AUTHORIZATION="Bearer elk_deadbeef_notarealsecret") + response = _post(api_client, HTTP_AUTHORIZATION="Bearer elk_deadbeef_notarealsecret") assert response.status_code == 401 assert response.json() == {"error": "Invalid API key"} @@ -42,31 +53,32 @@ def test_wrong_secret_for_a_real_key_id_is_rejected(api_client, db): key_type=ApiKeyType.ORGANIZATION, name="Test key", organization_id=1, - scopes=[ApiKeyScope.COURSES_READ], + scopes=[ApiKeyScope.ENROLLMENTS_CREATE], ) - response = api_client.get(URL, HTTP_AUTHORIZATION=f"Bearer elk_{api_key.key_id}_wrongsecret") + response = _post(api_client, HTTP_AUTHORIZATION=f"Bearer elk_{api_key.key_id}_wrongsecret") assert response.status_code == 401 # Identical to the unknown-key-id message, so a caller can't confirm which # key ids exist by comparing responses. assert response.json() == {"error": "Invalid API key"} -def test_valid_key_is_accepted(api_client, db): - token = make_key([ApiKeyScope.COURSES_READ]) - assert api_client.get(URL, HTTP_AUTHORIZATION=f"Bearer {token}").status_code == 200 +def test_valid_key_is_accepted(api_client, enabled_course, db): + token = make_key() + response = _post(api_client, course_slug=enabled_course.slug, HTTP_AUTHORIZATION=f"Bearer {token}") + assert response.status_code == 201 def test_platform_key_cannot_use_the_organization_api(api_client, db): """A platform key carries deployment-wide authority and no organization, so it must not fall through to an organization-scoped endpoint.""" _, token = ApiKey.create(key_type=ApiKeyType.PLATFORM, name="Ops key") - response = api_client.get(URL, HTTP_AUTHORIZATION=f"Bearer {token}") + response = _post(api_client, HTTP_AUTHORIZATION=f"Bearer {token}") assert response.status_code == 403 def test_organization_key_cannot_use_the_jobs_api(api_client, db): """The mirror image: an organization key must not reach platform endpoints.""" - token = make_key([ApiKeyScope.COURSES_READ]) + token = make_key() response = api_client.get( reverse("django_email_learning:api_jobs:check_imap_connections"), HTTP_AUTHORIZATION=f"Bearer {token}", @@ -74,11 +86,22 @@ def test_organization_key_cannot_use_the_jobs_api(api_client, db): assert response.status_code == 403 -def test_missing_scope_is_rejected(api_client, db): - token = make_key([ApiKeyScope.ENROLLMENTS_WRITE]) - response = api_client.get(URL, HTTP_AUTHORIZATION=f"Bearer {token}") +def test_key_without_the_required_scope_is_rejected(api_client, enabled_course, db): + """A key can't be *created* without scopes, but one can outlive the scope an + endpoint needs — if a scope is later renamed or removed from a key. The + decorator has to reject that rather than fall through. + """ + api_key, token = ApiKey.create( + key_type=ApiKeyType.ORGANIZATION, + name="Test key", + organization_id=1, + scopes=[ApiKeyScope.ENROLLMENTS_CREATE], + ) + ApiKey.objects.filter(pk=api_key.pk).update(scopes=["something:else"]) + + response = _post(api_client, course_slug=enabled_course.slug, HTTP_AUTHORIZATION=f"Bearer {token}") assert response.status_code == 403 - assert "courses:read" in response.json()["error"] + assert "enrollments:create" in response.json()["error"] def test_revoked_key_is_rejected(api_client, db): @@ -86,11 +109,11 @@ def test_revoked_key_is_rejected(api_client, db): key_type=ApiKeyType.ORGANIZATION, name="Test key", organization_id=1, - scopes=[ApiKeyScope.COURSES_READ], + scopes=[ApiKeyScope.ENROLLMENTS_CREATE], ) api_key.revoke() - response = api_client.get(URL, HTTP_AUTHORIZATION=f"Bearer {token}") + response = _post(api_client, HTTP_AUTHORIZATION=f"Bearer {token}") assert response.status_code == 401 assert response.json() == {"error": "API key has been revoked"} @@ -100,24 +123,24 @@ def test_expired_key_is_rejected(api_client, db): key_type=ApiKeyType.ORGANIZATION, name="Test key", organization_id=1, - scopes=[ApiKeyScope.COURSES_READ], + scopes=[ApiKeyScope.ENROLLMENTS_CREATE], expires_at=timezone.now() - datetime.timedelta(seconds=1), ) - response = api_client.get(URL, HTTP_AUTHORIZATION=f"Bearer {token}") + response = _post(api_client, HTTP_AUTHORIZATION=f"Bearer {token}") assert response.status_code == 401 assert response.json() == {"error": "API key has expired"} -def test_successful_request_records_last_used(api_client, db): +def test_successful_request_records_last_used(api_client, enabled_course, db): api_key, token = ApiKey.create( key_type=ApiKeyType.ORGANIZATION, name="Test key", organization_id=1, - scopes=[ApiKeyScope.COURSES_READ], + scopes=[ApiKeyScope.ENROLLMENTS_CREATE], ) assert api_key.last_used_at is None - api_client.get(URL, HTTP_AUTHORIZATION=f"Bearer {token}") + _post(api_client, course_slug=enabled_course.slug, HTTP_AUTHORIZATION=f"Bearer {token}") api_key.refresh_from_db() assert api_key.last_used_at is not None diff --git a/tests/organization_api/test_courses_api.py b/tests/organization_api/test_courses_api.py deleted file mode 100644 index f970003b..00000000 --- a/tests/organization_api/test_courses_api.py +++ /dev/null @@ -1,59 +0,0 @@ -from unittest import mock - -from django.urls import reverse - -URL = reverse("django_email_learning:api_v1:courses") - - -def test_listing_courses(api_client, auth, enabled_course): - response = api_client.get(URL, **auth) - assert response.status_code == 200 - - courses = response.json()["courses"] - assert len(courses) == 1 - assert courses[0]["slug"] == enabled_course.slug - assert courses[0]["title"] == enabled_course.title - assert courses[0]["enabled"] is True - - -def test_listing_excludes_other_organizations_courses(api_client, auth, enabled_course, other_organization_course): - courses = api_client.get(URL, **auth).json()["courses"] - assert [c["slug"] for c in courses] == [enabled_course.slug] - - -def test_listing_includes_disabled_courses(api_client, auth, course): - """A caller needs to see a disabled course to understand why enrolling into - it fails, so the listing isn't filtered by `enabled`.""" - courses = api_client.get(URL, **auth).json()["courses"] - assert [c["enabled"] for c in courses] == [False] - - -def test_rate_limit_returns_429(api_client, auth, enabled_course): - with mock.patch( - "django_email_learning.organization_api.views.get_rate_limit_settings", - return_value={"PER_KEY_LIMIT": 2, "PER_KEY_WINDOW_SECONDS": 60}, - ): - assert api_client.get(URL, **auth).status_code == 200 - assert api_client.get(URL, **auth).status_code == 200 - response = api_client.get(URL, **auth) - - assert response.status_code == 429 - assert response.json()["error"] == "Too many requests. Please try again later." - - -def test_rate_limit_is_per_key(api_client, auth, enabled_course, db): - """Budgets are keyed on key_id, so one caller exhausting its allowance - can't lock out another key on the same organization.""" - from django_email_learning.models import ApiKeyScope - - from .conftest import make_key - - other_token = make_key([ApiKeyScope.COURSES_READ]) - - with mock.patch( - "django_email_learning.organization_api.views.get_rate_limit_settings", - return_value={"PER_KEY_LIMIT": 1, "PER_KEY_WINDOW_SECONDS": 60}, - ): - assert api_client.get(URL, **auth).status_code == 200 - assert api_client.get(URL, **auth).status_code == 429 - assert api_client.get(URL, HTTP_AUTHORIZATION=f"Bearer {other_token}").status_code == 200 diff --git a/tests/organization_api/test_enrollments_api.py b/tests/organization_api/test_enrollments_api.py index 6f3fb764..45bb1970 100644 --- a/tests/organization_api/test_enrollments_api.py +++ b/tests/organization_api/test_enrollments_api.py @@ -1,16 +1,10 @@ import json +from unittest import mock from django.core import mail from django.urls import reverse -from django_email_learning.models import ( - ApiKeyScope, - Enrollment, - EnrollmentStatus, - Learner, -) - -from .conftest import make_key +from django_email_learning.models import Enrollment, EnrollmentStatus, Learner URL = reverse("django_email_learning:api_v1:enrollments") @@ -109,73 +103,30 @@ def test_malformed_json_is_rejected(api_client, auth, enabled_course): assert response.status_code == 400 -def test_write_scope_is_required_to_enroll(api_client, enabled_course, db): - token = make_key([ApiKeyScope.ENROLLMENTS_READ]) - response = api_client.post( - URL, - data=json.dumps({"email": "learner@example.com", "course_slug": enabled_course.slug}), - content_type="application/json", - HTTP_AUTHORIZATION=f"Bearer {token}", - ) - assert response.status_code == 403 - assert not Enrollment.objects.exists() - - -def test_listing_enrollments(api_client, auth, enabled_course): - _post(api_client, auth, email="a@example.com", course_slug=enabled_course.slug) - _post(api_client, auth, email="b@example.com", course_slug=enabled_course.slug) - - response = api_client.get(URL, **auth) - assert response.status_code == 200 - body = response.json() - assert body["total"] == 2 - assert {e["email"] for e in body["enrollments"]} == {"a@example.com", "b@example.com"} - - -def test_listing_filters_by_email_and_course(api_client, auth, enabled_course): - _post(api_client, auth, email="a@example.com", course_slug=enabled_course.slug) - _post(api_client, auth, email="b@example.com", course_slug=enabled_course.slug) - - body = api_client.get(URL, {"email": "a@example.com"}, **auth).json() - assert body["total"] == 1 - assert body["enrollments"][0]["email"] == "a@example.com" +def test_rate_limit_returns_429(api_client, auth, enabled_course): + with mock.patch( + "django_email_learning.organization_api.views.get_rate_limit_settings", + return_value={"PER_KEY_LIMIT": 2, "PER_KEY_WINDOW_SECONDS": 60}, + ): + assert _post(api_client, auth, email="a@example.com", course_slug=enabled_course.slug).status_code == 201 + assert _post(api_client, auth, email="b@example.com", course_slug=enabled_course.slug).status_code == 201 + response = _post(api_client, auth, email="c@example.com", course_slug=enabled_course.slug) - body = api_client.get(URL, {"course_slug": "no-such-course"}, **auth).json() - assert body["total"] == 0 + assert response.status_code == 429 + assert response.json()["error"] == "Too many requests. Please try again later." -def test_listing_rejects_an_unknown_status(api_client, auth, enabled_course): - assert api_client.get(URL, {"status": "banished"}, **auth).status_code == 400 +def test_rate_limit_is_per_key(api_client, auth, enabled_course, db): + """Budgets are keyed on key_id, so one caller exhausting its allowance + can't lock out another key on the same organization.""" + from .conftest import make_key + other_auth = {"HTTP_AUTHORIZATION": f"Bearer {make_key()}"} -def test_listing_caps_the_page_size(api_client, auth, enabled_course): - assert api_client.get(URL, {"limit": "5000"}, **auth).status_code == 400 - - -def test_listing_paginates(api_client, auth, enabled_course): - for i in range(3): - _post(api_client, auth, email=f"learner{i}@example.com", course_slug=enabled_course.slug) - - body = api_client.get(URL, {"limit": 2, "offset": 0}, **auth).json() - assert body["total"] == 3 - assert len(body["enrollments"]) == 2 - - body = api_client.get(URL, {"limit": 2, "offset": 2}, **auth).json() - assert len(body["enrollments"]) == 1 - - -def test_listing_excludes_other_organizations_enrollments(api_client, auth, enabled_course, other_organization_course): - other_learner = Learner(email="elsewhere@example.com", organization=other_organization_course.organization) - other_learner.save() - Enrollment.objects.create(learner=other_learner, course=other_organization_course) - _post(api_client, auth, email="ours@example.com", course_slug=enabled_course.slug) - - body = api_client.get(URL, **auth).json() - assert body["total"] == 1 - assert body["enrollments"][0]["email"] == "ours@example.com" - - -def test_read_scope_is_required_to_list(api_client, enabled_course, db): - token = make_key([ApiKeyScope.ENROLLMENTS_WRITE]) - response = api_client.get(URL, HTTP_AUTHORIZATION=f"Bearer {token}") - assert response.status_code == 403 + with mock.patch( + "django_email_learning.organization_api.views.get_rate_limit_settings", + return_value={"PER_KEY_LIMIT": 1, "PER_KEY_WINDOW_SECONDS": 60}, + ): + assert _post(api_client, auth, email="a@example.com", course_slug=enabled_course.slug).status_code == 201 + assert _post(api_client, auth, email="b@example.com", course_slug=enabled_course.slug).status_code == 429 + assert _post(api_client, other_auth, email="c@example.com", course_slug=enabled_course.slug).status_code == 201 diff --git a/tests/platform/api/test_views/test_api_key_view.py b/tests/platform/api/test_views/test_api_key_view.py index 4c23e552..e0e9bec8 100644 --- a/tests/platform/api/test_views/test_api_key_view.py +++ b/tests/platform/api/test_views/test_api_key_view.py @@ -55,7 +55,7 @@ def test_listing_excludes_organization_keys(superadmin_client, db): key_type=ApiKeyType.ORGANIZATION, name="Org key", organization_id=1, - scopes=[ApiKeyScope.COURSES_READ], + scopes=[ApiKeyScope.ENROLLMENTS_CREATE], ) superadmin_client.post(URL) @@ -85,7 +85,7 @@ def test_platform_delete_cannot_reach_an_organization_key(superadmin_client, db) key_type=ApiKeyType.ORGANIZATION, name="Org key", organization_id=1, - scopes=[ApiKeyScope.COURSES_READ], + scopes=[ApiKeyScope.ENROLLMENTS_CREATE], ) assert superadmin_client.delete(_detail_url(org_key.id)).status_code == 404 diff --git a/tests/platform/api/test_views/test_organization_api_key_view.py b/tests/platform/api/test_views/test_organization_api_key_view.py index c9dedf5d..05006325 100644 --- a/tests/platform/api/test_views/test_organization_api_key_view.py +++ b/tests/platform/api/test_views/test_organization_api_key_view.py @@ -29,7 +29,7 @@ def _detail_url(api_key_id: int, organization_id: int = 1) -> str: def _create_payload(**overrides) -> dict: - return {"name": "Partner integration", "scopes": [ApiKeyScope.ENROLLMENTS_WRITE.value], **overrides} + return {"name": "Partner integration", "scopes": [ApiKeyScope.ENROLLMENTS_CREATE.value], **overrides} @pytest.fixture() @@ -56,7 +56,7 @@ def other_org_admin_client(db, users, other_organization) -> Client: def test_org_admin_can_create_a_scoped_key(org_admin_client): response = org_admin_client.post( _list_url(), - data=json.dumps(_create_payload(scopes=[ApiKeyScope.ENROLLMENTS_WRITE.value, ApiKeyScope.COURSES_READ.value])), + data=json.dumps(_create_payload()), content_type="application/json", ) assert response.status_code == 201 @@ -65,10 +65,33 @@ def test_org_admin_can_create_a_scoped_key(org_admin_client): assert data["token"].startswith(f"elk_{data['key_id']}_") assert data["key_type"] == ApiKeyType.ORGANIZATION assert data["organization_id"] == 1 - assert data["scopes"] == [ApiKeyScope.COURSES_READ.value, ApiKeyScope.ENROLLMENTS_WRITE.value] + assert data["scopes"] == [ApiKeyScope.ENROLLMENTS_CREATE.value] assert data["created_by"] == "orgadmin" +def test_repeated_scopes_are_deduplicated(org_admin_client): + """The stored list should match what the caller sees back, so a repeated + scope collapses rather than being persisted twice.""" + response = org_admin_client.post( + _list_url(), + data=json.dumps( + _create_payload(scopes=[ApiKeyScope.ENROLLMENTS_CREATE.value, ApiKeyScope.ENROLLMENTS_CREATE.value]) + ), + content_type="application/json", + ) + assert response.status_code == 201 + assert response.json()["scopes"] == [ApiKeyScope.ENROLLMENTS_CREATE.value] + + +def test_empty_scopes_are_rejected(org_admin_client): + response = org_admin_client.post( + _list_url(), + data=json.dumps(_create_payload(scopes=[])), + content_type="application/json", + ) + assert response.status_code == 400 + + def test_created_key_is_scoped_to_the_url_organization(org_admin_client): org_admin_client.post(_list_url(), data=json.dumps(_create_payload()), content_type="application/json") assert ApiKey.objects.get(key_type=ApiKeyType.ORGANIZATION).organization_id == 1 @@ -89,7 +112,7 @@ def test_listing_only_returns_this_organizations_keys(org_admin_client, other_or key_type=ApiKeyType.ORGANIZATION, name="Someone else's key", organization_id=other_organization.id, - scopes=[ApiKeyScope.COURSES_READ], + scopes=[ApiKeyScope.ENROLLMENTS_CREATE], ) org_admin_client.post(_list_url(), data=json.dumps(_create_payload()), content_type="application/json") From 1a874f71c510ca58496fef6d009076b17c0bb6d5 Mon Sep 17 00:00:00 2001 From: Payam Date: Fri, 7 Aug 2026 19:57:50 +0400 Subject: [PATCH 3/5] Generate an OpenAPI 3.1 schema for the v1 API from the code Serves GET /api/v1/openapi.json, built by reading the implementation rather than by maintaining a parallel description of it: - paths come from the URLconf that is actually routed, including the mount prefix the including project chose, so a library consumer who mounts the app elsewhere gets correct paths - request and response schemas come from the Pydantic models the views validate and serialise with; Pydantic v2 emits JSON Schema 2020-12, which is what OpenAPI 3.1 consumes, so nothing is translated - security requirements come from the scopes the auth decorator enforces, which it now publishes, so the documented scope and the checked scope are the same value Only the prose and the status-code map are written by hand. A test fails if a routed endpoint has no OperationSpec, and a second test proves that guard can actually fire, so an endpoint added without documentation breaks the build instead of shipping a partial document. Output was checked against openapi-spec-validator as a one-off; the validator is deliberately not added as a dependency. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 1 + django_email_learning/decorators.py | 4 + .../organization_api/openapi.py | 202 ++++++++++++++++++ .../organization_api/serializers.py | 25 ++- .../organization_api/urls.py | 3 +- .../organization_api/views.py | 85 +++++++- docs/source/technical/organization-api.rst | 26 +++ tests/organization_api/test_openapi.py | 142 ++++++++++++ 8 files changed, 475 insertions(+), 13 deletions(-) create mode 100644 django_email_learning/organization_api/openapi.py create mode 100644 tests/organization_api/test_openapi.py diff --git a/CHANGELOG.md b/CHANGELOG.md index b7addf9c..9cc579b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ Changes prior to v1.0.0 are available in the [git history](https://github.com/Av ### Added - **Organization API keys and a new organization-scoped API** — Organization admins can now issue API keys for their own organization via `POST /api/platform/organizations//api-keys/`, and use them against a new `/api/v1/` surface. v1 covers one endpoint, `POST /api/v1/enrollments/`, which enrolls an email address in one of the organization's courses. Keys carry explicit scopes — `enrollments:create` is the only one for now, and an organization key must carry at least one — plus an optional expiry. The organization is taken from the key itself rather than from the URL or request body, so a key can only ever act on the organization it was issued for; a slug or id belonging to another organization reads as `404`. Only organization *admins* can issue keys, since a key acts with whatever scopes it carries. Requests are rate limited per key (defaults 120/60s, configurable via `DJANGO_EMAIL_LEARNING["ORGANIZATION_API_RATE_LIMITS"]`). This is separate from the existing unauthenticated `/api/public/` embed surface. Management is API-only for now — there is no organization-facing settings screen for these keys yet. See the new [Organization API](https://django-email-learning.readthedocs.io/en/latest/technical/organization-api.html) reference. +- **OpenAPI 3.1 schema for the organization API** — `GET /api/v1/openapi.json` serves a machine-readable description of the v1 API for Swagger UI, Redoc or a client generator. It takes no API key, since it describes the API's shape and carries no organization data; set `DJANGO_EMAIL_LEARNING["ORGANIZATION_API_DOCS_ENABLED"] = False` to stop serving it. The document is generated from the running code rather than maintained separately — paths from the URLconf, schemas from the Pydantic models the views validate with, and security requirements from the scopes the auth decorator enforces — and a test fails the build if a routed endpoint has no documentation. No new dependency. - **API keys now support naming, expiry, revocation and last-used tracking** — Both platform and organization keys take a `name` and an optional `expires_at`, record `last_used_at` on each authenticated request (at minute resolution, so recording activity doesn't cost a write per request), and can be revoked. ### Security diff --git a/django_email_learning/decorators.py b/django_email_learning/decorators.py index badc3f0c..1f17b72c 100644 --- a/django_email_learning/decorators.py +++ b/django_email_learning/decorators.py @@ -202,6 +202,10 @@ def _wrapped_view(request, *view_args, **view_kwargs) -> JsonResponse: # type: request.organization = api_key.organization return view_func(request, *view_args, **view_kwargs) + # Published so the OpenAPI generator can read the scopes off the view + # rather than being told them a second time. The documented security + # requirement is then the enforced one by construction, and can't drift. + _wrapped_view.required_api_key_scopes = frozenset(required_scopes) # type: ignore[attr-defined] return _wrapped_view return decorator diff --git a/django_email_learning/organization_api/openapi.py b/django_email_learning/organization_api/openapi.py new file mode 100644 index 00000000..031fbb6e --- /dev/null +++ b/django_email_learning/organization_api/openapi.py @@ -0,0 +1,202 @@ +"""Builds an OpenAPI 3.1 document for the v1 API from the code that serves it. + +Three things are read rather than restated, so the document can't drift from +the implementation: + +* **Paths** come from the URLconf that is actually routed. +* **Schemas** come from the Pydantic models the views validate and serialise + with. Pydantic v2 emits JSON Schema 2020-12, which is what OpenAPI 3.1 uses, + so no translation is needed. +* **Security** comes from the scopes the auth decorator enforces, which it + publishes as ``required_api_key_scopes``. + +What is declared by hand is the operation prose and the status-code map, in an +``openapi_operations`` attribute on each view. `test_openapi.py` fails if a +routed endpoint has no entry, so adding one without documenting it breaks the +build rather than silently shipping an incomplete document. +""" + +import re +import typing + +from django.urls import reverse +from pydantic import BaseModel + +OPENAPI_VERSION = "3.1.0" +API_VERSION = "1.0.0" +SECURITY_SCHEME_NAME = "organizationApiKey" +SCHEMA_REF_TEMPLATE = "#/components/schemas/{model}" + +# Django path converters, mapped to the OpenAPI types they accept. +_CONVERTER_TYPES = { + "int": {"type": "integer"}, + "str": {"type": "string"}, + "slug": {"type": "string"}, + "uuid": {"type": "string", "format": "uuid"}, + "path": {"type": "string"}, +} +_PATH_PARAM_RE = re.compile(r"<(?:(?P[^:>]+):)?(?P[^>]+)>") + + +class ResponseSpec(typing.NamedTuple): + """One documented status code. `model` is None for a body-less response.""" + + description: str + model: type[BaseModel] | None = None + + +class OperationSpec(typing.NamedTuple): + """The prose and status-code map for one operation. + + Deliberately does not carry the path, the method or the required scopes: + those are read from the routing and the decorator, and duplicating them + here would create exactly the drift this module exists to avoid. + + `operation_id` names the generated client method, so it is set explicitly + rather than derived from the path — the mount prefix is the including + project's choice, and generated clients shouldn't rename themselves + because a deployment moved the API. + """ + + operation_id: str + summary: str + description: str = "" + request: type[BaseModel] | None = None + responses: dict[int, ResponseSpec] = {} + + +def _openapi_path(route: str, prefix: str) -> str: + """Converts a Django route to an OpenAPI path template.""" + return prefix + _PATH_PARAM_RE.sub(lambda m: "{" + m.group("name") + "}", route) + + +def _path_parameters(route: str) -> list[dict]: + return [ + { + "name": match.group("name"), + "in": "path", + "required": True, + "schema": _CONVERTER_TYPES.get(match.group("converter") or "str", {"type": "string"}), + } + for match in _PATH_PARAM_RE.finditer(route) + ] + + +def _urlpatterns() -> list: + """Imported at call time, not module scope: the views this documents import + `OperationSpec` from here, and the URLconf imports those views.""" + from django_email_learning.organization_api import urls as organization_api_urls + + return organization_api_urls.urlpatterns + + +def _mount_prefix() -> str: + """Where the v1 URLconf is actually mounted. + + Derived by reversing a real route and removing its own segment, rather than + hardcoding ``/api/v1/`` — the including project chooses the prefix, and a + library can't assume it. + """ + pattern = _urlpatterns()[0] + full_path = reverse(f"django_email_learning:api_v1:{pattern.name}") + route = str(pattern.pattern) + return full_path[: len(full_path) - len(route)] + + +def _model_schema(model: type[BaseModel], components: dict) -> dict: + """Returns a ``$ref`` to `model`, registering it and its dependencies. + + Pydantic emits nested models into ``$defs``; those are lifted into the + shared components section so that models referenced from more than one + operation are defined once. + """ + schema = model.model_json_schema(ref_template=SCHEMA_REF_TEMPLATE) + components.update(schema.pop("$defs", {})) + components[model.__name__] = schema + return {"$ref": SCHEMA_REF_TEMPLATE.format(model=model.__name__)} + + +def _routed_operations() -> typing.Iterator[tuple[str, str, str, typing.Callable, OperationSpec | None]]: + """Yields (route, openapi_path, http_method, handler, spec) for every + routed v1 endpoint, including any the view has not documented.""" + prefix = _mount_prefix() + for pattern in _urlpatterns(): + view_class = getattr(pattern.callback, "view_class", None) + if view_class is None or getattr(view_class, "openapi_exclude", False): + continue + route = str(pattern.pattern) + specs = getattr(view_class, "openapi_operations", {}) + for method in view_class.http_method_names: + handler = getattr(view_class, method, None) + if handler is None or method == "options": + continue + yield route, _openapi_path(route, prefix), method, handler, specs.get(method) + + +def build_openapi_schema() -> dict: + components: dict = {} + paths: dict = {} + + for route, path, method, handler, spec in _routed_operations(): + if spec is None: + continue + + operation: dict = {"summary": spec.summary, "operationId": spec.operation_id} + if spec.description: + operation["description"] = spec.description + + parameters = _path_parameters(route) + if parameters: + operation["parameters"] = parameters + + # Read off the decorator rather than the spec, so the documented + # requirement is the enforced one. + scopes = getattr(handler, "required_api_key_scopes", None) + if scopes is not None: + operation["security"] = [{SECURITY_SCHEME_NAME: sorted(str(scope) for scope in scopes)}] + + if spec.request is not None: + operation["requestBody"] = { + "required": True, + "content": {"application/json": {"schema": _model_schema(spec.request, components)}}, + } + + operation["responses"] = { + str(status): ( + { + "description": response.description, + "content": {"application/json": {"schema": _model_schema(response.model, components)}}, + } + if response.model is not None + else {"description": response.description} + ) + for status, response in sorted(spec.responses.items()) + } + + paths.setdefault(path, {})[method] = operation + + return { + "openapi": OPENAPI_VERSION, + "info": { + "title": "Django Email Learning — Organization API", + "version": API_VERSION, + "description": ( + "Organization-scoped API authenticated with an organization API key. " + "Every request acts on the organization its key was issued for." + ), + }, + "paths": paths, + "components": { + "schemas": components, + "securitySchemes": { + SECURITY_SCHEME_NAME: { + "type": "http", + "scheme": "bearer", + "description": ( + "An organization API key, sent as `Authorization: Bearer elk__`. " + "The listed scopes must all be present on the key." + ), + } + }, + }, + } diff --git a/django_email_learning/organization_api/serializers.py b/django_email_learning/organization_api/serializers.py index 9ba7c4c0..0ea86977 100644 --- a/django_email_learning/organization_api/serializers.py +++ b/django_email_learning/organization_api/serializers.py @@ -1,5 +1,5 @@ from datetime import datetime -from typing import Optional +from typing import Literal, Optional from pydantic import BaseModel, ConfigDict, Field, field_validator @@ -40,3 +40,26 @@ def from_django_model(enrollment: Enrollment) -> "EnrollmentResponse": ) model_config = ConfigDict(from_attributes=True) + + +class EnrollmentCreatedResponse(BaseModel): + status: Literal["enrolled"] = "enrolled" + enrollment: Optional[EnrollmentResponse] = None + + +class AlreadyEnrolledResponse(BaseModel): + status: Literal["already_enrolled"] = "already_enrolled" + + +class ErrorResponse(BaseModel): + error: str + + +class ErrorWithReferenceResponse(ErrorResponse): + """Errors whose detail is withheld from the caller and logged instead. + + `error_id` correlates the response an integrator reports back to the full + detail in the server logs - see `django_email_learning.error_responses`. + """ + + error_id: str diff --git a/django_email_learning/organization_api/urls.py b/django_email_learning/organization_api/urls.py index 40862d05..8c33f350 100644 --- a/django_email_learning/organization_api/urls.py +++ b/django_email_learning/organization_api/urls.py @@ -1,9 +1,10 @@ from django.urls import path -from django_email_learning.organization_api.views import EnrollmentsView +from django_email_learning.organization_api.views import EnrollmentsView, OpenApiSchemaView app_name = "django_email_learning" urlpatterns = [ path("enrollments/", EnrollmentsView.as_view(), name="enrollments"), + path("openapi.json", OpenApiSchemaView.as_view(), name="openapi_schema"), ] diff --git a/django_email_learning/organization_api/views.py b/django_email_learning/organization_api/views.py index 2adbd37a..3dd1a03b 100644 --- a/django_email_learning/organization_api/views.py +++ b/django_email_learning/organization_api/views.py @@ -29,6 +29,17 @@ NewsletterSubscriber, ) from django_email_learning.organization_api import serializers +from django_email_learning.organization_api.openapi import ( + OperationSpec, + ResponseSpec, + build_openapi_schema, +) +from django_email_learning.organization_api.serializers import ( + AlreadyEnrolledResponse, + EnrollmentCreatedResponse, + ErrorResponse, + ErrorWithReferenceResponse, +) from django_email_learning.public.api.rate_limiting import is_rate_limited from django_email_learning.services.command_models.enroll_command import EnrollCommand from django_email_learning.services.command_models.exceptions.blocked_email_error import ( @@ -83,6 +94,39 @@ def check_rate_limit(self, request) -> JsonResponse | None: # type: ignore[no-u @method_decorator(csrf_exempt, name="dispatch") @method_decorator(require_organization_api_key(scopes=[ApiKeyScope.ENROLLMENTS_CREATE]), name="post") class EnrollmentsView(RateLimitedApiView): + openapi_operations = { + "post": OperationSpec( + operation_id="createEnrollment", + summary="Enrol an email address in a course", + description=( + "Creates an unverified enrollment and emails the learner a verification link; " + "the enrollment becomes active once they confirm. The course is resolved against " + "the organization the API key was issued for, so a slug belonging to another " + "organization is reported as not found. The course must be enabled, but — unlike " + "the embeddable public endpoint — it does not need to be public." + ), + request=serializers.EnrollmentCreateRequest, + responses={ + 201: ResponseSpec("Enrollment created, pending the learner's verification.", EnrollmentCreatedResponse), + 200: ResponseSpec( + "A non-deactivated enrollment already exists for this learner and course; " + "nothing was created and no email was sent.", + AlreadyEnrolledResponse, + ), + 400: ResponseSpec("The request body is malformed or fails validation.", ErrorResponse), + 401: ResponseSpec("The API key is missing, malformed, unknown, revoked or expired.", ErrorResponse), + 403: ResponseSpec( + "The key lacks the required scope or is not an organization key; " + "or the email is blocked, or the organization is at its learner cap.", + ErrorWithReferenceResponse, + ), + 404: ResponseSpec("No enabled course with that slug in this organization.", ErrorResponse), + 429: ResponseSpec("The key's request budget for the current window is exhausted.", ErrorResponse), + 500: ResponseSpec("The enrollment could not be completed.", ErrorWithReferenceResponse), + }, + ) + } + def post(self, request, *args, **kwargs) -> JsonResponse: # type: ignore[no-untyped-def] rate_limited = self.check_rate_limit(request) if rate_limited: @@ -114,7 +158,7 @@ def post(self, request, *args, **kwargs) -> JsonResponse: # type: ignore[no-unt try: command.execute() except EnrollmentAlreadyExistsError: - return JsonResponse({"status": "already_enrolled"}, status=200) + return JsonResponse(serializers.AlreadyEnrolledResponse().model_dump(mode="json"), status=200) except InvalidCourseSlugError: return JsonResponse({"error": "Course not found"}, status=404) except BlockedEmailError as e: @@ -153,14 +197,33 @@ def post(self, request, *args, **kwargs) -> JsonResponse: # type: ignore[no-unt payload.course_slug, organization_id, ) - if enrollment is None: - # Shouldn't happen - execute() succeeded - but returning a body - # that claims an id we couldn't read would be worse than saying so. - return JsonResponse({"status": "enrolled"}, status=201) - return JsonResponse( - { - "status": "enrolled", - "enrollment": serializers.EnrollmentResponse.from_django_model(enrollment).model_dump(mode="json"), - }, - status=201, + # `enrollment` is None only if execute() succeeded but the row couldn't + # be read back, which shouldn't happen. Omitting the key beats returning + # one that claims an id we don't have. + response = serializers.EnrollmentCreatedResponse( + enrollment=serializers.EnrollmentResponse.from_django_model(enrollment) if enrollment else None ) + return JsonResponse(response.model_dump(mode="json", exclude_none=True), status=201) + + +def organization_api_docs_enabled() -> bool: + return bool(getattr(settings, "DJANGO_EMAIL_LEARNING", {}).get("ORGANIZATION_API_DOCS_ENABLED", True)) + + +class OpenApiSchemaView(View): + """Serves the v1 OpenAPI document. + + Unauthenticated: it describes the shape of the API and carries no + organization data, so it's the same information as the published reference. + Deployments that would rather not advertise the surface can set + ``DJANGO_EMAIL_LEARNING["ORGANIZATION_API_DOCS_ENABLED"] = False``. + """ + + # The document describes the API; listing itself in it adds nothing. This + # is the only way to be routed without a spec — see test_openapi.py. + openapi_exclude = True + + def get(self, request, *args, **kwargs) -> JsonResponse: # type: ignore[no-untyped-def] + if not organization_api_docs_enabled(): + return JsonResponse({"error": "Not found"}, status=404) + return JsonResponse(build_openapi_schema(), json_dumps_params={"indent": 2}) diff --git a/docs/source/technical/organization-api.rst b/docs/source/technical/organization-api.rst index ad059f48..bb3a8b34 100644 --- a/docs/source/technical/organization-api.rst +++ b/docs/source/technical/organization-api.rst @@ -137,6 +137,32 @@ Responses: Creating an enrollment is the only endpoint in v1. Read endpoints for enrollments and courses will follow, each behind its own scope. +OpenAPI Schema +-------------- + +.. code-block:: http + + GET /api/v1/openapi.json + +Returns an OpenAPI 3.1 document for this API, suitable for feeding to Swagger +UI, Redoc, or a client generator. It needs no API key: it describes the shape +of the API and carries no organization data. + +The document is generated from the code that serves the API rather than +maintained alongside it — paths come from the URLconf, request and response +schemas from the Pydantic models the views validate with, and the security +requirements from the scopes the authentication decorator enforces. A test +fails the build if a routed endpoint has no documentation, so the two cannot +drift apart. + +To stop serving it: + +.. code-block:: python + + DJANGO_EMAIL_LEARNING = { + "ORGANIZATION_API_DOCS_ENABLED": False, + } + Rate Limiting ------------- diff --git a/tests/organization_api/test_openapi.py b/tests/organization_api/test_openapi.py new file mode 100644 index 00000000..eb48a25b --- /dev/null +++ b/tests/organization_api/test_openapi.py @@ -0,0 +1,142 @@ +import json + +import pytest +from django.test import override_settings +from django.urls import reverse + +from django_email_learning.organization_api import serializers +from django_email_learning.organization_api.openapi import ( + SCHEMA_REF_TEMPLATE, + SECURITY_SCHEME_NAME, + _routed_operations, + build_openapi_schema, +) + +URL = reverse("django_email_learning:api_v1:openapi_schema") + + +def _refs(node) -> set: + """Every $ref string anywhere in the document.""" + if isinstance(node, dict): + found = {node["$ref"]} if "$ref" in node else set() + return found.union(*(_refs(value) for value in node.values())) if node else found + if isinstance(node, list): + return set().union(*(_refs(item) for item in node)) if node else set() + return set() + + +def test_schema_endpoint_serves_the_document(api_client, db): + response = api_client.get(URL) + assert response.status_code == 200 + assert response["Content-Type"] == "application/json" + + schema = json.loads(response.content) + assert schema["openapi"].startswith("3.1") + assert schema["info"]["title"] + assert schema["paths"] + + +def test_schema_endpoint_needs_no_api_key(api_client, db): + """The document describes the API's shape and contains no organization + data, so it isn't gated behind a credential.""" + assert "HTTP_AUTHORIZATION" not in api_client.defaults + assert api_client.get(URL).status_code == 200 + + +@override_settings(DJANGO_EMAIL_LEARNING={"ORGANIZATION_API_DOCS_ENABLED": False}) +def test_schema_endpoint_can_be_disabled(api_client, db): + assert api_client.get(URL).status_code == 404 + + +def test_every_routed_endpoint_is_documented(): + """The drift guard. Adding a v1 endpoint without an OperationSpec fails + here rather than silently shipping an incomplete document. + """ + undocumented = [f"{method.upper()} {path}" for _, path, method, _, spec in _routed_operations() if spec is None] + assert undocumented == [], f"v1 endpoints missing an OperationSpec: {undocumented}" + + +def test_drift_guard_detects_an_undocumented_endpoint(monkeypatch): + """Proves the guard above can actually fail. Without this, a bug that made + `_routed_operations` yield nothing would leave it passing vacuously.""" + from django_email_learning.organization_api.views import EnrollmentsView + + monkeypatch.setattr(EnrollmentsView, "openapi_operations", {}, raising=False) + assert [method for _, _, method, _, spec in _routed_operations() if spec is None] == ["post"] + + +def test_documented_scopes_are_the_enforced_scopes(): + """Security requirements are read off the auth decorator, not restated in + the spec, so the document can't claim a scope the code doesn't check.""" + schema = build_openapi_schema() + + checked = 0 + for _, path, method, handler, spec in _routed_operations(): + if spec is None: + continue + enforced = getattr(handler, "required_api_key_scopes", None) + if enforced is None: + continue + documented = schema["paths"][path][method]["security"][0][SECURITY_SCHEME_NAME] + assert documented == sorted(str(scope) for scope in enforced) + checked += 1 + + assert checked > 0, "no scoped operations were checked - the guarantee is untested" + + +def test_path_matches_real_routing(): + """Paths come from the URLconf, so they follow wherever the including + project mounts the API rather than assuming /api/v1/.""" + schema = build_openapi_schema() + assert reverse("django_email_learning:api_v1:enrollments") in schema["paths"] + + +def test_request_schema_comes_from_the_pydantic_model(): + schema = build_openapi_schema() + operation = schema["paths"][reverse("django_email_learning:api_v1:enrollments")]["post"] + + ref = operation["requestBody"]["content"]["application/json"]["schema"]["$ref"] + assert ref == SCHEMA_REF_TEMPLATE.format(model="EnrollmentCreateRequest") + + documented = schema["components"]["schemas"]["EnrollmentCreateRequest"] + assert set(documented["properties"]) == set(serializers.EnrollmentCreateRequest.model_json_schema()["properties"]) + + +def test_nested_models_are_lifted_into_components(): + """EnrollmentCreatedResponse nests EnrollmentResponse; Pydantic emits that + into $defs, which has to be hoisted or the refs dangle.""" + schema = build_openapi_schema() + assert "EnrollmentResponse" in schema["components"]["schemas"] + assert "$defs" not in schema["components"]["schemas"]["EnrollmentCreatedResponse"] + + +def test_no_dangling_references(): + schema = build_openapi_schema() + defined = {SCHEMA_REF_TEMPLATE.format(model=name) for name in schema["components"]["schemas"]} + assert _refs(schema["paths"]) <= defined + assert _refs(schema["components"]["schemas"]) <= defined + + +def test_every_documented_response_has_a_description(): + """OpenAPI requires it, and an empty one renders as a blank row in every + tool that consumes this.""" + schema = build_openapi_schema() + for path, methods in schema["paths"].items(): + for method, operation in methods.items(): + for status, response in operation["responses"].items(): + assert response["description"].strip(), f"{method.upper()} {path} -> {status}" + + +def test_operation_ids_are_unique_and_path_independent(): + """operationId names the generated client method, so it must not embed the + deployment's mount prefix.""" + schema = build_openapi_schema() + operation_ids = [operation["operationId"] for methods in schema["paths"].values() for operation in methods.values()] + assert len(operation_ids) == len(set(operation_ids)) + assert all("/" not in operation_id for operation_id in operation_ids) + + +@pytest.mark.parametrize("scheme_field", ["type", "scheme"]) +def test_security_scheme_is_declared(scheme_field): + schema = build_openapi_schema() + assert schema["components"]["securitySchemes"][SECURITY_SCHEME_NAME][scheme_field] From 905fb52ff4f982f3fbb09c38571f8bc1f61734b6 Mon Sep 17 00:00:00 2001 From: Payam Date: Fri, 7 Aug 2026 20:46:44 +0400 Subject: [PATCH 4/5] Add an API Keys tab to the organization page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Organization admins can now issue, review and revoke their own keys from the UI rather than only over HTTP. The table follows the platform API keys screen — key id, status, created by, created at, last used — plus the scopes each key carries, which platform keys don't have. The available scopes come from ApiKeyScope via the page context rather than being listed again in the frontend, so the choices the form offers cannot drift from the scopes the API will accept. As on the platform screen there is no way to reveal a key after the fact: the secret is stored hashed, so the token appears once in a dialog at creation and the listing shows only the public key_id. Revoking asks for confirmation and leaves the row in place, and a revoked key offers no further action. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 2 +- .../platform/views/organisations.py | 43 ++- docs/source/technical/organization-api.rst | 8 +- .../platform/organization/Organization.jsx | 136 +++++++++- .../organization/components/ApiKeyForm.jsx | 124 +++++++++ .../components/NewApiKeyDialog.jsx | 56 ++++ .../components/RevokeApiKeyDialog.jsx | 39 +++ .../platform/OrganizationApiKeys.test.jsx | 252 ++++++++++++++++++ 8 files changed, 654 insertions(+), 6 deletions(-) create mode 100644 frontend/platform/organization/components/ApiKeyForm.jsx create mode 100644 frontend/platform/organization/components/NewApiKeyDialog.jsx create mode 100644 frontend/platform/organization/components/RevokeApiKeyDialog.jsx create mode 100644 frontend/src/test/platform/OrganizationApiKeys.test.jsx diff --git a/CHANGELOG.md b/CHANGELOG.md index 9cc579b4..51b57cc2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ Changes prior to v1.0.0 are available in the [git history](https://github.com/Av ### Added -- **Organization API keys and a new organization-scoped API** — Organization admins can now issue API keys for their own organization via `POST /api/platform/organizations//api-keys/`, and use them against a new `/api/v1/` surface. v1 covers one endpoint, `POST /api/v1/enrollments/`, which enrolls an email address in one of the organization's courses. Keys carry explicit scopes — `enrollments:create` is the only one for now, and an organization key must carry at least one — plus an optional expiry. The organization is taken from the key itself rather than from the URL or request body, so a key can only ever act on the organization it was issued for; a slug or id belonging to another organization reads as `404`. Only organization *admins* can issue keys, since a key acts with whatever scopes it carries. Requests are rate limited per key (defaults 120/60s, configurable via `DJANGO_EMAIL_LEARNING["ORGANIZATION_API_RATE_LIMITS"]`). This is separate from the existing unauthenticated `/api/public/` embed surface. Management is API-only for now — there is no organization-facing settings screen for these keys yet. See the new [Organization API](https://django-email-learning.readthedocs.io/en/latest/technical/organization-api.html) reference. +- **Organization API keys and a new organization-scoped API** — Organization admins can now issue API keys for their own organization via `POST /api/platform/organizations//api-keys/`, and use them against a new `/api/v1/` surface. v1 covers one endpoint, `POST /api/v1/enrollments/`, which enrolls an email address in one of the organization's courses. Keys carry explicit scopes — `enrollments:create` is the only one for now, and an organization key must carry at least one — plus an optional expiry. The organization is taken from the key itself rather than from the URL or request body, so a key can only ever act on the organization it was issued for; a slug or id belonging to another organization reads as `404`. Only organization *admins* can issue keys, since a key acts with whatever scopes it carries. Requests are rate limited per key (defaults 120/60s, configurable via `DJANGO_EMAIL_LEARNING["ORGANIZATION_API_RATE_LIMITS"]`). This is separate from the existing unauthenticated `/api/public/` embed surface. Keys are managed from a new **API Keys** tab on the organization page, which lists each key's id, scopes, status and last use, and shows the key itself only once, at creation. See the new [Organization API](https://django-email-learning.readthedocs.io/en/latest/technical/organization-api.html) reference. - **OpenAPI 3.1 schema for the organization API** — `GET /api/v1/openapi.json` serves a machine-readable description of the v1 API for Swagger UI, Redoc or a client generator. It takes no API key, since it describes the API's shape and carries no organization data; set `DJANGO_EMAIL_LEARNING["ORGANIZATION_API_DOCS_ENABLED"] = False` to stop serving it. The document is generated from the running code rather than maintained separately — paths from the URLconf, schemas from the Pydantic models the views validate with, and security requirements from the scopes the auth decorator enforces — and a test fails the build if a routed endpoint has no documentation. No new dependency. - **API keys now support naming, expiry, revocation and last-used tracking** — Both platform and organization keys take a `name` and an optional `expires_at`, record `last_used_at` on each authenticated request (at minute resolution, so recording activity doesn't cost a write per request), and can be revoked. diff --git a/django_email_learning/platform/views/organisations.py b/django_email_learning/platform/views/organisations.py index 6d3e2614..f3014859 100644 --- a/django_email_learning/platform/views/organisations.py +++ b/django_email_learning/platform/views/organisations.py @@ -5,7 +5,7 @@ from django.utils.translation import gettext as _ from django_email_learning.decorators import is_an_organization_member -from django_email_learning.models import Organization +from django_email_learning.models import ApiKeyScope, Organization from django_email_learning.platform.views.base import BasePlatformView @@ -88,6 +88,11 @@ def get_context_data(self, **kwargs): # type: ignore[no-untyped-def] context["organization"] = organization context["page_title"] = _("Organization: %(name)s") % {"name": organization.name} context["appContext"]["organizationId"] = organization.id + # Sourced from the enum the API validates against, so the choices the + # form offers can't drift from the scopes a key may actually carry. + context["appContext"]["apiKeyScopes"] = [ + {"value": value, "label": str(label)} for value, label in ApiKeyScope.choices + ] return context def get_locale_messages(self) -> Dict[str, str]: @@ -183,4 +188,40 @@ def get_locale_messages(self) -> Dict[str, str]: "view_public_organization_page": _("Public page"), "copy_public_organization_link": _("Copy public organization link"), "public_organization_link_copied": _("Link copied!"), + "api_keys": _("API Keys"), + "no_api_keys": _("No API keys yet."), + "create_api_key": _("Create API Key"), + "api_key_name": _("Name"), + "api_key_name_required": _("Name is required."), + "api_key_scopes": _("Scopes"), + "api_key_scopes_required": _("Select at least one scope."), + "api_key_expires_at": _("Expires on"), + "api_key_expires_at_helper_text": _("Optional. The key never expires if left blank."), + "api_key_create_error": _("Failed to create the API key. Please try again."), + "key_id": _("Key ID"), + "status": _("Status"), + "active": _("Active"), + "revoked": _("Revoked"), + "expired": _("Expired"), + "last_used": _("Last Used"), + "never_used": _("Never used"), + "created_by": _("Created By"), + "created_at": _("Created At"), + "revoke": _("Revoke"), + "confirm_revocation": _("Confirm Revocation"), + "are_you_sure_revoke_key": _( + "Are you sure you want to revoke API_KEY_NAME? Anything using this key will stop working immediately." + ), + "copy": _("Copy"), + "copied": _("Copied"), + "done": _("Done"), + "new_api_key_created": _("New API key created"), + "copy_key_now_warning": _( + "Copy this key now. For security it is stored hashed, so this is the only time it can be shown." + ), + "api_keys_intro": _( + "API keys let your own systems act on this organization's data — enrolling a learner from" + " your signup flow, for example. Each key carries only the scopes you grant it and acts" + " solely on this organization." + ), } diff --git a/docs/source/technical/organization-api.rst b/docs/source/technical/organization-api.rst index bb3a8b34..b5205d70 100644 --- a/docs/source/technical/organization-api.rst +++ b/docs/source/technical/organization-api.rst @@ -26,10 +26,12 @@ Organization **admins** can create keys for their own organization. Editors, instructors and viewers cannot: a key acts with whatever scopes it carries, so issuing one would let a non-admin hand out access it does not itself have. -.. note:: +In the platform UI, open the organization and choose the **API Keys** tab. The +key is shown once, in a dialog, immediately after it is created; the table +afterwards lists each key's id, scopes, status and last use, but never the key +itself. - Key management is API-only for now — there is no organization-facing - settings screen for these keys yet. +The same thing over HTTP: .. code-block:: http diff --git a/frontend/platform/organization/Organization.jsx b/frontend/platform/organization/Organization.jsx index 99aaa440..1856b65a 100644 --- a/frontend/platform/organization/Organization.jsx +++ b/frontend/platform/organization/Organization.jsx @@ -13,7 +13,7 @@ import TableCell from "@mui/material/TableCell"; import Typography from "@mui/material/Typography"; import LinearProgress from "@mui/material/LinearProgress"; import Dialog from "@mui/material/Dialog"; -import { Tabs, Tab, Link, Tooltip } from "@mui/material"; +import { Tabs, Tab, Link, Tooltip, Chip } from "@mui/material"; import DeleteIcon from '@mui/icons-material/Delete'; import EditIcon from '@mui/icons-material/Edit'; import PeopleIcon from '@mui/icons-material/People'; @@ -21,6 +21,7 @@ import EmailIcon from '@mui/icons-material/Email'; import InfoIcon from '@mui/icons-material/Info'; import PublicIcon from '@mui/icons-material/Public'; import ContentCopyIcon from '@mui/icons-material/ContentCopy'; +import VpnKeyOutlinedIcon from '@mui/icons-material/VpnKeyOutlined'; import { useState, useEffect } from "react"; import apiClient from "../../src/apiClient.js"; import { Button } from "@mui/material"; @@ -32,11 +33,21 @@ const DeleteUserDialog = lazy(() => import("./components/DeleteUserDialog.jsx")) const NewsletterForm = lazy(() => import("./components/NewsletterForm.jsx")); const DeleteNewsletterDialog = lazy(() => import("./components/DeleteNewsletterDialog.jsx")); const OrganizationForm = lazy(() => import("../organizations/components/OrganizationForm.jsx")); +const ApiKeyForm = lazy(() => import("./components/ApiKeyForm.jsx")); +const NewApiKeyDialog = lazy(() => import("./components/NewApiKeyDialog.jsx")); +const RevokeApiKeyDialog = lazy(() => import("./components/RevokeApiKeyDialog.jsx")); + +const apiKeyStatusOf = (apiKey) => { + if (apiKey.revoked_at) return 'revoked'; + if (apiKey.expires_at && new Date(apiKey.expires_at) <= new Date()) return 'expired'; + return 'active'; +}; function Organization() { const [organization, setOrganization] = useState(null); const [organizationUsers, setOrganizationUsers] = useState([]); const [newsletters, setNewsletters] = useState([]); + const [apiKeys, setApiKeys] = useState([]); const initialTab = new URLSearchParams(window.location.search).get('tab') || 'members'; const [activeTab, setActiveTab] = useState(initialTab); const [dialogOpen, setDialogOpen] = useState(false); @@ -66,6 +77,12 @@ function Organization() { .catch(error => console.error('Error fetching newsletters:', error)); }; + const refreshApiKeys = () => { + apiClient.get(`${apiBaseUrl}/organizations/${organizationId}/api-keys/`) + .then(data => setApiKeys(data.api_keys)) + .catch(error => console.error('Error fetching API keys:', error)); + }; + useEffect(() => { apiClient.get(`${apiBaseUrl}/organizations/${organizationId}/`) .then(data => setOrganization(data)) @@ -76,6 +93,10 @@ function Organization() { if (newslettersEnabled) { refreshNewsletters(); } + + if (canEditOrganization) { + refreshApiKeys(); + } }, []); const showDialog = (content) => { @@ -183,6 +204,14 @@ function Organization() { label={localeMessages["newsletters"]} /> )} + {canEditOrganization && ( + } + iconPosition="start" + label={localeMessages["api_keys"]} + /> + )} @@ -394,6 +423,111 @@ function Organization() { )} + + {/* API keys tab */} + {canEditOrganization && activeTab === 'api_keys' && ( + <> + {localeMessages["api_keys_intro"]} + + + {apiKeys.length > 0 ? ( + + + + + {localeMessages["api_key_name"]} + {localeMessages["key_id"]} + {localeMessages["api_key_scopes"]} + {localeMessages["status"]} + {localeMessages["created_by"]} + {localeMessages["created_at"]} + {localeMessages["last_used"]} + {localeMessages["actions"]} + + + + {apiKeys.map((apiKey) => { + const status = apiKeyStatusOf(apiKey); + return ( + + {apiKey.name} + + {/* The public half of the token. The secret is + hashed and is never returned by the listing. */} + + {apiKey.key_id} + + + + + {apiKey.scopes.map(scope => ( + + ))} + + + + + + {apiKey.created_by} + {apiKey.created_at} + {apiKey.last_used_at || localeMessages["never_used"]} + + {status !== 'revoked' && ( + showDialog( + }> + + + )} + > + + + )} + + + ); + })} + +
+ + ) : ( + {localeMessages["no_api_keys"]} + )} + + + )} diff --git a/frontend/platform/organization/components/ApiKeyForm.jsx b/frontend/platform/organization/components/ApiKeyForm.jsx new file mode 100644 index 00000000..0d6a6184 --- /dev/null +++ b/frontend/platform/organization/components/ApiKeyForm.jsx @@ -0,0 +1,124 @@ +import { useState } from 'react'; +import Box from '@mui/material/Box'; +import TextField from '@mui/material/TextField'; +import Typography from '@mui/material/Typography'; +import Button from '@mui/material/Button'; +import Checkbox from '@mui/material/Checkbox'; +import FormControl from '@mui/material/FormControl'; +import FormControlLabel from '@mui/material/FormControlLabel'; +import FormGroup from '@mui/material/FormGroup'; +import FormLabel from '@mui/material/FormLabel'; +import FormHelperText from '@mui/material/FormHelperText'; +import { useAppContext } from '../../../src/render.jsx'; +import apiClient from '../../../src/apiClient.js'; +import { sanitizeEndpointUrl } from '../../../src/sanitizeUrl.js'; + +const ApiKeyForm = ({ onClose, organizationId, onCreated }) => { + const { localeMessages, apiBaseUrl: rawApiBaseUrl, apiKeyScopes = [] } = useAppContext(); + const apiBaseUrl = sanitizeEndpointUrl(rawApiBaseUrl); + const [name, setName] = useState(''); + const [selectedScopes, setSelectedScopes] = useState([]); + const [expiresAt, setExpiresAt] = useState(''); + const [nameError, setNameError] = useState(''); + const [scopesError, setScopesError] = useState(''); + const [error, setError] = useState(''); + const [submitting, setSubmitting] = useState(false); + + const toggleScope = (value) => { + setSelectedScopes(current => + current.includes(value) ? current.filter(scope => scope !== value) : [...current, value] + ); + }; + + const validate = () => { + setNameError(''); + setScopesError(''); + let valid = true; + if (!name.trim()) { + setNameError(localeMessages['api_key_name_required']); + valid = false; + } + // The API rejects a scopeless key too; checking here saves a round trip + // and puts the message next to the field. + if (selectedScopes.length === 0) { + setScopesError(localeMessages['api_key_scopes_required']); + valid = false; + } + return valid; + }; + + const handleSubmit = () => { + if (!validate()) return; + setError(''); + setSubmitting(true); + + const payload = { name: name.trim(), scopes: selectedScopes }; + if (expiresAt) { + // The field is a date; the API takes a datetime. + payload.expires_at = `${expiresAt}T00:00:00Z`; + } + + apiClient.post(`${apiBaseUrl}/organizations/${organizationId}/api-keys/`, payload) + .then(data => onCreated(data)) + .catch(() => { + setError(localeMessages['api_key_create_error']); + setSubmitting(false); + }); + }; + + return ( + + {localeMessages['create_api_key']} + {error && {error}} + + setName(e.target.value)} + error={!!nameError} + helperText={nameError} + slotProps={{ htmlInput: { maxLength: 100 } }} + fullWidth + required + /> + + + {localeMessages['api_key_scopes']} + + {apiKeyScopes.map(scope => ( + toggleScope(scope.value)} + /> + } + label={`${scope.label} (${scope.value})`} + /> + ))} + + {scopesError && {scopesError}} + + + setExpiresAt(e.target.value)} + helperText={localeMessages['api_key_expires_at_helper_text']} + slotProps={{ inputLabel: { shrink: true } }} + fullWidth + /> + + + + + + + ); +}; + +export default ApiKeyForm; diff --git a/frontend/platform/organization/components/NewApiKeyDialog.jsx b/frontend/platform/organization/components/NewApiKeyDialog.jsx new file mode 100644 index 00000000..72893ab7 --- /dev/null +++ b/frontend/platform/organization/components/NewApiKeyDialog.jsx @@ -0,0 +1,56 @@ +import { useState } from 'react'; +import Box from '@mui/material/Box'; +import Alert from '@mui/material/Alert'; +import Typography from '@mui/material/Typography'; +import Button from '@mui/material/Button'; +import IconButton from '@mui/material/IconButton'; +import ContentCopyIcon from '@mui/icons-material/ContentCopy'; +import { useAppContext } from '../../../src/render.jsx'; + +/** + * Shown once, immediately after a key is created. The server stores only a + * hash, so this is the only opportunity to copy the token — which is why the + * key table deliberately offers no way to reveal it later. + */ +const NewApiKeyDialog = ({ token, onClose }) => { + const { localeMessages } = useAppContext(); + const [copied, setCopied] = useState(false); + + const copyToken = async () => { + try { + await navigator.clipboard.writeText(token); + setCopied(true); + } catch (error) { + console.error('Failed to copy API key:', error); + } + }; + + return ( + + + {localeMessages['new_api_key_created']} + + + {localeMessages['copy_key_now_warning']} + + + + {token} + + + + + + + {copied && {localeMessages['copied']}} + + + + ); +}; + +export default NewApiKeyDialog; diff --git a/frontend/platform/organization/components/RevokeApiKeyDialog.jsx b/frontend/platform/organization/components/RevokeApiKeyDialog.jsx new file mode 100644 index 00000000..5a5ac437 --- /dev/null +++ b/frontend/platform/organization/components/RevokeApiKeyDialog.jsx @@ -0,0 +1,39 @@ +import { Container, Typography, Button, Box } from '@mui/material'; +import { useAppContext } from '../../../src/render.jsx'; +import apiClient from '../../../src/apiClient.js'; +import { sanitizeEndpointUrl } from '../../../src/sanitizeUrl.js'; + +const RevokeApiKeyDialog = ({ apiKey, organizationId, onClose, onSuccess }) => { + const { localeMessages, apiBaseUrl: rawApiBaseUrl } = useAppContext(); + const apiBaseUrl = sanitizeEndpointUrl(rawApiBaseUrl); + + const handleRevoke = () => { + apiClient.del(`${apiBaseUrl}/organizations/${organizationId}/api-keys/${apiKey.id}/`) + .then(() => { + onSuccess(); + onClose(); + }) + .catch(error => console.error('Error revoking API key:', error)); + }; + + return ( + + + {localeMessages['confirm_revocation']} + + + {localeMessages['are_you_sure_revoke_key'].replace('API_KEY_NAME', apiKey.name)} + + + + + + + ); +}; + +export default RevokeApiKeyDialog; diff --git a/frontend/src/test/platform/OrganizationApiKeys.test.jsx b/frontend/src/test/platform/OrganizationApiKeys.test.jsx new file mode 100644 index 00000000..cb10cb43 --- /dev/null +++ b/frontend/src/test/platform/OrganizationApiKeys.test.jsx @@ -0,0 +1,252 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { renderWithProviders } from '../test-utils'; +import Organization from '../../../platform/organization/Organization'; + +vi.mock('../../render.jsx'); + +const localeMessages = { + organizations: 'Organizations', + general_info: 'General Info', + members: 'Members', + actions: 'Actions', + cancel: 'Cancel', + api_keys: 'API Keys', + api_keys_intro: 'API keys let your own systems act on this organization.', + no_api_keys: 'No API keys yet.', + create_api_key: 'Create API Key', + api_key_name: 'Name', + api_key_name_required: 'Name is required.', + api_key_scopes: 'Scopes', + api_key_scopes_required: 'Select at least one scope.', + api_key_expires_at: 'Expires on', + api_key_expires_at_helper_text: 'Optional.', + api_key_create_error: 'Failed to create the API key.', + key_id: 'Key ID', + status: 'Status', + active: 'Active', + revoked: 'Revoked', + expired: 'Expired', + last_used: 'Last Used', + never_used: 'Never used', + created_by: 'Created By', + created_at: 'Created At', + revoke: 'Revoke', + confirm_revocation: 'Confirm Revocation', + are_you_sure_revoke_key: 'Are you sure you want to revoke API_KEY_NAME?', + copy: 'Copy', + copied: 'Copied', + done: 'Done', + new_api_key_created: 'New API key created', + copy_key_now_warning: 'Copy this key now. It cannot be shown again.', +}; + +const organization = { + id: 1, + name: 'Acme Corp', + description: 'A great company.', + logo: null, + logo_path: null, + social_links: [], + is_public: false, + public_url: null, +}; + +const activeKey = { + id: 7, + key_id: 'a1b2c3d4e5f60718293a4b5c', + name: 'Partner integration', + key_type: 'organization', + organization_id: 1, + scopes: ['enrollments:create'], + created_by: 'orgadmin', + created_at: '2026-08-07', + expires_at: null, + revoked_at: null, + last_used_at: null, +}; + +const baseAppContext = { + organizationId: '1', + localeMessages, + isOrganizationAdmin: true, + apiKeyScopes: [{ value: 'enrollments:create', label: 'Create enrollments' }], +}; + +function setupFetch({ apiKeys = [], onPost } = {}) { + global.fetch.mockImplementation((url, options) => { + if (url.includes('/api-keys/') && options?.method === 'POST') { + return Promise.resolve({ ok: true, json: () => Promise.resolve(onPost(JSON.parse(options.body))) }); + } + if (url.includes('/api-keys/') && options?.method === 'DELETE') { + return Promise.resolve({ ok: true, json: () => Promise.resolve({ message: 'API Key revoked successfully' }) }); + } + if (url.includes('/api-keys/')) { + return Promise.resolve({ ok: true, json: () => Promise.resolve({ api_keys: apiKeys }) }); + } + if (url.includes('/users/')) { + return Promise.resolve({ ok: true, json: () => Promise.resolve({ organization_users: [] }) }); + } + if (url.endsWith('/organizations/1/')) { + return Promise.resolve({ ok: true, json: () => Promise.resolve(organization) }); + } + return Promise.resolve({ ok: true, json: () => Promise.resolve({}) }); + }); +} + +const openApiKeysTab = async (user) => { + await user.click(await screen.findByRole('tab', { name: /API Keys/i })); +}; + +describe('Organization API keys tab', () => { + beforeEach(() => { + setupFetch(); + }); + + it('shows the tab for an organization admin', async () => { + renderWithProviders(, { appContext: baseAppContext }); + expect(await screen.findByRole('tab', { name: /API Keys/i })).toBeInTheDocument(); + }); + + it('hides the tab from a non-admin', async () => { + renderWithProviders(, { + appContext: { ...baseAppContext, isOrganizationAdmin: false, isPlatformAdmin: false }, + }); + await screen.findByRole('tab', { name: /Members/i }); + expect(screen.queryByRole('tab', { name: /API Keys/i })).not.toBeInTheDocument(); + }); + + it('shows the empty state when there are no keys', async () => { + const user = userEvent.setup(); + renderWithProviders(, { appContext: baseAppContext }); + await openApiKeysTab(user); + expect(await screen.findByText('No API keys yet.')).toBeInTheDocument(); + }); + + it('lists a key with its scopes and metadata', async () => { + setupFetch({ apiKeys: [activeKey] }); + const user = userEvent.setup(); + renderWithProviders(, { appContext: baseAppContext }); + await openApiKeysTab(user); + + expect(await screen.findByText('Partner integration')).toBeInTheDocument(); + expect(screen.getByText('a1b2c3d4e5f60718293a4b5c')).toBeInTheDocument(); + expect(screen.getByText('enrollments:create')).toBeInTheDocument(); + expect(screen.getByText('Active')).toBeInTheDocument(); + expect(screen.getByText('orgadmin')).toBeInTheDocument(); + expect(screen.getByText('Never used')).toBeInTheDocument(); + }); + + it('never renders a token in the listing', async () => { + setupFetch({ apiKeys: [activeKey] }); + const user = userEvent.setup(); + renderWithProviders(, { appContext: baseAppContext }); + await openApiKeysTab(user); + await screen.findByText('Partner integration'); + + expect(screen.queryByTestId('new-api-key-token')).not.toBeInTheDocument(); + }); + + it('marks a revoked key and offers no revoke action', async () => { + setupFetch({ apiKeys: [{ ...activeKey, revoked_at: '2026-08-08' }] }); + const user = userEvent.setup(); + renderWithProviders(, { appContext: baseAppContext }); + await openApiKeysTab(user); + + expect(await screen.findByText('Revoked')).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /Revoke Partner integration/ })).not.toBeInTheDocument(); + }); + + it('marks a key past its expiry as expired', async () => { + setupFetch({ apiKeys: [{ ...activeKey, expires_at: '2020-01-01T00:00:00Z' }] }); + const user = userEvent.setup(); + renderWithProviders(, { appContext: baseAppContext }); + await openApiKeysTab(user); + + expect(await screen.findByText('Expired')).toBeInTheDocument(); + }); + + it('creates a key and shows the token once', async () => { + const posted = []; + setupFetch({ + onPost: (body) => { + posted.push(body); + return { ...activeKey, token: 'elk_a1b2c3d4e5f60718293a4b5c_supersecret' }; + }, + }); + const user = userEvent.setup(); + renderWithProviders(, { appContext: baseAppContext }); + await openApiKeysTab(user); + + await user.click(await screen.findByRole('button', { name: 'Create API Key' })); + await user.type(await screen.findByLabelText(/Name/), 'Partner integration'); + await user.click(screen.getByRole('checkbox', { name: /Create enrollments/ })); + await user.click(screen.getByRole('button', { name: 'Create API Key' })); + + expect(await screen.findByText('New API key created')).toBeInTheDocument(); + expect(screen.getByTestId('new-api-key-token')).toHaveTextContent('elk_a1b2c3d4e5f60718293a4b5c_supersecret'); + expect(screen.getByText('Copy this key now. It cannot be shown again.')).toBeInTheDocument(); + + expect(posted).toEqual([{ name: 'Partner integration', scopes: ['enrollments:create'] }]); + }); + + it('refuses to submit without a scope', async () => { + const posted = []; + setupFetch({ onPost: (body) => { posted.push(body); return activeKey; } }); + const user = userEvent.setup(); + renderWithProviders(, { appContext: baseAppContext }); + await openApiKeysTab(user); + + await user.click(await screen.findByRole('button', { name: 'Create API Key' })); + await user.type(await screen.findByLabelText(/Name/), 'No scopes'); + await user.click(screen.getByRole('button', { name: 'Create API Key' })); + + expect(await screen.findByText('Select at least one scope.')).toBeInTheDocument(); + expect(posted).toEqual([]); + }); + + it('requires a name', async () => { + const posted = []; + setupFetch({ onPost: (body) => { posted.push(body); return activeKey; } }); + const user = userEvent.setup(); + renderWithProviders(, { appContext: baseAppContext }); + await openApiKeysTab(user); + + await user.click(await screen.findByRole('button', { name: 'Create API Key' })); + await user.click(await screen.findByRole('checkbox', { name: /Create enrollments/ })); + await user.click(screen.getByRole('button', { name: 'Create API Key' })); + + expect(await screen.findByText('Name is required.')).toBeInTheDocument(); + expect(posted).toEqual([]); + }); + + it('sends an expiry as a datetime when one is chosen', async () => { + const posted = []; + setupFetch({ onPost: (body) => { posted.push(body); return { ...activeKey, token: 'elk_x_y' }; } }); + const user = userEvent.setup(); + renderWithProviders(, { appContext: baseAppContext }); + await openApiKeysTab(user); + + await user.click(await screen.findByRole('button', { name: 'Create API Key' })); + await user.type(await screen.findByLabelText(/Name/), 'Expiring key'); + await user.click(screen.getByRole('checkbox', { name: /Create enrollments/ })); + await user.type(screen.getByLabelText(/Expires on/), '2027-01-01'); + await user.click(screen.getByRole('button', { name: 'Create API Key' })); + + await waitFor(() => expect(posted).toHaveLength(1)); + expect(posted[0].expires_at).toBe('2027-01-01T00:00:00Z'); + }); + + it('confirms before revoking', async () => { + setupFetch({ apiKeys: [activeKey] }); + const user = userEvent.setup(); + renderWithProviders(, { appContext: baseAppContext }); + await openApiKeysTab(user); + + await user.click(await screen.findByRole('button', { name: /Revoke Partner integration/ })); + + expect(await screen.findByText('Confirm Revocation')).toBeInTheDocument(); + expect(screen.getByText('Are you sure you want to revoke Partner integration?')).toBeInTheDocument(); + }); +}); From 0f697cd10112f52df0a2ea61387f05fb21bbf3b4 Mon Sep 17 00:00:00 2001 From: Payam Date: Fri, 7 Aug 2026 21:04:23 +0400 Subject: [PATCH 5/5] Gate the API keys tab on a feature and add permission hooks Adds PlatformFeature.ORGANIZATION_API, included in the default feature set so it reaches the frontend through the existing availableFeatures context. The organization page's API Keys tab, and the request that loads the keys, are both gated on it. The flag governs the UI only. For the operations themselves, OrganizationApiKeyView and SingleOrganizationApiKeyView each gain an overridable hook - can_create_organization_api_key and can_delete_organization_api_key - defaulting to True and rejecting with 403 before any database work, matching the existing can_create_course pattern. Both take the request and the resolved Organization rather than an id, so a plan-limit check can read the organization's state without a second query. The delete hook runs before the key is looked up, so a caller who may not revoke can't tell a real key id from a made-up one by comparing 403 against 404. Also switches the empty API keys table to the shared EmptyTableState component used by the courses, learners and platform API key tables. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 1 + .../platform/api/views/__init__.py | 2 + .../platform/api/views/misc.py | 43 ++++- django_email_learning/platform/features.py | 1 + django_email_learning/platform/views/base.py | 6 +- docs/source/technical/organization-api.rst | 40 +++++ .../platform/organization/Organization.jsx | 159 +++++++++--------- .../platform/OrganizationApiKeys.test.jsx | 27 ++- .../test_organization_api_key_view.py | 86 ++++++++++ .../test_single_organization_view.py | 22 +++ 10 files changed, 305 insertions(+), 82 deletions(-) create mode 100644 tests/platform/test_views/test_single_organization_view.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 51b57cc2..7f9824e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ Changes prior to v1.0.0 are available in the [git history](https://github.com/Av ### Added - **Organization API keys and a new organization-scoped API** — Organization admins can now issue API keys for their own organization via `POST /api/platform/organizations//api-keys/`, and use them against a new `/api/v1/` surface. v1 covers one endpoint, `POST /api/v1/enrollments/`, which enrolls an email address in one of the organization's courses. Keys carry explicit scopes — `enrollments:create` is the only one for now, and an organization key must carry at least one — plus an optional expiry. The organization is taken from the key itself rather than from the URL or request body, so a key can only ever act on the organization it was issued for; a slug or id belonging to another organization reads as `404`. Only organization *admins* can issue keys, since a key acts with whatever scopes it carries. Requests are rate limited per key (defaults 120/60s, configurable via `DJANGO_EMAIL_LEARNING["ORGANIZATION_API_RATE_LIMITS"]`). This is separate from the existing unauthenticated `/api/public/` embed surface. Keys are managed from a new **API Keys** tab on the organization page, which lists each key's id, scopes, status and last use, and shows the key itself only once, at creation. See the new [Organization API](https://django-email-learning.readthedocs.io/en/latest/technical/organization-api.html) reference. +- **`ORGANIZATION_API` platform feature and permission hooks for key management** — A new `PlatformFeature.ORGANIZATION_API`, present by default, controls whether the organization page shows its API Keys tab; remove it from `get_available_features()` to hide it. The API side has its own control: override `can_create_organization_api_key(request, organization)` on `OrganizationApiKeyView` or `can_delete_organization_api_key(request, organization)` on `SingleOrganizationApiKeyView`. Both default to `True` and reject with `403` before any database work when they return `False`, following the same pattern as the existing `can_create_course` hook. They receive the resolved `Organization` rather than its id, so a check can read its state without a second query. - **OpenAPI 3.1 schema for the organization API** — `GET /api/v1/openapi.json` serves a machine-readable description of the v1 API for Swagger UI, Redoc or a client generator. It takes no API key, since it describes the API's shape and carries no organization data; set `DJANGO_EMAIL_LEARNING["ORGANIZATION_API_DOCS_ENABLED"] = False` to stop serving it. The document is generated from the running code rather than maintained separately — paths from the URLconf, schemas from the Pydantic models the views validate with, and security requirements from the scopes the auth decorator enforces — and a test fails the build if a routed endpoint has no documentation. No new dependency. - **API keys now support naming, expiry, revocation and last-used tracking** — Both platform and organization keys take a `name` and an optional `expires_at`, record `last_used_at` on each authenticated request (at minute resolution, so recording activity doesn't cost a write per request), and can be revoked. diff --git a/django_email_learning/platform/api/views/__init__.py b/django_email_learning/platform/api/views/__init__.py index f604d695..33423d5b 100644 --- a/django_email_learning/platform/api/views/__init__.py +++ b/django_email_learning/platform/api/views/__init__.py @@ -25,6 +25,7 @@ FileView, JobHealthStatus, JobsStatus, + OrganizationApiKeyPermissionMixin, OrganizationApiKeyView, RootView, SingleApiKeyView, @@ -95,6 +96,7 @@ "ApiKeyView", "SingleApiKeyView", "OrganizationApiKeyView", + "OrganizationApiKeyPermissionMixin", "SingleOrganizationApiKeyView", "JobsStatus", "RootView", diff --git a/django_email_learning/platform/api/views/misc.py b/django_email_learning/platform/api/views/misc.py index f081d829..32ee3e24 100644 --- a/django_email_learning/platform/api/views/misc.py +++ b/django_email_learning/platform/api/views/misc.py @@ -9,7 +9,7 @@ from django.core.exceptions import ValidationError as DjangoValidationError from django.core.files.storage import default_storage from django.db.utils import IntegrityError -from django.http import JsonResponse +from django.http import HttpRequest, JsonResponse from django.utils import timezone from django.utils.decorators import method_decorator from django.views import View @@ -26,6 +26,7 @@ ApiKeyType, JobExecution, JobName, + Organization, OrganizationUser, ) from django_email_learning.platform.api import serializers @@ -114,9 +115,31 @@ def delete(self, request, *args, **kwargs): # type: ignore[no-untyped-def] return JsonResponse({"message": "API Key revoked successfully"}, status=200) +class OrganizationApiKeyPermissionMixin: + """Provides the hooks gating organization API key management. + + Override either in a subclass to add custom logic (plan limits, feature + flags, a stricter role rule). Returning False rejects the request with a + 403 before any database work happens. Both receive the resolved + `Organization` rather than its id, so a check can read the organization's + own state without a second query. + """ + + def can_create_organization_api_key(self, request: HttpRequest, organization: Organization) -> bool: + return True + + def can_delete_organization_api_key(self, request: HttpRequest, organization: Organization) -> bool: + return True + + def get_target_organization(self, organization_id: int) -> Organization | None: + # Membership has already been checked by the decorator, but a superuser + # bypasses that and could name an organization that doesn't exist. + return Organization.objects.filter(id=organization_id).first() + + @method_decorator(is_an_organization_member(only_admin=True), name="post") @method_decorator(is_an_organization_member(only_admin=True), name="get") -class OrganizationApiKeyView(View): +class OrganizationApiKeyView(OrganizationApiKeyPermissionMixin, View): """Keys an organization's own admins issue for the public API. Admin-only within the organization: a key is a bearer credential that acts @@ -125,6 +148,12 @@ class OrganizationApiKeyView(View): """ def post(self, request, organization_id: int, *args, **kwargs) -> JsonResponse: # type: ignore[no-untyped-def] + organization = self.get_target_organization(organization_id) + if organization is None: + return JsonResponse({"error": "Organization not found"}, status=404) + if not self.can_create_organization_api_key(request, organization): + return JsonResponse({"error": "API key creation not allowed."}, status=403) + try: payload = serializers.CreateOrganizationApiKeyRequest.model_validate(_parse_json_body(request)) except json.JSONDecodeError: @@ -173,8 +202,16 @@ def get(self, request, organization_id: int, *args, **kwargs) -> JsonResponse: @method_decorator(is_an_organization_member(only_admin=True), name="delete") -class SingleOrganizationApiKeyView(View): +class SingleOrganizationApiKeyView(OrganizationApiKeyPermissionMixin, View): def delete(self, request, organization_id: int, api_key_id: int, *args, **kwargs) -> JsonResponse: # type: ignore[no-untyped-def] + organization = self.get_target_organization(organization_id) + if organization is None: + return JsonResponse({"error": "Organization not found"}, status=404) + # Checked before the key is looked up, so a caller who may not revoke + # can't use the 404/200 difference to probe which key ids exist. + if not self.can_delete_organization_api_key(request, organization): + return JsonResponse({"error": "API key deletion not allowed."}, status=403) + # Filtering on organization_id as well as the key id keeps one # organization's admin from revoking another's key by guessing an id. try: diff --git a/django_email_learning/platform/features.py b/django_email_learning/platform/features.py index 1a5ace09..382b16e0 100644 --- a/django_email_learning/platform/features.py +++ b/django_email_learning/platform/features.py @@ -8,3 +8,4 @@ class PlatformFeature(enum.StrEnum): GOOGLE_WORKSPACE_ENROLL = "google_workspace_enroll" NEWSLETTERS = "newsletters" CREATE_NEWSLETTER = "create_newsletter" + ORGANIZATION_API = "organization_api" diff --git a/django_email_learning/platform/views/base.py b/django_email_learning/platform/views/base.py index f77e08e9..4cd9e1a1 100644 --- a/django_email_learning/platform/views/base.py +++ b/django_email_learning/platform/views/base.py @@ -167,7 +167,11 @@ def get_shared_context(self) -> Dict[str, Any]: } def get_available_features(self) -> set[PlatformFeature]: - features: set[PlatformFeature] = {PlatformFeature.CREATE_COURSE, PlatformFeature.CAN_ADD_MEMBER} + features: set[PlatformFeature] = { + PlatformFeature.CREATE_COURSE, + PlatformFeature.CAN_ADD_MEMBER, + PlatformFeature.ORGANIZATION_API, + } if AI_CONFIGURATIONS.get("TEXT_EDITING_MODEL"): features.add(PlatformFeature.AI_EDIT) if DJANGO_EMAIL_LEARNING_SETTINGS.get("GOOGLE_OAUTH", {}).get("CLIENT_ID") and apps.is_installed( diff --git a/docs/source/technical/organization-api.rst b/docs/source/technical/organization-api.rst index b5205d70..6e52f4c0 100644 --- a/docs/source/technical/organization-api.rst +++ b/docs/source/technical/organization-api.rst @@ -31,6 +31,46 @@ key is shown once, in a dialog, immediately after it is created; the table afterwards lists each key's id, scopes, status and last use, but never the key itself. +The tab is shown when the ``organization_api`` platform feature is available, +which it is by default. To hide it, drop +``PlatformFeature.ORGANIZATION_API`` from ``get_available_features()`` on your +platform view: + +.. code-block:: python + + class MyOrganizationView(SingleOrganization): + def get_available_features(self): + return super().get_available_features() - {PlatformFeature.ORGANIZATION_API} + +.. note:: + + The flag decides what the UI offers; the permission hooks below decide what + the API allows. Removing the flag hides the tab, and overriding the hooks + refuses the operations — set both if you want the feature fully off. + +Restricting who may issue keys +------------------------------ + +``OrganizationApiKeyView`` and ``SingleOrganizationApiKeyView`` each expose a +hook, both defaulting to ``True``. Return ``False`` to reject the request with +a ``403`` before any database work happens — useful for plan limits, a key +quota, or a stricter rule than "organization admin". + +.. code-block:: python + + from django_email_learning.platform.api.views import OrganizationApiKeyView + + + class LimitedApiKeyView(OrganizationApiKeyView): + def can_create_organization_api_key(self, request, organization): + return organization.api_keys.filter(revoked_at__isnull=True).count() < 5 + +The matching hook for revocation is +``can_delete_organization_api_key(request, organization)`` on +``SingleOrganizationApiKeyView``. Both receive the resolved ``Organization`` +rather than its id, so a check can read the organization's own state without a +second query. Route your subclass in place of the shipped view to apply it. + The same thing over HTTP: .. code-block:: http diff --git a/frontend/platform/organization/Organization.jsx b/frontend/platform/organization/Organization.jsx index 1856b65a..4fd8a304 100644 --- a/frontend/platform/organization/Organization.jsx +++ b/frontend/platform/organization/Organization.jsx @@ -1,5 +1,6 @@ import { lazy, Suspense } from "react"; import Base from "../../src/components/Base"; +import EmptyTableState from "../../src/components/EmptyTableState.jsx"; import Avatar from "@mui/material/Avatar"; import Box from "@mui/material/Box"; import Grid from "@mui/material/Grid"; @@ -63,6 +64,8 @@ function Organization() { const newslettersEnabled = availableFeatures.includes('newsletters'); const createNewsletterEnabled = availableFeatures.includes('create_newsletter'); + const organizationApiEnabled = availableFeatures.includes('organization_api'); + const canManageApiKeys = canEditOrganization && organizationApiEnabled; const [canAddMember, setCanAddMember] = useState(availableFeatures.includes('can_add_member')); const refreshUsers = () => { @@ -94,7 +97,7 @@ function Organization() { refreshNewsletters(); } - if (canEditOrganization) { + if (canManageApiKeys) { refreshApiKeys(); } }, []); @@ -204,7 +207,7 @@ function Organization() { label={localeMessages["newsletters"]} /> )} - {canEditOrganization && ( + {canManageApiKeys && ( } @@ -425,7 +428,7 @@ function Organization() { )} {/* API keys tab */} - {canEditOrganization && activeTab === 'api_keys' && ( + {canManageApiKeys && activeTab === 'api_keys' && ( <> {localeMessages["api_keys_intro"]}