-
Notifications
You must be signed in to change notification settings - Fork 46
Add optional API key authentication for /checkin/ and /verify/ endpoints #152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,115 @@ | ||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||
| API Key Authentication Middleware for Crypt Server. | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| This middleware protects the /checkin/ and /verify/ API endpoints | ||||||||||||||||||||||||||||||||||||
| with API key authentication while leaving the web UI accessible | ||||||||||||||||||||||||||||||||||||
| for SSO/session-based authentication. | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Configuration: | ||||||||||||||||||||||||||||||||||||
| Set the CRYPT_API_KEY environment variable to enable API key authentication. | ||||||||||||||||||||||||||||||||||||
| If not set, the endpoints remain open (backward compatible). | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Usage: | ||||||||||||||||||||||||||||||||||||
| Clients must include the API key in the X-API-Key header: | ||||||||||||||||||||||||||||||||||||
| curl -X POST https://crypt.example.com/checkin/ \ | ||||||||||||||||||||||||||||||||||||
| -H "X-API-Key: your-secret-key" \ | ||||||||||||||||||||||||||||||||||||
| -d "serial=ABC123&recovery_password=..." | ||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||
| import os | ||||||||||||||||||||||||||||||||||||
| import hmac | ||||||||||||||||||||||||||||||||||||
| import logging | ||||||||||||||||||||||||||||||||||||
| from django.http import JsonResponse | ||||||||||||||||||||||||||||||||||||
| from django.conf import settings | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| logger = logging.getLogger(__name__) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| class APIKeyAuthMiddleware: | ||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||
| Middleware to authenticate API requests using an API key. | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Only applies to: | ||||||||||||||||||||||||||||||||||||
| - /checkin/ (POST) | ||||||||||||||||||||||||||||||||||||
| - /verify/<serial>/<secret_type>/ (GET) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| The API key is read from: | ||||||||||||||||||||||||||||||||||||
| 1. CRYPT_API_KEY environment variable | ||||||||||||||||||||||||||||||||||||
| 2. settings.CRYPT_API_KEY (if defined in settings.py) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| If no API key is configured, requests are allowed through (backward compatible). | ||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| # Paths that require API key authentication | ||||||||||||||||||||||||||||||||||||
| PROTECTED_PATHS = ['/checkin/', '/verify/'] | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| # Header name for API key | ||||||||||||||||||||||||||||||||||||
| API_KEY_HEADER = 'HTTP_X_API_KEY' # Django converts X-API-Key to HTTP_X_API_KEY | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| def __init__(self, get_response): | ||||||||||||||||||||||||||||||||||||
| self.get_response = get_response | ||||||||||||||||||||||||||||||||||||
| self.api_key = self._get_api_key() | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| if self.api_key: | ||||||||||||||||||||||||||||||||||||
| logger.info("API key authentication enabled for /checkin/ and /verify/ endpoints") | ||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||
| logger.warning( | ||||||||||||||||||||||||||||||||||||
| "No CRYPT_API_KEY configured. API endpoints are UNPROTECTED. " | ||||||||||||||||||||||||||||||||||||
| "Set CRYPT_API_KEY environment variable to enable authentication." | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| def _get_api_key(self): | ||||||||||||||||||||||||||||||||||||
| """Get API key from environment or settings.""" | ||||||||||||||||||||||||||||||||||||
| # Try environment variable first | ||||||||||||||||||||||||||||||||||||
| api_key = os.environ.get('CRYPT_API_KEY') | ||||||||||||||||||||||||||||||||||||
| if api_key: | ||||||||||||||||||||||||||||||||||||
| return api_key.strip() | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| # Fall back to settings | ||||||||||||||||||||||||||||||||||||
| if hasattr(settings, 'CRYPT_API_KEY'): | ||||||||||||||||||||||||||||||||||||
| return getattr(settings, 'CRYPT_API_KEY', '').strip() | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
|
Comment on lines
+66
to
+70
|
||||||||||||||||||||||||||||||||||||
| # Fall back to settings | |
| if hasattr(settings, 'CRYPT_API_KEY'): | |
| return getattr(settings, 'CRYPT_API_KEY', '').strip() | |
| # Fall back to settings | |
| if hasattr(settings, 'CRYPT_API_KEY'): | |
| raw_key = getattr(settings, 'CRYPT_API_KEY', None) | |
| if not raw_key: | |
| return None | |
| if isinstance(raw_key, str): | |
| api_key = raw_key.strip() | |
| else: | |
| # Coerce non-string values to string safely | |
| api_key = str(raw_key).strip() | |
| return api_key or None |
Copilot
AI
Mar 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The incoming header value isn’t normalized before comparison. A client/proxy can include leading/trailing whitespace in X-API-Key, which will cause a mismatch even when the key is otherwise correct. Consider stripping the request-provided key (and possibly rejecting empty-after-strip) before calling compare_digest().
Copilot
AI
Mar 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new auth behavior is security-sensitive and currently has no automated tests. Since the repo already has Django TestCase coverage in server/tests.py, it would be good to add tests for: (1) when CRYPT_API_KEY is unset, requests to /checkin/ and /verify/ succeed as before; (2) when set, missing key returns 401; (3) wrong key returns 403; (4) correct key passes through.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This middleware is always installed via
MIDDLEWARE, and whenCRYPT_API_KEYis not set it logs a WARNING that endpoints are "UNPROTECTED". In environments that intentionally run without an API key (backward compatibility), this can create noisy/alert-worthy logs on every startup. Consider lowering this to INFO/DEBUG or only warning when an explicit setting indicates API key auth is required.