Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
mikedevita opened this issue Dec 14, 2016 · 7 comments

Comments

@mikedevita
Copy link
Contributor

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
Copy link
Owner

@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
Copy link
Contributor Author

Does it override or does it merge?

@ghaiklor
Copy link
Owner

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

@mikedevita
Copy link
Contributor Author

mikedevita commented Dec 15, 2016

@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
Copy link
Owner

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

@mikedevita
Copy link
Contributor Author

mikedevita commented Dec 15, 2016 via email

@mikedevita
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants