-
Notifications
You must be signed in to change notification settings - Fork 23
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
Implement API create World #248
Changes from 8 commits
4a28c09
a49e355
957b01f
dc8212b
d9b7427
a5ea67a
df49cc3
dda1490
e517931
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 |
---|---|---|
|
@@ -3,12 +3,16 @@ | |
from contextlib import suppress | ||
from urllib.parse import urlparse | ||
|
||
import jwt | ||
import requests | ||
from asgiref.sync import async_to_sync | ||
from django.conf import settings | ||
from django.core import exceptions | ||
from django.db import transaction | ||
from django.forms.models import model_to_dict | ||
from django.http import JsonResponse | ||
from django.shortcuts import get_object_or_404 | ||
from django.utils.crypto import get_random_string | ||
from django.utils.timezone import now | ||
from rest_framework import viewsets | ||
from rest_framework.authentication import get_authorization_header | ||
|
@@ -115,6 +119,90 @@ def get(self, request, **kwargs): | |
) | ||
|
||
|
||
class CreateWorldView(APIView): | ||
authentication_classes = [] # disables authentication | ||
permission_classes = [] | ||
|
||
@staticmethod | ||
def post(request, *args, **kwargs) -> JsonResponse: | ||
payload = CreateWorldView.get_payload_from_token(request) | ||
|
||
# check if user has permission to create world | ||
if payload.get("has_permission"): | ||
secret = get_random_string(length=64) | ||
config = { | ||
"JWT_secrets": [ | ||
{ | ||
"issuer": "any", | ||
"audience": "venueless", | ||
"secret": secret, | ||
} | ||
] | ||
} | ||
|
||
title_default = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This impementation leave to many holes:
Also, if you don't use |
||
value for key, value in request.data.get("title").items() if value | ||
}.pop() | ||
|
||
# if world already exists, update it, otherwise create a new world | ||
try: | ||
if World.objects.filter(id=request.data.get("id")).exists(): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Data received from outside work (form, HTTP request) have to be validated. You are calling World.objects.get(id=request.data.get("id") but what if some attackers pass "id=abc"? This code will throw exception. |
||
world = World.objects.get(id=request.data.get("id")) | ||
world.title = ( | ||
request.data.get("title")[request.data.get("locale")] | ||
or request.data.get("title")["en"] | ||
or title_default | ||
) | ||
world.domain = "{}{}/{}".format( | ||
settings.DOMAIN_PATH, settings.BASE_PATH, request.data.get("id") | ||
) | ||
world.locale = request.data.get("locale") | ||
world.timezone = request.data.get("timezone") | ||
world.save() | ||
else: | ||
world = World.objects.create( | ||
id=request.data.get("id"), | ||
title=request.data.get("title")[request.data.get("locale")] | ||
or request.data.get("title")["en"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if |
||
or title_default, | ||
domain="{}{}/{}".format( | ||
settings.DOMAIN_PATH, | ||
settings.BASE_PATH, | ||
request.data.get("id"), | ||
), | ||
locale=request.data.get("locale"), | ||
timezone=request.data.get("timezone"), | ||
config=config, | ||
) | ||
except Exception as e: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (bug_risk): Implement more granular exception handling Catching a broad Exception can mask specific errors. Consider catching and handling specific exceptions (e.g., IntegrityError, ValidationError) to provide more informative error messages and easier debugging.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a bad practice to catch a very general exception class. This has been reminded before. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for your feedback, we will generate fewer comments like this in the future according to the following instructions:
|
||
logger.error("An error occurred while creating a world: {}".format(e)) | ||
return JsonResponse( | ||
{"error": "Unable to create or update world"}, status=400 | ||
) | ||
|
||
return JsonResponse(model_to_dict(world, exclude=["roles"]), status=201) | ||
else: | ||
return JsonResponse( | ||
{"error": "World cannot be created due to missing permission"}, | ||
status=403, | ||
) | ||
|
||
@staticmethod | ||
def get_payload_from_token(request): | ||
auth_header = get_authorization_header(request).split() | ||
if auth_header and auth_header[0].lower() == b"bearer": | ||
if len(auth_header) == 1: | ||
raise exceptions.AuthenticationFailed( | ||
"Invalid token header. No credentials provided." | ||
) | ||
elif len(auth_header) > 2: | ||
raise exceptions.AuthenticationFailed( | ||
"Invalid token header. Token string should not contain spaces." | ||
) | ||
payload = jwt.decode(auth_header[1], settings.SECRET_KEY, algorithms=["HS256"]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Add exception handling for JWT decoding JWT decoding can raise exceptions if the token is invalid or expired. Implement try-except blocks to handle potential jwt.DecodeError and jwt.ExpiredSignatureError exceptions.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please follow sourcery-api. |
||
return payload | ||
|
||
|
||
class UserFavouriteView(APIView): | ||
permission_classes = [] | ||
|
||
|
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.
🚨 suggestion (security): Implement proper authentication and authorization for CreateWorldView
The current implementation with empty authentication_classes and permission_classes could allow unauthorized access to world creation. Consider implementing appropriate authentication and permission checks.