apiko-rest-api / apiko

The Apiko itself!
https://apiko-rest-api.github.io/
MIT License
16 stars 6 forks source link

Unused verifyPassword() in data.js #53

Closed ilabacheuski closed 7 years ago

ilabacheuski commented 7 years ago

While it's cool to have a function that uniformally verify password in DB, but this one is never used and contains linting errors. Unused function verifyPassword()

verifyPassword (username, password) {
      return new Promise((resolve, reject) => {
        g.data.store.models.users.findOne({ where: { username: username } }).then((user) => {
          if (bcrypt.compareSync(password, user.password)) {
            resolve()
          } else {
            reject() // !!! --->> reject requires a new Error instance
          }
        }).catch(() => {
          reject() // !!! --->> reject requires a new Error instance
        })
      })
    }
kasp1 commented 7 years ago

What do you suggest? :)

ilabacheuski commented 7 years ago

Delete unused function. If no one use it - why is it here?

kasp1 commented 7 years ago

Perhaps it was originally supposed to be a central function to verify password. This "central" function would be used from user/login handler and also would be available through "api" to any custom user handlers.

Maybe we could also just add it to the handler API (in this function: https://github.com/apiko-rest-api/apiko/blob/master/src/ender.js#L258 ), so it is available to everyone and just use it in user/login.js from there.

ilabacheuski commented 7 years ago

Now it's just a code without usage. That's all. 😄 We can change it od course. And I don't think it should be in data module. It's authorization logic.

kasp1 commented 7 years ago

Where do you think it should be?

ilabacheuski commented 7 years ago

Right now I think we can delete it safely. If we need it in future we can add module auth and keep all of such functions. I made PR to delete it and this will solve last 2 errors with linting the code with ESLint

kasp1 commented 7 years ago

Good idea.

ilabacheuski commented 7 years ago

61 closed