Storing permissions (again) ~ AoaH Ten

Storing permissions in an SQLite database (again)

Opening

Last week I looked at using SQLite as a database and, as we all know, I didn't really have a clue what I was doing, here's a link to my previous posts. I got some helpful comments about security from @buinauskas and @tiguchi , also @avalander , rightfully, questioned my use of a class so I've had a look at that too.

ignis-pwa / permissions_helper

Create and modify an SQLite file for managing permissions

permissions_helper

Create and modify an SQLite file for managing permissions

View on GitHub

What were the security issues?

The code I already had in place was not a problem but my style of coding was. I was placing strings into queries using template liberals which meant a savvy user could manipulate their input to get the result they wanted.

For instance, if I were to check for a string in their cookies to see if they were logged in or not it might have looked something like this.

function cookieLogin(session_id) {
  user_id = sql.get(`SELECT user_id FROM users_sessions WHERE session_id= "${session_id}"`);
  return user_id || false
}

Which would be called like so

// normal execution
cookieLogin('$2b$09$oGdnXBsnb8QPCJk/VY1JKuxG/QW66K8RB7n01kBhJsbB45nvIK0pK')
// SELECT user_id FROM users_sessions WHERE session_id="$2b$09$oGdnXBsnb8QPCJk/VY1JKuxG/QW66K8RB7n01kBhJsbB45nvIK0pK"

// problematicexecution
cookieLogin('foo" OR user_id="1')
// SELECT user_id FROM users_sessions WHERE session_id="foo" OR user_id="1"

In the problematic calling of cookieLogin, we'll always get the user_id 1 back, no password required.

I was told to look at "prepared statements", which are built into the SQLite package I'm using. An SQL injection will now only return an error!

Why were you using a class?

I thought it was the done thing, Avalander showed my some of their projects where they had functions and passed the database reference to the function. That made sense to me so I'm no longer using classes, at least not with this project.

I now have a block of exports at the end of my file though, is that normal/the done thing? If there is a better way please feel free to let me know.

module.exports.actionLogin = actionLogin;
module.exports.addUser = addUser;
module.exports.checkPassword = checkPassword;
module.exports.setup = setup;
module.exports.updatePassword = updatePassword;
module.exports.userLogin = userLogin;

How does it work now?

It works, mostly, the same as before. Now rather than having to instantiate a new class I just run a setup function and tell it where the permissions database is.

const ph = require('./permissions_helper');

(async _ => {
  const sql = await ph.setup('perm.db');
  ph.userLogin(sql, 'admin', 'default', 1).then(key => {
    console.log(`Welcome admin you secure key is ${key}`);
  }).catch(err => {
    console.log(err);
  })
})()

It feels a bit hacking doing async like this but I don't like having a whole app with a .then(sql=>{}), maybe there's a better way to do this too?

End of post

Hopefully, the code is a bit better now and less likely to cause me problems in the future, if you see any problems please feel free to leave a comment to let me know I really do appreciate it.

Thank you for reading!

🦄❤🦄🦄❤