Add query endpoint for trusted users#97
Conversation
c19a8df to
2105678
Compare
758239d to
719ed0c
Compare
b5b3818 to
4bb05c5
Compare
for more information, see https://pre-commit.ci
warunawickramasingha
left a comment
There was a problem hiding this comment.
Proposed a few suggestions to improve the code, style and better handle exceptions + rate limiting rule at nginx level. Please check and apply as necessary.
| def verify_token(request) -> bool: | ||
| token = get_bearer_token(request) | ||
| secret = environ.get("QUERY_SECRET_KEY", "") | ||
| return token == secret |
There was a problem hiding this comment.
Not a huge concern in this context. But for secret validations, it is recommended to use something like compare_digest instead of == to avoid timing attacks.
| return token == secret | |
| from hmac import compare_digest | |
| return compare_digest(token, secret) |
| # Create your views here. | ||
| from django.views.decorators.cache import cache_page | ||
| from services.models import Message, Usage, FeatureUsage, Location | ||
| from rest_framework import response, viewsets |
There was a problem hiding this comment.
| from rest_framework import response, viewsets, status |
| @api_view(("POST",)) | ||
| def query(request, format=None): | ||
| if not verify_token(request): | ||
| return response.Response(status=401, data="UNAUTHORIZED") |
There was a problem hiding this comment.
For improved readability
| return response.Response(status=401, data="UNAUTHORIZED") | |
| return response.Response(status=status.HTTP_401_UNAUTHORIZED, data="UNAUTHORIZED") |
| if sql_err: | ||
| return response.Response(status=400, data=f"Invalid Parameters: {sql_err}") | ||
| conn = connections["readonly"] | ||
| with conn.cursor() as cur: | ||
| cur.execute(sql) | ||
| res = cur.fetchall() | ||
| return response.Response(res) |
There was a problem hiding this comment.
Exception handling for db operations
| if sql_err: | |
| return response.Response(status=400, data=f"Invalid Parameters: {sql_err}") | |
| conn = connections["readonly"] | |
| with conn.cursor() as cur: | |
| cur.execute(sql) | |
| res = cur.fetchall() | |
| return response.Response(res) | |
| if sql_err: | |
| return response.Response(status=status.HTTP_400_BAD_REQUEST, data=f"Invalid Parameters: {sql_err}") | |
| try: | |
| conn = connections["readonly"] | |
| with conn.cursor() as cur: | |
| cur.execute(sql) | |
| res = cur.fetchall() | |
| except Exception: | |
| return Response({"error": "Query failed"}, status=status.HTTP_400_BAD_REQUEST) | |
| return response.Response(res) |
| err = "" | ||
| if not val: | ||
| err = f"No {param} parameter provided" | ||
| return err, val |
There was a problem hiding this comment.
To work better with --data-urlencode "sql="
| return err, val | |
| def get_parameter(request, param): | |
| val = request.POST.get(param) | |
| if val is None or val.strip() == "": | |
| return f"No {param} parameter provided", None | |
| return None, val |
|
|
||
| location /api/query { | ||
| # allow ISIS VPN traffic | ||
| allow 130.246.0.0/16; |
There was a problem hiding this comment.
DI network team has confirmed the IP range of the STFC Corporate VPN to be 130.246.160.0/21;
| allow 130.246.0.0/16; | |
| allow 130.246.160.0/21; |
This PR adds an endpoint that allows trusted users to query the report database.
To test (contact me for the token, it's on keeper):
Error on write:
Read action:
Closes #95