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

Node.js module using Promises (client for KeePassHttp)

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

Problem

This is my first Node module, as well as the first time using Promises in Javascript. It is a client for the KeePass plugin "KeePassHTTP" to expose passwords securely, which I am planning on using to pass credentials to gulp.

All feedback welcome!

```
// Code based on https://github.com/belaviyo/keepass-macpass-helper
// Keepass HTTP protocol documentation - https://github.com/pfn/keepasshttp

const sjcl = require('./sjcl').sjcl; // not using npm module because it doesn't have support for cbc
const nconf = require('nconf');
const rp = require('request-promise-native');

let port = null;
let key = null;
let id = null;

function iv(len = 16) {
let iv = [];
for (let i = 0; i {
let request = {
RequestType: 'test-associate',
TriggerUnlock: false,
};
request = verify(request);
post(request)
.then(response => {
if (response && response['Success']) {
resolve(response);
} else {
reject(response);
}
})
.catch(response => reject(response));
});
}

function associate() {
return new Promise((resolve, reject) => {
let request = {
RequestType: 'associate',
Key: key
};
request = verify(request);
post(request)
.then(response => {
if (response && response['Success']) {
id = response['Id'];
nconf.set('id', id);
nconf.save();
resolve(response);
} else {
reject(response);
}
})
.catch(response => reject(response));
});
}

function logins(url) {
return new Promise((resolve, reject) => {
let request = {
RequestType: 'get-logins',
TriggerUnlock: 'false',
SortSelection: 'false',
};
request = verify(request);
const iv = request['Nonce'];
request['Url'] = encrypt(url, iv);

post(request)
.then(response => {
if (response && response['Entries']) {

Solution

Since no one else has commented on your use of promises I will do so.

Just mainly looking at your promise code I see one repeating pattern that could be Improved. Your use of new Promise() in your code is often not needed in the ways you are using it. Generally creating a new promise is only needed when working with asynchronous code that is not already using promises.

Let's look at one example of how your usage of new Promise is verbose, unnecessary, and a little redundant.

function logins(url) {
  return new Promise((resolve, reject) => {
    let request = {
      RequestType: 'get-logins',
      TriggerUnlock: 'false',
      SortSelection: 'false',
    };
    request = verify(request);
    const iv = request['Nonce'];
    request['Url'] = encrypt(url, iv);

    post(request)
        .then(response => {
          if (response && response['Entries']) {
            let nonce = response.Nonce;
            response.Entries = response.Entries.map(entry => {
              return Object.assign(entry, {
                Login: decrypt(entry.Login, nonce),
                Name: decrypt(entry.Name, nonce),
                Password: decrypt(entry.Password, nonce)
              })
            });
            resolve(response);
          } else {
            reject(response);
          }
        })
        .catch(response => reject(response));
  });
}


The above can be simplified to this

function logins(url) {
    let request = {
        RequestType: 'get-logins',
        TriggerUnlock: 'false',
        SortSelection: 'false',
    };
    request = verify(request);
    const iv = request['Nonce'];
    request['Url'] = encrypt(url, iv);

    return post(request)
        .then(response => {
            if (response && response['Entries']) {
                let nonce = response.Nonce;
                response.Entries = response.Entries.map(entry => {
                    return Object.assign(entry, {
                    Login: decrypt(entry.Login, nonce),
                    Name: decrypt(entry.Name, nonce),
                    Password: decrypt(entry.Password, nonce)
                    })
                });
                return response;
            } else {
                throw new Error(response);
            }
        })
}


In both of those pieces of code a promise is being returned. In both cases the promise will need to be caught by the calling function when an error happens, but will provide the response when no error is there. All I did was refactor the code and it should behave the exact same way. There is now just less complexity. I would do this for all the other times you wrap a promise around another promise.

Note how I threw an error inside the then method of your promise this will just send the error to the next catch in your promise chain (which will be in the calling function since there is no catch there.) Also note that I removed the catch this means any error that would be caught by the catch that was there now must be caught by the calling function.

Hope that helped if you need further help or have any questions let me know.

Code Snippets

function logins(url) {
  return new Promise((resolve, reject) => {
    let request = {
      RequestType: 'get-logins',
      TriggerUnlock: 'false',
      SortSelection: 'false',
    };
    request = verify(request);
    const iv = request['Nonce'];
    request['Url'] = encrypt(url, iv);

    post(request)
        .then(response => {
          if (response && response['Entries']) {
            let nonce = response.Nonce;
            response.Entries = response.Entries.map(entry => {
              return Object.assign(entry, {
                Login: decrypt(entry.Login, nonce),
                Name: decrypt(entry.Name, nonce),
                Password: decrypt(entry.Password, nonce)
              })
            });
            resolve(response);
          } else {
            reject(response);
          }
        })
        .catch(response => reject(response));
  });
}
function logins(url) {
    let request = {
        RequestType: 'get-logins',
        TriggerUnlock: 'false',
        SortSelection: 'false',
    };
    request = verify(request);
    const iv = request['Nonce'];
    request['Url'] = encrypt(url, iv);

    return post(request)
        .then(response => {
            if (response && response['Entries']) {
                let nonce = response.Nonce;
                response.Entries = response.Entries.map(entry => {
                    return Object.assign(entry, {
                    Login: decrypt(entry.Login, nonce),
                    Name: decrypt(entry.Name, nonce),
                    Password: decrypt(entry.Password, nonce)
                    })
                });
                return response;
            } else {
                throw new Error(response);
            }
        })
}

Context

StackExchange Code Review Q#157013, answer score: 3

Revisions (0)

No revisions yet.