Skip to content
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

Solution #790

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Solution #790

wants to merge 2 commits into from

Conversation

sumseer
Copy link

@sumseer sumseer commented Dec 10, 2024

No description provided.

Copy link

@sofiiasavkova sofiiasavkova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your hard work on this project! 🚀 While your code is mostly well-structured and meets many of the requirements, there are a couple of critical security issues that need to be addressed before we can proceed. Specifically, the DEBUG setting is currently set to True, which should be False in a production environment to prevent exposure of sensitive information. Additionally, the SECRET_KEY is hardcoded, and it's important to use environment variables to manage this sensitive information securely. Once these issues are resolved, your code will be in great shape! Keep up the great work! 😊

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.

# See https://docs.djangoproject.com/en/5.1/howto/deployment/checklist/

# SECURITY WARNING: keep the secret key used in production secret!
SECRET_KEY = "django-insecure-#ey3olt$nma!q#-@+kk%ys*w)esqge%z8*g8elb=u55*53xi$+"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security Issue: The SECRET_KEY is hardcoded. It's recommended to use environment variables to manage sensitive information securely.

core/settings.py Outdated
SECRET_KEY = "django-insecure-#ey3olt$nma!q#-@+kk%ys*w)esqge%z8*g8elb=u55*53xi$+"

# SECURITY WARNING: don't run with debug turned on in production!
DEBUG = True

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security Issue: DEBUG is set to True. This should be set to False in production to avoid exposing sensitive information.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job on your Django project! 🎉 Your implementation meets the task requirements, with a well-structured project and app setup, correctly implemented Movie model, serializer, API views, and URL configurations. The .gitignore file is comprehensive, and your code style is clean and follows guidelines.

However, there is a critical issue with the SECRET_KEY being hardcoded in the settings.py file. For security reasons, especially in production environments, it's important to manage sensitive information like the SECRET_KEY using environment variables. Please address this in future updates to enhance the security of your application.

Overall, your work is impressive, and this decision is made to help you move forward with your learning. Keep up the great work and continue improving your code! 🚀

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

# See https://docs.djangoproject.com/en/5.1/howto/deployment/checklist/

# SECURITY WARNING: keep the secret key used in production secret!
SECRET_KEY = "django-insecure-#ey3olt$nma!q#-@+kk%ys*w)esqge%z8*g8elb=u55*53xi$+"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SECRET_KEY is hardcoded, which is not secure for production. Consider using environment variables to manage this sensitive information securely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants