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

Added option to specify host address to bind servers to #37

Merged
merged 5 commits into from
Jan 24, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

### 2.3.2 (Next)

* [#36](https://github.com/alexa-js/alexa-app-server/pull/36): Added option to specify host address to bind servers to - [@tejashah88](https://github.com/tejashah88).
* [#35](https://github.com/alexa-js/alexa-app-server/pull/35): Error occurs when `verify` and `debug` are both set to true - [@tejashah88](https://github.com/tejashah88).
* [#34](https://github.com/alexa-js/alexa-app-server/pull/34): Added tests for fail cases and schemas/utterances - [@tejashah88](https://github.com/tejashah88).
* [#34](https://github.com/alexa-js/alexa-app-server/pull/34): Fixed bug while trying to retrieve schema/utterances with GET request - [@tejashah88](https://github.com/tejashah88).
Expand Down
8 changes: 6 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,16 @@ require('alexa-app-server').start({
// root of your web server.
app_root: "/alexa/",

// The directory containing server-side processing modules (see below)
// The directory containing server-side processing modules (see below).
server_dir: "server",

// The port the server should bind to
// The port the server should bind to.
port: 80,

// The host address in which the server should bind to. If not specified,
// this argument will be ignored.
host: undefined,

// By default, GET requests to Alexa App endpoints will show the debugger
// UI. This can be disabled. If 'verify' and 'debug' are both set to true,
// an error will be thrown.
Expand Down
46 changes: 33 additions & 13 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ var appServer = function(config) {
if (config.verify) {
self.express.use(endpoint, alexaVerifierMiddleware());
}

self.express.post(endpoint, function(req, res) {
var json = req.body,
response_json;
Expand Down Expand Up @@ -126,6 +127,7 @@ var appServer = function(config) {
self.error("Error loading app [" + main + "]: " + e);
}
});

return self.apps;
};

Expand All @@ -149,10 +151,17 @@ var appServer = function(config) {

self.start = function() {
// Instantiate up the server
// TODO: add i18n support (i18n-node might be a good look)
// Issue #12: https://github.com/alexa-js/alexa-app-server/issues/12
self.express = express();

// TODO: change this to make sure it doesn't affect other non-Alexa services/apps
// Issue #35: https://github.com/alexa-js/alexa-app-server/issues/35
self.express.use(bodyParser.urlencoded({ extended: true }));

//We need the rawBody for request verification
// We need the rawBody for request verification
// TODO: change code to possibly use bodyParser.json()
// Issue #21: https://github.com/alexa-js/alexa-app-server/issues/21
self.express.use(function(req, res, next) {
// mark the request body as already having been parsed so it's ignored by
// other body parser middlewares
Expand Down Expand Up @@ -208,43 +217,54 @@ var appServer = function(config) {
if (config.httpsEnabled == true) {
self.log("httpsEnabled is true. Reading HTTPS config");

if (config.privateKey != undefined && config.certificate != undefined && config.httpsPort != undefined) { //Ensure that all of the needed properties are set
if (config.privateKey != undefined && config.certificate != undefined && config.httpsPort != undefined) { // Ensure that all of the needed properties are set
var privateKeyFile = server_root + '/sslcert/' + config.privateKey;
var certificateFile = server_root + '/sslcert/' + config.certificate;

if (fs.existsSync(privateKeyFile) && fs.existsSync(certificateFile)) { //Make sure the key and cert exist.
if (fs.existsSync(privateKeyFile) && fs.existsSync(certificateFile)) { // Make sure the key and cert exist.

var privateKey = fs.readFileSync(privateKeyFile, 'utf8');
var certificate = fs.readFileSync(certificateFile, 'utf8');

if (privateKey != undefined && certificate != undefined) {
var credentials = { key: privateKey, cert: certificate };

try { //The line below can fail it the certs were generated incorrectly. But we can continue startup without HTTPS
self.httpsInstance = https.createServer(credentials, self.express).listen(config.httpsPort); //create the HTTPS server
self.log("Listening on HTTPS port " + config.httpsPort);
try { // These two lines below can fail it the certs were generated incorrectly. But we can continue startup without HTTPS
var httpsServer = https.createServer(credentials, self.express); // create the HTTPS server

// TODO: add separate option to specify specific host address for HTTPS server to bind to ???
// Issue #38: https://github.com/alexa-js/alexa-app-server/issues/38
if (typeof config.host === 'string') {
self.httpsInstance = httpsServer.listen(config.httpsPort, config.host);
self.log("Listening on https://" + config.host + ":" + config.httpsPort);
} else {
self.httpsInstance = httpsServer.listen(config.httpsPort);
self.log("Listening on HTTPS port " + config.httpsPort);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This would say "Listening on https://127.0.0.1:port or localhost ...

}
} catch (error) {
self.error("Failed to listen via HTTPS Error: " + error);
}
} else {
self.error("Failed to load privateKey or certificate from /sslcert. HTTPS will not be enabled");

}

} else {
self.error("privateKey: '" + config.privateKey + "' or certificate: '" + config.certificate + "' do not exist in /sslcert. HTTPS will not be enabled");

}
} else {
self.error("privatekey, httpsPort, or certificate paramater not set in config. HTTPS will not be enabled");

}

}

// Start the server listening
config.port = config.port || process.env.port || 80;
self.httpInstance = self.express.listen(config.port);
self.log("Listening on HTTP port " + config.port);

if (typeof config.host === 'string') {
self.httpInstance = self.express.listen(config.port, config.host);
self.log("Listening on http://" + config.host + ":" + config.port);
} else {
self.httpInstance = self.express.listen(config.port);
self.log("Listening on HTTP port " + config.port);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Reading this, are we creating both an http and https endpoint? Shouldn't be just https if that's enabled?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not really, since the initial check already checks if https is enabled, and if so, will then proceed to initialize httpsInstance here. Otherwise, only the httpInstance should contain a server instance.


// Run the post() method if defined
if (typeof config.post == "function") {
Expand Down
46 changes: 46 additions & 0 deletions test/test-examples-server-custom-bindings.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*jshint expr: true*/
"use strict";
var chai = require("chai");
var expect = chai.expect;
chai.config.includeStack = true;
var request = require("supertest-as-promised");
var alexaAppServer = require("../index");

// used so that testing https requests will work properly
process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0';
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'll merge, however it feels like this should be set per-test, or in a beforeEach and then reset back in an afterEach.


describe("Alexa App Server with Examples & Custom Server Bindings", function() {
var testServer;

afterEach(function() {
testServer.stop();
});

it("mounts hello world app (HTTP only) and bind to the specified address", function() {
testServer = alexaAppServer.start({
port: 3000,
host: "127.0.0.1",
server_root: 'examples'
});

return request("http://127.0.0.1:3000")
.get('/alexa/helloworld')
.expect(200);
});

it("mounts hello world app (HTTP & HTTPS) and bind to the specified address", function() {
testServer = alexaAppServer.start({
port: 3000,
host: "127.0.0.1",
server_root: 'examples',
httpsEnabled: true,
httpsPort: 6000,
privateKey: 'private-key.pem',
certificate: 'cert.cer'
});

return request("https://127.0.0.1:6000")
.get('/alexa/helloworld')
.expect(200);
});
});