Code review comment for lp:~widelands-dev/widelands-website/django1_11

kaputtnik (franku) wrote :

Possible code reviews:

Providing usernames for JS when writing PMs: This is maybe a security risk because a username can contain an at sign (@). The Django documentation says:

"If additional escaping is not desired, you will need to use mark_safe() if you are absolutely sure that your code does not contain XSS vulnerabilities."

I can't evaluate the security risk here. Code: https://bazaar.launchpad.net/~widelands-dev/widelands-website/django1_11/revision/509#mainpage/templatetags/wl_extras.py

RegEx urls: Please check the regexes for urls.py and mainpage/urls.py: https://bazaar.launchpad.net/~widelands-dev/widelands-website/django1_11/revision/503

Password hashers: I am not sure if we need all default hashers. As far i understand the first in PASSWORDHASHERS (so PBKDF2) is used by default. Explanantion: https://docs.djangoproject.com/en/1.11/topics/auth/passwords/#auth-password-storage
Code: https://bazaar.launchpad.net/~widelands-dev/widelands-website/django1_11/revision/500#settings.py

Replacing lambdas with callables: Django can't serialize lambdas for migrations. For the screens app i have replaced the lambdas with callables: https://bazaar.launchpad.net/~widelands-dev/widelands-website/django1_11/revision/494#wlscreens/views.py

For the other things i just followed the recommendations by django, e.g. the additional database options.

« Back to merge proposal