Django API Implementation

Question

I am building a backend to a mobile application that will be hosted on a Django site connected to a PostgreSQL database.

I have never built anything to accomplish this before and this is my first go at it. I have not studied this type of application before so please let me know if the below code follows best practices for this task, is secure, and doesn't have any obvious problems. If there is an industry standard for this that is very different, please let me know what it is and why it is preferred.

My client connects via the view readJSON:

@csrf_exempt
def readJSON(request):
    if request.method == 'POST':
        try:
            c = connections['postgres_db'].cursor()
            json_data = json.loads(request.body)
            if json_data['tag'] == 'register':
                return HttpResponse(cc().register(c, json_data))
            elif json_data['tag'] == 'syncprofile':
                return HttpResponse(cc().syncProfile(c, json_data))
################ A long list of other possible tags
            else:
                raise Http404
        finally:
            c.close()
    else:
        raise Http404

I have a separate class that holds all the code to handle the incoming JSON requests. This is one such function.

    def register(self, c, json_data):
    import bcrypt as bc
    salt = bc.gensalt()
    pwrd = json_data['data']['password']
    hash = bc.hashpw(pwrd, salt)
    try:
        c.execute("INSERT INTO profiles VALUES (DEFAULT, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s)",
            (json_data['data']['email'], hash, salt, json_data['data']['date_created'],
            json_data['data']['state_lvl'], True,
            json_data['data']['first_name'], json_data['data']['last_name'],
            json_data['data']['sex'], json_data['data']['age'],
            json_data['data']['state'], json_data['data']['country'],
            json_data['data']['language'], json_data['data']['device_type'],
            json_data['data']['device'], json_data['data']['device_width'],
            json_data['data']['device_height'], json_data['data']['device_screen_density']
        ))
        json_response = json.dumps({"success": 1, "errors": 0})
        return HttpResponse(json_response, mimetype="application/json")
    except Exception,e:
        json_response = json.dumps({"success": 0, "errors": 1, "msg": str(e)})
        return HttpResponse(json_response, mimetype="application/json")

And here is the JSON structure for the above function.

    {
   "tag":"register",
   "data":{
      "email":"[email protected]",
      "password":"app123",
      "date_created":"07/10/1967",
      "state_lvl":"1",
      "first_name":"John",
      "last_name":"Smith",
      "sex":"m",
      "age":23,
      "state":"CA",
      "country":"USA",
      "language":"english",
      "device_type":"phone",
      "device":"Nexus-5",
      "device_width":400,
      "device_height":800,
      "device_screen_density":331
   }
}

Answer 1

readJSON()

Returning HTTP 404 ("Not Found") is improper, unless you are deliberately sending obfuscatory responses as a kind of security-by-obscurity measure. If you require a POST, and the client sends anything but a POST, the proper response should HTTP 405 ("Method Not Allowed"). For a body that does not have an appropriate JSON tag, I think that the appropriate response would be a non-specific HTTP 400 ("Bad Request").

In if-else branches, prefer to put the branch with less code first, to get it out of the way and reduce mental load. Usually, that means putting the error handler first. As a bonus, you can eliminate one level of indentation.

Instead of the try-finally construct, use a with block for the database cursor.

Assuming that there is an exact correspondence between the JSON tags and the method names of cc(), you could dispatch dynamically. (Your syncProfile() method doesn't match 'syncprofile', but you could rename your methods accordingly.)

@csrf_exempt
def readJSON(request):
    if request.method != 'POST':
        raise Http405
    json_data = json.loads(request.body)

    # Whitelist allowable function calls for security
    if json_data['tag'] not in ['register', 'syncprofile', … ]:
        raise Http400

    cc_method = getattr(cc(), json_data['tag'])
    with connections['postgres_db'].cursor() as c:
        return HttpResponse(cc_method(c, json_data))

register()

Imports should be listed at the top of the file. I wouldn't alias bcrypt as bc, as the latter sacrifices clarity just to save a few characters.

There are a lot of columns to be inserted, and a long parameter list. I think it would be less error prone if you used named placeholders.

import bcrypt

def register(self, c, json_data):
    attr = dict(json_data['data'])
    attr['salt'] = bcrypt.gensalt()
    password = attr['password']
    attr['hash'] = bcrypt.hashpw(password, attr['salt'])
    attr['true'] = True
    try:
        c.execute("""INSERT INTO profiles VALUES (DEFAULT
                         , %(email)s
                         , %(hash)s
                         , %(salt)s
                         , %(date_created)s
                         , %(state_lvl)s
                         , %(true)s
                         , %(first_name)s
                         , %(last_name)s
                         , %(sex)s
                         , %(age)s
                         , %(state)s
                         , %(country)s
                         , %(language)s
                         , %(device_type)s
                         , %(device)s
                         , %(device_width)s
                         , %(device_height)s
                         , %(device_screen_density)s
                     )""", attr);
        json_response = json.dumps({"success": 1, "errors": 0})
    except Exception, e:
        json_response = json.dumps({"success": 0, "errors": 1, "msg": str(e)})
    return HttpResponse(json_response, mimetype="application/json")

It's a bit odd that you let the client specify the profile creation date. I would just let the database issue a timestamp.

I think that the long parameter list is indicative of a problematic database schema. If you ever want to add another attribute to the user profile, that would require a schema alteration. The profiles table should probably be split into an accounts table containing the essentials and an account_attributes table that stores arbitrary key-value pairs.

CREATE TABLE accounts
( id SERIAL PRIMARY KEY
, email TEXT NOT NULL
, hash TEXT NOT NULL
, salt TEXT NOT NULL
, date_created DATE NOT NULL DEFAULT statement_timestamp()
);

CREATE TABLE account_attributes
( account_id INTEGER NOT NULL
, attribute TEXT NOT NULL
, value TEXT NOT NULL
, PRIMARY KEY (account_id, attribute)
, FOREIGN KEY (account_id) REFERENCES accounts (id)
);

Answer 2

In case it helps a little ... in your register function, you can simplify the creation of the string for the c.execute statement.

execute_text = ("INSERT INTO profiles VALUES (DEFAULT, %s, %s, %s, %s, %s, %s" %
                (json_data['data']['email'], hash, salt, 
                 json_data['data']['date_created'], json_data['data']['state_lvl'], 
                 True,))
json_fields = ['first_name', 'last_name', 'sex', 'age', 'state', 'country',
               'language', 'device_type', 'device', 'device_width', 'device_height',
               'device_screen_density']

for field in json_fields:
  execute_text = "%s, %s" % (execute_text, json_data['data'][field])
execute_text = "%s)" % execute_text