celiao / django-rest-authemail

A RESTful API for user signup and authentication using email addresses.
GNU General Public License v3.0
315 stars 92 forks source link

Issue with ipaddr and nginx #28

Closed DeeeeLAN closed 3 years ago

DeeeeLAN commented 4 years ago

Hi,

My server is throwing the following exception:

Internal Server Error: /api/register/
Traceback (most recent call last):
  File ".../env/lib/python3.8/site-packages/django/db/backends/mysql/base.py", line 73, in execute
    return self.cursor.execute(query, args)
  File ".../env/lib/python3.8/site-packages/MySQLdb/cursors.py", line 209, in execute
    res = self._query(query)
  File ".../env/lib/python3.8/site-packages/MySQLdb/cursors.py", line 315, in _query
    db.query(q)
  File ".../env/lib/python3.8/site-packages/MySQLdb/connections.py", line 226, in query
    _mysql.connection.query(self, query)
MySQLdb._exceptions.OperationalError: (1048, "Column 'ipaddr' cannot be null")

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File ".../env/lib/python3.8/site-packages/django/core/handlers/exception.py", line 47, in inner
    response = get_response(request)
  File ".../env/lib/python3.8/site-packages/django/core/handlers/base.py", line 179, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File ".../env/lib/python3.8/site-packages/django/views/decorators/csrf.py", line 54, in wrapped_view
    return view_func(*args, **kwargs)
  File ".../env/lib/python3.8/site-packages/django/views/generic/base.py", line 70, in view
    return self.dispatch(request, *args, **kwargs)
  File ".../env/lib/python3.8/site-packages/rest_framework/views.py", line 509, in dispatch
    response = self.handle_exception(exc)
  File ".../env/lib/python3.8/site-packages/rest_framework/views.py", line 469, in handle_exception
    self.raise_uncaught_exception(exc)
  File ".../env/lib/python3.8/site-packages/rest_framework/views.py", line 480, in raise_uncaught_exception
    raise exc
  File ".../env/lib/python3.8/site-packages/rest_framework/views.py", line 506, in dispatch
    response = handler(request, *args, **kwargs)
  File ".../env/lib/python3.8/site-packages/authemail/views.py", line 71, in post
    signup_code = SignupCode.objects.create_signup_code(user)
  File ".../env/lib/python3.8/site-packages/authemail/models.py", line 105, in create_signup_code
    signup_code = self.create(user=user, code=code)
  File ".../env/lib/python3.8/site-packages/django/db/models/manager.py", line 85, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File ".../env/lib/python3.8/site-packages/django/db/models/query.py", line 447, in create
    obj.save(force_insert=True, using=self.db)
  File ".../env/lib/python3.8/site-packages/django/db/models/base.py", line 753, in save
    self.save_base(using=using, force_insert=force_insert,
  File ".../env/lib/python3.8/site-packages/django/db/models/base.py", line 790, in save_base
    updated = self._save_table(
  File ".../env/lib/python3.8/site-packages/django/db/models/base.py", line 895, in _save_table
    results = self._do_insert(cls._base_manager, using, fields, returning_fields, raw)
  File ".../env/lib/python3.8/site-packages/django/db/models/base.py", line 933, in _do_insert
    return manager._insert(
  File ".../env/lib/python3.8/site-packages/django/db/models/manager.py", line 85, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File ".../env/lib/python3.8/site-packages/django/db/models/query.py", line 1254, in _insert
    return query.get_compiler(using=using).execute_sql(returning_fields)
  File ".../env/lib/python3.8/site-packages/django/db/models/sql/compiler.py", line 1397, in execute_sql
    cursor.execute(sql, params)
  File ".../env/lib/python3.8/site-packages/django/db/backends/utils.py", line 66, in execute
    return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
  File ".../env/lib/python3.8/site-packages/django/db/backends/utils.py", line 75, in _execute_with_wrappers
    return executor(sql, params, many, context)
  File ".../env/lib/python3.8/site-packages/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
  File ".../env/lib/python3.8/site-packages/django/db/backends/mysql/base.py", line 78, in execute
    raise IntegrityError(*tuple(e.args))
django.db.utils.IntegrityError: (1048, "Column 'ipaddr' cannot be null")

I was looking for solutions online and stumbled on this post from a long time ago indicating the 'REMOTE_ADDR' field is really bad practice to use: https://www.djangoproject.com/weblog/2009/jul/28/security/#secondary-issue

See more here: https://stackoverflow.com/questions/4581789/how-do-i-get-user-ip-address-in-django

Is the IP address necessary? I noticed it gets saved with the code to the database, but as far as I can tell it isn't used for anything after that.

DeeeeLAN commented 4 years ago

I bring it up because nginx is causing IP_ADDR to be set to null, so the .get call doesn't set it to the default 0.0.0.0.

celiao commented 4 years ago

I added the storing of an IP address with the signup code, in case the application programmer wants to take action if multiple signup codes are being requested from the same IP address.

How do you propose the getting the IP address of the client?

Alternately, I could check if ipaddr is null and set it to 0.0.0.0.

DeeeeLAN commented 4 years ago

As mentioned in that post, this seems like it is probably your best bet, it will handle all the various address locations for you:

https://github.com/un33k/django-ipware#notice

But since there isn’t really a good defense against spoofing, I think it would be nice to have an option to just disable it as well. Make the database key optional and just skip it if I disable it.

celiao commented 4 years ago

Great. Do you think this would be sufficient?

from ipware import get_client_ip
...
    client_ip = get_client_ip(request)[0]
    if client_ip is None:
        client_ip = '0.0.0.0'    # Unable to get the client's IP address
    signup_code = SignupCode.objects.create_signup_code(user, client_ip)
...
DeeeeLAN commented 4 years ago

Makes sense to me. I will let you know if it works on my end.

celiao commented 4 years ago

Great. Works on my end. LMK.

celiao commented 4 years ago

@DeeeeLAN Any luck getting it to work on your end?

DeeeeLAN commented 4 years ago

Sorry, I have been working on other things and haven't had time to get around to testing this. I will try and work on it and get it tested this week.

celiao commented 3 years ago

Closing due to inactivity.

DeeeeLAN commented 3 years ago

I finally got around to testing this and I can confirm it has fixed the failures I was seeing, if you want to merge it into master.

celiao commented 1 year ago

Fixed with #54