-
Notifications
You must be signed in to change notification settings - Fork 28
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 4 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 |
---|---|---|
|
@@ -15,6 +15,7 @@ | |
from rest_framework.decorators import api_view, permission_classes | ||
from rest_framework.response import Response | ||
from rest_framework.views import APIView | ||
from django.forms.models import model_to_dict | ||
|
||
from venueless.api.auth import ( | ||
ApiAccessRequiredPermission, | ||
|
@@ -27,6 +28,9 @@ | |
from venueless.core.services.world import notify_schedule_change, notify_world_change | ||
|
||
from ..core.models import Room, World | ||
import jwt | ||
from django.conf import settings | ||
from django.utils.crypto import get_random_string | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
@@ -115,6 +119,70 @@ 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, | ||
} | ||
] | ||
} | ||
|
||
# if world already exists, update it, otherwise create a new world | ||
try: | ||
if World.objects.filter(id=request.data.get('id')).exists(): | ||
world = World.objects.get(id=request.data.get('id')) | ||
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): Use .get() with default value or check key existence Using .get() without a default value on request.data can raise KeyError if the key doesn't exist. Consider using .get('id', None) or check if 'id' in request.data before accessing it.
|
||
world.title = request.data.get('title')[request.data.get('locale')] or request.data.get('title')[ | ||
'en'] | ||
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'], | ||
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(f"An error occurred while creating a world: %s" % 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 = [] | ||
|
||
|
@@ -183,19 +251,19 @@ def get(request, *args, **kwargs): | |
talk_config = world.config.get("pretalx") | ||
user = User.objects.filter(token_id=request.user) | ||
talk_base_url = ( | ||
talk_config.get("domain") | ||
+ "/" | ||
+ talk_config.get("event") | ||
+ "/schedule/export/" | ||
talk_config.get("domain") | ||
+ "/" | ||
+ talk_config.get("event") | ||
+ "/schedule/export/" | ||
) | ||
export_endpoint = "schedule." + export_type | ||
talk_url = talk_base_url + export_endpoint | ||
if "my" in export_type and user: | ||
user_state = user.first().client_state | ||
if ( | ||
user_state | ||
and user_state.get("schedule") | ||
and user_state.get("schedule").get("favs") | ||
user_state | ||
and user_state.get("schedule") | ||
and user_state.get("schedule").get("favs") | ||
): | ||
talk_list = user_state.get("schedule").get("favs") | ||
talk_list_str = ",".join(talk_list) | ||
|
@@ -231,7 +299,7 @@ def schedule_update(request, **kwargs): | |
|
||
pretalx_config = request.world.config.get("pretalx", {}) | ||
if domain != get_domain( | ||
pretalx_config.get("domain") | ||
pretalx_config.get("domain") | ||
) or event != pretalx_config.get("event"): | ||
return Response("Incorrect domain or event data", status=401) | ||
|
||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -40,6 +40,7 @@ | |||||
"VENUELESS_DJANGO_SECRET", config.get("django", "secret", fallback="") | ||||||
) | ||||||
BASE_PATH = config.get("venueless", "base_path", fallback="") | ||||||
DOMAIN_PATH = config.get("venueless", "domain_path", fallback="https://app.eventyay.com") | ||||||
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 (security): Consider using environment variables for configuration Using environment variables for configuration settings like DOMAIN_PATH can improve security and flexibility, especially when deploying in different environments.
Suggested change
|
||||||
if not SECRET_KEY: | ||||||
SECRET_FILE = os.path.join(DATA_DIR, ".secret") | ||||||
if os.path.exists(SECRET_FILE): | ||||||
|
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.