-
Notifications
You must be signed in to change notification settings - Fork 19
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
Added header authentication #8
Conversation
@@ -57,6 +57,14 @@ def wrapper(request, *args, **kwargs): | |||
request.user = user | |||
return view_func(request, *args, **kwargs) | |||
|
|||
if "HTTP_X_API_KEY" in request.META: |
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 block is not at the right level. The two if <header> in request.META
statements should be at the same level.
keys = set(os.getenv("ALLOWED_API_KEYS").split(",")) | ||
if key in keys: | ||
return view_func(request, *args, **kwargs) | ||
else: |
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.
You can drop the else HttpResponse("Forbidden", status=403)
and let it fall through to the return HttpResponse("Unauthorized", status=401)
. The 401 is actually correct. I incorrectly said 403 in the ticket.
For interest's sake:
- 403 is used when someone is authorised but do not have access
- 401 is used when someone is not authorised (which is the case here)
@@ -57,6 +58,12 @@ def wrapper(request, *args, **kwargs): | |||
request.user = user | |||
return view_func(request, *args, **kwargs) | |||
|
|||
if "HTTP_X_API_KEY" in request.META: | |||
key = request.META["HTTP_X_API_KEY"] | |||
keys = set(os.getenv("ALLOWED_API_KEYS").split(",")) |
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.
Can you make this a global, please: ALLOWED_API_KEYS = set(os.getenv("ALLOWED_API_KEYS").split(","))
And then just change the line below to if key in ALLOWED_API_KEYS:
The reason for this is that expensive task of retrieving the keys from the environment, splitting it and building a set is only done once (when the module is loaded) and not for every API call.
👍 |
No description provided.