Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

Add config options 'domain' #871

Closed
wants to merge 1 commit into from
Closed

Add config options 'domain' #871

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions config/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,15 @@ var validateEnvironmentVariable = function () {
console.log(chalk.white(''));
};

/**
* Validate config.domain is set
*/
var validateDomainIsSet = function (config) {
if(!config.app.domain){
console.log(chalk.red('+ Important warning: config.domain is empty. For security reasons it should be set to the domain of the app.'));
}
};

/**
* Validate Secure=true parameter can actually be turned on
* because it requires certs and key files to be available
Expand Down Expand Up @@ -187,6 +196,9 @@ var initGlobalConfig = function () {
var pkg = require(path.resolve('./package.json'));
config.meanjs = pkg;

// Print a warning if config.domain is not set
validateDomainIsSet(config);

// We only extend the config object with the local.js custom/local environment if we are on
// production or development environment. If test environment is used we don't merge it with local.js
// to avoid running test suites on a prod/dev environment (which delete records and make modifications)
Expand Down
3 changes: 2 additions & 1 deletion config/env/default.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ module.exports = {
title: 'MEAN.JS',
description: 'Full-Stack JavaScript with MongoDB, Express, AngularJS, and Node.js',
keywords: 'mongodb, express, angularjs, node.js, mongoose, passport',
googleAnalyticsTrackingID: process.env.GOOGLE_ANALYTICS_TRACKING_ID || 'GOOGLE_ANALYTICS_TRACKING_ID'
googleAnalyticsTrackingID: process.env.GOOGLE_ANALYTICS_TRACKING_ID || 'GOOGLE_ANALYTICS_TRACKING_ID',
domain: process.env.DOMAIN
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be domain: process.env.DOMAIN || 'localhost' as @mleanos suggested

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

domain: process.env.DOMAIN || 'localhost' won't work since it has to include the port.

For example it could (and should) be used to assemble the URL to reset the password. This url has to include the port.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course, so just doing something like this then:

domain: process.env.DOMAIN || 'localhost' + port

where port is already a configuration parameter as part of the config.

Does that make sense?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes, that would work. But what purpose does it serve? If the domain is missing it is already set dynamically in express.js.

Also I would only do this in development.js, not in default.js because 'localhost' + port is most certainly wrong for production. Setting it development.js would prevent the 'no-domain-set' warning in development mode, so that would be useful.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Koblaid I think you're right and we can leave this setting as is, but make the discussed change here in the development.js file and test.js file. Look on my comments there.

},
port: process.env.PORT || 3000,
templateEngine: 'swig',
Expand Down
3 changes: 2 additions & 1 deletion config/env/development.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ module.exports = {
}
},
app: {
title: defaultEnvConfig.app.title + ' - Development Environment'
title: defaultEnvConfig.app.title + ' - Development Environment',
domain: process.env.DOMAIN || 'localhost:' + defaultEnvConfig.port
},
facebook: {
clientID: process.env.FACEBOOK_ID || 'APP_ID',
Expand Down
3 changes: 2 additions & 1 deletion config/env/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ module.exports = {
},
port: process.env.PORT || 3001,
app: {
title: defaultEnvConfig.app.title + ' - Test Environment'
title: defaultEnvConfig.app.title + ' - Test Environment',
domain: process.env.DOMAIN || 'localhost:3001'
},
facebook: {
clientID: process.env.FACEBOOK_ID || 'APP_ID',
Expand Down
5 changes: 3 additions & 2 deletions config/lib/express.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@ module.exports.initLocalVariables = function (app) {

// Passing the request url to environment locals
app.use(function (req, res, next) {
res.locals.host = req.protocol + '://' + req.hostname;
res.locals.url = req.protocol + '://' + req.headers.host + req.originalUrl;
var domain = config.app.domain || req.headers.host;
res.locals.host = req.protocol + '://' + domain;
res.locals.url = req.protocol + '://' + domain + req.originalUrl;
next();
});
};
Expand Down
7 changes: 6 additions & 1 deletion modules/core/server/views/layout.server.view.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,12 @@

{% if livereload %}
<!--Livereload script rendered -->
<script type="text/javascript" src="{{host}}:35729/livereload.js"></script>
<script type="text/javascript">
var scriptTag = document.createElement('script');
scriptTag.setAttribute('type', 'text/javascript');
scriptTag.setAttribute('src', '//' + window.location.hostname + ':35729/livereload.js');
document.head.appendChild(scriptTag);
</script>
{% endif %}
</body>

Expand Down