Is this implementation for token based authentication in flask-peewee secure and efficient?

Question

I'm making an app with a Flask API backend using a Flask-Peewee ORM and an AngularJS frontend.

The Flask-Peewee ORM doesn't support token based authentication, so I decided to try to implement this myself. This is quite a bit beyond what I usually do, and I have in fact no idea whether this is secure or not. It does work, but that's all I know. Any feedback, from pointing out obvious security flaws to advice on good practices, is more than welcome.

This is my implementation:

Models

I made an extra model for the api-key:

class APIKey(db.Model):
 key = CharField()
 secret = CharField()
 user = ForeignKeyField(User)

Controllers

I made controllers for logging in, logging out, and registration. These all return a response, which is then used by AngularJS. Basically, it creates an APIKey on succesful login and destroys all APIkeys related to the user on logout.

@app.route('/login', methods=['POST'])
def login():
 try:
 key = request.json["key"]
 secret = request.json["secret"]
 print "Key and secret delivered"
 try:
 user = User.select().join(APIKey).where(APIKey.key == key, APIKey.secret == secret).get()
 print "APIKey found"
 return jsonify ({"success" : True, "user_id" : user.id})
 except: #key and secret were invalid
 print "Key and secret invalid"
 return jsonify({"success" : False, "reason" : "key invalid"})
 except KeyError: # no key delivered, check for any existing keys for the user, if not, create one
 try: #check whether a username and password were delivered
 username = request.json["username"]
 password = request.json["password"]
 print "Username and password found"
 user = User.get(User.username == username)
 print "Username exists"
 if user.check_password(password): #if the user exists and the password is correct
 print "Password correct"
 key = urandom(50).encode('hex')
 secret = urandom(50).encode('hex')
 key_secret = APIKey.get_or_create(key = key, secret = secret, user = user)
 return jsonify ({"success" : True, "user_id" : user.id, "key" : key, "secret" : secret})
 else: #the user exists, but the password is incorrect
 return jsonify ({"success" : False, "reason" : "Password incorrect"})
 except (User.DoesNotExist, KeyError): #the user does not exist
 return jsonify ({"success" : False, "reason" : "User does not exit"})
@app.route('/logout', methods=['POST'])
def logout():
 key = request.json["key"]
 secret = request.json["secret"]
 try:
 user = User.select().join(APIKey).where(APIKey.key == key, APIKey.secret == secret)
 api_keys = APIKey.delete().where(APIKey.user == user)
 api_keys.execute()
 return jsonify({"success" : True, "key_removed" : True})
 except APIKey.DoesNotExist:
 return jsonify({"success" : True, "key_removed" : False})
@app.route('/join', methods=['POST'])
def join():
 user_to_join = request.json
 user = User(username = user_to_join["username"], email = user_to_join["email"])
 user.set_password(user_to_join["password"])
 user.save()
 return jsonify ({"success" : True})

I also modified the APIKeyAuthentication class to work with the token:

class APIKeyAuthentication(Authentication):
 """
 Requires a model that has at least two fields, "key" and "secret", which will
 be searched for when authing a request.
 """
 key_field = 'key'
 secret_field = 'secret'
 def __init__(self, auth, model, protected_methods=None):
 super(APIKeyAuthentication, self).__init__(protected_methods)
 self.model = model
 self._key_field = model._meta.fields[self.key_field]
 self._secret_field = model._meta.fields[self.secret_field]
 def get_query(self):
 return self.model.select()
 def get_key(self, k, s):
 try:
 return self.get_query().where(
 self._key_field==k,
 self._secret_field==s
 ).get()
 except self.model.DoesNotExist:
 pass
 def get_key_secret(self):
 for search in [request.headers, request.args, request.form]:
 if 'key' in search and 'secret' in search:
 return search['key'], search['secret']
 return None, None
 def authorize(self):
 if request.method not in self.protected_methods:
 return True
 key, secret = self.get_key_secret()
 try:
 g.user = User.select().join(APIKey).where(APIKey.key == key, APIKey.secret == secret).get()
 return g.user
 except User.DoesNotExist:
 return False

Front-end

In Angular, I do the following:

$scope.$on('$viewContentLoaded', function() {
 if (localStorageService.get('key') == undefined)
 {
 $rootScope.logged_in = false;
 $scope.openLoginWindow();
 }
 else
 {
 $rootScope.key = localStorageService.get('key');
 $rootScope.secret = localStorageService.get('secret');
 $http.post('/login', {"key" : $rootScope.key, "secret" : $rootScope.secret})
 .success(function(response){
 if (response.success)
 {
 $rootScope.user_id = response.user_id;
 $rootScope.$broadcast("key_set");
 $rootScope.logged_in = true;
 [...further code while logged in ...]

If the user is not logged in, they will be taken to a login window (this app is for logged-in users only), that posts the username and password as follows:

$scope.login = function(){
 $http.post('/login', $scope.user_to_login)
 .success(function(response){
 if (response.success)
 {
 $rootScope.key = response.key;
 $rootScope.secret = response.secret;
 $rootScope.user_id = response.user_id;
 $rootScope.$broadcast("key_set");
 if ($scope.user_to_login.remember)
 {
 localStorageService.add('key', $rootScope.key);
 localStorageService.add('secret', $rootScope.secret);
 }
 $modalInstance.close(response);
 }
 else
 {
 ///
 }
 });
 };

• Erik Lieben: If you need to get a resource from the service later on, do you post the userid, the secret and key or only the key to the server?
• Ben: @erik both key and secret are used to authenticate to the API; userid is unnecessary because the key is linked to the user in the database through APIKey.user.

Answer 1

A short review:

• I havent spotted anything inherently insecure or inefficient
• It behooves you to check that the generated key and secret were not already assigned to a different user (it does not matter that the odds are astronomically low)

• The below should probably return "success": False

  except APIKey.DoesNotExist:
   return jsonify({"success" : True, "key_removed" : False})
  

• Since you use api_keys only once, you could replace the below with a oneliner. Also, in this case, api_keys is a terrible name

  api_keys = APIKey.delete().where(APIKey.user == user)
  api_keys.execute()
  

  becomes

   APIKey.delete().where(APIKey.user == user).execute()