ghaiklor / generator-sails-rest-api

Yeoman generator for scaffolding Sails REST API with predefined features
http://ghaiklor.github.io/generator-sails-rest-api/
MIT License
334 stars 52 forks source link

should move sensitive configs into local.js to prevent them from being in the repos #301

Open mikedevita opened 7 years ago

mikedevita commented 7 years ago

I will be submitting another PR to resolve this if you like, basically in the api/services/* files you do something like this.. this can be applied for things like db name, user, host and password.

const _      = require('lodash')
const config = _.merge(require('../../config/services/mailer'), require('../../config/local'));

and create a config/local.js with something like this....

"use strict";

module.exports = {
  services: {
    cipher: {},
    hash: {},
    image: {},
    location: {},
    mailer: {},
    payment: {},
    pusher: {},
    sms: {},
    social: {},
    storage: {}
  }
}
ghaiklor commented 7 years ago

@mikedevita you can achieve the same behavior with default Sails setup. Just create config/local.js file and it overrides any properties you declare there. There is no needs for implementing such kind of features.

http://sailsjs.com/documentation/concepts/configuration/the-local-js-file

mikedevita commented 7 years ago

Does it override or does it merge?

ghaiklor commented 7 years ago

@mikedevita it overrides and merges. If property exists, it will be overridden, otherwise it will be merged.

mikedevita commented 7 years ago

@ghaiklor that idea wont work as is because you call const config = require('../../config/services/mailer'); independently and don't hook into sails.config.services.* so either there needs to be a change in the config to use sails.config.services or use merge like i suggested.

which if going the route of sails.config.services then i don't think it works out of box because sails isn't accessible in the services files as is. Some minor refactoring will need to be redone..

each service should module.exports and then be wrapped in a function..

module.exports = {
  jwt: function() {
    return cipher('jwt', sails.config.services.cipher.jwt)
  }
}
ghaiklor commented 7 years ago

@mikedevita yeah, I see, makes sense. It will be great to get rid of direct requiring of configuration files and use sails.config.

mikedevita commented 7 years ago

Ill look into this and submit another PR..

mikedevita commented 7 years ago

a bit of an update, by making the services functions you can then obtain access to sails.config

e.g;

api/services/CipherService.js

module.exports = {
  jwt: (config) => cipher('jwt', _.merge({}, sails.config.services.cipher.jwt, config))
}

doing this you can then change anywhere CipherService.jwt.encodeSync() to be CipherService.jwt().encodeSync() I am not sure how to modify the yo generators to include this new syntax though. So any help would be appreciated there.