HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Custom Google App Engine Python user managment

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
enginegoogleappusercustompythonmanagment

Problem

I've written this script as a basic authentication system for use on Google App Engine. I would appreciate it if anyone would mind having a look and seeing if there are any obvious security vulnerabilities.

Overview

The whole thing is an API so obviously there will be a client (probably written in JavaScript) which will interface with it.

User creates an account using /user/register and a 'random' code is generated using the time, their email address and the key for 'their' entity. This is then emailed to them with a link to the verification page.

This then checks the verification code against one which is generated again using the same values as before (the rationale being that an attacker would need to have access to the code as well as the datastore to be able to generate a code to use in the verification).

Authentication is then done using either and auth key or the users password.

The auth key is generated using the entity key, their IP address and their email address and is stored. This key is returned if the correct password had been given and therefore 'proves' that the client knows/knew the password. This would then be stored by the client (perhaps using the JavaScript localstorage API) so that actions can be done on the account without needing to re-enter the password.

An example of the auth code being used can be seen in the protectedthing class. This would allow some action to be take which required access to the account but which wasn't too critical.

There is also the option to authenticate using a password as seen in the realyprotectedthing class. This would be used for something which was more critical and where we needed to ensure that it really was the user.

There are some obvious areas of improvement such as password validation but I would like to check that the basic idea seems safe.

protectedthing and reallyprotectedthing are examples of how the other functions would be used. protectedthing is some sort of action which only

Solution

I've never used Google App Engine so I can't comment on your approach and the biggest questions,
but there are a couple of coding practice issues worth noting.

PEP8

You are clearly not following PEP8,
the official coding style guide of Python.
Give it a good read and start following it to improve the readability of your code.
The most notable violations that jump in the eye:

  • Need more line breaks:



  • There should be 2 empty lines before top-level class or function definitions



  • There should be 1 empty line before function definitions within a class



  • Put spaces around operators, for example:



  • Instead of: str(self.key)[8:10]+self.email



  • Write like this: str(self.key)[8:10] + self.email



-
Some lines are way too long

-
For example this is a bit excessive, don't you think?

mail.send_mail("email@example.com",user.email,"Account Registration Email Confirmation",'You have succesfully registered for an account, please confirm you email address using the link bellow to get started  ')


  • Use snake_case instead manywordsstucktogether for function and variable names



Overall, this code looks very messy.
For an API, especially security related,
it's really better to make the code as clear and readable as possible,
and following the PEP8 rules will help with that.

Duplicated logic

This kind of call appears at multiple places in the code:

((str(self.key)[8:10]+self.email+str(self.verificationcoderequested))

(str(self.key)[8:10]+self.email + ip)


The problem with duplicated logic is that later if you need to change something in that expression, you have to remember to make the same change everywhere.
To avoid making a mistake,
it's better to put this logic into a dedicated helper function, for example:

def create_key(self, extra):
    return str(self.key)[8:10] + self.email + extra


Pythonic conditions

These conditions are not Pythonic:

if self.verificationcoderequested != None:
          # ...

      if self.verified == True:
          # ...


The Pythonic way is simply:

if self.verificationcoderequested:
          # ...

      if self.verified:
          # ...


Also, when you really need to compare with None,
you should use is None and is not None instead of == None and != None, respectively.

Other issues

At the end of checkverificationcode,
the last return statement (return "failure") is unreachable code,
you should remove it.

Code Snippets

mail.send_mail("email@example.com",user.email,"Account Registration Email Confirmation",'You have succesfully registered for an account, please confirm you email address using the link bellow to get started </br> <a href="http://localhost:9080/user/confirmregistration?code=' + user.getverificationcode() + '&email='+user.email+'"></a>')
((str(self.key)[8:10]+self.email+str(self.verificationcoderequested))

(str(self.key)[8:10]+self.email + ip)
def create_key(self, extra):
    return str(self.key)[8:10] + self.email + extra
if self.verificationcoderequested != None:
          # ...

      if self.verified == True:
          # ...
if self.verificationcoderequested:
          # ...

      if self.verified:
          # ...

Context

StackExchange Code Review Q#74312, answer score: 3

Revisions (0)

No revisions yet.